Skip to content

Commit 190234f

Browse files
PersistentJZHPersistentJZH
committed
use setEmulationVersionForTest
Signed-off-by: zhihao jian <zhihao.kan17@gmail.com> fix
1 parent 463f95f commit 190234f

File tree

3 files changed

+34
-153
lines changed

3 files changed

+34
-153
lines changed

‎staging/src/k8s.io/component-base/featuregate/feature_gate.go

Lines changed: 8 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -165,13 +165,7 @@ type MutableVersionedFeatureGate interface {
165165
// Otherwise, the emulationVersion will be the same as the binary version.
166166
// If set, the feature defaults and availability will be as if the binary is at the emulated version.
167167
SetEmulationVersion(emulationVersion *version.Version) error
168-
// SetEmulationVersionWithContext is like SetEmulationVersion but accepts a context for controlling behavior.
169-
// When features have been queried and would change value due to the emulation version change,
170-
// warnings will be logged. If a test logger is provided in the context via WithTestLoggerContext,
171-
// warnings will be redirected to the test output instead of the global klog.
172-
// If features would change and no test logger is provided, an error will be returned to prevent
173-
// unexpected behavior in production environments.
174-
SetEmulationVersionWithContext(ctx context.Context, emulationVersion *version.Version) error
168+
175169
// GetAll returns a copy of the map of known feature names to versioned feature specs.
176170
GetAllVersioned() map[Feature]VersionedSpecs
177171
// AddVersioned adds versioned feature specs to the featureGate.
@@ -550,10 +544,16 @@ func (f *featureGate) GetAllVersioned() map[Feature]VersionedSpecs {
550544
}
551545

552546
func (f *featureGate) SetEmulationVersion(emulationVersion *version.Version) error {
553-
return f.SetEmulationVersionWithContext(context.Background(), emulationVersion)
547+
return f.setEmulationVersionWithContext(context.Background(), emulationVersion)
554548
}
555549

550+
// SetEmulationVersionWithContext allows passing a context for test logger support.
551+
// This is primarily intended for testing scenarios.
556552
func (f *featureGate) SetEmulationVersionWithContext(ctx context.Context, emulationVersion *version.Version) error {
553+
return f.setEmulationVersionWithContext(ctx, emulationVersion)
554+
}
555+
556+
func (f *featureGate) setEmulationVersionWithContext(ctx context.Context, emulationVersion *version.Version) error {
557557
if emulationVersion.EqualTo(f.EmulationVersion()) {
558558
return nil
559559
}
@@ -590,11 +590,6 @@ func (f *featureGate) SetEmulationVersionWithContext(ctx context.Context, emulat
590590
}
591591
}
592592

593-
// If features would change and no context logger is provided, return an error
594-
if len(changingFeatures) > 0 && contextLogger == nil {
595-
return fmt.Errorf("SetEmulationVersion would change already queried features: %s", strings.Join(changingFeatures, ", "))
596-
}
597-
598593
if len(errs) == 0 {
599594
// Persist changes
600595
f.enabled.Store(enabled)

‎staging/src/k8s.io/component-base/featuregate/feature_gate_test.go

Lines changed: 5 additions & 134 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ limitations under the License.
1717
package featuregate
1818

1919
import (
20-
"context"
2120
"fmt"
2221
"reflect"
2322
"sort"
@@ -1331,14 +1330,14 @@ func TestVersionedFeatureGateOverrideDefault(t *testing.T) {
13311330
if !f.Enabled("TestFeature2") {
13321331
t.Error("expected TestFeature2 to have effective default of true")
13331332
}
1334-
require.NoError(t, f.SetEmulationVersionWithContext(WithTestLoggerContext(context.Background(), t), version.MustParse("1.29")))
1333+
require.NoError(t, f.SetEmulationVersion(version.MustParse("1.29")))
13351334
if !f.Enabled("TestFeature1") {
13361335
t.Error("expected TestFeature1 to have effective default of true")
13371336
}
13381337
if f.Enabled("TestFeature2") {
13391338
t.Error("expected TestFeature2 to have effective default of false")
13401339
}
1341-
require.NoError(t, f.SetEmulationVersionWithContext(WithTestLoggerContext(context.Background(), t), version.MustParse("1.26")))
1340+
require.NoError(t, f.SetEmulationVersion(version.MustParse("1.26")))
13421341
if f.Enabled("TestFeature1") {
13431342
t.Error("expected TestFeature1 to have effective default of false")
13441343
}
@@ -1384,11 +1383,11 @@ func TestVersionedFeatureGateOverrideDefault(t *testing.T) {
13841383
assert.True(t, f.Enabled("TestFeature"))
13851384
assert.False(t, fcopy.Enabled("TestFeature"))
13861385

1387-
require.NoError(t, f.SetEmulationVersionWithContext(WithTestLoggerContext(context.Background(), t), version.MustParse("1.29")))
1386+
require.NoError(t, f.SetEmulationVersion(version.MustParse("1.29")))
13881387
assert.False(t, f.Enabled("TestFeature"))
13891388
assert.False(t, fcopy.Enabled("TestFeature"))
13901389

1391-
require.NoError(t, fcopy.SetEmulationVersionWithContext(WithTestLoggerContext(context.Background(), t), version.MustParse("1.29")))
1390+
require.NoError(t, fcopy.SetEmulationVersion(version.MustParse("1.29")))
13921391
assert.False(t, f.Enabled("TestFeature"))
13931392
assert.True(t, fcopy.Enabled("TestFeature"))
13941393
})
@@ -1738,7 +1737,7 @@ func TestResetFeatureValueToDefault(t *testing.T) {
17381737
assert.True(t, f.Enabled("TestAlpha"))
17391738
assert.True(t, f.Enabled("TestBeta"))
17401739

1741-
require.NoError(t, f.SetEmulationVersionWithContext(WithTestLoggerContext(context.Background(), t), version.MustParse("1.28")))
1740+
require.NoError(t, f.SetEmulationVersion(version.MustParse("1.28")))
17421741
assert.False(t, f.Enabled("AllAlpha"))
17431742
assert.False(t, f.Enabled("AllBeta"))
17441743
assert.False(t, f.Enabled("TestAlpha"))
@@ -1906,131 +1905,3 @@ func TestAddVersioned(t *testing.T) {
19061905
})
19071906
}
19081907
}
1909-
1910-
// TestSetEmulationVersionWithContext tests various scenarios for SetEmulationVersionWithContext
1911-
func TestSetEmulationVersionWithContext(t *testing.T) {
1912-
tests := []struct {
1913-
name string
1914-
features map[Feature]VersionedSpecs
1915-
queriedFeatures []string
1916-
targetVersion *version.Version
1917-
context context.Context
1918-
expectError bool
1919-
errorContains []string
1920-
expectNoError bool
1921-
}{
1922-
{
1923-
name: "warning redirection with test logger",
1924-
features: map[Feature]VersionedSpecs{
1925-
"TestFeature": {
1926-
{Version: version.MustParse("1.28"), Default: false, PreRelease: Alpha},
1927-
{Version: version.MustParse("1.29"), Default: true, PreRelease: Beta},
1928-
},
1929-
},
1930-
queriedFeatures: []string{"TestFeature"},
1931-
targetVersion: version.MustParse("1.28"),
1932-
context: WithTestLoggerContext(context.Background(), t),
1933-
expectNoError: true,
1934-
},
1935-
{
1936-
name: "error on feature change without test logger",
1937-
features: map[Feature]VersionedSpecs{
1938-
"TestFeature": {
1939-
{Version: version.MustParse("1.28"), Default: false, PreRelease: Alpha},
1940-
{Version: version.MustParse("1.29"), Default: true, PreRelease: Beta},
1941-
},
1942-
},
1943-
queriedFeatures: []string{"TestFeature"},
1944-
targetVersion: version.MustParse("1.28"),
1945-
context: context.Background(),
1946-
expectError: true,
1947-
errorContains: []string{"SetEmulationVersion would change already queried features"},
1948-
},
1949-
{
1950-
name: "multiple features changing",
1951-
features: map[Feature]VersionedSpecs{
1952-
"TestFeature1": {
1953-
{Version: version.MustParse("1.28"), Default: false, PreRelease: Alpha},
1954-
{Version: version.MustParse("1.29"), Default: true, PreRelease: Beta},
1955-
},
1956-
"TestFeature2": {
1957-
{Version: version.MustParse("1.28"), Default: true, PreRelease: Alpha},
1958-
{Version: version.MustParse("1.29"), Default: false, PreRelease: Beta},
1959-
},
1960-
},
1961-
queriedFeatures: []string{"TestFeature1", "TestFeature2"},
1962-
targetVersion: version.MustParse("1.28"),
1963-
context: context.Background(),
1964-
expectError: true,
1965-
errorContains: []string{"TestFeature1", "TestFeature2"},
1966-
},
1967-
{
1968-
name: "no change with regular context",
1969-
features: map[Feature]VersionedSpecs{
1970-
"TestFeature": {
1971-
{Version: version.MustParse("1.28"), Default: true, PreRelease: Alpha},
1972-
{Version: version.MustParse("1.29"), Default: true, PreRelease: Beta},
1973-
},
1974-
},
1975-
queriedFeatures: []string{"TestFeature"},
1976-
targetVersion: version.MustParse("1.28"),
1977-
context: context.Background(),
1978-
expectNoError: true,
1979-
},
1980-
{
1981-
name: "no change with test logger context",
1982-
features: map[Feature]VersionedSpecs{
1983-
"TestFeature": {
1984-
{Version: version.MustParse("1.28"), Default: true, PreRelease: Alpha},
1985-
{Version: version.MustParse("1.29"), Default: true, PreRelease: Beta},
1986-
},
1987-
},
1988-
queriedFeatures: []string{"TestFeature"},
1989-
targetVersion: version.MustParse("1.28"),
1990-
context: WithTestLoggerContext(context.Background(), t),
1991-
expectNoError: true,
1992-
},
1993-
{
1994-
name: "nil context with feature change",
1995-
features: map[Feature]VersionedSpecs{
1996-
"TestFeature": {
1997-
{Version: version.MustParse("1.28"), Default: false, PreRelease: Alpha},
1998-
{Version: version.MustParse("1.29"), Default: true, PreRelease: Beta},
1999-
},
2000-
},
2001-
queriedFeatures: []string{"TestFeature"},
2002-
targetVersion: version.MustParse("1.28"),
2003-
context: nil,
2004-
expectError: true,
2005-
errorContains: []string{"SetEmulationVersion would change already queried features"},
2006-
},
2007-
}
2008-
2009-
for _, tt := range tests {
2010-
t.Run(tt.name, func(t *testing.T) {
2011-
// Create feature gate
2012-
f := NewVersionedFeatureGate(version.MustParse("1.29"))
2013-
2014-
// Add features
2015-
err := f.AddVersioned(tt.features)
2016-
require.NoError(t, err)
2017-
2018-
// Query features to mark them as queried
2019-
for _, featureName := range tt.queriedFeatures {
2020-
_ = f.Enabled(Feature(featureName))
2021-
}
2022-
2023-
// Test SetEmulationVersionWithContext
2024-
err = f.SetEmulationVersionWithContext(tt.context, tt.targetVersion)
2025-
2026-
if tt.expectError {
2027-
require.Error(t, err)
2028-
for _, expectedText := range tt.errorContains {
2029-
require.Contains(t, err.Error(), expectedText)
2030-
}
2031-
} else if tt.expectNoError {
2032-
require.NoError(t, err)
2033-
}
2034-
})
2035-
}
2036-
}

‎staging/src/k8s.io/component-base/featuregate/testing/feature_gate.go

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -125,22 +125,37 @@ func SetFeatureGateEmulationVersionDuringTest(tb TB, gate featuregate.FeatureGat
125125
tb.Helper()
126126
detectParallelOverrideCleanup := detectParallelOverrideEmulationVersion(tb, ver)
127127
originalEmuVer := gate.(featuregate.MutableVersionedFeatureGate).EmulationVersion()
128-
// Use context with test logger to redirect warnings to test output
129-
ctx := featuregate.WithTestLoggerContext(context.Background(), tb)
130-
if err := gate.(featuregate.MutableVersionedFeatureGate).SetEmulationVersionWithContext(ctx, ver); err != nil {
128+
129+
if err := setEmulationVersionForTest(gate.(featuregate.MutableVersionedFeatureGate), ver, tb); err != nil {
131130
tb.Fatalf("failed to set emulation version to %s during test: %v", ver.String(), err)
132131
}
133132
tb.Cleanup(func() {
134133
tb.Helper()
135134
detectParallelOverrideCleanup()
136-
// Use context with test logger to redirect warnings to test output during cleanup
137-
ctx := featuregate.WithTestLoggerContext(context.Background(), tb)
138-
if err := gate.(featuregate.MutableVersionedFeatureGate).SetEmulationVersionWithContext(ctx, originalEmuVer); err != nil {
135+
if err := setEmulationVersionForTest(gate.(featuregate.MutableVersionedFeatureGate), originalEmuVer, tb); err != nil {
139136
tb.Fatalf("failed to restore emulation version to %s during test", originalEmuVer.String())
140137
}
141138
})
142139
}
143140

141+
// setEmulationVersionForTest sets emulation version with test-friendly warning handling.
142+
// This uses context to redirect warnings to test output.
143+
func setEmulationVersionForTest(gate featuregate.MutableVersionedFeatureGate, ver *version.Version, tb TB) error {
144+
// Check if this gate supports context-aware emulation version setting
145+
type contextAwareGate interface {
146+
SetEmulationVersionWithContext(ctx context.Context, emulationVersion *version.Version) error
147+
}
148+
149+
if contextGate, ok := gate.(contextAwareGate); ok {
150+
// Use context with test logger to redirect warnings to test output
151+
ctx := featuregate.WithTestLoggerContext(context.Background(), tb)
152+
return contextGate.SetEmulationVersionWithContext(ctx, ver)
153+
}
154+
155+
// Fallback to regular method for implementations that don't support context
156+
return gate.SetEmulationVersion(ver)
157+
}
158+
144159
func detectParallelOverride(tb TB, f featuregate.Feature) func() {
145160
tb.Helper()
146161
overrideLock.Lock()

0 commit comments

Comments
 (0)