Promote block volume features to GA

Author: jsafrane

pkg/api/persistentvolume/util.go

@@ -25,10 +25,6 @@ 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) && !volumeModeInUse(oldPVSpec) {
-		pvSpec.VolumeMode = nil
-	}
-
 	if !utilfeature.DefaultFeatureGate.Enabled(features.ExpandCSIVolumes) && !hasExpansionSecrets(oldPVSpec) {
 		if pvSpec.CSI != nil {
 			pvSpec.CSI.ControllerExpandSecretRef = nil
@@ -46,13 +42,3 @@ func hasExpansionSecrets(oldPVSpec *api.PersistentVolumeSpec) bool {
 	}
 	return false
 }
-
-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

@@ -28,68 +28,18 @@ import (
 )
 
 func TestDropDisabledFields(t *testing.T) {
-	specWithMode := func(mode *api.PersistentVolumeMode) *api.PersistentVolumeSpec {
-		return &api.PersistentVolumeSpec{VolumeMode: mode}
-	}
-
 	secretRef := &api.SecretReference{
 		Name:      "expansion-secret",
 		Namespace: "default",
 	}
 
-	modeBlock := api.PersistentVolumeBlock
-
 	tests := map[string]struct {
 		oldSpec             *api.PersistentVolumeSpec
 		newSpec             *api.PersistentVolumeSpec
 		expectOldSpec       *api.PersistentVolumeSpec
 		expectNewSpec       *api.PersistentVolumeSpec
-		blockEnabled        bool
 		csiExpansionEnabled bool
 	}{
-		"disabled block clears new": {
-			blockEnabled:  false,
-			newSpec:       specWithMode(&modeBlock),
-			expectNewSpec: specWithMode(nil),
-			oldSpec:       nil,
-			expectOldSpec: nil,
-		},
-		"disabled block clears update when old pv did not use block": {
-			blockEnabled:  false,
-			newSpec:       specWithMode(&modeBlock),
-			expectNewSpec: specWithMode(nil),
-			oldSpec:       specWithMode(nil),
-			expectOldSpec: specWithMode(nil),
-		},
-		"disabled block does not clear new on update when old pv did use block": {
-			blockEnabled:  false,
-			newSpec:       specWithMode(&modeBlock),
-			expectNewSpec: specWithMode(&modeBlock),
-			oldSpec:       specWithMode(&modeBlock),
-			expectOldSpec: specWithMode(&modeBlock),
-		},
-
-		"enabled block preserves new": {
-			blockEnabled:  true,
-			newSpec:       specWithMode(&modeBlock),
-			expectNewSpec: specWithMode(&modeBlock),
-			oldSpec:       nil,
-			expectOldSpec: nil,
-		},
-		"enabled block preserves update when old pv did not use block": {
-			blockEnabled:  true,
-			newSpec:       specWithMode(&modeBlock),
-			expectNewSpec: specWithMode(&modeBlock),
-			oldSpec:       specWithMode(nil),
-			expectOldSpec: specWithMode(nil),
-		},
-		"enabled block preserves update when old pv did use block": {
-			blockEnabled:  true,
-			newSpec:       specWithMode(&modeBlock),
-			expectNewSpec: specWithMode(&modeBlock),
-			oldSpec:       specWithMode(&modeBlock),
-			expectOldSpec: specWithMode(&modeBlock),
-		},
 		"disabled csi expansion clears secrets": {
 			csiExpansionEnabled: false,
 			newSpec:             specWithCSISecrets(secretRef),
@@ -129,7 +79,6 @@ func TestDropDisabledFields(t *testing.T) {
 
 	for name, tc := range tests {
 		t.Run(name, func(t *testing.T) {
-			defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.BlockVolume, tc.blockEnabled)()
 			defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.ExpandCSIVolumes, tc.csiExpansionEnabled)()
 
 			DropDisabledFields(tc.newSpec, tc.oldSpec)

pkg/api/persistentvolumeclaim/util.go

@@ -30,24 +30,11 @@ const (
 // 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) && !volumeModeInUse(oldPVCSpec) {
-		pvcSpec.VolumeMode = nil
-	}
 	if !dataSourceIsEnabled(pvcSpec) && !dataSourceInUse(oldPVCSpec) {
 		pvcSpec.DataSource = nil
 	}
 }
 
-func volumeModeInUse(oldPVCSpec *core.PersistentVolumeClaimSpec) bool {
-	if oldPVCSpec == nil {
-		return false
-	}
-	if oldPVCSpec.VolumeMode != nil {
-		return true
-	}
-	return false
-}
-
 func dataSourceInUse(oldPVCSpec *core.PersistentVolumeClaimSpec) bool {
 	if oldPVCSpec == nil {
 		return false

pkg/api/persistentvolumeclaim/util_test.go

@@ -28,96 +28,6 @@ import (
 	"k8s.io/kubernetes/pkg/features"
 )
 
-func TestDropAlphaPVCVolumeMode(t *testing.T) {
-	vmode := core.PersistentVolumeFilesystem
-
-	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 },
-		},
-	}
-
-	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
-				}
-
-				t.Run(fmt.Sprintf("feature enabled=%v, old pvc %v, new pvc %v", enabled, oldpvcInfo.description, newpvcInfo.description), func(t *testing.T) {
-					defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.BlockVolume, enabled)()
-
-					var oldpvcSpec *core.PersistentVolumeClaimSpec
-					if oldpvc != nil {
-						oldpvcSpec = &oldpvc.Spec
-					}
-					DropDisabledFields(&newpvc.Spec, oldpvcSpec)
-
-					// 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()))
-						}
-					}
-				})
-			}
-		}
-	}
-}
-
 func TestDropDisabledSnapshotDataSource(t *testing.T) {
 	pvcWithoutDataSource := func() *core.PersistentVolumeClaim {
 		return &core.PersistentVolumeClaim{

pkg/api/pod/util.go

@@ -385,8 +385,6 @@ func dropDisabledFields(
 		})
 	}
 
-	dropDisabledVolumeDevicesFields(podSpec, oldPodSpec)
-
 	dropDisabledRunAsGroupField(podSpec, oldPodSpec)
 
 	dropDisabledGMSAFields(podSpec, oldPodSpec)
@@ -484,17 +482,6 @@ func dropDisabledProcMountField(podSpec, oldPodSpec *api.PodSpec) {
 	}
 }
 
-// dropDisabledVolumeDevicesFields removes disabled fields from []VolumeDevice if it has not been already populated.
-// This should be called from PrepareForCreate/PrepareForUpdate for all resources containing a VolumeDevice
-func dropDisabledVolumeDevicesFields(podSpec, oldPodSpec *api.PodSpec) {
-	if !utilfeature.DefaultFeatureGate.Enabled(features.BlockVolume) && !volumeDevicesInUse(oldPodSpec) {
-		VisitContainers(podSpec, func(c *api.Container) bool {
-			c.VolumeDevices = nil
-			return true
-		})
-	}
-}
-
 // dropDisabledCSIVolumeSourceAlphaFields removes disabled alpha fields from []CSIVolumeSource.
 // This should be called from PrepareForCreate/PrepareForUpdate for all pod specs resources containing a CSIVolumeSource
 func dropDisabledCSIVolumeSourceAlphaFields(podSpec, oldPodSpec *api.PodSpec) {
@@ -646,24 +633,6 @@ func emptyDirSizeLimitInUse(podSpec *api.PodSpec) bool {
 	return false
 }
 
-// volumeDevicesInUse returns true if the pod spec is non-nil and has VolumeDevices set.
-func volumeDevicesInUse(podSpec *api.PodSpec) bool {
-	if podSpec == nil {
-		return false
-	}
-
-	var inUse bool
-	VisitContainers(podSpec, func(c *api.Container) bool {
-		if c.VolumeDevices != nil {
-			inUse = true
-			return false
-		}
-		return true
-	})
-
-	return inUse
-}
-
 // runAsGroupInUse returns true if the pod spec is non-nil and has a SecurityContext's RunAsGroup field set
 func runAsGroupInUse(podSpec *api.PodSpec) bool {
 	if podSpec == nil {