From 5ff42b23687584eff6e7baf7921f77bb328f1844 Mon Sep 17 00:00:00 2001 From: carlory Date: Fri, 24 Nov 2023 14:54:51 +0800 Subject: [PATCH] fix issue with using feature HonorPVReclaimPolicy in csi-provisioner --- .../volume/persistentvolume/delete_test.go | 28 +++++++++++++++++-- .../volume/persistentvolume/pv_controller.go | 12 ++++++++ 2 files changed, 37 insertions(+), 3 deletions(-) diff --git a/pkg/controller/volume/persistentvolume/delete_test.go b/pkg/controller/volume/persistentvolume/delete_test.go index 3fc269cb63c..04de3024594 100644 --- a/pkg/controller/volume/persistentvolume/delete_test.go +++ b/pkg/controller/volume/persistentvolume/delete_test.go @@ -148,9 +148,9 @@ func TestDeleteSync(t *testing.T) { }, { // PV requires external deleter - name: "8-10 - external deleter", - initialVolumes: []*v1.PersistentVolume{newExternalProvisionedVolume("volume8-10", "1Gi", "uid10-1", "claim10-1", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty, gceDriver, nil, volume.AnnBoundByController)}, - expectedVolumes: []*v1.PersistentVolume{newExternalProvisionedVolume("volume8-10", "1Gi", "uid10-1", "claim10-1", v1.VolumeReleased, v1.PersistentVolumeReclaimDelete, classEmpty, gceDriver, nil, volume.AnnBoundByController)}, + name: "8-10-1 - external deleter when volume is dynamic provisioning", + initialVolumes: []*v1.PersistentVolume{newExternalProvisionedVolume("volume8-10-1", "1Gi", "uid10-1-1", "claim10-1-1", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty, gceDriver, nil, volume.AnnBoundByController)}, + expectedVolumes: []*v1.PersistentVolume{newExternalProvisionedVolume("volume8-10-1", "1Gi", "uid10-1-1", "claim10-1-1", v1.VolumeReleased, v1.PersistentVolumeReclaimDelete, classEmpty, gceDriver, nil, volume.AnnBoundByController)}, initialClaims: noclaims, expectedClaims: noclaims, expectedEvents: noevents, @@ -162,6 +162,28 @@ func TestDeleteSync(t *testing.T) { return testSyncVolume(ctrl, reactor, test) }, }, + { + // PV requires external deleter + name: "8-10-2 - external deleter when volume is static provisioning", + initialVolumes: []*v1.PersistentVolume{newExternalProvisionedVolume("volume8-10-2", "1Gi", "uid10-1-2", "claim10-1-2", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty, gceDriver, nil, volume.AnnBoundByController)}, + expectedVolumes: []*v1.PersistentVolume{newExternalProvisionedVolume("volume8-10-2", "1Gi", "uid10-1-2", "claim10-1-2", v1.VolumeReleased, v1.PersistentVolumeReclaimDelete, classEmpty, gceDriver, nil, volume.AnnBoundByController)}, + initialClaims: noclaims, + expectedClaims: noclaims, + expectedEvents: noevents, + errors: noerrors, + test: testSyncVolume, + }, + { + // PV requires external deleter + name: "8-10-3 - external deleter when volume is migrated", + initialVolumes: []*v1.PersistentVolume{volumeWithAnnotation(volume.AnnMigratedTo, "pd.csi.storage.gke.io", volumeWithAnnotation(volume.AnnDynamicallyProvisioned, "kubernetes.io/gce-pd", newVolume("volume8-10-3", "1Gi", "uid10-1-3", "claim10-1-3", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty, volume.AnnDynamicallyProvisioned)))}, + expectedVolumes: []*v1.PersistentVolume{volumeWithAnnotation(volume.AnnMigratedTo, "pd.csi.storage.gke.io", volumeWithAnnotation(volume.AnnDynamicallyProvisioned, "kubernetes.io/gce-pd", newVolume("volume8-10-3", "1Gi", "uid10-1-3", "claim10-1-3", v1.VolumeReleased, v1.PersistentVolumeReclaimDelete, classEmpty, volume.AnnDynamicallyProvisioned)))}, + initialClaims: noclaims, + expectedClaims: noclaims, + expectedEvents: noevents, + errors: noerrors, + test: testSyncVolume, + }, { // delete success - two PVs are provisioned for a single claim. // One of the PVs is deleted. diff --git a/pkg/controller/volume/persistentvolume/pv_controller.go b/pkg/controller/volume/persistentvolume/pv_controller.go index 2002c62db4f..3b3c3aee1ae 100644 --- a/pkg/controller/volume/persistentvolume/pv_controller.go +++ b/pkg/controller/volume/persistentvolume/pv_controller.go @@ -1952,6 +1952,18 @@ func (ctrl *PersistentVolumeController) findDeletablePlugin(volume *v1.Persisten } } + if utilfeature.DefaultFeatureGate.Enabled(features.HonorPVReclaimPolicy) { + if metav1.HasAnnotation(volume.ObjectMeta, storagehelpers.AnnMigratedTo) { + // CSI migration scenario - do not depend on in-tree plugin + return nil, nil + } + + if volume.Spec.CSI != nil { + // CSI volume source scenario - external provisioner is requested + return nil, nil + } + } + // The plugin that provisioned the volume was not found or the volume // was not dynamically provisioned. Try to find a plugin by spec. spec := vol.NewSpecFromPersistentVolume(volume, false)