diff --git a/pkg/util/config/feature_gate.go b/pkg/util/config/feature_gate.go index 07982ae1ffc..ccdbde12448 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 is a global toggle for alpha features. Per-feature key + // values override the default set by allAlpha, 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 allAlpha = "allAlpha" ) var ( - // DefaultFeatureGate is a shared global FeatureGate. - DefaultFeatureGate = &featureGate{} - - // Default values for recorded features. - knownFeatures = map[string]featureInfo{ + // Default values for recorded features. Every new feature gate should be + // represented here. + knownFeatures = map[string]featureSpec{ allAlpha: {false, alpha}, } + // Special handling for a few gates. + specialFeatures = map[string]func(f *featureGate, val bool){ + allAlpha: 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,17 +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 { + defaultValue := f.known[key].enabled + if f.enabled == nil { panic(fmt.Sprintf("--%s has not been parsed", flagName)) } - if v, ok := f.features[key]; ok { + if v, ok := f.enabled[key]; ok { return v } return defaultValue @@ -138,7 +169,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..a9c9721d3bb 100644 --- a/pkg/util/config/feature_gate_test.go +++ b/pkg/util/config/feature_gate_test.go @@ -25,32 +25,117 @@ import ( ) func TestFeatureGateFlag(t *testing.T) { + // gates for testing + const testAlpha = "testAlpha" + const testBeta = "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{ + allAlpha: false, + testAlpha: false, + testBeta: false, + }, + }, + { + arg: "fooBarBaz=maybeidk", + expect: map[string]bool{ + allAlpha: false, + testAlpha: false, + testBeta: false, + }, + parseError: "unrecognized key: fooBarBaz", + }, + { + arg: "allAlpha=false", + expect: map[string]bool{ + allAlpha: false, + testAlpha: false, + testBeta: false, + }, + }, + { + arg: "allAlpha=true", + expect: map[string]bool{ + allAlpha: true, + testAlpha: true, + testBeta: false, + }, + }, + { + arg: "allAlpha=banana", + expect: map[string]bool{ + allAlpha: false, + testAlpha: false, + testBeta: false, + }, + parseError: "invalid value of allAlpha", + }, + { + arg: "allAlpha=false,testAlpha=true", + expect: map[string]bool{ + allAlpha: false, + testAlpha: true, + testBeta: false, + }, + }, + { + arg: "testAlpha=true,allAlpha=false", + expect: map[string]bool{ + allAlpha: false, + testAlpha: true, + testBeta: false, + }, + }, + { + arg: "allAlpha=true,testAlpha=false", + expect: map[string]bool{ + allAlpha: true, + testAlpha: false, + testBeta: false, + }, + }, + { + arg: "testAlpha=false,allAlpha=true", + expect: map[string]bool{ + allAlpha: true, + testAlpha: false, + testBeta: false, + }, + }, + { + arg: "testBeta=true,allAlpha=false", + expect: map[string]bool{ + allAlpha: false, + testAlpha: false, + testBeta: true, + }, + }, } for i, test := range tests { fs := pflag.NewFlagSet("testfeaturegateflag", pflag.ContinueOnError) - f := &featureGate{} + f := DefaultFeatureGate + f.known[testAlpha] = featureSpec{false, alpha} + f.known[testBeta] = 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]) + } } } }