Merge pull request #68145 from tallclair/ga-features

Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions here: https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md.

Hide & warn on GA & deprecated feature gates

**What this PR does / why we need it**:

1. Hide GA & deprecated feature gates from the help text
2. Print a warning when GA & deprecated feature gates are explicitly set

**Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*:
For #46404

**Special notes for your reviewer**:

I need to add to my list of things I dislike about glog that it is impossible to test.

**Release note**:
```release-note
NONE
```

/kind cleanup

staging/src/k8s.io/apiserver/pkg/util/feature/BUILD

@@ -10,7 +10,10 @@ go_test(
     name = "go_default_test",
     srcs = ["feature_gate_test.go"],
     embed = [":go_default_library"],
-    deps = ["//vendor/github.com/spf13/pflag:go_default_library"],
+    deps = [
+        "//vendor/github.com/spf13/pflag:go_default_library",
+        "//vendor/github.com/stretchr/testify/assert:go_default_library",
+    ],
 )
 
 go_library(

staging/src/k8s.io/apiserver/pkg/util/feature/feature_gate.go

@@ -145,54 +145,24 @@ func NewFeatureGate() *featureGate {
 // Set parses a string of the form "key1=value1,key2=value2,..." into a
 // map[string]bool of known keys or returns an error.
 func (f *featureGate) Set(value string) error {
-	f.lock.Lock()
+	m := make(map[string]bool)
-	defer f.lock.Unlock()
-
-	// Copy existing state
-	known := map[Feature]FeatureSpec{}
-	for k, v := range f.known.Load().(map[Feature]FeatureSpec) {
-		known[k] = v
-	}
-	enabled := map[Feature]bool{}
-	for k, v := range f.enabled.Load().(map[Feature]bool) {
-		enabled[k] = v
-	}
-
 	for _, s := range strings.Split(value, ",") {
 		if len(s) == 0 {
 			continue
 		}
 		arr := strings.SplitN(s, "=", 2)
-		k := Feature(strings.TrimSpace(arr[0]))
+		k := strings.TrimSpace(arr[0])
-		featureSpec, ok := known[k]
-		if !ok {
-			return fmt.Errorf("unrecognized key: %s", k)
-		}
 		if len(arr) != 2 {
 			return fmt.Errorf("missing bool value for %s", k)
 		}
 		v := strings.TrimSpace(arr[1])
 		boolValue, err := strconv.ParseBool(v)
 		if err != nil {
-			return fmt.Errorf("invalid value of %s: %s, err: %v", k, v, err)
+			return fmt.Errorf("invalid value of %s=%s, err: %v", k, v, err)
-		}
-		enabled[k] = boolValue
-		if boolValue && featureSpec.PreRelease == Deprecated {
-			glog.Warningf("enabling deprecated feature gate %s", k)
-		}
-
-		// Handle "special" features like "all alpha gates"
-		if fn, found := f.special[k]; found {
-			fn(known, enabled, boolValue)
 		}
+		m[k] = boolValue
 	}
-
+	return f.SetFromMap(m)
-	// Persist changes
-	f.known.Store(known)
-	f.enabled.Store(enabled)
-
-	glog.V(1).Infof("feature gates: %v", enabled)
-	return nil
 }
 
 // SetFromMap stores flag gates for known features from a map[string]bool or returns an error
@@ -212,15 +182,21 @@ func (f *featureGate) SetFromMap(m map[string]bool) error {
 
 	for k, v := range m {
 		k := Feature(k)
-		_, ok := known[k]
+		featureSpec, ok := known[k]
 		if !ok {
-			return fmt.Errorf("unrecognized key: %s", k)
+			return fmt.Errorf("unrecognized feature gate: %s", k)
 		}
 		enabled[k] = v
 		// Handle "special" features like "all alpha gates"
 		if fn, found := f.special[k]; found {
 			fn(known, enabled, v)
 		}
+
+		if featureSpec.PreRelease == Deprecated {
+			glog.Warningf("Setting deprecated feature gate %s=%t. It will be removed in a future release.", k, v)
+		} else if featureSpec.PreRelease == GA {
+			glog.Warningf("Setting GA feature gate %s=%t. It will be removed in a future release.", k, v)
+		}
 	}
 
 	// Persist changes
@@ -302,14 +278,14 @@ func (f *featureGate) AddFlag(fs *pflag.FlagSet) {
 }
 
 // KnownFeatures returns a slice of strings describing the FeatureGate's known features.
+// Deprecated and GA features are hidden from the list.
 func (f *featureGate) KnownFeatures() []string {
 	var known []string
 	for k, v := range f.known.Load().(map[Feature]FeatureSpec) {
-		pre := ""
+		if v.PreRelease == GA || v.PreRelease == Deprecated {
-		if v.PreRelease != GA {
+			continue
-			pre = fmt.Sprintf("%s - ", v.PreRelease)
 		}
-		known = append(known, fmt.Sprintf("%s=true|false (%sdefault=%t)", k, pre, v.Default))
+		known = append(known, fmt.Sprintf("%s=true|false (%s - default=%t)", k, v.PreRelease, v.Default))
 	}
 	sort.Strings(known)
 	return known

staging/src/k8s.io/apiserver/pkg/util/feature/feature_gate_test.go

@@ -22,6 +22,7 @@ import (
 	"testing"
 
 	"github.com/spf13/pflag"
+	"github.com/stretchr/testify/assert"
 )
 
 func TestFeatureGateFlag(t *testing.T) {
@@ -43,13 +44,13 @@ func TestFeatureGateFlag(t *testing.T) {
 			},
 		},
 		{
-			arg: "fooBarBaz=maybeidk",
+			arg: "fooBarBaz=true",
 			expect: map[Feature]bool{
 				allAlphaGate:  false,
 				testAlphaGate: false,
 				testBetaGate:  false,
 			},
-			parseError: "unrecognized key: fooBarBaz",
+			parseError: "unrecognized feature gate: fooBarBaz",
 		},
 		{
 			arg: "AllAlpha=false",
@@ -190,6 +191,32 @@ func TestFeatureGateFlagDefaults(t *testing.T) {
 	}
 }
 
+func TestFeatureGateKnownFeatures(t *testing.T) {
+	// gates for testing
+	const (
+		testAlphaGate      Feature = "TestAlpha"
+		testBetaGate       Feature = "TestBeta"
+		testGAGate         Feature = "TestGA"
+		testDeprecatedGate Feature = "TestDeprecated"
+	)
+
+	// Don't parse the flag, assert defaults are used.
+	var f FeatureGate = NewFeatureGate()
+	f.Add(map[Feature]FeatureSpec{
+		testAlphaGate:      {Default: false, PreRelease: Alpha},
+		testBetaGate:       {Default: true, PreRelease: Beta},
+		testGAGate:         {Default: true, PreRelease: GA},
+		testDeprecatedGate: {Default: false, PreRelease: Deprecated},
+	})
+
+	known := strings.Join(f.KnownFeatures(), " ")
+
+	assert.Contains(t, known, testAlphaGate)
+	assert.Contains(t, known, testBetaGate)
+	assert.NotContains(t, known, testGAGate)
+	assert.NotContains(t, known, testDeprecatedGate)
+}
+
 func TestFeatureGateSetFromMap(t *testing.T) {
 	// gates for testing
 	const testAlphaGate Feature = "TestAlpha"
@@ -241,7 +268,7 @@ func TestFeatureGateSetFromMap(t *testing.T) {
 				testAlphaGate: false,
 				testBetaGate:  false,
 			},
-			setmapError: "unrecognized key:",
+			setmapError: "unrecognized feature gate:",
 		},
 	}
 	for i, test := range tests {