Skip to content

Commit

Permalink
UPSTREAM: <carry>: openshift KCM volume plugin manager uses a disjoin…
Browse files Browse the repository at this point in the history
…t 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 <carrying> the patch when CSI migration becomes GA (i.e.
features.CSIMigrationAWS / features.CSIMigrationOpenStack are GA).

UPSTREAM: <carry>: add CSI migration feature gates for GCE PD and Azure Disk

This commit is the next natural step for commit 2d9a8f9. It
introduces custom feature gates to enable the CSI migration in
GCE PD and Azure Disk plugins.

See openshift/enhancements#549 for details.

Stop <carrying> the patch when CSI migration becomes GA (i.e.
features.CSIMigrationAzureDisk / features.CSIMigrationGCE are GA).

 UPSTREAM: <carry>: Set CSI migration off when a test needs it

In OCP we carry a patch that forces CSI migration to be enabled in
Attach/Detach controller (ADC). Update ADC unit tests to disable the
migration there when an unit test needs it disabled.

openshift-rebase(v1.24):source=ec8a203cd68
  • Loading branch information
jsafrane authored and soltysh committed Aug 22, 2022
1 parent 1ec3b00 commit 88c4f34
Show file tree
Hide file tree
Showing 5 changed files with 103 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,14 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/labels"
"k8s.io/apimachinery/pkg/types"
utilfeature "k8s.io/apiserver/pkg/util/feature"
"k8s.io/client-go/informers"
kcache "k8s.io/client-go/tools/cache"
featuregatetesting "k8s.io/component-base/featuregate/testing"
"k8s.io/kubernetes/pkg/controller"
"k8s.io/kubernetes/pkg/controller/volume/attachdetach/cache"
controllervolumetesting "k8s.io/kubernetes/pkg/controller/volume/attachdetach/testing"
"k8s.io/kubernetes/pkg/features"
"k8s.io/kubernetes/pkg/volume"
"k8s.io/kubernetes/pkg/volume/csi"
"k8s.io/kubernetes/pkg/volume/util"
Expand Down Expand Up @@ -431,6 +434,10 @@ func volumeAttachmentRecoveryTestCase(t *testing.T, tc vaTest) {
plugins = append(plugins, controllervolumetesting.CreateTestPlugin()...)
plugins = append(plugins, csi.ProbeVolumePlugins()...)

// OCP Carry: disable ADCCSIMigrationGCEPD feature in this unit test when necessary.
// OCP forces CSI migration in ADC "on", this unit test may need it "off".
defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.ADCCSIMigrationGCEPD, tc.csiMigration)()

nodeInformer := informerFactory.Core().V1().Nodes().Informer()
podInformer := informerFactory.Core().V1().Pods().Informer()
pvInformer := informerFactory.Core().V1().PersistentVolumes().Informer()
Expand Down
44 changes: 44 additions & 0 deletions pkg/features/patch_kube_features.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
package features

import (
"k8s.io/apimachinery/pkg/util/runtime"
utilfeature "k8s.io/apiserver/pkg/util/feature"
"k8s.io/component-base/featuregate"
)

var (
// owner: @jsafrane
// beta: v1.21
//
// Enables the AWS EBS CSI migration for the Attach/Detach controller (ADC) only.
ADCCSIMigrationAWS featuregate.Feature = "ADC_CSIMigrationAWS"

// owner: @fbertina
// beta: v1.22
//
// Enables the Azure Disk CSI migration for the Attach/Detach controller (ADC) only.
ADCCSIMigrationAzureDisk featuregate.Feature = "ADC_CSIMigrationAzureDisk"

// owner: @jsafrane
// beta: v1.21
//
// Enables the Cinder CSI migration for the Attach/Detach controller (ADC) only.
ADCCSIMigrationCinder featuregate.Feature = "ADC_CSIMigrationCinder"

// owner: @fbertina
// beta: v1.22
//
// Enables the GCE CSI migration for the Attach/Detach controller (ADC) only.
ADCCSIMigrationGCEPD featuregate.Feature = "ADC_CSIMigrationGCEPD"
)

var ocpDefaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureSpec{
ADCCSIMigrationAWS: {Default: true, PreRelease: featuregate.Beta},
ADCCSIMigrationAzureDisk: {Default: true, PreRelease: featuregate.Beta},
ADCCSIMigrationCinder: {Default: true, PreRelease: featuregate.Beta},
ADCCSIMigrationGCEPD: {Default: true, PreRelease: featuregate.Beta},
}

func init() {
runtime.Must(utilfeature.DefaultMutableFeatureGate.Add(ocpDefaultKubernetesFeatureGates))
}
48 changes: 48 additions & 0 deletions pkg/volume/csimigration/patch_adc_plugin_manager.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
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.AzureDiskInTreePluginName:
return pm.featureGate.Enabled(features.ADCCSIMigrationAzureDisk)
case csilibplugins.CinderInTreePluginName:
return pm.featureGate.Enabled(features.ADCCSIMigrationCinder)
case csilibplugins.GCEPDInTreePluginName:
return pm.featureGate.Enabled(features.ADCCSIMigrationGCEPD)
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)
}
6 changes: 3 additions & 3 deletions pkg/volume/csimigration/plugin_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ type PluginNameMapper interface {
type PluginManager struct {
PluginNameMapper
featureGate featuregate.FeatureGate

useADCPluginManagerFeatureGates bool
}

// NewPluginManager returns a new PluginManager instance
Expand Down Expand Up @@ -81,9 +83,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
// CSIMigration has been GA. It will be enabled by default.

Expand Down

0 comments on commit 88c4f34

Please sign in to comment.