github/kubernetes
1
0
Fork 0
mirror of https://github.com/k3s-io/kubernetes.git synced 2025-07-22 03:11:40 +00:00
Code Issues Projects Releases Wiki Activity

Merge pull request #109205 from deepakkinni/fix_fin_v1

Browse Source 
Add or Remove PV deletion protection finalizer based on PV recalimPolicy
This commit is contained in:
Kubernetes Prow Robot 2022-04-07 02:57:57 -07:00 committed by GitHub
commit df1a3ddc98
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 226 additions and 113 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

18
pkg/controller/volume/persistentvolume/delete_test.go
View File

@ -18,6 +18,9 @@ package persistentvolume
import (
"errors"
utilfeature "k8s.io/apiserver/pkg/util/feature"
featuregatetesting "k8s.io/component-base/featuregate/testing"
"k8s.io/kubernetes/pkg/features"
"testing"
v1 "k8s.io/api/core/v1"
@ -31,6 +34,9 @@ import (
// 2. Call the syncVolume *once*.
// 3. Compare resulting volumes with expected volumes.
func TestDeleteSync(t *testing.T) {
const gceDriver = "pd.csi.storage.gke.io"
// Default enable the HonorPVReclaimPolicy feature gate.
defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.HonorPVReclaimPolicy, true)()
tests := []controllerTest{
{
// delete volume bound by controller
@ -132,8 +138,8 @@ func TestDeleteSync(t *testing.T) {
{
// PV requires external deleter
"8-10 - external deleter",
newVolumeArray("volume8-10", "1Gi", "uid10-1", "claim10-1", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty, volume.AnnBoundByController),
newVolumeArray("volume8-10", "1Gi", "uid10-1", "claim10-1", v1.VolumeReleased, v1.PersistentVolumeReclaimDelete, classEmpty, volume.AnnBoundByController),
[]*v1.PersistentVolume{newExternalProvisionedVolume("volume8-10", "1Gi", "uid10-1", "claim10-1", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty, gceDriver, nil, volume.AnnBoundByController)},
[]*v1.PersistentVolume{newExternalProvisionedVolume("volume8-10", "1Gi", "uid10-1", "claim10-1", v1.VolumeReleased, v1.PersistentVolumeReclaimDelete, classEmpty, gceDriver, nil, volume.AnnBoundByController)},
noclaims,
noclaims,
noevents, noerrors,
@ -169,12 +175,12 @@ func TestDeleteSync(t *testing.T) {
// external provisioner.
"8-12 - two PVs externally provisioned for a single claim",
[]*v1.PersistentVolume{
newVolume("volume8-12-1", "1Gi", "uid8-12", "claim8-12", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty, volume.AnnDynamicallyProvisioned),
newVolume("volume8-12-2", "1Gi", "uid8-12", "claim8-12", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty, volume.AnnDynamicallyProvisioned),
newExternalProvisionedVolume("volume8-12-1", "1Gi", "uid8-12", "claim8-12", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty, gceDriver, nil, volume.AnnDynamicallyProvisioned),
newExternalProvisionedVolume("volume8-12-2", "1Gi", "uid8-12", "claim8-12", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty, gceDriver, nil, volume.AnnDynamicallyProvisioned),
},
[]*v1.PersistentVolume{
newVolume("volume8-12-1", "1Gi", "uid8-12", "claim8-12", v1.VolumeReleased, v1.PersistentVolumeReclaimDelete, classEmpty, volume.AnnDynamicallyProvisioned),
newVolume("volume8-12-2", "1Gi", "uid8-12", "claim8-12", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty, volume.AnnDynamicallyProvisioned),
newExternalProvisionedVolume("volume8-12-1", "1Gi", "uid8-12", "claim8-12", v1.VolumeReleased, v1.PersistentVolumeReclaimDelete, classEmpty, gceDriver, nil, volume.AnnDynamicallyProvisioned),
newExternalProvisionedVolume("volume8-12-2", "1Gi", "uid8-12", "claim8-12", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty, gceDriver, nil, volume.AnnDynamicallyProvisioned),
},
// the claim is bound to volume8-12-2 -> volume8-12-1 has lost the race and will be "Released"
newClaimArray("claim8-12", "uid8-12", "10Gi", "volume8-12-2", v1.ClaimBound, nil),

67
pkg/controller/volume/persistentvolume/framework_test.go
View File

@ -254,7 +254,6 @@ func newVolume(name, capacity, boundToClaimUID, boundToClaimName string, phase v
ObjectMeta: metav1.ObjectMeta{
Name: name,
ResourceVersion: "1",
Finalizers: []string{storagehelpers.PVDeletionInTreeProtectionFinalizer},
},
Spec: v1.PersistentVolumeSpec{
Capacity: v1.ResourceList{
@ -298,6 +297,72 @@ func newVolume(name, capacity, boundToClaimUID, boundToClaimName string, phase v
return &volume
}
// newExternalProvisionedVolume returns a new volume with given attributes
func newExternalProvisionedVolume(name, capacity, boundToClaimUID, boundToClaimName string, phase v1.PersistentVolumePhase, reclaimPolicy v1.PersistentVolumeReclaimPolicy, class string, driverName string, finalizers []string, annotations ...string) *v1.PersistentVolume {
fs := v1.PersistentVolumeFilesystem
volume := v1.PersistentVolume{
ObjectMeta: metav1.ObjectMeta{
Name: name,
ResourceVersion: "1",
Finalizers: finalizers,
},
Spec: v1.PersistentVolumeSpec{
Capacity: v1.ResourceList{
v1.ResourceName(v1.ResourceStorage): resource.MustParse(capacity),
},
PersistentVolumeSource: v1.PersistentVolumeSource{
CSI: &v1.CSIPersistentVolumeSource{
Driver: driverName,
VolumeHandle: "527b55dc-c7db-4574-9226-2e33318b06a3",
ReadOnly: false,
FSType: "ext4",
VolumeAttributes: map[string]string{
"Test-Key": "Test-Value",
},
},
},
AccessModes: []v1.PersistentVolumeAccessMode{v1.ReadWriteOnce, v1.ReadOnlyMany},
PersistentVolumeReclaimPolicy: reclaimPolicy,
StorageClassName: class,
VolumeMode: &fs,
},
Status: v1.PersistentVolumeStatus{
Phase: phase,
},
}
if boundToClaimName != "" {
volume.Spec.ClaimRef = &v1.ObjectReference{
Kind: "PersistentVolumeClaim",
APIVersion: "v1",
UID: types.UID(boundToClaimUID),
Namespace: testNamespace,
Name: boundToClaimName,
}
}
if len(annotations) > 0 {
volume.Annotations = make(map[string]string)
for _, a := range annotations {
switch a {
case storagehelpers.AnnDynamicallyProvisioned:
volume.Annotations[a] = driverName
default:
volume.Annotations[a] = "yes"
}
}
}
return &volume
}
// newVolume returns a new volume with given attributes
func newVolumeWithFinalizers(name, capacity, boundToClaimUID, boundToClaimName string, phase v1.PersistentVolumePhase, reclaimPolicy v1.PersistentVolumeReclaimPolicy, class string, finalizers []string, annotations ...string) *v1.PersistentVolume {
retVolume := newVolume(name, capacity, boundToClaimUID, boundToClaimName, phase, reclaimPolicy, class, annotations...)
retVolume.SetFinalizers(finalizers)
return retVolume
}
// withLabels applies the given labels to the first volume in the array and
// returns the array. Meant to be used to compose volumes specified inline in
// a test.

9
pkg/controller/volume/persistentvolume/provision_test.go
View File

@ -19,6 +19,9 @@ package persistentvolume
import (
"context"
"errors"
utilfeature "k8s.io/apiserver/pkg/util/feature"
featuregatetesting "k8s.io/component-base/featuregate/testing"
"k8s.io/kubernetes/pkg/features"
"testing"
v1 "k8s.io/api/core/v1"
@ -167,6 +170,8 @@ var provision2Success = provisionCall{
// 2. Call the syncVolume *once*.
// 3. Compare resulting volumes with expected volumes.
func TestProvisionSync(t *testing.T) {
// Default enable the HonorPVReclaimPolicy feature gate.
defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.HonorPVReclaimPolicy, true)()
tests := []controllerTest{
{
// Provision a volume (with a default class)
@ -493,8 +498,8 @@ func TestProvisionSync(t *testing.T) {
"11-23 - skip finding PV and provision for PVC annotated with AnnSelectedNode",
newVolumeArray("volume11-23", "1Gi", "", "", v1.VolumeAvailable, v1.PersistentVolumeReclaimDelete, classCopper),
[]*v1.PersistentVolume{
newVolume("volume11-23", "1Gi", "", "", v1.VolumeAvailable, v1.PersistentVolumeReclaimDelete, classCopper),
newVolume("pvc-uid11-23", "1Gi", "uid11-23", "claim11-23", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classCopper, volume.AnnDynamicallyProvisioned, volume.AnnBoundByController),
newVolumeWithFinalizers("volume11-23", "1Gi", "", "", v1.VolumeAvailable, v1.PersistentVolumeReclaimDelete, classCopper, nil /*No Finalizer is added here since the test doesn't trigger syncVolume, instead just syncClaim*/),
newVolumeWithFinalizers("pvc-uid11-23", "1Gi", "uid11-23", "claim11-23", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classCopper, []string{volume.PVDeletionInTreeProtectionFinalizer}, volume.AnnDynamicallyProvisioned, volume.AnnBoundByController),
},
claimWithAnnotation(volume.AnnSelectedNode, "node1",
newClaimArray("claim11-23", "uid11-23", "1Gi", "", v1.ClaimPending, &classCopper)),

9
pkg/controller/volume/persistentvolume/pv_controller.go
View File

@ -1459,11 +1459,12 @@ func (ctrl *PersistentVolumeController) removeDeletionProtectionFinalizer(ctx co
var err error
pvUpdateNeeded := false
// Retrieve latest version
volume, err = ctrl.kubeClient.CoreV1().PersistentVolumes().Get(context.TODO(), volume.Name, metav1.GetOptions{})
newVolume, err := ctrl.kubeClient.CoreV1().PersistentVolumes().Get(ctx, volume.Name, metav1.GetOptions{})
if err != nil {
klog.Errorf("error reading persistent volume %q: %v", volume.Name, err)
return err
}
volume = newVolume
volumeClone := volume.DeepCopy()
pvFinalizers := volumeClone.Finalizers
if pvFinalizers != nil && slice.ContainsString(pvFinalizers, storagehelpers.PVDeletionInTreeProtectionFinalizer, nil) {
@ -1661,8 +1662,10 @@ func (ctrl *PersistentVolumeController) provisionClaimOperation(
metav1.SetMetaDataAnnotation(&volume.ObjectMeta, storagehelpers.AnnDynamicallyProvisioned, plugin.GetPluginName())
if utilfeature.DefaultFeatureGate.Enabled(features.HonorPVReclaimPolicy) {
// Add finalizer here
volume.SetFinalizers([]string{storagehelpers.PVDeletionInTreeProtectionFinalizer})
if volume.Spec.PersistentVolumeReclaimPolicy == v1.PersistentVolumeReclaimDelete {
// Add In-Tree protection finalizer here only when the reclaim policy is `Delete`
volume.SetFinalizers([]string{storagehelpers.PVDeletionInTreeProtectionFinalizer})
}
}
// Try to create the PV object several times

118
pkg/controller/volume/persistentvolume/pv_controller_base.go
View File

@ -20,6 +20,7 @@ import (
"context"
"fmt"
"strconv"
"strings"
"time"
v1 "k8s.io/api/core/v1"
@ -334,7 +335,7 @@ func (ctrl *PersistentVolumeController) updateClaimMigrationAnnotations(ctx cont
// when no modifications are required this function could sometimes return a
// copy of the volume and sometimes return a ref to the original
claimClone := claim.DeepCopy()
modified := updateMigrationAnnotationsAndFinalizers(ctrl.csiMigratedPluginManager, ctrl.translator, claimClone.Annotations, nil, true)
modified := updateMigrationAnnotations(ctrl.csiMigratedPluginManager, ctrl.translator, claimClone.Annotations, true)
if !modified {
return claimClone, nil
}
@ -352,11 +353,14 @@ func (ctrl *PersistentVolumeController) updateClaimMigrationAnnotations(ctx cont
func (ctrl *PersistentVolumeController) updateVolumeMigrationAnnotationsAndFinalizers(ctx context.Context,
volume *v1.PersistentVolume) (*v1.PersistentVolume, error) {
volumeClone := volume.DeepCopy()
modified := updateMigrationAnnotationsAndFinalizers(ctrl.csiMigratedPluginManager, ctrl.translator,
volumeClone.Annotations, &volumeClone.Finalizers, false)
if !modified {
annModified := updateMigrationAnnotations(ctrl.csiMigratedPluginManager, ctrl.translator, volumeClone.Annotations, false)
modifiedFinalizers, finalizersModified := modifyDeletionFinalizers(ctrl.csiMigratedPluginManager, volumeClone)
if !annModified && !finalizersModified {
return volumeClone, nil
}
if finalizersModified {
volumeClone.ObjectMeta.SetFinalizers(modifiedFinalizers)
}
newVol, err := ctrl.kubeClient.CoreV1().PersistentVolumes().Update(ctx, volumeClone, metav1.UpdateOptions{})
if err != nil {
return nil, fmt.Errorf("persistent Volume Controller can't anneal migration annotations or finalizer: %v", err)
@ -368,17 +372,68 @@ func (ctrl *PersistentVolumeController) updateVolumeMigrationAnnotationsAndFinal
return newVol, nil
}
// updateMigrationAnnotationsAndFinalizers takes an Annotations map and finalizers slice and checks for a
// provisioner name depending if the annotation came from a PVC or not.
// It will then add a "pv.kubernetes.io/migrated-to" annotation if migration with
// the CSI driver name for that provisioner is "on" based on feature flags, it will also
// remove the PV deletion protection finalizer and "pv.kubernetes.io/migrated-to" annotation
// if migration is "off" for that provisioner in rollback scenarios. Returns true if the annotations
// map or finalizers were modified and false otherwise.
// Parameters:
// - claim: true means the ann came from a PVC, false means the ann came from a PV
func updateMigrationAnnotationsAndFinalizers(cmpm CSIMigratedPluginManager, translator CSINameTranslator,
ann map[string]string, finalizers *[]string, claim bool) bool {
// modifyDeletionFinalizers updates the finalizers based on the reclaim policy and if it is a in-tree volume or not.
// The in-tree PV deletion protection finalizer is only added if the reclaimPolicy associated with the PV is `Delete`.
// The in-tree PV deletion protection finalizer is removed if the reclaimPolicy associated with the PV is `Retain` or
// `Recycle`, removing the finalizer is necessary to reflect the recalimPolicy updates on the PV.
// The method also removes any external PV Deletion Protection finalizers added on the PV, this represents CSI migration
// rollback/disable scenarios.
func modifyDeletionFinalizers(cmpm CSIMigratedPluginManager, volume *v1.PersistentVolume) ([]string, bool) {
modified := false
var outFinalizers []string
if !utilfeature.DefaultFeatureGate.Enabled(features.HonorPVReclaimPolicy) {
return volume.Finalizers, false
}
if !metav1.HasAnnotation(volume.ObjectMeta, storagehelpers.AnnDynamicallyProvisioned) {
// PV deletion protection finalizer is currently supported only for dynamically
// provisioned volumes.
return volume.Finalizers, false
}
if volume.Finalizers != nil {
outFinalizers = append(outFinalizers, volume.Finalizers...)
}
provisioner := volume.Annotations[storagehelpers.AnnDynamicallyProvisioned]
if cmpm.IsMigrationEnabledForPlugin(provisioner) {
// Remove in-tree delete finalizer on the PV as migration is enabled.
if slice.ContainsString(outFinalizers, storagehelpers.PVDeletionInTreeProtectionFinalizer, nil) {
outFinalizers = slice.RemoveString(outFinalizers, storagehelpers.PVDeletionInTreeProtectionFinalizer, nil)
modified = true
}
return outFinalizers, modified
}
// Check if it is a in-tree volume.
if !strings.HasPrefix(provisioner, "kubernetes.io/") {
// The provision plugin does not begin with known in-tree plugin volume prefix annotation.
return volume.Finalizers, false
}
reclaimPolicy := volume.Spec.PersistentVolumeReclaimPolicy
// Add back the in-tree PV deletion protection finalizer if does not already exists
if reclaimPolicy == v1.PersistentVolumeReclaimDelete && !slice.ContainsString(outFinalizers, storagehelpers.PVDeletionInTreeProtectionFinalizer, nil) {
klog.V(4).Infof("Adding in-tree pv deletion protection finalizer on %s", volume.Name)
outFinalizers = append(outFinalizers, storagehelpers.PVDeletionInTreeProtectionFinalizer)
modified = true
} else if (reclaimPolicy == v1.PersistentVolumeReclaimRetain || reclaimPolicy == v1.PersistentVolumeReclaimRecycle) && slice.ContainsString(outFinalizers, storagehelpers.PVDeletionInTreeProtectionFinalizer, nil) {
// Remove the in-tree PV deletion protection finalizer if the reclaim policy is 'Retain' or 'Recycle'
klog.V(4).Infof("Removing in-tree pv deletion protection finalizer on %s", volume.Name)
outFinalizers = slice.RemoveString(outFinalizers, storagehelpers.PVDeletionInTreeProtectionFinalizer, nil)
modified = true
}
// Remove the external PV deletion protection finalizer
if slice.ContainsString(outFinalizers, storagehelpers.PVDeletionProtectionFinalizer, nil) {
klog.V(4).Infof("Removing external pv deletion protection finalizer on %s", volume.Name)
outFinalizers = slice.RemoveString(outFinalizers, storagehelpers.PVDeletionProtectionFinalizer, nil)
modified = true
}
return outFinalizers, modified
}
// updateMigrationAnnotations takes an Annotations map and checks for a
// provisioner name using the provisionerKey. It will then add a
// "pv.kubernetes.io/migrated-to" annotation if migration with the CSI
// driver name for that provisioner is "on" based on feature flags, it will also
// remove the annotation is migration is "off" for that provisioner in rollback
// scenarios. Returns true if the annotations map was modified and false otherwise.
func updateMigrationAnnotations(cmpm CSIMigratedPluginManager, translator CSINameTranslator, ann map[string]string, claim bool) bool {
var csiDriverName string
var err error
@ -402,14 +457,13 @@ func updateMigrationAnnotationsAndFinalizers(cmpm CSIMigratedPluginManager, tran
return false
}
} else {
// Volume not dynamically provisioned. Ignore
// Volume Statically provisioned.
return false
}
}
migratedToDriver := ann[storagehelpers.AnnMigratedTo]
if cmpm.IsMigrationEnabledForPlugin(provisioner) {
modified := false
csiDriverName, err = translator.GetCSINameFromInTreeName(provisioner)
if err != nil {
klog.Errorf("Could not update volume migration annotations. Migration enabled for plugin %s but could not find corresponding driver name: %v", provisioner, err)
@ -417,37 +471,13 @@ func updateMigrationAnnotationsAndFinalizers(cmpm CSIMigratedPluginManager, tran
}
if migratedToDriver != csiDriverName {
ann[storagehelpers.AnnMigratedTo] = csiDriverName
modified = true
return true
}
// Remove in-tree delete finalizer on the PV as migration is enabled.
if !claim && utilfeature.DefaultFeatureGate.Enabled(features.HonorPVReclaimPolicy) {
if finalizers != nil && slice.ContainsString(*finalizers, storagehelpers.PVDeletionInTreeProtectionFinalizer, nil) {
*finalizers = slice.RemoveString(*finalizers, storagehelpers.PVDeletionInTreeProtectionFinalizer, nil)
modified = true
}
}
return modified
} else {
if migratedToDriver != "" {
// Migration annotation exists but the driver isn't migrated currently
delete(ann, storagehelpers.AnnMigratedTo)
if !claim && utilfeature.DefaultFeatureGate.Enabled(features.HonorPVReclaimPolicy) {
modified := false
if finalizers == nil {
finalizers = &([]string{})
}
// Add back the in-tree PV deletion protection finalizer if does not already exists
if !slice.ContainsString(*finalizers, storagehelpers.PVDeletionInTreeProtectionFinalizer, nil) {
*finalizers = append(*finalizers, storagehelpers.PVDeletionInTreeProtectionFinalizer)
modified = true
}
// Remove the external PV deletion protection finalizer
if slice.ContainsString(*finalizers, storagehelpers.PVDeletionProtectionFinalizer, nil) {
*finalizers = slice.RemoveString(*finalizers, storagehelpers.PVDeletionProtectionFinalizer, nil)
modified = true
}
return modified
}
return true
}
}
return false
@ -607,7 +637,7 @@ func (ctrl *PersistentVolumeController) setClaimProvisioner(ctx context.Context,
// TODO: remove the beta storage provisioner anno after the deprecation period
metav1.SetMetaDataAnnotation(&claimClone.ObjectMeta, storagehelpers.AnnBetaStorageProvisioner, provisionerName)
metav1.SetMetaDataAnnotation(&claimClone.ObjectMeta, storagehelpers.AnnStorageProvisioner, provisionerName)
updateMigrationAnnotationsAndFinalizers(ctrl.csiMigratedPluginManager, ctrl.translator, claimClone.Annotations, nil, true)
updateMigrationAnnotations(ctrl.csiMigratedPluginManager, ctrl.translator, claimClone.Annotations, true)
newClaim, err := ctrl.kubeClient.CoreV1().PersistentVolumeClaims(claim.Namespace).Update(context.TODO(), claimClone, metav1.UpdateOptions{})
if err != nil {
return newClaim, err

118
pkg/controller/volume/persistentvolume/pv_controller_test.go
View File

@ -52,6 +52,8 @@ import (
// can't reliably simulate periodic sync of volumes/claims - it would be
// either very timing-sensitive or slow to wait for real periodic sync.
func TestControllerSync(t *testing.T) {
// Default enable the HonorPVReclaimPolicy feature gate.
defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.HonorPVReclaimPolicy, true)()
tests := []controllerTest{
// [Unit test set 5] - controller tests.
// We test the controller as if
@ -162,10 +164,8 @@ func TestControllerSync(t *testing.T) {
// deleteClaim with a bound claim makes bound volume released with external deleter pending
// there should be an entry in operation timestamps cache in controller
"5-6 - delete claim and waiting for external volume deletion",
volumesWithAnnotation(volume.AnnDynamicallyProvisioned, "gcr.io/vendor-csi",
newVolumeArray("volume5-6", "10Gi", "uid5-6", "claim5-6", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classExternal, volume.AnnBoundByController)),
volumesWithAnnotation(volume.AnnDynamicallyProvisioned, "gcr.io/vendor-csi",
newVolumeArray("volume5-6", "10Gi", "uid5-6", "claim5-6", v1.VolumeReleased, v1.PersistentVolumeReclaimDelete, classExternal, volume.AnnBoundByController)),
volumesWithAnnotation(volume.AnnDynamicallyProvisioned, "gcr.io/vendor-csi", []*v1.PersistentVolume{newExternalProvisionedVolume("volume5-6", "10Gi", "uid5-6", "claim5-6", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classExternal, "fake.driver.csi", nil, volume.AnnBoundByController)}),
volumesWithAnnotation(volume.AnnDynamicallyProvisioned, "gcr.io/vendor-csi", []*v1.PersistentVolume{newExternalProvisionedVolume("volume5-6", "10Gi", "uid5-6", "claim5-6", v1.VolumeReleased, v1.PersistentVolumeReclaimDelete, classExternal, "fake.driver.csi", nil, volume.AnnBoundByController)}),
claimWithAnnotation(volume.AnnStorageProvisioner, "gcr.io/vendor-csi",
newClaimArray("claim5-6", "uid5-6", "1Gi", "volume5-6", v1.ClaimBound, &classExternal, volume.AnnBoundByController, volume.AnnBindCompleted)),
noclaims,
@ -288,7 +288,7 @@ func TestControllerSync(t *testing.T) {
{
// Test that the finalizer gets removed if CSI migration is disabled. The in-tree finalizer is added
// back on the PV since migration is disabled.
"5-9 - volume has its PV deletion protection finalizer removed as CSI migration is disabled",
"5-9 - volume has its external PV deletion protection finalizer removed as CSI migration is disabled",
volumesWithFinalizers(
volumesWithAnnotation(volume.AnnMigratedTo, "pd.csi.storage.gke.io",
newVolumeArray("volume-5-9", "1Gi", "", "", v1.VolumeAvailable, v1.PersistentVolumeReclaimDelete, classEmpty, volume.AnnDynamicallyProvisioned)),
@ -590,14 +590,14 @@ func TestAnnealMigrationAnnotations(t *testing.T) {
}
if tc.volumeAnnotations != nil {
ann := tc.volumeAnnotations
updateMigrationAnnotationsAndFinalizers(cmpm, translator, ann, nil, false)
updateMigrationAnnotations(cmpm, translator, ann, false)
if !reflect.DeepEqual(tc.expVolumeAnnotations, ann) {
t.Errorf("got volume annoations: %v, but expected: %v", ann, tc.expVolumeAnnotations)
}
}
if tc.claimAnnotations != nil {
ann := tc.claimAnnotations
updateMigrationAnnotationsAndFinalizers(cmpm, translator, ann, nil, true)
updateMigrationAnnotations(cmpm, translator, ann, true)
if !reflect.DeepEqual(tc.expClaimAnnotations, ann) {
t.Errorf("got volume annoations: %v, but expected: %v", ann, tc.expVolumeAnnotations)
}
@ -607,26 +607,26 @@ func TestAnnealMigrationAnnotations(t *testing.T) {
}
}
func TestUpdateFinalizer(t *testing.T) {
func TestModifyDeletionFinalizers(t *testing.T) {
// This set of tests ensures that protection finalizer is removed when CSI migration is disabled
// and PV controller needs to remove finalizers added by the external-provisioner.
defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSIMigrationGCE, false)()
defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.HonorPVReclaimPolicy, true)()
const gcePlugin = "kubernetes.io/gce-pd"
const gceDriver = "pd.csi.storage.gke.io"
const customFinalizer = "test.volume.kubernetes.io/finalizer"
tests := []struct {
name string
initialVolume *v1.PersistentVolume
volumeAnnotations map[string]string
volumeFinalizers []string
expVolumeFinalizers []string
expModified bool
migratedDriverGates []featuregate.Feature
}{
{
// Represents a volume provisioned through external-provisioner
// Represents a CSI volume provisioned through external-provisioner, no CSI migration enabled.
name: "13-1 migration was never enabled, volume has the finalizer",
volumeAnnotations: map[string]string{volume.AnnDynamicallyProvisioned: gceDriver},
volumeFinalizers: []string{volume.PVDeletionProtectionFinalizer},
initialVolume: newExternalProvisionedVolume("volume-13-1", "1Gi", "uid11-23", "claim11-23", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classCopper, gceDriver, []string{volume.PVDeletionProtectionFinalizer}, volume.AnnDynamicallyProvisioned, volume.AnnBoundByController),
expVolumeFinalizers: []string{volume.PVDeletionProtectionFinalizer},
expModified: false,
migratedDriverGates: []featuregate.Feature{},
@ -635,8 +635,7 @@ func TestUpdateFinalizer(t *testing.T) {
// Represents a volume provisioned through external-provisioner but the external-provisioner has
// yet to sync the volume to add the new finalizer
name: "13-2 migration was never enabled, volume does not have the finalizer",
volumeAnnotations: map[string]string{volume.AnnDynamicallyProvisioned: gceDriver},
volumeFinalizers: nil,
initialVolume: newExternalProvisionedVolume("volume-13-2", "1Gi", "uid11-23", "claim11-23", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classCopper, gceDriver, nil, volume.AnnDynamicallyProvisioned, volume.AnnBoundByController),
expVolumeFinalizers: nil,
expModified: false,
migratedDriverGates: []featuregate.Feature{},
@ -647,17 +646,15 @@ func TestUpdateFinalizer(t *testing.T) {
// pre-existing finalizer, for example the pv-protection finalizer. When csi-migration is disabled,
// the migrated-to annotation will be removed shortly when updateVolumeMigrationAnnotationsAndFinalizers
// is called followed by adding back the in-tree pv protection finalizer.
name: "13-3 migration was disabled but still has migrated-to annotation, volume does not have pv deletion protection finalizer",
volumeAnnotations: map[string]string{volume.AnnDynamicallyProvisioned: gcePlugin, volume.AnnMigratedTo: gceDriver},
volumeFinalizers: []string{customFinalizer},
name: "13-3 migration was disabled, volume has existing custom finalizer, does not have in-tree pv deletion protection finalizer",
initialVolume: newVolumeWithFinalizers("volume-13-3", "1Gi", "uid11-23", "claim11-23", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classCopper, []string{customFinalizer}, volume.AnnDynamicallyProvisioned, volume.AnnBoundByController),
expVolumeFinalizers: []string{customFinalizer, volume.PVDeletionInTreeProtectionFinalizer},
expModified: true,
migratedDriverGates: []featuregate.Feature{},
},
{
name: "13-4 migration was disabled but still has migrated-to annotation, volume has no finalizers",
volumeAnnotations: map[string]string{volume.AnnDynamicallyProvisioned: gcePlugin, volume.AnnMigratedTo: gceDriver},
volumeFinalizers: nil,
name: "13-4 migration was disabled, volume has no finalizers",
initialVolume: newVolumeWithFinalizers("volume-13-4", "1Gi", "uid11-23", "claim11-23", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classCopper, nil, volume.AnnDynamicallyProvisioned, volume.AnnBoundByController),
expVolumeFinalizers: []string{volume.PVDeletionInTreeProtectionFinalizer},
expModified: true,
migratedDriverGates: []featuregate.Feature{},
@ -666,9 +663,8 @@ func TestUpdateFinalizer(t *testing.T) {
// Represents roll back scenario where the external-provisioner has added the pv deletion protection
// finalizer and later the csi migration was disabled. The pv deletion protection finalizer added through
// external-provisioner will be removed and the in-tree pv deletion protection finalizer will be added.
name: "13-5 migration was disabled as it has the migrated-to annotation, volume has the finalizer",
volumeAnnotations: map[string]string{volume.AnnDynamicallyProvisioned: gcePlugin, volume.AnnMigratedTo: gceDriver},
volumeFinalizers: []string{volume.PVDeletionProtectionFinalizer},
name: "13-5 migration was disabled, volume has external PV deletion finalizer",
initialVolume: newVolumeWithFinalizers("volume-13-5", "1Gi", "uid11-23", "claim11-23", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classCopper, []string{volume.PVDeletionProtectionFinalizer}, volume.AnnDynamicallyProvisioned, volume.AnnBoundByController),
expVolumeFinalizers: []string{volume.PVDeletionInTreeProtectionFinalizer},
expModified: true,
migratedDriverGates: []featuregate.Feature{},
@ -677,9 +673,8 @@ func TestUpdateFinalizer(t *testing.T) {
// Represents roll-back of csi-migration as 13-5, here there are multiple finalizers, only the pv deletion
// protection finalizer added by external-provisioner will be removed and the in-tree pv deletion protection
// finalizer will be added.
name: "13-6 migration was disabled as it has the migrated-to annotation, volume has multiple finalizers",
volumeAnnotations: map[string]string{volume.AnnDynamicallyProvisioned: gcePlugin, volume.AnnMigratedTo: gceDriver},
volumeFinalizers: []string{volume.PVDeletionProtectionFinalizer, customFinalizer},
name: "13-6 migration was disabled, volume has multiple finalizers",
initialVolume: newVolumeWithFinalizers("volume-13-6", "1Gi", "uid11-23", "claim11-23", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classCopper, []string{volume.PVDeletionProtectionFinalizer, customFinalizer}, volume.AnnDynamicallyProvisioned, volume.AnnBoundByController),
expVolumeFinalizers: []string{customFinalizer, volume.PVDeletionInTreeProtectionFinalizer},
expModified: true,
migratedDriverGates: []featuregate.Feature{},
@ -687,9 +682,9 @@ func TestUpdateFinalizer(t *testing.T) {
{
// csi migration is enabled, the pv controller should not delete the finalizer added by the
// external-provisioner and the in-tree finalizer should be deleted.
name: "13-7 migration is enabled, has the migrated-to annotation, volume has the finalizer",
name: "13-7 migration is enabled, volume has both the in-tree and external PV deletion protection finalizer",
initialVolume: newVolumeWithFinalizers("volume-13-7", "1Gi", "uid11-23", "claim11-23", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classCopper, []string{volume.PVDeletionProtectionFinalizer, volume.PVDeletionInTreeProtectionFinalizer}, volume.AnnDynamicallyProvisioned, volume.AnnBoundByController),
volumeAnnotations: map[string]string{volume.AnnDynamicallyProvisioned: gcePlugin, volume.AnnMigratedTo: gceDriver},
volumeFinalizers: []string{volume.PVDeletionProtectionFinalizer, volume.PVDeletionInTreeProtectionFinalizer},
expVolumeFinalizers: []string{volume.PVDeletionProtectionFinalizer},
expModified: true,
migratedDriverGates: []featuregate.Feature{features.CSIMigration, features.CSIMigrationGCE},
@ -697,9 +692,8 @@ func TestUpdateFinalizer(t *testing.T) {
{
// csi-migration is not completely enabled as the specific plugin feature is not present. This is equivalent
// of disabled csi-migration.
name: "13-8 migration is enabled but plugin migration feature is disabled, has the migrated-to annotation, volume has the finalizer",
volumeAnnotations: map[string]string{volume.AnnDynamicallyProvisioned: gcePlugin, volume.AnnMigratedTo: gceDriver},
volumeFinalizers: []string{volume.PVDeletionProtectionFinalizer},
name: "13-8 migration is enabled but plugin migration feature is disabled, volume has the external PV deletion protection finalizer",
initialVolume: newVolumeWithFinalizers("volume-13-8", "1Gi", "uid11-23", "claim11-23", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classCopper, []string{volume.PVDeletionProtectionFinalizer}, volume.AnnDynamicallyProvisioned, volume.AnnBoundByController),
expVolumeFinalizers: []string{volume.PVDeletionInTreeProtectionFinalizer},
expModified: true,
migratedDriverGates: []featuregate.Feature{features.CSIMigration},
@ -707,9 +701,8 @@ func TestUpdateFinalizer(t *testing.T) {
{
// same as 13-8 but multiple finalizers exists, only the pv deletion protection finalizer needs to be
// removed and the in-tree pv deletion protection finalizer needs to be added.
name: "13-9 migration is enabled but plugin migration feature is disabled, has the migrated-to annotation, volume has multiple finalizers",
volumeAnnotations: map[string]string{volume.AnnDynamicallyProvisioned: gcePlugin, volume.AnnMigratedTo: gceDriver},
volumeFinalizers: []string{volume.PVDeletionProtectionFinalizer, customFinalizer},
name: "13-9 migration is enabled but plugin migration feature is disabled, volume has multiple finalizers including external PV deletion protection finalizer",
initialVolume: newVolumeWithFinalizers("volume-13-9", "1Gi", "uid11-23", "claim11-23", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classCopper, []string{volume.PVDeletionProtectionFinalizer, customFinalizer}, volume.AnnDynamicallyProvisioned, volume.AnnBoundByController),
expVolumeFinalizers: []string{customFinalizer, volume.PVDeletionInTreeProtectionFinalizer},
expModified: true,
migratedDriverGates: []featuregate.Feature{features.CSIMigration},
@ -717,37 +710,49 @@ func TestUpdateFinalizer(t *testing.T) {
{
// corner error case.
name: "13-10 missing annotations but finalizers exist",
volumeAnnotations: nil,
volumeFinalizers: []string{volume.PVDeletionProtectionFinalizer},
initialVolume: newVolumeWithFinalizers("volume-13-10", "1Gi", "uid11-23", "claim11-23", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classCopper, []string{volume.PVDeletionProtectionFinalizer}),
expVolumeFinalizers: []string{volume.PVDeletionProtectionFinalizer},
expModified: false,
migratedDriverGates: []featuregate.Feature{},
},
{
name: "13-11 missing annotations and finalizers",
volumeAnnotations: nil,
volumeFinalizers: nil,
initialVolume: newVolumeWithFinalizers("volume-13-11", "1Gi", "uid11-23", "claim11-23", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classCopper, nil),
expVolumeFinalizers: nil,
expModified: false,
migratedDriverGates: []featuregate.Feature{},
},
{
// corner error case
name: "13-12 missing provisioned-by annotation, existing finalizers",
volumeAnnotations: map[string]string{"fake": gcePlugin},
volumeFinalizers: []string{volume.PVDeletionProtectionFinalizer},
expVolumeFinalizers: []string{volume.PVDeletionProtectionFinalizer},
// When ReclaimPolicy is Retain ensure that in-tree pv deletion protection finalizer is not added.
name: "13-12 migration is disabled, volume has no finalizers, reclaimPolicy is Retain",
initialVolume: newVolumeWithFinalizers("volume-13-12", "1Gi", "uid11-23", "claim11-23", v1.VolumeBound, v1.PersistentVolumeReclaimRetain, classCopper, nil, volume.AnnDynamicallyProvisioned, volume.AnnBoundByController),
expVolumeFinalizers: nil,
expModified: false,
migratedDriverGates: []featuregate.Feature{},
},
{
// csi migration is enabled, the pv controller should delete the in-tree finalizer
name: "13-13 migration is enabled, has the migrated-to annotation, volume has the in-tree finalizer",
volumeAnnotations: map[string]string{volume.AnnDynamicallyProvisioned: gcePlugin, volume.AnnMigratedTo: gceDriver},
volumeFinalizers: []string{volume.PVDeletionInTreeProtectionFinalizer},
// When ReclaimPolicy is Recycle ensure that in-tree pv deletion protection finalizer is not added.
name: "13-13 migration is disabled, volume has no finalizers, reclaimPolicy is Recycle",
initialVolume: newVolumeWithFinalizers("volume-13-13", "1Gi", "uid11-23", "claim11-23", v1.VolumeBound, v1.PersistentVolumeReclaimRecycle, classCopper, nil, volume.AnnDynamicallyProvisioned, volume.AnnBoundByController),
expVolumeFinalizers: nil,
expModified: false,
migratedDriverGates: []featuregate.Feature{},
},
{
// When ReclaimPolicy is Retain ensure that in-tree pv deletion protection finalizer present is removed.
name: "13-14 migration is disabled, volume has in-tree pv deletion finalizers, reclaimPolicy is Retain",
initialVolume: newVolumeWithFinalizers("volume-13-14", "1Gi", "uid11-23", "claim11-23", v1.VolumeBound, v1.PersistentVolumeReclaimRetain, classCopper, []string{volume.PVDeletionInTreeProtectionFinalizer}, volume.AnnDynamicallyProvisioned, volume.AnnBoundByController),
expVolumeFinalizers: nil,
expModified: true,
migratedDriverGates: []featuregate.Feature{features.CSIMigration, features.CSIMigrationGCE},
migratedDriverGates: []featuregate.Feature{},
},
{
// Statically provisioned volumes should not have the in-tree pv deletion protection finalizer
name: "13-15 migration is disabled, statically provisioned PV",
initialVolume: newVolumeWithFinalizers("volume-13-14", "1Gi", "", "", v1.VolumeAvailable, v1.PersistentVolumeReclaimDelete, classCopper, nil),
expVolumeFinalizers: nil,
expModified: false,
migratedDriverGates: []featuregate.Feature{},
},
}
@ -760,15 +765,14 @@ func TestUpdateFinalizer(t *testing.T) {
defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, f, true)()
}
if tc.volumeAnnotations != nil {
ann := tc.volumeAnnotations
finalizers := tc.volumeFinalizers
modified := updateMigrationAnnotationsAndFinalizers(cmpm, translator, ann, &finalizers, false)
if modified != tc.expModified {
t.Errorf("got modified: %v, but expected: %v", modified, tc.expModified)
}
if !reflect.DeepEqual(tc.expVolumeFinalizers, finalizers) {
t.Errorf("got volume finaliers: %v, but expected: %v", finalizers, tc.expVolumeFinalizers)
}
tc.initialVolume.SetAnnotations(tc.volumeAnnotations)
}
modifiedFinalizers, modified := modifyDeletionFinalizers(cmpm, tc.initialVolume)
if modified != tc.expModified {
t.Errorf("got modified: %v, but expected: %v", modified, tc.expModified)
}
if !reflect.DeepEqual(tc.expVolumeFinalizers, modifiedFinalizers) {
t.Errorf("got volume finaliers: %v, but expected: %v", modifiedFinalizers, tc.expVolumeFinalizers)
}
})