From 76755034a168c3ebd0dba57944847cb443c03d29 Mon Sep 17 00:00:00 2001 From: Jan Safranek Date: Tue, 15 Nov 2016 17:22:32 +0100 Subject: [PATCH] Fix recycler pod deletion race. We should use clone of recycler pod template instead of reusing the same one for two or more recyclers running in parallel. Also add some logs to relevant places to spot the error easily next time. --- pkg/volume/host_path/BUILD | 1 + pkg/volume/host_path/host_path.go | 7 ++++++- pkg/volume/nfs/BUILD | 1 + pkg/volume/nfs/nfs.go | 7 ++++++- pkg/volume/util.go | 8 +++++++- 5 files changed, 21 insertions(+), 3 deletions(-) diff --git a/pkg/volume/host_path/BUILD b/pkg/volume/host_path/BUILD index 7fc20f5d5a7..f4ec83fad93 100644 --- a/pkg/volume/host_path/BUILD +++ b/pkg/volume/host_path/BUILD @@ -19,6 +19,7 @@ go_library( tags = ["automanaged"], deps = [ "//pkg/api:go_default_library", + "//pkg/conversion:go_default_library", "//pkg/types:go_default_library", "//pkg/util/uuid:go_default_library", "//pkg/volume:go_default_library", diff --git a/pkg/volume/host_path/host_path.go b/pkg/volume/host_path/host_path.go index cc14bcb3c6f..cc2c5e1c0f8 100644 --- a/pkg/volume/host_path/host_path.go +++ b/pkg/volume/host_path/host_path.go @@ -22,6 +22,7 @@ import ( "regexp" "k8s.io/kubernetes/pkg/api" + "k8s.io/kubernetes/pkg/conversion" "k8s.io/kubernetes/pkg/types" "k8s.io/kubernetes/pkg/util/uuid" "k8s.io/kubernetes/pkg/volume" @@ -244,7 +245,11 @@ func (r *hostPathRecycler) GetPath() string { // Recycle blocks until the pod has completed or any error occurs. // HostPath recycling only works in single node clusters and is meant for testing purposes only. func (r *hostPathRecycler) Recycle() error { - pod := r.config.RecyclerPodTemplate + templateClone, err := conversion.NewCloner().DeepCopy(r.config.RecyclerPodTemplate) + if err != nil { + return err + } + pod := templateClone.(*api.Pod) // overrides pod.Spec.ActiveDeadlineSeconds = &r.timeout pod.Spec.Volumes[0].VolumeSource = api.VolumeSource{ diff --git a/pkg/volume/nfs/BUILD b/pkg/volume/nfs/BUILD index a1b2b1d9671..67e1e5c4525 100644 --- a/pkg/volume/nfs/BUILD +++ b/pkg/volume/nfs/BUILD @@ -19,6 +19,7 @@ go_library( tags = ["automanaged"], deps = [ "//pkg/api:go_default_library", + "//pkg/conversion:go_default_library", "//pkg/types:go_default_library", "//pkg/util/exec:go_default_library", "//pkg/util/mount:go_default_library", diff --git a/pkg/volume/nfs/nfs.go b/pkg/volume/nfs/nfs.go index abacb8cb46d..25d3cb58d76 100644 --- a/pkg/volume/nfs/nfs.go +++ b/pkg/volume/nfs/nfs.go @@ -23,6 +23,7 @@ import ( "github.com/golang/glog" "k8s.io/kubernetes/pkg/api" + "k8s.io/kubernetes/pkg/conversion" "k8s.io/kubernetes/pkg/types" "k8s.io/kubernetes/pkg/util/exec" "k8s.io/kubernetes/pkg/util/mount" @@ -332,7 +333,11 @@ func (r *nfsRecycler) GetPath() string { // Recycle recycles/scrubs clean an NFS volume. // Recycle blocks until the pod has completed or any error occurs. func (r *nfsRecycler) Recycle() error { - pod := r.config.RecyclerPodTemplate + templateClone, err := conversion.NewCloner().DeepCopy(r.config.RecyclerPodTemplate) + if err != nil { + return err + } + pod := templateClone.(*api.Pod) // overrides pod.Spec.ActiveDeadlineSeconds = &r.timeout pod.GenerateName = "pv-recycler-nfs-" diff --git a/pkg/volume/util.go b/pkg/volume/util.go index a5ff196114d..9b53518f2cb 100644 --- a/pkg/volume/util.go +++ b/pkg/volume/util.go @@ -83,7 +83,13 @@ func internalRecycleVolumeByWatchingPodUntilCompletion(pvName string, pod *api.P return fmt.Errorf("unexpected error creating recycler pod: %+v\n", err) } } - defer recyclerClient.DeletePod(pod.Name, pod.Namespace) + defer func(pod *api.Pod) { + if err := recyclerClient.DeletePod(pod.Name, pod.Namespace); err != nil { + glog.Errorf("failed to delete recycler pod %s/%s: %v", pod.Namespace, pod.Name, err) + } else { + glog.V(4).Infof("deleted recycler pod %s/%s", pod.Namespace, pod.Name) + } + }(pod) // Now only the old pod or the new pod run. Watch it until it finishes // and send all events on the pod to the PV