From 9bbf768ad647641c83dd01b04487269295b52144 Mon Sep 17 00:00:00 2001 From: Matthew Wong Date: Thu, 15 Nov 2018 23:36:42 -0500 Subject: [PATCH] Fix BlockVolume feature gate toggling in validation & defaults unit tests --- pkg/apis/core/v1/defaults_test.go | 10 +- pkg/apis/core/validation/validation_test.go | 144 +++++++------------- 2 files changed, 54 insertions(+), 100 deletions(-) diff --git a/pkg/apis/core/v1/defaults_test.go b/pkg/apis/core/v1/defaults_test.go index 50740a9ca6a..b082014864d 100644 --- a/pkg/apis/core/v1/defaults_test.go +++ b/pkg/apis/core/v1/defaults_test.go @@ -825,10 +825,7 @@ func TestSetDefaultPersistentVolume(t *testing.T) { } // When feature gate is enabled, field should be defaulted - err := utilfeature.DefaultFeatureGate.Set("BlockVolume=true") - if err != nil { - t.Fatalf("Failed to enable feature gate for BlockVolume: %v", err) - } + defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.BlockVolume, true)() obj3 := roundTrip(t, runtime.Object(pv)).(*v1.PersistentVolume) outputMode3 := obj3.Spec.VolumeMode @@ -857,10 +854,7 @@ func TestSetDefaultPersistentVolumeClaim(t *testing.T) { } // When feature gate is enabled, field should be defaulted - err := utilfeature.DefaultFeatureGate.Set("BlockVolume=true") - if err != nil { - t.Fatalf("Failed to enable feature gate for BlockVolume: %v", err) - } + defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.BlockVolume, true)() obj3 := roundTrip(t, runtime.Object(pvc)).(*v1.PersistentVolumeClaim) outputMode3 := obj3.Spec.VolumeMode diff --git a/pkg/apis/core/validation/validation_test.go b/pkg/apis/core/validation/validation_test.go index 15746271956..94bff049cda 100644 --- a/pkg/apis/core/validation/validation_test.go +++ b/pkg/apis/core/validation/validation_test.go @@ -68,9 +68,9 @@ func TestValidatePersistentVolumes(t *testing.T) { validMode := core.PersistentVolumeFilesystem invalidMode := core.PersistentVolumeMode("fakeVolumeMode") scenarios := map[string]struct { - isExpectedFailure bool - volume *core.PersistentVolume - disableBlockVolume bool + isExpectedFailure bool + volume *core.PersistentVolume + disableBlock bool }{ "good-volume": { isExpectedFailure: false, @@ -371,8 +371,8 @@ func TestValidatePersistentVolumes(t *testing.T) { }), }, "feature disabled valid volume mode": { - disableBlockVolume: true, - isExpectedFailure: true, + disableBlock: true, + isExpectedFailure: true, volume: testVolume("foo", "", core.PersistentVolumeSpec{ Capacity: core.ResourceList{ core.ResourceName(core.ResourceStorage): resource.MustParse("10G"), @@ -433,20 +433,16 @@ func TestValidatePersistentVolumes(t *testing.T) { } for name, scenario := range scenarios { - var restore func() - if scenario.disableBlockVolume { - restore = utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.BlockVolume, false) - } - errs := ValidatePersistentVolume(scenario.volume) - 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) - } - if scenario.disableBlockVolume { - restore() - } + t.Run(name, func(t *testing.T) { + defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.BlockVolume, !scenario.disableBlock)() + errs := ValidatePersistentVolume(scenario.volume) + 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) + } + }) } } @@ -834,9 +830,9 @@ func TestValidatePersistentVolumeClaim(t *testing.T) { invalidMode := core.PersistentVolumeMode("fakeVolumeMode") validMode := core.PersistentVolumeFilesystem scenarios := map[string]struct { - isExpectedFailure bool - claim *core.PersistentVolumeClaim - disableBlockVolume bool + isExpectedFailure bool + claim *core.PersistentVolumeClaim + disableBlock bool }{ "good-claim": { isExpectedFailure: false, @@ -1032,8 +1028,8 @@ func TestValidatePersistentVolumeClaim(t *testing.T) { }), }, "feature disabled valid volume mode": { - disableBlockVolume: true, - isExpectedFailure: true, + disableBlock: true, + isExpectedFailure: true, claim: testVolumeClaim("foo", "ns", core.PersistentVolumeClaimSpec{ Selector: &metav1.LabelSelector{ MatchExpressions: []metav1.LabelSelectorRequirement{ @@ -1074,20 +1070,16 @@ func TestValidatePersistentVolumeClaim(t *testing.T) { } for name, scenario := range scenarios { - var restore func() - if scenario.disableBlockVolume { - restore = utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.BlockVolume, false) - } - errs := ValidatePersistentVolumeClaim(scenario.claim) - 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) - } - if scenario.disableBlockVolume { - restore() - } + t.Run(name, func(t *testing.T) { + defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.BlockVolume, !scenario.disableBlock)() + errs := ValidatePersistentVolumeClaim(scenario.claim) + 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) + } + }) } } @@ -1170,15 +1162,17 @@ func TestAlphaPVVolumeModeUpdate(t *testing.T) { } for name, scenario := range scenarios { - // ensure we have a resource version specified for updates - toggleBlockVolumeFeature(scenario.enableBlock, t) - errs := ValidatePersistentVolumeUpdate(scenario.newPV, scenario.oldPV) - 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.BlockVolume, scenario.enableBlock)() + errs := ValidatePersistentVolumeUpdate(scenario.newPV, scenario.oldPV) + 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) + } + }) } } @@ -1622,8 +1616,7 @@ func TestValidatePersistentVolumeClaimUpdate(t *testing.T) { 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) + defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.BlockVolume, scenario.enableBlock)() scenario.oldClaim.ResourceVersion = "1" scenario.newClaim.ResourceVersion = "1" errs := ValidatePersistentVolumeClaimUpdate(scenario.newClaim, scenario.oldClaim) @@ -1637,23 +1630,6 @@ func TestValidatePersistentVolumeClaimUpdate(t *testing.T) { } } -func toggleBlockVolumeFeature(toggleFlag bool, t *testing.T) { - if toggleFlag { - // Enable alpha feature BlockVolume - err := utilfeature.DefaultFeatureGate.Set("BlockVolume=true") - if err != nil { - t.Errorf("Failed to enable feature gate for BlockVolume: %v", err) - return - } - } else { - err := utilfeature.DefaultFeatureGate.Set("BlockVolume=false") - if err != nil { - t.Errorf("Failed to disable feature gate for BlockVolume: %v", err) - return - } - } -} - func TestValidateKeyToPath(t *testing.T) { testCases := []struct { kp core.KeyToPath @@ -3944,13 +3920,9 @@ func TestAlphaHugePagesIsolation(t *testing.T) { } } -func TestAlphaPVCVolumeMode(t *testing.T) { - // Enable alpha feature BlockVolume for PVC - err := utilfeature.DefaultFeatureGate.Set("BlockVolume=true") - if err != nil { - t.Errorf("Failed to enable feature gate for BlockVolume: %v", err) - return - } +func TestPVCVolumeMode(t *testing.T) { + // Enable feature BlockVolume for PVC + defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.BlockVolume, true)() block := core.PersistentVolumeBlock file := core.PersistentVolumeFilesystem @@ -3981,13 +3953,9 @@ func TestAlphaPVCVolumeMode(t *testing.T) { } } -func TestAlphaPVVolumeMode(t *testing.T) { - // Enable alpha feature BlockVolume for PV - err := utilfeature.DefaultFeatureGate.Set("BlockVolume=true") - if err != nil { - t.Errorf("Failed to enable feature gate for BlockVolume: %v", err) - return - } +func TestPVVolumeMode(t *testing.T) { + // Enable feature BlockVolume for PVC + defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.BlockVolume, true)() block := core.PersistentVolumeBlock file := core.PersistentVolumeFilesystem @@ -5152,12 +5120,8 @@ func TestAlphaValidateVolumeDevices(t *testing.T) { {Name: "abc-123", MountPath: "/this/path/exists"}, } - // enable Alpha BlockVolume - err1 := utilfeature.DefaultFeatureGate.Set("BlockVolume=true") - if err1 != nil { - t.Errorf("Failed to enable feature gate for BlockVolume: %v", err1) - return - } + // enable BlockVolume + defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.BlockVolume, true)() // Success Cases: // Validate normal success cases - only PVC volumeSource if errs := ValidateVolumeDevices(successCase, GetVolumeMountMap(goodVolumeMounts), vols, field.NewPath("field")); len(errs) != 0 { @@ -5172,12 +5136,8 @@ func TestAlphaValidateVolumeDevices(t *testing.T) { } } - // disable Alpha BlockVolume - err2 := utilfeature.DefaultFeatureGate.Set("BlockVolume=false") - if err2 != nil { - t.Errorf("Failed to disable feature gate for BlockVolume: %v", err2) - return - } + // disable BlockVolume + defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.BlockVolume, false)() if errs := ValidateVolumeDevices(disabledAlphaVolDevice, GetVolumeMountMap(goodVolumeMounts), vols, field.NewPath("field")); len(errs) == 0 { t.Errorf("expected failure: %v", errs) }