diff --git a/staging/src/k8s.io/component-base/featuregate/feature_gate.go b/staging/src/k8s.io/component-base/featuregate/feature_gate.go index c4d8480cf3b..b6f08a6cd6a 100644 --- a/staging/src/k8s.io/component-base/featuregate/feature_gate.go +++ b/staging/src/k8s.io/component-base/featuregate/feature_gate.go @@ -115,9 +115,6 @@ type FeatureGate interface { // set on the copy without mutating the original. This is useful for validating // config against potential feature gate changes before committing those changes. DeepCopy() MutableVersionedFeatureGate - // CopyKnownFeatures returns a partial copy of the FeatureGate object, with all the known features and overrides. - // This is useful for creating a new instance of feature gate without inheriting all the enabled configurations of the base feature gate. - CopyKnownFeatures() MutableVersionedFeatureGate // Validate checks if the flag gates are valid at the emulated version. Validate() []error } @@ -189,6 +186,10 @@ type MutableVersionedFeatureGate interface { ExplicitlySet(name Feature) bool // ResetFeatureValueToDefault resets the value of the feature back to the default value. ResetFeatureValueToDefault(name Feature) error + // DeepCopyAndReset copies all the registered features of the FeatureGate object, with all the known features and overrides, + // and resets all the enabled status of the new feature gate. + // This is useful for creating a new instance of feature gate without inheriting all the enabled configurations of the base feature gate. + DeepCopyAndReset() MutableVersionedFeatureGate } // featureGate implements FeatureGate as well as pflag.Value for flag parsing. @@ -423,10 +424,7 @@ func (f *featureGate) AddVersioned(features map[Feature]VersionedSpecs) error { } // Copy existing state - known := map[Feature]VersionedSpecs{} - for k, v := range f.known.Load().(map[Feature]VersionedSpecs) { - known[k] = v - } + known := f.GetAllVersioned() for name, specs := range features { sort.Sort(specs) @@ -458,11 +456,8 @@ func (f *featureGate) OverrideDefaultAtVersion(name Feature, override bool, ver return fmt.Errorf("cannot override default for feature %q: gates already added to a flag set", name) } - known := map[Feature]VersionedSpecs{} - for k, v := range f.known.Load().(map[Feature]VersionedSpecs) { - sort.Sort(v) - known[k] = v - } + // Copy existing state + known := f.GetAllVersioned() specs, ok := known[name] if !ok { @@ -509,7 +504,9 @@ func (f *featureGate) GetAll() map[Feature]FeatureSpec { func (f *featureGate) GetAllVersioned() map[Feature]VersionedSpecs { retval := map[Feature]VersionedSpecs{} for k, v := range f.known.Load().(map[Feature]VersionedSpecs) { - retval[k] = v + vCopy := make([]FeatureSpec, len(v)) + _ = copy(vCopy, v) + retval[k] = vCopy } return retval } @@ -660,9 +657,10 @@ func (f *featureGate) KnownFeatures() []string { return known } -// CopyKnownFeatures returns a partial copy of the FeatureGate object, with all the known features and overrides. +// DeepCopyAndReset copies all the registered features of the FeatureGate object, with all the known features and overrides, +// and resets all the enabled status of the new feature gate. // This is useful for creating a new instance of feature gate without inheriting all the enabled configurations of the base feature gate. -func (f *featureGate) CopyKnownFeatures() MutableVersionedFeatureGate { +func (f *featureGate) DeepCopyAndReset() MutableVersionedFeatureGate { fg := NewVersionedFeatureGate(f.EmulationVersion()) known := f.GetAllVersioned() fg.known.Store(known) @@ -676,10 +674,7 @@ func (f *featureGate) DeepCopy() MutableVersionedFeatureGate { f.lock.Lock() defer f.lock.Unlock() // Copy existing state. - known := map[Feature]VersionedSpecs{} - for k, v := range f.known.Load().(map[Feature]VersionedSpecs) { - known[k] = v - } + known := f.GetAllVersioned() enabled := map[Feature]bool{} for k, v := range f.enabled.Load().(map[Feature]bool) { enabled[k] = v diff --git a/staging/src/k8s.io/component-base/featuregate/feature_gate_test.go b/staging/src/k8s.io/component-base/featuregate/feature_gate_test.go index cd6eeb83ecb..b0e0413dcd4 100644 --- a/staging/src/k8s.io/component-base/featuregate/feature_gate_test.go +++ b/staging/src/k8s.io/component-base/featuregate/feature_gate_test.go @@ -600,7 +600,7 @@ func TestFeatureGateOverrideDefault(t *testing.T) { f := NewFeatureGate() require.NoError(t, f.Add(map[Feature]FeatureSpec{"TestFeature": {Default: false}})) require.NoError(t, f.OverrideDefault("TestFeature", true)) - fcopy := f.CopyKnownFeatures() + fcopy := f.DeepCopyAndReset() if !f.Enabled("TestFeature") { t.Error("TestFeature should be enabled by override") } @@ -609,6 +609,19 @@ func TestFeatureGateOverrideDefault(t *testing.T) { } }) + t.Run("overrides are not passed over after CopyKnownFeatures", func(t *testing.T) { + f := NewFeatureGate() + require.NoError(t, f.Add(map[Feature]FeatureSpec{"TestFeature": {Default: false}})) + fcopy := f.DeepCopyAndReset() + require.NoError(t, f.OverrideDefault("TestFeature", true)) + if !f.Enabled("TestFeature") { + t.Error("TestFeature should be enabled by override") + } + if fcopy.Enabled("TestFeature") { + t.Error("default override should not be passed over after CopyKnownFeatures") + } + }) + t.Run("reflected in known features", func(t *testing.T) { f := NewFeatureGate() if err := f.Add(map[Feature]FeatureSpec{"TestFeature": { @@ -1351,6 +1364,34 @@ func TestVersionedFeatureGateOverrideDefault(t *testing.T) { } }) + t.Run("overrides are not passed over after deep copies", func(t *testing.T) { + f := NewVersionedFeatureGate(version.MustParse("1.29")) + require.NoError(t, f.SetEmulationVersion(version.MustParse("1.28"))) + if err := f.AddVersioned(map[Feature]VersionedSpecs{ + "TestFeature": { + {Version: version.MustParse("1.28"), Default: false}, + {Version: version.MustParse("1.29"), Default: true}, + }, + }); err != nil { + t.Fatal(err) + } + assert.False(t, f.Enabled("TestFeature")) + + fcopy := f.DeepCopy() + require.NoError(t, f.OverrideDefault("TestFeature", true)) + require.NoError(t, f.OverrideDefaultAtVersion("TestFeature", false, version.MustParse("1.29"))) + assert.True(t, f.Enabled("TestFeature")) + assert.False(t, fcopy.Enabled("TestFeature")) + + require.NoError(t, f.SetEmulationVersion(version.MustParse("1.29"))) + assert.False(t, f.Enabled("TestFeature")) + assert.False(t, fcopy.Enabled("TestFeature")) + + require.NoError(t, fcopy.SetEmulationVersion(version.MustParse("1.29"))) + assert.False(t, f.Enabled("TestFeature")) + assert.True(t, fcopy.Enabled("TestFeature")) + }) + t.Run("reflected in known features", func(t *testing.T) { f := NewVersionedFeatureGate(version.MustParse("1.29")) require.NoError(t, f.SetEmulationVersion(version.MustParse("1.28"))) @@ -1536,7 +1577,7 @@ func TestCopyKnownFeatures(t *testing.T) { require.NoError(t, f.Add(map[Feature]FeatureSpec{"FeatureA": {Default: false}, "FeatureB": {Default: false}})) require.NoError(t, f.Set("FeatureA=true")) require.NoError(t, f.OverrideDefault("FeatureB", true)) - fcopy := f.CopyKnownFeatures() + fcopy := f.DeepCopyAndReset() require.NoError(t, f.Add(map[Feature]FeatureSpec{"FeatureC": {Default: false}})) assert.True(t, f.Enabled("FeatureA")) diff --git a/staging/src/k8s.io/sample-apiserver/pkg/cmd/server/start.go b/staging/src/k8s.io/sample-apiserver/pkg/cmd/server/start.go index be233af68d6..0ae2690b4b8 100644 --- a/staging/src/k8s.io/sample-apiserver/pkg/cmd/server/start.go +++ b/staging/src/k8s.io/sample-apiserver/pkg/cmd/server/start.go @@ -36,6 +36,7 @@ import ( utilfeature "k8s.io/apiserver/pkg/util/feature" utilversion "k8s.io/apiserver/pkg/util/version" "k8s.io/component-base/featuregate" + baseversion "k8s.io/component-base/version" "k8s.io/sample-apiserver/pkg/admission/plugin/banflunder" "k8s.io/sample-apiserver/pkg/admission/wardleinitializer" "k8s.io/sample-apiserver/pkg/apis/wardle/v1alpha1" @@ -59,14 +60,18 @@ type WardleServerOptions struct { AlternateDNS []string } -func wardleEmulationVersionToKubeEmulationVersion(ver *version.Version) *version.Version { +func WardleVersionToKubeVersion(ver *version.Version) *version.Version { if ver.Major() != 1 { return nil } kubeVer := utilversion.DefaultKubeEffectiveVersion().BinaryVersion() - // "1.1" maps to kubeVer - offset := int(ver.Minor()) - 1 - return kubeVer.OffsetMinor(offset) + // "1.2" maps to kubeVer + offset := int(ver.Minor()) - 2 + mappedVer := kubeVer.OffsetMinor(offset) + if mappedVer.GreaterThan(kubeVer) { + return kubeVer + } + return mappedVer } // NewWardleServerOptions returns a new WardleServerOptions @@ -112,19 +117,44 @@ func NewCommandStartWardleServer(ctx context.Context, defaults *WardleServerOpti flags := cmd.Flags() o.RecommendedOptions.AddFlags(flags) - wardleEffectiveVersion := utilversion.NewEffectiveVersion("1.2") - wardleFeatureGate := utilfeature.DefaultFeatureGate.CopyKnownFeatures() + // The following lines demonstrate how to configure version compatibility and feature gates + // for the "Wardle" component, as an example of KEP-4330. + + // Create an effective version object for the "Wardle" component. + // This initializes the binary version, the emulation version and the minimum compatibility version. + // + // Note: + // - The binary version represents the actual version of the running source code. + // - The emulation version is the version whose capabilities are being emulated by the binary. + // - The minimum compatibility version specifies the minimum version that the component remains compatible with. + // + // Refer to KEP-4330 for more details: https://github.com/kubernetes/enhancements/blob/master/keps/sig-architecture/4330-compatibility-versions + defaultWardleVersion := "1.2" + // Register the "Wardle" component with the global component registry, + // associating it with its effective version and feature gate configuration. + // Will skip if the component has been registered, like in the integration test. + _, wardleFeatureGate := utilversion.DefaultComponentGlobalsRegistry.ComponentGlobalsOrRegister( + apiserver.WardleComponentName, utilversion.NewEffectiveVersion(defaultWardleVersion), + featuregate.NewVersionedFeatureGate(version.MustParse(defaultWardleVersion))) + + // Add versioned feature specifications for the "BanFlunder" feature. + // These specifications, together with the effective version, determine if the feature is enabled. utilruntime.Must(wardleFeatureGate.AddVersioned(map[featuregate.Feature]featuregate.VersionedSpecs{ "BanFlunder": { - {Version: version.MustParse("1.2"), Default: true, PreRelease: featuregate.GA}, - {Version: version.MustParse("1.1"), Default: false, PreRelease: featuregate.Beta}, + {Version: version.MustParse("1.2"), Default: true, PreRelease: featuregate.GA, LockToDefault: true}, + {Version: version.MustParse("1.1"), Default: true, PreRelease: featuregate.Beta}, {Version: version.MustParse("1.0"), Default: false, PreRelease: featuregate.Alpha}, }, })) - utilruntime.Must(utilversion.DefaultComponentGlobalsRegistry.Register(apiserver.WardleComponentName, wardleEffectiveVersion, wardleFeatureGate)) - _, _ = utilversion.DefaultComponentGlobalsRegistry.ComponentGlobalsOrRegister( - utilversion.DefaultKubeComponent, utilversion.DefaultKubeEffectiveVersion(), utilfeature.DefaultMutableFeatureGate) - utilruntime.Must(utilversion.DefaultComponentGlobalsRegistry.SetEmulationVersionMapping(apiserver.WardleComponentName, utilversion.DefaultKubeComponent, wardleEmulationVersionToKubeEmulationVersion)) + + // Register the default kube component if not already present in the global registry. + _, _ = utilversion.DefaultComponentGlobalsRegistry.ComponentGlobalsOrRegister(utilversion.DefaultKubeComponent, + utilversion.NewEffectiveVersion(baseversion.DefaultKubeBinaryVersion), utilfeature.DefaultMutableFeatureGate) + + // Set the emulation version mapping from the "Wardle" component to the kube component. + // This ensures that the emulation version of the latter is determined by the emulation version of the former. + utilruntime.Must(utilversion.DefaultComponentGlobalsRegistry.SetEmulationVersionMapping(apiserver.WardleComponentName, utilversion.DefaultKubeComponent, WardleVersionToKubeVersion)) + utilversion.DefaultComponentGlobalsRegistry.AddFlags(flags) return cmd @@ -177,7 +207,7 @@ func (o *WardleServerOptions) Config() (*apiserver.Config, error) { serverConfig.OpenAPIV3Config.Info.Title = "Wardle" serverConfig.OpenAPIV3Config.Info.Version = "0.1" - serverConfig.FeatureGate = utilversion.DefaultComponentGlobalsRegistry.FeatureGateFor(apiserver.WardleComponentName) + serverConfig.FeatureGate = utilversion.DefaultComponentGlobalsRegistry.FeatureGateFor(utilversion.DefaultKubeComponent) serverConfig.EffectiveVersion = utilversion.DefaultComponentGlobalsRegistry.EffectiveVersionFor(apiserver.WardleComponentName) if err := o.RecommendedOptions.ApplyTo(serverConfig); err != nil { diff --git a/staging/src/k8s.io/sample-apiserver/pkg/cmd/server/start_test.go b/staging/src/k8s.io/sample-apiserver/pkg/cmd/server/start_test.go index 5a803318c12..ab460815083 100644 --- a/staging/src/k8s.io/sample-apiserver/pkg/cmd/server/start_test.go +++ b/staging/src/k8s.io/sample-apiserver/pkg/cmd/server/start_test.go @@ -35,13 +35,23 @@ func TestWardleEmulationVersionToKubeEmulationVersion(t *testing.T) { }{ { desc: "same version as than kube binary", - wardleEmulationVer: version.MajorMinor(1, 1), + wardleEmulationVer: version.MajorMinor(1, 2), expectedKubeEmulationVer: defaultKubeEffectiveVersion.BinaryVersion(), }, { - desc: "1 version higher than kube binary", - wardleEmulationVer: version.MajorMinor(1, 2), - expectedKubeEmulationVer: defaultKubeEffectiveVersion.BinaryVersion().OffsetMinor(1), + desc: "1 version lower than kube binary", + wardleEmulationVer: version.MajorMinor(1, 1), + expectedKubeEmulationVer: defaultKubeEffectiveVersion.BinaryVersion().OffsetMinor(-1), + }, + { + desc: "2 versions lower than kube binary", + wardleEmulationVer: version.MajorMinor(1, 0), + expectedKubeEmulationVer: defaultKubeEffectiveVersion.BinaryVersion().OffsetMinor(-2), + }, + { + desc: "capped at kube binary", + wardleEmulationVer: version.MajorMinor(1, 3), + expectedKubeEmulationVer: defaultKubeEffectiveVersion.BinaryVersion(), }, { desc: "no mapping", @@ -51,7 +61,7 @@ func TestWardleEmulationVersionToKubeEmulationVersion(t *testing.T) { for _, tc := range testCases { t.Run(tc.desc, func(t *testing.T) { - mappedKubeEmulationVer := wardleEmulationVersionToKubeEmulationVersion(tc.wardleEmulationVer) + mappedKubeEmulationVer := WardleVersionToKubeVersion(tc.wardleEmulationVer) assert.True(t, mappedKubeEmulationVer.EqualTo(tc.expectedKubeEmulationVer)) }) } diff --git a/test/integration/examples/apiserver_test.go b/test/integration/examples/apiserver_test.go index e2ccf28079f..2bbc7666a33 100644 --- a/test/integration/examples/apiserver_test.go +++ b/test/integration/examples/apiserver_test.go @@ -37,14 +37,17 @@ import ( corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/version" "k8s.io/apimachinery/pkg/util/wait" "k8s.io/apiserver/pkg/server/dynamiccertificates" genericapiserveroptions "k8s.io/apiserver/pkg/server/options" + utilversion "k8s.io/apiserver/pkg/util/version" client "k8s.io/client-go/kubernetes" "k8s.io/client-go/rest" "k8s.io/client-go/tools/clientcmd" clientcmdapi "k8s.io/client-go/tools/clientcmd/api" "k8s.io/client-go/util/cert" + "k8s.io/component-base/featuregate" apiregistrationv1 "k8s.io/kube-aggregator/pkg/apis/apiregistration/v1" aggregatorclient "k8s.io/kube-aggregator/pkg/client/clientset_generated/clientset" "k8s.io/kubernetes/cmd/kube-apiserver/app" @@ -53,6 +56,7 @@ import ( "k8s.io/kubernetes/test/integration/framework" wardlev1alpha1 "k8s.io/sample-apiserver/pkg/apis/wardle/v1alpha1" wardlev1beta1 "k8s.io/sample-apiserver/pkg/apis/wardle/v1beta1" + "k8s.io/sample-apiserver/pkg/apiserver" sampleserver "k8s.io/sample-apiserver/pkg/cmd/server" wardlev1alpha1client "k8s.io/sample-apiserver/pkg/generated/clientset/versioned/typed/wardle/v1alpha1" netutils "k8s.io/utils/net" @@ -226,30 +230,40 @@ func TestAPIServiceWaitOnStart(t *testing.T) { } func TestAggregatedAPIServer(t *testing.T) { + // Testing default, BanFlunder default=true in 1.2 t.Run("WithoutWardleFeatureGateAtV1.2", func(t *testing.T) { - testAggregatedAPIServer(t, false, "1.2") + testAggregatedAPIServer(t, false, true, "1.2", "1.2") }) + // Testing emulation version N, BanFlunder default=true in 1.1 t.Run("WithoutWardleFeatureGateAtV1.1", func(t *testing.T) { - testAggregatedAPIServer(t, false, "1.1") + testAggregatedAPIServer(t, false, true, "1.1", "1.1") }) - t.Run("WithWardleFeatureGateAtV1.1", func(t *testing.T) { - testAggregatedAPIServer(t, true, "1.1") + // Testing emulation version N-1, BanFlunder default=false in 1.0 + t.Run("WithoutWardleFeatureGateAtV1.0", func(t *testing.T) { + testAggregatedAPIServer(t, false, false, "1.1", "1.0") + }) + // Testing emulation version N-1, Explicitly set BanFlunder=true in 1.0 + t.Run("WithWardleFeatureGateAtV1.0", func(t *testing.T) { + testAggregatedAPIServer(t, true, true, "1.1", "1.0") }) } -func testAggregatedAPIServer(t *testing.T, flunderBanningFeatureGate bool, emulationVersion string) { +func testAggregatedAPIServer(t *testing.T, setWardleFeatureGate, banFlunder bool, wardleBinaryVersion, wardleEmulationVersion string) { const testNamespace = "kube-wardle" ctx, cancel := context.WithTimeout(context.Background(), 10*time.Minute) t.Cleanup(cancel) - testKAS, wardleOptions, wardlePort := prepareAggregatedWardleAPIServer(ctx, t, testNamespace) + // each wardle binary is bundled with a specific kube binary. + kubeBinaryVersion := sampleserver.WardleVersionToKubeVersion(version.MustParse(wardleBinaryVersion)).String() + + testKAS, wardleOptions, wardlePort := prepareAggregatedWardleAPIServer(ctx, t, testNamespace, kubeBinaryVersion, wardleBinaryVersion) kubeClientConfig := getKubeConfig(testKAS) wardleCertDir, _ := os.MkdirTemp("", "test-integration-wardle-server") defer os.RemoveAll(wardleCertDir) - directWardleClientConfig := runPreparedWardleServer(ctx, t, wardleOptions, wardleCertDir, wardlePort, flunderBanningFeatureGate, emulationVersion, kubeClientConfig) + directWardleClientConfig := runPreparedWardleServer(ctx, t, wardleOptions, wardleCertDir, wardlePort, setWardleFeatureGate, banFlunder, wardleEmulationVersion, kubeClientConfig) // now we're finally ready to test. These are what's run by default now wardleDirectClient := client.NewForConfigOrDie(directWardleClientConfig) @@ -289,7 +303,6 @@ func testAggregatedAPIServer(t *testing.T, flunderBanningFeatureGate bool, emula Name: "badname", }, }, metav1.CreateOptions{}) - banFlunder := flunderBanningFeatureGate || emulationVersion == "1.2" if banFlunder && err == nil { t.Fatal("expect flunder:badname not admitted when wardle feature gates are specified") } @@ -524,7 +537,7 @@ func TestAggregatedAPIServerRejectRedirectResponse(t *testing.T) { } } -func prepareAggregatedWardleAPIServer(ctx context.Context, t *testing.T, namespace string) (*kastesting.TestServer, *sampleserver.WardleServerOptions, int) { +func prepareAggregatedWardleAPIServer(ctx context.Context, t *testing.T, namespace, kubebinaryVersion, wardleBinaryVersion string) (*kastesting.TestServer, *sampleserver.WardleServerOptions, int) { // makes the kube-apiserver very responsive. it's normally a minute dynamiccertificates.FileRefreshDuration = 1 * time.Second @@ -536,9 +549,13 @@ func prepareAggregatedWardleAPIServer(ctx context.Context, t *testing.T, namespa // endpoints cannot have loopback IPs so we need to override the resolver itself t.Cleanup(app.SetServiceResolverForTests(staticURLServiceResolver(fmt.Sprintf("https://127.0.0.1:%d", wardlePort)))) - testServer := kastesting.StartTestServerOrDie(t, &kastesting.TestServerInstanceOptions{EnableCertAuth: true, BinaryVersion: "1.32"}, nil, framework.SharedEtcd()) + testServer := kastesting.StartTestServerOrDie(t, &kastesting.TestServerInstanceOptions{EnableCertAuth: true, BinaryVersion: kubebinaryVersion}, nil, framework.SharedEtcd()) t.Cleanup(func() { testServer.TearDownFn() }) + _, _ = utilversion.DefaultComponentGlobalsRegistry.ComponentGlobalsOrRegister( + apiserver.WardleComponentName, utilversion.NewEffectiveVersion(wardleBinaryVersion), + featuregate.NewVersionedFeatureGate(version.MustParse(wardleBinaryVersion))) + kubeClient := client.NewForConfigOrDie(getKubeConfig(testServer)) // create the bare minimum resources required to be able to get the API service into an available state @@ -581,6 +598,7 @@ func runPreparedWardleServer( certDir string, wardlePort int, flunderBanningFeatureGate bool, + banFlunder bool, emulationVersion string, kubeConfig *rest.Config, ) *rest.Config { @@ -599,7 +617,7 @@ func runPreparedWardleServer( "--emulated-version", fmt.Sprintf("wardle=%s", emulationVersion), } if flunderBanningFeatureGate { - args = append(args, "--feature-gates", "wardle:BanFlunder=true") + args = append(args, "--feature-gates", fmt.Sprintf("wardle:BanFlunder=%v", banFlunder)) } wardleCmd := sampleserver.NewCommandStartWardleServer(ctx, wardleOptions) wardleCmd.SetArgs(args)