Merge pull request #130079 from yongruilin/compatibility-remove-reset

[compatibility version] Avoid resetting config when adding flags
This commit is contained in:
Kubernetes Prow Robot 2025-03-19 11:52:33 -07:00 committed by GitHub
commit 076c7b0b65
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 98 additions and 14 deletions

View File

@ -26,15 +26,17 @@ import (
"github.com/spf13/pflag"
"k8s.io/apimachinery/pkg/util/wait"
utilcompatibility "k8s.io/apiserver/pkg/util/compatibility"
utilfeature "k8s.io/apiserver/pkg/util/feature"
"k8s.io/client-go/kubernetes"
restclient "k8s.io/client-go/rest"
"k8s.io/component-base/compatibility"
"k8s.io/component-base/configz"
logsapi "k8s.io/component-base/logs/api/v1"
"k8s.io/klog/v2"
"k8s.io/kubernetes/cmd/kube-scheduler/app"
kubeschedulerconfig "k8s.io/kubernetes/cmd/kube-scheduler/app/config"
"k8s.io/kubernetes/cmd/kube-scheduler/app/options"
"k8s.io/klog/v2"
)
func init() {
@ -98,6 +100,16 @@ func StartTestServer(ctx context.Context, customFlags []string) (result TestServ
fs := pflag.NewFlagSet("test", pflag.PanicOnError)
opts := options.NewOptions()
// set up new instance of ComponentGlobalsRegistry instead of using the DefaultComponentGlobalsRegistry to avoid contention in parallel tests.
featureGate := utilfeature.DefaultMutableFeatureGate.DeepCopy()
effectiveVersion := utilcompatibility.DefaultKubeEffectiveVersionForTest()
effectiveVersion.SetEmulationVersion(featureGate.EmulationVersion())
componentGlobalsRegistry := compatibility.NewComponentGlobalsRegistry()
if err := componentGlobalsRegistry.Register(compatibility.DefaultKubeComponent, effectiveVersion, featureGate); err != nil {
return result, err
}
opts.ComponentGlobalsRegistry = componentGlobalsRegistry
nfs := opts.Flags
for _, f := range nfs.FlagSets {
fs.AddFlagSet(f)
@ -106,6 +118,20 @@ func StartTestServer(ctx context.Context, customFlags []string) (result TestServ
if err := opts.ComponentGlobalsRegistry.Set(); err != nil {
return result, err
}
// If the local ComponentGlobalsRegistry is changed by the flags,
// we need to copy the new feature values back to the DefaultFeatureGate because most feature checks still use the DefaultFeatureGate.
if !featureGate.EmulationVersion().EqualTo(utilfeature.DefaultMutableFeatureGate.EmulationVersion()) {
if err := utilfeature.DefaultMutableFeatureGate.SetEmulationVersion(effectiveVersion.EmulationVersion()); err != nil {
return result, err
}
}
for f := range utilfeature.DefaultMutableFeatureGate.GetAll() {
if featureGate.Enabled(f) != utilfeature.DefaultFeatureGate.Enabled(f) {
if err := utilfeature.DefaultMutableFeatureGate.Set(fmt.Sprintf("%s=%v", f, featureGate.Enabled(f))); err != nil {
return result, err
}
}
}
if opts.SecureServing.BindPort != 0 {
opts.SecureServing.Listener, opts.SecureServing.BindPort, err = createListenerOnFreePort()

View File

@ -102,6 +102,9 @@ type componentGlobalsRegistry struct {
// When the `--feature-gates` flag is parsed, it would not take effect until Set() is called,
// because the emulation version needs to be set before the feature gate is set.
featureGatesConfig map[string][]string
// featureGatesConfigFlags stores a pointer to the flag value, allowing other commands
// to append to the feature gates configuration rather than overwriting it
featureGatesConfigFlags *cliflag.ColonSeparatedMultimapStringString
// set stores if the Set() function for the registry is already called.
set bool
}
@ -120,6 +123,7 @@ func (r *componentGlobalsRegistry) Reset() {
r.componentGlobals = make(map[string]*ComponentGlobals)
r.emulationVersionConfig = nil
r.featureGatesConfig = nil
r.featureGatesConfigFlags = nil
r.set = false
}
@ -226,11 +230,6 @@ func (r *componentGlobalsRegistry) AddFlags(fs *pflag.FlagSet) {
globals.featureGate.Close()
}
}
if r.emulationVersionConfig != nil || r.featureGatesConfig != nil {
klog.Warning("calling componentGlobalsRegistry.AddFlags more than once, the registry will be set by the latest flags")
}
r.emulationVersionConfig = []string{}
r.featureGatesConfig = make(map[string][]string)
fs.StringSliceVar(&r.emulationVersionConfig, "emulated-version", r.emulationVersionConfig, ""+
"The versions different components emulate their capabilities (APIs, features, ...) of.\n"+
@ -238,7 +237,10 @@ func (r *componentGlobalsRegistry) AddFlags(fs *pflag.FlagSet) {
"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")+
"If the component is not specified, defaults to \"kube\"")
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"+
if r.featureGatesConfigFlags == nil {
r.featureGatesConfigFlags = cliflag.NewColonSeparatedMultimapStringStringAllowDefaultEmptyKey(&r.featureGatesConfig)
}
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"+
"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'"+
"Options are:\n"+strings.Join(r.unsafeKnownFeatures(), "\n"))
}
@ -415,7 +417,7 @@ func (r *componentGlobalsRegistry) SetEmulationVersionMapping(fromComponent, toC
return fmt.Errorf("EmulationVersion from %s to %s already exists", fromComponent, toComponent)
}
versionMapping[toComponent] = f
klog.V(klogLevel).Infof("setting the default EmulationVersion of %s based on mapping from the default EmulationVersion of %s", fromComponent, toComponent)
klog.V(klogLevel).Infof("setting the default EmulationVersion of %s based on mapping from the default EmulationVersion of %s", toComponent, fromComponent)
defaultFromVersion := r.componentGlobals[fromComponent].effectiveVersion.EmulationVersion()
emulationVersions, err := r.getFullEmulationVersionConfig(map[string]*version.Version{fromComponent: defaultFromVersion})
if err != nil {

View File

@ -150,6 +150,7 @@ func TestVersionedFeatureGateFlags(t *testing.T) {
func TestFlags(t *testing.T) {
tests := []struct {
name string
setupRegistry func(r *componentGlobalsRegistry) error
flags []string
parseError string
expectedKubeEmulationVersion string
@ -272,14 +273,46 @@ func TestFlags(t *testing.T) {
},
parseError: "component not registered: test3",
},
{
name: "feature gates config should accumulate across multiple flag sets",
setupRegistry: func(r *componentGlobalsRegistry) error {
fs := pflag.NewFlagSet("setupTestflag", pflag.ContinueOnError)
r.AddFlags(fs)
return fs.Parse([]string{"--feature-gates=test:commonC=true"})
},
flags: []string{
"--feature-gates=test:testA=true",
},
expectedTestFeatureValues: map[featuregate.Feature]bool{"testA": true, "commonC": true},
},
{
name: "feature gates config should be overridden when set multiple times one the same feature",
setupRegistry: func(r *componentGlobalsRegistry) error {
fs := pflag.NewFlagSet("setupTestflag", pflag.ContinueOnError)
r.AddFlags(fs)
return fs.Parse([]string{"--feature-gates=test:testA=false"})
},
flags: []string{
"--feature-gates=test:testA=true",
},
expectedTestFeatureValues: map[featuregate.Feature]bool{"testA": true},
},
}
for i, test := range tests {
t.Run(test.name, func(t *testing.T) {
fs := pflag.NewFlagSet("testflag", pflag.ContinueOnError)
r := testRegistry(t)
if test.setupRegistry != nil {
if err := test.setupRegistry(r); err != nil {
t.Fatalf("failed to setup registry: %v", err)
}
}
r.AddFlags(fs)
err := fs.Parse(test.flags)
if err == nil {
// AddFlags again to check whether there is no resetting on the config.
fs = pflag.NewFlagSet("testflag2", pflag.ContinueOnError)
r.AddFlags(fs)
err = r.Set()
}
if test.parseError != "" {

View File

@ -48,6 +48,7 @@ import (
"k8s.io/apiserver/pkg/features"
"k8s.io/apiserver/pkg/server/dynamiccertificates"
genericapiserveroptions "k8s.io/apiserver/pkg/server/options"
utilcompatibility "k8s.io/apiserver/pkg/util/compatibility"
utilfeature "k8s.io/apiserver/pkg/util/feature"
client "k8s.io/client-go/kubernetes"
"k8s.io/client-go/rest"
@ -263,6 +264,7 @@ func TestFrontProxyConfig(t *testing.T) {
testFrontProxyConfig(t, false)
})
t.Run("WithUID", func(t *testing.T) {
featuregatetesting.SetFeatureGateEmulationVersionDuringTest(t, utilfeature.DefaultFeatureGate, version.MajorMinor(1, 33))
featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.RemoteRequestHeaderUID, true)
testFrontProxyConfig(t, true)
})
@ -277,7 +279,13 @@ func testFrontProxyConfig(t *testing.T, withUID bool) {
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Minute)
t.Cleanup(cancel)
var extraKASFlags []string
// Set the emulation version for the kube-apiserver testserver by mapping
// the wardle version to the kube version.
wardleEmulationVersion := version.MustParse(wardleBinaryVersion)
kubeEmulationVersion := sampleserver.WardleVersionToKubeVersion(wardleEmulationVersion)
extraKASFlags := []string{
fmt.Sprintf("--emulated-version=kube=%s", kubeEmulationVersion.String()),
}
if withUID {
extraKASFlags = []string{"--requestheader-uid-headers=x-remote-uid"}
}
@ -393,19 +401,27 @@ func (f roundTripperFunc) RoundTrip(req *http.Request) (*http.Response, error) {
return f(req)
}
func testAggregatedAPIServer(t *testing.T, setWardleFeatureGate, banFlunder bool, wardleBinaryVersion, wardleEmulationVersion string) {
func testAggregatedAPIServer(t *testing.T, setWardleFeatureGate, banFlunder bool, wardleBinaryVersionRaw, wardleEmulationVersionRaw string) {
const testNamespace = "kube-wardle"
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Minute)
t.Cleanup(cancel)
testKAS, wardleOptions, wardlePort := prepareAggregatedWardleAPIServer(ctx, t, testNamespace, wardleBinaryVersion, nil, false)
// set the emulation version for the kube-apiserver testserver by mapping
// the wardle version to the kube version.
wardleEmulationVersion := version.MustParse(wardleEmulationVersionRaw)
kubeEmulationVersion := sampleserver.WardleVersionToKubeVersion(wardleEmulationVersion)
extraKASFlags := []string{
fmt.Sprintf("--emulated-version=kube=%s", kubeEmulationVersion.String()),
}
testKAS, wardleOptions, wardlePort := prepareAggregatedWardleAPIServer(ctx, t, testNamespace, wardleBinaryVersionRaw, extraKASFlags, false)
kubeClientConfig := getKubeConfig(testKAS)
wardleCertDir, _ := os.MkdirTemp("", "test-integration-wardle-server")
defer os.RemoveAll(wardleCertDir)
directWardleClientConfig := runPreparedWardleServer(ctx, t, wardleOptions, wardleCertDir, wardlePort, setWardleFeatureGate, banFlunder, wardleEmulationVersion, kubeClientConfig, false)
directWardleClientConfig := runPreparedWardleServer(ctx, t, wardleOptions, wardleCertDir, wardlePort, setWardleFeatureGate, banFlunder, wardleEmulationVersionRaw, kubeClientConfig, false)
// now we're finally ready to test. These are what's run by default now
wardleDirectClient := client.NewForConfigOrDie(directWardleClientConfig)
@ -699,7 +715,14 @@ func prepareAggregatedWardleAPIServer(ctx context.Context, t *testing.T, namespa
framework.SharedEtcd())
t.Cleanup(func() { testServer.TearDownFn() })
componentGlobalsRegistry := testServer.ServerOpts.Options.GenericServerRunOptions.ComponentGlobalsRegistry
// Create a new registry since the testServer's ComponentGlobalsRegistry is already Set(),
// and wardle server would try to Set() again in the test.
componentGlobalsRegistry := basecompatibility.NewComponentGlobalsRegistry()
_, _ = componentGlobalsRegistry.ComponentGlobalsOrRegister(
basecompatibility.DefaultKubeComponent,
utilcompatibility.DefaultKubeEffectiveVersionForTest(),
utilfeature.DefaultFeatureGate.DeepCopy(),
)
_, _ = componentGlobalsRegistry.ComponentGlobalsOrRegister(
apiserver.WardleComponentName, basecompatibility.NewEffectiveVersionFromString(wardleBinaryVersion, "", ""),
featuregate.NewVersionedFeatureGate(version.MustParse(wardleBinaryVersion)))