Move Snapshot to GA

pkg/api/persistentvolumeclaim/util.go

@@ -61,9 +61,7 @@ func dataSourceIsEnabled(pvcSpec *core.PersistentVolumeClaimSpec) bool {
 
 		}
 
-		if utilfeature.DefaultFeatureGate.Enabled(features.VolumeSnapshotDataSource) &&
-			pvcSpec.DataSource.Kind == volumeSnapshot &&
-			apiGroup == "snapshot.storage.k8s.io" {
+		if pvcSpec.DataSource.Kind == volumeSnapshot && apiGroup == "snapshot.storage.k8s.io" {
 			return true
 		}
 	}

pkg/api/persistentvolumeclaim/util_test.go

@@ -50,76 +50,51 @@ func TestDropDisabledSnapshotDataSource(t *testing.T) {
 	}
 
 	pvcInfo := []struct {
-		description   string
-		hasDataSource bool
-		pvc           func() *core.PersistentVolumeClaim
+		description string
+		pvc         func() *core.PersistentVolumeClaim
 	}{
 		{
-			description:   "pvc without DataSource",
-			hasDataSource: false,
-			pvc:           pvcWithoutDataSource,
+			description: "pvc without DataSource",
+			pvc:         pvcWithoutDataSource,
 		},
 		{
-			description:   "pvc with DataSource",
-			hasDataSource: true,
-			pvc:           pvcWithDataSource,
+			description: "pvc with DataSource",
+			pvc:         pvcWithDataSource,
 		},
 		{
-			description:   "is nil",
-			hasDataSource: false,
-			pvc:           func() *core.PersistentVolumeClaim { return nil },
+			description: "is nil",
+			pvc:         func() *core.PersistentVolumeClaim { return nil },
 		},
 	}
 
 	// Ensure that any data sources aren't enabled for this test
 	defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.AnyVolumeDataSource, false)()
 
-	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 featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.VolumeSnapshotDataSource, enabled)()
+	for _, oldpvcInfo := range pvcInfo {
+		for _, newpvcInfo := range pvcInfo {
+			oldpvc := oldpvcInfo.pvc()
+			newpvc := newpvcInfo.pvc()
+			if newpvc == nil {
+				continue
+			}
 
-					var oldpvcSpec *core.PersistentVolumeClaimSpec
-					if oldpvc != nil {
-						oldpvcSpec = &oldpvc.Spec
-					}
-					DropDisabledFields(&newpvc.Spec, oldpvcSpec)
+			t.Run(fmt.Sprintf("old pvc %v, new pvc %v", oldpvcInfo.description, newpvcInfo.description), func(t *testing.T) {
+				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()))
-					}
+				// 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()))
-						}
-					}
-				})
-			}
+				// new pvc should not be changed
+				if !reflect.DeepEqual(newpvc, newpvcInfo.pvc()) {
+					t.Errorf("new pvc changed: %v", diff.ObjectReflectDiff(newpvc, newpvcInfo.pvc()))
+				}
+			})
 		}
 	}
 }
@@ -203,24 +178,23 @@ func TestAnyDataSourceFilter(t *testing.T) {
 	genericDataSource := makeDataSource("generic.storage.k8s.io", "Generic", "my-foo")
 
 	var tests = map[string]struct {
-		spec            core.PersistentVolumeClaimSpec
-		snapshotEnabled bool
-		anyEnabled      bool
-		want            *core.TypedLocalObjectReference
+		spec       core.PersistentVolumeClaimSpec
+		anyEnabled bool
+		want       *core.TypedLocalObjectReference
 	}{
-		"both disabled with empty ds": {
+		"any disabled with empty ds": {
 			spec: core.PersistentVolumeClaimSpec{},
 			want: nil,
 		},
-		"both disabled with volume ds": {
+		"any disabled with volume ds": {
 			spec: core.PersistentVolumeClaimSpec{DataSource: volumeDataSource},
 			want: volumeDataSource,
 		},
-		"both disabled with snapshot ds": {
+		"any disabled with snapshot ds": {
 			spec: core.PersistentVolumeClaimSpec{DataSource: snapshotDataSource},
-			want: nil,
+			want: snapshotDataSource,
 		},
-		"both disabled with generic ds": {
+		"any disabled with generic ds": {
 			spec: core.PersistentVolumeClaimSpec{DataSource: genericDataSource},
 			want: nil,
 		},
@@ -244,50 +218,15 @@ func TestAnyDataSourceFilter(t *testing.T) {
 			anyEnabled: true,
 			want:       genericDataSource,
 		},
-		"snapshot enabled with snapshot ds": {
-			spec:            core.PersistentVolumeClaimSpec{DataSource: snapshotDataSource},
-			snapshotEnabled: true,
-			want:            snapshotDataSource,
-		},
-		"snapshot enabled with generic ds": {
-			spec:            core.PersistentVolumeClaimSpec{DataSource: genericDataSource},
-			snapshotEnabled: true,
-			want:            nil,
-		},
-		"both enabled with empty ds": {
-			spec:            core.PersistentVolumeClaimSpec{},
-			snapshotEnabled: true,
-			anyEnabled:      true,
-			want:            nil,
-		},
-		"both enabled with volume ds": {
-			spec:            core.PersistentVolumeClaimSpec{DataSource: volumeDataSource},
-			snapshotEnabled: true,
-			anyEnabled:      true,
-			want:            volumeDataSource,
-		},
-		"both enabled with snapshot ds": {
-			spec:            core.PersistentVolumeClaimSpec{DataSource: snapshotDataSource},
-			snapshotEnabled: true,
-			anyEnabled:      true,
-			want:            snapshotDataSource,
-		},
-		"both enabled with generic ds": {
-			spec:            core.PersistentVolumeClaimSpec{DataSource: genericDataSource},
-			snapshotEnabled: true,
-			anyEnabled:      true,
-			want:            genericDataSource,
-		},
 	}
 
 	for testName, test := range tests {
 		t.Run(testName, func(t *testing.T) {
-			defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.VolumeSnapshotDataSource, test.snapshotEnabled)()
 			defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.AnyVolumeDataSource, test.anyEnabled)()
 			DropDisabledFields(&test.spec, nil)
 			if test.spec.DataSource != test.want {
-				t.Errorf("expected condition was not met, test: %s, snapshotEnabled: %v, anyEnabled: %v, spec: %v, expected: %v",
-					testName, test.snapshotEnabled, test.anyEnabled, test.spec, test.want)
+				t.Errorf("expected condition was not met, test: %s, anyEnabled: %v, spec: %v, expected: %v",
+					testName, test.anyEnabled, test.spec, test.want)
 			}
 		})
 	}

pkg/apis/core/types.go

@@ -445,17 +445,13 @@ type PersistentVolumeClaimSpec struct {
 	// +optional
 	VolumeMode *PersistentVolumeMode
 	// This field can be used to specify either:
-	// * An existing VolumeSnapshot object (snapshot.storage.k8s.io/VolumeSnapshot - Beta)
+	// * An existing VolumeSnapshot object (snapshot.storage.k8s.io/VolumeSnapshot)
 	// * An existing PVC (PersistentVolumeClaim)
-	// * An existing custom resource/object that implements data population (Alpha)
-	// In order to use VolumeSnapshot object types, the appropriate feature gate
-	// must be enabled (VolumeSnapshotDataSource or AnyVolumeDataSource)
+	// * An existing custom resource that implements data population (Alpha)
+	// In order to use custom resource types that implement data population,
+	// the AnyVolumeDataSource feature gate must be enabled.
 	// If the provisioner or an external controller can support the specified data source,
 	// it will create a new volume based on the contents of the specified data source.
-	// If the specified data source is not supported, 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.
 	// +optional
 	DataSource *TypedLocalObjectReference
 }

pkg/features/kube_features.go

@@ -338,6 +338,7 @@ const (
 	// owner: @xing-yang
 	// alpha: v1.12
 	// beta: v1.17
+	// GA: v1.20
 	//
 	// Enable volume snapshot data source support.
 	VolumeSnapshotDataSource featuregate.Feature = "VolumeSnapshotDataSource"
@@ -718,7 +719,7 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS
 	RuntimeClass:                   {Default: true, PreRelease: featuregate.Beta},
 	NodeLease:                      {Default: true, PreRelease: featuregate.GA, LockToDefault: true},
 	SCTPSupport:                    {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.22
-	VolumeSnapshotDataSource:       {Default: true, PreRelease: featuregate.Beta},
+	VolumeSnapshotDataSource:       {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.21
 	ProcMountType:                  {Default: false, PreRelease: featuregate.Alpha},
 	TTLAfterFinished:               {Default: false, PreRelease: featuregate.Alpha},
 	KubeletPodResources:            {Default: true, PreRelease: featuregate.Beta},

staging/src/k8s.io/api/core/v1/types.go

@@ -489,17 +489,13 @@ type PersistentVolumeClaimSpec struct {
 	// +optional
 	VolumeMode *PersistentVolumeMode `json:"volumeMode,omitempty" protobuf:"bytes,6,opt,name=volumeMode,casttype=PersistentVolumeMode"`
 	// This field can be used to specify either:
-	// * An existing VolumeSnapshot object (snapshot.storage.k8s.io/VolumeSnapshot - Beta)
+	// * An existing VolumeSnapshot object (snapshot.storage.k8s.io/VolumeSnapshot)
 	// * An existing PVC (PersistentVolumeClaim)
-	// * An existing custom resource/object that implements data population (Alpha)
-	// In order to use VolumeSnapshot object types, the appropriate feature gate
-	// must be enabled (VolumeSnapshotDataSource or AnyVolumeDataSource)
+	// * An existing custom resource that implements data population (Alpha)
+	// In order to use custom resource types that implement data population,
+	// the AnyVolumeDataSource feature gate must be enabled.
 	// If the provisioner or an external controller can support the specified data source,
 	// it will create a new volume based on the contents of the specified data source.
-	// If the specified data source is not supported, 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.
 	// +optional
 	DataSource *TypedLocalObjectReference `json:"dataSource,omitempty" protobuf:"bytes,7,opt,name=dataSource"`
 }