Skip to content

Commit 6720141

Browse files
yongruilink8s-publishing-bot
authored andcommitted
chore: avoid resetting config of emulation verison and featuregates when adding flags
This change introduces improvements to the component compatibility registry: - Modify the kube-scheduler test server to create a separate ComponentGlobalsRegistry - Update the compatibility registry to handle multiple flag configurations - Enhance test cases to support emulation version mapping between components Kubernetes-commit: dab8758a59de023c066b785200c000c7951479cb
1 parent 7c899b0 commit 6720141

File tree

2 files changed

+42
-7
lines changed

2 files changed

+42
-7
lines changed

compatibility/registry.go

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,9 @@ type componentGlobalsRegistry struct {
102102
// When the `--feature-gates` flag is parsed, it would not take effect until Set() is called,
103103
// because the emulation version needs to be set before the feature gate is set.
104104
featureGatesConfig map[string][]string
105+
// featureGatesConfigFlags stores a pointer to the flag value, allowing other commands
106+
// to append to the feature gates configuration rather than overwriting it
107+
featureGatesConfigFlags *cliflag.ColonSeparatedMultimapStringString
105108
// set stores if the Set() function for the registry is already called.
106109
set bool
107110
}
@@ -120,6 +123,7 @@ func (r *componentGlobalsRegistry) Reset() {
120123
r.componentGlobals = make(map[string]*ComponentGlobals)
121124
r.emulationVersionConfig = nil
122125
r.featureGatesConfig = nil
126+
r.featureGatesConfigFlags = nil
123127
r.set = false
124128
}
125129

@@ -226,19 +230,17 @@ func (r *componentGlobalsRegistry) AddFlags(fs *pflag.FlagSet) {
226230
globals.featureGate.Close()
227231
}
228232
}
229-
if r.emulationVersionConfig != nil || r.featureGatesConfig != nil {
230-
klog.Warning("calling componentGlobalsRegistry.AddFlags more than once, the registry will be set by the latest flags")
231-
}
232-
r.emulationVersionConfig = []string{}
233-
r.featureGatesConfig = make(map[string][]string)
234233

235234
fs.StringSliceVar(&r.emulationVersionConfig, "emulated-version", r.emulationVersionConfig, ""+
236235
"The versions different components emulate their capabilities (APIs, features, ...) of.\n"+
237236
"If set, the component will emulate the behavior of this version instead of the underlying binary version.\n"+
238237
"Version format could only be major.minor, for example: '--emulated-version=wardle=1.2,kube=1.31'. Options are:\n"+strings.Join(r.unsafeVersionFlagOptions(true), "\n")+
239238
"If the component is not specified, defaults to \"kube\"")
240239

241-
fs.Var(cliflag.NewColonSeparatedMultimapStringStringAllowDefaultEmptyKey(&r.featureGatesConfig), "feature-gates", "Comma-separated list of component:key=value pairs that describe feature gates for alpha/experimental features of different components.\n"+
240+
if r.featureGatesConfigFlags == nil {
241+
r.featureGatesConfigFlags = cliflag.NewColonSeparatedMultimapStringStringAllowDefaultEmptyKey(&r.featureGatesConfig)
242+
}
243+
fs.Var(r.featureGatesConfigFlags, "feature-gates", "Comma-separated list of component:key=value pairs that describe feature gates for alpha/experimental features of different components.\n"+
242244
"If the component is not specified, defaults to \"kube\". This flag can be repeatedly invoked. For example: --feature-gates 'wardle:featureA=true,wardle:featureB=false' --feature-gates 'kube:featureC=true'"+
243245
"Options are:\n"+strings.Join(r.unsafeKnownFeatures(), "\n"))
244246
}
@@ -415,7 +417,7 @@ func (r *componentGlobalsRegistry) SetEmulationVersionMapping(fromComponent, toC
415417
return fmt.Errorf("EmulationVersion from %s to %s already exists", fromComponent, toComponent)
416418
}
417419
versionMapping[toComponent] = f
418-
klog.V(klogLevel).Infof("setting the default EmulationVersion of %s based on mapping from the default EmulationVersion of %s", fromComponent, toComponent)
420+
klog.V(klogLevel).Infof("setting the default EmulationVersion of %s based on mapping from the default EmulationVersion of %s", toComponent, fromComponent)
419421
defaultFromVersion := r.componentGlobals[fromComponent].effectiveVersion.EmulationVersion()
420422
emulationVersions, err := r.getFullEmulationVersionConfig(map[string]*version.Version{fromComponent: defaultFromVersion})
421423
if err != nil {

compatibility/registry_test.go

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,7 @@ func TestVersionedFeatureGateFlags(t *testing.T) {
150150
func TestFlags(t *testing.T) {
151151
tests := []struct {
152152
name string
153+
setupRegistry func(r *componentGlobalsRegistry) error
153154
flags []string
154155
parseError string
155156
expectedKubeEmulationVersion string
@@ -272,14 +273,46 @@ func TestFlags(t *testing.T) {
272273
},
273274
parseError: "component not registered: test3",
274275
},
276+
{
277+
name: "feature gates config should accumulate across multiple flag sets",
278+
setupRegistry: func(r *componentGlobalsRegistry) error {
279+
fs := pflag.NewFlagSet("setupTestflag", pflag.ContinueOnError)
280+
r.AddFlags(fs)
281+
return fs.Parse([]string{"--feature-gates=test:commonC=true"})
282+
},
283+
flags: []string{
284+
"--feature-gates=test:testA=true",
285+
},
286+
expectedTestFeatureValues: map[featuregate.Feature]bool{"testA": true, "commonC": true},
287+
},
288+
{
289+
name: "feature gates config should be overridden when set multiple times one the same feature",
290+
setupRegistry: func(r *componentGlobalsRegistry) error {
291+
fs := pflag.NewFlagSet("setupTestflag", pflag.ContinueOnError)
292+
r.AddFlags(fs)
293+
return fs.Parse([]string{"--feature-gates=test:testA=false"})
294+
},
295+
flags: []string{
296+
"--feature-gates=test:testA=true",
297+
},
298+
expectedTestFeatureValues: map[featuregate.Feature]bool{"testA": true},
299+
},
275300
}
276301
for i, test := range tests {
277302
t.Run(test.name, func(t *testing.T) {
278303
fs := pflag.NewFlagSet("testflag", pflag.ContinueOnError)
279304
r := testRegistry(t)
305+
if test.setupRegistry != nil {
306+
if err := test.setupRegistry(r); err != nil {
307+
t.Fatalf("failed to setup registry: %v", err)
308+
}
309+
}
280310
r.AddFlags(fs)
281311
err := fs.Parse(test.flags)
282312
if err == nil {
313+
// AddFlags again to check whether there is no resetting on the config.
314+
fs = pflag.NewFlagSet("testflag2", pflag.ContinueOnError)
315+
r.AddFlags(fs)
283316
err = r.Set()
284317
}
285318
if test.parseError != "" {

0 commit comments

Comments
 (0)