Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Promote CSIPersistentVolume feature to GA #69929

Merged
merged 7 commits into from Nov 15, 2018
Merged

Promote CSIPersistentVolume feature to GA #69929

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
534ce94
Move CSIPersistentVolume feature to GA
jsafrane Nov 14, 2018
633431c
Promote CSIPersistentVolumeSource to GA
jsafrane Nov 14, 2018
b9d9b7f
Promote VolumeAttachment to storage.k8s.io/v1 API
jsafrane Nov 14, 2018
e95b188
Generated files
jsafrane Nov 14, 2018
498cd61
Add volumeattachment status subresource
msau42 Nov 14, 2018
8cfce0a
Tighten up validation of VolumeAttachment fields
jsafrane Nov 14, 2018
eaff4fd
Tolerate existing CSI fields in PVs when the feature is disabled
liggitt Nov 14, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1,054 changes: 1,046 additions & 8 deletions api/openapi-spec/swagger.json
View file
Open in desktop

Large diffs are not rendered by default.

1,030 changes: 1,030 additions & 0 deletions api/swagger-spec/storage.k8s.io_v1.json
View file
Open in desktop

Large diffs are not rendered by default.

706 changes: 505 additions & 201 deletions docs/api-reference/storage.k8s.io/v1/definitions.html
View file
Open in desktop

Large diffs are not rendered by default.

1,791 changes: 1,777 additions & 14 deletions docs/api-reference/storage.k8s.io/v1/operations.html
View file
Open in desktop

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions pkg/apis/core/types.go
View file
Open in desktop
Expand Up @@ -229,7 +229,7 @@ type PersistentVolumeSource struct {
// More info: https://releases.k8s.io/HEAD/examples/volumes/storageos/README.md
// +optional
StorageOS *StorageOSPersistentVolumeSource
// CSI (Container Storage Interface) represents storage that handled by an external CSI driver (Beta feature).
// CSI (Container Storage Interface) represents storage that handled by an external CSI driver.
// +optional
CSI *CSIPersistentVolumeSource
}
Expand Down Expand Up @@ -1547,7 +1547,7 @@ type LocalVolumeSource struct {
FSType *string
}

// Represents storage that is managed by an external CSI volume driver (Beta feature)
// Represents storage that is managed by an external CSI volume driver.
type CSIPersistentVolumeSource struct {
// Driver is the name of the driver to use for this volume.
// Required.
Expand Down
25 changes: 16 additions & 9 deletions pkg/apis/core/validation/validation.go
View file
Open in desktop
Expand Up @@ -1443,25 +1443,32 @@ func validateStorageOSPersistentVolumeSource(storageos *core.StorageOSPersistent
return allErrs
}

func validateCSIPersistentVolumeSource(csi *core.CSIPersistentVolumeSource, fldPath *field.Path) field.ErrorList {
func ValidateCSIDriverName(driverName string, fldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}

if !utilfeature.DefaultFeatureGate.Enabled(features.CSIPersistentVolume) {
allErrs = append(allErrs, field.Forbidden(fldPath, "CSIPersistentVolume disabled by feature-gate"))
if len(driverName) == 0 {
allErrs = append(allErrs, field.Required(fldPath, ""))
}

if len(csi.Driver) == 0 {
allErrs = append(allErrs, field.Required(fldPath.Child("driver"), ""))
if len(driverName) > 63 {
allErrs = append(allErrs, field.TooLong(fldPath, driverName, 63))
}

if len(csi.Driver) > 63 {
allErrs = append(allErrs, field.TooLong(fldPath.Child("driver"), csi.Driver, 63))
if !csiDriverNameRexp.MatchString(driverName) {
liggitt marked this conversation as resolved.
Show resolved Hide resolved
allErrs = append(allErrs, field.Invalid(fldPath, driverName, validation.RegexError(csiDriverNameRexpErrMsg, csiDriverNameRexpFmt, "csi-hostpath")))
}
return allErrs
}

if !csiDriverNameRexp.MatchString(csi.Driver) {
allErrs = append(allErrs, field.Invalid(fldPath.Child("driver"), csi.Driver, validation.RegexError(csiDriverNameRexpErrMsg, csiDriverNameRexpFmt, "csi-hostpath")))
func validateCSIPersistentVolumeSource(csi *core.CSIPersistentVolumeSource, fldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}

if !utilfeature.DefaultFeatureGate.Enabled(features.CSIPersistentVolume) {
allErrs = append(allErrs, field.Forbidden(fldPath, "CSIPersistentVolume disabled by feature-gate"))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this have the same issue where you can't delete the Pod object after downgrading to a version where the feature is disabled?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well, not the pod, since this is in PV, but the same issue, yes

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pushed a commit to https://github.com/liggitt/kubernetes/commits/csi-ga to resolve this field for this object. I think that looks like what we'll want to do more broadly (consider the existing field on update, and not block in validation based on feature enablement, since it can break update/delete scenarios)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks. I will pull your commit and update this branch.

}

allErrs = append(allErrs, ValidateCSIDriverName(csi.Driver, fldPath.Child("driver"))...)

if len(csi.VolumeHandle) == 0 {
allErrs = append(allErrs, field.Required(fldPath.Child("volumeHandle"), ""))
}
Expand Down
210 changes: 210 additions & 0 deletions pkg/apis/storage/v1/zz_generated.conversion.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

17 changes: 16 additions & 1 deletion pkg/apis/storage/validation/validation.go
View file
Open in desktop
Expand Up @@ -133,14 +133,28 @@ func validateAllowVolumeExpansion(allowExpand *bool, fldPath *field.Path) field.
return allErrs
}

// ValidateVolumeAttachment validates a VolumeAttachment.
// ValidateVolumeAttachment validates a VolumeAttachment. This function is common for v1 and v1beta1 objects,
func ValidateVolumeAttachment(volumeAttachment *storage.VolumeAttachment) field.ErrorList {
allErrs := apivalidation.ValidateObjectMeta(&volumeAttachment.ObjectMeta, false, apivalidation.ValidateClassName, field.NewPath("metadata"))
allErrs = append(allErrs, validateVolumeAttachmentSpec(&volumeAttachment.Spec, field.NewPath("spec"))...)
allErrs = append(allErrs, validateVolumeAttachmentStatus(&volumeAttachment.Status, field.NewPath("status"))...)
return allErrs
}

// ValidateVolumeAttachmentV1 validates a v1/VolumeAttachment. It contains only extra checks missing in
// ValidateVolumeAttachment.
func ValidateVolumeAttachmentV1(volumeAttachment *storage.VolumeAttachment) field.ErrorList {
allErrs := apivalidation.ValidateCSIDriverName(volumeAttachment.Spec.Attacher, field.NewPath("spec.attacher"))

if volumeAttachment.Spec.Source.PersistentVolumeName != nil {
pvName := *volumeAttachment.Spec.Source.PersistentVolumeName
for _, msg := range apivalidation.ValidatePersistentVolumeName(pvName, false) {
allErrs = append(allErrs, field.Invalid(field.NewPath("spec.source.persistentVolumeName"), pvName, msg))
}
}
return allErrs
}

// ValidateVolumeAttachmentSpec tests that the specified VolumeAttachmentSpec
// has valid data.
func validateVolumeAttachmentSpec(
Expand Down Expand Up @@ -218,6 +232,7 @@ func ValidateVolumeAttachmentUpdate(new, old *storage.VolumeAttachment) field.Er
allErrs := ValidateVolumeAttachment(new)

// Spec is read-only
// If this ever relaxes in the future, make sure to increment the Generation number in PrepareForUpdate
if !apiequality.Semantic.DeepEqual(old.Spec, new.Spec) {
allErrs = append(allErrs, field.Invalid(field.NewPath("spec"), new.Spec, "field is immutable"))
}
Expand Down