Merge pull request kubernetes#76913 from j-griffith/cleaned_enable_pvc_datasource

Cleaned enable pvc datasource

Author: k8s-ci-robot

pkg/api/persistentvolumeclaim/util.go

@@ -22,13 +22,18 @@ import (
 	"k8s.io/kubernetes/pkg/features"
 )
 
+const (
+	pvc            string = "PersistentVolumeClaim"
+	volumeSnapshot string = "VolumeSnapshot"
+)
+
 // 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 !utilfeature.DefaultFeatureGate.Enabled(features.VolumeSnapshotDataSource) && !volumeSnapshotDataSourceInUse(oldPVCSpec) {
+	if !dataSourceIsEnabled(pvcSpec) && !dataSourceInUse(oldPVCSpec) {
 		pvcSpec.DataSource = nil
 	}
 }
@@ -43,7 +48,7 @@ func volumeModeInUse(oldPVCSpec *core.PersistentVolumeClaimSpec) bool {
 	return false
 }
 
-func volumeSnapshotDataSourceInUse(oldPVCSpec *core.PersistentVolumeClaimSpec) bool {
+func dataSourceInUse(oldPVCSpec *core.PersistentVolumeClaimSpec) bool {
 	if oldPVCSpec == nil {
 		return false
 	}
@@ -52,3 +57,20 @@ func volumeSnapshotDataSourceInUse(oldPVCSpec *core.PersistentVolumeClaimSpec) b
 	}
 	return false
 }
+
+func dataSourceIsEnabled(pvcSpec *core.PersistentVolumeClaimSpec) bool {
+	if pvcSpec.DataSource != nil {
+		if pvcSpec.DataSource.Kind == pvc &&
+			*pvcSpec.DataSource.APIGroup == "" &&
+			utilfeature.DefaultFeatureGate.Enabled(features.VolumePVCDataSource) {
+			return true
+
+		}
+		if pvcSpec.DataSource.Kind == volumeSnapshot &&
+			*pvcSpec.DataSource.APIGroup == "snapshot.storage.k8s.io" &&
+			utilfeature.DefaultFeatureGate.Enabled(features.VolumeSnapshotDataSource) {
+			return true
+		}
+	}
+	return false
+}

pkg/api/persistentvolumeclaim/util_test.go

@@ -118,7 +118,7 @@ func TestDropAlphaPVCVolumeMode(t *testing.T) {
 	}
 }
 
-func TestDropDisabledDataSource(t *testing.T) {
+func TestDropDisabledSnapshotDataSource(t *testing.T) {
 	pvcWithoutDataSource := func() *core.PersistentVolumeClaim {
 		return &core.PersistentVolumeClaim{
 			Spec: core.PersistentVolumeClaimSpec{
@@ -210,3 +210,74 @@ func TestDropDisabledDataSource(t *testing.T) {
 		}
 	}
 }
+
+// TestPVCDataSourceSpecFilter checks to ensure the DropDisabledFields function behaves correctly for PVCDataSource featuregate
+func TestPVCDataSourceSpecFilter(t *testing.T) {
+	apiGroup := ""
+	validSpec := core.PersistentVolumeClaimSpec{
+		DataSource: &core.TypedLocalObjectReference{
+			APIGroup: &apiGroup,
+			Kind:     "PersistentVolumeClaim",
+			Name:     "test_clone",
+		},
+	}
+
+	invalidAPIGroup := "invalid.pvc.api.group"
+	invalidSpec := core.PersistentVolumeClaimSpec{
+		DataSource: &core.TypedLocalObjectReference{
+			APIGroup: &invalidAPIGroup,
+			Kind:     "PersistentVolumeClaim",
+			Name:     "test_clone_invalid",
+		},
+	}
+
+	var tests = map[string]struct {
+		spec        core.PersistentVolumeClaimSpec
+		gateEnabled bool
+		want        *core.TypedLocalObjectReference
+	}{
+		"enabled with empty ds": {
+			spec:        core.PersistentVolumeClaimSpec{},
+			gateEnabled: true,
+			want:        nil,
+		},
+		"enabled with invalid spec": {
+			spec:        invalidSpec,
+			gateEnabled: true,
+			want:        nil,
+		},
+		"enabled with valid spec": {
+			spec:        validSpec,
+			gateEnabled: true,
+			want:        validSpec.DataSource,
+		},
+		"disabled with invalid spec": {
+			spec:        invalidSpec,
+			gateEnabled: false,
+			want:        nil,
+		},
+		"disabled with valid spec": {
+			spec:        validSpec,
+			gateEnabled: false,
+			want:        nil,
+		},
+		"diabled with empty ds": {
+			spec:        core.PersistentVolumeClaimSpec{},
+			gateEnabled: false,
+			want:        nil,
+		},
+	}
+
+	for testName, test := range tests {
+		t.Run(testName, func(t *testing.T) {
+			defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.VolumePVCDataSource, test.gateEnabled)()
+			DropDisabledFields(&test.spec, nil)
+			if test.spec.DataSource != test.want {
+				t.Errorf("expected drop datasource condition was not met, test: %s, gateEnabled: %v, spec: %v, expected: %v", testName, test.gateEnabled, test.spec, test.want)
+			}
+
+		})
+
+	}
+
+}

pkg/apis/core/types.go

@@ -413,11 +413,14 @@ type PersistentVolumeClaimSpec struct {
 	// This is a beta feature.
 	// +optional
 	VolumeMode *PersistentVolumeMode
-	// This field requires the VolumeSnapshotDataSource alpha feature gate to be
-	// enabled and currently VolumeSnapshot is the only supported data source.
-	// If the provisioner can support VolumeSnapshot data source, it will create
-	// a new volume and data will be restored to the volume at the same time.
-	// If the provisioner does not support VolumeSnapshot data source, volume will
+	// This field can be used to specify either:
+	// * An existing VolumeSnapshot object (snapshot.storage.k8s.io/VolumeSnapshot)
+	// * An existing PVC (PersistentVolumeClaim)
+	// In order to use either of these DataSource types, the appropriate feature gate
+	// must be enabled (VolumeSnapshotDataSource, VolumePVCDataSource)
+	// If the provisioner can support the specified data source, it will create
+	// a new volume based on the contents of the specified PVC or Snapshot.
+	// If the provisioner does not support the specified data source, the volume will
 	// not be created and the failure will be reported as an event.
 	// In the future, we plan to support more data source types and the behavior
 	// of the provisioner may change.

pkg/apis/core/validation/validation.go

@@ -1537,6 +1537,7 @@ var supportedVolumeModes = sets.NewString(string(core.PersistentVolumeBlock), st
 
 var supportedDataSourceAPIGroupKinds = map[schema.GroupKind]bool{
 	{Group: "snapshot.storage.k8s.io", Kind: "VolumeSnapshot"}: true,
+	{Group: "", Kind: "PersistentVolumeClaim"}:                 true,
 }
 
 func ValidatePersistentVolume(pv *core.PersistentVolume) field.ErrorList {

pkg/apis/core/validation/validation_test.go

@@ -13288,3 +13288,72 @@ func TestValidateWindowsSecurityContextOptions(t *testing.T) {
 		})
 	}
 }
+
+func testDataSourceInSpec(name string, kind string, apiGroup string) *core.PersistentVolumeClaimSpec {
+	scName := "csi-plugin"
+	dataSourceInSpec := core.PersistentVolumeClaimSpec{
+		AccessModes: []core.PersistentVolumeAccessMode{
+			core.ReadOnlyMany,
+		},
+		Resources: core.ResourceRequirements{
+			Requests: core.ResourceList{
+				core.ResourceName(core.ResourceStorage): resource.MustParse("10G"),
+			},
+		},
+		StorageClassName: &scName,
+		DataSource: &core.TypedLocalObjectReference{
+			APIGroup: &apiGroup,
+			Kind:     kind,
+			Name:     name,
+		},
+	}
+
+	return &dataSourceInSpec
+}
+
+func TestAlphaVolumePVCDataSource(t *testing.T) {
+
+	testCases := []struct {
+		testName     string
+		claimSpec    core.PersistentVolumeClaimSpec
+		expectedFail bool
+	}{
+		{
+			testName:  "test create from valid snapshot source",
+			claimSpec: *testDataSourceInSpec("test_snapshot", "VolumeSnapshot", "snapshot.storage.k8s.io"),
+		},
+		{
+			testName:  "test create from valid pvc source",
+			claimSpec: *testDataSourceInSpec("test_pvc", "PersistentVolumeClaim", ""),
+		},
+		{
+			testName:     "test missing name in snapshot datasource should fail",
+			claimSpec:    *testDataSourceInSpec("", "VolumeSnapshot", "snapshot.storage.k8s.io"),
+			expectedFail: true,
+		},
+		{
+			testName:     "test specifying pvc with snapshot api group should fail",
+			claimSpec:    *testDataSourceInSpec("test_snapshot", "PersistentVolumeClaim", "snapshot.storage.k8s.io"),
+			expectedFail: true,
+		},
+		{
+			testName:     "test invalid group name in snapshot datasource should fail",
+			claimSpec:    *testDataSourceInSpec("test_snapshot", "VolumeSnapshot", "storage.k8s.io"),
+			expectedFail: true,
+		},
+	}
+
+	for _, tc := range testCases {
+		if tc.expectedFail {
+			if errs := ValidatePersistentVolumeClaimSpec(&tc.claimSpec, field.NewPath("spec")); len(errs) == 0 {
+				t.Errorf("expected failure: %v", errs)
+			}
+
+		} else {
+			if errs := ValidatePersistentVolumeClaimSpec(&tc.claimSpec, field.NewPath("spec")); len(errs) != 0 {
+				t.Errorf("expected success: %v", errs)
+			}
+
+		}
+	}
+}

pkg/features/kube_features.go

@@ -465,6 +465,12 @@ const (
 	//
 	// Enables NonPreempting option for priorityClass and pod.
 	NonPreemptingPriority featuregate.Feature = "NonPreemptingPriority"
+
+	// owner: @j-griffith
+	// alpha: v1.15
+	//
+	// Enable support for specifying an existing PVC as a DataSource
+	VolumePVCDataSource featuregate.Feature = "VolumePVCDataSource"
 )
 
 func init() {
@@ -543,6 +549,7 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS
 	WindowsGMSA:                                 {Default: false, PreRelease: featuregate.Alpha},
 	LocalStorageCapacityIsolationFSQuotaMonitoring: {Default: false, PreRelease: featuregate.Alpha},
 	NonPreemptingPriority:                          {Default: false, PreRelease: featuregate.Alpha},
+	VolumePVCDataSource:                            {Default: false, PreRelease: featuregate.Alpha},
 
 	// inherited features from generic apiserver, relisted here to get a conflict if it is changed
 	// unintentionally on either side: