diff --git a/pkg/api/persistentvolume/util.go b/pkg/api/persistentvolume/util.go index 0d3a79889c..0ca16f932a 100644 --- a/pkg/api/persistentvolume/util.go +++ b/pkg/api/persistentvolume/util.go @@ -28,7 +28,9 @@ func DropDisabledFields(pvSpec *api.PersistentVolumeSpec, oldPVSpec *api.Persist if !utilfeature.DefaultFeatureGate.Enabled(features.BlockVolume) && !volumeModeInUse(oldPVSpec) { pvSpec.VolumeMode = nil } - + if !utilfeature.DefaultFeatureGate.Enabled(features.PersistentLocalVolumes) && !persistentLocalVolumesInUse(oldPVSpec) { + pvSpec.PersistentVolumeSource.Local = nil + } if !utilfeature.DefaultFeatureGate.Enabled(features.CSIPersistentVolume) { // if this is a new PV, or the old PV didn't already have the CSI field, clear it if oldPVSpec == nil || oldPVSpec.PersistentVolumeSource.CSI == nil { @@ -46,3 +48,13 @@ func volumeModeInUse(oldPVSpec *api.PersistentVolumeSpec) bool { } return false } + +func persistentLocalVolumesInUse(oldPVSpec *api.PersistentVolumeSpec) bool { + if oldPVSpec == nil { + return false + } + if oldPVSpec.PersistentVolumeSource.Local != nil { + return true + } + return false +} diff --git a/pkg/api/persistentvolume/util_test.go b/pkg/api/persistentvolume/util_test.go index 3b31f298c1..4c5ec05603 100644 --- a/pkg/api/persistentvolume/util_test.go +++ b/pkg/api/persistentvolume/util_test.go @@ -17,6 +17,7 @@ limitations under the License. package persistentvolume import ( + "fmt" "reflect" "testing" @@ -152,3 +153,99 @@ func TestDropDisabledFields(t *testing.T) { }) } } + +func TestDropDisabledFieldsPersistentLocalVolume(t *testing.T) { + pvWithoutLocalVolume := func() *api.PersistentVolume { + return &api.PersistentVolume{ + Spec: api.PersistentVolumeSpec{ + PersistentVolumeSource: api.PersistentVolumeSource{ + Local: nil, + }, + }, + } + } + pvWithLocalVolume := func() *api.PersistentVolume { + fsType := "ext4" + return &api.PersistentVolume{ + Spec: api.PersistentVolumeSpec{ + PersistentVolumeSource: api.PersistentVolumeSource{ + Local: &api.LocalVolumeSource{ + Path: "/a/b/c", + FSType: &fsType, + }, + }, + }, + } + } + + pvInfo := []struct { + description string + hasLocalVolume bool + pv func() *api.PersistentVolume + }{ + { + description: "pv without LocalVolume", + hasLocalVolume: false, + pv: pvWithoutLocalVolume, + }, + { + description: "pv with LocalVolume", + hasLocalVolume: true, + pv: pvWithLocalVolume, + }, + { + description: "is nil", + hasLocalVolume: false, + pv: func() *api.PersistentVolume { return nil }, + }, + } + + for _, enabled := range []bool{true, false} { + for _, oldpvInfo := range pvInfo { + for _, newpvInfo := range pvInfo { + oldpvHasLocalVolume, oldpv := oldpvInfo.hasLocalVolume, oldpvInfo.pv() + newpvHasLocalVolume, newpv := newpvInfo.hasLocalVolume, newpvInfo.pv() + if newpv == nil { + continue + } + + t.Run(fmt.Sprintf("feature enabled=%v, old pvc %v, new pvc %v", enabled, oldpvInfo.description, newpvInfo.description), func(t *testing.T) { + defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.PersistentLocalVolumes, enabled)() + + var oldpvSpec *api.PersistentVolumeSpec + if oldpv != nil { + oldpvSpec = &oldpv.Spec + } + DropDisabledFields(&newpv.Spec, oldpvSpec) + + // old pv should never be changed + if !reflect.DeepEqual(oldpv, oldpvInfo.pv()) { + t.Errorf("old pv changed: %v", diff.ObjectReflectDiff(oldpv, oldpvInfo.pv())) + } + + switch { + case enabled || oldpvHasLocalVolume: + // new pv should not be changed if the feature is enabled, or if the old pv had LocalVolume source + if !reflect.DeepEqual(newpv, newpvInfo.pv()) { + t.Errorf("new pv changed: %v", diff.ObjectReflectDiff(newpv, newpvInfo.pv())) + } + case newpvHasLocalVolume: + // new pv should be changed + if reflect.DeepEqual(newpv, newpvInfo.pv()) { + t.Errorf("new pv was not changed") + } + // new pv should not have LocalVolume + if !reflect.DeepEqual(newpv, pvWithoutLocalVolume()) { + t.Errorf("new pv had LocalVolume source: %v", diff.ObjectReflectDiff(newpv, pvWithoutLocalVolume())) + } + default: + // new pv should not need to be changed + if !reflect.DeepEqual(newpv, newpvInfo.pv()) { + t.Errorf("new pv changed: %v", diff.ObjectReflectDiff(newpv, newpvInfo.pv())) + } + } + }) + } + } + } +} diff --git a/pkg/apis/core/validation/validation.go b/pkg/apis/core/validation/validation.go index 44da6a3bd9..790351dde4 100644 --- a/pkg/apis/core/validation/validation.go +++ b/pkg/apis/core/validation/validation.go @@ -1714,9 +1714,6 @@ func ValidatePersistentVolume(pv *core.PersistentVolume) field.ErrorList { allErrs = append(allErrs, field.Forbidden(specPath.Child("local"), "may not specify more than 1 volume type")) } else { numVolumes++ - if !utilfeature.DefaultFeatureGate.Enabled(features.PersistentLocalVolumes) { - allErrs = append(allErrs, field.Forbidden(specPath.Child("local"), "Local volumes are disabled by feature-gate")) - } allErrs = append(allErrs, validateLocalVolumeSource(pv.Spec.Local, specPath.Child("local"))...) // NodeAffinity is required diff --git a/pkg/apis/core/validation/validation_test.go b/pkg/apis/core/validation/validation_test.go index df88524842..f7fe318db0 100644 --- a/pkg/apis/core/validation/validation_test.go +++ b/pkg/apis/core/validation/validation_test.go @@ -553,32 +553,6 @@ func TestValidateLocalVolumes(t *testing.T) { } } -func TestValidateLocalVolumesDisabled(t *testing.T) { - scenarios := map[string]struct { - isExpectedFailure bool - volume *core.PersistentVolume - }{ - "feature disabled valid local volume": { - isExpectedFailure: true, - volume: testVolume("valid-local-volume", "", - testLocalVolume("/foo", simpleVolumeNodeAffinity("foo", "bar"))), - }, - } - - for name, scenario := range scenarios { - t.Run(name+" PersistentLocalVolumes disabled", func(t *testing.T) { - defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.PersistentLocalVolumes, 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) - } - }) - } -} - func testVolumeWithNodeAffinity(affinity *core.VolumeNodeAffinity) *core.PersistentVolume { return testVolume("test-affinity-volume", "", core.PersistentVolumeSpec{