From d4c6e0bfd04a9efb1ddb33afdcaa49eb8695d8e5 Mon Sep 17 00:00:00 2001 From: Siyuan Zhang Date: Fri, 31 Oct 2025 10:04:06 -0500 Subject: [PATCH] change client go default features to versioned so that their enablement can be consistent with emulated version. Signed-off-by: Siyuan Zhang --- pkg/features/client_adapter.go | 42 +++++++++++- pkg/features/client_adapter_test.go | 66 +++++++++++++++++++ pkg/features/kube_features.go | 2 +- .../src/k8s.io/client-go/features/features.go | 43 +++++++++++- .../client-go/features/features_test.go | 63 +++++++++++++++++- .../client-go/features/known_features.go | 29 ++++++-- .../client-go/features/testing/features.go | 1 + 7 files changed, 232 insertions(+), 14 deletions(-) diff --git a/pkg/features/client_adapter.go b/pkg/features/client_adapter.go index 00a0fd92ab4..d07f7d4aeb5 100644 --- a/pkg/features/client_adapter.go +++ b/pkg/features/client_adapter.go @@ -39,7 +39,7 @@ func (a *clientAdapter) Enabled(name clientfeatures.Feature) bool { return a.mfg.Enabled(featuregate.Feature(name)) } -var _ clientfeatures.Registry = &clientAdapter{} +var _ clientfeatures.VersionedRegistry = &clientAdapter{} func (a *clientAdapter) Add(in map[clientfeatures.Feature]clientfeatures.FeatureSpec) error { out := map[featuregate.Feature]featuregate.FeatureSpec{} @@ -65,7 +65,45 @@ func (a *clientAdapter) Add(in map[clientfeatures.Feature]clientfeatures.Feature } out[featuregate.Feature(name)] = converted } - return a.mfg.Add(out) //nolint:forbidigo // No need to support versioned feature gates in client adapter + return a.mfg.Add(out) //nolint:forbidigo +} + +// AddVersioned adds the provided versioned feature gates. +func (a *clientAdapter) AddVersioned(in map[clientfeatures.Feature]clientfeatures.VersionedSpecs) error { + mvfg, ok := a.mfg.(featuregate.MutableVersionedFeatureGate) + if !ok { + return fmt.Errorf("feature gate does not support versioning") + } + + out := make(map[featuregate.Feature]featuregate.VersionedSpecs) + for name, specs := range in { + convertedSpecs := make(featuregate.VersionedSpecs, len(specs)) + for i, spec := range specs { + converted := featuregate.FeatureSpec{ + Default: spec.Default, + LockToDefault: spec.LockToDefault, + Version: spec.Version, + } + switch spec.PreRelease { + case clientfeatures.Alpha: + converted.PreRelease = featuregate.Alpha + case clientfeatures.Beta: + converted.PreRelease = featuregate.Beta + case clientfeatures.GA: + converted.PreRelease = featuregate.GA + case clientfeatures.Deprecated: + converted.PreRelease = featuregate.Deprecated + default: + // The default case implies programmer error. The same set of prerelease + // constants must exist in both component-base and client-go, and each one + // must have a case here. + panic(fmt.Sprintf("unrecognized prerelease %q of feature %q", spec.PreRelease, name)) + } + convertedSpecs[i] = converted + } + out[featuregate.Feature(name)] = convertedSpecs + } + return mvfg.AddVersioned(out) } // Set implements the unexported interface that client-go feature gate testing expects for diff --git a/pkg/features/client_adapter_test.go b/pkg/features/client_adapter_test.go index cb3665a235c..0eba10fe28f 100644 --- a/pkg/features/client_adapter_test.go +++ b/pkg/features/client_adapter_test.go @@ -21,6 +21,7 @@ import ( "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" + "k8s.io/apimachinery/pkg/util/version" clientfeatures "k8s.io/client-go/features" "k8s.io/component-base/featuregate" ) @@ -98,3 +99,68 @@ func TestClientAdapterAdd(t *testing.T) { t.Error("expected panic when adding feature with unknown prerelease") } } + +func TestClientAdapterAddVersioned(t *testing.T) { + fg := featuregate.NewVersionedFeatureGate(version.MustParse("1.29")) + a := &clientAdapter{fg} + defaults := fg.GetAllVersioned() + + clientFeatures := map[clientfeatures.Feature]clientfeatures.VersionedSpecs{ + "FeatureA": { + {Version: version.MustParse("1.28"), PreRelease: clientfeatures.Beta, Default: true}, + }, + "FeatureB": { + {Version: version.MustParse("1.28"), PreRelease: clientfeatures.Alpha, Default: false}, + {Version: version.MustParse("1.30"), PreRelease: clientfeatures.Beta, Default: true}, + }, + } + + if err := a.AddVersioned(clientFeatures); err != nil { + t.Fatal(err) + } + all := fg.GetAllVersioned() + allexpected := map[featuregate.Feature]featuregate.VersionedSpecs{ + "FeatureA": { + {Version: version.MustParse("1.28"), PreRelease: featuregate.Beta, Default: true}, + }, + "FeatureB": { + {Version: version.MustParse("1.28"), PreRelease: featuregate.Alpha, Default: false}, + {Version: version.MustParse("1.30"), PreRelease: featuregate.Beta, Default: true}, + }, + } + for name, spec := range defaults { + allexpected[name] = spec + } + if len(all) != len(allexpected) { + t.Errorf("expected %d registered features, got %d", len(allexpected), len(all)) + } + for name, expected := range allexpected { + actual, ok := all[name] + if !ok { + t.Errorf("expected feature %q not found", name) + continue + } + if diff := cmp.Diff(actual, expected, cmpopts.IgnoreFields(featuregate.FeatureSpec{}, "Version")); diff != "" { + t.Errorf("feature %q spec mismatch (-want +got):\n%s", name, diff) + } + } + + var r interface{} + func() { + defer func() { + r = recover() + }() + _ = a.AddVersioned(map[clientfeatures.Feature]clientfeatures.VersionedSpecs{ + "FeaturePanic": {{PreRelease: "foobar"}}, + }) + }() + if r == nil { + t.Error("expected panic when adding feature with unknown prerelease") + } + if !a.Enabled("FeatureA") { + t.Error("expected Enabled(\"FeatureA\") to return true") + } + if a.Enabled("FeatureB") { + t.Error("expected Enabled(\"FeatureB\") to return false") + } +} diff --git a/pkg/features/kube_features.go b/pkg/features/kube_features.go index 64dd7459212..5870b3a4cfa 100644 --- a/pkg/features/kube_features.go +++ b/pkg/features/kube_features.go @@ -2406,6 +2406,6 @@ func init() { // are. Further, client-go features automatically support the existing mechanisms for // feature enablement metrics and test overrides. ca := &clientAdapter{utilfeature.DefaultMutableFeatureGate} - runtime.Must(clientfeatures.AddFeaturesToExistingFeatureGates(ca)) + runtime.Must(clientfeatures.AddVersionedFeaturesToExistingFeatureGates(ca)) clientfeatures.ReplaceFeatureGates(ca) } diff --git a/staging/src/k8s.io/client-go/features/features.go b/staging/src/k8s.io/client-go/features/features.go index 5ccdcc55f6c..cabb7468d61 100644 --- a/staging/src/k8s.io/client-go/features/features.go +++ b/staging/src/k8s.io/client-go/features/features.go @@ -21,6 +21,7 @@ import ( "sync/atomic" utilruntime "k8s.io/apimachinery/pkg/util/runtime" + "k8s.io/apimachinery/pkg/util/version" ) // NOTE: types Feature, FeatureSpec, prerelease (and its values) @@ -49,8 +50,14 @@ type FeatureSpec struct { LockToDefault bool // PreRelease indicates the maturity level of the feature PreRelease prerelease + // Version indicates the earliest version from which this FeatureSpec is valid. + // If multiple FeatureSpecs exist for a Feature, the one with the highest version that is less + // than or equal to the effective version of the component is used. + Version *version.Version } +type VersionedSpecs []FeatureSpec + // Gates indicates whether a given feature is enabled or not. type Gates interface { // Enabled returns true if the key is enabled. @@ -66,6 +73,15 @@ type Registry interface { Add(map[Feature]FeatureSpec) error } +// VersionedRegistry represents an external versioned feature gates registry. +type VersionedRegistry interface { + // AddVersioned adds existing versioned feature gates to the provided registry. + // + // As of today, this method is used by AddVersionedFeaturesToExistingFeatureGates and + // ReplaceFeatureGates to take control of the features exposed by this library. + AddVersioned(in map[Feature]VersionedSpecs) error +} + // FeatureGates returns the feature gates exposed by this library. // // By default, only the default features gates will be returned. @@ -85,7 +101,15 @@ func FeatureGates() Gates { // Usually this function is combined with ReplaceFeatureGates to take control of the // features exposed by this library. func AddFeaturesToExistingFeatureGates(registry Registry) error { - return registry.Add(defaultKubernetesFeatureGates) + return registry.Add(unversionedFeatureGates(defaultVersionedKubernetesFeatureGates)) +} + +// AddFeaturesToExistingFeatureGates adds the default versioned feature gates to the provided registry. +// Usually this function is combined with ReplaceFeatureGates to take control of the +// features exposed by this library. +// Generally only used by k/k. +func AddVersionedFeaturesToExistingFeatureGates(registry VersionedRegistry) error { + return registry.AddVersioned(defaultVersionedKubernetesFeatureGates) } // ReplaceFeatureGates overwrites the default implementation of the feature gates @@ -121,8 +145,23 @@ func replaceFeatureGatesWithWarningIndicator(newFeatureGates Gates) bool { return shouldProduceWarning } +// unversionedFeatureGates takes the latest entry from the VersionedSpecs of each feature, and clears out the version information, +// so that the result can be used with an unversioned feature gate. +func unversionedFeatureGates(featureGates map[Feature]VersionedSpecs) map[Feature]FeatureSpec { + unversioned := map[Feature]FeatureSpec{} + for feature, specs := range featureGates { + if len(specs) == 0 { + continue + } + latestSpec := specs[len(specs)-1] + latestSpec.Version = nil // Clear version information. + unversioned[feature] = latestSpec + } + return unversioned +} + func init() { - envVarGates := newEnvVarFeatureGates(defaultKubernetesFeatureGates) + envVarGates := newEnvVarFeatureGates(unversionedFeatureGates(defaultVersionedKubernetesFeatureGates)) wrappedFeatureGates := &featureGatesWrapper{envVarGates} featureGates.Store(wrappedFeatureGates) diff --git a/staging/src/k8s.io/client-go/features/features_test.go b/staging/src/k8s.io/client-go/features/features_test.go index 5a68303768a..4d4b7fe65da 100644 --- a/staging/src/k8s.io/client-go/features/features_test.go +++ b/staging/src/k8s.io/client-go/features/features_test.go @@ -17,17 +17,67 @@ limitations under the License. package features import ( + "reflect" "testing" "github.com/stretchr/testify/require" + "k8s.io/apimachinery/pkg/util/version" ) // TestAddFeaturesToExistingFeatureGates ensures that -// the defaultKubernetesFeatureGates are added to a test feature gates registry. +// the defaultVersionedKubernetesFeatureGates are added to a test feature gates registry. func TestAddFeaturesToExistingFeatureGates(t *testing.T) { fakeFeatureGates := &fakeRegistry{} require.NoError(t, AddFeaturesToExistingFeatureGates(fakeFeatureGates)) - require.Equal(t, defaultKubernetesFeatureGates, fakeFeatureGates.specs) + require.Equal(t, unversionedFeatureGates(defaultVersionedKubernetesFeatureGates), fakeFeatureGates.specs) +} + +// TestAddVersionedFeaturesToExistingFeatureGates ensures that +// the defaultVersionedKubernetesFeatureGates are added to a versioned test feature gates registry. +func TestAddVersionedFeaturesToExistingFeatureGates(t *testing.T) { + fakeFeatureGates := &fakeVersionedRegistry{} + require.NoError(t, AddVersionedFeaturesToExistingFeatureGates(fakeFeatureGates)) + require.Equal(t, defaultVersionedKubernetesFeatureGates, fakeFeatureGates.specs) +} + +func TestUnversionedFeatureGates(t *testing.T) { + testCases := []struct { + name string + featureGates map[Feature]VersionedSpecs + expected map[Feature]FeatureSpec + }{ + { + name: "multiple features", + featureGates: map[Feature]VersionedSpecs{ + "AlphaFeature": { + {Version: version.MustParse("1.30"), Default: false, PreRelease: Alpha}, + }, + "BetaFeature": { + {Version: version.MustParse("1.28"), Default: false, PreRelease: Alpha}, + {Version: version.MustParse("1.30"), Default: true, PreRelease: Beta}, + }, + "GAFeature": { + {Version: version.MustParse("1.25"), Default: false, PreRelease: Alpha}, + {Version: version.MustParse("1.27"), Default: true, PreRelease: Beta}, + {Version: version.MustParse("1.29"), Default: true, PreRelease: GA, LockToDefault: true}, + }, + }, + expected: map[Feature]FeatureSpec{ + "AlphaFeature": {Default: false, PreRelease: Alpha}, + "BetaFeature": {Default: true, PreRelease: Beta}, + "GAFeature": {Default: true, PreRelease: GA, LockToDefault: true}, + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + actual := unversionedFeatureGates(tc.featureGates) + if !reflect.DeepEqual(actual, tc.expected) { + t.Errorf("unversionedFeatureGates() = %v, want %v", actual, tc.expected) + } + }) + } } func TestReplaceFeatureGatesWithWarningIndicator(t *testing.T) { @@ -47,3 +97,12 @@ func (f *fakeRegistry) Add(specs map[Feature]FeatureSpec) error { f.specs = specs return nil } + +type fakeVersionedRegistry struct { + specs map[Feature]VersionedSpecs +} + +func (f *fakeVersionedRegistry) AddVersioned(specs map[Feature]VersionedSpecs) error { + f.specs = specs + return nil +} diff --git a/staging/src/k8s.io/client-go/features/known_features.go b/staging/src/k8s.io/client-go/features/known_features.go index 32317ba87f8..ea582fd337d 100644 --- a/staging/src/k8s.io/client-go/features/known_features.go +++ b/staging/src/k8s.io/client-go/features/known_features.go @@ -16,6 +16,10 @@ limitations under the License. package features +import ( + "k8s.io/apimachinery/pkg/util/version" +) + // Every feature gate should have an entry here following this template: // // // owner: @username @@ -70,15 +74,26 @@ const ( WatchListClient Feature = "WatchListClient" ) -// defaultKubernetesFeatureGates consists of all known Kubernetes-specific feature keys. +// defaultVersionedKubernetesFeatureGates consists of all known Kubernetes-specific feature keys. // // To add a new feature, define a key for it above and add it here. // After registering with the binary, the features are, by default, controllable using environment variables. // For more details, please see envVarFeatureGates implementation. -var defaultKubernetesFeatureGates = map[Feature]FeatureSpec{ - ClientsAllowCBOR: {Default: false, PreRelease: Alpha}, - ClientsPreferCBOR: {Default: false, PreRelease: Alpha}, - InOrderInformers: {Default: true, PreRelease: Beta}, - InformerResourceVersion: {Default: true, PreRelease: GA}, - WatchListClient: {Default: false, PreRelease: Beta}, +var defaultVersionedKubernetesFeatureGates = map[Feature]VersionedSpecs{ + ClientsAllowCBOR: { + {Version: version.MustParse("1.32"), Default: false, PreRelease: Alpha}, + }, + ClientsPreferCBOR: { + {Version: version.MustParse("1.32"), Default: false, PreRelease: Alpha}, + }, + InOrderInformers: { + {Version: version.MustParse("1.33"), Default: true, PreRelease: Beta}, + }, + InformerResourceVersion: { + {Version: version.MustParse("1.30"), Default: false, PreRelease: Alpha}, + {Version: version.MustParse("1.35"), Default: true, PreRelease: GA}, + }, + WatchListClient: { + {Version: version.MustParse("1.30"), Default: false, PreRelease: Beta}, + }, } diff --git a/staging/src/k8s.io/client-go/features/testing/features.go b/staging/src/k8s.io/client-go/features/testing/features.go index 4f4c3ed4de9..b5555ff800b 100644 --- a/staging/src/k8s.io/client-go/features/testing/features.go +++ b/staging/src/k8s.io/client-go/features/testing/features.go @@ -77,6 +77,7 @@ func setFeatureDuringTestInternal(tb testing.TB, feature clientfeatures.Feature, overriddenFeaturesLock.Lock() defer overriddenFeaturesLock.Unlock() delete(overriddenFeatures, feature) + // if default is not set if err := featureGates.Set(feature, originalFeatureValue); err != nil { tb.Errorf("failed restoring client-go feature: %v to its original value: %v, err: %v", feature, originalFeatureValue, err) }