Merge pull request #123974 from p0lyn0mial/upstream-client-go-features-testing

client-go/features/testing: intro SetFeatureGatesDuringTest

staging/src/k8s.io/client-go/features/testing/features.go

@@ -0,0 +1,90 @@
+/*
+Copyright 2024 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"
+
+	clientfeatures "k8s.io/client-go/features"
+)
+
+var (
+	overriddenFeaturesLock sync.Mutex
+	overriddenFeatures     map[clientfeatures.Feature]string
+)
+
+func init() {
+	overriddenFeatures = map[clientfeatures.Feature]string{}
+}
+
+type featureGatesSetter interface {
+	clientfeatures.Gates
+
+	Set(clientfeatures.Feature, bool) error
+}
+
+// SetFeatureDuringTest sets the specified feature to the specified value for the duration of the test.
+//
+// Example use:
+//
+//	clientfeaturestesting.SetFeatureDuringTest(t, clientfeatures.WatchListClient, true)
+func SetFeatureDuringTest(tb testing.TB, feature clientfeatures.Feature, featureValue bool) {
+	if err := setFeatureDuringTestInternal(tb, feature, featureValue); err != nil {
+		tb.Fatal(err)
+	}
+}
+
+func setFeatureDuringTestInternal(tb testing.TB, feature clientfeatures.Feature, featureValue bool) error {
+	overriddenFeaturesLock.Lock()
+	defer overriddenFeaturesLock.Unlock()
+
+	currentFeatureGates := clientfeatures.FeatureGates()
+	featureGates, ok := currentFeatureGates.(featureGatesSetter)
+	if !ok {
+		panic(fmt.Errorf("clientfeatures.FeatureGates(): %T does not implement featureGatesSetter interface", currentFeatureGates))
+	}
+
+	originalFeatureValue := featureGates.Enabled(feature)
+	if overridingTestName, ok := overriddenFeatures[feature]; ok {
+		if !sameTestOrSubtest(tb, overridingTestName) {
+			return fmt.Errorf("client-go feature %q is currently overridden by %q test and cannot be also modified by %q", feature, overridingTestName, tb.Name())
+		}
+	}
+
+	if err := featureGates.Set(feature, featureValue); err != nil {
+		return err
+	}
+	overriddenFeatures[feature] = tb.Name()
+
+	tb.Cleanup(func() {
+		overriddenFeaturesLock.Lock()
+		defer overriddenFeaturesLock.Unlock()
+		delete(overriddenFeatures, feature)
+		if err := featureGates.Set(feature, originalFeatureValue); err != nil {
+			tb.Errorf("failed restoring client-go feature: %v to its original value: %v, err: %v", feature, originalFeatureValue, err)
+		}
+	})
+	return nil
+}
+
+// copied from component-base/featuregate/testing
+func sameTestOrSubtest(tb testing.TB, testName string) bool {
+	return tb.Name() == testName || strings.HasPrefix(tb.Name(), testName+"/")
+}

staging/src/k8s.io/client-go/features/testing/features_init_test.go

@@ -30,24 +30,123 @@ func TestDriveInitDefaultFeatureGates(t *testing.T) {
 	featureGates := features.FeatureGates()
 	assertFunctionPanicsWithMessage(t, func() { featureGates.Enabled("FakeFeatureGate") }, "features.FeatureGates().Enabled", fmt.Sprintf("feature %q is not registered in FeatureGate", "FakeFeatureGate"))
 
-	fakeFeatureGates := &alwaysEnabledFakeGates{}
-	require.True(t, fakeFeatureGates.Enabled("FakeFeatureGate"))
+	fakeGates := &fakeFeatureGates{features: map[features.Feature]bool{"FakeFeatureGate": true}}
+	require.True(t, fakeGates.Enabled("FakeFeatureGate"))
 
-	features.ReplaceFeatureGates(fakeFeatureGates)
+	features.ReplaceFeatureGates(fakeGates)
 	featureGates = features.FeatureGates()
 
 	assertFeatureGatesType(t, featureGates)
 	require.True(t, featureGates.Enabled("FakeFeatureGate"))
 }
 
-type alwaysEnabledFakeGates struct{}
+func TestSetFeatureGatesDuringTest(t *testing.T) {
+	featureA := features.Feature("FeatureA")
+	featureB := features.Feature("FeatureB")
+	fakeGates := &fakeFeatureGates{map[features.Feature]bool{featureA: true, featureB: true}}
+	features.ReplaceFeatureGates(fakeGates)
+	t.Cleanup(func() {
+		// since cleanup functions will be called in last added, first called order.
+		// check if the original feature wasn't restored
+		require.True(t, features.FeatureGates().Enabled(featureA), "the original feature = %v wasn't restored", featureA)
+	})
 
-func (f *alwaysEnabledFakeGates) Enabled(features.Feature) bool {
-	return true
+	SetFeatureDuringTest(t, featureA, false)
+
+	require.False(t, features.FeatureGates().Enabled(featureA))
+	require.True(t, features.FeatureGates().Enabled(featureB))
+}
+
+func TestSetFeatureGatesDuringTestPanics(t *testing.T) {
+	fakeGates := &fakeFeatureGates{features: map[features.Feature]bool{"FakeFeatureGate": true}}
+
+	features.ReplaceFeatureGates(fakeGates)
+	assertFunctionPanicsWithMessage(t, func() { SetFeatureDuringTest(t, "UnknownFeature", false) }, "SetFeatureDuringTest", fmt.Sprintf("feature %q is not registered in featureGates", "UnknownFeature"))
+
+	readOnlyGates := &readOnlyAlwaysDisabledFeatureGates{}
+	features.ReplaceFeatureGates(readOnlyGates)
+	assertFunctionPanicsWithMessage(t, func() { SetFeatureDuringTest(t, "FakeFeature", false) }, "SetFeatureDuringTest", fmt.Sprintf("clientfeatures.FeatureGates(): %T does not implement featureGatesSetter interface", readOnlyGates))
+}
+
+func TestOverridesForSetFeatureGatesDuringTest(t *testing.T) {
+	scenarios := []struct {
+		name           string
+		firstTestName  string
+		secondTestName string
+		expectError    bool
+	}{
+		{
+			name:           "concurrent tests setting the same feature fail",
+			firstTestName:  "fooTest",
+			secondTestName: "barTest",
+			expectError:    true,
+		},
+
+		{
+			name:           "same test setting the same feature does not fail",
+			firstTestName:  "fooTest",
+			secondTestName: "fooTest",
+			expectError:    false,
+		},
+
+		{
+			name:           "subtests setting the same feature don't not fail",
+			firstTestName:  "fooTest",
+			secondTestName: "fooTest/scenario1",
+			expectError:    false,
+		},
+	}
+	for _, scenario := range scenarios {
+		t.Run(scenario.name, func(t *testing.T) {
+			featureA := features.Feature("FeatureA")
+			fakeGates := &fakeFeatureGates{map[features.Feature]bool{featureA: true}}
+			fakeTesting := &fakeT{fakeTestName: scenario.firstTestName, TB: t}
+
+			features.ReplaceFeatureGates(fakeGates)
+			require.NoError(t, setFeatureDuringTestInternal(fakeTesting, featureA, true))
+			require.True(t, features.FeatureGates().Enabled(featureA))
+
+			fakeTesting.fakeTestName = scenario.secondTestName
+			err := setFeatureDuringTestInternal(fakeTesting, featureA, false)
+			require.Equal(t, scenario.expectError, err != nil)
+		})
+	}
+}
+
+type fakeFeatureGates struct {
+	features map[features.Feature]bool
+}
+
+func (f *fakeFeatureGates) Enabled(feature features.Feature) bool {
+	featureValue, ok := f.features[feature]
+	if !ok {
+		panic(fmt.Errorf("feature %q is not registered in featureGates", feature))
+	}
+	return featureValue
+}
+
+func (f *fakeFeatureGates) Set(feature features.Feature, value bool) error {
+	f.features[feature] = value
+	return nil
+}
+
+type readOnlyAlwaysDisabledFeatureGates struct{}
+
+func (f *readOnlyAlwaysDisabledFeatureGates) Enabled(feature features.Feature) bool {
+	return false
+}
+
+type fakeT struct {
+	fakeTestName string
+	testing.TB
+}
+
+func (t *fakeT) Name() string {
+	return t.fakeTestName
 }
 
 func assertFeatureGatesType(t *testing.T, fg features.Gates) {
-	_, ok := fg.(*alwaysEnabledFakeGates)
+	_, ok := fg.(*fakeFeatureGates)
 	if !ok {
 		t.Fatalf("passed features.FeatureGates() is NOT of type *alwaysEnabledFakeGates, it is of type = %T", fg)
 	}

staging/src/k8s.io/client-go/rest/request_watchlist_test.go

@@ -31,6 +31,8 @@ import (
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/watch"
+	clientfeatures "k8s.io/client-go/features"
+	clientfeaturestesting "k8s.io/client-go/features/testing"
 )
 
 func TestWatchListResult(t *testing.T) {
@@ -320,6 +322,22 @@ func TestWatchListFailure(t *testing.T) {
 	}
 }
 
+func TestWatchListWhenFeatureGateDisabled(t *testing.T) {
+	clientfeaturestesting.SetFeatureDuringTest(t, clientfeatures.WatchListClient, false)
+	expectedError := fmt.Errorf("%q feature gate is not enabled", clientfeatures.WatchListClient)
+	target := &Request{}
+
+	res := target.WatchList(context.TODO())
+
+	resErr := res.Into(nil)
+	if resErr == nil {
+		t.Fatal("expected to get an error, got nil")
+	}
+	if resErr.Error() != expectedError.Error() {
+		t.Fatalf("unexpected error: %v, expected: %v", resErr, expectedError)
+	}
+}
+
 func makePod(rv uint64) *v1.Pod {
 	return &v1.Pod{
 		ObjectMeta: metav1.ObjectMeta{

test/e2e/apimachinery/watchlist.go

@@ -30,8 +30,11 @@ import (
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/util/wait"
 	"k8s.io/apimachinery/pkg/watch"
+	utilfeature "k8s.io/apiserver/pkg/util/feature"
 	clientfeatures "k8s.io/client-go/features"
 	"k8s.io/client-go/tools/cache"
+	"k8s.io/component-base/featuregate"
+	featuregatetesting "k8s.io/component-base/featuregate/testing"
 	"k8s.io/kubernetes/test/e2e/feature"
 	"k8s.io/kubernetes/test/e2e/framework"
 )
@@ -39,13 +42,7 @@ import (
 var _ = SIGDescribe("API Streaming (aka. WatchList)", framework.WithSerial(), feature.WatchList, func() {
 	f := framework.NewDefaultFramework("watchlist")
 	ginkgo.It("should be requested when WatchListClient is enabled", func(ctx context.Context) {
-		// TODO(p0lyn0mial): use https://github.com/kubernetes/kubernetes/pull/123974
-		//  instead of using directly clientfeatures.ReplaceFeatureGates
-		prevClientFeatureGates := clientfeatures.FeatureGates()
-		defer func() {
-			clientfeatures.ReplaceFeatureGates(prevClientFeatureGates)
-		}()
-		clientfeatures.ReplaceFeatureGates(newEnabledWatchListClientFeatureGateRegistry(prevClientFeatureGates))
+		featuregatetesting.SetFeatureGateDuringTest(ginkgo.GinkgoTB(), utilfeature.DefaultFeatureGate, featuregate.Feature(clientfeatures.WatchListClient), true)
 		stopCh := make(chan struct{})
 		defer close(stopCh)
 		secretInformer := cache.NewSharedIndexInformer(
@@ -121,18 +118,3 @@ func newSecret(name string) *v1.Secret {
 		ObjectMeta: metav1.ObjectMeta{Name: name},
 	}
 }
-
-type enabledWatchListClientFeatureGateRegistry struct {
-	originalGates clientfeatures.Gates
-}
-
-func newEnabledWatchListClientFeatureGateRegistry(originalGates clientfeatures.Gates) *enabledWatchListClientFeatureGateRegistry {
-	return &enabledWatchListClientFeatureGateRegistry{originalGates: originalGates}
-}
-
-func (r *enabledWatchListClientFeatureGateRegistry) Enabled(feature clientfeatures.Feature) bool {
-	if feature == clientfeatures.WatchListClient {
-		return true
-	}
-	return r.originalGates.Enabled(feature)
-}