From c34309acdf1813961051023e913e07ec7a535428 Mon Sep 17 00:00:00 2001 From: Deep Debroy Date: Thu, 30 May 2019 09:34:47 +0000 Subject: [PATCH] API changes to support CSI migration of inline volumes Signed-off-by: Deep Debroy --- api/openapi-spec/swagger.json | 12 + pkg/api/testing/defaulting_test.go | 4 + pkg/apis/core/v1/conversion.go | 12 +- pkg/apis/core/validation/validation.go | 452 ++++++++++-------- pkg/apis/core/validation/validation_test.go | 141 ++++++ pkg/apis/storage/types.go | 13 +- pkg/apis/storage/v1alpha1/BUILD | 3 + pkg/apis/storage/validation/validation.go | 26 +- .../storage/validation/validation_test.go | 193 +++++++- pkg/registry/storage/volumeattachment/BUILD | 7 + .../storage/volumeattachment/strategy.go | 19 +- .../storage/volumeattachment/strategy_test.go | 64 +++ .../src/k8s.io/api/storage/v1alpha1/types.go | 14 +- .../src/k8s.io/api/storage/v1beta1/types.go | 9 +- 14 files changed, 738 insertions(+), 231 deletions(-) diff --git a/api/openapi-spec/swagger.json b/api/openapi-spec/swagger.json index 189dce84756..b84b54288f3 100644 --- a/api/openapi-spec/swagger.json +++ b/api/openapi-spec/swagger.json @@ -15638,6 +15638,10 @@ "io.k8s.api.storage.v1.VolumeAttachmentSource": { "description": "VolumeAttachmentSource represents a volume that should be attached. Right now only PersistenVolumes can be attached via external attacher, in future we may allow also inline volumes in pods. Exactly one member can be set.", "properties": { + "inlineVolumeSpec": { + "$ref": "#/definitions/io.k8s.api.core.v1.PersistentVolumeSpec", + "description": "inlineVolumeSpec contains all the information necessary to attach a persistent volume defined by a pod's inline VolumeSource. This field is populated only for the CSIMigration feature. It contains translated fields from a pod's inline VolumeSource to a PersistentVolumeSpec. This field is alpha-level and is only honored by servers that enabled the CSIMigration feature." + }, "persistentVolumeName": { "description": "Name of the persistent volume to attach.", "type": "string" @@ -15784,6 +15788,10 @@ "io.k8s.api.storage.v1alpha1.VolumeAttachmentSource": { "description": "VolumeAttachmentSource represents a volume that should be attached. Right now only PersistenVolumes can be attached via external attacher, in future we may allow also inline volumes in pods. Exactly one member can be set.", "properties": { + "inlineVolumeSpec": { + "$ref": "#/definitions/io.k8s.api.core.v1.PersistentVolumeSpec", + "description": "inlineVolumeSpec contains all the information necessary to attach a persistent volume defined by a pod's inline VolumeSource. This field is populated only for the CSIMigration feature. It contains translated fields from a pod's inline VolumeSource to a PersistentVolumeSpec. This field is alpha-level and is only honored by servers that enabled the CSIMigration feature." + }, "persistentVolumeName": { "description": "Name of the persistent volume to attach.", "type": "string" @@ -16221,6 +16229,10 @@ "io.k8s.api.storage.v1beta1.VolumeAttachmentSource": { "description": "VolumeAttachmentSource represents a volume that should be attached. Right now only PersistenVolumes can be attached via external attacher, in future we may allow also inline volumes in pods. Exactly one member can be set.", "properties": { + "inlineVolumeSpec": { + "$ref": "#/definitions/io.k8s.api.core.v1.PersistentVolumeSpec", + "description": "inlineVolumeSpec contains all the information necessary to attach a persistent volume defined by a pod's inline VolumeSource. This field is populated only for the CSIMigration feature. It contains translated fields from a pod's inline VolumeSource to a PersistentVolumeSpec. This field is alpha-level and is only honored by servers that enabled the CSIMigration feature." + }, "persistentVolumeName": { "description": "Name of the persistent volume to attach.", "type": "string" diff --git a/pkg/api/testing/defaulting_test.go b/pkg/api/testing/defaulting_test.go index 32995e4bab5..00d5bfefe12 100644 --- a/pkg/api/testing/defaulting_test.go +++ b/pkg/api/testing/defaulting_test.go @@ -146,6 +146,10 @@ func TestDefaulting(t *testing.T) { {Group: "storage.k8s.io", Version: "v1beta1", Kind: "CSIDriverList"}: {}, {Group: "storage.k8s.io", Version: "v1", Kind: "StorageClass"}: {}, {Group: "storage.k8s.io", Version: "v1", Kind: "StorageClassList"}: {}, + {Group: "storage.k8s.io", Version: "v1", Kind: "VolumeAttachment"}: {}, + {Group: "storage.k8s.io", Version: "v1", Kind: "VolumeAttachmentList"}: {}, + {Group: "storage.k8s.io", Version: "v1beta1", Kind: "VolumeAttachment"}: {}, + {Group: "storage.k8s.io", Version: "v1beta1", Kind: "VolumeAttachmentList"}: {}, {Group: "authentication.k8s.io", Version: "v1", Kind: "TokenRequest"}: {}, } diff --git a/pkg/apis/core/v1/conversion.go b/pkg/apis/core/v1/conversion.go index a616784d5bb..09a3590f996 100644 --- a/pkg/apis/core/v1/conversion.go +++ b/pkg/apis/core/v1/conversion.go @@ -20,7 +20,7 @@ import ( "fmt" "reflect" - "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/conversion" @@ -463,3 +463,13 @@ func dropInitContainerAnnotations(oldAnnotations map[string]string) map[string]s } return newAnnotations } + +// Convert_core_PersistentVolumeSpec_To_v1_PersistentVolumeSpec is defined outside the autogenerated file for use by other API packages +func Convert_core_PersistentVolumeSpec_To_v1_PersistentVolumeSpec(in *core.PersistentVolumeSpec, out *v1.PersistentVolumeSpec, s conversion.Scope) error { + return autoConvert_core_PersistentVolumeSpec_To_v1_PersistentVolumeSpec(in, out, s) +} + +// Convert_v1_PersistentVolumeSpec_To_core_PersistentVolumeSpec is defined outside the autogenerated file for use by other API packages +func Convert_v1_PersistentVolumeSpec_To_core_PersistentVolumeSpec(in *v1.PersistentVolumeSpec, out *core.PersistentVolumeSpec, s conversion.Scope) error { + return autoConvert_v1_PersistentVolumeSpec_To_core_PersistentVolumeSpec(in, out, s) +} diff --git a/pkg/apis/core/validation/validation.go b/pkg/apis/core/validation/validation.go index 7eca587e120..c3324f64556 100644 --- a/pkg/apis/core/validation/validation.go +++ b/pkg/apis/core/validation/validation.go @@ -705,11 +705,15 @@ func validateISCSIVolumeSource(iscsi *core.ISCSIVolumeSource, fldPath *field.Pat return allErrs } -func validateISCSIPersistentVolumeSource(iscsi *core.ISCSIPersistentVolumeSource, fldPath *field.Path) field.ErrorList { +func validateISCSIPersistentVolumeSource(iscsi *core.ISCSIPersistentVolumeSource, pvName string, fldPath *field.Path) field.ErrorList { allErrs := field.ErrorList{} if len(iscsi.TargetPortal) == 0 { allErrs = append(allErrs, field.Required(fldPath.Child("targetPortal"), "")) } + if iscsi.InitiatorName != nil && len(pvName+":"+iscsi.TargetPortal) > 64 { + tooLongErr := "Total length of : must be under 64 characters if iscsi.initiatorName is specified." + allErrs = append(allErrs, field.Invalid(fldPath.Child("targetportal"), iscsi.TargetPortal, tooLongErr)) + } if len(iscsi.IQN) == 0 { allErrs = append(allErrs, field.Required(fldPath.Child("iqn"), "")) } else { @@ -1539,247 +1543,287 @@ var supportedDataSourceAPIGroupKinds = map[schema.GroupKind]bool{ {Group: "snapshot.storage.k8s.io", Kind: "VolumeSnapshot"}: true, } -func ValidatePersistentVolume(pv *core.PersistentVolume) field.ErrorList { - metaPath := field.NewPath("metadata") - allErrs := ValidateObjectMeta(&pv.ObjectMeta, false, ValidatePersistentVolumeName, metaPath) +func ValidatePersistentVolumeSpec(pvSpec *core.PersistentVolumeSpec, pvName string, validateInlinePersistentVolumeSpec bool, fldPath *field.Path) field.ErrorList { + allErrs := field.ErrorList{} - specPath := field.NewPath("spec") - if len(pv.Spec.AccessModes) == 0 { - allErrs = append(allErrs, field.Required(specPath.Child("accessModes"), "")) + if validateInlinePersistentVolumeSpec { + if pvSpec.ClaimRef != nil { + allErrs = append(allErrs, field.Forbidden(fldPath.Child("claimRef"), "may not be specified in the context of inline volumes")) + } + if len(pvSpec.Capacity) != 0 { + allErrs = append(allErrs, field.Forbidden(fldPath.Child("capacity"), "may not be specified in the context of inline volumes")) + } + if pvSpec.CSI == nil { + allErrs = append(allErrs, field.Required(fldPath.Child("csi"), "has to be specified in the context of inline volumes")) + } } - for _, mode := range pv.Spec.AccessModes { + + if len(pvSpec.AccessModes) == 0 { + allErrs = append(allErrs, field.Required(fldPath.Child("accessModes"), "")) + } + for _, mode := range pvSpec.AccessModes { if !supportedAccessModes.Has(string(mode)) { - allErrs = append(allErrs, field.NotSupported(specPath.Child("accessModes"), mode, supportedAccessModes.List())) + allErrs = append(allErrs, field.NotSupported(fldPath.Child("accessModes"), mode, supportedAccessModes.List())) } } - if len(pv.Spec.Capacity) == 0 { - allErrs = append(allErrs, field.Required(specPath.Child("capacity"), "")) - } + if !validateInlinePersistentVolumeSpec { + if len(pvSpec.Capacity) == 0 { + allErrs = append(allErrs, field.Required(fldPath.Child("capacity"), "")) + } - if _, ok := pv.Spec.Capacity[core.ResourceStorage]; !ok || len(pv.Spec.Capacity) > 1 { - allErrs = append(allErrs, field.NotSupported(specPath.Child("capacity"), pv.Spec.Capacity, []string{string(core.ResourceStorage)})) - } - capPath := specPath.Child("capacity") - for r, qty := range pv.Spec.Capacity { - allErrs = append(allErrs, validateBasicResource(qty, capPath.Key(string(r)))...) - allErrs = append(allErrs, ValidatePositiveQuantityValue(qty, capPath.Key(string(r)))...) - } - if len(string(pv.Spec.PersistentVolumeReclaimPolicy)) > 0 { - if !supportedReclaimPolicy.Has(string(pv.Spec.PersistentVolumeReclaimPolicy)) { - allErrs = append(allErrs, field.NotSupported(specPath.Child("persistentVolumeReclaimPolicy"), pv.Spec.PersistentVolumeReclaimPolicy, supportedReclaimPolicy.List())) + if _, ok := pvSpec.Capacity[core.ResourceStorage]; !ok || len(pvSpec.Capacity) > 1 { + allErrs = append(allErrs, field.NotSupported(fldPath.Child("capacity"), pvSpec.Capacity, []string{string(core.ResourceStorage)})) + } + capPath := fldPath.Child("capacity") + for r, qty := range pvSpec.Capacity { + allErrs = append(allErrs, validateBasicResource(qty, capPath.Key(string(r)))...) + allErrs = append(allErrs, ValidatePositiveQuantityValue(qty, capPath.Key(string(r)))...) } } - nodeAffinitySpecified, errs := validateVolumeNodeAffinity(pv.Spec.NodeAffinity, specPath.Child("nodeAffinity")) - allErrs = append(allErrs, errs...) - - numVolumes := 0 - if pv.Spec.HostPath != nil { - if numVolumes > 0 { - allErrs = append(allErrs, field.Forbidden(specPath.Child("hostPath"), "may not specify more than 1 volume type")) + if len(string(pvSpec.PersistentVolumeReclaimPolicy)) > 0 { + if validateInlinePersistentVolumeSpec { + if pvSpec.PersistentVolumeReclaimPolicy != core.PersistentVolumeReclaimRetain { + allErrs = append(allErrs, field.Forbidden(fldPath.Child("persistentVolumeReclaimPolicy"), "may only be "+string(core.PersistentVolumeReclaimRetain)+" in the context of inline volumes")) + } } else { - numVolumes++ - allErrs = append(allErrs, validateHostPathVolumeSource(pv.Spec.HostPath, specPath.Child("hostPath"))...) - } - } - if pv.Spec.GCEPersistentDisk != nil { - if numVolumes > 0 { - allErrs = append(allErrs, field.Forbidden(specPath.Child("gcePersistentDisk"), "may not specify more than 1 volume type")) - } else { - numVolumes++ - allErrs = append(allErrs, validateGCEPersistentDiskVolumeSource(pv.Spec.GCEPersistentDisk, specPath.Child("persistentDisk"))...) - } - } - if pv.Spec.AWSElasticBlockStore != nil { - if numVolumes > 0 { - allErrs = append(allErrs, field.Forbidden(specPath.Child("awsElasticBlockStore"), "may not specify more than 1 volume type")) - } else { - numVolumes++ - allErrs = append(allErrs, validateAWSElasticBlockStoreVolumeSource(pv.Spec.AWSElasticBlockStore, specPath.Child("awsElasticBlockStore"))...) - } - } - if pv.Spec.Glusterfs != nil { - if numVolumes > 0 { - allErrs = append(allErrs, field.Forbidden(specPath.Child("glusterfs"), "may not specify more than 1 volume type")) - } else { - numVolumes++ - allErrs = append(allErrs, validateGlusterfsPersistentVolumeSource(pv.Spec.Glusterfs, specPath.Child("glusterfs"))...) - } - } - if pv.Spec.Flocker != nil { - if numVolumes > 0 { - allErrs = append(allErrs, field.Forbidden(specPath.Child("flocker"), "may not specify more than 1 volume type")) - } else { - numVolumes++ - allErrs = append(allErrs, validateFlockerVolumeSource(pv.Spec.Flocker, specPath.Child("flocker"))...) - } - } - if pv.Spec.NFS != nil { - if numVolumes > 0 { - allErrs = append(allErrs, field.Forbidden(specPath.Child("nfs"), "may not specify more than 1 volume type")) - } else { - numVolumes++ - allErrs = append(allErrs, validateNFSVolumeSource(pv.Spec.NFS, specPath.Child("nfs"))...) - } - } - if pv.Spec.RBD != nil { - if numVolumes > 0 { - allErrs = append(allErrs, field.Forbidden(specPath.Child("rbd"), "may not specify more than 1 volume type")) - } else { - numVolumes++ - allErrs = append(allErrs, validateRBDPersistentVolumeSource(pv.Spec.RBD, specPath.Child("rbd"))...) - } - } - if pv.Spec.Quobyte != nil { - if numVolumes > 0 { - allErrs = append(allErrs, field.Forbidden(specPath.Child("quobyte"), "may not specify more than 1 volume type")) - } else { - numVolumes++ - allErrs = append(allErrs, validateQuobyteVolumeSource(pv.Spec.Quobyte, specPath.Child("quobyte"))...) - } - } - if pv.Spec.CephFS != nil { - if numVolumes > 0 { - allErrs = append(allErrs, field.Forbidden(specPath.Child("cephFS"), "may not specify more than 1 volume type")) - } else { - numVolumes++ - allErrs = append(allErrs, validateCephFSPersistentVolumeSource(pv.Spec.CephFS, specPath.Child("cephfs"))...) - } - } - if pv.Spec.ISCSI != nil { - if numVolumes > 0 { - allErrs = append(allErrs, field.Forbidden(specPath.Child("iscsi"), "may not specify more than 1 volume type")) - } else { - numVolumes++ - allErrs = append(allErrs, validateISCSIPersistentVolumeSource(pv.Spec.ISCSI, specPath.Child("iscsi"))...) - } - if pv.Spec.ISCSI.InitiatorName != nil && len(pv.ObjectMeta.Name+":"+pv.Spec.ISCSI.TargetPortal) > 64 { - tooLongErr := "Total length of : must be under 64 characters if iscsi.initiatorName is specified." - allErrs = append(allErrs, field.Invalid(metaPath.Child("name"), pv.ObjectMeta.Name, tooLongErr)) - } - } - if pv.Spec.Cinder != nil { - if numVolumes > 0 { - allErrs = append(allErrs, field.Forbidden(specPath.Child("cinder"), "may not specify more than 1 volume type")) - } else { - numVolumes++ - allErrs = append(allErrs, validateCinderPersistentVolumeSource(pv.Spec.Cinder, specPath.Child("cinder"))...) - } - } - if pv.Spec.FC != nil { - if numVolumes > 0 { - allErrs = append(allErrs, field.Forbidden(specPath.Child("fc"), "may not specify more than 1 volume type")) - } else { - numVolumes++ - allErrs = append(allErrs, validateFCVolumeSource(pv.Spec.FC, specPath.Child("fc"))...) - } - } - if pv.Spec.FlexVolume != nil { - numVolumes++ - allErrs = append(allErrs, validateFlexPersistentVolumeSource(pv.Spec.FlexVolume, specPath.Child("flexVolume"))...) - } - if pv.Spec.AzureFile != nil { - if numVolumes > 0 { - allErrs = append(allErrs, field.Forbidden(specPath.Child("azureFile"), "may not specify more than 1 volume type")) - - } else { - numVolumes++ - allErrs = append(allErrs, validateAzureFilePV(pv.Spec.AzureFile, specPath.Child("azureFile"))...) - } - } - - if pv.Spec.VsphereVolume != nil { - if numVolumes > 0 { - allErrs = append(allErrs, field.Forbidden(specPath.Child("vsphereVolume"), "may not specify more than 1 volume type")) - } else { - numVolumes++ - allErrs = append(allErrs, validateVsphereVolumeSource(pv.Spec.VsphereVolume, specPath.Child("vsphereVolume"))...) - } - } - if pv.Spec.PhotonPersistentDisk != nil { - if numVolumes > 0 { - allErrs = append(allErrs, field.Forbidden(specPath.Child("photonPersistentDisk"), "may not specify more than 1 volume type")) - } else { - numVolumes++ - allErrs = append(allErrs, validatePhotonPersistentDiskVolumeSource(pv.Spec.PhotonPersistentDisk, specPath.Child("photonPersistentDisk"))...) - } - } - if pv.Spec.PortworxVolume != nil { - if numVolumes > 0 { - allErrs = append(allErrs, field.Forbidden(specPath.Child("portworxVolume"), "may not specify more than 1 volume type")) - } else { - numVolumes++ - allErrs = append(allErrs, validatePortworxVolumeSource(pv.Spec.PortworxVolume, specPath.Child("portworxVolume"))...) - } - } - if pv.Spec.AzureDisk != nil { - if numVolumes > 0 { - allErrs = append(allErrs, field.Forbidden(specPath.Child("azureDisk"), "may not specify more than 1 volume type")) - } else { - numVolumes++ - allErrs = append(allErrs, validateAzureDisk(pv.Spec.AzureDisk, specPath.Child("azureDisk"))...) - } - } - if pv.Spec.ScaleIO != nil { - if numVolumes > 0 { - allErrs = append(allErrs, field.Forbidden(specPath.Child("scaleIO"), "may not specify more than 1 volume type")) - } else { - numVolumes++ - allErrs = append(allErrs, validateScaleIOPersistentVolumeSource(pv.Spec.ScaleIO, specPath.Child("scaleIO"))...) - } - } - if pv.Spec.Local != nil { - if numVolumes > 0 { - allErrs = append(allErrs, field.Forbidden(specPath.Child("local"), "may not specify more than 1 volume type")) - } else { - numVolumes++ - allErrs = append(allErrs, validateLocalVolumeSource(pv.Spec.Local, specPath.Child("local"))...) - - // NodeAffinity is required - if !nodeAffinitySpecified { - allErrs = append(allErrs, field.Required(metaPath.Child("annotations"), "Local volume requires node affinity")) + if !supportedReclaimPolicy.Has(string(pvSpec.PersistentVolumeReclaimPolicy)) { + allErrs = append(allErrs, field.NotSupported(fldPath.Child("persistentVolumeReclaimPolicy"), pvSpec.PersistentVolumeReclaimPolicy, supportedReclaimPolicy.List())) } } } - if pv.Spec.StorageOS != nil { + + var nodeAffinitySpecified bool + var errs field.ErrorList + if pvSpec.NodeAffinity != nil { + if validateInlinePersistentVolumeSpec { + allErrs = append(allErrs, field.Forbidden(fldPath.Child("nodeAffinity"), "may not be specified in the context of inline volumes")) + } else { + nodeAffinitySpecified, errs = validateVolumeNodeAffinity(pvSpec.NodeAffinity, fldPath.Child("nodeAffinity")) + allErrs = append(allErrs, errs...) + } + } + numVolumes := 0 + if pvSpec.HostPath != nil { if numVolumes > 0 { - allErrs = append(allErrs, field.Forbidden(specPath.Child("storageos"), "may not specify more than 1 volume type")) + allErrs = append(allErrs, field.Forbidden(fldPath.Child("hostPath"), "may not specify more than 1 volume type")) } else { numVolumes++ - allErrs = append(allErrs, validateStorageOSPersistentVolumeSource(pv.Spec.StorageOS, specPath.Child("storageos"))...) + allErrs = append(allErrs, validateHostPathVolumeSource(pvSpec.HostPath, fldPath.Child("hostPath"))...) + } + } + if pvSpec.GCEPersistentDisk != nil { + if numVolumes > 0 { + allErrs = append(allErrs, field.Forbidden(fldPath.Child("gcePersistentDisk"), "may not specify more than 1 volume type")) + } else { + numVolumes++ + allErrs = append(allErrs, validateGCEPersistentDiskVolumeSource(pvSpec.GCEPersistentDisk, fldPath.Child("persistentDisk"))...) + } + } + if pvSpec.AWSElasticBlockStore != nil { + if numVolumes > 0 { + allErrs = append(allErrs, field.Forbidden(fldPath.Child("awsElasticBlockStore"), "may not specify more than 1 volume type")) + } else { + numVolumes++ + allErrs = append(allErrs, validateAWSElasticBlockStoreVolumeSource(pvSpec.AWSElasticBlockStore, fldPath.Child("awsElasticBlockStore"))...) + } + } + if pvSpec.Glusterfs != nil { + if numVolumes > 0 { + allErrs = append(allErrs, field.Forbidden(fldPath.Child("glusterfs"), "may not specify more than 1 volume type")) + } else { + numVolumes++ + allErrs = append(allErrs, validateGlusterfsPersistentVolumeSource(pvSpec.Glusterfs, fldPath.Child("glusterfs"))...) + } + } + if pvSpec.Flocker != nil { + if numVolumes > 0 { + allErrs = append(allErrs, field.Forbidden(fldPath.Child("flocker"), "may not specify more than 1 volume type")) + } else { + numVolumes++ + allErrs = append(allErrs, validateFlockerVolumeSource(pvSpec.Flocker, fldPath.Child("flocker"))...) + } + } + if pvSpec.NFS != nil { + if numVolumes > 0 { + allErrs = append(allErrs, field.Forbidden(fldPath.Child("nfs"), "may not specify more than 1 volume type")) + } else { + numVolumes++ + allErrs = append(allErrs, validateNFSVolumeSource(pvSpec.NFS, fldPath.Child("nfs"))...) + } + } + if pvSpec.RBD != nil { + if numVolumes > 0 { + allErrs = append(allErrs, field.Forbidden(fldPath.Child("rbd"), "may not specify more than 1 volume type")) + } else { + numVolumes++ + allErrs = append(allErrs, validateRBDPersistentVolumeSource(pvSpec.RBD, fldPath.Child("rbd"))...) + } + } + if pvSpec.Quobyte != nil { + if numVolumes > 0 { + allErrs = append(allErrs, field.Forbidden(fldPath.Child("quobyte"), "may not specify more than 1 volume type")) + } else { + numVolumes++ + allErrs = append(allErrs, validateQuobyteVolumeSource(pvSpec.Quobyte, fldPath.Child("quobyte"))...) + } + } + if pvSpec.CephFS != nil { + if numVolumes > 0 { + allErrs = append(allErrs, field.Forbidden(fldPath.Child("cephFS"), "may not specify more than 1 volume type")) + } else { + numVolumes++ + allErrs = append(allErrs, validateCephFSPersistentVolumeSource(pvSpec.CephFS, fldPath.Child("cephfs"))...) + } + } + if pvSpec.ISCSI != nil { + if numVolumes > 0 { + allErrs = append(allErrs, field.Forbidden(fldPath.Child("iscsi"), "may not specify more than 1 volume type")) + } else { + numVolumes++ + allErrs = append(allErrs, validateISCSIPersistentVolumeSource(pvSpec.ISCSI, pvName, fldPath.Child("iscsi"))...) + } + } + if pvSpec.Cinder != nil { + if numVolumes > 0 { + allErrs = append(allErrs, field.Forbidden(fldPath.Child("cinder"), "may not specify more than 1 volume type")) + } else { + numVolumes++ + allErrs = append(allErrs, validateCinderPersistentVolumeSource(pvSpec.Cinder, fldPath.Child("cinder"))...) + } + } + if pvSpec.FC != nil { + if numVolumes > 0 { + allErrs = append(allErrs, field.Forbidden(fldPath.Child("fc"), "may not specify more than 1 volume type")) + } else { + numVolumes++ + allErrs = append(allErrs, validateFCVolumeSource(pvSpec.FC, fldPath.Child("fc"))...) + } + } + if pvSpec.FlexVolume != nil { + numVolumes++ + allErrs = append(allErrs, validateFlexPersistentVolumeSource(pvSpec.FlexVolume, fldPath.Child("flexVolume"))...) + } + if pvSpec.AzureFile != nil { + if numVolumes > 0 { + allErrs = append(allErrs, field.Forbidden(fldPath.Child("azureFile"), "may not specify more than 1 volume type")) + + } else { + numVolumes++ + allErrs = append(allErrs, validateAzureFilePV(pvSpec.AzureFile, fldPath.Child("azureFile"))...) } } - if pv.Spec.CSI != nil { + if pvSpec.VsphereVolume != nil { if numVolumes > 0 { - allErrs = append(allErrs, field.Forbidden(specPath.Child("csi"), "may not specify more than 1 volume type")) + allErrs = append(allErrs, field.Forbidden(fldPath.Child("vsphereVolume"), "may not specify more than 1 volume type")) } else { numVolumes++ - allErrs = append(allErrs, validateCSIPersistentVolumeSource(pv.Spec.CSI, specPath.Child("csi"))...) + allErrs = append(allErrs, validateVsphereVolumeSource(pvSpec.VsphereVolume, fldPath.Child("vsphereVolume"))...) + } + } + if pvSpec.PhotonPersistentDisk != nil { + if numVolumes > 0 { + allErrs = append(allErrs, field.Forbidden(fldPath.Child("photonPersistentDisk"), "may not specify more than 1 volume type")) + } else { + numVolumes++ + allErrs = append(allErrs, validatePhotonPersistentDiskVolumeSource(pvSpec.PhotonPersistentDisk, fldPath.Child("photonPersistentDisk"))...) + } + } + if pvSpec.PortworxVolume != nil { + if numVolumes > 0 { + allErrs = append(allErrs, field.Forbidden(fldPath.Child("portworxVolume"), "may not specify more than 1 volume type")) + } else { + numVolumes++ + allErrs = append(allErrs, validatePortworxVolumeSource(pvSpec.PortworxVolume, fldPath.Child("portworxVolume"))...) + } + } + if pvSpec.AzureDisk != nil { + if numVolumes > 0 { + allErrs = append(allErrs, field.Forbidden(fldPath.Child("azureDisk"), "may not specify more than 1 volume type")) + } else { + numVolumes++ + allErrs = append(allErrs, validateAzureDisk(pvSpec.AzureDisk, fldPath.Child("azureDisk"))...) + } + } + if pvSpec.ScaleIO != nil { + if numVolumes > 0 { + allErrs = append(allErrs, field.Forbidden(fldPath.Child("scaleIO"), "may not specify more than 1 volume type")) + } else { + numVolumes++ + allErrs = append(allErrs, validateScaleIOPersistentVolumeSource(pvSpec.ScaleIO, fldPath.Child("scaleIO"))...) + } + } + if pvSpec.Local != nil { + if numVolumes > 0 { + allErrs = append(allErrs, field.Forbidden(fldPath.Child("local"), "may not specify more than 1 volume type")) + } else { + numVolumes++ + allErrs = append(allErrs, validateLocalVolumeSource(pvSpec.Local, fldPath.Child("local"))...) + // NodeAffinity is required + if !nodeAffinitySpecified { + allErrs = append(allErrs, field.Required(fldPath.Child("nodeAffinity"), "Local volume requires node affinity")) + } + } + } + if pvSpec.StorageOS != nil { + if numVolumes > 0 { + allErrs = append(allErrs, field.Forbidden(fldPath.Child("storageos"), "may not specify more than 1 volume type")) + } else { + numVolumes++ + allErrs = append(allErrs, validateStorageOSPersistentVolumeSource(pvSpec.StorageOS, fldPath.Child("storageos"))...) + } + } + + if pvSpec.CSI != nil { + if numVolumes > 0 { + allErrs = append(allErrs, field.Forbidden(fldPath.Child("csi"), "may not specify more than 1 volume type")) + } else { + numVolumes++ + allErrs = append(allErrs, validateCSIPersistentVolumeSource(pvSpec.CSI, fldPath.Child("csi"))...) } } if numVolumes == 0 { - allErrs = append(allErrs, field.Required(specPath, "must specify a volume type")) + allErrs = append(allErrs, field.Required(fldPath, "must specify a volume type")) } // do not allow hostPath mounts of '/' to have a 'recycle' reclaim policy - if pv.Spec.HostPath != nil && path.Clean(pv.Spec.HostPath.Path) == "/" && pv.Spec.PersistentVolumeReclaimPolicy == core.PersistentVolumeReclaimRecycle { - allErrs = append(allErrs, field.Forbidden(specPath.Child("persistentVolumeReclaimPolicy"), "may not be 'recycle' for a hostPath mount of '/'")) + if pvSpec.HostPath != nil && path.Clean(pvSpec.HostPath.Path) == "/" && pvSpec.PersistentVolumeReclaimPolicy == core.PersistentVolumeReclaimRecycle { + allErrs = append(allErrs, field.Forbidden(fldPath.Child("persistentVolumeReclaimPolicy"), "may not be 'recycle' for a hostPath mount of '/'")) } - if len(pv.Spec.StorageClassName) > 0 { - for _, msg := range ValidateClassName(pv.Spec.StorageClassName, false) { - allErrs = append(allErrs, field.Invalid(specPath.Child("storageClassName"), pv.Spec.StorageClassName, msg)) + if len(pvSpec.StorageClassName) > 0 { + if validateInlinePersistentVolumeSpec { + allErrs = append(allErrs, field.Forbidden(fldPath.Child("storageClassName"), "may not be specified in the context of inline volumes")) + } else { + for _, msg := range ValidateClassName(pvSpec.StorageClassName, false) { + allErrs = append(allErrs, field.Invalid(fldPath.Child("storageClassName"), pvSpec.StorageClassName, msg)) + } } } - if pv.Spec.VolumeMode != nil && !supportedVolumeModes.Has(string(*pv.Spec.VolumeMode)) { - allErrs = append(allErrs, field.NotSupported(specPath.Child("volumeMode"), *pv.Spec.VolumeMode, supportedVolumeModes.List())) + if pvSpec.VolumeMode != nil { + if validateInlinePersistentVolumeSpec { + if *pvSpec.VolumeMode != core.PersistentVolumeFilesystem { + allErrs = append(allErrs, field.Forbidden(fldPath.Child("volumeMode"), "may not specify volumeMode other than "+string(core.PersistentVolumeFilesystem)+" in the context of inline volumes")) + } + } else { + if !supportedVolumeModes.Has(string(*pvSpec.VolumeMode)) { + allErrs = append(allErrs, field.NotSupported(fldPath.Child("volumeMode"), *pvSpec.VolumeMode, supportedVolumeModes.List())) + } + } } return allErrs } +func ValidatePersistentVolume(pv *core.PersistentVolume) field.ErrorList { + metaPath := field.NewPath("metadata") + allErrs := ValidateObjectMeta(&pv.ObjectMeta, false, ValidatePersistentVolumeName, metaPath) + allErrs = append(allErrs, ValidatePersistentVolumeSpec(&pv.Spec, pv.ObjectMeta.Name, false, field.NewPath("spec"))...) + return allErrs +} + // ValidatePersistentVolumeUpdate tests to see if the update is legal for an end user to make. // newPv is updated with fields that cannot be changed. func ValidatePersistentVolumeUpdate(newPv, oldPv *core.PersistentVolume) field.ErrorList { diff --git a/pkg/apis/core/validation/validation_test.go b/pkg/apis/core/validation/validation_test.go index 07ef601f50b..8a212fbf44b 100644 --- a/pkg/apis/core/validation/validation_test.go +++ b/pkg/apis/core/validation/validation_test.go @@ -426,6 +426,147 @@ func TestValidatePersistentVolumes(t *testing.T) { } +func TestValidatePersistentVolumeSpec(t *testing.T) { + fsmode := core.PersistentVolumeFilesystem + blockmode := core.PersistentVolumeBlock + scenarios := map[string]struct { + isExpectedFailure bool + isInlineSpec bool + pvSpec *core.PersistentVolumeSpec + }{ + "pv-pvspec-valid": { + isExpectedFailure: false, + isInlineSpec: false, + pvSpec: &core.PersistentVolumeSpec{ + Capacity: core.ResourceList{ + core.ResourceName(core.ResourceStorage): resource.MustParse("10G"), + }, + StorageClassName: "testclass", + PersistentVolumeReclaimPolicy: core.PersistentVolumeReclaimRecycle, + AccessModes: []core.PersistentVolumeAccessMode{core.ReadWriteOnce}, + PersistentVolumeSource: core.PersistentVolumeSource{ + HostPath: &core.HostPathVolumeSource{ + Path: "/foo", + Type: newHostPathType(string(core.HostPathDirectory)), + }, + }, + VolumeMode: &fsmode, + NodeAffinity: simpleVolumeNodeAffinity("foo", "bar"), + }, + }, + "inline-pvspec-with-capacity": { + isExpectedFailure: true, + isInlineSpec: true, + pvSpec: &core.PersistentVolumeSpec{ + Capacity: core.ResourceList{ + core.ResourceName(core.ResourceStorage): resource.MustParse("10G"), + }, + PersistentVolumeSource: core.PersistentVolumeSource{ + CSI: &core.CSIPersistentVolumeSource{Driver: "test-driver", VolumeHandle: "test-123", ReadOnly: true}, + }, + AccessModes: []core.PersistentVolumeAccessMode{core.ReadWriteOnce}, + }, + }, + "inline-pvspec-with-sc": { + isExpectedFailure: true, + isInlineSpec: true, + pvSpec: &core.PersistentVolumeSpec{ + PersistentVolumeSource: core.PersistentVolumeSource{ + CSI: &core.CSIPersistentVolumeSource{Driver: "test-driver", VolumeHandle: "test-123", ReadOnly: true}, + }, + AccessModes: []core.PersistentVolumeAccessMode{core.ReadWriteOnce}, + StorageClassName: "testclass", + }, + }, + "inline-pvspec-with-non-fs-volume-mode": { + isExpectedFailure: true, + isInlineSpec: true, + pvSpec: &core.PersistentVolumeSpec{ + PersistentVolumeSource: core.PersistentVolumeSource{ + CSI: &core.CSIPersistentVolumeSource{Driver: "test-driver", VolumeHandle: "test-123", ReadOnly: true}, + }, + AccessModes: []core.PersistentVolumeAccessMode{core.ReadWriteOnce}, + VolumeMode: &blockmode, + }, + }, + "inline-pvspec-with-non-retain-reclaim-policy": { + isExpectedFailure: true, + isInlineSpec: true, + pvSpec: &core.PersistentVolumeSpec{ + PersistentVolumeReclaimPolicy: core.PersistentVolumeReclaimRecycle, + PersistentVolumeSource: core.PersistentVolumeSource{ + CSI: &core.CSIPersistentVolumeSource{Driver: "test-driver", VolumeHandle: "test-123", ReadOnly: true}, + }, + AccessModes: []core.PersistentVolumeAccessMode{core.ReadWriteOnce}, + }, + }, + "inline-pvspec-with-node-affinity": { + isExpectedFailure: true, + isInlineSpec: true, + pvSpec: &core.PersistentVolumeSpec{ + PersistentVolumeSource: core.PersistentVolumeSource{ + CSI: &core.CSIPersistentVolumeSource{Driver: "test-driver", VolumeHandle: "test-123", ReadOnly: true}, + }, + AccessModes: []core.PersistentVolumeAccessMode{core.ReadWriteOnce}, + NodeAffinity: simpleVolumeNodeAffinity("foo", "bar"), + }, + }, + "inline-pvspec-with-non-csi-source": { + isExpectedFailure: true, + isInlineSpec: true, + pvSpec: &core.PersistentVolumeSpec{ + PersistentVolumeSource: core.PersistentVolumeSource{ + HostPath: &core.HostPathVolumeSource{ + Path: "/foo", + Type: newHostPathType(string(core.HostPathDirectory)), + }, + }, + AccessModes: []core.PersistentVolumeAccessMode{core.ReadWriteOnce}, + }, + }, + "inline-pvspec-valid-with-access-modes-and-mount-options": { + isExpectedFailure: false, + isInlineSpec: true, + pvSpec: &core.PersistentVolumeSpec{ + PersistentVolumeSource: core.PersistentVolumeSource{ + CSI: &core.CSIPersistentVolumeSource{Driver: "test-driver", VolumeHandle: "test-123", ReadOnly: true}, + }, + AccessModes: []core.PersistentVolumeAccessMode{core.ReadWriteOnce}, + MountOptions: []string{"soft", "read-write"}, + }, + }, + "inline-pvspec-valid-with-access-modes": { + isExpectedFailure: false, + isInlineSpec: true, + pvSpec: &core.PersistentVolumeSpec{ + PersistentVolumeSource: core.PersistentVolumeSource{ + CSI: &core.CSIPersistentVolumeSource{Driver: "test-driver", VolumeHandle: "test-123", ReadOnly: true}, + }, + AccessModes: []core.PersistentVolumeAccessMode{core.ReadWriteOnce}, + }, + }, + "inline-pvspec-with-missing-acess-modes": { + isExpectedFailure: true, + isInlineSpec: true, + pvSpec: &core.PersistentVolumeSpec{ + PersistentVolumeSource: core.PersistentVolumeSource{ + CSI: &core.CSIPersistentVolumeSource{Driver: "test-driver", VolumeHandle: "test-123", ReadOnly: true}, + }, + MountOptions: []string{"soft", "read-write"}, + }, + }, + } + for name, scenario := range scenarios { + errs := ValidatePersistentVolumeSpec(scenario.pvSpec, "", scenario.isInlineSpec, field.NewPath("field")) + 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 TestValidatePersistentVolumeSourceUpdate(t *testing.T) { validVolume := testVolume("foo", "", core.PersistentVolumeSpec{ Capacity: core.ResourceList{ diff --git a/pkg/apis/storage/types.go b/pkg/apis/storage/types.go index 795033c14e7..5f0345d44f3 100644 --- a/pkg/apis/storage/types.go +++ b/pkg/apis/storage/types.go @@ -145,15 +145,22 @@ type VolumeAttachmentSpec struct { } // VolumeAttachmentSource represents a volume that should be attached. -// Right now only PersistenVolumes can be attached via external attacher, -// in future we may allow also inline volumes in pods. +// Right now persistent volumes as well as inline volumes (only in +// CSI Migration scenarios) can be attached via external attacher. // Exactly one member can be set. type VolumeAttachmentSource struct { // Name of the persistent volume to attach. // +optional PersistentVolumeName *string - // Placeholder for *VolumeSource to accommodate inline volumes in pods. + // inlineVolumeSpec contains all the information necessary to attach + // a persistent volume defined by a pod's inline VolumeSource. This field + // is populated only for the CSIMigration feature. It contains + // translated fields from a pod's inline VolumeSource to a + // PersistentVolumeSpec. This field is alpha-level and is only + // honored by servers that enabled the CSIMigration feature. + // +optional + InlineVolumeSpec *api.PersistentVolumeSpec } // The status of a VolumeAttachment request. diff --git a/pkg/apis/storage/v1alpha1/BUILD b/pkg/apis/storage/v1alpha1/BUILD index bf7b15e50e3..e5a86e68318 100644 --- a/pkg/apis/storage/v1alpha1/BUILD +++ b/pkg/apis/storage/v1alpha1/BUILD @@ -11,7 +11,10 @@ go_library( importpath = "k8s.io/kubernetes/pkg/apis/storage/v1alpha1", visibility = ["//visibility:public"], deps = [ + "//pkg/apis/core:go_default_library", + "//pkg/apis/core/v1:go_default_library", "//pkg/apis/storage:go_default_library", + "//staging/src/k8s.io/api/core/v1:go_default_library", "//staging/src/k8s.io/api/storage/v1alpha1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/conversion:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library", diff --git a/pkg/apis/storage/validation/validation.go b/pkg/apis/storage/validation/validation.go index 4100627d4a5..a48b4d87208 100644 --- a/pkg/apis/storage/validation/validation.go +++ b/pkg/apis/storage/validation/validation.go @@ -28,7 +28,11 @@ import ( api "k8s.io/kubernetes/pkg/apis/core" "k8s.io/kubernetes/pkg/apis/core/helper" apivalidation "k8s.io/kubernetes/pkg/apis/core/validation" + storagevalidation "k8s.io/kubernetes/pkg/apis/core/validation" "k8s.io/kubernetes/pkg/apis/storage" + + utilfeature "k8s.io/apiserver/pkg/util/feature" + "k8s.io/kubernetes/pkg/features" ) const ( @@ -168,8 +172,26 @@ func validateAttacher(attacher string, fldPath *field.Path) field.ErrorList { // validateSource tests if the source is valid for VolumeAttachment. func validateVolumeAttachmentSource(source *storage.VolumeAttachmentSource, fldPath *field.Path) field.ErrorList { allErrs := field.ErrorList{} - if source.PersistentVolumeName == nil || len(*source.PersistentVolumeName) == 0 { - allErrs = append(allErrs, field.Required(fldPath, "")) + switch { + case source.InlineVolumeSpec == nil && source.PersistentVolumeName == nil: + if utilfeature.DefaultFeatureGate.Enabled(features.CSIMigration) { + allErrs = append(allErrs, field.Required(fldPath, "must specify exactly one of inlineVolumeSpec and persistentVolumeName")) + } else { + allErrs = append(allErrs, field.Required(fldPath, "must specify persistentVolumeName when CSIMigration feature is disabled")) + } + case source.InlineVolumeSpec != nil && source.PersistentVolumeName != nil: + allErrs = append(allErrs, field.Forbidden(fldPath, "must specify exactly one of inlineVolumeSpec and persistentVolumeName")) + case source.PersistentVolumeName != nil: + if len(*source.PersistentVolumeName) == 0 { + // Invalid err + allErrs = append(allErrs, field.Required(fldPath.Child("persistentVolumeName"), "must specify non empty persistentVolumeName")) + } + case source.InlineVolumeSpec != nil: + if utilfeature.DefaultFeatureGate.Enabled(features.CSIMigration) { + allErrs = append(allErrs, storagevalidation.ValidatePersistentVolumeSpec(source.InlineVolumeSpec, "", true, fldPath.Child("inlineVolumeSpec"))...) + } else { + allErrs = append(allErrs, field.Forbidden(fldPath, "may not specify inlineVolumeSpec when CSIMigration feature is disabled")) + } } return allErrs } diff --git a/pkg/apis/storage/validation/validation_test.go b/pkg/apis/storage/validation/validation_test.go index 8c9c4c26886..74e6eddba73 100644 --- a/pkg/apis/storage/validation/validation_test.go +++ b/pkg/apis/storage/validation/validation_test.go @@ -21,9 +21,13 @@ import ( "strings" "testing" + "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + utilfeature "k8s.io/apiserver/pkg/util/feature" + featuregatetesting "k8s.io/component-base/featuregate/testing" api "k8s.io/kubernetes/pkg/apis/core" "k8s.io/kubernetes/pkg/apis/storage" + "k8s.io/kubernetes/pkg/features" ) var ( @@ -32,6 +36,15 @@ var ( immediateMode2 = storage.VolumeBindingImmediate waitingMode = storage.VolumeBindingWaitForFirstConsumer invalidMode = storage.VolumeBindingMode("foo") + inlineSpec = api.PersistentVolumeSpec{ + AccessModes: []api.PersistentVolumeAccessMode{api.ReadWriteOnce}, + PersistentVolumeSource: api.PersistentVolumeSource{ + CSI: &api.CSIPersistentVolumeSource{ + Driver: "com.test.foo", + VolumeHandle: "foobar", + }, + }, + } ) func TestValidateStorageClass(t *testing.T) { @@ -138,9 +151,10 @@ func TestValidateStorageClass(t *testing.T) { } func TestVolumeAttachmentValidation(t *testing.T) { + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSIMigration, true)() volumeName := "pv-name" empty := "" - successCases := []storage.VolumeAttachment{ + migrationEnabledSuccessCases := []storage.VolumeAttachment{ { ObjectMeta: metav1.ObjectMeta{Name: "foo"}, Spec: storage.VolumeAttachmentSpec{ @@ -151,6 +165,16 @@ func TestVolumeAttachmentValidation(t *testing.T) { NodeName: "mynode", }, }, + { + ObjectMeta: metav1.ObjectMeta{Name: "foo-with-inlinespec"}, + Spec: storage.VolumeAttachmentSpec{ + Attacher: "myattacher", + Source: storage.VolumeAttachmentSource{ + InlineVolumeSpec: &inlineSpec, + }, + NodeName: "mynode", + }, + }, { ObjectMeta: metav1.ObjectMeta{Name: "foo-with-status"}, Spec: storage.VolumeAttachmentSpec{ @@ -175,14 +199,38 @@ func TestVolumeAttachmentValidation(t *testing.T) { }, }, }, + { + ObjectMeta: metav1.ObjectMeta{Name: "foo-with-inlinespec-and-status"}, + Spec: storage.VolumeAttachmentSpec{ + Attacher: "myattacher", + Source: storage.VolumeAttachmentSource{ + InlineVolumeSpec: &inlineSpec, + }, + NodeName: "mynode", + }, + Status: storage.VolumeAttachmentStatus{ + Attached: true, + AttachmentMetadata: map[string]string{ + "foo": "bar", + }, + AttachError: &storage.VolumeError{ + Time: metav1.Time{}, + Message: "hello world", + }, + DetachError: &storage.VolumeError{ + Time: metav1.Time{}, + Message: "hello world", + }, + }, + }, } - for _, volumeAttachment := range successCases { + for _, volumeAttachment := range migrationEnabledSuccessCases { if errs := ValidateVolumeAttachment(&volumeAttachment); len(errs) != 0 { - t.Errorf("expected success: %v", errs) + t.Errorf("expected success: %v %v", volumeAttachment, errs) } } - errorCases := []storage.VolumeAttachment{ + migrationEnabledErrorCases := []storage.VolumeAttachment{ { // Empty attacher name ObjectMeta: metav1.ObjectMeta{Name: "foo"}, @@ -277,16 +325,105 @@ func TestVolumeAttachmentValidation(t *testing.T) { }, }, }, + { + // VolumeAttachmentSource with no PersistentVolumeName nor InlineSpec + ObjectMeta: metav1.ObjectMeta{Name: "foo"}, + Spec: storage.VolumeAttachmentSpec{ + Attacher: "myattacher", + NodeName: "node", + Source: storage.VolumeAttachmentSource{}, + }, + }, + { + // VolumeAttachmentSource with PersistentVolumeName and InlineSpec + ObjectMeta: metav1.ObjectMeta{Name: "foo"}, + Spec: storage.VolumeAttachmentSpec{ + Attacher: "myattacher", + NodeName: "node", + Source: storage.VolumeAttachmentSource{ + PersistentVolumeName: &volumeName, + InlineVolumeSpec: &inlineSpec, + }, + }, + }, + { + // VolumeAttachmentSource with InlineSpec without CSI PV Source + ObjectMeta: metav1.ObjectMeta{Name: "foo"}, + Spec: storage.VolumeAttachmentSpec{ + Attacher: "myattacher", + NodeName: "node", + Source: storage.VolumeAttachmentSource{ + PersistentVolumeName: &volumeName, + InlineVolumeSpec: &api.PersistentVolumeSpec{ + Capacity: api.ResourceList{ + api.ResourceName(api.ResourceStorage): resource.MustParse("10G"), + }, + AccessModes: []api.PersistentVolumeAccessMode{api.ReadWriteOnce}, + PersistentVolumeSource: api.PersistentVolumeSource{ + FlexVolume: &api.FlexPersistentVolumeSource{ + Driver: "kubernetes.io/blue", + FSType: "ext4", + }, + }, + StorageClassName: "test-storage-class", + }, + }, + }, + }, } - for _, volumeAttachment := range errorCases { + for _, volumeAttachment := range migrationEnabledErrorCases { if errs := ValidateVolumeAttachment(&volumeAttachment); len(errs) == 0 { - t.Errorf("Expected failure for test: %v", volumeAttachment) + t.Errorf("expected failure for test: %v", volumeAttachment) } } + + // validate with CSIMigration disabled + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSIMigration, false)() + + migrationDisabledSuccessCases := []storage.VolumeAttachment{ + { + // PVName specified with migration disabled + ObjectMeta: metav1.ObjectMeta{Name: "foo"}, + Spec: storage.VolumeAttachmentSpec{ + Attacher: "myattacher", + NodeName: "node", + Source: storage.VolumeAttachmentSource{ + PersistentVolumeName: &volumeName, + }, + }, + }, + } + for _, volumeAttachment := range migrationDisabledSuccessCases { + if errs := ValidateVolumeAttachment(&volumeAttachment); len(errs) != 0 { + t.Errorf("expected success: %v %v", volumeAttachment, errs) + } + } + + migrationDisabledErrorCases := []storage.VolumeAttachment{ + { + // InlineSpec specified with migration disabled + ObjectMeta: metav1.ObjectMeta{Name: "foo"}, + Spec: storage.VolumeAttachmentSpec{ + Attacher: "myattacher", + NodeName: "node", + Source: storage.VolumeAttachmentSource{ + InlineVolumeSpec: &inlineSpec, + }, + }, + }, + } + + for _, volumeAttachment := range migrationDisabledErrorCases { + if errs := ValidateVolumeAttachment(&volumeAttachment); len(errs) == 0 { + t.Errorf("expected failure: %v %v", volumeAttachment, errs) + } + } + } func TestVolumeAttachmentUpdateValidation(t *testing.T) { + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSIMigration, true)() volumeName := "foo" newVolumeName := "bar" @@ -294,21 +431,18 @@ func TestVolumeAttachmentUpdateValidation(t *testing.T) { ObjectMeta: metav1.ObjectMeta{Name: "foo"}, Spec: storage.VolumeAttachmentSpec{ Attacher: "myattacher", - Source: storage.VolumeAttachmentSource{ - PersistentVolumeName: &volumeName, - }, + Source: storage.VolumeAttachmentSource{}, NodeName: "mynode", }, } + successCases := []storage.VolumeAttachment{ { // no change ObjectMeta: metav1.ObjectMeta{Name: "foo"}, Spec: storage.VolumeAttachmentSpec{ Attacher: "myattacher", - Source: storage.VolumeAttachmentSource{ - PersistentVolumeName: &volumeName, - }, + Source: storage.VolumeAttachmentSource{}, NodeName: "mynode", }, }, @@ -317,9 +451,7 @@ func TestVolumeAttachmentUpdateValidation(t *testing.T) { ObjectMeta: metav1.ObjectMeta{Name: "foo"}, Spec: storage.VolumeAttachmentSpec{ Attacher: "myattacher", - Source: storage.VolumeAttachmentSource{ - PersistentVolumeName: &volumeName, - }, + Source: storage.VolumeAttachmentSource{}, NodeName: "mynode", }, Status: storage.VolumeAttachmentStatus{ @@ -340,11 +472,29 @@ func TestVolumeAttachmentUpdateValidation(t *testing.T) { } for _, volumeAttachment := range successCases { + volumeAttachment.Spec.Source = storage.VolumeAttachmentSource{} + old.Spec.Source = storage.VolumeAttachmentSource{} + // test scenarios with PersistentVolumeName set + volumeAttachment.Spec.Source.PersistentVolumeName = &volumeName + old.Spec.Source.PersistentVolumeName = &volumeName + if errs := ValidateVolumeAttachmentUpdate(&volumeAttachment, &old); len(errs) != 0 { + t.Errorf("expected success: %+v", errs) + } + + volumeAttachment.Spec.Source = storage.VolumeAttachmentSource{} + old.Spec.Source = storage.VolumeAttachmentSource{} + // test scenarios with InlineVolumeSpec set + volumeAttachment.Spec.Source.InlineVolumeSpec = &inlineSpec + old.Spec.Source.InlineVolumeSpec = &inlineSpec if errs := ValidateVolumeAttachmentUpdate(&volumeAttachment, &old); len(errs) != 0 { t.Errorf("expected success: %+v", errs) } } + // reset old's source with volumeName in case it was left with something else by earlier tests + old.Spec.Source = storage.VolumeAttachmentSource{} + old.Spec.Source.PersistentVolumeName = &volumeName + errorCases := []storage.VolumeAttachment{ { // change attacher @@ -358,7 +508,7 @@ func TestVolumeAttachmentUpdateValidation(t *testing.T) { }, }, { - // change volume + // change source volume name ObjectMeta: metav1.ObjectMeta{Name: "foo"}, Spec: storage.VolumeAttachmentSpec{ Attacher: "myattacher", @@ -379,6 +529,17 @@ func TestVolumeAttachmentUpdateValidation(t *testing.T) { NodeName: "anothernode", }, }, + { + // change source + ObjectMeta: metav1.ObjectMeta{Name: "foo"}, + Spec: storage.VolumeAttachmentSpec{ + Attacher: "myattacher", + Source: storage.VolumeAttachmentSource{ + InlineVolumeSpec: &inlineSpec, + }, + NodeName: "mynode", + }, + }, { // add invalid status ObjectMeta: metav1.ObjectMeta{Name: "foo"}, diff --git a/pkg/registry/storage/volumeattachment/BUILD b/pkg/registry/storage/volumeattachment/BUILD index 6156e767673..c47461b8333 100644 --- a/pkg/registry/storage/volumeattachment/BUILD +++ b/pkg/registry/storage/volumeattachment/BUILD @@ -12,6 +12,7 @@ go_library( "//pkg/api/legacyscheme:go_default_library", "//pkg/apis/storage:go_default_library", "//pkg/apis/storage/validation:go_default_library", + "//pkg/features:go_default_library", "//staging/src/k8s.io/api/storage/v1beta1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library", @@ -19,6 +20,7 @@ go_library( "//staging/src/k8s.io/apimachinery/pkg/util/validation/field:go_default_library", "//staging/src/k8s.io/apiserver/pkg/endpoints/request:go_default_library", "//staging/src/k8s.io/apiserver/pkg/storage/names:go_default_library", + "//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library", ], ) @@ -27,12 +29,17 @@ go_test( srcs = ["strategy_test.go"], embed = [":go_default_library"], deps = [ + "//pkg/apis/core:go_default_library", "//pkg/apis/storage:go_default_library", + "//pkg/features:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/api/equality:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/api/resource:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/diff:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/validation/field:go_default_library", "//staging/src/k8s.io/apiserver/pkg/endpoints/request:go_default_library", + "//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library", + "//staging/src/k8s.io/component-base/featuregate/testing:go_default_library", ], ) diff --git a/pkg/registry/storage/volumeattachment/strategy.go b/pkg/registry/storage/volumeattachment/strategy.go index 1b3a56c51aa..183fe02d955 100644 --- a/pkg/registry/storage/volumeattachment/strategy.go +++ b/pkg/registry/storage/volumeattachment/strategy.go @@ -26,9 +26,11 @@ import ( "k8s.io/apimachinery/pkg/util/validation/field" genericapirequest "k8s.io/apiserver/pkg/endpoints/request" "k8s.io/apiserver/pkg/storage/names" + utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/kubernetes/pkg/api/legacyscheme" "k8s.io/kubernetes/pkg/apis/storage" "k8s.io/kubernetes/pkg/apis/storage/validation" + "k8s.io/kubernetes/pkg/features" ) // volumeAttachmentStrategy implements behavior for VolumeAttachment objects @@ -53,6 +55,8 @@ func (volumeAttachmentStrategy) PrepareForCreate(ctx context.Context, obj runtim groupVersion = schema.GroupVersion{Group: requestInfo.APIGroup, Version: requestInfo.APIVersion} } + volumeAttachment := obj.(*storage.VolumeAttachment) + switch groupVersion { case storageapiv1beta1.SchemeGroupVersion: // allow modification of status for v1beta1 @@ -60,6 +64,11 @@ func (volumeAttachmentStrategy) PrepareForCreate(ctx context.Context, obj runtim volumeAttachment := obj.(*storage.VolumeAttachment) volumeAttachment.Status = storage.VolumeAttachmentStatus{} } + + if !utilfeature.DefaultFeatureGate.Enabled(features.CSIMigration) { + volumeAttachment.Spec.Source.InlineVolumeSpec = nil + } + } func (volumeAttachmentStrategy) Validate(ctx context.Context, obj runtime.Object) field.ErrorList { @@ -98,15 +107,21 @@ func (volumeAttachmentStrategy) PrepareForUpdate(ctx context.Context, obj, old r if requestInfo, found := genericapirequest.RequestInfoFrom(ctx); found { groupVersion = schema.GroupVersion{Group: requestInfo.APIGroup, Version: requestInfo.APIVersion} } + + newVolumeAttachment := obj.(*storage.VolumeAttachment) + oldVolumeAttachment := old.(*storage.VolumeAttachment) + switch groupVersion { case storageapiv1beta1.SchemeGroupVersion: // allow modification of Status via main resource for v1beta1 default: - newVolumeAttachment := obj.(*storage.VolumeAttachment) - oldVolumeAttachment := old.(*storage.VolumeAttachment) newVolumeAttachment.Status = oldVolumeAttachment.Status // No need to increment Generation because we don't allow updates to spec } + + if !utilfeature.DefaultFeatureGate.Enabled(features.CSIMigration) && oldVolumeAttachment.Spec.Source.InlineVolumeSpec == nil { + newVolumeAttachment.Spec.Source.InlineVolumeSpec = nil + } } func (volumeAttachmentStrategy) ValidateUpdate(ctx context.Context, obj, old runtime.Object) field.ErrorList { diff --git a/pkg/registry/storage/volumeattachment/strategy_test.go b/pkg/registry/storage/volumeattachment/strategy_test.go index eadde7ccc1c..ed33a9da014 100644 --- a/pkg/registry/storage/volumeattachment/strategy_test.go +++ b/pkg/registry/storage/volumeattachment/strategy_test.go @@ -20,11 +20,16 @@ import ( "testing" apiequality "k8s.io/apimachinery/pkg/api/equality" + "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/diff" "k8s.io/apimachinery/pkg/util/validation/field" genericapirequest "k8s.io/apiserver/pkg/endpoints/request" + utilfeature "k8s.io/apiserver/pkg/util/feature" + featuregatetesting "k8s.io/component-base/featuregate/testing" + api "k8s.io/kubernetes/pkg/apis/core" "k8s.io/kubernetes/pkg/apis/storage" + "k8s.io/kubernetes/pkg/features" ) func getValidVolumeAttachment(name string) *storage.VolumeAttachment { @@ -42,6 +47,25 @@ func getValidVolumeAttachment(name string) *storage.VolumeAttachment { } } +func getValidVolumeAttachmentWithInlineSpec(name string) *storage.VolumeAttachment { + volumeAttachment := getValidVolumeAttachment(name) + volumeAttachment.Spec.Source.PersistentVolumeName = nil + volumeAttachment.Spec.Source.InlineVolumeSpec = &api.PersistentVolumeSpec{ + Capacity: api.ResourceList{ + api.ResourceName(api.ResourceStorage): resource.MustParse("10"), + }, + AccessModes: []api.PersistentVolumeAccessMode{api.ReadWriteOnce}, + PersistentVolumeSource: api.PersistentVolumeSource{ + CSI: &api.CSIPersistentVolumeSource{ + Driver: "com.test.foo", + VolumeHandle: name, + }, + }, + MountOptions: []string{"soft"}, + } + return volumeAttachment +} + func TestVolumeAttachmentStrategy(t *testing.T) { ctx := genericapirequest.WithRequestInfo(genericapirequest.NewContext(), &genericapirequest.RequestInfo{ APIGroup: "storage.k8s.io", @@ -94,6 +118,46 @@ func TestVolumeAttachmentStrategy(t *testing.T) { } } +func TestVolumeAttachmentStrategySourceInlineSpec(t *testing.T) { + ctx := genericapirequest.WithRequestInfo(genericapirequest.NewContext(), &genericapirequest.RequestInfo{ + APIGroup: "storage.k8s.io", + APIVersion: "v1", + Resource: "volumeattachments", + }) + + volumeAttachment := getValidVolumeAttachmentWithInlineSpec("valid-attachment") + volumeAttachmentSaved := volumeAttachment.DeepCopy() + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSIMigration, true)() + Strategy.PrepareForCreate(ctx, volumeAttachment) + if volumeAttachment.Spec.Source.InlineVolumeSpec == nil { + t.Errorf("InlineVolumeSpec unexpectedly set to nil during PrepareForCreate") + } + if !apiequality.Semantic.DeepEqual(volumeAttachmentSaved, volumeAttachment) { + t.Errorf("unexpected difference in object after creation: %v", diff.ObjectDiff(volumeAttachment, volumeAttachmentSaved)) + } + Strategy.PrepareForUpdate(ctx, volumeAttachmentSaved, volumeAttachment) + if volumeAttachmentSaved.Spec.Source.InlineVolumeSpec == nil { + t.Errorf("InlineVolumeSpec unexpectedly set to nil during PrepareForUpdate") + } + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSIMigration, false)() + Strategy.PrepareForUpdate(ctx, volumeAttachmentSaved, volumeAttachment) + if volumeAttachmentSaved.Spec.Source.InlineVolumeSpec == nil { + t.Errorf("InlineVolumeSpec unexpectedly set to nil during PrepareForUpdate") + } + + volumeAttachment = getValidVolumeAttachmentWithInlineSpec("valid-attachment") + volumeAttachmentNew := volumeAttachment.DeepCopy() + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSIMigration, false)() + Strategy.PrepareForCreate(ctx, volumeAttachment) + if volumeAttachment.Spec.Source.InlineVolumeSpec != nil { + t.Errorf("InlineVolumeSpec unexpectedly not dropped during PrepareForCreate") + } + Strategy.PrepareForUpdate(ctx, volumeAttachmentNew, volumeAttachment) + if volumeAttachmentNew.Spec.Source.InlineVolumeSpec != nil { + t.Errorf("InlineVolumeSpec unexpectedly not dropped during PrepareForUpdate") + } +} + func TestVolumeAttachmentStatusStrategy(t *testing.T) { ctx := genericapirequest.WithRequestInfo(genericapirequest.NewContext(), &genericapirequest.RequestInfo{ APIGroup: "storage.k8s.io", diff --git a/staging/src/k8s.io/api/storage/v1alpha1/types.go b/staging/src/k8s.io/api/storage/v1alpha1/types.go index 964bb5f7b17..76ad6dc0dd8 100644 --- a/staging/src/k8s.io/api/storage/v1alpha1/types.go +++ b/staging/src/k8s.io/api/storage/v1alpha1/types.go @@ -16,7 +16,10 @@ limitations under the License. package v1alpha1 -import metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +import ( + "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) // +genclient // +genclient:nonNamespaced @@ -81,7 +84,14 @@ type VolumeAttachmentSource struct { // +optional PersistentVolumeName *string `json:"persistentVolumeName,omitempty" protobuf:"bytes,1,opt,name=persistentVolumeName"` - // Placeholder for *VolumeSource to accommodate inline volumes in pods. + // inlineVolumeSpec contains all the information necessary to attach + // a persistent volume defined by a pod's inline VolumeSource. This field + // is populated only for the CSIMigration feature. It contains + // translated fields from a pod's inline VolumeSource to a + // PersistentVolumeSpec. This field is alpha-level and is only + // honored by servers that enabled the CSIMigration feature. + // +optional + InlineVolumeSpec *v1.PersistentVolumeSpec `json:"inlineVolumeSpec,omitempty" protobuf:"bytes,2,opt,name=inlineVolumeSpec"` } // VolumeAttachmentStatus is the status of a VolumeAttachment request. diff --git a/staging/src/k8s.io/api/storage/v1beta1/types.go b/staging/src/k8s.io/api/storage/v1beta1/types.go index 49005791202..cca50d82095 100644 --- a/staging/src/k8s.io/api/storage/v1beta1/types.go +++ b/staging/src/k8s.io/api/storage/v1beta1/types.go @@ -166,7 +166,14 @@ type VolumeAttachmentSource struct { // +optional PersistentVolumeName *string `json:"persistentVolumeName,omitempty" protobuf:"bytes,1,opt,name=persistentVolumeName"` - // Placeholder for *VolumeSource to accommodate inline volumes in pods. + // inlineVolumeSpec contains all the information necessary to attach + // a persistent volume defined by a pod's inline VolumeSource. This field + // is populated only for the CSIMigration feature. It contains + // translated fields from a pod's inline VolumeSource to a + // PersistentVolumeSpec. This field is alpha-level and is only + // honored by servers that enabled the CSIMigration feature. + // +optional + InlineVolumeSpec *v1.PersistentVolumeSpec `json:"inlineVolumeSpec,omitempty" protobuf:"bytes,2,opt,name=inlineVolumeSpec"` } // VolumeAttachmentStatus is the status of a VolumeAttachment request.