From 2d9a8f90b24a71b7c80a4284d1c2dfe9d358cbb6 Mon Sep 17 00:00:00 2001 From: Jan Safranek Date: Mon, 22 Mar 2021 14:36:01 +0100 Subject: [PATCH] UPSTREAM: : openshift KCM volume plugin manager uses a disjoint set of featuregates The volume plugin manager for openshfit's Attach Detach controller in kube-controller-manager uses a set of featuregates that are NOT the same as the the other controllers in KCM and the kubelet. This means these featuregates (if we kept the old names) would be inconsistent inside of a single binary. There are now separate featuregates for the volumepluginmanger when running in the Attach Detach controller to reflect this distintion. See openshift/enhancements#549 for details. Stop the patch when CSI migration becomes GA (i.e. features.CSIMigrationAWS / features.CSIMigrationOpenStack are GA). --- .../attachdetach/attach_detach_controller.go | 2 +- pkg/features/patch_kube_features.go | 30 +++++++++++++ .../csimigration/patch_adc_plugin_manager.go | 44 +++++++++++++++++++ pkg/volume/csimigration/plugin_manager.go | 6 +-- 4 files changed, 78 insertions(+), 4 deletions(-) create mode 100644 pkg/features/patch_kube_features.go create mode 100644 pkg/volume/csimigration/patch_adc_plugin_manager.go diff --git a/pkg/controller/volume/attachdetach/attach_detach_controller.go b/pkg/controller/volume/attachdetach/attach_detach_controller.go index e3545e3602c6d..e7a05690c0177 100644 --- a/pkg/controller/volume/attachdetach/attach_detach_controller.go +++ b/pkg/controller/volume/attachdetach/attach_detach_controller.go @@ -184,7 +184,7 @@ func NewAttachDetachController( csiTranslator := csitrans.New() adc.intreeToCSITranslator = csiTranslator - adc.csiMigratedPluginManager = csimigration.NewPluginManager(csiTranslator, utilfeature.DefaultFeatureGate) + adc.csiMigratedPluginManager = csimigration.NewADCPluginManager(csiTranslator, utilfeature.DefaultFeatureGate) adc.desiredStateOfWorldPopulator = populator.NewDesiredStateOfWorldPopulator( timerConfig.DesiredStateOfWorldPopulatorLoopSleepPeriod, diff --git a/pkg/features/patch_kube_features.go b/pkg/features/patch_kube_features.go new file mode 100644 index 0000000000000..f4f094f461c8a --- /dev/null +++ b/pkg/features/patch_kube_features.go @@ -0,0 +1,30 @@ +package features + +import ( + "k8s.io/apimachinery/pkg/util/runtime" + utilfeature "k8s.io/apiserver/pkg/util/feature" + "k8s.io/component-base/featuregate" +) + +var ( + // owner: @jsafrane + // alpha: v1.21 + // + // Enables the AWS EBS CSI migration for the Attach/Detach controller (ADC) only. + ADCCSIMigrationAWS featuregate.Feature = "ADC_CSIMigrationAWS" + + // owner: @jsafrane + // alpha: v1.21 + // + // Enables the Cinder CSI migration for the Attach/Detach controller (ADC) only. + ADCCSIMigrationCinder featuregate.Feature = "ADC_CSIMigrationCinder" +) + +var ocpDefaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureSpec{ + ADCCSIMigrationAWS: {Default: true, PreRelease: featuregate.Beta}, + ADCCSIMigrationCinder: {Default: true, PreRelease: featuregate.Beta}, +} + +func init() { + runtime.Must(utilfeature.DefaultMutableFeatureGate.Add(ocpDefaultKubernetesFeatureGates)) +} diff --git a/pkg/volume/csimigration/patch_adc_plugin_manager.go b/pkg/volume/csimigration/patch_adc_plugin_manager.go new file mode 100644 index 0000000000000..dc3a3bae86d66 --- /dev/null +++ b/pkg/volume/csimigration/patch_adc_plugin_manager.go @@ -0,0 +1,44 @@ +package csimigration + +import ( + "k8s.io/component-base/featuregate" + csilibplugins "k8s.io/csi-translation-lib/plugins" + "k8s.io/kubernetes/pkg/features" +) + +// NewADCPluginManager returns a new PluginManager instance for the Attach Detach controller which uses different +// featuregates in openshift to control enablement/disablement which *DO NOT MATCH* the featuregates for the rest of the +// cluster. +func NewADCPluginManager(m PluginNameMapper, featureGate featuregate.FeatureGate) PluginManager { + ret := NewPluginManager(m, featureGate) + ret.useADCPluginManagerFeatureGates = true + return ret +} + +// adcIsMigrationEnabledForPlugin indicates whether CSI migration has been enabled +// for a particular storage plugin in Attach/Detach controller. +func (pm PluginManager) adcIsMigrationEnabledForPlugin(pluginName string) bool { + // CSIMigration feature should be enabled along with the plugin-specific one + if !pm.featureGate.Enabled(features.CSIMigration) { + return false + } + + switch pluginName { + case csilibplugins.AWSEBSInTreePluginName: + return pm.featureGate.Enabled(features.ADCCSIMigrationAWS) + case csilibplugins.CinderInTreePluginName: + return pm.featureGate.Enabled(features.ADCCSIMigrationCinder) + default: + return pm.isMigrationEnabledForPlugin(pluginName) + } +} + +// IsMigrationEnabledForPlugin indicates whether CSI migration has been enabled +// for a particular storage plugin +func (pm PluginManager) IsMigrationEnabledForPlugin(pluginName string) bool { + if pm.useADCPluginManagerFeatureGates { + return pm.adcIsMigrationEnabledForPlugin(pluginName) + } + + return pm.isMigrationEnabledForPlugin(pluginName) +} diff --git a/pkg/volume/csimigration/plugin_manager.go b/pkg/volume/csimigration/plugin_manager.go index 1f32962583007..2cf8df980bb03 100644 --- a/pkg/volume/csimigration/plugin_manager.go +++ b/pkg/volume/csimigration/plugin_manager.go @@ -38,6 +38,8 @@ type PluginNameMapper interface { type PluginManager struct { PluginNameMapper featureGate featuregate.FeatureGate + + useADCPluginManagerFeatureGates bool } // NewPluginManager returns a new PluginManager instance @@ -77,9 +79,7 @@ func (pm PluginManager) IsMigrationCompleteForPlugin(pluginName string) bool { } } -// IsMigrationEnabledForPlugin indicates whether CSI migration has been enabled -// for a particular storage plugin -func (pm PluginManager) IsMigrationEnabledForPlugin(pluginName string) bool { +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