Refactor CSI migration plugin manager to get featureGates as a parameter

This allows caller to provide fake ones for testing of various corner cases
(migration on A/D controller disabled while enabled on kubelet).
This commit is contained in:
Jan Safranek 2021-03-08 13:49:57 +01:00
parent a517eccd9f
commit 219cbc818a
11 changed files with 33 additions and 28 deletions

View File

@ -379,7 +379,7 @@ func startVolumeExpandController(ctx ControllerContext) (http.Handler, bool, err
ctx.Cloud, ctx.Cloud,
plugins, plugins,
csiTranslator, csiTranslator,
csimigration.NewPluginManager(csiTranslator), csimigration.NewPluginManager(csiTranslator, utilfeature.DefaultFeatureGate),
filteredDialOptions, filteredDialOptions,
) )

View File

@ -184,7 +184,7 @@ func NewAttachDetachController(
csiTranslator := csitrans.New() csiTranslator := csitrans.New()
adc.intreeToCSITranslator = csiTranslator adc.intreeToCSITranslator = csiTranslator
adc.csiMigratedPluginManager = csimigration.NewPluginManager(csiTranslator) adc.csiMigratedPluginManager = csimigration.NewPluginManager(csiTranslator, utilfeature.DefaultFeatureGate)
adc.desiredStateOfWorldPopulator = populator.NewDesiredStateOfWorldPopulator( adc.desiredStateOfWorldPopulator = populator.NewDesiredStateOfWorldPopulator(
timerConfig.DesiredStateOfWorldPopulatorLoopSleepPeriod, timerConfig.DesiredStateOfWorldPopulatorLoopSleepPeriod,

View File

@ -23,6 +23,7 @@ import (
"k8s.io/apimachinery/pkg/api/resource" "k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
k8stypes "k8s.io/apimachinery/pkg/types" k8stypes "k8s.io/apimachinery/pkg/types"
utilfeature "k8s.io/apiserver/pkg/util/feature"
"k8s.io/client-go/informers" "k8s.io/client-go/informers"
"k8s.io/client-go/kubernetes/fake" "k8s.io/client-go/kubernetes/fake"
csitrans "k8s.io/csi-translation-lib" csitrans "k8s.io/csi-translation-lib"
@ -117,7 +118,7 @@ func TestVolumesInUseMetricCollection(t *testing.T) {
nil, nil,
nil, nil,
fakeVolumePluginMgr, fakeVolumePluginMgr,
csimigration.NewPluginManager(csiTranslator), csimigration.NewPluginManager(csiTranslator, utilfeature.DefaultFeatureGate),
csiTranslator) csiTranslator)
nodeUseMap := metricCollector.getVolumeInUseCount() nodeUseMap := metricCollector.getVolumeInUseCount()
if len(nodeUseMap) < 1 { if len(nodeUseMap) < 1 {
@ -158,7 +159,7 @@ func TestTotalVolumesMetricCollection(t *testing.T) {
asw, asw,
dsw, dsw,
fakeVolumePluginMgr, fakeVolumePluginMgr,
csimigration.NewPluginManager(csiTranslator), csimigration.NewPluginManager(csiTranslator, utilfeature.DefaultFeatureGate),
csiTranslator) csiTranslator)
totalVolumesMap := metricCollector.getTotalVolumesCount() totalVolumesMap := metricCollector.getTotalVolumesCount()

View File

@ -23,6 +23,7 @@ import (
v1 "k8s.io/api/core/v1" v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
k8stypes "k8s.io/apimachinery/pkg/types" k8stypes "k8s.io/apimachinery/pkg/types"
utilfeature "k8s.io/apiserver/pkg/util/feature"
"k8s.io/client-go/informers" "k8s.io/client-go/informers"
"k8s.io/client-go/kubernetes/fake" "k8s.io/client-go/kubernetes/fake"
csitrans "k8s.io/csi-translation-lib" csitrans "k8s.io/csi-translation-lib"
@ -84,7 +85,7 @@ func TestFindAndAddActivePods_FindAndRemoveDeletedPods(t *testing.T) {
podLister: fakePodInformer.Lister(), podLister: fakePodInformer.Lister(),
pvcLister: pvcLister, pvcLister: pvcLister,
pvLister: pvLister, pvLister: pvLister,
csiMigratedPluginManager: csimigration.NewPluginManager(csiTranslator), csiMigratedPluginManager: csimigration.NewPluginManager(csiTranslator, utilfeature.DefaultFeatureGate),
intreeToCSITranslator: csiTranslator, intreeToCSITranslator: csiTranslator,
} }
@ -189,7 +190,7 @@ func TestFindAndRemoveNonattachableVolumes(t *testing.T) {
podLister: fakePodInformer.Lister(), podLister: fakePodInformer.Lister(),
pvcLister: pvcLister, pvcLister: pvcLister,
pvLister: pvLister, pvLister: pvLister,
csiMigratedPluginManager: csimigration.NewPluginManager(csiTranslator), csiMigratedPluginManager: csimigration.NewPluginManager(csiTranslator, utilfeature.DefaultFeatureGate),
intreeToCSITranslator: csiTranslator, intreeToCSITranslator: csiTranslator,
} }

View File

@ -105,7 +105,7 @@ func TestSyncHandler(t *testing.T) {
allPlugins := []volume.VolumePlugin{} allPlugins := []volume.VolumePlugin{}
allPlugins = append(allPlugins, awsebs.ProbeVolumePlugins()...) allPlugins = append(allPlugins, awsebs.ProbeVolumePlugins()...)
translator := csitrans.New() translator := csitrans.New()
expc, err := NewExpandController(fakeKubeClient, pvcInformer, pvInformer, nil, allPlugins, translator, csimigration.NewPluginManager(translator), nil) expc, err := NewExpandController(fakeKubeClient, pvcInformer, pvInformer, nil, allPlugins, translator, csimigration.NewPluginManager(translator, utilfeature.DefaultFeatureGate), nil)
if err != nil { if err != nil {
t.Fatalf("error creating expand controller : %v", err) t.Fatalf("error creating expand controller : %v", err)
} }

View File

@ -29,6 +29,7 @@ import (
"k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/labels"
utilruntime "k8s.io/apimachinery/pkg/util/runtime" utilruntime "k8s.io/apimachinery/pkg/util/runtime"
"k8s.io/apimachinery/pkg/util/wait" "k8s.io/apimachinery/pkg/util/wait"
utilfeature "k8s.io/apiserver/pkg/util/feature"
coreinformers "k8s.io/client-go/informers/core/v1" coreinformers "k8s.io/client-go/informers/core/v1"
storageinformers "k8s.io/client-go/informers/storage/v1" storageinformers "k8s.io/client-go/informers/storage/v1"
clientset "k8s.io/client-go/kubernetes" clientset "k8s.io/client-go/kubernetes"
@ -142,7 +143,7 @@ func NewController(p ControllerParameters) (*PersistentVolumeController, error)
csiTranslator := csitrans.New() csiTranslator := csitrans.New()
controller.translator = csiTranslator controller.translator = csiTranslator
controller.csiMigratedPluginManager = csimigration.NewPluginManager(csiTranslator) controller.csiMigratedPluginManager = csimigration.NewPluginManager(csiTranslator, utilfeature.DefaultFeatureGate)
controller.filteredDialOptions = p.FilteredDialOptions controller.filteredDialOptions = p.FilteredDialOptions

View File

@ -604,7 +604,7 @@ func TestAnnealMigrationAnnotations(t *testing.T) {
} }
translator := csitrans.New() translator := csitrans.New()
cmpm := csimigration.NewPluginManager(translator) cmpm := csimigration.NewPluginManager(translator, utilfeature.DefaultFeatureGate)
for _, tc := range tests { for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) { t.Run(tc.name, func(t *testing.T) {

View File

@ -1303,7 +1303,7 @@ func createDswpWithVolumeWithCustomPluginMgr(t *testing.T, pv *v1.PersistentVolu
processedPods: make(map[types.UniquePodName]bool)}, processedPods: make(map[types.UniquePodName]bool)},
kubeContainerRuntime: fakeRuntime, kubeContainerRuntime: fakeRuntime,
keepTerminatedPodVolumes: false, keepTerminatedPodVolumes: false,
csiMigratedPluginManager: csimigration.NewPluginManager(csiTranslator), csiMigratedPluginManager: csimigration.NewPluginManager(csiTranslator, utilfeature.DefaultFeatureGate),
intreeToCSITranslator: csiTranslator, intreeToCSITranslator: csiTranslator,
volumePluginMgr: fakeVolumePluginMgr, volumePluginMgr: fakeVolumePluginMgr,
} }

View File

@ -24,6 +24,7 @@ import (
"strings" "strings"
"time" "time"
utilfeature "k8s.io/apiserver/pkg/util/feature"
"k8s.io/klog/v2" "k8s.io/klog/v2"
"k8s.io/mount-utils" "k8s.io/mount-utils"
@ -178,7 +179,7 @@ func NewVolumeManager(
} }
intreeToCSITranslator := csitrans.New() intreeToCSITranslator := csitrans.New()
csiMigratedPluginManager := csimigration.NewPluginManager(intreeToCSITranslator) csiMigratedPluginManager := csimigration.NewPluginManager(intreeToCSITranslator, utilfeature.DefaultFeatureGate)
vm.intreeToCSITranslator = intreeToCSITranslator vm.intreeToCSITranslator = intreeToCSITranslator
vm.csiMigratedPluginManager = csiMigratedPluginManager vm.csiMigratedPluginManager = csiMigratedPluginManager

View File

@ -21,7 +21,6 @@ import (
"fmt" "fmt"
v1 "k8s.io/api/core/v1" v1 "k8s.io/api/core/v1"
utilfeature "k8s.io/apiserver/pkg/util/feature"
"k8s.io/component-base/featuregate" "k8s.io/component-base/featuregate"
csilibplugins "k8s.io/csi-translation-lib/plugins" csilibplugins "k8s.io/csi-translation-lib/plugins"
"k8s.io/kubernetes/pkg/features" "k8s.io/kubernetes/pkg/features"
@ -38,12 +37,14 @@ type PluginNameMapper interface {
// PluginManager keeps track of migrated state of in-tree plugins // PluginManager keeps track of migrated state of in-tree plugins
type PluginManager struct { type PluginManager struct {
PluginNameMapper PluginNameMapper
featureGate featuregate.FeatureGate
} }
// NewPluginManager returns a new PluginManager instance // NewPluginManager returns a new PluginManager instance
func NewPluginManager(m PluginNameMapper) PluginManager { func NewPluginManager(m PluginNameMapper, featureGate featuregate.FeatureGate) PluginManager {
return PluginManager{ return PluginManager{
PluginNameMapper: m, PluginNameMapper: m,
featureGate: featureGate,
} }
} }
@ -60,17 +61,17 @@ func (pm PluginManager) IsMigrationCompleteForPlugin(pluginName string) bool {
switch pluginName { switch pluginName {
case csilibplugins.AWSEBSInTreePluginName: case csilibplugins.AWSEBSInTreePluginName:
return utilfeature.DefaultFeatureGate.Enabled(features.InTreePluginAWSUnregister) return pm.featureGate.Enabled(features.InTreePluginAWSUnregister)
case csilibplugins.GCEPDInTreePluginName: case csilibplugins.GCEPDInTreePluginName:
return utilfeature.DefaultFeatureGate.Enabled(features.InTreePluginGCEUnregister) return pm.featureGate.Enabled(features.InTreePluginGCEUnregister)
case csilibplugins.AzureFileInTreePluginName: case csilibplugins.AzureFileInTreePluginName:
return utilfeature.DefaultFeatureGate.Enabled(features.InTreePluginAzureFileUnregister) return pm.featureGate.Enabled(features.InTreePluginAzureFileUnregister)
case csilibplugins.AzureDiskInTreePluginName: case csilibplugins.AzureDiskInTreePluginName:
return utilfeature.DefaultFeatureGate.Enabled(features.InTreePluginAzureDiskUnregister) return pm.featureGate.Enabled(features.InTreePluginAzureDiskUnregister)
case csilibplugins.CinderInTreePluginName: case csilibplugins.CinderInTreePluginName:
return utilfeature.DefaultFeatureGate.Enabled(features.InTreePluginOpenStackUnregister) return pm.featureGate.Enabled(features.InTreePluginOpenStackUnregister)
case csilibplugins.VSphereInTreePluginName: case csilibplugins.VSphereInTreePluginName:
return utilfeature.DefaultFeatureGate.Enabled(features.CSIMigrationvSphereComplete) || utilfeature.DefaultFeatureGate.Enabled(features.InTreePluginvSphereUnregister) return pm.featureGate.Enabled(features.CSIMigrationvSphereComplete) || pm.featureGate.Enabled(features.InTreePluginvSphereUnregister)
default: default:
return false return false
} }
@ -80,23 +81,23 @@ func (pm PluginManager) IsMigrationCompleteForPlugin(pluginName string) bool {
// for a particular storage plugin // 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 feature should be enabled along with the plugin-specific one
if !utilfeature.DefaultFeatureGate.Enabled(features.CSIMigration) { if !pm.featureGate.Enabled(features.CSIMigration) {
return false return false
} }
switch pluginName { switch pluginName {
case csilibplugins.AWSEBSInTreePluginName: case csilibplugins.AWSEBSInTreePluginName:
return utilfeature.DefaultFeatureGate.Enabled(features.CSIMigrationAWS) return pm.featureGate.Enabled(features.CSIMigrationAWS)
case csilibplugins.GCEPDInTreePluginName: case csilibplugins.GCEPDInTreePluginName:
return utilfeature.DefaultFeatureGate.Enabled(features.CSIMigrationGCE) return pm.featureGate.Enabled(features.CSIMigrationGCE)
case csilibplugins.AzureFileInTreePluginName: case csilibplugins.AzureFileInTreePluginName:
return utilfeature.DefaultFeatureGate.Enabled(features.CSIMigrationAzureFile) return pm.featureGate.Enabled(features.CSIMigrationAzureFile)
case csilibplugins.AzureDiskInTreePluginName: case csilibplugins.AzureDiskInTreePluginName:
return utilfeature.DefaultFeatureGate.Enabled(features.CSIMigrationAzureDisk) return pm.featureGate.Enabled(features.CSIMigrationAzureDisk)
case csilibplugins.CinderInTreePluginName: case csilibplugins.CinderInTreePluginName:
return utilfeature.DefaultFeatureGate.Enabled(features.CSIMigrationOpenStack) return pm.featureGate.Enabled(features.CSIMigrationOpenStack)
case csilibplugins.VSphereInTreePluginName: case csilibplugins.VSphereInTreePluginName:
return utilfeature.DefaultFeatureGate.Enabled(features.CSIMigrationvSphere) return pm.featureGate.Enabled(features.CSIMigrationvSphere)
default: default:
return false return false
} }

View File

@ -125,7 +125,7 @@ func TestIsMigratable(t *testing.T) {
} }
csiTranslator := csitrans.New() csiTranslator := csitrans.New()
for _, test := range testCases { for _, test := range testCases {
pm := NewPluginManager(csiTranslator) pm := NewPluginManager(csiTranslator, utilfeature.DefaultFeatureGate)
t.Run(fmt.Sprintf("Testing %v", test.name), func(t *testing.T) { 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, features.CSIMigration, test.csiMigrationEnabled)()
defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, test.pluginFeature, test.pluginFeatureEnabled)() defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, test.pluginFeature, test.pluginFeatureEnabled)()
@ -337,7 +337,7 @@ func TestMigrationFeatureFlagStatus(t *testing.T) {
} }
csiTranslator := csitrans.New() csiTranslator := csitrans.New()
for _, test := range testCases { for _, test := range testCases {
pm := NewPluginManager(csiTranslator) pm := NewPluginManager(csiTranslator, utilfeature.DefaultFeatureGate)
t.Run(fmt.Sprintf("Testing %v", test.name), func(t *testing.T) { 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, features.CSIMigration, test.csiMigrationEnabled)()
defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, test.pluginFeature, test.pluginFeatureEnabled)() defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, test.pluginFeature, test.pluginFeatureEnabled)()