From 13462bf34113ed3000aaf6dbe5eb66198214a3d6 Mon Sep 17 00:00:00 2001 From: "Jose A. Rivera" Date: Thu, 2 Aug 2018 12:48:50 -0500 Subject: [PATCH] PVC Protection: Wait for Pod delete Currently, the PVC protection controller will remove its finalizer when all Pods using a PVC reach at least a Terminating state. However, certain volumes cannot be guaranteed to be umounted until a Pod is deleted. Only Pods not in the current pods list can be considered deleted, so we're removing the exception to not check Terminating Pods. Signed-off-by: Jose A. Rivera --- .../pvcprotection/pvc_protection_controller.go | 5 ----- .../pvc_protection_controller_test.go | 16 ++++++---------- test/e2e/storage/persistent_volumes.go | 2 ++ 3 files changed, 8 insertions(+), 15 deletions(-) diff --git a/pkg/controller/volume/pvcprotection/pvc_protection_controller.go b/pkg/controller/volume/pvcprotection/pvc_protection_controller.go index 8ff28cf6413..cfa7658e6e7 100644 --- a/pkg/controller/volume/pvcprotection/pvc_protection_controller.go +++ b/pkg/controller/volume/pvcprotection/pvc_protection_controller.go @@ -221,11 +221,6 @@ func (c *Controller) isBeingUsed(pvc *v1.PersistentVolumeClaim) (bool, error) { glog.V(4).Infof("Skipping unscheduled pod %s when checking PVC %s/%s", pod.Name, pvc.Namespace, pvc.Name) continue } - if volumeutil.IsPodTerminated(pod, pod.Status) { - // This pod is being unmounted/detached or is already - // unmounted/detached. It does not block the PVC from deletion. - continue - } for _, volume := range pod.Spec.Volumes { if volume.PersistentVolumeClaim == nil { continue diff --git a/pkg/controller/volume/pvcprotection/pvc_protection_controller_test.go b/pkg/controller/volume/pvcprotection/pvc_protection_controller_test.go index 531b6fbea74..288be8f20bf 100644 --- a/pkg/controller/volume/pvcprotection/pvc_protection_controller_test.go +++ b/pkg/controller/volume/pvcprotection/pvc_protection_controller_test.go @@ -264,14 +264,12 @@ func TestPVCProtectionController(t *testing.T) { storageObjectInUseProtectionEnabled: true, }, { - name: "deleted PVC with finalizer + pods with the PVC and is finished -> finalizer is removed", + name: "deleted PVC with finalizer + pods with the PVC finished but is not deleted -> finalizer is not removed", initialObjects: []runtime.Object{ withStatus(v1.PodFailed, withPVC(defaultPVCName, pod())), }, - updatedPVC: deleted(withProtectionFinalizer(pvc())), - expectedActions: []clienttesting.Action{ - clienttesting.NewUpdateAction(pvcVer, defaultNS, deleted(pvc())), - }, + updatedPVC: deleted(withProtectionFinalizer(pvc())), + expectedActions: []clienttesting.Action{}, storageObjectInUseProtectionEnabled: true, }, // @@ -287,14 +285,12 @@ func TestPVCProtectionController(t *testing.T) { storageObjectInUseProtectionEnabled: true, }, { - name: "updated finished Pod -> finalizer is removed", + name: "updated finished Pod -> finalizer is not removed", initialObjects: []runtime.Object{ deleted(withProtectionFinalizer(pvc())), }, - updatedPod: withStatus(v1.PodSucceeded, withPVC(defaultPVCName, pod())), - expectedActions: []clienttesting.Action{ - clienttesting.NewUpdateAction(pvcVer, defaultNS, deleted(pvc())), - }, + updatedPod: withStatus(v1.PodSucceeded, withPVC(defaultPVCName, pod())), + expectedActions: []clienttesting.Action{}, storageObjectInUseProtectionEnabled: true, }, { diff --git a/test/e2e/storage/persistent_volumes.go b/test/e2e/storage/persistent_volumes.go index b9dea5813dc..35342d4390f 100644 --- a/test/e2e/storage/persistent_volumes.go +++ b/test/e2e/storage/persistent_volumes.go @@ -283,6 +283,7 @@ var _ = utils.SIGDescribe("PersistentVolumes", func() { framework.ExpectNoError(framework.WaitForPodSuccessInNamespace(c, pod.Name, ns)) By("Deleting the claim") + framework.ExpectNoError(framework.DeletePodWithWait(f, c, pod)) framework.ExpectNoError(framework.DeletePVCandValidatePV(c, ns, pvc, pv, v1.VolumeAvailable)) By("Re-mounting the volume.") @@ -298,6 +299,7 @@ var _ = utils.SIGDescribe("PersistentVolumes", func() { pod, err = c.CoreV1().Pods(ns).Create(pod) Expect(err).NotTo(HaveOccurred()) framework.ExpectNoError(framework.WaitForPodSuccessInNamespace(c, pod.Name, ns)) + framework.ExpectNoError(framework.DeletePodWithWait(f, c, pod)) framework.Logf("Pod exited without failure; the volume has been recycled.") }) })