Unverified Commit d655c9a8 authored by Kubernetes Submit Queue's avatar Kubernetes Submit Queue Committed by GitHub

Merge pull request #66923 from jarrpa/pvc-protect-until-pod-delete

Automatic merge from submit-queue (batch tested with PRs 66491, 66587, 66856, 66657, 66923). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>. 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. ```release-note NONE ``` Resolves: #65552 Signed-off-by: 's avatarJose A. Rivera <jarrpa@redhat.com>
parents 4ff782df 13462bf3
...@@ -221,11 +221,6 @@ func (c *Controller) isBeingUsed(pvc *v1.PersistentVolumeClaim) (bool, error) { ...@@ -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) glog.V(4).Infof("Skipping unscheduled pod %s when checking PVC %s/%s", pod.Name, pvc.Namespace, pvc.Name)
continue 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 { for _, volume := range pod.Spec.Volumes {
if volume.PersistentVolumeClaim == nil { if volume.PersistentVolumeClaim == nil {
continue continue
......
...@@ -264,14 +264,12 @@ func TestPVCProtectionController(t *testing.T) { ...@@ -264,14 +264,12 @@ func TestPVCProtectionController(t *testing.T) {
storageObjectInUseProtectionEnabled: true, 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{ initialObjects: []runtime.Object{
withStatus(v1.PodFailed, withPVC(defaultPVCName, pod())), withStatus(v1.PodFailed, withPVC(defaultPVCName, pod())),
}, },
updatedPVC: deleted(withProtectionFinalizer(pvc())), updatedPVC: deleted(withProtectionFinalizer(pvc())),
expectedActions: []clienttesting.Action{ expectedActions: []clienttesting.Action{},
clienttesting.NewUpdateAction(pvcVer, defaultNS, deleted(pvc())),
},
storageObjectInUseProtectionEnabled: true, storageObjectInUseProtectionEnabled: true,
}, },
// //
...@@ -287,14 +285,12 @@ func TestPVCProtectionController(t *testing.T) { ...@@ -287,14 +285,12 @@ func TestPVCProtectionController(t *testing.T) {
storageObjectInUseProtectionEnabled: true, storageObjectInUseProtectionEnabled: true,
}, },
{ {
name: "updated finished Pod -> finalizer is removed", name: "updated finished Pod -> finalizer is not removed",
initialObjects: []runtime.Object{ initialObjects: []runtime.Object{
deleted(withProtectionFinalizer(pvc())), deleted(withProtectionFinalizer(pvc())),
}, },
updatedPod: withStatus(v1.PodSucceeded, withPVC(defaultPVCName, pod())), updatedPod: withStatus(v1.PodSucceeded, withPVC(defaultPVCName, pod())),
expectedActions: []clienttesting.Action{ expectedActions: []clienttesting.Action{},
clienttesting.NewUpdateAction(pvcVer, defaultNS, deleted(pvc())),
},
storageObjectInUseProtectionEnabled: true, storageObjectInUseProtectionEnabled: true,
}, },
{ {
......
...@@ -283,6 +283,7 @@ var _ = utils.SIGDescribe("PersistentVolumes", func() { ...@@ -283,6 +283,7 @@ var _ = utils.SIGDescribe("PersistentVolumes", func() {
framework.ExpectNoError(framework.WaitForPodSuccessInNamespace(c, pod.Name, ns)) framework.ExpectNoError(framework.WaitForPodSuccessInNamespace(c, pod.Name, ns))
By("Deleting the claim") By("Deleting the claim")
framework.ExpectNoError(framework.DeletePodWithWait(f, c, pod))
framework.ExpectNoError(framework.DeletePVCandValidatePV(c, ns, pvc, pv, v1.VolumeAvailable)) framework.ExpectNoError(framework.DeletePVCandValidatePV(c, ns, pvc, pv, v1.VolumeAvailable))
By("Re-mounting the volume.") By("Re-mounting the volume.")
...@@ -298,6 +299,7 @@ var _ = utils.SIGDescribe("PersistentVolumes", func() { ...@@ -298,6 +299,7 @@ var _ = utils.SIGDescribe("PersistentVolumes", func() {
pod, err = c.CoreV1().Pods(ns).Create(pod) pod, err = c.CoreV1().Pods(ns).Create(pod)
Expect(err).NotTo(HaveOccurred()) Expect(err).NotTo(HaveOccurred())
framework.ExpectNoError(framework.WaitForPodSuccessInNamespace(c, pod.Name, ns)) 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.") framework.Logf("Pod exited without failure; the volume has been recycled.")
}) })
}) })
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment