Merge pull request #134934 from siyuanfoundation/client-fg

change client go default features to versioned

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

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")
+	}
+}

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)
 }

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)

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
+}

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},
+	},
 }

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)
 		}