From e8b09d36440b81fe913aa66f1709269858764349 Mon Sep 17 00:00:00 2001 From: Ben Swartzlander Date: Wed, 4 Mar 2020 17:25:48 -0500 Subject: [PATCH] Add AnyVolumeDataSource feature gate Allow any custom resource to be the data source of a PVC, if the AnyVolumeDataSource feature gate is enabled. This is an alpha feature. --- api/openapi-spec/swagger.json | 2 +- pkg/api/persistentvolumeclaim/util.go | 4 + pkg/api/persistentvolumeclaim/util_test.go | 111 ++++++++++++++++++ pkg/apis/core/types.go | 9 +- pkg/apis/core/validation/BUILD | 1 - pkg/apis/core/validation/validation.go | 22 ++-- pkg/apis/core/validation/validation_test.go | 15 ++- pkg/features/kube_features.go | 7 ++ .../src/k8s.io/api/core/v1/generated.proto | 9 +- staging/src/k8s.io/api/core/v1/types.go | 9 +- .../core/v1/types_swagger_doc_generated.go | 2 +- 11 files changed, 155 insertions(+), 36 deletions(-) diff --git a/api/openapi-spec/swagger.json b/api/openapi-spec/swagger.json index 74415c164b0..97d155786dd 100644 --- a/api/openapi-spec/swagger.json +++ b/api/openapi-spec/swagger.json @@ -7832,7 +7832,7 @@ }, "dataSource": { "$ref": "#/definitions/io.k8s.api.core.v1.TypedLocalObjectReference", - "description": "This field can be used to specify either: * An existing VolumeSnapshot object (snapshot.storage.k8s.io/VolumeSnapshot - Beta) * An existing PVC (PersistentVolumeClaim) In order to use VolumeSnapshot object types, the appropriate feature gate must be enabled (VolumeSnapshotDataSource) 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." + "description": "This field can be used to specify either: * An existing VolumeSnapshot object (snapshot.storage.k8s.io/VolumeSnapshot - Beta) * 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) 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." }, "resources": { "$ref": "#/definitions/io.k8s.api.core.v1.ResourceRequirements", diff --git a/pkg/api/persistentvolumeclaim/util.go b/pkg/api/persistentvolumeclaim/util.go index 749b51f4883..435e9e1b3b8 100644 --- a/pkg/api/persistentvolumeclaim/util.go +++ b/pkg/api/persistentvolumeclaim/util.go @@ -47,6 +47,10 @@ func dataSourceInUse(oldPVCSpec *core.PersistentVolumeClaimSpec) bool { func dataSourceIsEnabled(pvcSpec *core.PersistentVolumeClaimSpec) bool { if pvcSpec.DataSource != nil { + if utilfeature.DefaultFeatureGate.Enabled(features.AnyVolumeDataSource) { + return true + } + apiGroup := "" if pvcSpec.DataSource.APIGroup != nil { apiGroup = *pvcSpec.DataSource.APIGroup diff --git a/pkg/api/persistentvolumeclaim/util_test.go b/pkg/api/persistentvolumeclaim/util_test.go index 4a13ba1bd72..6bb4647be73 100644 --- a/pkg/api/persistentvolumeclaim/util_test.go +++ b/pkg/api/persistentvolumeclaim/util_test.go @@ -71,6 +71,9 @@ func TestDropDisabledSnapshotDataSource(t *testing.T) { }, } + // 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 { @@ -169,6 +172,9 @@ func TestPVCDataSourceSpecFilter(t *testing.T) { }, } + // Ensure that any data sources aren't enabled for this test + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.AnyVolumeDataSource, false)() + for testName, test := range tests { t.Run(testName, func(t *testing.T) { DropDisabledFields(&test.spec, nil) @@ -181,3 +187,108 @@ func TestPVCDataSourceSpecFilter(t *testing.T) { } } + +// TestAnyDataSourceFilter checks to ensure the AnyVolumeDataSource feature gate works +func TestAnyDataSourceFilter(t *testing.T) { + makeDataSource := func(apiGroup, kind, name string) *core.TypedLocalObjectReference { + return &core.TypedLocalObjectReference{ + APIGroup: &apiGroup, + Kind: kind, + Name: name, + } + } + + volumeDataSource := makeDataSource("", "PersistentVolumeClaim", "my-vol") + snapshotDataSource := makeDataSource("snapshot.storage.k8s.io", "VolumeSnapshot", "my-snap") + genericDataSource := makeDataSource("generic.storage.k8s.io", "Generic", "my-foo") + + var tests = map[string]struct { + spec core.PersistentVolumeClaimSpec + snapshotEnabled bool + anyEnabled bool + want *core.TypedLocalObjectReference + }{ + "both disabled with empty ds": { + spec: core.PersistentVolumeClaimSpec{}, + want: nil, + }, + "both disabled with volume ds": { + spec: core.PersistentVolumeClaimSpec{DataSource: volumeDataSource}, + want: volumeDataSource, + }, + "both disabled with snapshot ds": { + spec: core.PersistentVolumeClaimSpec{DataSource: snapshotDataSource}, + want: nil, + }, + "both disabled with generic ds": { + spec: core.PersistentVolumeClaimSpec{DataSource: genericDataSource}, + want: nil, + }, + "any enabled with empty ds": { + spec: core.PersistentVolumeClaimSpec{}, + anyEnabled: true, + want: nil, + }, + "any enabled with volume ds": { + spec: core.PersistentVolumeClaimSpec{DataSource: volumeDataSource}, + anyEnabled: true, + want: volumeDataSource, + }, + "any enabled with snapshot ds": { + spec: core.PersistentVolumeClaimSpec{DataSource: snapshotDataSource}, + anyEnabled: true, + want: snapshotDataSource, + }, + "any enabled with generic ds": { + spec: core.PersistentVolumeClaimSpec{DataSource: genericDataSource}, + 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) + } + }) + } +} diff --git a/pkg/apis/core/types.go b/pkg/apis/core/types.go index e7252819206..59154ef770e 100644 --- a/pkg/apis/core/types.go +++ b/pkg/apis/core/types.go @@ -420,11 +420,12 @@ type PersistentVolumeClaimSpec struct { // This field can be used to specify either: // * An existing VolumeSnapshot object (snapshot.storage.k8s.io/VolumeSnapshot - Beta) // * 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) - // 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 + // must be enabled (VolumeSnapshotDataSource or AnyVolumeDataSource) + // 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. diff --git a/pkg/apis/core/validation/BUILD b/pkg/apis/core/validation/BUILD index 024c3632d9e..30f0bdcc3c2 100644 --- a/pkg/apis/core/validation/BUILD +++ b/pkg/apis/core/validation/BUILD @@ -33,7 +33,6 @@ go_library( "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/validation:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/labels:go_default_library", - "//staging/src/k8s.io/apimachinery/pkg/runtime/schema:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/diff:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/intstr:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library", diff --git a/pkg/apis/core/validation/validation.go b/pkg/apis/core/validation/validation.go index 5dfea2c69d5..ff1655a5dd5 100644 --- a/pkg/apis/core/validation/validation.go +++ b/pkg/apis/core/validation/validation.go @@ -38,7 +38,6 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" unversionedvalidation "k8s.io/apimachinery/pkg/apis/meta/v1/validation" "k8s.io/apimachinery/pkg/labels" - "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/util/diff" "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/apimachinery/pkg/util/sets" @@ -1563,11 +1562,6 @@ var supportedReclaimPolicy = sets.NewString(string(core.PersistentVolumeReclaimD var supportedVolumeModes = sets.NewString(string(core.PersistentVolumeBlock), string(core.PersistentVolumeFilesystem)) -var supportedDataSourceAPIGroupKinds = map[schema.GroupKind]bool{ - {Group: "snapshot.storage.k8s.io", Kind: "VolumeSnapshot"}: true, - {Group: "", Kind: "PersistentVolumeClaim"}: true, -} - func ValidatePersistentVolumeSpec(pvSpec *core.PersistentVolumeSpec, pvName string, validateInlinePersistentVolumeSpec bool, fldPath *field.Path) field.ErrorList { allErrs := field.ErrorList{} @@ -1929,17 +1923,15 @@ func ValidatePersistentVolumeClaimSpec(spec *core.PersistentVolumeClaimSpec, fld if len(spec.DataSource.Name) == 0 { allErrs = append(allErrs, field.Required(fldPath.Child("dataSource", "name"), "")) } - - groupKind := schema.GroupKind{Group: "", Kind: spec.DataSource.Kind} + if len(spec.DataSource.Kind) == 0 { + allErrs = append(allErrs, field.Required(fldPath.Child("dataSource", "kind"), "")) + } + apiGroup := "" if spec.DataSource.APIGroup != nil { - groupKind.Group = string(*spec.DataSource.APIGroup) + apiGroup = *spec.DataSource.APIGroup } - groupKindList := make([]string, 0, len(supportedDataSourceAPIGroupKinds)) - for grp := range supportedDataSourceAPIGroupKinds { - groupKindList = append(groupKindList, grp.String()) - } - if !supportedDataSourceAPIGroupKinds[groupKind] { - allErrs = append(allErrs, field.NotSupported(fldPath.Child("dataSource"), groupKind.String(), groupKindList)) + if len(apiGroup) == 0 && spec.DataSource.Kind != "PersistentVolumeClaim" { + allErrs = append(allErrs, field.Invalid(fldPath.Child("dataSource"), spec.DataSource.Kind, "")) } } diff --git a/pkg/apis/core/validation/validation_test.go b/pkg/apis/core/validation/validation_test.go index e2a87a5d407..231867181ab 100644 --- a/pkg/apis/core/validation/validation_test.go +++ b/pkg/apis/core/validation/validation_test.go @@ -904,8 +904,7 @@ func TestAlphaVolumeSnapshotDataSource(t *testing.T) { } failedTestCases := []core.PersistentVolumeClaimSpec{ *testVolumeSnapshotDataSourceInSpec("", "VolumeSnapshot", "snapshot.storage.k8s.io"), - *testVolumeSnapshotDataSourceInSpec("test_snapshot", "PersistentVolumeClaim", "snapshot.storage.k8s.io"), - *testVolumeSnapshotDataSourceInSpec("test_snapshot", "VolumeSnapshot", "storage.k8s.io"), + *testVolumeSnapshotDataSourceInSpec("test_snapshot", "", "snapshot.storage.k8s.io"), } for _, tc := range successTestCases { @@ -14864,13 +14863,17 @@ func TestAlphaVolumePVCDataSource(t *testing.T) { expectedFail: true, }, { - testName: "test specifying pvc with snapshot api group should fail", - claimSpec: *testDataSourceInSpec("test_snapshot", "PersistentVolumeClaim", "snapshot.storage.k8s.io"), + testName: "test missing kind in snapshot datasource should fail", + claimSpec: *testDataSourceInSpec("test_snapshot", "", "snapshot.storage.k8s.io"), expectedFail: true, }, { - testName: "test invalid group name in snapshot datasource should fail", - claimSpec: *testDataSourceInSpec("test_snapshot", "VolumeSnapshot", "storage.k8s.io"), + testName: "test create from valid generic custom resource source", + claimSpec: *testDataSourceInSpec("test_generic", "Generic", "generic.storage.k8s.io"), + }, + { + testName: "test invalid datasource should fail", + claimSpec: *testDataSourceInSpec("test_pod", "Pod", ""), expectedFail: true, }, } diff --git a/pkg/features/kube_features.go b/pkg/features/kube_features.go index d0aff78e9d7..ff761f44c77 100644 --- a/pkg/features/kube_features.go +++ b/pkg/features/kube_features.go @@ -564,6 +564,12 @@ const ( // e.g. emptyDir: // medium: HugePages-1Gi HugePageStorageMediumSize featuregate.Feature = "HugePageStorageMediumSize" + + // owner: @bswartz + // alpha: v1.18 + // + // Enables usage of any object for volume data source in PVCs + AnyVolumeDataSource featuregate.Feature = "AnyVolumeDataSource" ) func init() { @@ -652,6 +658,7 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS ImmutableEphemeralVolumes: {Default: false, PreRelease: featuregate.Alpha}, DefaultIngressClass: {Default: true, PreRelease: featuregate.Beta}, HugePageStorageMediumSize: {Default: false, PreRelease: featuregate.Alpha}, + AnyVolumeDataSource: {Default: false, PreRelease: featuregate.Alpha}, // inherited features from generic apiserver, relisted here to get a conflict if it is changed // unintentionally on either side: diff --git a/staging/src/k8s.io/api/core/v1/generated.proto b/staging/src/k8s.io/api/core/v1/generated.proto index e046346de40..8b569b260cb 100644 --- a/staging/src/k8s.io/api/core/v1/generated.proto +++ b/staging/src/k8s.io/api/core/v1/generated.proto @@ -2640,11 +2640,12 @@ message PersistentVolumeClaimSpec { // This field can be used to specify either: // * An existing VolumeSnapshot object (snapshot.storage.k8s.io/VolumeSnapshot - Beta) // * 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) - // 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 + // must be enabled (VolumeSnapshotDataSource or AnyVolumeDataSource) + // 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. diff --git a/staging/src/k8s.io/api/core/v1/types.go b/staging/src/k8s.io/api/core/v1/types.go index e54e2cf8403..75049c3bfcc 100644 --- a/staging/src/k8s.io/api/core/v1/types.go +++ b/staging/src/k8s.io/api/core/v1/types.go @@ -464,11 +464,12 @@ type PersistentVolumeClaimSpec struct { // This field can be used to specify either: // * An existing VolumeSnapshot object (snapshot.storage.k8s.io/VolumeSnapshot - Beta) // * 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) - // 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 + // must be enabled (VolumeSnapshotDataSource or AnyVolumeDataSource) + // 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. diff --git a/staging/src/k8s.io/api/core/v1/types_swagger_doc_generated.go b/staging/src/k8s.io/api/core/v1/types_swagger_doc_generated.go index a12aca71c2c..cbc4c932888 100644 --- a/staging/src/k8s.io/api/core/v1/types_swagger_doc_generated.go +++ b/staging/src/k8s.io/api/core/v1/types_swagger_doc_generated.go @@ -1301,7 +1301,7 @@ var map_PersistentVolumeClaimSpec = map[string]string{ "volumeName": "VolumeName is the binding reference to the PersistentVolume backing this claim.", "storageClassName": "Name of the StorageClass required by the claim. More info: https://kubernetes.io/docs/concepts/storage/persistent-volumes#class-1", "volumeMode": "volumeMode defines what type of volume is required by the claim. Value of Filesystem is implied when not included in claim spec.", - "dataSource": "This field can be used to specify either: * An existing VolumeSnapshot object (snapshot.storage.k8s.io/VolumeSnapshot - Beta) * An existing PVC (PersistentVolumeClaim) In order to use VolumeSnapshot object types, the appropriate feature gate must be enabled (VolumeSnapshotDataSource) 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.", + "dataSource": "This field can be used to specify either: * An existing VolumeSnapshot object (snapshot.storage.k8s.io/VolumeSnapshot - Beta) * 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) 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.", } func (PersistentVolumeClaimSpec) SwaggerDoc() map[string]string {