diff --git a/pkg/api/persistentvolumeclaim/util.go b/pkg/api/persistentvolumeclaim/util.go index e55c02fa30..0c7116de5e 100644 --- a/pkg/api/persistentvolumeclaim/util.go +++ b/pkg/api/persistentvolumeclaim/util.go @@ -28,6 +28,9 @@ func DropDisabledFields(pvcSpec, oldPVCSpec *core.PersistentVolumeClaimSpec) { if !utilfeature.DefaultFeatureGate.Enabled(features.BlockVolume) && !volumeModeInUse(oldPVCSpec) { pvcSpec.VolumeMode = nil } + if !utilfeature.DefaultFeatureGate.Enabled(features.VolumeSnapshotDataSource) && !volumeSnapshotDataSourceInUse(oldPVCSpec) { + pvcSpec.DataSource = nil + } } func volumeModeInUse(oldPVCSpec *core.PersistentVolumeClaimSpec) bool { @@ -39,3 +42,13 @@ func volumeModeInUse(oldPVCSpec *core.PersistentVolumeClaimSpec) bool { } return false } + +func volumeSnapshotDataSourceInUse(oldPVCSpec *core.PersistentVolumeClaimSpec) bool { + if oldPVCSpec == nil { + return false + } + if oldPVCSpec.DataSource != nil { + return true + } + return false +} diff --git a/pkg/api/persistentvolumeclaim/util_test.go b/pkg/api/persistentvolumeclaim/util_test.go index d5242c03a8..5f8ec27014 100644 --- a/pkg/api/persistentvolumeclaim/util_test.go +++ b/pkg/api/persistentvolumeclaim/util_test.go @@ -117,3 +117,96 @@ func TestDropAlphaPVCVolumeMode(t *testing.T) { } } } + +func TestDropDisabledDataSource(t *testing.T) { + pvcWithoutDataSource := func() *core.PersistentVolumeClaim { + return &core.PersistentVolumeClaim{ + Spec: core.PersistentVolumeClaimSpec{ + DataSource: nil, + }, + } + } + apiGroup := "snapshot.storage.k8s.io" + pvcWithDataSource := func() *core.PersistentVolumeClaim { + return &core.PersistentVolumeClaim{ + Spec: core.PersistentVolumeClaimSpec{ + DataSource: &core.TypedLocalObjectReference{ + APIGroup: &apiGroup, + Kind: "VolumeSnapshot", + Name: "test_snapshot", + }, + }, + } + } + + pvcInfo := []struct { + description string + hasDataSource bool + pvc func() *core.PersistentVolumeClaim + }{ + { + description: "pvc without DataSource", + hasDataSource: false, + pvc: pvcWithoutDataSource, + }, + { + description: "pvc with DataSource", + hasDataSource: true, + pvc: pvcWithDataSource, + }, + { + description: "is nil", + hasDataSource: false, + pvc: func() *core.PersistentVolumeClaim { return nil }, + }, + } + + for _, enabled := range []bool{true, false} { + for _, oldpvcInfo := range pvcInfo { + for _, newpvcInfo := range pvcInfo { + oldPvcHasDataSource, oldpvc := oldpvcInfo.hasDataSource, oldpvcInfo.pvc() + newPvcHasDataSource, newpvc := newpvcInfo.hasDataSource, 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 utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.VolumeSnapshotDataSource, 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 || oldPvcHasDataSource: + // new pvc should not be changed if the feature is enabled, or if the old pvc had DataSource + if !reflect.DeepEqual(newpvc, newpvcInfo.pvc()) { + t.Errorf("new pvc changed: %v", diff.ObjectReflectDiff(newpvc, newpvcInfo.pvc())) + } + case newPvcHasDataSource: + // new pvc should be changed + if reflect.DeepEqual(newpvc, newpvcInfo.pvc()) { + t.Errorf("new pvc was not changed") + } + // new pvc should not have DataSource + if !reflect.DeepEqual(newpvc, pvcWithoutDataSource()) { + t.Errorf("new pvc had DataSource: %v", diff.ObjectReflectDiff(newpvc, pvcWithoutDataSource())) + } + 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/validation/validation.go b/pkg/apis/core/validation/validation.go index cfd9bd7493..1de05adab4 100644 --- a/pkg/apis/core/validation/validation.go +++ b/pkg/apis/core/validation/validation.go @@ -1834,9 +1834,7 @@ func ValidatePersistentVolumeClaimSpec(spec *core.PersistentVolumeClaimSpec, fld allErrs = append(allErrs, field.NotSupported(fldPath.Child("volumeMode"), *spec.VolumeMode, supportedVolumeModes.List())) } - if spec.DataSource != nil && !utilfeature.DefaultFeatureGate.Enabled(features.VolumeSnapshotDataSource) { - allErrs = append(allErrs, field.Forbidden(fldPath.Child("dataSource"), "VolumeSnapshotDataSource is disabled by feature-gate")) - } else if spec.DataSource != nil { + if spec.DataSource != nil { if len(spec.DataSource.Name) == 0 { allErrs = append(allErrs, field.Required(fldPath.Child("dataSource", "name"), "")) } diff --git a/pkg/apis/core/validation/validation_test.go b/pkg/apis/core/validation/validation_test.go index 03769ca0d2..2d9229847f 100644 --- a/pkg/apis/core/validation/validation_test.go +++ b/pkg/apis/core/validation/validation_test.go @@ -749,8 +749,6 @@ func TestAlphaVolumeSnapshotDataSource(t *testing.T) { *testVolumeSnapshotDataSourceInSpec("test_snapshot", "VolumeSnapshot", "storage.k8s.io"), } - // Enable alpha feature VolumeSnapshotDataSource - defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.VolumeSnapshotDataSource, true)() for _, tc := range successTestCases { if errs := ValidatePersistentVolumeClaimSpec(&tc, field.NewPath("spec")); len(errs) != 0 { t.Errorf("expected success: %v", errs) @@ -761,13 +759,6 @@ func TestAlphaVolumeSnapshotDataSource(t *testing.T) { t.Errorf("expected failure: %v", errs) } } - // Disable alpha feature VolumeSnapshotDataSource - defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.VolumeSnapshotDataSource, false)() - for _, tc := range successTestCases { - if errs := ValidatePersistentVolumeClaimSpec(&tc, field.NewPath("spec")); len(errs) == 0 { - t.Errorf("expected failure: %v", errs) - } - } } func testVolumeClaimStorageClassInAnnotationAndSpec(name, namespace, scNameInAnn, scName string, spec core.PersistentVolumeClaimSpec) *core.PersistentVolumeClaim {