Skip to content

Commit

Permalink
[featuregate] Finalizing purpose of ToVersion. (#7626)
Browse files Browse the repository at this point in the history
Finalizes the purpose of `toVersion`.  `toVersion` will represent the version where the feature gate is completely removed.  Setting a `stable` gate to `true` will result in a warning log.  Setting a `stable` gate to `false` will result in an error.

Closes #7621
  • Loading branch information
TylerHelmuth authored May 4, 2023
1 parent 4266b88 commit 9ef6429
Show file tree
Hide file tree
Showing 6 changed files with 34 additions and 13 deletions.
16 changes: 16 additions & 0 deletions .chloggen/featuregate-finalize-toversion.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
change_type: enhancement

# The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver)
component: featuregate

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: Finalize purpose of `toVersion`. Allow stable gates to be explicitly set to true, but produce a warning log.

# One or more tracking issues or pull requests related to the change
issues: [7626]

# (Optional) One or more lines of additional information to render under the primary note.
# These lines will be padded with 2 spaces and then inserted directly into the document.
# Use pipe (|) for multiline entries.
subtext:
8 changes: 4 additions & 4 deletions featuregate/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,10 @@ modeled after the [system used by Kubernetes](https://kubernetes.io/docs/referen
through a `Gate`.
2. A `beta` stage where the feature has been well tested and is enabled by
default but can be disabled through a `Gate`.
3. A generally available or `stable` stage where the feature is permanently enabled and
the `Gate` is no longer operative.
4. A feature gate will be removed once it has been `stable` for at least two releases
of the collector.
3. A generally available or `stable` stage where the feature is permanently enabled. At this stage
the gate should no longer be explicitly used. Disabling the gate will produce an error and
explicitly enabling will produce a warning log.
4. A `stable` feature gate will be removed in the version specified by its `ToVersion` value.

Features that prove unworkable in the `alpha` stage may be discontinued
without proceeding to the `beta` stage. Features that make it to the `beta`
Expand Down
11 changes: 5 additions & 6 deletions featuregate/flag_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,15 +78,14 @@ func TestNewFlag(t *testing.T) {
expectedStr: "alpha,beta,stable",
},
{
name: "enable stable",
input: "stable",
expectedSetErr: true,
expected: map[string]bool{"alpha": false, "beta": true, "stable": true},
expectedStr: "-alpha,beta,stable",
name: "enable stable",
input: "stable",
expected: map[string]bool{"alpha": false, "beta": true, "stable": true},
expectedStr: "-alpha,beta,stable",
},
{
name: "disable stable",
input: "stable",
input: "-stable",
expectedSetErr: true,
expected: map[string]bool{"alpha": false, "beta": true, "stable": true},
expectedStr: "-alpha,beta,stable",
Expand Down
5 changes: 4 additions & 1 deletion featuregate/registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,10 @@ func (r *Registry) Set(id string, enabled bool) error {
}
g := v.(*Gate)
if g.stage == StageStable {
return fmt.Errorf("feature gate %q is stable, can not be modified", id)
if !enabled {
return fmt.Errorf("feature gate %q is stable, can not be disabled", id)
}
fmt.Printf("Feature gate %q is stable and already enabled. It will be removed in version %v and continued use of the gate after version %v will result in an error.\n", id, g.toVersion, g.toVersion)
}
g.enabled.Store(enabled)
return nil
Expand Down
5 changes: 4 additions & 1 deletion featuregate/registry_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,11 @@ func TestRegistryApplyError(t *testing.T) {
assert.Error(t, r.Set("foo", true))
r.MustRegister("bar", StageAlpha)
assert.Error(t, r.Set("foo", true))
r.MustRegister("foo", StageStable, WithRegisterToVersion("next"))
_, err := r.Register("foo", StageStable)
assert.Error(t, err)
assert.Error(t, r.Set("foo", true))
r.MustRegister("foo", StageStable, WithRegisterToVersion("next"))
assert.Error(t, r.Set("foo", false))
}

func TestRegistryApply(t *testing.T) {
Expand Down
2 changes: 1 addition & 1 deletion featuregate/stage.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ const (
// StageStable is used when feature is permanently enabled and can not be disabled by a Gate.
// This value is used to provide feedback to the user that the gate will be removed in the next versions.
//
// The Gate will be enabled by default and will return an error if modified.
// The Gate will be enabled by default and will return an error if disabled.
StageStable
// StageDeprecated is used when feature is permanently disabled and can not be enabled by a Gate.
// This value is used to provide feedback to the user that the gate will be removed in the next versions.
Expand Down

0 comments on commit 9ef6429

Please sign in to comment.