Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[featuregate] Finalizing purpose of ToVersion. #7626

Merged
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
6 changes: 5 additions & 1 deletion featuregate/registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,11 @@ 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 {
bogdandrutu marked this conversation as resolved.
Show resolved Hide resolved
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)
return nil
}
return fmt.Errorf("feature gate %q is stable, can not be disabled", id)
}
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