Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent deletion of PVs that are already deleted #88146

Merged
merged 1 commit into from Feb 19, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
11 changes: 11 additions & 0 deletions pkg/controller/volume/persistentvolume/framework_test.go
Expand Up @@ -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
Expand Down
5 changes: 5 additions & 0 deletions pkg/controller/volume/persistentvolume/pv_controller.go
Expand Up @@ -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)
Expand Down
15 changes: 15 additions & 0 deletions pkg/controller/volume/persistentvolume/pv_controller_test.go
Expand Up @@ -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 {
Expand Down