diff --git a/pkg/util/config/feature_gate.go b/pkg/util/config/feature_gate.go index 07982ae1ffc..f8777d58c97 100644 --- a/pkg/util/config/feature_gate.go +++ b/pkg/util/config/feature_gate.go @@ -31,48 +31,78 @@ const ( // All known feature keys // To add a new feature, define a key for it below and add - // a featureInfo entry to knownFeatures. + // a featureSpec entry to knownFeatures. - // allAlpha is a global toggle for alpha features. Per feature key - // values override the default set by allAlpha. - // e.g. allAlpha=false,newFeature=true will result in newFeature=true - allAlpha = "allAlpha" + // allAlphaGate is a global toggle for alpha features. Per-feature key + // values override the default set by allAlphaGate, if they come later in the + // specification of gates. Examples: + // AllAlpha=false,NewFeature=true will result in newFeature=true + // AllAlpha=true,NewFeature=false will result in newFeature=false + allAlphaGate = "AllAlpha" ) var ( - // DefaultFeatureGate is a shared global FeatureGate. - DefaultFeatureGate = &featureGate{} - - // Default values for recorded features. - knownFeatures = map[string]featureInfo{ - allAlpha: {false, alpha}, + // Default values for recorded features. Every new feature gate should be + // represented here. + knownFeatures = map[string]featureSpec{ + allAlphaGate: {false, alpha}, } + // Special handling for a few gates. + specialFeatures = map[string]func(f *featureGate, val bool){ + allAlphaGate: setUnsetAlphaGates, + } + + // DefaultFeatureGate is a shared global FeatureGate. + DefaultFeatureGate = &featureGate{ + known: knownFeatures, + special: specialFeatures, + } +) + +type featureSpec struct { + enabled bool + prerelease prerelease +} + +type prerelease string + +const ( + // Values for prerelease. alpha = prerelease("ALPHA") beta = prerelease("BETA") ga = prerelease("") ) -type prerelease string - -type featureInfo struct { - enabled bool - prerelease prerelease -} - // FeatureGate parses and stores flag gates for known features from // a string like feature1=true,feature2=false,... type FeatureGate interface { AddFlag(fs *pflag.FlagSet) - // owner: @jlowdermilk - // alpha: v1.4 - AllAlpha() bool + + // Every feature gate should add method here following this template: + // + // // owner: @username + // // alpha: v1.4 + // MyFeature() bool + // TODO: Define accessors for each non-API alpha feature. } // featureGate implements FeatureGate as well as pflag.Value for flag parsing. type featureGate struct { - features map[string]bool + known map[string]featureSpec + special map[string]func(*featureGate, bool) + enabled map[string]bool +} + +func setUnsetAlphaGates(f *featureGate, val bool) { + for k, v := range f.known { + if v.prerelease == alpha { + if _, found := f.enabled[k]; !found { + f.enabled[k] = val + } + } + } } // Set, String, and Type implement pflag.Value @@ -80,14 +110,14 @@ type featureGate struct { // 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.features = make(map[string]bool) + f.enabled = make(map[string]bool) for _, s := range strings.Split(value, ",") { if len(s) == 0 { continue } arr := strings.SplitN(s, "=", 2) k := strings.TrimSpace(arr[0]) - _, ok := knownFeatures[k] + _, ok := f.known[k] if !ok { return fmt.Errorf("unrecognized key: %s", k) } @@ -99,15 +129,21 @@ func (f *featureGate) Set(value string) error { if err != nil { return fmt.Errorf("invalid value of %s: %s, err: %v", k, v, err) } - f.features[k] = boolValue + f.enabled[k] = boolValue + + // Handle "special" features like "all alpha gates" + if fn, found := f.special[k]; found { + fn(f, boolValue) + } } - glog.Infof("feature gates: %v", f.features) + + glog.Infof("feature gates: %v", f.enabled) return nil } func (f *featureGate) String() string { pairs := []string{} - for k, v := range f.features { + for k, v := range f.enabled { pairs = append(pairs, fmt.Sprintf("%s=%t", k, v)) } sort.Strings(pairs) @@ -118,18 +154,12 @@ func (f *featureGate) Type() string { return "mapStringBool" } -// AllAlpha returns value for allAlpha. -func (f *featureGate) AllAlpha() bool { - return f.lookup(allAlpha) -} - func (f *featureGate) lookup(key string) bool { - defaultValue := knownFeatures[key].enabled - if f.features == nil { - panic(fmt.Sprintf("--%s has not been parsed", flagName)) - } - if v, ok := f.features[key]; ok { - return v + defaultValue := f.known[key].enabled + if f.enabled != nil { + if v, ok := f.enabled[key]; ok { + return v + } } return defaultValue @@ -138,7 +168,7 @@ func (f *featureGate) lookup(key string) bool { // AddFlag adds a flag for setting global feature gates to the specified FlagSet. func (f *featureGate) AddFlag(fs *pflag.FlagSet) { var known []string - for k, v := range knownFeatures { + for k, v := range f.known { pre := "" if v.prerelease != ga { pre = fmt.Sprintf("%s - ", v.prerelease) diff --git a/pkg/util/config/feature_gate_test.go b/pkg/util/config/feature_gate_test.go index fa2c96bdc57..02b90b88f8f 100644 --- a/pkg/util/config/feature_gate_test.go +++ b/pkg/util/config/feature_gate_test.go @@ -25,32 +25,135 @@ import ( ) func TestFeatureGateFlag(t *testing.T) { + // gates for testing + const testAlphaGate = "TestAlpha" + const testBetaGate = "TestBeta" + tests := []struct { arg string - allAlpha bool - parseError error + expect map[string]bool + parseError string }{ - {fmt.Sprintf("--%s=fooBarBaz=maybeidk", flagName), false, fmt.Errorf("unrecognized key: fooBarBaz")}, - {fmt.Sprintf("--%s=", flagName), false, nil}, - {fmt.Sprintf("--%s=allAlpha=false", flagName), false, nil}, - {fmt.Sprintf("--%s=allAlpha=true", flagName), true, nil}, - {fmt.Sprintf("--%s=allAlpha=banana", flagName), false, fmt.Errorf("invalid value of allAlpha")}, + { + arg: "", + expect: map[string]bool{ + allAlphaGate: false, + testAlphaGate: false, + testBetaGate: false, + }, + }, + { + arg: "fooBarBaz=maybeidk", + expect: map[string]bool{ + allAlphaGate: false, + testAlphaGate: false, + testBetaGate: false, + }, + parseError: "unrecognized key: fooBarBaz", + }, + { + arg: "AllAlpha=false", + expect: map[string]bool{ + allAlphaGate: false, + testAlphaGate: false, + testBetaGate: false, + }, + }, + { + arg: "AllAlpha=true", + expect: map[string]bool{ + allAlphaGate: true, + testAlphaGate: true, + testBetaGate: false, + }, + }, + { + arg: "AllAlpha=banana", + expect: map[string]bool{ + allAlphaGate: false, + testAlphaGate: false, + testBetaGate: false, + }, + parseError: "invalid value of AllAlpha", + }, + { + arg: "AllAlpha=false,TestAlpha=true", + expect: map[string]bool{ + allAlphaGate: false, + testAlphaGate: true, + testBetaGate: false, + }, + }, + { + arg: "TestAlpha=true,AllAlpha=false", + expect: map[string]bool{ + allAlphaGate: false, + testAlphaGate: true, + testBetaGate: false, + }, + }, + { + arg: "AllAlpha=true,TestAlpha=false", + expect: map[string]bool{ + allAlphaGate: true, + testAlphaGate: false, + testBetaGate: false, + }, + }, + { + arg: "TestAlpha=false,AllAlpha=true", + expect: map[string]bool{ + allAlphaGate: true, + testAlphaGate: false, + testBetaGate: false, + }, + }, + { + arg: "TestBeta=true,AllAlpha=false", + expect: map[string]bool{ + allAlphaGate: false, + testAlphaGate: false, + testBetaGate: true, + }, + }, } for i, test := range tests { fs := pflag.NewFlagSet("testfeaturegateflag", pflag.ContinueOnError) - f := &featureGate{} + f := DefaultFeatureGate + f.known[testAlphaGate] = featureSpec{false, alpha} + f.known[testBetaGate] = featureSpec{false, beta} f.AddFlag(fs) - err := fs.Parse([]string{test.arg}) - if test.parseError != nil { - if !strings.Contains(err.Error(), test.parseError.Error()) { + err := fs.Parse([]string{fmt.Sprintf("--%s=%s", flagName, test.arg)}) + if test.parseError != "" { + if !strings.Contains(err.Error(), test.parseError) { t.Errorf("%d: Parse() Expected %v, Got %v", i, test.parseError, err) } } else if err != nil { t.Errorf("%d: Parse() Expected nil, Got %v", i, err) } - if alpha := f.AllAlpha(); alpha != test.allAlpha { - t.Errorf("%d: AlphaEnabled() expected %v, Got %v", i, test.allAlpha, alpha) + for k, v := range test.expect { + if f.enabled[k] != v { + t.Errorf("%d: expected %s=%v, Got %v", i, k, v, f.enabled[k]) + } } } } + +func TestFeatureGateFlagDefaults(t *testing.T) { + // gates for testing + const testAlphaGate = "TestAlpha" + const testBetaGate = "TestBeta" + + // Don't parse the flag, assert defaults are used. + f := DefaultFeatureGate + f.known[testAlphaGate] = featureSpec{false, alpha} + f.known[testBetaGate] = featureSpec{true, beta} + + if f.lookup(testAlphaGate) != false { + t.Errorf("Expected false") + } + if f.lookup(testBetaGate) != true { + t.Errorf("Expected true") + } +}