From fd64c082400b31d93c6f9676431e1f03ec4f2448 Mon Sep 17 00:00:00 2001 From: Michelle Au Date: Thu, 15 Nov 2018 18:57:14 -0800 Subject: [PATCH] Fix storage feature gate test setting --- pkg/api/persistentvolumeclaim/BUILD | 1 + pkg/api/persistentvolumeclaim/util_test.go | 25 ++---- pkg/api/pod/BUILD | 1 + pkg/api/pod/util_test.go | 35 +++----- pkg/apis/core/validation/validation_test.go | 69 ++++++-------- pkg/apis/storage/v1/BUILD | 2 + pkg/apis/storage/v1/defaults_test.go | 30 +++---- pkg/apis/storage/v1beta1/BUILD | 2 + pkg/apis/storage/v1beta1/defaults_test.go | 31 +++---- pkg/apis/storage/validation/BUILD | 2 + .../storage/validation/validation_test.go | 18 ++-- pkg/controller/volume/expand/BUILD | 5 +- pkg/controller/volume/persistentvolume/BUILD | 2 + .../volume/persistentvolume/binder_test.go | 3 - .../volume/persistentvolume/index_test.go | 64 ++++++------- .../volume/persistentvolume/provision_test.go | 38 ++++---- .../persistentvolume/pv_controller_test.go | 59 ++++++------ .../persistentvolume/scheduler_binder_test.go | 13 --- pkg/kubelet/volumemanager/populator/BUILD | 1 + .../desired_state_of_world_populator_test.go | 34 ++----- pkg/kubelet/volumemanager/reconciler/BUILD | 1 + .../reconciler/reconciler_test.go | 43 ++++----- pkg/volume/util/BUILD | 2 + pkg/volume/util/util_test.go | 90 +++++++++---------- plugin/pkg/admission/resourcequota/BUILD | 2 + .../admission/resourcequota/admission_test.go | 26 ++---- 26 files changed, 256 insertions(+), 343 deletions(-) diff --git a/pkg/api/persistentvolumeclaim/BUILD b/pkg/api/persistentvolumeclaim/BUILD index 1ff7b1dc95..3a72b78c8f 100644 --- a/pkg/api/persistentvolumeclaim/BUILD +++ b/pkg/api/persistentvolumeclaim/BUILD @@ -38,5 +38,6 @@ go_test( "//pkg/apis/core:go_default_library", "//pkg/features: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", ], ) diff --git a/pkg/api/persistentvolumeclaim/util_test.go b/pkg/api/persistentvolumeclaim/util_test.go index e12acb3a9e..7bb61edd1d 100644 --- a/pkg/api/persistentvolumeclaim/util_test.go +++ b/pkg/api/persistentvolumeclaim/util_test.go @@ -20,6 +20,7 @@ import ( "testing" utilfeature "k8s.io/apiserver/pkg/util/feature" + utilfeaturetesting "k8s.io/apiserver/pkg/util/feature/testing" "k8s.io/kubernetes/pkg/apis/core" "k8s.io/kubernetes/pkg/features" ) @@ -36,36 +37,24 @@ func TestDropAlphaPVCVolumeMode(t *testing.T) { } // Enable alpha feature BlockVolume - err1 := utilfeature.DefaultFeatureGate.Set("BlockVolume=true") - if err1 != nil { - t.Fatalf("Failed to enable feature gate for BlockVolume: %v", err1) - } - + defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.BlockVolume, true)() // now test dropping the fields - should not be dropped DropDisabledAlphaFields(&pvc.Spec) // check to make sure VolumeDevices is still present // if featureset is set to true - if utilfeature.DefaultFeatureGate.Enabled(features.BlockVolume) { - if pvc.Spec.VolumeMode == nil { - t.Error("VolumeMode in pvc.Spec should not have been dropped based on feature-gate") - } + if pvc.Spec.VolumeMode == nil { + t.Error("VolumeMode in pvc.Spec should not have been dropped based on feature-gate") } // Disable alpha feature BlockVolume - err := utilfeature.DefaultFeatureGate.Set("BlockVolume=false") - if err != nil { - t.Fatalf("Failed to disable feature gate for BlockVolume: %v", err) - } - + defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.BlockVolume, false)() // now test dropping the fields DropDisabledAlphaFields(&pvc.Spec) // check to make sure VolumeDevices is nil // if featureset is set to false - if !utilfeature.DefaultFeatureGate.Enabled(features.BlockVolume) { - if pvc.Spec.VolumeMode != nil { - t.Error("DropDisabledAlphaFields VolumeMode for pvc.Spec failed") - } + if pvc.Spec.VolumeMode != nil { + t.Error("DropDisabledAlphaFields VolumeMode for pvc.Spec failed") } } diff --git a/pkg/api/pod/BUILD b/pkg/api/pod/BUILD index 77b08bc48b..2da65ee0c5 100644 --- a/pkg/api/pod/BUILD +++ b/pkg/api/pod/BUILD @@ -41,5 +41,6 @@ go_test( "//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/validation/field: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", ], ) diff --git a/pkg/api/pod/util_test.go b/pkg/api/pod/util_test.go index e27ac9c4e7..afbaa8fd69 100644 --- a/pkg/api/pod/util_test.go +++ b/pkg/api/pod/util_test.go @@ -24,6 +24,7 @@ import ( "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/validation/field" utilfeature "k8s.io/apiserver/pkg/util/feature" + utilfeaturetesting "k8s.io/apiserver/pkg/util/feature/testing" api "k8s.io/kubernetes/pkg/apis/core" "k8s.io/kubernetes/pkg/features" ) @@ -303,42 +304,32 @@ func TestDropAlphaVolumeDevices(t *testing.T) { } // Enable alpha feature BlockVolume - err1 := utilfeature.DefaultFeatureGate.Set("BlockVolume=true") - if err1 != nil { - t.Fatalf("Failed to enable feature gate for BlockVolume: %v", err1) - } + defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.BlockVolume, true)() // now test dropping the fields - should not be dropped DropDisabledAlphaFields(&testPod.Spec) // check to make sure VolumeDevices is still present // if featureset is set to true - if utilfeature.DefaultFeatureGate.Enabled(features.BlockVolume) { - if testPod.Spec.Containers[0].VolumeDevices == nil { - t.Error("VolumeDevices in Container should not have been dropped based on feature-gate") - } - if testPod.Spec.InitContainers[0].VolumeDevices == nil { - t.Error("VolumeDevices in InitContainers should not have been dropped based on feature-gate") - } + if testPod.Spec.Containers[0].VolumeDevices == nil { + t.Error("VolumeDevices in Container should not have been dropped based on feature-gate") + } + if testPod.Spec.InitContainers[0].VolumeDevices == nil { + t.Error("VolumeDevices in InitContainers should not have been dropped based on feature-gate") } // Disable alpha feature BlockVolume - err := utilfeature.DefaultFeatureGate.Set("BlockVolume=false") - if err != nil { - t.Fatalf("Failed to disable feature gate for BlockVolume: %v", err) - } + defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.BlockVolume, false)() // now test dropping the fields DropDisabledAlphaFields(&testPod.Spec) // check to make sure VolumeDevices is nil // if featureset is set to false - if !utilfeature.DefaultFeatureGate.Enabled(features.BlockVolume) { - if testPod.Spec.Containers[0].VolumeDevices != nil { - t.Error("DropDisabledAlphaFields for Containers failed") - } - if testPod.Spec.InitContainers[0].VolumeDevices != nil { - t.Error("DropDisabledAlphaFields for InitContainers failed") - } + if testPod.Spec.Containers[0].VolumeDevices != nil { + t.Error("DropDisabledAlphaFields for Containers failed") + } + if testPod.Spec.InitContainers[0].VolumeDevices != nil { + t.Error("DropDisabledAlphaFields for InitContainers failed") } } diff --git a/pkg/apis/core/validation/validation_test.go b/pkg/apis/core/validation/validation_test.go index c4c44e9719..d4f228935c 100644 --- a/pkg/apis/core/validation/validation_test.go +++ b/pkg/apis/core/validation/validation_test.go @@ -1553,18 +1553,21 @@ func TestValidatePersistentVolumeClaimUpdate(t *testing.T) { } for name, scenario := range scenarios { - // ensure we have a resource version specified for updates - togglePVExpandFeature(scenario.enableResize, t) - toggleBlockVolumeFeature(scenario.enableBlock, t) - scenario.oldClaim.ResourceVersion = "1" - scenario.newClaim.ResourceVersion = "1" - errs := ValidatePersistentVolumeClaimUpdate(scenario.newClaim, scenario.oldClaim) - if len(errs) == 0 && scenario.isExpectedFailure { - t.Errorf("Unexpected success for scenario: %s", name) - } - if len(errs) > 0 && !scenario.isExpectedFailure { - t.Errorf("Unexpected failure for scenario: %s - %+v", name, errs) - } + t.Run(name, func(t *testing.T) { + // ensure we have a resource version specified for updates + defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.ExpandPersistentVolumes, scenario.enableResize)() + + toggleBlockVolumeFeature(scenario.enableBlock, t) + scenario.oldClaim.ResourceVersion = "1" + scenario.newClaim.ResourceVersion = "1" + errs := ValidatePersistentVolumeClaimUpdate(scenario.newClaim, scenario.oldClaim) + if len(errs) == 0 && scenario.isExpectedFailure { + t.Errorf("Unexpected success for scenario: %s", name) + } + if len(errs) > 0 && !scenario.isExpectedFailure { + t.Errorf("Unexpected failure for scenario: %s - %+v", name, errs) + } + }) } } @@ -1585,23 +1588,6 @@ func toggleBlockVolumeFeature(toggleFlag bool, t *testing.T) { } } -func togglePVExpandFeature(toggleFlag bool, t *testing.T) { - if toggleFlag { - // Enable alpha feature LocalStorageCapacityIsolation - err := utilfeature.DefaultFeatureGate.Set("ExpandPersistentVolumes=true") - if err != nil { - t.Errorf("Failed to enable feature gate for ExpandPersistentVolumes: %v", err) - return - } - } else { - err := utilfeature.DefaultFeatureGate.Set("ExpandPersistentVolumes=false") - if err != nil { - t.Errorf("Failed to disable feature gate for ExpandPersistentVolumes: %v", err) - return - } - } -} - func TestValidateKeyToPath(t *testing.T) { testCases := []struct { kp core.KeyToPath @@ -11577,17 +11563,20 @@ func TestValidatePersistentVolumeClaimStatusUpdate(t *testing.T) { }, } for name, scenario := range scenarios { - // ensure we have a resource version specified for updates - togglePVExpandFeature(scenario.enableResize, t) - scenario.oldClaim.ResourceVersion = "1" - scenario.newClaim.ResourceVersion = "1" - errs := ValidatePersistentVolumeClaimStatusUpdate(scenario.newClaim, scenario.oldClaim) - if len(errs) == 0 && scenario.isExpectedFailure { - t.Errorf("Unexpected success for scenario: %s", name) - } - if len(errs) > 0 && !scenario.isExpectedFailure { - t.Errorf("Unexpected failure for scenario: %s - %+v", name, errs) - } + t.Run(name, func(t *testing.T) { + // ensure we have a resource version specified for updates + defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.ExpandPersistentVolumes, scenario.enableResize)() + + scenario.oldClaim.ResourceVersion = "1" + scenario.newClaim.ResourceVersion = "1" + errs := ValidatePersistentVolumeClaimStatusUpdate(scenario.newClaim, scenario.oldClaim) + if len(errs) == 0 && scenario.isExpectedFailure { + t.Errorf("Unexpected success for scenario: %s", name) + } + if len(errs) > 0 && !scenario.isExpectedFailure { + t.Errorf("Unexpected failure for scenario: %s - %+v", name, errs) + } + }) } } diff --git a/pkg/apis/storage/v1/BUILD b/pkg/apis/storage/v1/BUILD index f84557ef2c..05bd5565a7 100644 --- a/pkg/apis/storage/v1/BUILD +++ b/pkg/apis/storage/v1/BUILD @@ -52,8 +52,10 @@ go_test( deps = [ "//pkg/api/legacyscheme:go_default_library", "//pkg/apis/storage/install:go_default_library", + "//pkg/features:go_default_library", "//staging/src/k8s.io/api/storage/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/runtime: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", ], ) diff --git a/pkg/apis/storage/v1/defaults_test.go b/pkg/apis/storage/v1/defaults_test.go index 4143ad75e2..82ea60f0bd 100644 --- a/pkg/apis/storage/v1/defaults_test.go +++ b/pkg/apis/storage/v1/defaults_test.go @@ -23,8 +23,10 @@ import ( storagev1 "k8s.io/api/storage/v1" "k8s.io/apimachinery/pkg/runtime" utilfeature "k8s.io/apiserver/pkg/util/feature" + utilfeaturetesting "k8s.io/apiserver/pkg/util/feature/testing" "k8s.io/kubernetes/pkg/api/legacyscheme" _ "k8s.io/kubernetes/pkg/apis/storage/install" + "k8s.io/kubernetes/pkg/features" ) func roundTrip(t *testing.T, obj runtime.Object) runtime.Object { @@ -51,25 +53,9 @@ func roundTrip(t *testing.T, obj runtime.Object) runtime.Object { func TestSetDefaultVolumeBindingMode(t *testing.T) { class := &storagev1.StorageClass{} - // When feature gate is disabled, field should not be defaulted - err := utilfeature.DefaultFeatureGate.Set("VolumeScheduling=false") - if err != nil { - t.Fatalf("Failed to enable feature gate for VolumeScheduling: %v", err) - } - output := roundTrip(t, runtime.Object(class)).(*storagev1.StorageClass) - if output.VolumeBindingMode != nil { - t.Errorf("Expected VolumeBindingMode to not be defaulted, got: %+v", output.VolumeBindingMode) - } - - class = &storagev1.StorageClass{} - // When feature gate is enabled, field should be defaulted - err = utilfeature.DefaultFeatureGate.Set("VolumeScheduling=true") - if err != nil { - t.Fatalf("Failed to enable feature gate for VolumeScheduling: %v", err) - } defaultMode := storagev1.VolumeBindingImmediate - output = roundTrip(t, runtime.Object(class)).(*storagev1.StorageClass) + output := roundTrip(t, runtime.Object(class)).(*storagev1.StorageClass) outMode := output.VolumeBindingMode if outMode == nil { t.Errorf("Expected VolumeBindingMode to be defaulted to: %+v, got: nil", defaultMode) @@ -77,8 +63,12 @@ func TestSetDefaultVolumeBindingMode(t *testing.T) { t.Errorf("Expected VolumeBindingMode to be defaulted to: %+v, got: %+v", defaultMode, outMode) } - err = utilfeature.DefaultFeatureGate.Set("VolumeScheduling=false") - if err != nil { - t.Fatalf("Failed to disable feature gate for VolumeScheduling: %v", err) + class = &storagev1.StorageClass{} + + // When feature gate is disabled, field should not be defaulted + defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.VolumeScheduling, false)() + output = roundTrip(t, runtime.Object(class)).(*storagev1.StorageClass) + if output.VolumeBindingMode != nil { + t.Errorf("Expected VolumeBindingMode to not be defaulted, got: %+v", output.VolumeBindingMode) } } diff --git a/pkg/apis/storage/v1beta1/BUILD b/pkg/apis/storage/v1beta1/BUILD index 3006ca9307..7be3387015 100644 --- a/pkg/apis/storage/v1beta1/BUILD +++ b/pkg/apis/storage/v1beta1/BUILD @@ -52,8 +52,10 @@ go_test( deps = [ "//pkg/api/legacyscheme:go_default_library", "//pkg/apis/storage/install:go_default_library", + "//pkg/features:go_default_library", "//staging/src/k8s.io/api/storage/v1beta1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/runtime: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", ], ) diff --git a/pkg/apis/storage/v1beta1/defaults_test.go b/pkg/apis/storage/v1beta1/defaults_test.go index e846c7d573..2e1086dad8 100644 --- a/pkg/apis/storage/v1beta1/defaults_test.go +++ b/pkg/apis/storage/v1beta1/defaults_test.go @@ -23,8 +23,10 @@ import ( storagev1beta1 "k8s.io/api/storage/v1beta1" "k8s.io/apimachinery/pkg/runtime" utilfeature "k8s.io/apiserver/pkg/util/feature" + utilfeaturetesting "k8s.io/apiserver/pkg/util/feature/testing" "k8s.io/kubernetes/pkg/api/legacyscheme" _ "k8s.io/kubernetes/pkg/apis/storage/install" + "k8s.io/kubernetes/pkg/features" ) func roundTrip(t *testing.T, obj runtime.Object) runtime.Object { @@ -51,25 +53,9 @@ func roundTrip(t *testing.T, obj runtime.Object) runtime.Object { func TestSetDefaultVolumeBindingMode(t *testing.T) { class := &storagev1beta1.StorageClass{} - // When feature gate is disabled, field should not be defaulted - err := utilfeature.DefaultFeatureGate.Set("VolumeScheduling=false") - if err != nil { - t.Fatalf("Failed to enable feature gate for VolumeScheduling: %v", err) - } - output := roundTrip(t, runtime.Object(class)).(*storagev1beta1.StorageClass) - if output.VolumeBindingMode != nil { - t.Errorf("Expected VolumeBindingMode to not be defaulted, got: %+v", output.VolumeBindingMode) - } - - class = &storagev1beta1.StorageClass{} - // When feature gate is enabled, field should be defaulted - err = utilfeature.DefaultFeatureGate.Set("VolumeScheduling=true") - if err != nil { - t.Fatalf("Failed to enable feature gate for VolumeScheduling: %v", err) - } defaultMode := storagev1beta1.VolumeBindingImmediate - output = roundTrip(t, runtime.Object(class)).(*storagev1beta1.StorageClass) + output := roundTrip(t, runtime.Object(class)).(*storagev1beta1.StorageClass) outMode := output.VolumeBindingMode if outMode == nil { t.Errorf("Expected VolumeBindingMode to be defaulted to: %+v, got: nil", defaultMode) @@ -77,8 +63,13 @@ func TestSetDefaultVolumeBindingMode(t *testing.T) { t.Errorf("Expected VolumeBindingMode to be defaulted to: %+v, got: %+v", defaultMode, outMode) } - err = utilfeature.DefaultFeatureGate.Set("VolumeScheduling=false") - if err != nil { - t.Fatalf("Failed to disable feature gate for VolumeScheduling: %v", err) + class = &storagev1beta1.StorageClass{} + + // When feature gate is disabled, field should not be defaulted + defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.VolumeScheduling, false)() + output = roundTrip(t, runtime.Object(class)).(*storagev1beta1.StorageClass) + if output.VolumeBindingMode != nil { + t.Errorf("Expected VolumeBindingMode to not be defaulted, got: %+v", output.VolumeBindingMode) } + } diff --git a/pkg/apis/storage/validation/BUILD b/pkg/apis/storage/validation/BUILD index 90dd3fe682..a91f11d93e 100644 --- a/pkg/apis/storage/validation/BUILD +++ b/pkg/apis/storage/validation/BUILD @@ -31,8 +31,10 @@ go_test( deps = [ "//pkg/apis/core:go_default_library", "//pkg/apis/storage:go_default_library", + "//pkg/features:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1: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", ], ) diff --git a/pkg/apis/storage/validation/validation_test.go b/pkg/apis/storage/validation/validation_test.go index c79abf465e..905469735d 100644 --- a/pkg/apis/storage/validation/validation_test.go +++ b/pkg/apis/storage/validation/validation_test.go @@ -23,8 +23,10 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" utilfeature "k8s.io/apiserver/pkg/util/feature" + utilfeaturetesting "k8s.io/apiserver/pkg/util/feature/testing" api "k8s.io/kubernetes/pkg/apis/core" "k8s.io/kubernetes/pkg/apis/storage" + "k8s.io/kubernetes/pkg/features" ) var ( @@ -151,21 +153,13 @@ func TestAlphaExpandPersistentVolumesFeatureValidation(t *testing.T) { VolumeBindingMode: &immediateMode1, } - // Enable alpha feature ExpandPersistentVolumes - err := utilfeature.DefaultFeatureGate.Set("ExpandPersistentVolumes=true") - if err != nil { - t.Errorf("Failed to enable feature gate for ExpandPersistentVolumes: %v", err) - return - } + // Enable feature ExpandPersistentVolumes + defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.ExpandPersistentVolumes, true)() if errs := ValidateStorageClass(testSC); len(errs) != 0 { t.Errorf("expected success: %v", errs) } - // Disable alpha feature ExpandPersistentVolumes - err = utilfeature.DefaultFeatureGate.Set("ExpandPersistentVolumes=false") - if err != nil { - t.Errorf("Failed to disable feature gate for ExpandPersistentVolumes: %v", err) - return - } + // Disable feature ExpandPersistentVolumes + defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.ExpandPersistentVolumes, false)() if errs := ValidateStorageClass(testSC); len(errs) == 0 { t.Errorf("expected failure, but got no error") } diff --git a/pkg/controller/volume/expand/BUILD b/pkg/controller/volume/expand/BUILD index 5b20360dc3..c633196b32 100644 --- a/pkg/controller/volume/expand/BUILD +++ b/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", diff --git a/pkg/controller/volume/persistentvolume/BUILD b/pkg/controller/volume/persistentvolume/BUILD index 65fd161348..aac87f389d 100644 --- a/pkg/controller/volume/persistentvolume/BUILD +++ b/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", diff --git a/pkg/controller/volume/persistentvolume/binder_test.go b/pkg/controller/volume/persistentvolume/binder_test.go index c278c8c01d..e6f8e86757 100644 --- a/pkg/controller/volume/persistentvolume/binder_test.go +++ b/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}, diff --git a/pkg/controller/volume/persistentvolume/index_test.go b/pkg/controller/volume/persistentvolume/index_test.go index 19ef3fd4d1..87c4559604 100644 --- a/pkg/controller/volume/persistentvolume/index_test.go +++ b/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) { diff --git a/pkg/controller/volume/persistentvolume/provision_test.go b/pkg/controller/volume/persistentvolume/provision_test.go index 0337996e46..879e72dd50 100644 --- a/pkg/controller/volume/persistentvolume/provision_test.go +++ b/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, }, } diff --git a/pkg/controller/volume/persistentvolume/pv_controller_test.go b/pkg/controller/volume/persistentvolume/pv_controller_test.go index 5f1c5ad761..514617e5f1 100644 --- a/pkg/controller/volume/persistentvolume/pv_controller_test.go +++ b/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) + } +} diff --git a/pkg/controller/volume/persistentvolume/scheduler_binder_test.go b/pkg/controller/volume/persistentvolume/scheduler_binder_test.go index b5b9f14d7f..728f478c19 100644 --- a/pkg/controller/volume/persistentvolume/scheduler_binder_test.go +++ b/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} diff --git a/pkg/kubelet/volumemanager/populator/BUILD b/pkg/kubelet/volumemanager/populator/BUILD index 1e2b7d2dfd..f038b0d32e 100644 --- a/pkg/kubelet/volumemanager/populator/BUILD +++ b/pkg/kubelet/volumemanager/populator/BUILD @@ -67,6 +67,7 @@ go_test( "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/runtime: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/kubernetes/fake:go_default_library", "//staging/src/k8s.io/client-go/testing:go_default_library", ], diff --git a/pkg/kubelet/volumemanager/populator/desired_state_of_world_populator_test.go b/pkg/kubelet/volumemanager/populator/desired_state_of_world_populator_test.go index 969ac66911..c92aa30807 100644 --- a/pkg/kubelet/volumemanager/populator/desired_state_of_world_populator_test.go +++ b/pkg/kubelet/volumemanager/populator/desired_state_of_world_populator_test.go @@ -27,6 +27,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" utilfeature "k8s.io/apiserver/pkg/util/feature" + utilfeaturetesting "k8s.io/apiserver/pkg/util/feature/testing" "k8s.io/client-go/kubernetes/fake" core "k8s.io/client-go/testing" "k8s.io/kubernetes/pkg/features" @@ -151,7 +152,7 @@ func TestFindAndAddNewPods_FindAndRemoveDeletedPods(t *testing.T) { func TestFindAndAddNewPods_FindAndRemoveDeletedPods_Valid_Block_VolumeDevices(t *testing.T) { // Enable BlockVolume feature gate - utilfeature.DefaultFeatureGate.Set("BlockVolume=true") + defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.BlockVolume, true)() // create dswp mode := v1.PersistentVolumeBlock @@ -256,9 +257,6 @@ func TestFindAndAddNewPods_FindAndRemoveDeletedPods_Valid_Block_VolumeDevices(t expectedVolumeName) } } - - // Rollback feature gate to false. - utilfeature.DefaultFeatureGate.Set("BlockVolume=false") } func TestCreateVolumeSpec_Valid_File_VolumeMounts(t *testing.T) { @@ -309,7 +307,7 @@ func TestCreateVolumeSpec_Valid_File_VolumeMounts(t *testing.T) { func TestCreateVolumeSpec_Valid_Block_VolumeDevices(t *testing.T) { // Enable BlockVolume feature gate - utilfeature.DefaultFeatureGate.Set("BlockVolume=true") + defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.BlockVolume, true)() // create dswp mode := v1.PersistentVolumeBlock @@ -354,14 +352,11 @@ func TestCreateVolumeSpec_Valid_Block_VolumeDevices(t *testing.T) { if volumeSpec == nil || err != nil { t.Fatalf("Failed to create volumeSpec with combination of block mode and volumeDevices. err: %v", err) } - - // Rollback feature gate to false. - utilfeature.DefaultFeatureGate.Set("BlockVolume=false") } func TestCreateVolumeSpec_Invalid_File_VolumeDevices(t *testing.T) { // Enable BlockVolume feature gate - utilfeature.DefaultFeatureGate.Set("BlockVolume=true") + defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.BlockVolume, true)() // create dswp mode := v1.PersistentVolumeFilesystem @@ -406,14 +401,11 @@ func TestCreateVolumeSpec_Invalid_File_VolumeDevices(t *testing.T) { if volumeSpec != nil || err == nil { t.Fatalf("Unexpected volumeMode and volumeMounts/volumeDevices combination is accepted") } - - // Rollback feature gate to false. - utilfeature.DefaultFeatureGate.Set("BlockVolume=false") } func TestCreateVolumeSpec_Invalid_Block_VolumeMounts(t *testing.T) { // Enable BlockVolume feature gate - utilfeature.DefaultFeatureGate.Set("BlockVolume=true") + defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.BlockVolume, true)() // create dswp mode := v1.PersistentVolumeBlock @@ -458,9 +450,6 @@ func TestCreateVolumeSpec_Invalid_Block_VolumeMounts(t *testing.T) { if volumeSpec != nil || err == nil { t.Fatalf("Unexpected volumeMode and volumeMounts/volumeDevices combination is accepted") } - - // Rollback feature gate to false. - utilfeature.DefaultFeatureGate.Set("BlockVolume=false") } func TestCheckVolumeFSResize(t *testing.T) { @@ -510,7 +499,7 @@ func TestCheckVolumeFSResize(t *testing.T) { reconcileASW(fakeASW, fakeDSW, t) // No resize request for volume, volumes in ASW shouldn't be marked as fsResizeRequired. - setExpandOnlinePersistentVolumesFeatureGate("true", t) + defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.ExpandInUsePersistentVolumes, true)() resizeRequiredVolumes := reprocess(dswp, uniquePodName, fakeDSW, fakeASW) if len(resizeRequiredVolumes) > 0 { t.Fatalf("No resize request for any volumes, but found resize required volumes in ASW: %v", resizeRequiredVolumes) @@ -521,14 +510,14 @@ func TestCheckVolumeFSResize(t *testing.T) { pvc.Spec.Resources.Requests = volumeCapacity(2) // Disable the feature gate, so volume shouldn't be marked as fsResizeRequired. - setExpandOnlinePersistentVolumesFeatureGate("false", t) + defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.ExpandInUsePersistentVolumes, false)() resizeRequiredVolumes = reprocess(dswp, uniquePodName, fakeDSW, fakeASW) if len(resizeRequiredVolumes) > 0 { t.Fatalf("Feature gate disabled, but found resize required volumes in ASW: %v", resizeRequiredVolumes) } // Make volume used as ReadOnly, so volume shouldn't be marked as fsResizeRequired. - setExpandOnlinePersistentVolumesFeatureGate("true", t) + defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.ExpandInUsePersistentVolumes, true)() pod.Spec.Containers[0].VolumeMounts[0].ReadOnly = true resizeRequiredVolumes = reprocess(dswp, uniquePodName, fakeDSW, fakeASW) if len(resizeRequiredVolumes) > 0 { @@ -561,13 +550,6 @@ func volumeCapacity(size int) v1.ResourceList { return v1.ResourceList{v1.ResourceStorage: resource.MustParse(fmt.Sprintf("%dGi", size))} } -func setExpandOnlinePersistentVolumesFeatureGate(value string, t *testing.T) { - err := utilfeature.DefaultFeatureGate.Set(fmt.Sprintf("%s=%s", features.ExpandInUsePersistentVolumes, value)) - if err != nil { - t.Fatalf("Set ExpandInUsePersistentVolumes feature gate to %s failed: %v", value, err) - } -} - func reconcileASW(asw cache.ActualStateOfWorld, dsw cache.DesiredStateOfWorld, t *testing.T) { for _, volumeToMount := range dsw.GetVolumesToMount() { err := asw.MarkVolumeAsAttached(volumeToMount.VolumeName, volumeToMount.VolumeSpec, "", "") diff --git a/pkg/kubelet/volumemanager/reconciler/BUILD b/pkg/kubelet/volumemanager/reconciler/BUILD index 2a264aa4ee..e230dab455 100644 --- a/pkg/kubelet/volumemanager/reconciler/BUILD +++ b/pkg/kubelet/volumemanager/reconciler/BUILD @@ -52,6 +52,7 @@ go_test( "//staging/src/k8s.io/apimachinery/pkg/types:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/wait: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/kubernetes/fake:go_default_library", "//staging/src/k8s.io/client-go/testing:go_default_library", "//staging/src/k8s.io/client-go/tools/record:go_default_library", diff --git a/pkg/kubelet/volumemanager/reconciler/reconciler_test.go b/pkg/kubelet/volumemanager/reconciler/reconciler_test.go index 38eaba2f03..765ad9277d 100644 --- a/pkg/kubelet/volumemanager/reconciler/reconciler_test.go +++ b/pkg/kubelet/volumemanager/reconciler/reconciler_test.go @@ -29,6 +29,7 @@ import ( k8stypes "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/wait" utilfeature "k8s.io/apiserver/pkg/util/feature" + utilfeaturetesting "k8s.io/apiserver/pkg/util/feature/testing" "k8s.io/client-go/kubernetes/fake" core "k8s.io/client-go/testing" "k8s.io/client-go/tools/record" @@ -439,7 +440,8 @@ func Test_Run_Positive_VolumeUnmountControllerAttachEnabled(t *testing.T) { // no detach/teardownDevice calls. func Test_Run_Positive_VolumeAttachAndMap(t *testing.T) { // Enable BlockVolume feature gate - utilfeature.DefaultFeatureGate.Set("BlockVolume=true") + defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.BlockVolume, true)() + // Arrange volumePluginMgr, fakePlugin := volumetesting.GetTestVolumePluginMgr(t) dsw := cache.NewDesiredStateOfWorld(volumePluginMgr) @@ -513,9 +515,6 @@ func Test_Run_Positive_VolumeAttachAndMap(t *testing.T) { 1 /* expectedGetMapDeviceCallCount */, fakePlugin)) assert.NoError(t, volumetesting.VerifyZeroTearDownDeviceCallCount(fakePlugin)) assert.NoError(t, volumetesting.VerifyZeroDetachCallCount(fakePlugin)) - - // Rollback feature gate to false. - utilfeature.DefaultFeatureGate.Set("BlockVolume=false") } // Populates desiredStateOfWorld cache with one volume/pod. @@ -526,7 +525,8 @@ func Test_Run_Positive_VolumeAttachAndMap(t *testing.T) { // Verifies there are no attach/detach calls. func Test_Run_Positive_BlockVolumeMapControllerAttachEnabled(t *testing.T) { // Enable BlockVolume feature gate - utilfeature.DefaultFeatureGate.Set("BlockVolume=true") + defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.BlockVolume, true)() + // Arrange volumePluginMgr, fakePlugin := volumetesting.GetTestVolumePluginMgr(t) dsw := cache.NewDesiredStateOfWorld(volumePluginMgr) @@ -601,9 +601,6 @@ func Test_Run_Positive_BlockVolumeMapControllerAttachEnabled(t *testing.T) { 1 /* expectedGetMapDeviceCallCount */, fakePlugin)) assert.NoError(t, volumetesting.VerifyZeroTearDownDeviceCallCount(fakePlugin)) assert.NoError(t, volumetesting.VerifyZeroDetachCallCount(fakePlugin)) - - // Rollback feature gate to false. - utilfeature.DefaultFeatureGate.Set("BlockVolume=false") } // Populates desiredStateOfWorld cache with one volume/pod. @@ -614,7 +611,8 @@ func Test_Run_Positive_BlockVolumeMapControllerAttachEnabled(t *testing.T) { // Verifies one detach/teardownDevice calls are issued. func Test_Run_Positive_BlockVolumeAttachMapUnmapDetach(t *testing.T) { // Enable BlockVolume feature gate - utilfeature.DefaultFeatureGate.Set("BlockVolume=true") + defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.BlockVolume, true)() + // Arrange volumePluginMgr, fakePlugin := volumetesting.GetTestVolumePluginMgr(t) dsw := cache.NewDesiredStateOfWorld(volumePluginMgr) @@ -698,9 +696,6 @@ func Test_Run_Positive_BlockVolumeAttachMapUnmapDetach(t *testing.T) { 1 /* expectedTearDownDeviceCallCount */, fakePlugin)) assert.NoError(t, volumetesting.VerifyDetachCallCount( 1 /* expectedDetachCallCount */, fakePlugin)) - - // Rollback feature gate to false. - utilfeature.DefaultFeatureGate.Set("BlockVolume=false") } // Populates desiredStateOfWorld cache with one volume/pod. @@ -712,7 +707,8 @@ func Test_Run_Positive_BlockVolumeAttachMapUnmapDetach(t *testing.T) { // Verifies there are no attach/detach calls made. func Test_Run_Positive_VolumeUnmapControllerAttachEnabled(t *testing.T) { // Enable BlockVolume feature gate - utilfeature.DefaultFeatureGate.Set("BlockVolume=true") + defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.BlockVolume, true)() + // Arrange volumePluginMgr, fakePlugin := volumetesting.GetTestVolumePluginMgr(t) dsw := cache.NewDesiredStateOfWorld(volumePluginMgr) @@ -797,9 +793,6 @@ func Test_Run_Positive_VolumeUnmapControllerAttachEnabled(t *testing.T) { assert.NoError(t, volumetesting.VerifyTearDownDeviceCallCount( 1 /* expectedTearDownDeviceCallCount */, fakePlugin)) assert.NoError(t, volumetesting.VerifyZeroDetachCallCount(fakePlugin)) - - // Rollback feature gate to false. - utilfeature.DefaultFeatureGate.Set("BlockVolume=false") } func Test_GenerateMapVolumeFunc_Plugin_Not_Found(t *testing.T) { @@ -821,7 +814,8 @@ func Test_GenerateMapVolumeFunc_Plugin_Not_Found(t *testing.T) { } // Enable BlockVolume feature gate - utilfeature.DefaultFeatureGate.Set("BlockVolume=true") + defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.BlockVolume, true)() + for name, tc := range testCases { t.Run(name, func(t *testing.T) { volumePluginMgr := &volume.VolumePluginMgr{} @@ -853,8 +847,6 @@ func Test_GenerateMapVolumeFunc_Plugin_Not_Found(t *testing.T) { } }) } - // Rollback feature gate to false. - utilfeature.DefaultFeatureGate.Set("BlockVolume=false") } func Test_GenerateUnmapVolumeFunc_Plugin_Not_Found(t *testing.T) { @@ -876,7 +868,8 @@ func Test_GenerateUnmapVolumeFunc_Plugin_Not_Found(t *testing.T) { } // Enable BlockVolume feature gate - utilfeature.DefaultFeatureGate.Set("BlockVolume=true") + defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.BlockVolume, true)() + for name, tc := range testCases { t.Run(name, func(t *testing.T) { volumePluginMgr := &volume.VolumePluginMgr{} @@ -900,8 +893,6 @@ func Test_GenerateUnmapVolumeFunc_Plugin_Not_Found(t *testing.T) { } }) } - // Rollback feature gate to false. - utilfeature.DefaultFeatureGate.Set("BlockVolume=false") } func Test_GenerateUnmapDeviceFunc_Plugin_Not_Found(t *testing.T) { @@ -923,7 +914,8 @@ func Test_GenerateUnmapDeviceFunc_Plugin_Not_Found(t *testing.T) { } // Enable BlockVolume feature gate - utilfeature.DefaultFeatureGate.Set("BlockVolume=true") + defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.BlockVolume, true)() + for name, tc := range testCases { t.Run(name, func(t *testing.T) { volumePluginMgr := &volume.VolumePluginMgr{} @@ -946,8 +938,6 @@ func Test_GenerateUnmapDeviceFunc_Plugin_Not_Found(t *testing.T) { } }) } - // Rollback feature gate to false. - utilfeature.DefaultFeatureGate.Set("BlockVolume=false") } // Populates desiredStateOfWorld cache with one volume/pod. @@ -957,7 +947,8 @@ func Test_GenerateUnmapDeviceFunc_Plugin_Not_Found(t *testing.T) { // Mark volume as fsResizeRequired in ASW. // Verifies volume's fsResizeRequired flag is cleared later. func Test_Run_Positive_VolumeFSResizeControllerAttachEnabled(t *testing.T) { - utilfeature.DefaultFeatureGate.Set(fmt.Sprintf("%s=true", features.ExpandInUsePersistentVolumes)) + defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.ExpandInUsePersistentVolumes, true)() + pv := &v1.PersistentVolume{ ObjectMeta: metav1.ObjectMeta{ Name: "pv", diff --git a/pkg/volume/util/BUILD b/pkg/volume/util/BUILD index c6ac0cf233..ffd1d03215 100644 --- a/pkg/volume/util/BUILD +++ b/pkg/volume/util/BUILD @@ -60,6 +60,7 @@ go_test( deps = [ "//pkg/apis/core/install:go_default_library", "//pkg/apis/core/v1/helper:go_default_library", + "//pkg/features:go_default_library", "//pkg/kubelet/apis:go_default_library", "//pkg/util/mount:go_default_library", "//pkg/util/slice:go_default_library", @@ -70,6 +71,7 @@ go_test( "//staging/src/k8s.io/apimachinery/pkg/types:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/sets: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/util/testing:go_default_library", ], ) diff --git a/pkg/volume/util/util_test.go b/pkg/volume/util/util_test.go index c5e4e8574f..8f376a3adb 100644 --- a/pkg/volume/util/util_test.go +++ b/pkg/volume/util/util_test.go @@ -24,12 +24,15 @@ import ( "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/util/sets" + utilfeaturetesting "k8s.io/apiserver/pkg/util/feature/testing" utiltesting "k8s.io/client-go/util/testing" + // util.go uses api.Codecs.LegacyCodec so import this package to do some // resource initialization. "hash/fnv" _ "k8s.io/kubernetes/pkg/apis/core/install" + "k8s.io/kubernetes/pkg/features" "k8s.io/kubernetes/pkg/util/mount" "reflect" @@ -1426,59 +1429,54 @@ func TestSelectZoneForVolume(t *testing.T) { } for _, test := range tests { - utilfeature.DefaultFeatureGate.Set("VolumeScheduling=false") - if test.VolumeScheduling { - utilfeature.DefaultFeatureGate.Set("VolumeScheduling=true") - } + t.Run(test.Name, func(t *testing.T) { + defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.VolumeScheduling, test.VolumeScheduling)() - var zonesParameter, zonesWithNodes sets.String - var err error + var zonesParameter, zonesWithNodes sets.String + var err error - if test.Zones != "" { - zonesParameter, err = ZonesToSet(test.Zones) - if err != nil { - t.Errorf("Could not convert Zones to a set: %s. This is a test error %s", test.Zones, test.Name) - continue - } - } - - if test.ZonesWithNodes != "" { - zonesWithNodes, err = ZonesToSet(test.ZonesWithNodes) - if err != nil { - t.Errorf("Could not convert specified ZonesWithNodes to a set: %s. This is a test error %s", test.ZonesWithNodes, test.Name) - continue - } - } - - zone, err := SelectZoneForVolume(test.ZonePresent, test.ZonesPresent, test.Zone, zonesParameter, zonesWithNodes, test.Node, test.AllowedTopologies, test.Name) - - if test.Reject && err == nil { - t.Errorf("Unexpected zone from SelectZoneForVolume for %s", zone) - continue - } - - if !test.Reject { - if err != nil { - t.Errorf("Unexpected error from SelectZoneForVolume for %s; Error: %v", test.Name, err) - continue - } - - if test.ExpectSpecificZone == true { - if zone != test.ExpectedZone { - t.Errorf("Expected zone %v does not match obtained zone %v for %s", test.ExpectedZone, zone, test.Name) + if test.Zones != "" { + zonesParameter, err = ZonesToSet(test.Zones) + if err != nil { + t.Fatalf("Could not convert Zones to a set: %s. This is a test error %s", test.Zones, test.Name) } - continue } - expectedZones, err := ZonesToSet(test.ExpectedZones) - if err != nil { - t.Errorf("Could not convert ExpectedZones to a set: %s. This is a test error", test.ExpectedZones) - continue + if test.ZonesWithNodes != "" { + zonesWithNodes, err = ZonesToSet(test.ZonesWithNodes) + if err != nil { + t.Fatalf("Could not convert specified ZonesWithNodes to a set: %s. This is a test error %s", test.ZonesWithNodes, test.Name) + } } - if !expectedZones.Has(zone) { - t.Errorf("Obtained zone %s not member of expectedZones %s", zone, expectedZones) + + zone, err := SelectZoneForVolume(test.ZonePresent, test.ZonesPresent, test.Zone, zonesParameter, zonesWithNodes, test.Node, test.AllowedTopologies, test.Name) + + if test.Reject && err == nil { + t.Errorf("Unexpected zone from SelectZoneForVolume for %s", zone) } - } + + if !test.Reject { + if err != nil { + t.Errorf("Unexpected error from SelectZoneForVolume for %s; Error: %v", test.Name, err) + } + + if test.ExpectSpecificZone == true { + if zone != test.ExpectedZone { + t.Errorf("Expected zone %v does not match obtained zone %v for %s", test.ExpectedZone, zone, test.Name) + } + } + + if test.ExpectedZones != "" { + expectedZones, err := ZonesToSet(test.ExpectedZones) + if err != nil { + t.Fatalf("Could not convert ExpectedZones to a set: %s. This is a test error", test.ExpectedZones) + } + if !expectedZones.Has(zone) { + t.Errorf("Obtained zone %s not member of expectedZones %s", zone, expectedZones) + } + } + } + }) } } diff --git a/plugin/pkg/admission/resourcequota/BUILD b/plugin/pkg/admission/resourcequota/BUILD index 0920987180..99c73f482f 100644 --- a/plugin/pkg/admission/resourcequota/BUILD +++ b/plugin/pkg/admission/resourcequota/BUILD @@ -56,6 +56,7 @@ go_test( deps = [ "//pkg/apis/core:go_default_library", "//pkg/controller:go_default_library", + "//pkg/features:go_default_library", "//pkg/quota/v1/generic:go_default_library", "//pkg/quota/v1/install:go_default_library", "//plugin/pkg/admission/resourcequota/apis/resourcequota:go_default_library", @@ -65,6 +66,7 @@ go_test( "//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library", "//staging/src/k8s.io/apiserver/pkg/admission: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/fake:go_default_library", "//staging/src/k8s.io/client-go/testing:go_default_library", diff --git a/plugin/pkg/admission/resourcequota/admission_test.go b/plugin/pkg/admission/resourcequota/admission_test.go index 09e29b0b8f..be7a8af6bb 100644 --- a/plugin/pkg/admission/resourcequota/admission_test.go +++ b/plugin/pkg/admission/resourcequota/admission_test.go @@ -31,12 +31,14 @@ import ( "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apiserver/pkg/admission" 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" testcore "k8s.io/client-go/testing" "k8s.io/client-go/tools/cache" api "k8s.io/kubernetes/pkg/apis/core" "k8s.io/kubernetes/pkg/controller" + "k8s.io/kubernetes/pkg/features" "k8s.io/kubernetes/pkg/quota/v1/generic" "k8s.io/kubernetes/pkg/quota/v1/install" resourcequotaapi "k8s.io/kubernetes/plugin/pkg/admission/resourcequota/apis/resourcequota" @@ -452,15 +454,7 @@ func TestAdmitHandlesNegativePVCUpdates(t *testing.T) { stopCh := make(chan struct{}) defer close(stopCh) - err := utilfeature.DefaultFeatureGate.Set("ExpandPersistentVolumes=true") - if err != nil { - t.Errorf("Failed to enable feature gate for LocalPersistentVolumes: %v", err) - return - } - - defer func() { - utilfeature.DefaultFeatureGate.Set("ExpandPersistentVolumes=false") - }() + defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.ExpandPersistentVolumes, true)() kubeClient := fake.NewSimpleClientset(resourceQuota) informerFactory := informers.NewSharedInformerFactory(kubeClient, controller.NoResyncPeriodFunc()) @@ -491,7 +485,7 @@ func TestAdmitHandlesNegativePVCUpdates(t *testing.T) { }, } - err = handler.Validate(admission.NewAttributesRecord(newPVC, oldPVC, api.Kind("PersistentVolumeClaim").WithVersion("version"), newPVC.Namespace, newPVC.Name, corev1.Resource("persistentvolumeclaims").WithVersion("version"), "", admission.Update, false, nil)) + err := handler.Validate(admission.NewAttributesRecord(newPVC, oldPVC, api.Kind("PersistentVolumeClaim").WithVersion("version"), newPVC.Namespace, newPVC.Name, corev1.Resource("persistentvolumeclaims").WithVersion("version"), "", admission.Update, false, nil)) if err != nil { t.Errorf("Unexpected error: %v", err) } @@ -515,15 +509,7 @@ func TestAdmitHandlesPVCUpdates(t *testing.T) { }, } - err := utilfeature.DefaultFeatureGate.Set("ExpandPersistentVolumes=true") - if err != nil { - t.Errorf("Failed to enable feature gate for LocalPersistentVolumes: %v", err) - return - } - - defer func() { - utilfeature.DefaultFeatureGate.Set("ExpandPersistentVolumes=false") - }() + defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.ExpandPersistentVolumes, true)() // start up quota system stopCh := make(chan struct{}) @@ -558,7 +544,7 @@ func TestAdmitHandlesPVCUpdates(t *testing.T) { }, } - err = handler.Validate(admission.NewAttributesRecord(newPVC, oldPVC, api.Kind("PersistentVolumeClaim").WithVersion("version"), newPVC.Namespace, newPVC.Name, corev1.Resource("persistentvolumeclaims").WithVersion("version"), "", admission.Update, false, nil)) + err := handler.Validate(admission.NewAttributesRecord(newPVC, oldPVC, api.Kind("PersistentVolumeClaim").WithVersion("version"), newPVC.Namespace, newPVC.Name, corev1.Resource("persistentvolumeclaims").WithVersion("version"), "", admission.Update, false, nil)) if err != nil { t.Errorf("Unexpected error: %v", err) }