Skip to content

Commit 3baaa16

Browse files
jsafranesoltysh
authored andcommitted
UPSTREAM: <carry>: 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 <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
1 parent aa9dde2 commit 3baaa16

File tree

5 files changed

+103
-4
lines changed

5 files changed

+103
-4
lines changed

Diff for: pkg/controller/volume/attachdetach/attach_detach_controller.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,7 @@ func NewAttachDetachController(
180180

181181
csiTranslator := csitrans.New()
182182
adc.intreeToCSITranslator = csiTranslator
183-
adc.csiMigratedPluginManager = csimigration.NewPluginManager(csiTranslator, utilfeature.DefaultFeatureGate)
183+
adc.csiMigratedPluginManager = csimigration.NewADCPluginManager(csiTranslator, utilfeature.DefaultFeatureGate)
184184

185185
adc.desiredStateOfWorldPopulator = populator.NewDesiredStateOfWorldPopulator(
186186
timerConfig.DesiredStateOfWorldPopulatorLoopSleepPeriod,

Diff for: pkg/controller/volume/attachdetach/attach_detach_controller_test.go

+7
Original file line numberDiff line numberDiff line change
@@ -26,11 +26,14 @@ import (
2626
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2727
"k8s.io/apimachinery/pkg/labels"
2828
"k8s.io/apimachinery/pkg/types"
29+
utilfeature "k8s.io/apiserver/pkg/util/feature"
2930
"k8s.io/client-go/informers"
3031
kcache "k8s.io/client-go/tools/cache"
32+
featuregatetesting "k8s.io/component-base/featuregate/testing"
3133
"k8s.io/kubernetes/pkg/controller"
3234
"k8s.io/kubernetes/pkg/controller/volume/attachdetach/cache"
3335
controllervolumetesting "k8s.io/kubernetes/pkg/controller/volume/attachdetach/testing"
36+
"k8s.io/kubernetes/pkg/features"
3437
"k8s.io/kubernetes/pkg/volume"
3538
"k8s.io/kubernetes/pkg/volume/csi"
3639
"k8s.io/kubernetes/pkg/volume/util"
@@ -431,6 +434,10 @@ func volumeAttachmentRecoveryTestCase(t *testing.T, tc vaTest) {
431434
plugins = append(plugins, controllervolumetesting.CreateTestPlugin()...)
432435
plugins = append(plugins, csi.ProbeVolumePlugins()...)
433436

437+
// OCP Carry: disable ADCCSIMigrationGCEPD feature in this unit test when necessary.
438+
// OCP forces CSI migration in ADC "on", this unit test may need it "off".
439+
defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.ADCCSIMigrationGCEPD, tc.csiMigration)()
440+
434441
nodeInformer := informerFactory.Core().V1().Nodes().Informer()
435442
podInformer := informerFactory.Core().V1().Pods().Informer()
436443
pvInformer := informerFactory.Core().V1().PersistentVolumes().Informer()

Diff for: pkg/features/patch_kube_features.go

+44
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
package features
2+
3+
import (
4+
"k8s.io/apimachinery/pkg/util/runtime"
5+
utilfeature "k8s.io/apiserver/pkg/util/feature"
6+
"k8s.io/component-base/featuregate"
7+
)
8+
9+
var (
10+
// owner: @jsafrane
11+
// beta: v1.21
12+
//
13+
// Enables the AWS EBS CSI migration for the Attach/Detach controller (ADC) only.
14+
ADCCSIMigrationAWS featuregate.Feature = "ADC_CSIMigrationAWS"
15+
16+
// owner: @fbertina
17+
// beta: v1.22
18+
//
19+
// Enables the Azure Disk CSI migration for the Attach/Detach controller (ADC) only.
20+
ADCCSIMigrationAzureDisk featuregate.Feature = "ADC_CSIMigrationAzureDisk"
21+
22+
// owner: @jsafrane
23+
// beta: v1.21
24+
//
25+
// Enables the Cinder CSI migration for the Attach/Detach controller (ADC) only.
26+
ADCCSIMigrationCinder featuregate.Feature = "ADC_CSIMigrationCinder"
27+
28+
// owner: @fbertina
29+
// beta: v1.22
30+
//
31+
// Enables the GCE CSI migration for the Attach/Detach controller (ADC) only.
32+
ADCCSIMigrationGCEPD featuregate.Feature = "ADC_CSIMigrationGCEPD"
33+
)
34+
35+
var ocpDefaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureSpec{
36+
ADCCSIMigrationAWS: {Default: true, PreRelease: featuregate.Beta},
37+
ADCCSIMigrationAzureDisk: {Default: true, PreRelease: featuregate.Beta},
38+
ADCCSIMigrationCinder: {Default: true, PreRelease: featuregate.Beta},
39+
ADCCSIMigrationGCEPD: {Default: true, PreRelease: featuregate.Beta},
40+
}
41+
42+
func init() {
43+
runtime.Must(utilfeature.DefaultMutableFeatureGate.Add(ocpDefaultKubernetesFeatureGates))
44+
}

Diff for: pkg/volume/csimigration/patch_adc_plugin_manager.go

+48
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
package csimigration
2+
3+
import (
4+
"k8s.io/component-base/featuregate"
5+
csilibplugins "k8s.io/csi-translation-lib/plugins"
6+
"k8s.io/kubernetes/pkg/features"
7+
)
8+
9+
// NewADCPluginManager returns a new PluginManager instance for the Attach Detach controller which uses different
10+
// featuregates in openshift to control enablement/disablement which *DO NOT MATCH* the featuregates for the rest of the
11+
// cluster.
12+
func NewADCPluginManager(m PluginNameMapper, featureGate featuregate.FeatureGate) PluginManager {
13+
ret := NewPluginManager(m, featureGate)
14+
ret.useADCPluginManagerFeatureGates = true
15+
return ret
16+
}
17+
18+
// adcIsMigrationEnabledForPlugin indicates whether CSI migration has been enabled
19+
// for a particular storage plugin in Attach/Detach controller.
20+
func (pm PluginManager) adcIsMigrationEnabledForPlugin(pluginName string) bool {
21+
// CSIMigration feature should be enabled along with the plugin-specific one
22+
if !pm.featureGate.Enabled(features.CSIMigration) {
23+
return false
24+
}
25+
26+
switch pluginName {
27+
case csilibplugins.AWSEBSInTreePluginName:
28+
return pm.featureGate.Enabled(features.ADCCSIMigrationAWS)
29+
case csilibplugins.AzureDiskInTreePluginName:
30+
return pm.featureGate.Enabled(features.ADCCSIMigrationAzureDisk)
31+
case csilibplugins.CinderInTreePluginName:
32+
return pm.featureGate.Enabled(features.ADCCSIMigrationCinder)
33+
case csilibplugins.GCEPDInTreePluginName:
34+
return pm.featureGate.Enabled(features.ADCCSIMigrationGCEPD)
35+
default:
36+
return pm.isMigrationEnabledForPlugin(pluginName)
37+
}
38+
}
39+
40+
// IsMigrationEnabledForPlugin indicates whether CSI migration has been enabled
41+
// for a particular storage plugin
42+
func (pm PluginManager) IsMigrationEnabledForPlugin(pluginName string) bool {
43+
if pm.useADCPluginManagerFeatureGates {
44+
return pm.adcIsMigrationEnabledForPlugin(pluginName)
45+
}
46+
47+
return pm.isMigrationEnabledForPlugin(pluginName)
48+
}

Diff for: pkg/volume/csimigration/plugin_manager.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,8 @@ type PluginNameMapper interface {
3838
type PluginManager struct {
3939
PluginNameMapper
4040
featureGate featuregate.FeatureGate
41+
42+
useADCPluginManagerFeatureGates bool
4143
}
4244

4345
// NewPluginManager returns a new PluginManager instance
@@ -81,9 +83,7 @@ func (pm PluginManager) IsMigrationCompleteForPlugin(pluginName string) bool {
8183
}
8284
}
8385

84-
// IsMigrationEnabledForPlugin indicates whether CSI migration has been enabled
85-
// for a particular storage plugin
86-
func (pm PluginManager) IsMigrationEnabledForPlugin(pluginName string) bool {
86+
func (pm PluginManager) isMigrationEnabledForPlugin(pluginName string) bool {
8787
// CSIMigration feature should be enabled along with the plugin-specific one
8888
// CSIMigration has been GA. It will be enabled by default.
8989

0 commit comments

Comments
 (0)