Fix storage feature gate test setting

pkg/controller/volume/expand/BUILD

@@ -1,9 +1,6 @@
 package(default_visibility = ["//visibility:public"])
 
-load(
-    "@io_bazel_rules_go//go:def.bzl",
-    "go_library",
-)
+load("@io_bazel_rules_go//go:def.bzl", "go_library")
 
 go_library(
     name = "go_default_library",

pkg/controller/volume/persistentvolume/BUILD

@@ -82,6 +82,7 @@ go_test(
         "//pkg/api/testapi:go_default_library",
         "//pkg/apis/core:go_default_library",
         "//pkg/controller:go_default_library",
+        "//pkg/features:go_default_library",
         "//pkg/volume:go_default_library",
         "//pkg/volume/util:go_default_library",
         "//pkg/volume/util/recyclerclient:go_default_library",
@@ -96,6 +97,7 @@ go_test(
         "//staging/src/k8s.io/apimachinery/pkg/util/wait:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/watch:go_default_library",
         "//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library",
+        "//staging/src/k8s.io/apiserver/pkg/util/feature/testing:go_default_library",
         "//staging/src/k8s.io/client-go/informers:go_default_library",
         "//staging/src/k8s.io/client-go/kubernetes:go_default_library",
         "//staging/src/k8s.io/client-go/kubernetes/fake:go_default_library",

pkg/controller/volume/persistentvolume/binder_test.go

@@ -639,9 +639,6 @@ func TestSync(t *testing.T) {
 		},
 	}
 
-	utilfeature.DefaultFeatureGate.Set("VolumeScheduling=true")
-	defer utilfeature.DefaultFeatureGate.Set("VolumeScheduling=false")
-
 	runSyncTests(t, tests, []*storage.StorageClass{
 		{
 			ObjectMeta:        metav1.ObjectMeta{Name: classWait},

pkg/controller/volume/persistentvolume/index_test.go

@@ -24,9 +24,11 @@ import (
 	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	utilfeature "k8s.io/apiserver/pkg/util/feature"
+	utilfeaturetesting "k8s.io/apiserver/pkg/util/feature/testing"
 	"k8s.io/client-go/kubernetes/scheme"
 	ref "k8s.io/client-go/tools/reference"
 	"k8s.io/kubernetes/pkg/api/testapi"
+	"k8s.io/kubernetes/pkg/features"
 	"k8s.io/kubernetes/pkg/volume/util"
 )
 
@@ -1207,7 +1209,7 @@ func TestAlphaFilteringVolumeModes(t *testing.T) {
 	toggleFeature(false, "BlockVolume", t)
 }
 
-func TestAlphaStorageObjectInUseProtectionFiltering(t *testing.T) {
+func TestStorageObjectInUseProtectionFiltering(t *testing.T) {
 	pv := &v1.PersistentVolume{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:        "pv1",
@@ -1271,17 +1273,19 @@ func TestAlphaStorageObjectInUseProtectionFiltering(t *testing.T) {
 	}
 
 	for name, testCase := range satisfyingTestCases {
-		toggleFeature(testCase.enableStorageObjectInUseProtection, "StorageObjectInUseProtection", t)
-		err := checkVolumeSatisfyClaim(testCase.vol, testCase.pvc)
-		// expected to match but got an error
-		if err != nil && testCase.isExpectedMatch {
-			t.Errorf("%s: expected to match but got an error: %v", name, err)
-		}
-		// not expected to match but did
-		if err == nil && !testCase.isExpectedMatch {
-			t.Errorf("%s: not expected to match but did", name)
-		}
+		t.Run(name, func(t *testing.T) {
+			defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.StorageObjectInUseProtection, testCase.enableStorageObjectInUseProtection)()
 
+			err := checkVolumeSatisfyClaim(testCase.vol, testCase.pvc)
+			// expected to match but got an error
+			if err != nil && testCase.isExpectedMatch {
+				t.Errorf("%s: expected to match but got an error: %v", name, err)
+			}
+			// not expected to match but did
+			if err == nil && !testCase.isExpectedMatch {
+				t.Errorf("%s: not expected to match but did", name)
+			}
+		})
 	}
 
 	filteringTestCases := map[string]struct {
@@ -1316,26 +1320,26 @@ func TestAlphaStorageObjectInUseProtectionFiltering(t *testing.T) {
 		},
 	}
 	for name, testCase := range filteringTestCases {
-		toggleFeature(testCase.enableStorageObjectInUseProtection, "StorageObjectInUseProtection", t)
-		pvmatch, err := testCase.vol.findBestMatchForClaim(testCase.pvc, false)
-		// expected to match but either got an error or no returned pvmatch
-		if pvmatch == nil && testCase.isExpectedMatch {
-			t.Errorf("Unexpected failure for testcase, no matching volume: %s", name)
-		}
-		if err != nil && testCase.isExpectedMatch {
-			t.Errorf("Unexpected failure for testcase: %s - %+v", name, err)
-		}
-		// expected to not match but either got an error or a returned pvmatch
-		if pvmatch != nil && !testCase.isExpectedMatch {
-			t.Errorf("Unexpected failure for testcase, expected no matching volume: %s", name)
-		}
-		if err != nil && !testCase.isExpectedMatch {
-			t.Errorf("Unexpected failure for testcase: %s - %+v", name, err)
-		}
-	}
+		t.Run(name, func(t *testing.T) {
+			defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.StorageObjectInUseProtection, testCase.enableStorageObjectInUseProtection)()
 
-	// make sure feature gate is turned off
-	toggleFeature(false, "StorageObjectInUseProtection", t)
+			pvmatch, err := testCase.vol.findBestMatchForClaim(testCase.pvc, false)
+			// expected to match but either got an error or no returned pvmatch
+			if pvmatch == nil && testCase.isExpectedMatch {
+				t.Errorf("Unexpected failure for testcase, no matching volume: %s", name)
+			}
+			if err != nil && testCase.isExpectedMatch {
+				t.Errorf("Unexpected failure for testcase: %s - %+v", name, err)
+			}
+			// expected to not match but either got an error or a returned pvmatch
+			if pvmatch != nil && !testCase.isExpectedMatch {
+				t.Errorf("Unexpected failure for testcase, expected no matching volume: %s", name)
+			}
+			if err != nil && !testCase.isExpectedMatch {
+				t.Errorf("Unexpected failure for testcase: %s - %+v", name, err)
+			}
+		})
+	}
 }
 
 func TestFindingPreboundVolumes(t *testing.T) {

pkg/controller/volume/persistentvolume/provision_test.go

@@ -34,6 +34,7 @@ var class2Parameters = map[string]string{
 	"param2": "value2",
 }
 var deleteReclaimPolicy = v1.PersistentVolumeReclaimDelete
+var modeImmediate = storage.VolumeBindingImmediate
 var storageClasses = []*storage.StorageClass{
 	{
 		TypeMeta: metav1.TypeMeta{
@@ -44,9 +45,10 @@ var storageClasses = []*storage.StorageClass{
 			Name: "gold",
 		},
 
-		Provisioner:   mockPluginName,
-		Parameters:    class1Parameters,
-		ReclaimPolicy: &deleteReclaimPolicy,
+		Provisioner:       mockPluginName,
+		Parameters:        class1Parameters,
+		ReclaimPolicy:     &deleteReclaimPolicy,
+		VolumeBindingMode: &modeImmediate,
 	},
 	{
 		TypeMeta: metav1.TypeMeta{
@@ -55,9 +57,10 @@ var storageClasses = []*storage.StorageClass{
 		ObjectMeta: metav1.ObjectMeta{
 			Name: "silver",
 		},
-		Provisioner:   mockPluginName,
-		Parameters:    class2Parameters,
-		ReclaimPolicy: &deleteReclaimPolicy,
+		Provisioner:       mockPluginName,
+		Parameters:        class2Parameters,
+		ReclaimPolicy:     &deleteReclaimPolicy,
+		VolumeBindingMode: &modeImmediate,
 	},
 	{
 		TypeMeta: metav1.TypeMeta{
@@ -66,9 +69,10 @@ var storageClasses = []*storage.StorageClass{
 		ObjectMeta: metav1.ObjectMeta{
 			Name: "external",
 		},
-		Provisioner:   "vendor.com/my-volume",
-		Parameters:    class1Parameters,
-		ReclaimPolicy: &deleteReclaimPolicy,
+		Provisioner:       "vendor.com/my-volume",
+		Parameters:        class1Parameters,
+		ReclaimPolicy:     &deleteReclaimPolicy,
+		VolumeBindingMode: &modeImmediate,
 	},
 	{
 		TypeMeta: metav1.TypeMeta{
@@ -77,9 +81,10 @@ var storageClasses = []*storage.StorageClass{
 		ObjectMeta: metav1.ObjectMeta{
 			Name: "unknown-internal",
 		},
-		Provisioner:   "kubernetes.io/unknown",
-		Parameters:    class1Parameters,
-		ReclaimPolicy: &deleteReclaimPolicy,
+		Provisioner:       "kubernetes.io/unknown",
+		Parameters:        class1Parameters,
+		ReclaimPolicy:     &deleteReclaimPolicy,
+		VolumeBindingMode: &modeImmediate,
 	},
 	{
 		TypeMeta: metav1.TypeMeta{
@@ -88,10 +93,11 @@ var storageClasses = []*storage.StorageClass{
 		ObjectMeta: metav1.ObjectMeta{
 			Name: "unsupported-mountoptions",
 		},
-		Provisioner:   mockPluginName,
-		Parameters:    class1Parameters,
-		ReclaimPolicy: &deleteReclaimPolicy,
-		MountOptions:  []string{"foo"},
+		Provisioner:       mockPluginName,
+		Parameters:        class1Parameters,
+		ReclaimPolicy:     &deleteReclaimPolicy,
+		MountOptions:      []string{"foo"},
+		VolumeBindingMode: &modeImmediate,
 	},
 }
 

pkg/controller/volume/persistentvolume/pv_controller_test.go

@@ -20,18 +20,26 @@ import (
 	"testing"
 	"time"
 
-	"k8s.io/klog"
-
 	"k8s.io/api/core/v1"
 	storagev1 "k8s.io/api/storage/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/watch"
 	utilfeature "k8s.io/apiserver/pkg/util/feature"
+	utilfeaturetesting "k8s.io/apiserver/pkg/util/feature/testing"
 	"k8s.io/client-go/informers"
 	"k8s.io/client-go/kubernetes/fake"
 	core "k8s.io/client-go/testing"
 	"k8s.io/client-go/tools/cache"
+	"k8s.io/klog"
 	"k8s.io/kubernetes/pkg/controller"
+	"k8s.io/kubernetes/pkg/features"
+)
+
+var (
+	classNotHere       = "not-here"
+	classNoMode        = "no-mode"
+	classImmediateMode = "immediate-mode"
+	classWaitMode      = "wait-mode"
 )
 
 // Test the real controller methods (add/update/delete claim/volume) with
@@ -264,16 +272,6 @@ func makeStorageClass(scName string, mode *storagev1.VolumeBindingMode) *storage
 }
 
 func TestDelayBinding(t *testing.T) {
-	var (
-		classNotHere       = "not-here"
-		classNoMode        = "no-mode"
-		classImmediateMode = "immediate-mode"
-		classWaitMode      = "wait-mode"
-
-		modeImmediate = storagev1.VolumeBindingImmediate
-		modeWait      = storagev1.VolumeBindingWaitForFirstConsumer
-	)
-
 	tests := map[string]struct {
 		pvc         *v1.PersistentVolumeClaim
 		shouldDelay bool
@@ -325,22 +323,8 @@ func TestDelayBinding(t *testing.T) {
 		}
 	}
 
-	// When volumeScheduling feature gate is disabled, should always be delayed
-	name := "volumeScheduling-feature-disabled"
-	shouldDelay, err := ctrl.shouldDelayBinding(makePVCClass(&classWaitMode, false))
-	if err != nil {
-		t.Errorf("Test %q returned error: %v", name, err)
-	}
-	if shouldDelay {
-		t.Errorf("Test %q returned true, expected false", name)
-	}
-
-	// Enable volumeScheduling feature gate
-	utilfeature.DefaultFeatureGate.Set("VolumeScheduling=true")
-	defer utilfeature.DefaultFeatureGate.Set("VolumeScheduling=false")
-
 	for name, test := range tests {
-		shouldDelay, err = ctrl.shouldDelayBinding(test.pvc)
+		shouldDelay, err := ctrl.shouldDelayBinding(test.pvc)
 		if err != nil && !test.shouldFail {
 			t.Errorf("Test %q returned error: %v", name, err)
 		}
@@ -352,3 +336,24 @@ func TestDelayBinding(t *testing.T) {
 		}
 	}
 }
+
+func TestDelayBindingDisabled(t *testing.T) {
+	// When volumeScheduling feature gate is disabled, should always be immediate
+	defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.VolumeScheduling, false)()
+
+	client := &fake.Clientset{}
+	informerFactory := informers.NewSharedInformerFactory(client, controller.NoResyncPeriodFunc())
+	classInformer := informerFactory.Storage().V1().StorageClasses()
+	ctrl := &PersistentVolumeController{
+		classLister: classInformer.Lister(),
+	}
+
+	name := "volumeScheduling-feature-disabled"
+	shouldDelay, err := ctrl.shouldDelayBinding(makePVCClass(&classWaitMode, false))
+	if err != nil {
+		t.Errorf("Test %q returned error: %v", name, err)
+	}
+	if shouldDelay {
+		t.Errorf("Test %q returned true, expected false", name)
+	}
+}

pkg/controller/volume/persistentvolume/scheduler_binder_test.go

@@ -30,7 +30,6 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/diff"
-	utilfeature "k8s.io/apiserver/pkg/util/feature"
 	"k8s.io/client-go/informers"
 	clientset "k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/kubernetes/fake"
@@ -728,10 +727,6 @@ func TestFindPodVolumesWithoutProvisioning(t *testing.T) {
 		},
 	}
 
-	// Set feature gate
-	utilfeature.DefaultFeatureGate.Set("VolumeScheduling=true")
-	defer utilfeature.DefaultFeatureGate.Set("VolumeScheduling=false")
-
 	testNode := &v1.Node{
 		ObjectMeta: metav1.ObjectMeta{
 			Name: "node1",
@@ -844,10 +839,6 @@ func TestFindPodVolumesWithProvisioning(t *testing.T) {
 		},
 	}
 
-	// Set VolumeScheduling feature gate
-	utilfeature.DefaultFeatureGate.Set("VolumeScheduling=true")
-	defer utilfeature.DefaultFeatureGate.Set("VolumeScheduling=false")
-
 	testNode := &v1.Node{
 		ObjectMeta: metav1.ObjectMeta{
 			Name: "node1",
@@ -1426,10 +1417,6 @@ func TestBindPodVolumes(t *testing.T) {
 }
 
 func TestFindAssumeVolumes(t *testing.T) {
-	// Set feature gate
-	utilfeature.DefaultFeatureGate.Set("VolumeScheduling=true")
-	defer utilfeature.DefaultFeatureGate.Set("VolumeScheduling=false")
-
 	// Test case
 	podPVCs := []*v1.PersistentVolumeClaim{unboundPVC}
 	pvs := []*v1.PersistentVolume{pvNode2, pvNode1a, pvNode1c}