From 760365d5c9a9dc2f9314ef0a1c9344d87d128880 Mon Sep 17 00:00:00 2001 From: Jiawei Wang Date: Mon, 6 Jun 2022 21:19:19 +0000 Subject: [PATCH] CSIMigration feature gate to GA --- pkg/apis/storage/validation/validation.go | 9 +---- .../storage/validation/validation_test.go | 38 ------------------- .../attachdetach/attach_detach_controller.go | 7 +--- .../attach_detach_controller_test.go | 1 - .../volume/attachdetach/util/util.go | 7 ---- .../volume/expand/expand_controller_test.go | 2 - .../persistentvolume/pv_controller_test.go | 8 ++-- pkg/features/kube_features.go | 2 +- .../storage/volumeattachment/strategy.go | 10 ----- .../storage/volumeattachment/strategy_test.go | 17 --------- .../plugins/nodevolumelimits/csi_test.go | 2 - .../plugins/nodevolumelimits/utils.go | 6 +-- .../framework/plugins/volumebinding/binder.go | 4 -- .../plugins/volumebinding/binder_test.go | 2 - pkg/volume/csi/csi_plugin.go | 26 ++++++------- pkg/volume/csimigration/plugin_manager.go | 10 +---- .../csimigration/plugin_manager_test.go | 31 +-------------- pkg/volume/portworx/portworx.go | 3 +- pkg/volume/rbd/rbd.go | 3 +- pkg/volume/vsphere_volume/vsphere_volume.go | 3 +- .../rbac/bootstrappolicy/controller_policy.go | 4 +- test/integration/volume/attach_detach_test.go | 6 +-- 22 files changed, 29 insertions(+), 172 deletions(-) diff --git a/pkg/apis/storage/validation/validation.go b/pkg/apis/storage/validation/validation.go index 8bb8aa2f0ec..4497f1e5513 100644 --- a/pkg/apis/storage/validation/validation.go +++ b/pkg/apis/storage/validation/validation.go @@ -31,9 +31,6 @@ import ( "k8s.io/kubernetes/pkg/apis/core/helper" apivalidation "k8s.io/kubernetes/pkg/apis/core/validation" "k8s.io/kubernetes/pkg/apis/storage" - - utilfeature "k8s.io/apiserver/pkg/util/feature" - "k8s.io/kubernetes/pkg/features" ) const ( @@ -179,11 +176,7 @@ func validateVolumeAttachmentSource(source *storage.VolumeAttachmentSource, fldP allErrs := field.ErrorList{} switch { case source.InlineVolumeSpec == nil && source.PersistentVolumeName == nil: - if utilfeature.DefaultFeatureGate.Enabled(features.CSIMigration) { - allErrs = append(allErrs, field.Required(fldPath, "must specify exactly one of inlineVolumeSpec and persistentVolumeName")) - } else { - allErrs = append(allErrs, field.Required(fldPath, "must specify persistentVolumeName when CSIMigration feature is disabled")) - } + allErrs = append(allErrs, field.Required(fldPath, "must specify exactly one of inlineVolumeSpec and persistentVolumeName")) case source.InlineVolumeSpec != nil && source.PersistentVolumeName != nil: allErrs = append(allErrs, field.Forbidden(fldPath, "must specify exactly one of inlineVolumeSpec and persistentVolumeName")) case source.PersistentVolumeName != nil: diff --git a/pkg/apis/storage/validation/validation_test.go b/pkg/apis/storage/validation/validation_test.go index 2374f500497..2c21554569f 100644 --- a/pkg/apis/storage/validation/validation_test.go +++ b/pkg/apis/storage/validation/validation_test.go @@ -23,11 +23,8 @@ import ( "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - utilfeature "k8s.io/apiserver/pkg/util/feature" - featuregatetesting "k8s.io/component-base/featuregate/testing" api "k8s.io/kubernetes/pkg/apis/core" "k8s.io/kubernetes/pkg/apis/storage" - "k8s.io/kubernetes/pkg/features" utilpointer "k8s.io/utils/pointer" ) @@ -158,7 +155,6 @@ func TestValidateStorageClass(t *testing.T) { } func TestVolumeAttachmentValidation(t *testing.T) { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSIMigration, true)() volumeName := "pv-name" empty := "" migrationEnabledSuccessCases := []storage.VolumeAttachment{ @@ -384,43 +380,9 @@ func TestVolumeAttachmentValidation(t *testing.T) { t.Errorf("expected failure for test: %v", volumeAttachment) } } - - // validate with CSIMigration disabled - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSIMigration, false)() - - migrationDisabledSuccessCases := []storage.VolumeAttachment{ - { - // PVName specified with migration disabled - ObjectMeta: metav1.ObjectMeta{Name: "foo"}, - Spec: storage.VolumeAttachmentSpec{ - Attacher: "myattacher", - NodeName: "node", - Source: storage.VolumeAttachmentSource{ - PersistentVolumeName: &volumeName, - }, - }, - }, - { - // InlineSpec specified with migration disabled - ObjectMeta: metav1.ObjectMeta{Name: "foo"}, - Spec: storage.VolumeAttachmentSpec{ - Attacher: "myattacher", - NodeName: "node", - Source: storage.VolumeAttachmentSource{ - InlineVolumeSpec: &inlineSpec, - }, - }, - }, - } - for _, volumeAttachment := range migrationDisabledSuccessCases { - if errs := ValidateVolumeAttachment(&volumeAttachment); len(errs) != 0 { - t.Errorf("expected success: %v %v", volumeAttachment, errs) - } - } } func TestVolumeAttachmentUpdateValidation(t *testing.T) { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSIMigration, true)() volumeName := "foo" newVolumeName := "bar" diff --git a/pkg/controller/volume/attachdetach/attach_detach_controller.go b/pkg/controller/volume/attachdetach/attach_detach_controller.go index 56f404f1e1f..509846859c8 100644 --- a/pkg/controller/volume/attachdetach/attach_detach_controller.go +++ b/pkg/controller/volume/attachdetach/attach_detach_controller.go @@ -54,7 +54,6 @@ import ( "k8s.io/kubernetes/pkg/controller/volume/attachdetach/statusupdater" "k8s.io/kubernetes/pkg/controller/volume/attachdetach/util" "k8s.io/kubernetes/pkg/controller/volume/common" - "k8s.io/kubernetes/pkg/features" proxyutil "k8s.io/kubernetes/pkg/proxy/util" "k8s.io/kubernetes/pkg/volume" "k8s.io/kubernetes/pkg/volume/csi" @@ -138,10 +137,8 @@ func NewAttachDetachController( filteredDialOptions: filteredDialOptions, } - if utilfeature.DefaultFeatureGate.Enabled(features.CSIMigration) { - adc.csiNodeLister = csiNodeInformer.Lister() - adc.csiNodeSynced = csiNodeInformer.Informer().HasSynced - } + adc.csiNodeLister = csiNodeInformer.Lister() + adc.csiNodeSynced = csiNodeInformer.Informer().HasSynced adc.csiDriverLister = csiDriverInformer.Lister() adc.csiDriversSynced = csiDriverInformer.Informer().HasSynced diff --git a/pkg/controller/volume/attachdetach/attach_detach_controller_test.go b/pkg/controller/volume/attachdetach/attach_detach_controller_test.go index 605ee535a72..e4a54d36171 100644 --- a/pkg/controller/volume/attachdetach/attach_detach_controller_test.go +++ b/pkg/controller/volume/attachdetach/attach_detach_controller_test.go @@ -431,7 +431,6 @@ func volumeAttachmentRecoveryTestCase(t *testing.T, tc vaTest) { informerFactory := informers.NewSharedInformerFactory(fakeKubeClient, time.Second*1) var plugins []volume.VolumePlugin - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSIMigration, tc.csiMigration)() defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSIMigrationGCE, tc.csiMigration)() defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.InTreePluginGCEUnregister, tc.csiMigration)() diff --git a/pkg/controller/volume/attachdetach/util/util.go b/pkg/controller/volume/attachdetach/util/util.go index dfcad47cc52..3abb4d68b37 100644 --- a/pkg/controller/volume/attachdetach/util/util.go +++ b/pkg/controller/volume/attachdetach/util/util.go @@ -24,12 +24,10 @@ import ( v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/sets" - utilfeature "k8s.io/apiserver/pkg/util/feature" corelisters "k8s.io/client-go/listers/core/v1" "k8s.io/component-helpers/storage/ephemeral" "k8s.io/klog/v2" "k8s.io/kubernetes/pkg/controller/volume/attachdetach/cache" - "k8s.io/kubernetes/pkg/features" "k8s.io/kubernetes/pkg/volume" "k8s.io/kubernetes/pkg/volume/csimigration" "k8s.io/kubernetes/pkg/volume/util" @@ -315,11 +313,6 @@ func translateInTreeSpecToCSIIfNeeded(spec *volume.Spec, nodeName types.NodeName } func isCSIMigrationSupportedOnNode(nodeName types.NodeName, spec *volume.Spec, vpm *volume.VolumePluginMgr, csiMigratedPluginManager csimigration.PluginManager) (bool, error) { - if !utilfeature.DefaultFeatureGate.Enabled(features.CSIMigration) { - // If CSIMigration is disabled, CSI migration paths will not be taken for the node. - return false, nil - } - pluginName, err := csiMigratedPluginManager.GetInTreePluginNameFromSpec(spec.PersistentVolume, spec.Volume) if err != nil { return false, err diff --git a/pkg/controller/volume/expand/expand_controller_test.go b/pkg/controller/volume/expand/expand_controller_test.go index 02902755552..a1297278645 100644 --- a/pkg/controller/volume/expand/expand_controller_test.go +++ b/pkg/controller/volume/expand/expand_controller_test.go @@ -126,10 +126,8 @@ func TestSyncHandler(t *testing.T) { } if test.csiMigrationEnabled { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSIMigration, true)() defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSIMigrationAWS, true)() } else { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSIMigration, false)() defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSIMigrationAWS, false)() } diff --git a/pkg/controller/volume/persistentvolume/pv_controller_test.go b/pkg/controller/volume/persistentvolume/pv_controller_test.go index a2dd56b61c5..ae68d961d9d 100644 --- a/pkg/controller/volume/persistentvolume/pv_controller_test.go +++ b/pkg/controller/volume/persistentvolume/pv_controller_test.go @@ -471,8 +471,6 @@ func makeStorageClass(scName string, mode *storagev1.VolumeBindingMode) *storage } func TestAnnealMigrationAnnotations(t *testing.T) { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSIMigration, true)() - const testPlugin = "non-migrated-plugin" const gcePlugin = "kubernetes.io/gce-pd" const gceDriver = "pd.csi.storage.gke.io" @@ -687,7 +685,7 @@ func TestModifyDeletionFinalizers(t *testing.T) { volumeAnnotations: map[string]string{volume.AnnDynamicallyProvisioned: gcePlugin, volume.AnnMigratedTo: gceDriver}, expVolumeFinalizers: []string{volume.PVDeletionProtectionFinalizer}, expModified: true, - migratedDriverGates: []featuregate.Feature{features.CSIMigration, features.CSIMigrationGCE}, + migratedDriverGates: []featuregate.Feature{features.CSIMigrationGCE}, }, { // csi-migration is not completely enabled as the specific plugin feature is not present. This is equivalent @@ -696,7 +694,7 @@ func TestModifyDeletionFinalizers(t *testing.T) { 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}, + migratedDriverGates: []featuregate.Feature{}, }, { // same as 13-8 but multiple finalizers exists, only the pv deletion protection finalizer needs to be @@ -705,7 +703,7 @@ func TestModifyDeletionFinalizers(t *testing.T) { 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}, + migratedDriverGates: []featuregate.Feature{}, }, { // corner error case. diff --git a/pkg/features/kube_features.go b/pkg/features/kube_features.go index 754d960bac1..d1db7013d18 100644 --- a/pkg/features/kube_features.go +++ b/pkg/features/kube_features.go @@ -817,7 +817,7 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS CSIInlineVolume: {Default: true, PreRelease: featuregate.Beta}, - CSIMigration: {Default: true, PreRelease: featuregate.Beta}, + CSIMigration: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.27 CSIMigrationAWS: {Default: true, PreRelease: featuregate.Beta}, diff --git a/pkg/registry/storage/volumeattachment/strategy.go b/pkg/registry/storage/volumeattachment/strategy.go index 26bd749286a..f802a6ab153 100644 --- a/pkg/registry/storage/volumeattachment/strategy.go +++ b/pkg/registry/storage/volumeattachment/strategy.go @@ -23,11 +23,9 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/validation/field" "k8s.io/apiserver/pkg/storage/names" - utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/kubernetes/pkg/api/legacyscheme" "k8s.io/kubernetes/pkg/apis/storage" "k8s.io/kubernetes/pkg/apis/storage/validation" - "k8s.io/kubernetes/pkg/features" "sigs.k8s.io/structured-merge-diff/v4/fieldpath" ) @@ -61,11 +59,6 @@ func (volumeAttachmentStrategy) GetResetFields() map[fieldpath.APIVersion]*field func (volumeAttachmentStrategy) PrepareForCreate(ctx context.Context, obj runtime.Object) { volumeAttachment := obj.(*storage.VolumeAttachment) volumeAttachment.Status = storage.VolumeAttachmentStatus{} - - if !utilfeature.DefaultFeatureGate.Enabled(features.CSIMigration) { - volumeAttachment.Spec.Source.InlineVolumeSpec = nil - } - } func (volumeAttachmentStrategy) Validate(ctx context.Context, obj runtime.Object) field.ErrorList { @@ -99,9 +92,6 @@ func (volumeAttachmentStrategy) PrepareForUpdate(ctx context.Context, obj, old r newVolumeAttachment.Status = oldVolumeAttachment.Status // No need to increment Generation because we don't allow updates to spec - if !utilfeature.DefaultFeatureGate.Enabled(features.CSIMigration) && oldVolumeAttachment.Spec.Source.InlineVolumeSpec == nil { - newVolumeAttachment.Spec.Source.InlineVolumeSpec = nil - } } func (volumeAttachmentStrategy) ValidateUpdate(ctx context.Context, obj, old runtime.Object) field.ErrorList { diff --git a/pkg/registry/storage/volumeattachment/strategy_test.go b/pkg/registry/storage/volumeattachment/strategy_test.go index a641761a764..cf3d5e657ba 100644 --- a/pkg/registry/storage/volumeattachment/strategy_test.go +++ b/pkg/registry/storage/volumeattachment/strategy_test.go @@ -25,11 +25,8 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/diff" genericapirequest "k8s.io/apiserver/pkg/endpoints/request" - utilfeature "k8s.io/apiserver/pkg/util/feature" - featuregatetesting "k8s.io/component-base/featuregate/testing" api "k8s.io/kubernetes/pkg/apis/core" "k8s.io/kubernetes/pkg/apis/storage" - "k8s.io/kubernetes/pkg/features" ) func getValidVolumeAttachment(name string) *storage.VolumeAttachment { @@ -127,7 +124,6 @@ func TestVolumeAttachmentStrategySourceInlineSpec(t *testing.T) { volumeAttachment := getValidVolumeAttachmentWithInlineSpec("valid-attachment") volumeAttachmentSaved := volumeAttachment.DeepCopy() - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSIMigration, true)() Strategy.PrepareForCreate(ctx, volumeAttachment) if volumeAttachment.Spec.Source.InlineVolumeSpec == nil { t.Errorf("InlineVolumeSpec unexpectedly set to nil during PrepareForCreate") @@ -139,23 +135,10 @@ func TestVolumeAttachmentStrategySourceInlineSpec(t *testing.T) { if volumeAttachmentSaved.Spec.Source.InlineVolumeSpec == nil { t.Errorf("InlineVolumeSpec unexpectedly set to nil during PrepareForUpdate") } - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSIMigration, false)() Strategy.PrepareForUpdate(ctx, volumeAttachmentSaved, volumeAttachment) if volumeAttachmentSaved.Spec.Source.InlineVolumeSpec == nil { t.Errorf("InlineVolumeSpec unexpectedly set to nil during PrepareForUpdate") } - - volumeAttachment = getValidVolumeAttachmentWithInlineSpec("valid-attachment") - volumeAttachmentNew := volumeAttachment.DeepCopy() - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSIMigration, false)() - Strategy.PrepareForCreate(ctx, volumeAttachment) - if volumeAttachment.Spec.Source.InlineVolumeSpec != nil { - t.Errorf("InlineVolumeSpec unexpectedly not dropped during PrepareForCreate") - } - Strategy.PrepareForUpdate(ctx, volumeAttachmentNew, volumeAttachment) - if volumeAttachmentNew.Spec.Source.InlineVolumeSpec != nil { - t.Errorf("InlineVolumeSpec unexpectedly not dropped during PrepareForUpdate") - } } func TestVolumeAttachmentStatusStrategy(t *testing.T) { diff --git a/pkg/scheduler/framework/plugins/nodevolumelimits/csi_test.go b/pkg/scheduler/framework/plugins/nodevolumelimits/csi_test.go index d2b841eb680..97ef1849748 100644 --- a/pkg/scheduler/framework/plugins/nodevolumelimits/csi_test.go +++ b/pkg/scheduler/framework/plugins/nodevolumelimits/csi_test.go @@ -534,11 +534,9 @@ func TestCSILimits(t *testing.T) { t.Run(test.test, func(t *testing.T) { node, csiNode := getNodeWithPodAndVolumeLimits(test.limitSource, test.existingPods, int64(test.maxVols), test.driverNames...) if test.migrationEnabled { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSIMigration, true)() defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSIMigrationAWS, true)() enableMigrationOnNode(csiNode, csilibplugins.AWSEBSInTreePluginName) } else { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSIMigration, false)() defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSIMigrationAWS, false)() } csiTranslator := csitrans.New() diff --git a/pkg/scheduler/framework/plugins/nodevolumelimits/utils.go b/pkg/scheduler/framework/plugins/nodevolumelimits/utils.go index 665f2cb2f7a..79d625e17c2 100644 --- a/pkg/scheduler/framework/plugins/nodevolumelimits/utils.go +++ b/pkg/scheduler/framework/plugins/nodevolumelimits/utils.go @@ -19,7 +19,7 @@ package nodevolumelimits import ( "strings" - "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" storagev1 "k8s.io/api/storage/v1" "k8s.io/apimachinery/pkg/util/sets" utilfeature "k8s.io/apiserver/pkg/util/feature" @@ -38,10 +38,6 @@ func isCSIMigrationOn(csiNode *storagev1.CSINode, pluginName string) bool { // In-tree storage to CSI driver migration feature should be enabled, // along with the plugin-specific one - if !utilfeature.DefaultFeatureGate.Enabled(features.CSIMigration) { - return false - } - switch pluginName { case csilibplugins.AWSEBSInTreePluginName: if !utilfeature.DefaultFeatureGate.Enabled(features.CSIMigrationAWS) { diff --git a/pkg/scheduler/framework/plugins/volumebinding/binder.go b/pkg/scheduler/framework/plugins/volumebinding/binder.go index ac1f02e640d..f715699e52d 100644 --- a/pkg/scheduler/framework/plugins/volumebinding/binder.go +++ b/pkg/scheduler/framework/plugins/volumebinding/binder.go @@ -1046,10 +1046,6 @@ func (b *volumeBinder) tryTranslatePVToCSI(pv *v1.PersistentVolume, csiNode *sto return pv, nil } - if !utilfeature.DefaultFeatureGate.Enabled(features.CSIMigration) { - return pv, nil - } - pluginName, err := b.translator.GetInTreePluginNameFromSpec(pv, nil) if err != nil { return nil, fmt.Errorf("could not get plugin name from pv: %v", err) diff --git a/pkg/scheduler/framework/plugins/volumebinding/binder_test.go b/pkg/scheduler/framework/plugins/volumebinding/binder_test.go index 3d15a2be49d..b006536bf95 100644 --- a/pkg/scheduler/framework/plugins/volumebinding/binder_test.go +++ b/pkg/scheduler/framework/plugins/volumebinding/binder_test.go @@ -1217,7 +1217,6 @@ func TestFindPodVolumesWithCSIMigration(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSIMigration, true)() defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSIMigrationGCE, true)() // Setup @@ -1844,7 +1843,6 @@ func TestCheckBindingsWithCSIMigration(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSIMigration, scenario.migrationEnabled)() defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSIMigrationGCE, scenario.migrationEnabled)() // Setup diff --git a/pkg/volume/csi/csi_plugin.go b/pkg/volume/csi/csi_plugin.go index dfef623220d..592aaaeaaf6 100644 --- a/pkg/volume/csi/csi_plugin.go +++ b/pkg/volume/csi/csi_plugin.go @@ -217,40 +217,38 @@ func (p *csiPlugin) Init(host volume.VolumeHost) error { var migratedPlugins = map[string](func() bool){ csitranslationplugins.GCEPDInTreePluginName: func() bool { - return utilfeature.DefaultFeatureGate.Enabled(features.CSIMigration) && utilfeature.DefaultFeatureGate.Enabled(features.CSIMigrationGCE) + return utilfeature.DefaultFeatureGate.Enabled(features.CSIMigrationGCE) }, csitranslationplugins.AWSEBSInTreePluginName: func() bool { - return utilfeature.DefaultFeatureGate.Enabled(features.CSIMigration) && utilfeature.DefaultFeatureGate.Enabled(features.CSIMigrationAWS) + return utilfeature.DefaultFeatureGate.Enabled(features.CSIMigrationAWS) }, csitranslationplugins.CinderInTreePluginName: func() bool { - return utilfeature.DefaultFeatureGate.Enabled(features.CSIMigration) + return true }, csitranslationplugins.AzureDiskInTreePluginName: func() bool { - return utilfeature.DefaultFeatureGate.Enabled(features.CSIMigration) && utilfeature.DefaultFeatureGate.Enabled(features.CSIMigrationAzureDisk) + return utilfeature.DefaultFeatureGate.Enabled(features.CSIMigrationAzureDisk) }, csitranslationplugins.AzureFileInTreePluginName: func() bool { - return utilfeature.DefaultFeatureGate.Enabled(features.CSIMigration) && utilfeature.DefaultFeatureGate.Enabled(features.CSIMigrationAzureFile) + return utilfeature.DefaultFeatureGate.Enabled(features.CSIMigrationAzureFile) }, csitranslationplugins.VSphereInTreePluginName: func() bool { - return utilfeature.DefaultFeatureGate.Enabled(features.CSIMigration) && utilfeature.DefaultFeatureGate.Enabled(features.CSIMigrationvSphere) + return utilfeature.DefaultFeatureGate.Enabled(features.CSIMigrationvSphere) }, csitranslationplugins.PortworxVolumePluginName: func() bool { - return utilfeature.DefaultFeatureGate.Enabled(features.CSIMigration) && utilfeature.DefaultFeatureGate.Enabled(features.CSIMigrationPortworx) + return utilfeature.DefaultFeatureGate.Enabled(features.CSIMigrationPortworx) }, csitranslationplugins.RBDVolumePluginName: func() bool { - return utilfeature.DefaultFeatureGate.Enabled(features.CSIMigration) && utilfeature.DefaultFeatureGate.Enabled(features.CSIMigrationRBD) + return utilfeature.DefaultFeatureGate.Enabled(features.CSIMigrationRBD) }, } // Initializing the label management channels nim = nodeinfomanager.NewNodeInfoManager(host.GetNodeName(), host, migratedPlugins) - if utilfeature.DefaultFeatureGate.Enabled(features.CSIMigration) { - // This function prevents Kubelet from posting Ready status until CSINode - // is both installed and initialized - if err := initializeCSINode(host); err != nil { - return errors.New(log("failed to initialize CSINode: %v", err)) - } + // This function prevents Kubelet from posting Ready status until CSINode + // is both installed and initialized + if err := initializeCSINode(host); err != nil { + return errors.New(log("failed to initialize CSINode: %v", err)) } return nil diff --git a/pkg/volume/csimigration/plugin_manager.go b/pkg/volume/csimigration/plugin_manager.go index 45acd1997e3..b568e85ac15 100644 --- a/pkg/volume/csimigration/plugin_manager.go +++ b/pkg/volume/csimigration/plugin_manager.go @@ -85,9 +85,7 @@ func (pm PluginManager) IsMigrationCompleteForPlugin(pluginName string) bool { // for a particular storage plugin func (pm PluginManager) IsMigrationEnabledForPlugin(pluginName string) bool { // CSIMigration feature should be enabled along with the plugin-specific one - if !pm.featureGate.Enabled(features.CSIMigration) { - return false - } + // CSIMigration has been GA. It will be enabled by default. switch pluginName { case csilibplugins.AWSEBSInTreePluginName: @@ -163,11 +161,7 @@ func TranslateInTreeSpecToCSI(spec *volume.Spec, podNamespace string, translator // by looking up the pluginUnregister flag func CheckMigrationFeatureFlags(f featuregate.FeatureGate, pluginMigration, pluginUnregister featuregate.Feature) (migrationComplete bool, err error) { - if f.Enabled(pluginMigration) && !f.Enabled(features.CSIMigration) { - return false, fmt.Errorf("enabling %q requires CSIMigration to be enabled", pluginMigration) - } - - // This is for other in-tree plugin that get migration finished + // This is for in-tree plugin that get migration finished if f.Enabled(pluginMigration) && f.Enabled(pluginUnregister) { return true, nil } diff --git a/pkg/volume/csimigration/plugin_manager_test.go b/pkg/volume/csimigration/plugin_manager_test.go index f2f4d2ce26c..3cef7595d54 100644 --- a/pkg/volume/csimigration/plugin_manager_test.go +++ b/pkg/volume/csimigration/plugin_manager_test.go @@ -127,7 +127,6 @@ func TestIsMigratable(t *testing.T) { for _, test := range testCases { pm := NewPluginManager(csiTranslator, utilfeature.DefaultFeatureGate) t.Run(fmt.Sprintf("Testing %v", test.name), func(t *testing.T) { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSIMigration, test.csiMigrationEnabled)() defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, test.pluginFeature, test.pluginFeatureEnabled)() migratable, err := pm.IsMigratable(test.spec) if migratable != test.isMigratable { @@ -145,37 +144,24 @@ func TestCheckMigrationFeatureFlags(t *testing.T) { name string pluginFeature featuregate.Feature pluginFeatureEnabled bool - csiMigrationEnabled bool pluginUnregsiterFeature featuregate.Feature pluginUnregsiterEnabled bool expectMigrationComplete bool expectErr bool }{ { - name: "plugin specific feature flag enabled with migration flag disabled", + name: "plugin specific migration feature enabled with plugin unregister disabled", pluginFeature: features.CSIMigrationvSphere, pluginFeatureEnabled: true, - csiMigrationEnabled: false, - pluginUnregsiterFeature: features.InTreePluginvSphereUnregister, - pluginUnregsiterEnabled: false, - expectMigrationComplete: false, - expectErr: true, - }, - { - name: "plugin specific migration feature and CSI migration flag both enabled with plugin unregister disabled", - pluginFeature: features.CSIMigrationvSphere, - pluginFeatureEnabled: true, - csiMigrationEnabled: true, pluginUnregsiterFeature: features.InTreePluginvSphereUnregister, pluginUnregsiterEnabled: false, expectMigrationComplete: false, expectErr: false, }, { - name: "plugin specific migration feature and plugin unregister disabled and CSI migration flag enabled", + name: "plugin specific migration feature and plugin unregister disabled", pluginFeature: features.CSIMigrationvSphere, pluginFeatureEnabled: false, - csiMigrationEnabled: true, pluginUnregsiterFeature: features.InTreePluginvSphereUnregister, pluginUnregsiterEnabled: false, expectMigrationComplete: false, @@ -185,26 +171,14 @@ func TestCheckMigrationFeatureFlags(t *testing.T) { name: "all features enabled", pluginFeature: features.CSIMigrationvSphere, pluginFeatureEnabled: true, - csiMigrationEnabled: true, pluginUnregsiterFeature: features.InTreePluginvSphereUnregister, pluginUnregsiterEnabled: true, expectMigrationComplete: true, expectErr: false, }, - { - name: "all features disabled", - pluginFeature: features.CSIMigrationvSphere, - pluginFeatureEnabled: false, - csiMigrationEnabled: false, - pluginUnregsiterFeature: features.InTreePluginvSphereUnregister, - pluginUnregsiterEnabled: false, - expectMigrationComplete: false, - expectErr: false, - }, } for _, test := range testCases { t.Run(fmt.Sprintf("Testing %v", test.name), func(t *testing.T) { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSIMigration, test.csiMigrationEnabled)() defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, test.pluginFeature, test.pluginFeatureEnabled)() defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, test.pluginUnregsiterFeature, test.pluginUnregsiterEnabled)() migrationComplete, err := CheckMigrationFeatureFlags(utilfeature.DefaultFeatureGate, test.pluginFeature, test.pluginUnregsiterFeature) @@ -326,7 +300,6 @@ func TestMigrationFeatureFlagStatus(t *testing.T) { for _, test := range testCases { pm := NewPluginManager(csiTranslator, utilfeature.DefaultFeatureGate) t.Run(fmt.Sprintf("Testing %v", test.name), func(t *testing.T) { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSIMigration, test.csiMigrationEnabled)() defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, test.pluginFeature, test.pluginFeatureEnabled)() defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, test.inTreePluginUnregister, test.inTreePluginUnregisterEnabled)() diff --git a/pkg/volume/portworx/portworx.go b/pkg/volume/portworx/portworx.go index 33baef06f22..b5429b9de61 100644 --- a/pkg/volume/portworx/portworx.go +++ b/pkg/volume/portworx/portworx.go @@ -67,8 +67,7 @@ func getPath(uid types.UID, volName string, host volume.VolumeHost) string { } func (plugin *portworxVolumePlugin) IsMigratedToCSI() bool { - return utilfeature.DefaultFeatureGate.Enabled(features.CSIMigration) && - utilfeature.DefaultFeatureGate.Enabled(features.CSIMigrationPortworx) + return utilfeature.DefaultFeatureGate.Enabled(features.CSIMigrationPortworx) } func (plugin *portworxVolumePlugin) Init(host volume.VolumeHost) error { diff --git a/pkg/volume/rbd/rbd.go b/pkg/volume/rbd/rbd.go index 18c2bdf0920..dcfb0880442 100644 --- a/pkg/volume/rbd/rbd.go +++ b/pkg/volume/rbd/rbd.go @@ -80,8 +80,7 @@ func getPath(uid types.UID, volName string, host volume.VolumeHost) string { } func (plugin *rbdPlugin) IsMigratedToCSI() bool { - return utilfeature.DefaultFeatureGate.Enabled(features.CSIMigration) && - utilfeature.DefaultFeatureGate.Enabled(features.CSIMigrationRBD) + return utilfeature.DefaultFeatureGate.Enabled(features.CSIMigrationRBD) } func (plugin *rbdPlugin) Init(host volume.VolumeHost) error { diff --git a/pkg/volume/vsphere_volume/vsphere_volume.go b/pkg/volume/vsphere_volume/vsphere_volume.go index 0c6c37d6607..904f3688669 100644 --- a/pkg/volume/vsphere_volume/vsphere_volume.go +++ b/pkg/volume/vsphere_volume/vsphere_volume.go @@ -75,8 +75,7 @@ func (plugin *vsphereVolumePlugin) GetPluginName() string { } func (plugin *vsphereVolumePlugin) IsMigratedToCSI() bool { - return utilfeature.DefaultFeatureGate.Enabled(features.CSIMigration) && - utilfeature.DefaultFeatureGate.Enabled(features.CSIMigrationvSphere) + return utilfeature.DefaultFeatureGate.Enabled(features.CSIMigrationvSphere) } func (plugin *vsphereVolumePlugin) GetVolumeName(spec *volume.Spec) (string, error) { diff --git a/plugin/pkg/auth/authorizer/rbac/bootstrappolicy/controller_policy.go b/plugin/pkg/auth/authorizer/rbac/bootstrappolicy/controller_policy.go index 6002e6cd6a0..ed03ac7d5d7 100644 --- a/plugin/pkg/auth/authorizer/rbac/bootstrappolicy/controller_policy.go +++ b/plugin/pkg/auth/authorizer/rbac/bootstrappolicy/controller_policy.go @@ -75,9 +75,7 @@ func buildControllerRoles() ([]rbacv1.ClusterRole, []rbacv1.ClusterRoleBinding) } role.Rules = append(role.Rules, rbacv1helpers.NewRule("get", "watch", "list").Groups("storage.k8s.io").Resources("csidrivers").RuleOrDie()) - if utilfeature.DefaultFeatureGate.Enabled(features.CSIMigration) { - role.Rules = append(role.Rules, rbacv1helpers.NewRule("get", "watch", "list").Groups("storage.k8s.io").Resources("csinodes").RuleOrDie()) - } + role.Rules = append(role.Rules, rbacv1helpers.NewRule("get", "watch", "list").Groups("storage.k8s.io").Resources("csinodes").RuleOrDie()) return role }()) diff --git a/test/integration/volume/attach_detach_test.go b/test/integration/volume/attach_detach_test.go index 6f1b5e7e7f2..1a8267bb069 100644 --- a/test/integration/volume/attach_detach_test.go +++ b/test/integration/volume/attach_detach_test.go @@ -26,7 +26,6 @@ import ( "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/wait" - utilfeature "k8s.io/apiserver/pkg/util/feature" clientgoinformers "k8s.io/client-go/informers" clientset "k8s.io/client-go/kubernetes" restclient "k8s.io/client-go/rest" @@ -37,7 +36,6 @@ import ( volumecache "k8s.io/kubernetes/pkg/controller/volume/attachdetach/cache" "k8s.io/kubernetes/pkg/controller/volume/persistentvolume" persistentvolumeoptions "k8s.io/kubernetes/pkg/controller/volume/persistentvolume/options" - "k8s.io/kubernetes/pkg/features" "k8s.io/kubernetes/pkg/volume" volumetest "k8s.io/kubernetes/pkg/volume/testing" "k8s.io/kubernetes/pkg/volume/util" @@ -213,9 +211,7 @@ func TestPodDeletionWithDswp(t *testing.T) { } func initCSIObjects(stopCh <-chan struct{}, informers clientgoinformers.SharedInformerFactory) { - if utilfeature.DefaultFeatureGate.Enabled(features.CSIMigration) { - go informers.Storage().V1().CSINodes().Informer().Run(stopCh) - } + go informers.Storage().V1().CSINodes().Informer().Run(stopCh) go informers.Storage().V1().CSIDrivers().Informer().Run(stopCh) }