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 #108400 from deepakkinni/in_tree_protect_v1

Browse Source 
Support for in-tree PV Deletion protection finalizer
This commit is contained in:
Kubernetes Prow Robot 2022-03-11 03:39:06 -08:00 committed by GitHub
commit 370b7cc25c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 113 additions and 40 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -187,6 +187,17 @@ func TestDeleteSync(t *testing.T) {
return testSyncVolume(ctrl, reactor, test)
},
},
{
// TODO: Change the expectedVolumes to novolumes after HonorPVReclaimPolicy is enabled by default.
// delete success - volume has deletion timestamp before doDelete() starts
"8-13 - volume has deletion timestamp and processed",
volumesWithFinalizers(withVolumeDeletionTimestamp(newVolumeArray("volume8-13", "1Gi", "uid8-13", "claim8-13", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty, pvutil.AnnBoundByController)), []string{pvutil.PVDeletionInTreeProtectionFinalizer}),
volumesWithFinalizers(withVolumeDeletionTimestamp(newVolumeArray("volume8-13", "1Gi", "uid8-13", "claim8-13", v1.VolumeReleased, v1.PersistentVolumeReclaimDelete, classEmpty, pvutil.AnnBoundByController)), []string{pvutil.PVDeletionInTreeProtectionFinalizer}),
noclaims,
noclaims,
noevents, noerrors,
wrapTestWithReclaimCalls(operationDelete, []error{nil}, testSyncVolume),
},
}
runSyncTests(t, tests, []*storage.StorageClass{}, []*v1.Pod{})
}

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

@ -23,6 +23,10 @@ import (
"strings"
"time"
utilfeature "k8s.io/apiserver/pkg/util/feature"
"k8s.io/kubernetes/pkg/features"
"k8s.io/kubernetes/pkg/util/slice"
v1 "k8s.io/api/core/v1"
storage "k8s.io/api/storage/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
@ -1247,9 +1251,11 @@ func (ctrl *PersistentVolumeController) deleteVolumeOperation(volume *v1.Persist
return "", nil
}
if newVolume.GetDeletionTimestamp() != nil {
klog.V(3).Infof("Volume %q is already being deleted", volume.Name)
return "", nil
if !utilfeature.DefaultFeatureGate.Enabled(features.HonorPVReclaimPolicy) {
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 {
@ -1439,11 +1445,41 @@ func (ctrl *PersistentVolumeController) doDeleteVolume(volume *v1.PersistentVolu
// Deleter failed
return pluginName, false, err
}
klog.V(2).Infof("volume %q deleted", volume.Name)
// Remove in-tree delete finalizer on the PV as the volume has been deleted from the underlying storage
if utilfeature.DefaultFeatureGate.Enabled(features.HonorPVReclaimPolicy) {
err = ctrl.removeDeletionProtectionFinalizer(context.TODO(), volume)
if err != nil {
return pluginName, true, err
}
}
return pluginName, true, nil
}
func (ctrl *PersistentVolumeController) removeDeletionProtectionFinalizer(ctx context.Context, volume *v1.PersistentVolume) error {
var err error
pvUpdateNeeded := false
volumeClone := volume.DeepCopy()
pvFinalizers := volumeClone.Finalizers
if pvFinalizers != nil && slice.ContainsString(pvFinalizers, pvutil.PVDeletionInTreeProtectionFinalizer, nil) {
pvUpdateNeeded = true
pvFinalizers = slice.RemoveString(pvFinalizers, pvutil.PVDeletionInTreeProtectionFinalizer, nil)
}
if pvUpdateNeeded {
volumeClone.SetFinalizers(pvFinalizers)
_, err = ctrl.kubeClient.CoreV1().PersistentVolumes().Update(ctx, volumeClone, metav1.UpdateOptions{})
if err != nil {
return fmt.Errorf("persistent volume controller can't update finalizer: %v", err)
}
_, err = ctrl.storeVolumeUpdate(volumeClone)
if err != nil {
return fmt.Errorf("persistent Volume Controller can't anneal migration finalizer: %v", err)
}
klog.V(2).Infof("PV in-tree protection finalizer removed from volume: %q", volume.Name)
}
return nil
}
// provisionClaim starts new asynchronous operation to provision a claim if
// provisioning is enabled.
func (ctrl *PersistentVolumeController) provisionClaim(ctx context.Context, claim *v1.PersistentVolumeClaim) error {
@ -1619,6 +1655,11 @@ func (ctrl *PersistentVolumeController) provisionClaimOperation(
metav1.SetMetaDataAnnotation(&volume.ObjectMeta, pvutil.AnnBoundByController, "yes")
metav1.SetMetaDataAnnotation(&volume.ObjectMeta, pvutil.AnnDynamicallyProvisioned, plugin.GetPluginName())
if utilfeature.DefaultFeatureGate.Enabled(features.HonorPVReclaimPolicy) {
// Add finalizer here
volume.SetFinalizers([]string{pvutil.PVDeletionInTreeProtectionFinalizer})
}
// Try to create the PV object several times
for i := 0; i < ctrl.createProvisionedPVRetryCount; i++ {
klog.V(4).Infof("provisionClaimOperation [%s]: trying to save volume %s", claimToClaimKey(claim), volume.Name)

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

@ -409,6 +409,7 @@ func updateMigrationAnnotationsAndFinalizers(cmpm CSIMigratedPluginManager, tran
migratedToDriver := ann[pvutil.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)
@ -416,19 +417,37 @@ func updateMigrationAnnotationsAndFinalizers(cmpm CSIMigratedPluginManager, tran
}
if migratedToDriver != csiDriverName {
ann[pvutil.AnnMigratedTo] = csiDriverName
return true
modified = 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, pvutil.PVDeletionInTreeProtectionFinalizer, nil) {
*finalizers = slice.RemoveString(*finalizers, pvutil.PVDeletionInTreeProtectionFinalizer, nil)
modified = true
}
}
return modified
} else {
if migratedToDriver != "" {
// Migration annotation exists but the driver isn't migrated currently
delete(ann, pvutil.AnnMigratedTo)
// Remove the PV deletion protection finalizer on volumes.
if !claim && finalizers != nil && utilfeature.DefaultFeatureGate.Enabled(features.HonorPVReclaimPolicy) {
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, pvutil.PVDeletionInTreeProtectionFinalizer, nil) {
*finalizers = append(*finalizers, pvutil.PVDeletionInTreeProtectionFinalizer)
modified = true
}
// Remove the external PV deletion protection finalizer
if slice.ContainsString(*finalizers, pvutil.PVDeletionProtectionFinalizer, nil) {
*finalizers = slice.RemoveString(*finalizers, pvutil.PVDeletionProtectionFinalizer, nil)
modified = true
}
return modified
}
return true
}
}
return false

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

@ -294,29 +294,15 @@ func TestControllerSync(t *testing.T) {
},
},
{
// 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
},
},
{
// Test that the finalizer gets removed if CSI migration is disabled.
// 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",
volumesWithFinalizers(
volumesWithAnnotation(pvutil.AnnMigratedTo, "pd.csi.storage.gke.io",
newVolumeArray("volume-5-9", "1Gi", "", "", v1.VolumeAvailable, v1.PersistentVolumeReclaimDelete, classEmpty, pvutil.AnnDynamicallyProvisioned)),
[]string{pvutil.PVDeletionProtectionFinalizer},
),
newVolumeArray("volume-5-9", "1Gi", "", "", v1.VolumeAvailable, v1.PersistentVolumeReclaimDelete, classEmpty, pvutil.AnnDynamicallyProvisioned),
volumesWithFinalizers(newVolumeArray("volume-5-9", "1Gi", "", "", v1.VolumeAvailable, v1.PersistentVolumeReclaimDelete, classEmpty, pvutil.AnnDynamicallyProvisioned), []string{pvutil.PVDeletionInTreeProtectionFinalizer}),
noclaims,
noclaims,
noevents,
@ -743,12 +729,13 @@ func TestUpdateFinalizer(t *testing.T) {
{
// Represents an in-tree volume that has the migrated-to annotation but the external-provisioner is
// yet to sync the volume and add the pv deletion protection finalizer. The custom finalizer is some
// pre-existing finalizer, for example the pv-protection finalizer. The csi-migration is disabled,
// the migrated-to annotation will be removed shortly when updateVolumeMigrationAnnotationsAndFinalizers is called.
// 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{pvutil.AnnDynamicallyProvisioned: gcePlugin, pvutil.AnnMigratedTo: gceDriver},
volumeFinalizers: []string{customFinalizer},
expVolumeFinalizers: []string{customFinalizer},
expVolumeFinalizers: []string{customFinalizer, pvutil.PVDeletionInTreeProtectionFinalizer},
expModified: true,
migratedDriverGates: []featuregate.Feature{},
},
@ -756,39 +743,40 @@ func TestUpdateFinalizer(t *testing.T) {
name: "13-4 migration was disabled but still has migrated-to annotation, volume has no finalizers",
volumeAnnotations: map[string]string{pvutil.AnnDynamicallyProvisioned: gcePlugin, pvutil.AnnMigratedTo: gceDriver},
volumeFinalizers: nil,
expVolumeFinalizers: nil,
expVolumeFinalizers: []string{pvutil.PVDeletionInTreeProtectionFinalizer},
expModified: true,
migratedDriverGates: []featuregate.Feature{},
},
{
// 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.
// 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{pvutil.AnnDynamicallyProvisioned: gcePlugin, pvutil.AnnMigratedTo: gceDriver},
volumeFinalizers: []string{pvutil.PVDeletionProtectionFinalizer},
expVolumeFinalizers: nil,
expVolumeFinalizers: []string{pvutil.PVDeletionInTreeProtectionFinalizer},
expModified: true,
migratedDriverGates: []featuregate.Feature{},
},
{
// Represents roll-back of csi-migration as 13-5, here there are multiple finalizers, only the pv deletion
// protection finalizer added by external-provisioner needs to be removed.
// 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{pvutil.AnnDynamicallyProvisioned: gcePlugin, pvutil.AnnMigratedTo: gceDriver},
volumeFinalizers: []string{pvutil.PVDeletionProtectionFinalizer, customFinalizer},
expVolumeFinalizers: []string{customFinalizer},
expVolumeFinalizers: []string{customFinalizer, pvutil.PVDeletionInTreeProtectionFinalizer},
expModified: true,
migratedDriverGates: []featuregate.Feature{},
},
{
// csi migration is enabled, the pv controller should not delete the finalizer added by the
// external-provisioner.
// 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",
volumeAnnotations: map[string]string{pvutil.AnnDynamicallyProvisioned: gcePlugin, pvutil.AnnMigratedTo: gceDriver},
volumeFinalizers: []string{pvutil.PVDeletionProtectionFinalizer},
volumeFinalizers: []string{pvutil.PVDeletionProtectionFinalizer, pvutil.PVDeletionInTreeProtectionFinalizer},
expVolumeFinalizers: []string{pvutil.PVDeletionProtectionFinalizer},
expModified: false,
expModified: true,
migratedDriverGates: []featuregate.Feature{features.CSIMigration, features.CSIMigrationGCE},
},
{
@ -797,16 +785,17 @@ func TestUpdateFinalizer(t *testing.T) {
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{pvutil.AnnDynamicallyProvisioned: gcePlugin, pvutil.AnnMigratedTo: gceDriver},
volumeFinalizers: []string{pvutil.PVDeletionProtectionFinalizer},
expVolumeFinalizers: nil,
expVolumeFinalizers: []string{pvutil.PVDeletionInTreeProtectionFinalizer},
expModified: true,
migratedDriverGates: []featuregate.Feature{features.CSIMigration},
},
{
// same as 13-8 but multiple finalizers exists, only the pv deletion protection finalizer needs to be removed.
// 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{pvutil.AnnDynamicallyProvisioned: gcePlugin, pvutil.AnnMigratedTo: gceDriver},
volumeFinalizers: []string{pvutil.PVDeletionProtectionFinalizer, customFinalizer},
expVolumeFinalizers: []string{customFinalizer},
expVolumeFinalizers: []string{customFinalizer, pvutil.PVDeletionInTreeProtectionFinalizer},
expModified: true,
migratedDriverGates: []featuregate.Feature{features.CSIMigration},
},
@ -836,6 +825,15 @@ func TestUpdateFinalizer(t *testing.T) {
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{pvutil.AnnDynamicallyProvisioned: gcePlugin, pvutil.AnnMigratedTo: gceDriver},
volumeFinalizers: []string{pvutil.PVDeletionInTreeProtectionFinalizer},
expVolumeFinalizers: nil,
expModified: true,
migratedDriverGates: []featuregate.Feature{features.CSIMigration, features.CSIMigrationGCE},
},
}
translator := csitrans.New()

3
pkg/controller/volume/persistentvolume/util/util.go
View File

@ -78,6 +78,9 @@ const (
//PVDeletionProtectionFinalizer is the finalizer added by the external-provisioner on the PV
PVDeletionProtectionFinalizer = "external-provisioner.volume.kubernetes.io/finalizer"
// PVDeletionInTreeProtectionFinalizer is the finalizer added to protect PV deletion for in-tree volumes.
PVDeletionInTreeProtectionFinalizer = "kubernetes.io/pv-controller"
)
// IsDelayBindingProvisioning checks if claim provisioning with selected-node annotation

3
pkg/features/kube_features.go
View File

@ -801,6 +801,7 @@ const (
// owner: @deepakkinni @xing-yang
// kep: http://kep.k8s.io/2680
// alpha: v1.23
// beta: v1.24
//
// Honor Persistent Volume Reclaim Policy when it is "Delete" irrespective of PV-PVC
// deletion ordering.
@ -943,7 +944,7 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS
JobMutableNodeSchedulingDirectives: {Default: true, PreRelease: featuregate.Beta},
IdentifyPodOS: {Default: false, PreRelease: featuregate.Alpha},
PodAndContainerStatsFromCRI: {Default: false, PreRelease: featuregate.Alpha},
HonorPVReclaimPolicy: {Default: false, PreRelease: featuregate.Alpha},
HonorPVReclaimPolicy: {Default: false, PreRelease: featuregate.Beta},
RecoverVolumeExpansionFailure: {Default: false, PreRelease: featuregate.Alpha},
GRPCContainerProbe: {Default: false, PreRelease: featuregate.Alpha},
LegacyServiceAccountTokenNoAutoGeneration: {Default: true, PreRelease: featuregate.Beta},