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

add functions to set client-go featuregates directly for testing #124780

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
46 changes: 44 additions & 2 deletions staging/src/k8s.io/client-go/features/envvar.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"fmt"
"os"
"strconv"
"strings"
"sync"
"sync/atomic"

Expand All @@ -34,7 +35,7 @@ var internalPackages = []string{"k8s.io/client-go/features/envvar.go"}

var _ Gates = &envVarFeatureGates{}

// newEnvVarFeatureGates creates a feature gate that allows for registration
// NewEnvVarFeatureGates creates a feature gate that allows for registration
// of features and checking if the features are enabled.
//
// On the first call to Enabled, the effective state of all known features is loaded from
Expand All @@ -47,7 +48,7 @@ var _ Gates = &envVarFeatureGates{}
//
// Please note that environmental variables can only be set to the boolean value.
// Incorrect values will be ignored and logged.
func newEnvVarFeatureGates(features map[Feature]FeatureSpec) *envVarFeatureGates {
func NewEnvVarFeatureGates(features map[Feature]FeatureSpec) *envVarFeatureGates {
known := map[Feature]FeatureSpec{}
for name, spec := range features {
known[name] = spec
Expand All @@ -74,6 +75,8 @@ type envVarFeatureGates struct {
// known holds known feature gates
known map[Feature]FeatureSpec

lock sync.Mutex

// enabled holds a map[Feature]bool
// with values explicitly set via env var
enabled atomic.Value
Expand All @@ -94,6 +97,45 @@ func (f *envVarFeatureGates) Enabled(key Feature) bool {
panic(fmt.Errorf("feature %q is not registered in FeatureGates %q", key, f.callSiteName))
}

// 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 *envVarFeatureGates) Set(value string) error {
for _, s := range strings.Split(value, ",") {
if len(s) == 0 {
continue
}
arr := strings.SplitN(s, "=", 2)
k := strings.TrimSpace(arr[0])
if len(arr) != 2 {
return fmt.Errorf("missing bool value for %s", k)
}
v := strings.TrimSpace(arr[1])
boolValue, err := strconv.ParseBool(v)
if err != nil {
return fmt.Errorf("invalid value of %s=%s, err: %v", k, v, err)
}

gateName := Feature(k)
featureSpec, ok := f.known[gateName]
if !ok {
return fmt.Errorf("unrecognized feature gate: %s", k)
}
if featureSpec.LockToDefault && featureSpec.Default != boolValue {
return fmt.Errorf("cannot set feature gate %v to %v, feature is locked to %v", k, v, featureSpec.Default)
}

func() {
f.lock.Lock()
defer f.lock.Unlock()
currGates := f.enabled.Load().(map[Feature]bool)
currGates[Feature(k)] = boolValue
f.enabled.Store(currGates)
}()
}

return nil
}

// getEnabledMapFromEnvVar will fill the enabled map on the first call.
// This is the only time a known feature can be set to a value
// read from the corresponding environmental variable.
Expand Down
6 changes: 3 additions & 3 deletions staging/src/k8s.io/client-go/features/envvar_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ func TestEnvVarFeatureGates(t *testing.T) {
for k, v := range scenario.envVariables {
t.Setenv(k, v)
}
target := newEnvVarFeatureGates(scenario.features)
target := NewEnvVarFeatureGates(scenario.features)

for expectedFeature, expectedValue := range scenario.expectedFeaturesState {
actualValue := target.Enabled(expectedFeature)
Expand All @@ -143,12 +143,12 @@ func TestEnvVarFeatureGates(t *testing.T) {
}

func TestEnvVarFeatureGatesEnabledPanic(t *testing.T) {
target := newEnvVarFeatureGates(nil)
target := NewEnvVarFeatureGates(nil)
require.PanicsWithError(t, fmt.Errorf("feature %q is not registered in FeatureGates %q", "UnknownFeature", target.callSiteName).Error(), func() { target.Enabled("UnknownFeature") })
}

func TestHasAlreadyReadEnvVar(t *testing.T) {
target := newEnvVarFeatureGates(nil)
target := NewEnvVarFeatureGates(nil)
require.False(t, target.hasAlreadyReadEnvVar())

_ = target.getEnabledMapFromEnvVar()
Expand Down
10 changes: 9 additions & 1 deletion staging/src/k8s.io/client-go/features/features.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,14 @@ type Gates interface {
Enabled(key Feature) bool
}

type MutableGates interface {
Gates

// Set parses and stores flag gates for known features
// from a string like feature1=true,feature2=false,...
Set(value string) error
}

// Registry represents an external feature gates registry.
type Registry interface {
// Add adds existing feature gates to the provided registry.
Expand Down Expand Up @@ -122,7 +130,7 @@ func replaceFeatureGatesWithWarningIndicator(newFeatureGates Gates) bool {
}

func init() {
envVarGates := newEnvVarFeatureGates(defaultKubernetesFeatureGates)
envVarGates := NewEnvVarFeatureGates(defaultKubernetesFeatureGates)

wrappedFeatureGates := &featureGatesWrapper{envVarGates}
featureGates.Store(wrappedFeatureGates)
Expand Down
89 changes: 89 additions & 0 deletions staging/src/k8s.io/client-go/features/testing/feature_gate.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
/*
Copyright 2017 The Kubernetes Authors.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package testing

import (
"fmt"
"strings"
"sync"
"testing"

"k8s.io/client-go/features"
)

// nearly direct copy from component-base with package shifting

var (
overrideLock sync.Mutex
featureFlagOverride map[features.Feature]string
)

func init() {
featureFlagOverride = map[features.Feature]string{}
}

// SetFeatureGateDuringTest sets the specified gate to the specified value for duration of the test.
// Fails when it detects second call to the same flag or is unable to set or restore feature flag.
//
// WARNING: Can leak set variable when called in test calling t.Parallel(), however second attempt to set the same feature flag will cause fatal.
//
// Example use:
//
// featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.<FeatureName>, true)
func SetFeatureGateDuringTest(tb testing.TB, gate features.Gates, f features.Feature, value bool) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is the "standard" function from component-base minus the allalpha and allbeta.

tb.Helper()
detectParallelOverrideCleanup := detectParallelOverride(tb, f)
originalValue := gate.Enabled(f)

if err := gate.(features.MutableGates).Set(fmt.Sprintf("%s=%v", f, value)); err != nil {
tb.Errorf("error setting %s=%v: %v", f, value, err)
}

tb.Cleanup(func() {
tb.Helper()
detectParallelOverrideCleanup()
if err := gate.(features.MutableGates).Set(fmt.Sprintf("%s=%v", f, originalValue)); err != nil {
tb.Errorf("error restoring %s=%v: %v", f, originalValue, err)
}
})
}

func detectParallelOverride(tb testing.TB, f features.Feature) func() {
tb.Helper()
overrideLock.Lock()
defer overrideLock.Unlock()
beforeOverrideTestName := featureFlagOverride[f]
if beforeOverrideTestName != "" && !sameTestOrSubtest(tb, beforeOverrideTestName) {
tb.Fatalf("Detected parallel setting of a feature gate by both %q and %q", beforeOverrideTestName, tb.Name())
}
featureFlagOverride[f] = tb.Name()

return func() {
tb.Helper()
overrideLock.Lock()
defer overrideLock.Unlock()
if afterOverrideTestName := featureFlagOverride[f]; afterOverrideTestName != tb.Name() {
tb.Fatalf("Detected parallel setting of a feature gate between both %q and %q", afterOverrideTestName, tb.Name())
}
featureFlagOverride[f] = beforeOverrideTestName
}
}

func sameTestOrSubtest(tb testing.TB, testName string) bool {
// Assumes that "/" is not used in test names.
return tb.Name() == testName || strings.HasPrefix(tb.Name(), testName+"/")
}
149 changes: 149 additions & 0 deletions staging/src/k8s.io/client-go/features/testing/feature_gate_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,149 @@
/*
Copyright 2023 The Kubernetes Authors.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package testing

import (
gotest "testing"

"github.com/stretchr/testify/assert"
"k8s.io/client-go/features"
)

func expect(t *gotest.T, gate features.Gates, expect map[features.Feature]bool) {
t.Helper()
for k, v := range expect {
if gate.Enabled(k) != v {
t.Errorf("Expected %v=%v, got %v", k, v, gate.Enabled(k))
}
}
}

func TestSetFeatureGateInTest(t *gotest.T) {
gate := features.NewEnvVarFeatureGates(map[features.Feature]features.FeatureSpec{
"feature": {PreRelease: features.Alpha, Default: false},
})

assert.False(t, gate.Enabled("feature"))
SetFeatureGateDuringTest(t, gate, "feature", true)
SetFeatureGateDuringTest(t, gate, "feature", true)

assert.True(t, gate.Enabled("feature"))
t.Run("Subtest", func(t *gotest.T) {
assert.True(t, gate.Enabled("feature"))
})

t.Run("ParallelSubtest", func(t *gotest.T) {
assert.True(t, gate.Enabled("feature"))
// Calling t.Parallel in subtest will resume the main test body
t.Parallel()
assert.True(t, gate.Enabled("feature"))
})
assert.True(t, gate.Enabled("feature"))

t.Run("OverwriteInSubtest", func(t *gotest.T) {
SetFeatureGateDuringTest(t, gate, "feature", false)
assert.False(t, gate.Enabled("feature"))
})
assert.True(t, gate.Enabled("feature"))
}

func TestDetectLeakToMainTest(t *gotest.T) {
t.Cleanup(func() {
featureFlagOverride = map[features.Feature]string{}
})
gate := features.NewEnvVarFeatureGates(map[features.Feature]features.FeatureSpec{
"feature": {PreRelease: features.Alpha, Default: false},
})

// Subtest setting feature gate and calling parallel will leak it out
t.Run("LeakingSubtest", func(t *gotest.T) {
fakeT := &ignoreFatalT{T: t}
SetFeatureGateDuringTest(fakeT, gate, "feature", true)
// Calling t.Parallel in subtest will resume the main test body
t.Parallel()
// Leaked false from main test
assert.False(t, gate.Enabled("feature"))
})
// Leaked true from subtest
assert.True(t, gate.Enabled("feature"))
fakeT := &ignoreFatalT{T: t}
SetFeatureGateDuringTest(fakeT, gate, "feature", false)
assert.True(t, fakeT.fatalRecorded)
}

func TestDetectLeakToOtherSubtest(t *gotest.T) {
t.Cleanup(func() {
featureFlagOverride = map[features.Feature]string{}
})
gate := features.NewEnvVarFeatureGates(map[features.Feature]features.FeatureSpec{
"feature": {PreRelease: features.Alpha, Default: false},
})

subtestName := "Subtest"
// Subtest setting feature gate and calling parallel will leak it out
t.Run(subtestName, func(t *gotest.T) {
fakeT := &ignoreFatalT{T: t}
SetFeatureGateDuringTest(fakeT, gate, "feature", true)
t.Parallel()
})
// Add suffix to name to prevent tests with the same prefix.
t.Run(subtestName+"Suffix", func(t *gotest.T) {
// Leaked true
assert.True(t, gate.Enabled("feature"))

fakeT := &ignoreFatalT{T: t}
SetFeatureGateDuringTest(fakeT, gate, "feature", false)
assert.True(t, fakeT.fatalRecorded)
})
}

func TestCannotDetectLeakFromSubtest(t *gotest.T) {
t.Cleanup(func() {
featureFlagOverride = map[features.Feature]string{}
})
gate := features.NewEnvVarFeatureGates(map[features.Feature]features.FeatureSpec{
"feature": {PreRelease: features.Alpha, Default: false},
})

SetFeatureGateDuringTest(t, gate, "feature", false)
// Subtest setting feature gate and calling parallel will leak it out
t.Run("Subtest", func(t *gotest.T) {
SetFeatureGateDuringTest(t, gate, "feature", true)
t.Parallel()
})
// Leaked true
assert.True(t, gate.Enabled("feature"))
}

type ignoreFatalT struct {
*gotest.T
fatalRecorded bool
}

func (f *ignoreFatalT) Fatal(args ...any) {
f.T.Helper()
f.fatalRecorded = true
newArgs := []any{"[IGNORED]"}
newArgs = append(newArgs, args...)
f.T.Log(newArgs...)
}

func (f *ignoreFatalT) Fatalf(format string, args ...any) {
f.T.Helper()
f.fatalRecorded = true
f.T.Logf("[IGNORED] "+format, args...)
}