From 285ee41ba71c706e9aa334eb5f52f23f5a0e35d5 Mon Sep 17 00:00:00 2001 From: mlmhl Date: Sun, 14 Jan 2018 18:01:19 +0800 Subject: [PATCH] admit upgrading storage class of pvc from beta annotation to spec field --- pkg/apis/core/validation/validation.go | 30 +++++- pkg/apis/core/validation/validation_test.go | 111 ++++++++++++++++++++ 2 files changed, 137 insertions(+), 4 deletions(-) diff --git a/pkg/apis/core/validation/validation.go b/pkg/apis/core/validation/validation.go index b86889a163..7427b9cc89 100644 --- a/pkg/apis/core/validation/validation.go +++ b/pkg/apis/core/validation/validation.go @@ -1796,6 +1796,16 @@ func ValidatePersistentVolumeClaimUpdate(newPvc, oldPvc *core.PersistentVolumeCl oldPvcClone.Spec.VolumeName = newPvcClone.Spec.VolumeName } + if validateStorageClassUpgrade(oldPvcClone.Annotations, newPvcClone.Annotations, + oldPvcClone.Spec.StorageClassName, newPvcClone.Spec.StorageClassName) { + newPvcClone.Spec.StorageClassName = nil + metav1.SetMetaDataAnnotation(&newPvcClone.ObjectMeta, core.BetaStorageClassAnnotation, oldPvcClone.Annotations[core.BetaStorageClassAnnotation]) + } else { + // storageclass annotation should be immutable after creation + // TODO: remove Beta when no longer needed + allErrs = append(allErrs, ValidateImmutableAnnotation(newPvc.ObjectMeta.Annotations[v1.BetaStorageClassAnnotation], oldPvc.ObjectMeta.Annotations[v1.BetaStorageClassAnnotation], v1.BetaStorageClassAnnotation, field.NewPath("metadata"))...) + } + if utilfeature.DefaultFeatureGate.Enabled(features.ExpandPersistentVolumes) { // lets make sure storage values are same. if newPvc.Status.Phase == core.ClaimBound && newPvcClone.Spec.Resources.Requests != nil { @@ -1820,16 +1830,28 @@ func ValidatePersistentVolumeClaimUpdate(newPvc, oldPvc *core.PersistentVolumeCl } } - // storageclass annotation should be immutable after creation - // TODO: remove Beta when no longer needed - allErrs = append(allErrs, ValidateImmutableAnnotation(newPvc.ObjectMeta.Annotations[v1.BetaStorageClassAnnotation], oldPvc.ObjectMeta.Annotations[v1.BetaStorageClassAnnotation], v1.BetaStorageClassAnnotation, field.NewPath("metadata"))...) - if utilfeature.DefaultFeatureGate.Enabled(features.BlockVolume) { allErrs = append(allErrs, ValidateImmutableField(newPvc.Spec.VolumeMode, oldPvc.Spec.VolumeMode, field.NewPath("volumeMode"))...) } return allErrs } +// Provide an upgrade path from PVC with storage class specified in beta +// annotation to storage class specified in attribute. We allow update of +// StorageClassName only if following four conditions are met at the same time: +// 1. The old pvc's StorageClassAnnotation is set +// 2. The old pvc's StorageClassName is not set +// 3. The new pvc's StorageClassName is set and equal to the old value in annotation +// 4. If the new pvc's StorageClassAnnotation is set,it must be equal to the old pv/pvc's StorageClassAnnotation +func validateStorageClassUpgrade(oldAnnotations, newAnnotations map[string]string, oldScName, newScName *string) bool { + oldSc, oldAnnotationExist := oldAnnotations[core.BetaStorageClassAnnotation] + newScInAnnotation, newAnnotationExist := newAnnotations[core.BetaStorageClassAnnotation] + return oldAnnotationExist /* condition 1 */ && + oldScName == nil /* condition 2*/ && + (newScName != nil && *newScName == oldSc) /* condition 3 */ && + (!newAnnotationExist || newScInAnnotation == oldSc) /* condition 4 */ +} + // ValidatePersistentVolumeClaimStatusUpdate validates an update to status of a PersistentVolumeClaim func ValidatePersistentVolumeClaimStatusUpdate(newPvc, oldPvc *core.PersistentVolumeClaim) field.ErrorList { allErrs := ValidateObjectMetaUpdate(&newPvc.ObjectMeta, &oldPvc.ObjectMeta, field.NewPath("metadata")) diff --git a/pkg/apis/core/validation/validation_test.go b/pkg/apis/core/validation/validation_test.go index 442adecfab..d6aaf9fcb9 100644 --- a/pkg/apis/core/validation/validation_test.go +++ b/pkg/apis/core/validation/validation_test.go @@ -736,6 +736,29 @@ func testVolumeClaimAnnotation(name string, namespace string, ann string, annval } } +func testVolumeClaimStorageClassInSpec(name, namespace, scName string, spec core.PersistentVolumeClaimSpec) *core.PersistentVolumeClaim { + spec.StorageClassName = &scName + return &core.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + Spec: spec, + } +} + +func testVolumeClaimStorageClassInAnnotationAndSpec(name, namespace, scNameInAnn, scName string, spec core.PersistentVolumeClaimSpec) *core.PersistentVolumeClaim { + spec.StorageClassName = &scName + return &core.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + Annotations: map[string]string{v1.BetaStorageClassAnnotation: scNameInAnn}, + }, + Spec: spec, + } +} + func TestValidatePersistentVolumeClaim(t *testing.T) { invalidClassName := "-invalid-" validClassName := "valid" @@ -1251,6 +1274,52 @@ func TestValidatePersistentVolumeClaimUpdate(t *testing.T) { Phase: core.ClaimPending, }) + validClaimStorageClassInSpec := testVolumeClaimStorageClassInSpec("foo", "ns", "fast", core.PersistentVolumeClaimSpec{ + AccessModes: []core.PersistentVolumeAccessMode{ + core.ReadOnlyMany, + }, + Resources: core.ResourceRequirements{ + Requests: core.ResourceList{ + core.ResourceName(core.ResourceStorage): resource.MustParse("10G"), + }, + }, + }) + + invalidClaimStorageClassInSpec := testVolumeClaimStorageClassInSpec("foo", "ns", "fast2", core.PersistentVolumeClaimSpec{ + AccessModes: []core.PersistentVolumeAccessMode{ + core.ReadOnlyMany, + }, + Resources: core.ResourceRequirements{ + Requests: core.ResourceList{ + core.ResourceName(core.ResourceStorage): resource.MustParse("10G"), + }, + }, + }) + + validClaimStorageClassInAnnotationAndSpec := testVolumeClaimStorageClassInAnnotationAndSpec( + "foo", "ns", "fast", "fast", core.PersistentVolumeClaimSpec{ + AccessModes: []core.PersistentVolumeAccessMode{ + core.ReadOnlyMany, + }, + Resources: core.ResourceRequirements{ + Requests: core.ResourceList{ + core.ResourceName(core.ResourceStorage): resource.MustParse("10G"), + }, + }, + }) + + invalidClaimStorageClassInAnnotationAndSpec := testVolumeClaimStorageClassInAnnotationAndSpec( + "foo", "ns", "fast2", "fast", core.PersistentVolumeClaimSpec{ + AccessModes: []core.PersistentVolumeAccessMode{ + core.ReadOnlyMany, + }, + Resources: core.ResourceRequirements{ + Requests: core.ResourceList{ + core.ResourceName(core.ResourceStorage): resource.MustParse("10G"), + }, + }, + }) + scenarios := map[string]struct { isExpectedFailure bool oldClaim *core.PersistentVolumeClaim @@ -1412,6 +1481,48 @@ func TestValidatePersistentVolumeClaimUpdate(t *testing.T) { enableResize: true, enableBlock: false, }, + "valid-upgrade-storage-class-annotation-to-spec": { + isExpectedFailure: false, + oldClaim: validClaimStorageClass, + newClaim: validClaimStorageClassInSpec, + enableResize: false, + enableBlock: false, + }, + "invalid-upgrade-storage-class-annotation-to-spec": { + isExpectedFailure: true, + oldClaim: validClaimStorageClass, + newClaim: invalidClaimStorageClassInSpec, + enableResize: false, + enableBlock: false, + }, + "valid-upgrade-storage-class-annotation-to-annotation-and-spec": { + isExpectedFailure: false, + oldClaim: validClaimStorageClass, + newClaim: validClaimStorageClassInAnnotationAndSpec, + enableResize: false, + enableBlock: false, + }, + "invalid-upgrade-storage-class-annotation-to-annotation-and-spec": { + isExpectedFailure: true, + oldClaim: validClaimStorageClass, + newClaim: invalidClaimStorageClassInAnnotationAndSpec, + enableResize: false, + enableBlock: false, + }, + "invalid-upgrade-storage-class-in-spec": { + isExpectedFailure: true, + oldClaim: validClaimStorageClassInSpec, + newClaim: invalidClaimStorageClassInSpec, + enableResize: false, + enableBlock: false, + }, + "invalid-downgrade-storage-class-spec-to-annotation": { + isExpectedFailure: true, + oldClaim: validClaimStorageClassInSpec, + newClaim: validClaimStorageClass, + enableResize: false, + enableBlock: false, + }, } for name, scenario := range scenarios {