Skip to content

Commit 358c092

Browse files
liggittliggitt
committed
fix storage unit tests leaking feature flag changes
1 parent fd64c08 commit 358c092

File tree

15 files changed

+149
-207
lines changed

15 files changed

+149
-207
lines changed

‎pkg/apis/core/validation/validation_test.go

+52-114
Original file line numberDiff line numberDiff line change
@@ -550,29 +550,31 @@ func TestValidateLocalVolumesDisabled(t *testing.T) {
550550
},
551551
}
552552

553-
utilfeature.DefaultFeatureGate.Set("PersistentLocalVolumes=false")
554553
for name, scenario := range scenarios {
555-
errs := ValidatePersistentVolume(scenario.volume)
556-
if len(errs) == 0 && scenario.isExpectedFailure {
557-
t.Errorf("Unexpected success for scenario: %s", name)
558-
}
559-
if len(errs) > 0 && !scenario.isExpectedFailure {
560-
t.Errorf("Unexpected failure for scenario: %s - %+v", name, errs)
561-
}
554+
t.Run(name+" PersistentLocalVolumes disabled", func(t *testing.T) {
555+
defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.PersistentLocalVolumes, false)()
556+
errs := ValidatePersistentVolume(scenario.volume)
557+
if len(errs) == 0 && scenario.isExpectedFailure {
558+
t.Errorf("Unexpected success for scenario: %s", name)
559+
}
560+
if len(errs) > 0 && !scenario.isExpectedFailure {
561+
t.Errorf("Unexpected failure for scenario: %s - %+v", name, errs)
562+
}
563+
})
562564
}
563-
utilfeature.DefaultFeatureGate.Set("PersistentLocalVolumes=true")
564565

565-
utilfeature.DefaultFeatureGate.Set("VolumeScheduling=false")
566566
for name, scenario := range scenarios {
567-
errs := ValidatePersistentVolume(scenario.volume)
568-
if len(errs) == 0 && scenario.isExpectedFailure {
569-
t.Errorf("Unexpected success for scenario: %s", name)
570-
}
571-
if len(errs) > 0 && !scenario.isExpectedFailure {
572-
t.Errorf("Unexpected failure for scenario: %s - %+v", name, errs)
573-
}
567+
t.Run(name+" VolumeScheduling disabled", func(t *testing.T) {
568+
defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.VolumeScheduling, false)()
569+
errs := ValidatePersistentVolume(scenario.volume)
570+
if len(errs) == 0 && scenario.isExpectedFailure {
571+
t.Errorf("Unexpected success for scenario: %s", name)
572+
}
573+
if len(errs) > 0 && !scenario.isExpectedFailure {
574+
t.Errorf("Unexpected failure for scenario: %s - %+v", name, errs)
575+
}
576+
})
574577
}
575-
utilfeature.DefaultFeatureGate.Set("VolumeScheduling=true")
576578
}
577579

578580
func testVolumeWithNodeAffinity(affinity *core.VolumeNodeAffinity) *core.PersistentVolume {
@@ -1911,11 +1913,7 @@ func TestValidateCSIVolumeSource(t *testing.T) {
19111913
},
19121914
}
19131915

1914-
err := utilfeature.DefaultFeatureGate.Set("CSIPersistentVolume=true")
1915-
if err != nil {
1916-
t.Errorf("Failed to enable feature gate for CSIPersistentVolumes: %v", err)
1917-
return
1918-
}
1916+
defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSIPersistentVolume, true)()
19191917

19201918
for i, tc := range testCases {
19211919
errs := validateCSIPersistentVolumeSource(tc.csi, field.NewPath("field"))
@@ -1932,11 +1930,6 @@ func TestValidateCSIVolumeSource(t *testing.T) {
19321930
}
19331931
}
19341932
}
1935-
err = utilfeature.DefaultFeatureGate.Set("CSIPersistentVolume=false")
1936-
if err != nil {
1937-
t.Errorf("Failed to disable feature gate for CSIPersistentVolumes: %v", err)
1938-
return
1939-
}
19401933
}
19411934

19421935
// This test is a little too top-to-bottom. Ideally we would test each volume
@@ -3750,20 +3743,14 @@ func TestValidateVolumes(t *testing.T) {
37503743
// Validate HugePages medium type for EmptyDir when HugePages feature is enabled/disabled
37513744
hugePagesCase := core.VolumeSource{EmptyDir: &core.EmptyDirVolumeSource{Medium: core.StorageMediumHugePages}}
37523745

3753-
// Enable alpha feature HugePages
3754-
err := utilfeature.DefaultFeatureGate.Set("HugePages=true")
3755-
if err != nil {
3756-
t.Errorf("Failed to enable feature gate for HugePages: %v", err)
3757-
}
3746+
// Enable HugePages
3747+
defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.HugePages, true)()
37583748
if errs := validateVolumeSource(&hugePagesCase, field.NewPath("field").Index(0), "working"); len(errs) != 0 {
37593749
t.Errorf("Unexpected error when HugePages feature is enabled.")
37603750
}
37613751

3762-
// Disable alpha feature HugePages
3763-
err = utilfeature.DefaultFeatureGate.Set("HugePages=false")
3764-
if err != nil {
3765-
t.Errorf("Failed to disable feature gate for HugePages: %v", err)
3766-
}
3752+
// Disable feature HugePages
3753+
defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.HugePages, false)()
37673754
if errs := validateVolumeSource(&hugePagesCase, field.NewPath("field").Index(0), "failing"); len(errs) == 0 {
37683755
t.Errorf("Expected error when HugePages feature is disabled got nothing.")
37693756
}
@@ -3868,12 +3855,8 @@ func TestAlphaHugePagesIsolation(t *testing.T) {
38683855
},
38693856
},
38703857
}
3871-
// Enable alpha feature HugePages
3872-
err := utilfeature.DefaultFeatureGate.Set("HugePages=true")
3873-
if err != nil {
3874-
t.Errorf("Failed to enable feature gate for HugePages: %v", err)
3875-
return
3876-
}
3858+
// Enable feature HugePages
3859+
defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.HugePages, true)()
38773860
for i := range successCases {
38783861
pod := &successCases[i]
38793862
if errs := ValidatePod(pod); len(errs) != 0 {
@@ -3886,13 +3869,9 @@ func TestAlphaHugePagesIsolation(t *testing.T) {
38863869
t.Errorf("Expected error for case[%d], pod: %v", i, pod.Name)
38873870
}
38883871
}
3889-
// Disable alpha feature HugePages
3890-
err = utilfeature.DefaultFeatureGate.Set("HugePages=false")
3891-
if err != nil {
3892-
t.Errorf("Failed to disable feature gate for HugePages: %v", err)
3893-
return
3894-
}
3895-
// Disable alpha feature HugePages and ensure all success cases fail
3872+
// Disable feature HugePages
3873+
defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.HugePages, false)()
3874+
// Disable feature HugePages and ensure all success cases fail
38963875
for i := range successCases {
38973876
pod := &successCases[i]
38983877
if errs := ValidatePod(pod); len(errs) == 0 {
@@ -4053,23 +4032,15 @@ func TestAlphaLocalStorageCapacityIsolation(t *testing.T) {
40534032
testCases := []core.VolumeSource{
40544033
{EmptyDir: &core.EmptyDirVolumeSource{SizeLimit: resource.NewQuantity(int64(5), resource.BinarySI)}},
40554034
}
4056-
// Enable alpha feature LocalStorageCapacityIsolation
4057-
err := utilfeature.DefaultFeatureGate.Set("LocalStorageCapacityIsolation=true")
4058-
if err != nil {
4059-
t.Errorf("Failed to enable feature gate for LocalStorageCapacityIsolation: %v", err)
4060-
return
4061-
}
4035+
// Enable feature LocalStorageCapacityIsolation
4036+
defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.LocalStorageCapacityIsolation, true)()
40624037
for _, tc := range testCases {
40634038
if errs := validateVolumeSource(&tc, field.NewPath("spec"), "tmpvol"); len(errs) != 0 {
40644039
t.Errorf("expected success: %v", errs)
40654040
}
40664041
}
4067-
// Disable alpha feature LocalStorageCapacityIsolation
4068-
err = utilfeature.DefaultFeatureGate.Set("LocalStorageCapacityIsolation=false")
4069-
if err != nil {
4070-
t.Errorf("Failed to disable feature gate for LocalStorageCapacityIsolation: %v", err)
4071-
return
4072-
}
4042+
// Disable feature LocalStorageCapacityIsolation
4043+
defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.LocalStorageCapacityIsolation, false)()
40734044
for _, tc := range testCases {
40744045
if errs := validateVolumeSource(&tc, field.NewPath("spec"), "tmpvol"); len(errs) == 0 {
40754046
t.Errorf("expected failure: %v", errs)
@@ -4083,21 +4054,13 @@ func TestAlphaLocalStorageCapacityIsolation(t *testing.T) {
40834054
resource.BinarySI),
40844055
},
40854056
}
4086-
// Enable alpha feature LocalStorageCapacityIsolation
4087-
err = utilfeature.DefaultFeatureGate.Set("LocalStorageCapacityIsolation=true")
4088-
if err != nil {
4089-
t.Errorf("Failed to enable feature gate for LocalStorageCapacityIsolation: %v", err)
4090-
return
4091-
}
4057+
// Enable feature LocalStorageCapacityIsolation
4058+
defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.LocalStorageCapacityIsolation, true)()
40924059
if errs := ValidateResourceRequirements(&containerLimitCase, field.NewPath("resources")); len(errs) != 0 {
40934060
t.Errorf("expected success: %v", errs)
40944061
}
4095-
// Disable alpha feature LocalStorageCapacityIsolation
4096-
err = utilfeature.DefaultFeatureGate.Set("LocalStorageCapacityIsolation=false")
4097-
if err != nil {
4098-
t.Errorf("Failed to disable feature gate for LocalStorageCapacityIsolation: %v", err)
4099-
return
4100-
}
4062+
// Disable feature LocalStorageCapacityIsolation
4063+
defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.LocalStorageCapacityIsolation, false)()
41014064
if errs := ValidateResourceRequirements(&containerLimitCase, field.NewPath("resources")); len(errs) == 0 {
41024065
t.Errorf("expected failure: %v", errs)
41034066
}
@@ -4132,22 +4095,14 @@ func TestValidateResourceQuotaWithAlphaLocalStorageCapacityIsolation(t *testing.
41324095
Spec: spec,
41334096
}
41344097

4135-
// Enable alpha feature LocalStorageCapacityIsolation
4136-
err := utilfeature.DefaultFeatureGate.Set("LocalStorageCapacityIsolation=true")
4137-
if err != nil {
4138-
t.Errorf("Failed to enable feature gate for LocalStorageCapacityIsolation: %v", err)
4139-
return
4140-
}
4098+
// Enable feature LocalStorageCapacityIsolation
4099+
defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.LocalStorageCapacityIsolation, true)()
41414100
if errs := ValidateResourceQuota(resourceQuota); len(errs) != 0 {
41424101
t.Errorf("expected success: %v", errs)
41434102
}
41444103

4145-
// Disable alpha feature LocalStorageCapacityIsolation
4146-
err = utilfeature.DefaultFeatureGate.Set("LocalStorageCapacityIsolation=false")
4147-
if err != nil {
4148-
t.Errorf("Failed to disable feature gate for LocalStorageCapacityIsolation: %v", err)
4149-
return
4150-
}
4104+
// Disable feature LocalStorageCapacityIsolation
4105+
defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.LocalStorageCapacityIsolation, false)()
41514106
errs := ValidateResourceQuota(resourceQuota)
41524107
if len(errs) == 0 {
41534108
t.Errorf("expected failure for %s", resourceQuota.Name)
@@ -4280,24 +4235,16 @@ func TestLocalStorageEnvWithFeatureGate(t *testing.T) {
42804235
},
42814236
},
42824237
}
4283-
// Enable alpha feature LocalStorageCapacityIsolation
4284-
err := utilfeature.DefaultFeatureGate.Set("LocalStorageCapacityIsolation=true")
4285-
if err != nil {
4286-
t.Errorf("Failed to enable feature gate for LocalStorageCapacityIsolation: %v", err)
4287-
return
4288-
}
4238+
// Enable feature LocalStorageCapacityIsolation
4239+
defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.LocalStorageCapacityIsolation, true)()
42894240
for _, testCase := range testCases {
42904241
if errs := validateEnvVarValueFrom(testCase, field.NewPath("field")); len(errs) != 0 {
42914242
t.Errorf("expected success, got: %v", errs)
42924243
}
42934244
}
42944245

4295-
// Disable alpha feature LocalStorageCapacityIsolation
4296-
err = utilfeature.DefaultFeatureGate.Set("LocalStorageCapacityIsolation=false")
4297-
if err != nil {
4298-
t.Errorf("Failed to disable feature gate for LocalStorageCapacityIsolation: %v", err)
4299-
return
4300-
}
4246+
// Disable feature LocalStorageCapacityIsolation
4247+
defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.LocalStorageCapacityIsolation, false)()
43014248
for _, testCase := range testCases {
43024249
if errs := validateEnvVarValueFrom(testCase, field.NewPath("field")); len(errs) == 0 {
43034250
t.Errorf("expected failure for %v", testCase.Name)
@@ -11187,26 +11134,17 @@ func TestValidateLimitRangeForLocalStorage(t *testing.T) {
1118711134
},
1118811135
}
1118911136

11190-
// Enable alpha feature LocalStorageCapacityIsolation
11191-
err := utilfeature.DefaultFeatureGate.Set("LocalStorageCapacityIsolation=true")
11192-
if err != nil {
11193-
t.Errorf("Failed to enable feature gate for LocalStorageCapacityIsolation: %v", err)
11194-
return
11195-
}
11196-
11137+
// Enable feature LocalStorageCapacityIsolation
11138+
defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.LocalStorageCapacityIsolation, true)()
1119711139
for _, testCase := range testCases {
1119811140
limitRange := &core.LimitRange{ObjectMeta: metav1.ObjectMeta{Name: testCase.name, Namespace: "foo"}, Spec: testCase.spec}
1119911141
if errs := ValidateLimitRange(limitRange); len(errs) != 0 {
1120011142
t.Errorf("Case %v, unexpected error: %v", testCase.name, errs)
1120111143
}
1120211144
}
1120311145

11204-
// Disable alpha feature LocalStorageCapacityIsolation
11205-
err = utilfeature.DefaultFeatureGate.Set("LocalStorageCapacityIsolation=false")
11206-
if err != nil {
11207-
t.Errorf("Failed to disable feature gate for LocalStorageCapacityIsolation: %v", err)
11208-
return
11209-
}
11146+
// Disable feature LocalStorageCapacityIsolation
11147+
defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.LocalStorageCapacityIsolation, false)()
1121011148
for _, testCase := range testCases {
1121111149
limitRange := &core.LimitRange{ObjectMeta: metav1.ObjectMeta{Name: testCase.name, Namespace: "foo"}, Spec: testCase.spec}
1121211150
if errs := ValidateLimitRange(limitRange); len(errs) == 0 {
@@ -11746,13 +11684,13 @@ func TestValidateResourceQuota(t *testing.T) {
1174611684
Spec: nonBestEffortSpec,
1174711685
},
1174811686
}
11749-
utilfeature.DefaultFeatureGate.Set("ResourceQuotaScopeSelectors=true")
11687+
defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.ResourceQuotaScopeSelectors, true)()
1175011688
for _, successCase := range successCases {
1175111689
if errs := ValidateResourceQuota(&successCase); len(errs) != 0 {
1175211690
t.Errorf("expected success: %v", errs)
1175311691
}
1175411692
}
11755-
utilfeature.DefaultFeatureGate.Set("ResourceQuotaScopeSelectors=false")
11693+
defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.ResourceQuotaScopeSelectors, false)()
1175611694

1175711695
errorCases := map[string]struct {
1175811696
R core.ResourceQuota

‎pkg/apis/storage/util/BUILD

+2
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,8 @@ go_test(
4141
deps = [
4242
"//pkg/apis/core:go_default_library",
4343
"//pkg/apis/storage:go_default_library",
44+
"//pkg/features:go_default_library",
4445
"//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library",
46+
"//staging/src/k8s.io/apiserver/pkg/util/feature/testing:go_default_library",
4547
],
4648
)

‎pkg/apis/storage/util/util_test.go

+4-10
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,10 @@ import (
2121
"testing"
2222

2323
utilfeature "k8s.io/apiserver/pkg/util/feature"
24+
utilfeaturetesting "k8s.io/apiserver/pkg/util/feature/testing"
2425
api "k8s.io/kubernetes/pkg/apis/core"
2526
"k8s.io/kubernetes/pkg/apis/storage"
27+
"k8s.io/kubernetes/pkg/features"
2628
)
2729

2830
func TestDropAlphaFields(t *testing.T) {
@@ -39,9 +41,7 @@ func TestDropAlphaFields(t *testing.T) {
3941
}
4042

4143
// Test that field gets dropped when feature gate is not set
42-
if err := utilfeature.DefaultFeatureGate.Set("VolumeScheduling=false"); err != nil {
43-
t.Fatalf("Failed to set feature gate for VolumeScheduling: %v", err)
44-
}
44+
defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.VolumeScheduling, false)()
4545
class := &storage.StorageClass{
4646
VolumeBindingMode: &bindingMode,
4747
AllowedTopologies: allowedTopologies,
@@ -59,18 +59,12 @@ func TestDropAlphaFields(t *testing.T) {
5959
VolumeBindingMode: &bindingMode,
6060
AllowedTopologies: allowedTopologies,
6161
}
62-
if err := utilfeature.DefaultFeatureGate.Set("VolumeScheduling=true"); err != nil {
63-
t.Fatalf("Failed to set feature gate for VolumeScheduling: %v", err)
64-
}
62+
defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.VolumeScheduling, true)()
6563
DropDisabledAlphaFields(class)
6664
if class.VolumeBindingMode != &bindingMode {
6765
t.Errorf("VolumeBindingMode field got unexpectantly modified: %+v", class.VolumeBindingMode)
6866
}
6967
if !reflect.DeepEqual(class.AllowedTopologies, allowedTopologies) {
7068
t.Errorf("AllowedTopologies field got unexpectantly modified: %+v", class.AllowedTopologies)
7169
}
72-
73-
if err := utilfeature.DefaultFeatureGate.Set("VolumeScheduling=false"); err != nil {
74-
t.Fatalf("Failed to disable feature gate for VolumeScheduling: %v", err)
75-
}
7670
}

0 commit comments

Comments
 (0)