Skip to content

Commit

Permalink
Removes dependency on other k8s packages in featuregate pkg.
Browse files Browse the repository at this point in the history
The removed packages are:

 * k8s.io/apimachinery/pkg/util/naming
 * k8s.io/klog/v2

Do note that removing naming package necessitates adding feature gate
name argument to featuregate.New().
  • Loading branch information
keabsb committed Jun 14, 2024
1 parent c27c5af commit 841d15b
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 36 deletions.
4 changes: 2 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
module go.etcd.io/etcd/v3

go 1.22.0
go 1.22

toolchain go1.22.4

Expand Down Expand Up @@ -95,6 +95,6 @@ require (
google.golang.org/genproto/googleapis/rpc v0.0.0-20240520151616-dc85e6b867a5 // indirect
gopkg.in/natefinch/lumberjack.v2 v2.2.1 // indirect
gopkg.in/yaml.v3 v3.0.1 // indirect
sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd // indirect
sigs.k8s.io/json v0.0.0-20211020170558-c049b76a60c6 // indirect
sigs.k8s.io/yaml v1.4.0 // indirect
)
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,7 @@ gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=
gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
honnef.co/go/tools v0.0.0-20190102054323-c2f93a96b099/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=
honnef.co/go/tools v0.0.0-20190523083050-ea95bdfd59fc/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=
sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd h1:EDPBXCAspyGV4jQlpZSudPeMmr1bNJefnuqLsRAsHZo=
sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd/go.mod h1:B8JuhiUyNFVKdsE8h686QcCxMaH6HrOAZj4vswFpcB0=
sigs.k8s.io/json v0.0.0-20211020170558-c049b76a60c6 h1:fD1pz4yfdADVNfFmcP2aBEtudwUQ1AlLnRBALr33v3s=
sigs.k8s.io/json v0.0.0-20211020170558-c049b76a60c6/go.mod h1:p4QtZmO4uMYipTQNzagwnNoseA6OxSUutVw05NhYDRs=
sigs.k8s.io/yaml v1.4.0 h1:Mk1wCc2gy/F0THH0TAp1QYyJNzRm2KCLy3o5ASXVI5E=
sigs.k8s.io/yaml v1.4.0/go.mod h1:Ejl7/uTz7PSA4eKMyQCUTnhZYNmLIl+5c2lQPGR2BPY=
30 changes: 12 additions & 18 deletions pkg/featuregate/feature_gate.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
package featuregate

import (
"context"
"fmt"
"sort"
"strconv"
Expand All @@ -25,9 +24,7 @@ import (
"sync/atomic"

"github.com/spf13/pflag"

"k8s.io/apimachinery/pkg/util/naming"
"k8s.io/klog/v2"
"go.uber.org/zap"
)

type Feature string
Expand Down Expand Up @@ -128,6 +125,8 @@ type MutableFeatureGate interface {

// featureGate implements FeatureGate as well as pflag.Value for flag parsing.
type featureGate struct {
lg *zap.Logger

featureGateName string

special map[Feature]func(map[Feature]FeatureSpec, map[Feature]bool, bool)
Expand Down Expand Up @@ -165,18 +164,15 @@ func setUnsetBetaGates(known map[Feature]FeatureSpec, enabled map[Feature]bool,
// Set, String, and Type implement pflag.Value
var _ pflag.Value = &featureGate{}

// internalPackages are packages that ignored when creating a name for featureGates. These packages are in the common
// call chains, so they'd be unhelpful as names.
var internalPackages = []string{"k8s.io/component-base/featuregate/feature_gate.go"}

func NewFeatureGate() *featureGate {
func New(name string, lg *zap.Logger) *featureGate {
known := map[Feature]FeatureSpec{}
for k, v := range defaultFeatures {
known[k] = v
}

f := &featureGate{
featureGateName: naming.GetNameFromCallsite(internalPackages...),
lg: lg,
featureGateName: name,
special: specialFeatures,
}
f.known.Store(known)
Expand Down Expand Up @@ -239,17 +235,17 @@ func (f *featureGate) SetFromMap(m map[string]bool) error {
}

if featureSpec.PreRelease == Deprecated {
klog.Warningf("Setting deprecated feature gate %s=%t. It will be removed in a future release.", k, v)
f.lg.Warn(fmt.Sprintf("Setting deprecated feature gate %s=%t. It will be removed in a future release.", k, v))
} else if featureSpec.PreRelease == GA {
klog.Warningf("Setting GA feature gate %s=%t. It will be removed in a future release.", k, v)
f.lg.Warn(fmt.Sprintf("Setting GA feature gate %s=%t. It will be removed in a future release.", k, v))
}
}

// Persist changes
f.known.Store(known)
f.enabled.Store(enabled)

klog.V(1).Infof("feature gates: %v", f.enabled)
f.lg.Info(fmt.Sprintf("feature gates: %v", f.enabled))
return nil
}

Expand Down Expand Up @@ -319,9 +315,9 @@ func (f *featureGate) OverrideDefault(name Feature, override bool) error {
case spec.LockToDefault:
return fmt.Errorf("cannot override default: feature %q default is locked to %t", name, spec.Default)
case spec.PreRelease == Deprecated:
klog.Warningf("Overriding default of deprecated feature gate %s=%t. It will be removed in a future release.", name, override)
f.lg.Warn(fmt.Sprintf("Overriding default of deprecated feature gate %s=%t. It will be removed in a future release.", name, override))
case spec.PreRelease == GA:
klog.Warningf("Overriding default of GA feature gate %s=%t. It will be removed in a future release.", name, override)
f.lg.Warn(fmt.Sprintf("Overriding default of GA feature gate %s=%t. It will be removed in a future release.", name, override))
}

spec.Default = override
Expand Down Expand Up @@ -369,9 +365,7 @@ func (f *featureGate) AddFlag(fs *pflag.FlagSet) {
}

func (f *featureGate) AddMetrics() {
for feature, featureSpec := range f.GetAll() {
featuremetrics.RecordFeatureInfo(context.Background(), string(feature), string(featureSpec.PreRelease), f.Enabled(feature))
}
// TODO(henrybear327): implement this.
}

// KnownFeatures returns a slice of strings describing the FeatureGate's known features.
Expand Down
29 changes: 15 additions & 14 deletions pkg/featuregate/feature_gate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (

"github.com/spf13/pflag"
"github.com/stretchr/testify/assert"
"go.uber.org/zap/zaptest"
)

func TestFeatureGateFlag(t *testing.T) {
Expand Down Expand Up @@ -203,7 +204,7 @@ func TestFeatureGateFlag(t *testing.T) {
for i, test := range tests {
t.Run(test.arg, func(t *testing.T) {
fs := pflag.NewFlagSet("testfeaturegateflag", pflag.ContinueOnError)
f := NewFeatureGate()
f := New("test", zaptest.NewLogger(t))
f.Add(map[Feature]FeatureSpec{
testAlphaGate: {Default: false, PreRelease: Alpha},
testBetaGate: {Default: false, PreRelease: Beta},
Expand Down Expand Up @@ -232,7 +233,7 @@ func TestFeatureGateOverride(t *testing.T) {
const testBetaGate Feature = "TestBeta"

// Don't parse the flag, assert defaults are used.
var f *featureGate = NewFeatureGate()
var f = New("test", zaptest.NewLogger(t))
f.Add(map[Feature]FeatureSpec{
testAlphaGate: {Default: false, PreRelease: Alpha},
testBetaGate: {Default: false, PreRelease: Beta},
Expand Down Expand Up @@ -261,7 +262,7 @@ func TestFeatureGateFlagDefaults(t *testing.T) {
const testBetaGate Feature = "TestBeta"

// Don't parse the flag, assert defaults are used.
var f *featureGate = NewFeatureGate()
var f = New("test", zaptest.NewLogger(t))
f.Add(map[Feature]FeatureSpec{
testAlphaGate: {Default: false, PreRelease: Alpha},
testBetaGate: {Default: true, PreRelease: Beta},
Expand All @@ -285,7 +286,7 @@ func TestFeatureGateKnownFeatures(t *testing.T) {
)

// Don't parse the flag, assert defaults are used.
var f *featureGate = NewFeatureGate()
var f = New("test", zaptest.NewLogger(t))
f.Add(map[Feature]FeatureSpec{
testAlphaGate: {Default: false, PreRelease: Alpha},
testBetaGate: {Default: true, PreRelease: Beta},
Expand Down Expand Up @@ -392,7 +393,7 @@ func TestFeatureGateSetFromMap(t *testing.T) {
}
for i, test := range tests {
t.Run(fmt.Sprintf("SetFromMap %s", test.name), func(t *testing.T) {
f := NewFeatureGate()
f := New("test", zaptest.NewLogger(t))
f.Add(map[Feature]FeatureSpec{
testAlphaGate: {Default: false, PreRelease: Alpha},
testBetaGate: {Default: false, PreRelease: Beta},
Expand Down Expand Up @@ -462,7 +463,7 @@ func TestFeatureGateString(t *testing.T) {
}
for i, test := range tests {
t.Run(fmt.Sprintf("SetFromMap %s", test.expect), func(t *testing.T) {
f := NewFeatureGate()
f := New("test", zaptest.NewLogger(t))
f.Add(featuremap)
f.SetFromMap(test.setmap)
result := f.String()
Expand All @@ -475,7 +476,7 @@ func TestFeatureGateString(t *testing.T) {

func TestFeatureGateOverrideDefault(t *testing.T) {
t.Run("overrides take effect", func(t *testing.T) {
f := NewFeatureGate()
f := New("test", zaptest.NewLogger(t))
if err := f.Add(map[Feature]FeatureSpec{
"TestFeature1": {Default: true},
"TestFeature2": {Default: false},
Expand All @@ -497,7 +498,7 @@ func TestFeatureGateOverrideDefault(t *testing.T) {
})

t.Run("overrides are preserved across deep copies", func(t *testing.T) {
f := NewFeatureGate()
f := New("test", zaptest.NewLogger(t))
if err := f.Add(map[Feature]FeatureSpec{"TestFeature": {Default: false}}); err != nil {
t.Fatal(err)
}
Expand All @@ -511,7 +512,7 @@ func TestFeatureGateOverrideDefault(t *testing.T) {
})

t.Run("reflected in known features", func(t *testing.T) {
f := NewFeatureGate()
f := New("test", zaptest.NewLogger(t))
if err := f.Add(map[Feature]FeatureSpec{"TestFeature": {
Default: false,
PreRelease: Alpha,
Expand All @@ -537,7 +538,7 @@ func TestFeatureGateOverrideDefault(t *testing.T) {
})

t.Run("may not change default for specs with locked defaults", func(t *testing.T) {
f := NewFeatureGate()
f := New("test", zaptest.NewLogger(t))
if err := f.Add(map[Feature]FeatureSpec{
"LockedFeature": {
Default: true,
Expand All @@ -555,7 +556,7 @@ func TestFeatureGateOverrideDefault(t *testing.T) {
})

t.Run("does not supersede explicitly-set value", func(t *testing.T) {
f := NewFeatureGate()
f := New("test", zaptest.NewLogger(t))
if err := f.Add(map[Feature]FeatureSpec{"TestFeature": {Default: true}}); err != nil {
t.Fatal(err)
}
Expand All @@ -571,7 +572,7 @@ func TestFeatureGateOverrideDefault(t *testing.T) {
})

t.Run("prevents re-registration of feature spec after overriding default", func(t *testing.T) {
f := NewFeatureGate()
f := New("test", zaptest.NewLogger(t))
if err := f.Add(map[Feature]FeatureSpec{
"TestFeature": {
Default: true,
Expand All @@ -594,14 +595,14 @@ func TestFeatureGateOverrideDefault(t *testing.T) {
})

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

t.Run("returns error if already added to flag set", func(t *testing.T) {
f := NewFeatureGate()
f := New("test", zaptest.NewLogger(t))
fs := pflag.NewFlagSet("test", pflag.ContinueOnError)
f.AddFlag(fs)

Expand Down

0 comments on commit 841d15b

Please sign in to comment.