Skip to content

Commit

Permalink
Support overrides for registered feature defaults.
Browse files Browse the repository at this point in the history
This is to support the goal of enabling a feature by default for a single component only when the
feature in question is consumed by multiple components.

Overriden defaults are reflected in KnownFeatures and registered flag text.
  • Loading branch information
benluddy committed Jan 8, 2024
1 parent 2cf7465 commit a0990b9
Show file tree
Hide file tree
Showing 2 changed files with 162 additions and 0 deletions.
34 changes: 34 additions & 0 deletions staging/src/k8s.io/component-base/featuregate/feature_gate.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,17 @@ type MutableFeatureGate interface {
GetAll() map[Feature]FeatureSpec
// AddMetrics adds feature enablement metrics
AddMetrics()
// OverrideDefault sets a local override for the registered default value of a named
// feature. If the feature has not been previously registered (e.g. by a call to Add), has a
// locked default, or if the gate has already registered itself with a FlagSet, a non-nil
// error is returned.
//
// When two or more components consume a common feature, one component can override its
// default at runtime in order to adopt new defaults before or after the other
// components. For example, a new feature can be evaluated with a limited blast radius by
// overriding its default to true for a limited number of components without simultaneously
// changing its default for all consuming components.
OverrideDefault(name Feature, override bool) error
}

// featureGate implements FeatureGate as well as pflag.Value for flag parsing.
Expand Down Expand Up @@ -296,6 +307,29 @@ func (f *featureGate) Add(features map[Feature]FeatureSpec) error {
return nil
}

func (f *featureGate) OverrideDefault(name Feature, override bool) error {
f.lock.Lock()
defer f.lock.Unlock()

known := map[Feature]FeatureSpec{}
for name, spec := range f.known.Load().(map[Feature]FeatureSpec) {
known[name] = spec
}

if spec, ok := known[name]; !ok {
return fmt.Errorf("cannot override default: feature %q is not registered", name)
} else if spec.LockToDefault && override != spec.Default {
return fmt.Errorf("cannot override default: feature %q default is locked to %t", name, spec.Default)
} else {
spec.Default = override
known[name] = spec
}

f.known.Store(known)

return nil
}

// GetAll returns a copy of the map of known feature names to feature specs.
func (f *featureGate) GetAll() map[Feature]FeatureSpec {
retval := map[Feature]FeatureSpec{}
Expand Down
128 changes: 128 additions & 0 deletions staging/src/k8s.io/component-base/featuregate/feature_gate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -508,3 +508,131 @@ func TestFeatureGateString(t *testing.T) {
})
}
}

func TestFeatureGateOverrideDefault(t *testing.T) {
t.Run("overrides take effect", func(t *testing.T) {
f := NewFeatureGate()
if err := f.Add(map[Feature]FeatureSpec{
"TestFeature1": {Default: true},
"TestFeature2": {Default: false},
}); err != nil {
t.Fatal(err)
}
if err := f.OverrideDefault("TestFeature1", false); err != nil {
t.Fatal(err)
}
if err := f.OverrideDefault("TestFeature2", true); err != nil {
t.Fatal(err)
}
if f.Enabled("TestFeature1") {
t.Error("expected TestFeature1 to have effective default of false")
}
if !f.Enabled("TestFeature2") {
t.Error("expected TestFeature2 to have effective default of true")
}
})

t.Run("overrides are preserved across deep copies", func(t *testing.T) {
f := NewFeatureGate()
if err := f.Add(map[Feature]FeatureSpec{"TestFeature": {Default: false}}); err != nil {
t.Fatal(err)
}
if err := f.OverrideDefault("TestFeature", true); err != nil {
t.Fatal(err)
}
fcopy := f.DeepCopy()
if !fcopy.Enabled("TestFeature") {
t.Error("default override was not preserved by deep copy")
}
})

t.Run("reflected in known features", func(t *testing.T) {
f := NewFeatureGate()
if err := f.Add(map[Feature]FeatureSpec{"TestFeature": {
Default: false,
PreRelease: Alpha,
}}); err != nil {
t.Fatal(err)
}
if err := f.OverrideDefault("TestFeature", true); err != nil {
t.Fatal(err)
}
var found bool
for _, s := range f.KnownFeatures() {
if !strings.Contains(s, "TestFeature") {
continue
}
found = true
if !strings.Contains(s, "default=true") {
t.Errorf("expected override of default to be reflected in known feature description %q", s)
}
}
if !found {
t.Error("found no entry for TestFeature in known features")
}
})

t.Run("may not change default for specs with locked defaults", func(t *testing.T) {
f := NewFeatureGate()
if err := f.Add(map[Feature]FeatureSpec{
"LockedFeature": {
Default: true,
LockToDefault: true,
},
}); err != nil {
t.Fatal(err)
}
if err := f.OverrideDefault("LockedFeature", true); err != nil {
t.Errorf("expected no error from attempt to override default to the registered default")
}
if f.OverrideDefault("LockedFeature", false) == nil {
t.Error("expected error when attempting to override the default for a feature with a locked default")
}
})

t.Run("does not supersede explicitly-set value", func(t *testing.T) {
f := NewFeatureGate()
if err := f.Add(map[Feature]FeatureSpec{"TestFeature": {Default: true}}); err != nil {
t.Fatal(err)
}
if err := f.OverrideDefault("TestFeature", false); err != nil {
t.Fatal(err)
}
if err := f.SetFromMap(map[string]bool{"TestFeature": true}); err != nil {
t.Fatal(err)
}
if !f.Enabled("TestFeature") {
t.Error("expected feature to be effectively enabled despite default override")
}
})

t.Run("prevents re-registration of feature spec after overriding default", func(t *testing.T) {
f := NewFeatureGate()
if err := f.Add(map[Feature]FeatureSpec{
"TestFeature": {
Default: true,
PreRelease: Alpha,
},
}); err != nil {
t.Fatal(err)
}
if err := f.OverrideDefault("TestFeature", false); err != nil {
t.Fatal(err)
}
if err := f.Add(map[Feature]FeatureSpec{
"TestFeature": {
Default: true,
PreRelease: Alpha,
},
}); err == nil {
t.Error("expected re-registration to return a non-nil error after overriding its default")
}
})

t.Run("does not allow override for an unknown feature", func(t *testing.T) {
f := NewFeatureGate()
if err := f.OverrideDefault("TestFeature", true); err == nil {
t.Error("expected an error to be returned in attempt to override default for unregistered feature")
}
})
}

0 comments on commit a0990b9

Please sign in to comment.