Merge pull request kubernetes#88146 from gnufied/avoid-multiple-pv-delete

Prevent deletion of PVs that are already deleted

Author: k8s-ci-robot

pkg/controller/volume/persistentvolume/framework_test.go

@@ -348,6 +348,17 @@ func newVolumeArray(name, capacity, boundToClaimUID, boundToClaimName string, ph
 	}
 }
 
+func withVolumeDeletionTimestamp(pvs []*v1.PersistentVolume) []*v1.PersistentVolume {
+	result := []*v1.PersistentVolume{}
+	for _, pv := range pvs {
+		// Using time.Now() here will cause mismatching deletion timestamps in tests
+		deleteTime := metav1.Date(2020, time.February, 18, 10, 30, 30, 10, time.UTC)
+		pv.SetDeletionTimestamp(&deleteTime)
+		result = append(result, pv)
+	}
+	return result
+}
+
 // newClaim returns a new claim with given attributes
 func newClaim(name, claimUID, capacity, boundToVolume string, phase v1.PersistentVolumeClaimPhase, class *string, annotations ...string) *v1.PersistentVolumeClaim {
 	fs := v1.PersistentVolumeFilesystem

pkg/controller/volume/persistentvolume/pv_controller.go

@@ -1183,6 +1183,11 @@ func (ctrl *PersistentVolumeController) deleteVolumeOperation(volume *v1.Persist
 		klog.V(3).Infof("error reading persistent volume %q: %v", volume.Name, err)
 		return "", nil
 	}
+
+	if newVolume.GetDeletionTimestamp() != nil {
+		klog.V(3).Infof("Volume %q is already being deleted", volume.Name)
+		return "", nil
+	}
 	needsReclaim, err := ctrl.isVolumeReleased(newVolume)
 	if err != nil {
 		klog.V(3).Infof("error reading claim for volume %q: %v", volume.Name, err)

pkg/controller/volume/persistentvolume/pv_controller_test.go

@@ -233,6 +233,21 @@ func TestControllerSync(t *testing.T) {
 				return nil
 			},
 		},
+		{
+			// delete success(?) - volume has deletion timestamp before doDelete() starts
+			"8-13 - volume is has deletion timestamp and is not processed",
+			withVolumeDeletionTimestamp(newVolumeArray("volume8-13", "1Gi", "uid8-13", "claim8-13", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty)),
+			withVolumeDeletionTimestamp(newVolumeArray("volume8-13", "1Gi", "uid8-13", "claim8-13", v1.VolumeReleased, v1.PersistentVolumeReclaimDelete, classEmpty)),
+			noclaims,
+			noclaims,
+			noevents, noerrors,
+			// We don't need to do anything in test function because deletion will be noticed automatically and synced.
+			// Attempting to use testSyncVolume here will cause an error because of race condition between manually
+			// calling testSyncVolume and volume loop running.
+			func(ctrl *PersistentVolumeController, reactor *pvtesting.VolumeReactor, test controllerTest) error {
+				return nil
+			},
+		},
 	}
 
 	for _, test := range tests {