VolumeMode Validation and tests

Signed-off-by: Serguei Bezverkhi <sbezverk@cisco.com>

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
+}

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": {

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",
     ],

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
+}

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()))
+						}
+					}
+				})
+			}
+		}
 	}
 }

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",
     ],
 )

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
 	}

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)
+		}
 	}
 }
 

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
 }
 

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,