diff --git a/pkg/api/persistentvolume/util.go b/pkg/api/persistentvolume/util.go index 298a38250f..0d3a79889c 100644 --- a/pkg/api/persistentvolume/util.go +++ b/pkg/api/persistentvolume/util.go @@ -25,13 +25,8 @@ import ( // DropDisabledFields removes disabled fields from the pv spec. // This should be called from PrepareForCreate/PrepareForUpdate for all resources containing a pv spec. func DropDisabledFields(pvSpec *api.PersistentVolumeSpec, oldPVSpec *api.PersistentVolumeSpec) { - if !utilfeature.DefaultFeatureGate.Enabled(features.BlockVolume) { - // TODO(liggitt): change this to only drop pvSpec.VolumeMode if (oldPVSpec == nil || oldPVSpec.VolumeMode == nil) - // Requires more coordinated changes to validation + if !utilfeature.DefaultFeatureGate.Enabled(features.BlockVolume) && !volumeModeInUse(oldPVSpec) { pvSpec.VolumeMode = nil - if oldPVSpec != nil { - oldPVSpec.VolumeMode = nil - } } if !utilfeature.DefaultFeatureGate.Enabled(features.CSIPersistentVolume) { @@ -41,3 +36,13 @@ func DropDisabledFields(pvSpec *api.PersistentVolumeSpec, oldPVSpec *api.Persist } } } + +func volumeModeInUse(oldPVSpec *api.PersistentVolumeSpec) bool { + if oldPVSpec == nil { + return false + } + if oldPVSpec.VolumeMode != nil { + return true + } + return false +} diff --git a/pkg/api/persistentvolume/util_test.go b/pkg/api/persistentvolume/util_test.go index 6ea33b5a38..3b31f298c1 100644 --- a/pkg/api/persistentvolume/util_test.go +++ b/pkg/api/persistentvolume/util_test.go @@ -106,13 +106,12 @@ func TestDropDisabledFields(t *testing.T) { oldSpec: specWithMode(nil), expectOldSpec: specWithMode(nil), }, - // TODO: consider changing this case to preserve - "disabled block clears old and new on update when old pv did use block": { + "disabled block does not clear new on update when old pv did use block": { blockEnabled: false, newSpec: specWithMode(&modeBlock), - expectNewSpec: specWithMode(nil), + expectNewSpec: specWithMode(&modeBlock), oldSpec: specWithMode(&modeBlock), - expectOldSpec: specWithMode(nil), + expectOldSpec: specWithMode(&modeBlock), }, "enabled block preserves new": { diff --git a/pkg/api/persistentvolumeclaim/BUILD b/pkg/api/persistentvolumeclaim/BUILD index 3a72b78c8f..e1b030325a 100644 --- a/pkg/api/persistentvolumeclaim/BUILD +++ b/pkg/api/persistentvolumeclaim/BUILD @@ -37,6 +37,7 @@ go_test( deps = [ "//pkg/apis/core:go_default_library", "//pkg/features:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/util/diff: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.go b/pkg/api/persistentvolumeclaim/util.go index df3f777f5c..e55c02fa30 100644 --- a/pkg/api/persistentvolumeclaim/util.go +++ b/pkg/api/persistentvolumeclaim/util.go @@ -25,10 +25,17 @@ import ( // DropDisabledFields removes disabled fields from the pvc spec. // This should be called from PrepareForCreate/PrepareForUpdate for all resources containing a pvc spec. func DropDisabledFields(pvcSpec, oldPVCSpec *core.PersistentVolumeClaimSpec) { - if !utilfeature.DefaultFeatureGate.Enabled(features.BlockVolume) { + if !utilfeature.DefaultFeatureGate.Enabled(features.BlockVolume) && !volumeModeInUse(oldPVCSpec) { pvcSpec.VolumeMode = nil - if oldPVCSpec != nil { - oldPVCSpec.VolumeMode = nil - } } } + +func volumeModeInUse(oldPVCSpec *core.PersistentVolumeClaimSpec) bool { + if oldPVCSpec == nil { + return false + } + if oldPVCSpec.VolumeMode != nil { + return true + } + return false +} diff --git a/pkg/api/persistentvolumeclaim/util_test.go b/pkg/api/persistentvolumeclaim/util_test.go index 63607af2db..d5242c03a8 100644 --- a/pkg/api/persistentvolumeclaim/util_test.go +++ b/pkg/api/persistentvolumeclaim/util_test.go @@ -17,8 +17,11 @@ limitations under the License. package persistentvolumeclaim import ( + "fmt" + "reflect" "testing" + "k8s.io/apimachinery/pkg/util/diff" utilfeature "k8s.io/apiserver/pkg/util/feature" utilfeaturetesting "k8s.io/apiserver/pkg/util/feature/testing" "k8s.io/kubernetes/pkg/apis/core" @@ -28,33 +31,89 @@ import ( func TestDropAlphaPVCVolumeMode(t *testing.T) { vmode := core.PersistentVolumeFilesystem - // PersistentVolume with VolumeMode set - pvc := core.PersistentVolumeClaim{ - Spec: core.PersistentVolumeClaimSpec{ - AccessModes: []core.PersistentVolumeAccessMode{core.ReadWriteOnce}, - VolumeMode: &vmode, + pvcWithoutVolumeMode := func() *core.PersistentVolumeClaim { + return &core.PersistentVolumeClaim{ + Spec: core.PersistentVolumeClaimSpec{ + VolumeMode: nil, + }, + } + } + pvcWithVolumeMode := func() *core.PersistentVolumeClaim { + return &core.PersistentVolumeClaim{ + Spec: core.PersistentVolumeClaimSpec{ + VolumeMode: &vmode, + }, + } + } + + pvcInfo := []struct { + description string + hasVolumeMode bool + pvc func() *core.PersistentVolumeClaim + }{ + { + description: "pvc without VolumeMode", + hasVolumeMode: false, + pvc: pvcWithoutVolumeMode, + }, + { + description: "pvc with Filesystem VolumeMode", + hasVolumeMode: true, + pvc: pvcWithVolumeMode, + }, + { + description: "is nil", + hasVolumeMode: false, + pvc: func() *core.PersistentVolumeClaim { return nil }, }, } - // Enable alpha feature BlockVolume - defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.BlockVolume, true)() - // now test dropping the fields - should not be dropped - DropDisabledFields(&pvc.Spec, nil) + for _, enabled := range []bool{true, false} { + for _, oldpvcInfo := range pvcInfo { + for _, newpvcInfo := range pvcInfo { + oldpvcHasVolumeMode, oldpvc := oldpvcInfo.hasVolumeMode, oldpvcInfo.pvc() + newpvcHasVolumeMode, newpvc := newpvcInfo.hasVolumeMode, newpvcInfo.pvc() + if newpvc == nil { + continue + } - // check to make sure VolumeDevices is still present - // if featureset is set to true - if pvc.Spec.VolumeMode == nil { - t.Error("VolumeMode in pvc.Spec should not have been dropped based on feature-gate") - } + t.Run(fmt.Sprintf("feature enabled=%v, old pvc %v, new pvc %v", enabled, oldpvcInfo.description, newpvcInfo.description), func(t *testing.T) { + defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.BlockVolume, enabled)() - // Disable alpha feature BlockVolume - defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.BlockVolume, false)() - // now test dropping the fields - DropDisabledFields(&pvc.Spec, nil) + var oldpvcSpec *core.PersistentVolumeClaimSpec + if oldpvc != nil { + oldpvcSpec = &oldpvc.Spec + } + DropDisabledFields(&newpvc.Spec, oldpvcSpec) - // check to make sure VolumeDevices is nil - // if featureset is set to false - if pvc.Spec.VolumeMode != nil { - t.Error("DropDisabledFields VolumeMode for pvc.Spec failed") + // old pvc should never be changed + if !reflect.DeepEqual(oldpvc, oldpvcInfo.pvc()) { + t.Errorf("old pvc changed: %v", diff.ObjectReflectDiff(oldpvc, oldpvcInfo.pvc())) + } + + switch { + case enabled || oldpvcHasVolumeMode: + // new pvc should not be changed if the feature is enabled, or if the old pvc had BlockVolume + if !reflect.DeepEqual(newpvc, newpvcInfo.pvc()) { + t.Errorf("new pvc changed: %v", diff.ObjectReflectDiff(newpvc, newpvcInfo.pvc())) + } + case newpvcHasVolumeMode: + // new pvc should be changed + if reflect.DeepEqual(newpvc, newpvcInfo.pvc()) { + t.Errorf("new pvc was not changed") + } + // new pvc should not have BlockVolume + if !reflect.DeepEqual(newpvc, pvcWithoutVolumeMode()) { + t.Errorf("new pvc had pvcBlockVolume: %v", diff.ObjectReflectDiff(newpvc, pvcWithoutVolumeMode())) + } + default: + // new pvc should not need to be changed + if !reflect.DeepEqual(newpvc, newpvcInfo.pvc()) { + t.Errorf("new pvc changed: %v", diff.ObjectReflectDiff(newpvc, newpvcInfo.pvc())) + } + } + }) + } + } } } diff --git a/pkg/apis/core/v1/BUILD b/pkg/apis/core/v1/BUILD index e52a3e4f90..7226462bbe 100644 --- a/pkg/apis/core/v1/BUILD +++ b/pkg/apis/core/v1/BUILD @@ -15,7 +15,6 @@ go_library( deps = [ "//pkg/apis/apps:go_default_library", "//pkg/apis/core:go_default_library", - "//pkg/features:go_default_library", "//pkg/util/parsers:go_default_library", "//staging/src/k8s.io/api/core/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/api/resource:go_default_library", @@ -26,7 +25,6 @@ go_library( "//staging/src/k8s.io/apimachinery/pkg/types:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/intstr: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", "//vendor/k8s.io/utils/pointer:go_default_library", ], ) @@ -44,7 +42,6 @@ go_test( "//pkg/apis/apps:go_default_library", "//pkg/apis/core:go_default_library", "//pkg/apis/core/fuzzer:go_default_library", - "//pkg/features:go_default_library", "//staging/src/k8s.io/api/apps/v1:go_default_library", "//staging/src/k8s.io/api/core/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/api/apitesting/fuzzer:go_default_library", @@ -55,8 +52,6 @@ go_test( "//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/diff:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/intstr: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", "//vendor/k8s.io/utils/pointer:go_default_library", ], ) diff --git a/pkg/apis/core/v1/defaults.go b/pkg/apis/core/v1/defaults.go index 172d3797bf..4f681a5a77 100644 --- a/pkg/apis/core/v1/defaults.go +++ b/pkg/apis/core/v1/defaults.go @@ -22,8 +22,6 @@ import ( "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/intstr" - utilfeature "k8s.io/apiserver/pkg/util/feature" - "k8s.io/kubernetes/pkg/features" "k8s.io/kubernetes/pkg/util/parsers" utilpointer "k8s.io/utils/pointer" ) @@ -247,7 +245,7 @@ func SetDefaults_PersistentVolume(obj *v1.PersistentVolume) { if obj.Spec.PersistentVolumeReclaimPolicy == "" { obj.Spec.PersistentVolumeReclaimPolicy = v1.PersistentVolumeReclaimRetain } - if obj.Spec.VolumeMode == nil && utilfeature.DefaultFeatureGate.Enabled(features.BlockVolume) { + if obj.Spec.VolumeMode == nil { obj.Spec.VolumeMode = new(v1.PersistentVolumeMode) *obj.Spec.VolumeMode = v1.PersistentVolumeFilesystem } @@ -256,7 +254,7 @@ func SetDefaults_PersistentVolumeClaim(obj *v1.PersistentVolumeClaim) { if obj.Status.Phase == "" { obj.Status.Phase = v1.ClaimPending } - if obj.Spec.VolumeMode == nil && utilfeature.DefaultFeatureGate.Enabled(features.BlockVolume) { + if obj.Spec.VolumeMode == nil { obj.Spec.VolumeMode = new(v1.PersistentVolumeMode) *obj.Spec.VolumeMode = v1.PersistentVolumeFilesystem } diff --git a/pkg/apis/core/v1/defaults_test.go b/pkg/apis/core/v1/defaults_test.go index b082014864..b3d9ad4316 100644 --- a/pkg/apis/core/v1/defaults_test.go +++ b/pkg/apis/core/v1/defaults_test.go @@ -26,11 +26,8 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/intstr" - utilfeature "k8s.io/apiserver/pkg/util/feature" - utilfeaturetesting "k8s.io/apiserver/pkg/util/feature/testing" "k8s.io/kubernetes/pkg/api/legacyscheme" corev1 "k8s.io/kubernetes/pkg/apis/core/v1" - "k8s.io/kubernetes/pkg/features" utilpointer "k8s.io/utils/pointer" // enforce that all types are installed @@ -805,63 +802,91 @@ func TestSetDefaultSecret(t *testing.T) { } func TestSetDefaultPersistentVolume(t *testing.T) { - defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.BlockVolume, false)() - pv := &v1.PersistentVolume{} - obj2 := roundTrip(t, runtime.Object(pv)) - pv2 := obj2.(*v1.PersistentVolume) + fsMode := v1.PersistentVolumeFilesystem + blockMode := v1.PersistentVolumeBlock - if pv2.Status.Phase != v1.VolumePending { - t.Errorf("Expected volume phase %v, got %v", v1.VolumePending, pv2.Status.Phase) - } - if pv2.Spec.PersistentVolumeReclaimPolicy != v1.PersistentVolumeReclaimRetain { - t.Errorf("Expected pv reclaim policy %v, got %v", v1.PersistentVolumeReclaimRetain, pv2.Spec.PersistentVolumeReclaimPolicy) + tests := []struct { + name string + volumeMode *v1.PersistentVolumeMode + expectedVolumeMode v1.PersistentVolumeMode + }{ + { + name: "volume mode nil", + volumeMode: nil, + expectedVolumeMode: v1.PersistentVolumeFilesystem, + }, + { + name: "volume mode filesystem", + volumeMode: &fsMode, + expectedVolumeMode: v1.PersistentVolumeFilesystem, + }, + { + name: "volume mode block", + volumeMode: &blockMode, + expectedVolumeMode: v1.PersistentVolumeBlock, + }, } - // When feature gate is disabled, field should not be defaulted - defaultMode := v1.PersistentVolumeFilesystem - outputMode := pv2.Spec.VolumeMode - if outputMode != nil { - t.Errorf("Expected VolumeMode to not be defaulted, got: %+v", outputMode) - } - - // When feature gate is enabled, field should be defaulted - defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.BlockVolume, true)() - obj3 := roundTrip(t, runtime.Object(pv)).(*v1.PersistentVolume) - outputMode3 := obj3.Spec.VolumeMode - - if outputMode3 == nil { - t.Errorf("Expected VolumeMode to be defaulted to: %+v, got: nil", defaultMode) - } else if *outputMode3 != defaultMode { - t.Errorf("Expected VolumeMode to be defaulted to: %+v, got: %+v", defaultMode, outputMode3) + for _, test := range tests { + pv := &v1.PersistentVolume{ + Spec: v1.PersistentVolumeSpec{ + VolumeMode: test.volumeMode, + }, + } + obj1 := roundTrip(t, runtime.Object(pv)) + pv1 := obj1.(*v1.PersistentVolume) + if pv1.Status.Phase != v1.VolumePending { + t.Errorf("Expected claim phase %v, got %v", v1.ClaimPending, pv1.Status.Phase) + } + if pv1.Spec.PersistentVolumeReclaimPolicy != v1.PersistentVolumeReclaimRetain { + t.Errorf("Expected pv reclaim policy %v, got %v", v1.PersistentVolumeReclaimRetain, pv1.Spec.PersistentVolumeReclaimPolicy) + } + if *pv1.Spec.VolumeMode != test.expectedVolumeMode { + t.Errorf("Test %s failed, Expected VolumeMode: %v, but got %v", test.name, test.volumeMode, *pv1.Spec.VolumeMode) + } } } func TestSetDefaultPersistentVolumeClaim(t *testing.T) { - defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.BlockVolume, false)() - pvc := &v1.PersistentVolumeClaim{} - obj2 := roundTrip(t, runtime.Object(pvc)) - pvc2 := obj2.(*v1.PersistentVolumeClaim) + fsMode := v1.PersistentVolumeFilesystem + blockMode := v1.PersistentVolumeBlock - if pvc2.Status.Phase != v1.ClaimPending { - t.Errorf("Expected claim phase %v, got %v", v1.ClaimPending, pvc2.Status.Phase) + tests := []struct { + name string + volumeMode *v1.PersistentVolumeMode + expectedVolumeMode v1.PersistentVolumeMode + }{ + { + name: "volume mode nil", + volumeMode: nil, + expectedVolumeMode: v1.PersistentVolumeFilesystem, + }, + { + name: "volume mode filesystem", + volumeMode: &fsMode, + expectedVolumeMode: v1.PersistentVolumeFilesystem, + }, + { + name: "volume mode block", + volumeMode: &blockMode, + expectedVolumeMode: v1.PersistentVolumeBlock, + }, } - // When feature gate is disabled, field should not be defaulted - defaultMode := v1.PersistentVolumeFilesystem - outputMode := pvc2.Spec.VolumeMode - if outputMode != nil { - t.Errorf("Expected VolumeMode to not be defaulted, got: %+v", outputMode) - } - - // When feature gate is enabled, field should be defaulted - defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.BlockVolume, true)() - obj3 := roundTrip(t, runtime.Object(pvc)).(*v1.PersistentVolumeClaim) - outputMode3 := obj3.Spec.VolumeMode - - if outputMode3 == nil { - t.Errorf("Expected VolumeMode to be defaulted to: %+v, got: nil", defaultMode) - } else if *outputMode3 != defaultMode { - t.Errorf("Expected VolumeMode to be defaulted to: %+v, got: %+v", defaultMode, outputMode3) + for _, test := range tests { + pvc := &v1.PersistentVolumeClaim{ + Spec: v1.PersistentVolumeClaimSpec{ + VolumeMode: test.volumeMode, + }, + } + obj1 := roundTrip(t, runtime.Object(pvc)) + pvc1 := obj1.(*v1.PersistentVolumeClaim) + if pvc1.Status.Phase != v1.ClaimPending { + t.Errorf("Expected claim phase %v, got %v", v1.ClaimPending, pvc1.Status.Phase) + } + if *pvc1.Spec.VolumeMode != test.expectedVolumeMode { + t.Errorf("Test %s failed, Expected VolumeMode: %v, but got %v", test.name, test.volumeMode, *pvc1.Spec.VolumeMode) + } } } diff --git a/pkg/apis/core/validation/validation.go b/pkg/apis/core/validation/validation.go index ba9463d563..2bc4265a07 100644 --- a/pkg/apis/core/validation/validation.go +++ b/pkg/apis/core/validation/validation.go @@ -1763,9 +1763,7 @@ func ValidatePersistentVolume(pv *core.PersistentVolume) field.ErrorList { allErrs = append(allErrs, field.Invalid(specPath.Child("storageClassName"), pv.Spec.StorageClassName, msg)) } } - if pv.Spec.VolumeMode != nil && !utilfeature.DefaultFeatureGate.Enabled(features.BlockVolume) { - allErrs = append(allErrs, field.Forbidden(specPath.Child("volumeMode"), "PersistentVolume volumeMode is disabled by feature-gate")) - } else if pv.Spec.VolumeMode != nil && !supportedVolumeModes.Has(string(*pv.Spec.VolumeMode)) { + if pv.Spec.VolumeMode != nil && !supportedVolumeModes.Has(string(*pv.Spec.VolumeMode)) { allErrs = append(allErrs, field.NotSupported(specPath.Child("volumeMode"), *pv.Spec.VolumeMode, supportedVolumeModes.List())) } return allErrs @@ -1781,12 +1779,9 @@ func ValidatePersistentVolumeUpdate(newPv, oldPv *core.PersistentVolume) field.E if !apiequality.Semantic.DeepEqual(newPv.Spec.PersistentVolumeSource, oldPv.Spec.PersistentVolumeSource) { allErrs = append(allErrs, field.Forbidden(field.NewPath("spec", "persistentvolumesource"), "is immutable after creation")) } - newPv.Status = oldPv.Status - if utilfeature.DefaultFeatureGate.Enabled(features.BlockVolume) { - allErrs = append(allErrs, ValidateImmutableField(newPv.Spec.VolumeMode, oldPv.Spec.VolumeMode, field.NewPath("volumeMode"))...) - } + allErrs = append(allErrs, ValidateImmutableField(newPv.Spec.VolumeMode, oldPv.Spec.VolumeMode, field.NewPath("volumeMode"))...) if utilfeature.DefaultFeatureGate.Enabled(features.VolumeScheduling) { // Allow setting NodeAffinity if oldPv NodeAffinity was not set @@ -1843,9 +1838,7 @@ func ValidatePersistentVolumeClaimSpec(spec *core.PersistentVolumeClaimSpec, fld allErrs = append(allErrs, field.Invalid(fldPath.Child("storageClassName"), *spec.StorageClassName, msg)) } } - if spec.VolumeMode != nil && !utilfeature.DefaultFeatureGate.Enabled(features.BlockVolume) { - allErrs = append(allErrs, field.Forbidden(fldPath.Child("volumeMode"), "PersistentVolumeClaim volumeMode is disabled by feature-gate")) - } else if spec.VolumeMode != nil && !supportedVolumeModes.Has(string(*spec.VolumeMode)) { + if spec.VolumeMode != nil && !supportedVolumeModes.Has(string(*spec.VolumeMode)) { allErrs = append(allErrs, field.NotSupported(fldPath.Child("volumeMode"), *spec.VolumeMode, supportedVolumeModes.List())) } @@ -1920,9 +1913,8 @@ func ValidatePersistentVolumeClaimUpdate(newPvc, oldPvc *core.PersistentVolumeCl } } - if utilfeature.DefaultFeatureGate.Enabled(features.BlockVolume) { - allErrs = append(allErrs, ValidateImmutableField(newPvc.Spec.VolumeMode, oldPvc.Spec.VolumeMode, field.NewPath("volumeMode"))...) - } + allErrs = append(allErrs, ValidateImmutableField(newPvc.Spec.VolumeMode, oldPvc.Spec.VolumeMode, field.NewPath("volumeMode"))...) + return allErrs } diff --git a/pkg/apis/core/validation/validation_test.go b/pkg/apis/core/validation/validation_test.go index 9cfe7369e7..f707aad0f4 100644 --- a/pkg/apis/core/validation/validation_test.go +++ b/pkg/apis/core/validation/validation_test.go @@ -69,7 +69,6 @@ func TestValidatePersistentVolumes(t *testing.T) { scenarios := map[string]struct { isExpectedFailure bool volume *core.PersistentVolume - disableBlock bool }{ "good-volume": { isExpectedFailure: false, @@ -369,24 +368,6 @@ func TestValidatePersistentVolumes(t *testing.T) { StorageClassName: "-invalid-", }), }, - "feature disabled valid volume mode": { - disableBlock: true, - isExpectedFailure: true, - volume: testVolume("foo", "", core.PersistentVolumeSpec{ - Capacity: core.ResourceList{ - core.ResourceName(core.ResourceStorage): resource.MustParse("10G"), - }, - AccessModes: []core.PersistentVolumeAccessMode{core.ReadWriteOnce}, - PersistentVolumeSource: core.PersistentVolumeSource{ - HostPath: &core.HostPathVolumeSource{ - Path: "/foo", - Type: newHostPathType(string(core.HostPathDirectory)), - }, - }, - StorageClassName: "valid", - VolumeMode: &validMode, - }), - }, "bad-hostpath-volume-backsteps": { isExpectedFailure: true, volume: testVolume("foo", "", core.PersistentVolumeSpec{ @@ -433,7 +414,6 @@ func TestValidatePersistentVolumes(t *testing.T) { for name, scenario := range scenarios { 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) @@ -823,7 +803,6 @@ func TestValidatePersistentVolumeClaim(t *testing.T) { scenarios := map[string]struct { isExpectedFailure bool claim *core.PersistentVolumeClaim - disableBlock bool }{ "good-claim": { isExpectedFailure: false, @@ -1018,31 +997,6 @@ func TestValidatePersistentVolumeClaim(t *testing.T) { StorageClassName: &invalidClassName, }), }, - "feature disabled valid volume mode": { - disableBlock: true, - isExpectedFailure: true, - claim: testVolumeClaim("foo", "ns", core.PersistentVolumeClaimSpec{ - Selector: &metav1.LabelSelector{ - MatchExpressions: []metav1.LabelSelectorRequirement{ - { - Key: "key2", - Operator: "Exists", - }, - }, - }, - AccessModes: []core.PersistentVolumeAccessMode{ - core.ReadWriteOnce, - core.ReadOnlyMany, - }, - Resources: core.ResourceRequirements{ - Requests: core.ResourceList{ - core.ResourceName(core.ResourceStorage): resource.MustParse("10G"), - }, - }, - StorageClassName: &validClassName, - VolumeMode: &validMode, - }), - }, "invalid-volume-mode": { isExpectedFailure: true, claim: testVolumeClaim("foo", "ns", core.PersistentVolumeClaimSpec{ @@ -1062,7 +1016,6 @@ func TestValidatePersistentVolumeClaim(t *testing.T) { for name, scenario := range scenarios { 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) @@ -1082,80 +1035,62 @@ func TestAlphaPVVolumeModeUpdate(t *testing.T) { isExpectedFailure bool oldPV *core.PersistentVolume newPV *core.PersistentVolume - enableBlock bool }{ "valid-update-volume-mode-block-to-block": { isExpectedFailure: false, oldPV: createTestVolModePV(&block), newPV: createTestVolModePV(&block), - enableBlock: true, }, "valid-update-volume-mode-file-to-file": { isExpectedFailure: false, oldPV: createTestVolModePV(&file), newPV: createTestVolModePV(&file), - enableBlock: true, }, "invalid-update-volume-mode-to-block": { isExpectedFailure: true, oldPV: createTestVolModePV(&file), newPV: createTestVolModePV(&block), - enableBlock: true, }, "invalid-update-volume-mode-to-file": { isExpectedFailure: true, oldPV: createTestVolModePV(&block), newPV: createTestVolModePV(&file), - enableBlock: true, - }, - "invalid-update-blocksupport-disabled": { - isExpectedFailure: true, - oldPV: createTestVolModePV(&block), - newPV: createTestVolModePV(&block), - enableBlock: false, }, "invalid-update-volume-mode-nil-to-file": { isExpectedFailure: true, oldPV: createTestVolModePV(nil), newPV: createTestVolModePV(&file), - enableBlock: true, }, "invalid-update-volume-mode-nil-to-block": { isExpectedFailure: true, oldPV: createTestVolModePV(nil), newPV: createTestVolModePV(&block), - enableBlock: true, }, "invalid-update-volume-mode-file-to-nil": { isExpectedFailure: true, oldPV: createTestVolModePV(&file), newPV: createTestVolModePV(nil), - enableBlock: true, }, "invalid-update-volume-mode-block-to-nil": { isExpectedFailure: true, oldPV: createTestVolModePV(&block), newPV: createTestVolModePV(nil), - enableBlock: true, }, "invalid-update-volume-mode-nil-to-nil": { isExpectedFailure: false, oldPV: createTestVolModePV(nil), newPV: createTestVolModePV(nil), - enableBlock: true, }, "invalid-update-volume-mode-empty-to-mode": { isExpectedFailure: true, oldPV: createTestPV(), newPV: createTestVolModePV(&block), - enableBlock: true, }, } for name, scenario := range scenarios { 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) @@ -1503,8 +1438,9 @@ func TestValidatePersistentVolumeClaimUpdate(t *testing.T) { enableResize: false, enableBlock: true, }, - "invalid-update-blocksupport-disabled": { - isExpectedFailure: true, + // with the new validation changes this test should not fail + "noop-update-volumemode-allowed": { + isExpectedFailure: false, oldClaim: validClaimVolumeModeFile, newClaim: validClaimVolumeModeFile, enableResize: false,