Merge pull request #101272 from Jiawei0227/deprecateflag

Remove CSIMigrationvSphereComplete flag
2025-07-24 12:15:52 +00:00 · 2021-06-05 10:40:38 -07:00 · 2021-06-05 10:40:38 -07:00 · 29a8105cec
commit 29a8105cec
parent e6a8bc9fbe fa1a4100c6
5 changed files with 74 additions and 118 deletions
--- a/cmd/kube-controller-manager/app/plugins_providers.go
+++ b/cmd/kube-controller-manager/app/plugins_providers.go
@ -36,20 +36,15 @@ import (
 type probeFn func() []volume.VolumePlugin

 func appendPluginBasedOnFeatureFlags(plugins []volume.VolumePlugin, inTreePluginName string, featureGate featuregate.FeatureGate, pluginInfo pluginInfo) ([]volume.VolumePlugin, error) {
-	// Skip appending the in-tree plugin to the list of plugins to be probed/initialized
-	// if the CSIMigration feature flag and plugin specific feature flag indicating
-	// CSI migration is complete
-	migrationComplete, err := csimigration.CheckMigrationFeatureFlags(featureGate, pluginInfo.pluginMigrationFeature,
-		pluginInfo.pluginMigrationCompleteFeature, pluginInfo.pluginUnregisterFeature)
+
+	_, err := csimigration.CheckMigrationFeatureFlags(featureGate, pluginInfo.pluginMigrationFeature, pluginInfo.pluginUnregisterFeature)
 	if err != nil {
 		klog.Warningf("Unexpected CSI Migration Feature Flags combination detected: %v. CSI Migration may not take effect", err)
 		// TODO: fail and return here once alpha only tests can set the feature flags for a plugin correctly
 	}
-	// TODO: This can be removed after feature flag CSIMigrationvSphereComplete is removed.
-	if migrationComplete {
-		klog.Infof("Skip registration of plugin %s since migration is complete", inTreePluginName)
-		return plugins, nil
-	}
+
+	// Skip appending the in-tree plugin to the list of plugins to be probed/initialized
+	// if the plugin unregister feature flag is set
 	if featureGate.Enabled(pluginInfo.pluginUnregisterFeature) {
 		klog.Infof("Skip registration of plugin %s since feature flag %v is enabled", inTreePluginName, pluginInfo.pluginUnregisterFeature)
 		return plugins, nil
@ -59,11 +54,9 @@ func appendPluginBasedOnFeatureFlags(plugins []volume.VolumePlugin, inTreePlugin
 }

 type pluginInfo struct {
-	pluginMigrationFeature featuregate.Feature
-	// deprecated, only to keep here for vSphere
-	pluginMigrationCompleteFeature featuregate.Feature
-	pluginUnregisterFeature        featuregate.Feature
-	pluginProbeFunction            probeFn
+	pluginMigrationFeature  featuregate.Feature
+	pluginUnregisterFeature featuregate.Feature
+	pluginProbeFunction     probeFn
 }

 func appendAttachableLegacyProviderVolumes(allPlugins []volume.VolumePlugin, featureGate featuregate.FeatureGate) ([]volume.VolumePlugin, error) {
@ -72,7 +65,7 @@ func appendAttachableLegacyProviderVolumes(allPlugins []volume.VolumePlugin, fea
 	pluginMigrationStatus[plugins.GCEPDInTreePluginName] = pluginInfo{pluginMigrationFeature: features.CSIMigrationGCE, pluginUnregisterFeature: features.InTreePluginGCEUnregister, pluginProbeFunction: gcepd.ProbeVolumePlugins}
 	pluginMigrationStatus[plugins.CinderInTreePluginName] = pluginInfo{pluginMigrationFeature: features.CSIMigrationOpenStack, pluginUnregisterFeature: features.InTreePluginOpenStackUnregister, pluginProbeFunction: cinder.ProbeVolumePlugins}
 	pluginMigrationStatus[plugins.AzureDiskInTreePluginName] = pluginInfo{pluginMigrationFeature: features.CSIMigrationAzureDisk, pluginUnregisterFeature: features.InTreePluginAzureDiskUnregister, pluginProbeFunction: azuredd.ProbeVolumePlugins}
-	pluginMigrationStatus[plugins.VSphereInTreePluginName] = pluginInfo{pluginMigrationFeature: features.CSIMigrationvSphere, pluginMigrationCompleteFeature: features.CSIMigrationvSphereComplete, pluginUnregisterFeature: features.InTreePluginvSphereUnregister, pluginProbeFunction: vsphere_volume.ProbeVolumePlugins}
+	pluginMigrationStatus[plugins.VSphereInTreePluginName] = pluginInfo{pluginMigrationFeature: features.CSIMigrationvSphere, pluginUnregisterFeature: features.InTreePluginvSphereUnregister, pluginProbeFunction: vsphere_volume.ProbeVolumePlugins}

 	var err error
 	for pluginName, pluginInfo := range pluginMigrationStatus {
--- a/cmd/kubelet/app/plugins_providers.go
+++ b/cmd/kubelet/app/plugins_providers.go
@ -42,20 +42,14 @@ type probeFn func() []volume.VolumePlugin

 func appendPluginBasedOnFeatureFlags(plugins []volume.VolumePlugin, inTreePluginName string,
 	featureGate featuregate.FeatureGate, pluginInfo pluginInfo) ([]volume.VolumePlugin, error) {
-	// Skip appending the in-tree plugin to the list of plugins to be probed/initialized
-	// if the CSIMigration feature flag and plugin specific feature flag indicating
-	// CSI migration is complete
-	migrationComplete, err := csimigration.CheckMigrationFeatureFlags(featureGate, pluginInfo.pluginMigrationFeature,
-		pluginInfo.pluginMigrationCompleteFeature, pluginInfo.pluginUnregisterFeature)
+	_, err := csimigration.CheckMigrationFeatureFlags(featureGate, pluginInfo.pluginMigrationFeature, pluginInfo.pluginUnregisterFeature)
 	if err != nil {
 		klog.InfoS("Unexpected CSI Migration Feature Flags combination detected, CSI Migration may not take effect", "err", err)
 		// TODO: fail and return here once alpha only tests can set the feature flags for a plugin correctly
 	}
-	// TODO: This can be removed after feature flag CSIMigrationvSphereComplete is removed.
-	if migrationComplete {
-		klog.InfoS("Skipped registration of plugin since migration is completed", "pluginName", inTreePluginName)
-		return plugins, nil
-	}
+
+	// Skip appending the in-tree plugin to the list of plugins to be probed/initialized
+	// if the plugin unregister feature flag is set
 	if featureGate.Enabled(pluginInfo.pluginUnregisterFeature) {
 		klog.InfoS("Skipped registration of plugin since feature flag is enabled", "pluginName", inTreePluginName, "featureFlag", pluginInfo.pluginUnregisterFeature)
 		return plugins, nil
@ -66,11 +60,9 @@ func appendPluginBasedOnFeatureFlags(plugins []volume.VolumePlugin, inTreePlugin
 }

 type pluginInfo struct {
-	pluginMigrationFeature featuregate.Feature
-	// deprecated, only to keep here for vSphere
-	pluginMigrationCompleteFeature featuregate.Feature
-	pluginUnregisterFeature        featuregate.Feature
-	pluginProbeFunction            probeFn
+	pluginMigrationFeature  featuregate.Feature
+	pluginUnregisterFeature featuregate.Feature
+	pluginProbeFunction     probeFn
 }

 func appendLegacyProviderVolumes(allPlugins []volume.VolumePlugin, featureGate featuregate.FeatureGate) ([]volume.VolumePlugin, error) {
@ -80,7 +72,7 @@ func appendLegacyProviderVolumes(allPlugins []volume.VolumePlugin, featureGate f
 	pluginMigrationStatus[plugins.CinderInTreePluginName] = pluginInfo{pluginMigrationFeature: features.CSIMigrationOpenStack, pluginUnregisterFeature: features.InTreePluginOpenStackUnregister, pluginProbeFunction: cinder.ProbeVolumePlugins}
 	pluginMigrationStatus[plugins.AzureDiskInTreePluginName] = pluginInfo{pluginMigrationFeature: features.CSIMigrationAzureDisk, pluginUnregisterFeature: features.InTreePluginAzureDiskUnregister, pluginProbeFunction: azuredd.ProbeVolumePlugins}
 	pluginMigrationStatus[plugins.AzureFileInTreePluginName] = pluginInfo{pluginMigrationFeature: features.CSIMigrationAzureFile, pluginUnregisterFeature: features.InTreePluginAzureFileUnregister, pluginProbeFunction: azure_file.ProbeVolumePlugins}
-	pluginMigrationStatus[plugins.VSphereInTreePluginName] = pluginInfo{pluginMigrationFeature: features.CSIMigrationvSphere, pluginMigrationCompleteFeature: features.CSIMigrationvSphereComplete, pluginUnregisterFeature: features.InTreePluginvSphereUnregister, pluginProbeFunction: vsphere_volume.ProbeVolumePlugins}
+	pluginMigrationStatus[plugins.VSphereInTreePluginName] = pluginInfo{pluginMigrationFeature: features.CSIMigrationvSphere, pluginUnregisterFeature: features.InTreePluginvSphereUnregister, pluginProbeFunction: vsphere_volume.ProbeVolumePlugins}

 	var err error
 	for pluginName, pluginInfo := range pluginMigrationStatus {
--- a/pkg/features/kube_features.go
+++ b/pkg/features/kube_features.go
@ -375,13 +375,6 @@ const (
 	// Enables the vSphere in-tree driver to vSphere CSI Driver migration feature.
 	CSIMigrationvSphere featuregate.Feature = "CSIMigrationvSphere"

-	// owner: @divyenpatel
-	// beta: v1.19 (requires: vSphere vCenter/ESXi Version: 7.0u1, HW Version: VM version 15)
-	//
-	// Disables the vSphere in-tree driver.
-	// Expects vSphere CSI Driver to be installed and configured on all nodes.
-	CSIMigrationvSphereComplete featuregate.Feature = "CSIMigrationvSphereComplete"
-
 	// owner: @divyenpatel
 	// alpha: v1.21
 	//
@ -802,7 +795,6 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS
 	CSIMigrationAzureFile:                          {Default: false, PreRelease: featuregate.Beta}, // Off by default (requires Azure File CSI driver)
 	InTreePluginAzureFileUnregister:                {Default: false, PreRelease: featuregate.Alpha},
 	CSIMigrationvSphere:                            {Default: false, PreRelease: featuregate.Beta}, // Off by default (requires vSphere CSI driver)
-	CSIMigrationvSphereComplete:                    {Default: false, PreRelease: featuregate.Beta}, // remove in 1.22
 	InTreePluginvSphereUnregister:                  {Default: false, PreRelease: featuregate.Alpha},
 	CSIMigrationOpenStack:                          {Default: true, PreRelease: featuregate.Beta},
 	InTreePluginOpenStackUnregister:                {Default: false, PreRelease: featuregate.Alpha},
--- a/pkg/volume/csimigration/plugin_manager.go
+++ b/pkg/volume/csimigration/plugin_manager.go
@ -71,7 +71,7 @@ func (pm PluginManager) IsMigrationCompleteForPlugin(pluginName string) bool {
 	case csilibplugins.CinderInTreePluginName:
 		return pm.featureGate.Enabled(features.InTreePluginOpenStackUnregister)
 	case csilibplugins.VSphereInTreePluginName:
-		return pm.featureGate.Enabled(features.CSIMigrationvSphereComplete) || pm.featureGate.Enabled(features.InTreePluginvSphereUnregister)
+		return pm.featureGate.Enabled(features.InTreePluginvSphereUnregister)
 	default:
 		return false
 	}
@ -152,21 +152,13 @@ func TranslateInTreeSpecToCSI(spec *volume.Spec, podNamespace string, translator

 // CheckMigrationFeatureFlags checks the configuration of feature flags related
 // to CSI Migration is valid. It will return whether the migration is complete
-// by looking up the pluginMigrationComplete and pluginUnregister flag
+// by looking up the pluginUnregister flag
 func CheckMigrationFeatureFlags(f featuregate.FeatureGate, pluginMigration,
-	pluginMigrationComplete, pluginUnregister featuregate.Feature) (migrationComplete bool, err error) {
+	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)
 	}
-	// TODO: Remove the following two checks once the CSIMigrationXXComplete flag is removed
-	if pluginMigrationComplete != "" && f.Enabled(pluginMigrationComplete) && !f.Enabled(pluginMigration) {
-		return false, fmt.Errorf("enabling %q requires %q to be enabled", pluginMigrationComplete, pluginMigration)
-	}
-	// This is only needed for vSphere since we will deprecate the CSIMigrationvSphereComplete flag soon
-	if pluginMigrationComplete != "" && f.Enabled(features.CSIMigration) &&
-		f.Enabled(pluginMigration) && f.Enabled(pluginMigrationComplete) {
-		return true, nil
-	}
+
 	// This is for other in-tree plugin that get migration finished
 	if f.Enabled(pluginMigration) && f.Enabled(pluginUnregister) {
 		return true, nil
--- a/pkg/volume/csimigration/plugin_manager_test.go
+++ b/pkg/volume/csimigration/plugin_manager_test.go
@ -142,74 +142,64 @@ func TestIsMigratable(t *testing.T) {

 func TestCheckMigrationFeatureFlags(t *testing.T) {
 	testCases := []struct {
-		name                         string
-		pluginFeature                featuregate.Feature
-		pluginFeatureEnabled         bool
-		csiMigrationEnabled          bool
-		pluginFeatureComplete        featuregate.Feature
-		pluginFeatureCompleteEnabled bool
-		pluginUnregsiterFeature      featuregate.Feature
-		pluginUnregsiterEnabled      bool
-		expectMigrationComplete      bool
-		expectErr                    bool
+		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",
-			pluginFeature:                features.CSIMigrationvSphere,
-			pluginFeatureEnabled:         true,
-			csiMigrationEnabled:          false,
-			pluginFeatureComplete:        features.CSIMigrationvSphereComplete,
-			pluginFeatureCompleteEnabled: false,
-			pluginUnregsiterFeature:      features.InTreePluginvSphereUnregister,
-			pluginUnregsiterEnabled:      false,
-			expectMigrationComplete:      false,
-			expectErr:                    false,
+			name:                    "plugin specific feature flag enabled with migration flag disabled",
+			pluginFeature:           features.CSIMigrationvSphere,
+			pluginFeatureEnabled:    true,
+			csiMigrationEnabled:     false,
+			pluginUnregsiterFeature: features.InTreePluginvSphereUnregister,
+			pluginUnregsiterEnabled: false,
+			expectMigrationComplete: false,
+			expectErr:               true,
 		},
 		{
-			name:                         "plugin specific complete flag enabled but plugin specific feature flag disabled",
-			pluginFeature:                features.CSIMigrationvSphere,
-			pluginFeatureEnabled:         false,
-			csiMigrationEnabled:          true,
-			pluginFeatureComplete:        features.CSIMigrationvSphereComplete,
-			pluginFeatureCompleteEnabled: true,
-			pluginUnregsiterFeature:      features.InTreePluginvSphereUnregister,
-			pluginUnregsiterEnabled:      false,
-			expectMigrationComplete:      false,
-			expectErr:                    false,
-		},
-		{
-			name:                         "plugin specific complete feature disabled but plugin specific migration feature and CSI migration flag enabled",
-			pluginFeature:                features.CSIMigrationvSphere,
-			pluginFeatureEnabled:         true,
-			csiMigrationEnabled:          true,
-			pluginFeatureComplete:        features.CSIMigrationvSphereComplete,
-			pluginFeatureCompleteEnabled: false,
-			pluginUnregsiterFeature:      features.InTreePluginvSphereUnregister,
-			pluginUnregsiterEnabled:      false,
-			expectMigrationComplete:      false,
-			expectErr:                    true,
-		},
-		{
-			name:                         "all features enabled",
-			pluginFeature:                features.CSIMigrationvSphere,
-			pluginFeatureEnabled:         true,
-			csiMigrationEnabled:          true,
-			pluginFeatureComplete:        features.CSIMigrationvSphereComplete,
-			pluginFeatureCompleteEnabled: true,
-			pluginUnregsiterFeature:      features.InTreePluginvSphereUnregister,
-			pluginUnregsiterEnabled:      true,
-			expectMigrationComplete:      true,
-			expectErr:                    true,
-		},
-		{
-			name:                    "no plugin feature complete set",
-			pluginFeature:           features.CSIMigrationGCE,
+			name:                    "plugin specific migration feature and CSI migration flag both enabled with plugin unregister disabled",
+			pluginFeature:           features.CSIMigrationvSphere,
 			pluginFeatureEnabled:    true,
 			csiMigrationEnabled:     true,
-			pluginUnregsiterFeature: features.InTreePluginGCEUnregister,
+			pluginUnregsiterFeature: features.InTreePluginvSphereUnregister,
+			pluginUnregsiterEnabled: false,
+			expectMigrationComplete: false,
+			expectErr:               false,
+		},
+		{
+			name:                    "plugin specific migration feature and plugin unregister disabled and CSI migration flag enabled",
+			pluginFeature:           features.CSIMigrationvSphere,
+			pluginFeatureEnabled:    false,
+			csiMigrationEnabled:     true,
+			pluginUnregsiterFeature: features.InTreePluginvSphereUnregister,
+			pluginUnregsiterEnabled: false,
+			expectMigrationComplete: false,
+			expectErr:               false,
+		},
+		{
+			name:                    "all features enabled",
+			pluginFeature:           features.CSIMigrationvSphere,
+			pluginFeatureEnabled:    true,
+			csiMigrationEnabled:     true,
+			pluginUnregsiterFeature: features.InTreePluginvSphereUnregister,
 			pluginUnregsiterEnabled: true,
 			expectMigrationComplete: true,
-			expectErr:               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 {
@ -217,14 +207,11 @@ func TestCheckMigrationFeatureFlags(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)()
-			if test.pluginFeatureComplete != "" {
-				defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, test.pluginFeatureComplete, test.pluginFeatureCompleteEnabled)()
-			}
-			migrationComplete, err := CheckMigrationFeatureFlags(utilfeature.DefaultFeatureGate, test.pluginFeature, test.pluginFeatureComplete, test.pluginUnregsiterFeature)
-			if err != nil && test.expectErr == true {
+			migrationComplete, err := CheckMigrationFeatureFlags(utilfeature.DefaultFeatureGate, test.pluginFeature, test.pluginUnregsiterFeature)
+			if err != nil && test.expectErr == false {
 				t.Errorf("Unexpected error: %v", err)
 			}
-			if err == nil && test.expectErr == false {
+			if err == nil && test.expectErr == true {
 				t.Errorf("Unexpected validation pass")
 			}
 			if migrationComplete != test.expectMigrationComplete {