diff --git a/pkg/apis/core/validation/validation.go b/pkg/apis/core/validation/validation.go index fd8a67b2313..f5d9a496d7b 100644 --- a/pkg/apis/core/validation/validation.go +++ b/pkg/apis/core/validation/validation.go @@ -2027,12 +2027,15 @@ type PersistentVolumeClaimSpecValidationOptions struct { AllowReadWriteOncePod bool // Allow users to recover from previously failing expansion operation EnableRecoverFromExpansionFailure bool + // Allow assigning StorageClass to unbound PVCs retroactively + EnableRetroactiveDefaultStorageClass bool } func ValidationOptionsForPersistentVolumeClaim(pvc, oldPvc *core.PersistentVolumeClaim) PersistentVolumeClaimSpecValidationOptions { opts := PersistentVolumeClaimSpecValidationOptions{ - AllowReadWriteOncePod: utilfeature.DefaultFeatureGate.Enabled(features.ReadWriteOncePod), - EnableRecoverFromExpansionFailure: utilfeature.DefaultFeatureGate.Enabled(features.RecoverVolumeExpansionFailure), + AllowReadWriteOncePod: utilfeature.DefaultFeatureGate.Enabled(features.ReadWriteOncePod), + EnableRecoverFromExpansionFailure: utilfeature.DefaultFeatureGate.Enabled(features.RecoverVolumeExpansionFailure), + EnableRetroactiveDefaultStorageClass: utilfeature.DefaultFeatureGate.Enabled(features.RetroactiveDefaultStorageClass), } if oldPvc == nil { // If there's no old PVC, use the options based solely on feature enablement @@ -2168,7 +2171,7 @@ func ValidatePersistentVolumeClaimUpdate(newPvc, oldPvc *core.PersistentVolumeCl oldPvcClone.Spec.VolumeName = newPvcClone.Spec.VolumeName // +k8s:verify-mutation:reason=clone } - if validateStorageClassUpgrade(oldPvcClone.Annotations, newPvcClone.Annotations, + if validateStorageClassUpgradeFromAnnotation(oldPvcClone.Annotations, newPvcClone.Annotations, oldPvcClone.Spec.StorageClassName, newPvcClone.Spec.StorageClassName) { newPvcClone.Spec.StorageClassName = nil metav1.SetMetaDataAnnotation(&newPvcClone.ObjectMeta, core.BetaStorageClassAnnotation, oldPvcClone.Annotations[core.BetaStorageClassAnnotation]) @@ -2176,6 +2179,13 @@ 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 update from annotation to attribute failed we can attempt try to validate update from nil value. + if validateStorageClassUpgradeFromNil(oldPvc.Annotations, oldPvc.Spec.StorageClassName, newPvc.Spec.StorageClassName, opts) { + newPvcClone.Spec.StorageClassName = oldPvcClone.Spec.StorageClassName // +k8s:verify-mutation:reason=clone + } + // TODO: add a specific error with a hint that storage class name can not be changed + // (instead of letting spec comparison below return generic field forbidden error) } // lets make sure storage values are same. @@ -2216,7 +2226,7 @@ func ValidatePersistentVolumeClaimUpdate(newPvc, oldPvc *core.PersistentVolumeCl // 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 { +func validateStorageClassUpgradeFromAnnotation(oldAnnotations, newAnnotations map[string]string, oldScName, newScName *string) bool { oldSc, oldAnnotationExist := oldAnnotations[core.BetaStorageClassAnnotation] newScInAnnotation, newAnnotationExist := newAnnotations[core.BetaStorageClassAnnotation] return oldAnnotationExist /* condition 1 */ && @@ -2225,6 +2235,20 @@ func validateStorageClassUpgrade(oldAnnotations, newAnnotations map[string]strin (!newAnnotationExist || newScInAnnotation == oldSc) /* condition 4 */ } +// Provide an upgrade path from PVC with nil storage class. We allow update of +// StorageClassName only if following four conditions are met at the same time: +// 1. RetroactiveDefaultStorageClass FeatureGate is enabled +// 2. The new pvc's StorageClassName is not nil +// 3. The old pvc's StorageClassName is nil +// 4. The old pvc either does not have beta annotation set, or the beta annotation matches new pvc's StorageClassName +func validateStorageClassUpgradeFromNil(oldAnnotations map[string]string, oldScName, newScName *string, opts PersistentVolumeClaimSpecValidationOptions) bool { + oldAnnotation, oldAnnotationExist := oldAnnotations[core.BetaStorageClassAnnotation] + return opts.EnableRetroactiveDefaultStorageClass /* condition 1 */ && + newScName != nil /* condition 2 */ && + oldScName == nil /* condition 3 */ && + (!oldAnnotationExist || *newScName == oldAnnotation) /* condition 4 */ +} + var resizeStatusSet = sets.NewString(string(core.PersistentVolumeClaimNoExpansionInProgress), string(core.PersistentVolumeClaimControllerExpansionInProgress), string(core.PersistentVolumeClaimControllerExpansionFailed), diff --git a/pkg/apis/core/validation/validation_test.go b/pkg/apis/core/validation/validation_test.go index ce7008bdbc8..fe641a511e5 100644 --- a/pkg/apis/core/validation/validation_test.go +++ b/pkg/apis/core/validation/validation_test.go @@ -1192,6 +1192,17 @@ func testVolumeClaimStorageClassInSpec(name, namespace, scName string, spec core } } +func testVolumeClaimStorageClassNilInSpec(name, namespace string, spec core.PersistentVolumeClaimSpec) *core.PersistentVolumeClaim { + spec.StorageClassName = nil + return &core.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + Spec: spec, + } +} + func testVolumeSnapshotDataSourceInSpec(name string, kind string, apiGroup string) *core.PersistentVolumeClaimSpec { scName := "csi-plugin" dataSourceInSpec := core.PersistentVolumeClaimSpec{ @@ -1249,6 +1260,18 @@ func testVolumeClaimStorageClassInAnnotationAndSpec(name, namespace, scNameInAnn } } +func testVolumeClaimStorageClassInAnnotationAndNilInSpec(name, namespace, scNameInAnn string, spec core.PersistentVolumeClaimSpec) *core.PersistentVolumeClaim { + spec.StorageClassName = nil + return &core.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + Annotations: map[string]string{v1.BetaStorageClassAnnotation: scNameInAnn}, + }, + Spec: spec, + } +} + func testValidatePVC(t *testing.T, ephemeral bool) { invalidClassName := "-invalid-" validClassName := "valid" @@ -1972,6 +1995,28 @@ func TestValidatePersistentVolumeClaimUpdate(t *testing.T) { }, }) + validClaimStorageClassInSpecChanged := testVolumeClaimStorageClassInSpec("foo", "ns", "fast2", core.PersistentVolumeClaimSpec{ + AccessModes: []core.PersistentVolumeAccessMode{ + core.ReadOnlyMany, + }, + Resources: core.ResourceRequirements{ + Requests: core.ResourceList{ + core.ResourceName(core.ResourceStorage): resource.MustParse("10G"), + }, + }, + }) + + validClaimStorageClassNil := testVolumeClaimStorageClassNilInSpec("foo", "ns", 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, @@ -1995,6 +2040,18 @@ func TestValidatePersistentVolumeClaimUpdate(t *testing.T) { }, }) + validClaimStorageClassInAnnotationAndNilInSpec := testVolumeClaimStorageClassInAnnotationAndNilInSpec( + "foo", "ns", "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{ @@ -2116,10 +2173,11 @@ func TestValidatePersistentVolumeClaimUpdate(t *testing.T) { }) scenarios := map[string]struct { - isExpectedFailure bool - oldClaim *core.PersistentVolumeClaim - newClaim *core.PersistentVolumeClaim - enableRecoverFromExpansion bool + isExpectedFailure bool + oldClaim *core.PersistentVolumeClaim + newClaim *core.PersistentVolumeClaim + enableRecoverFromExpansion bool + enableRetroactiveDefaultStorageClass bool }{ "valid-update-volumeName-only": { isExpectedFailure: false, @@ -2230,6 +2288,69 @@ func TestValidatePersistentVolumeClaimUpdate(t *testing.T) { oldClaim: validClaimStorageClass, newClaim: validClaimStorageClassInSpec, }, + "valid-upgrade-nil-storage-class-spec-to-spec": { + isExpectedFailure: false, + oldClaim: validClaimStorageClassNil, + newClaim: validClaimStorageClassInSpec, + enableRetroactiveDefaultStorageClass: true, + // Feature enabled - change from nil sc name is valid if there is no beta annotation. + }, + "invalid-upgrade-nil-storage-class-spec-to-spec": { + isExpectedFailure: true, + oldClaim: validClaimStorageClassNil, + newClaim: validClaimStorageClassInSpec, + enableRetroactiveDefaultStorageClass: false, + // Feature disabled - change from nil sc name is invalid if there is no beta annotation. + }, + "invalid-upgrade-not-nil-storage-class-spec-to-spec": { + isExpectedFailure: true, + oldClaim: validClaimStorageClassInSpec, + newClaim: validClaimStorageClassInSpecChanged, + enableRetroactiveDefaultStorageClass: true, + // Feature enablement must not allow non nil value change. + }, + "invalid-upgrade-to-nil-storage-class-spec-to-spec": { + isExpectedFailure: true, + oldClaim: validClaimStorageClassInSpec, + newClaim: validClaimStorageClassNil, + enableRetroactiveDefaultStorageClass: true, + // Feature enablement must not allow change to nil value change. + }, + "valid-upgrade-storage-class-annotation-and-nil-spec-to-spec": { + isExpectedFailure: false, + oldClaim: validClaimStorageClassInAnnotationAndNilInSpec, + newClaim: validClaimStorageClassInAnnotationAndSpec, + enableRetroactiveDefaultStorageClass: false, + // Change from nil sc name is valid if annotations match. + }, + "valid-upgrade-storage-class-annotation-and-nil-spec-to-spec-retro": { + isExpectedFailure: false, + oldClaim: validClaimStorageClassInAnnotationAndNilInSpec, + newClaim: validClaimStorageClassInAnnotationAndSpec, + enableRetroactiveDefaultStorageClass: true, + // Change from nil sc name is valid if annotations match, feature enablement must not break this old behavior. + }, + "invalid-upgrade-storage-class-annotation-and-spec-to-spec": { + isExpectedFailure: true, + oldClaim: validClaimStorageClassInAnnotationAndSpec, + newClaim: validClaimStorageClassInSpecChanged, + enableRetroactiveDefaultStorageClass: false, + // Change from non nil sc name is invalid if annotations don't match. + }, + "invalid-upgrade-storage-class-annotation-and-spec-to-spec-retro": { + isExpectedFailure: true, + oldClaim: validClaimStorageClassInAnnotationAndSpec, + newClaim: validClaimStorageClassInSpecChanged, + enableRetroactiveDefaultStorageClass: true, + // Change from non nil sc name is invalid if annotations don't match, feature enablement must not break this old behavior. + }, + "invalid-upgrade-storage-class-annotation-and-no-spec": { + isExpectedFailure: true, + oldClaim: validClaimStorageClassInAnnotationAndNilInSpec, + newClaim: validClaimStorageClassInSpecChanged, + enableRetroactiveDefaultStorageClass: true, + // Change from nil sc name is invalid if annotations don't match, feature enablement must not break this old behavior. + }, "invalid-upgrade-storage-class-annotation-to-spec": { isExpectedFailure: true, oldClaim: validClaimStorageClass, @@ -2294,6 +2415,7 @@ func TestValidatePersistentVolumeClaimUpdate(t *testing.T) { for name, scenario := range scenarios { t.Run(name, func(t *testing.T) { defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.RecoverVolumeExpansionFailure, scenario.enableRecoverFromExpansion)() + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.RetroactiveDefaultStorageClass, scenario.enableRetroactiveDefaultStorageClass)() scenario.oldClaim.ResourceVersion = "1" scenario.newClaim.ResourceVersion = "1" opts := ValidationOptionsForPersistentVolumeClaim(scenario.newClaim, scenario.oldClaim) diff --git a/pkg/controller/volume/persistentvolume/pv_controller.go b/pkg/controller/volume/persistentvolume/pv_controller.go index 381633c4f0a..20c61c038b1 100644 --- a/pkg/controller/volume/persistentvolume/pv_controller.go +++ b/pkg/controller/volume/persistentvolume/pv_controller.go @@ -350,6 +350,14 @@ func (ctrl *PersistentVolumeController) syncUnboundClaim(ctx context.Context, cl klog.V(4).Infof("synchronizing unbound PersistentVolumeClaim[%s]: no volume found", claimToClaimKey(claim)) // No PV could be found // OBSERVATION: pvc is "Pending", will retry + + if utilfeature.DefaultFeatureGate.Enabled(features.RetroactiveDefaultStorageClass) { + klog.V(4).Infof("FeatureGate[%s] is enabled, attempting to assign storage class to unbound PersistentVolumeClaim[%s]", features.RetroactiveDefaultStorageClass, claimToClaimKey(claim)) + if claim, err = ctrl.assignDefaultStorageClass(claim); err != nil { + return fmt.Errorf("can't update PersistentVolumeClaim[%q]: %w", claimToClaimKey(claim), err) + } + } + switch { case delayBinding && !storagehelpers.IsDelayBindingProvisioning(claim): if err = ctrl.emitEventForUnboundDelayBindingClaim(claim); err != nil { @@ -918,6 +926,35 @@ func (ctrl *PersistentVolumeController) updateVolumePhaseWithEvent(volume *v1.Pe return newVol, nil } +// assignDefaultStorageClass updates the claim storage class if there is any, the claim is updated to the API server. +// Ignores claims that already have a storage class. +// TODO: if resync is ever changed to a larger period, we might need to change how we set the default class on existing unbound claims +func (ctrl *PersistentVolumeController) assignDefaultStorageClass(claim *v1.PersistentVolumeClaim) (*v1.PersistentVolumeClaim, error) { + if claim.Spec.StorageClassName != nil { + return claim, nil + } + + class, err := util.GetDefaultClass(ctrl.classLister) + if err != nil { + // It is safe to ignore errors here because it means we either could not list SCs or there is more than one default. + // TODO: do not ignore errors after this PR is merged: https://github.com/kubernetes/kubernetes/pull/110559 + klog.V(4).Infof("failed to get default storage class: %v", err) + return claim, nil + } else if class == nil { + klog.V(4).Infof("can not assign storage class to PersistentVolumeClaim[%s]: default storage class not found", claimToClaimKey(claim)) + return claim, nil + } + + klog.V(4).Infof("assigning StorageClass[%s] to PersistentVolumeClaim[%s]", class.Name, claimToClaimKey(claim)) + claim.Spec.StorageClassName = &class.Name + newClaim, err := ctrl.kubeClient.CoreV1().PersistentVolumeClaims(claim.GetNamespace()).Update(context.TODO(), claim, metav1.UpdateOptions{}) + if err != nil { + return claim, err + } + + return newClaim, nil +} + // bindVolumeToClaim modifies given volume to be bound to a claim and saves it to // API server. The claim is not modified in this method! func (ctrl *PersistentVolumeController) bindVolumeToClaim(volume *v1.PersistentVolume, claim *v1.PersistentVolumeClaim) (*v1.PersistentVolume, error) { diff --git a/pkg/controller/volume/persistentvolume/pv_controller_test.go b/pkg/controller/volume/persistentvolume/pv_controller_test.go index 661c32c4148..85e7ebd8af6 100644 --- a/pkg/controller/volume/persistentvolume/pv_controller_test.go +++ b/pkg/controller/volume/persistentvolume/pv_controller_test.go @@ -465,6 +465,20 @@ func makeStorageClass(scName string, mode *storagev1.VolumeBindingMode) *storage ObjectMeta: metav1.ObjectMeta{ Name: scName, }, + Provisioner: "kubernetes.io/no-provisioner", + VolumeBindingMode: mode, + } +} + +func makeDefaultStorageClass(scName string, mode *storagev1.VolumeBindingMode) *storagev1.StorageClass { + return &storagev1.StorageClass{ + ObjectMeta: metav1.ObjectMeta{ + Name: scName, + Annotations: map[string]string{ + util.IsDefaultStorageClassAnnotation: "true", + }, + }, + Provisioner: "kubernetes.io/no-provisioner", VolumeBindingMode: mode, } } @@ -728,3 +742,121 @@ func TestModifyDeletionFinalizers(t *testing.T) { }) } } + +func TestRetroactiveStorageClassAssignment(t *testing.T) { + // Enable RetroactiveDefaultStorageClass feature gate. + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.RetroactiveDefaultStorageClass, true)() + tests := []struct { + storageClasses []*storagev1.StorageClass + tests []controllerTest + }{ + // [Unit test set 15] - retroactive storage class assignment tests + { + storageClasses: []*storagev1.StorageClass{}, + tests: []controllerTest{ + { + name: "15-1 - pvc storage class is not assigned retroactively if there are no default storage classes", + initialVolumes: novolumes, + expectedVolumes: novolumes, + initialClaims: newClaimArray("claim15-1", "uid15-1", "1Gi", "", v1.ClaimPending, nil), + expectedClaims: newClaimArray("claim15-1", "uid15-1", "1Gi", "", v1.ClaimPending, nil), + expectedEvents: noevents, + errors: noerrors, + test: testSyncClaim, + }, + }, + }, + { + storageClasses: []*storagev1.StorageClass{ + makeDefaultStorageClass(classGold, &modeImmediate), + makeDefaultStorageClass(classSilver, &modeImmediate)}, + tests: []controllerTest{ + { + name: "15-2 - pvc storage class is not assigned retroactively if there are multiple default storage classes", + initialVolumes: novolumes, + expectedVolumes: novolumes, + initialClaims: newClaimArray("claim15-2", "uid15-2", "1Gi", "", v1.ClaimPending, nil), + expectedClaims: newClaimArray("claim15-2", "uid15-2", "1Gi", "", v1.ClaimPending, nil), + expectedEvents: noevents, + errors: noerrors, + test: testSyncClaim, + }, + }, + }, + { + storageClasses: []*storagev1.StorageClass{ + makeDefaultStorageClass(classGold, &modeImmediate), + makeStorageClass(classSilver, &modeImmediate), + }, + tests: []controllerTest{ + { + name: "15-3 - pvc storage class is not assigned retroactively if claim is already bound", + initialVolumes: novolumes, + expectedVolumes: novolumes, + initialClaims: newClaimArray("claim15-3", "uid15-3", "1Gi", "test", v1.ClaimBound, &classCopper, volume.AnnBoundByController, volume.AnnBindCompleted), + expectedClaims: newClaimArray("claim15-3", "uid15-3", "1Gi", "test", v1.ClaimLost, &classCopper, volume.AnnBoundByController, volume.AnnBindCompleted), + expectedEvents: noevents, + errors: noerrors, + test: testSyncClaim, + }, + }, + }, + { + storageClasses: []*storagev1.StorageClass{ + makeDefaultStorageClass(classGold, &modeImmediate), + makeStorageClass(classSilver, &modeImmediate), + }, + tests: []controllerTest{ + { + name: "15-4 - pvc storage class is not assigned retroactively if claim is already bound but annotations are missing", + initialVolumes: novolumes, + expectedVolumes: novolumes, + initialClaims: newClaimArray("claim15-4", "uid15-4", "1Gi", "test", v1.ClaimBound, &classCopper), + expectedClaims: newClaimArray("claim15-4", "uid15-4", "1Gi", "test", v1.ClaimPending, &classCopper), + expectedEvents: noevents, + errors: noerrors, + test: testSyncClaim, + }, + }, + }, + { + storageClasses: []*storagev1.StorageClass{ + makeDefaultStorageClass(classGold, &modeImmediate), + makeStorageClass(classSilver, &modeImmediate), + }, + tests: []controllerTest{ + { + name: "15-5 - pvc storage class is assigned retroactively if there is a default", + initialVolumes: novolumes, + expectedVolumes: novolumes, + initialClaims: newClaimArray("claim15-5", "uid15-5", "1Gi", "", v1.ClaimPending, nil), + expectedClaims: newClaimArray("claim15-5", "uid15-5", "1Gi", "", v1.ClaimPending, &classGold), + expectedEvents: noevents, + errors: noerrors, + test: testSyncClaim, + }, + }, + }, + { + storageClasses: []*storagev1.StorageClass{ + makeDefaultStorageClass(classGold, &modeImmediate), + makeStorageClass(classCopper, &modeImmediate), + }, + tests: []controllerTest{ + { + name: "15-6 - pvc storage class is not changed if claim is not bound but already has a storage class", + initialVolumes: novolumes, + expectedVolumes: novolumes, + initialClaims: newClaimArray("claim15-6", "uid15-6", "1Gi", "", v1.ClaimPending, &classCopper), + expectedClaims: newClaimArray("claim15-6", "uid15-6", "1Gi", "", v1.ClaimPending, &classCopper), + expectedEvents: noevents, + errors: noerrors, + test: testSyncClaim, + }, + }, + }, + } + for _, test := range tests { + runSyncTests(t, test.tests, test.storageClasses, nil) + } +} diff --git a/pkg/features/kube_features.go b/pkg/features/kube_features.go index b70a830bf52..c6fbfb05bbb 100644 --- a/pkg/features/kube_features.go +++ b/pkg/features/kube_features.go @@ -720,6 +720,13 @@ const ( // Allow users to recover from volume expansion failure RecoverVolumeExpansionFailure featuregate.Feature = "RecoverVolumeExpansionFailure" + // owner: @RomanBednar + // kep: http://kep.k8s.io/3333 + // alpha: v1.25 + // + // Allow assigning StorageClass to unbound PVCs retroactively + RetroactiveDefaultStorageClass featuregate.Feature = "RetroactiveDefaultStorageClass" + // owner: @mikedanese // alpha: v1.7 // beta: v1.12 @@ -1045,6 +1052,8 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS RecoverVolumeExpansionFailure: {Default: false, PreRelease: featuregate.Alpha}, + RetroactiveDefaultStorageClass: {Default: false, PreRelease: featuregate.Alpha}, + RotateKubeletServerCertificate: {Default: true, PreRelease: featuregate.Beta}, SeccompDefault: {Default: true, PreRelease: featuregate.Beta}, diff --git a/pkg/volume/util/storageclass.go b/pkg/volume/util/storageclass.go new file mode 100644 index 00000000000..5350bc8e963 --- /dev/null +++ b/pkg/volume/util/storageclass.go @@ -0,0 +1,76 @@ +/* +Copyright 2022 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package util + +import ( + "fmt" + storagev1 "k8s.io/api/storage/v1" + "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/labels" + storagev1listers "k8s.io/client-go/listers/storage/v1" + "k8s.io/klog/v2" +) + +const ( + // isDefaultStorageClassAnnotation represents a StorageClass annotation that + // marks a class as the default StorageClass + IsDefaultStorageClassAnnotation = "storageclass.kubernetes.io/is-default-class" + + // betaIsDefaultStorageClassAnnotation is the beta version of IsDefaultStorageClassAnnotation. + // TODO: remove Beta when no longer used + BetaIsDefaultStorageClassAnnotation = "storageclass.beta.kubernetes.io/is-default-class" +) + +// GetDefaultClass returns the default StorageClass from the store, or nil. +func GetDefaultClass(lister storagev1listers.StorageClassLister) (*storagev1.StorageClass, error) { + list, err := lister.List(labels.Everything()) + if err != nil { + return nil, err + } + + defaultClasses := []*storagev1.StorageClass{} + for _, class := range list { + if IsDefaultAnnotation(class.ObjectMeta) { + defaultClasses = append(defaultClasses, class) + klog.V(4).Infof("GetDefaultClass added: %s", class.Name) + } + } + + if len(defaultClasses) == 0 { + return nil, nil + } + if len(defaultClasses) > 1 { + klog.V(4).Infof("GetDefaultClass %d defaults found", len(defaultClasses)) + return nil, errors.NewInternalError(fmt.Errorf("%d default StorageClasses were found", len(defaultClasses))) + } + return defaultClasses[0], nil +} + +// IsDefaultAnnotation returns a boolean if the default storage class +// annotation is set +// TODO: remove Beta when no longer needed +func IsDefaultAnnotation(obj metav1.ObjectMeta) bool { + if obj.Annotations[IsDefaultStorageClassAnnotation] == "true" { + return true + } + if obj.Annotations[BetaIsDefaultStorageClassAnnotation] == "true" { + return true + } + + return false +} diff --git a/plugin/pkg/admission/storage/storageclass/setdefault/admission.go b/plugin/pkg/admission/storage/storageclass/setdefault/admission.go index 33f85d4d256..aa4c1c14ba7 100644 --- a/plugin/pkg/admission/storage/storageclass/setdefault/admission.go +++ b/plugin/pkg/admission/storage/storageclass/setdefault/admission.go @@ -20,19 +20,16 @@ import ( "context" "fmt" "io" + "k8s.io/kubernetes/pkg/volume/util" "k8s.io/klog/v2" - storagev1 "k8s.io/api/storage/v1" - "k8s.io/apimachinery/pkg/api/errors" - "k8s.io/apimachinery/pkg/labels" "k8s.io/apiserver/pkg/admission" genericadmissioninitializer "k8s.io/apiserver/pkg/admission/initializer" "k8s.io/client-go/informers" storagev1listers "k8s.io/client-go/listers/storage/v1" api "k8s.io/kubernetes/pkg/apis/core" "k8s.io/kubernetes/pkg/apis/core/helper" - storageutil "k8s.io/kubernetes/pkg/apis/storage/util" ) const ( @@ -108,7 +105,7 @@ func (a *claimDefaulterPlugin) Admit(ctx context.Context, attr admission.Attribu klog.V(4).Infof("no storage class for claim %s (generate: %s)", pvc.Name, pvc.GenerateName) - def, err := getDefaultClass(a.lister) + def, err := util.GetDefaultClass(a.lister) if err != nil { return admission.NewForbidden(attr, err) } @@ -121,28 +118,3 @@ func (a *claimDefaulterPlugin) Admit(ctx context.Context, attr admission.Attribu pvc.Spec.StorageClassName = &def.Name return nil } - -// getDefaultClass returns the default StorageClass from the store, or nil. -func getDefaultClass(lister storagev1listers.StorageClassLister) (*storagev1.StorageClass, error) { - list, err := lister.List(labels.Everything()) - if err != nil { - return nil, err - } - - defaultClasses := []*storagev1.StorageClass{} - for _, class := range list { - if storageutil.IsDefaultAnnotation(class.ObjectMeta) { - defaultClasses = append(defaultClasses, class) - klog.V(4).Infof("getDefaultClass added: %s", class.Name) - } - } - - if len(defaultClasses) == 0 { - return nil, nil - } - if len(defaultClasses) > 1 { - klog.V(4).Infof("getDefaultClass %d defaults found", len(defaultClasses)) - return nil, errors.NewInternalError(fmt.Errorf("%d default StorageClasses were found", len(defaultClasses))) - } - return defaultClasses[0], nil -} diff --git a/test/e2e/framework/pv/pv.go b/test/e2e/framework/pv/pv.go index f16cd7f0c8b..fa02940773d 100644 --- a/test/e2e/framework/pv/pv.go +++ b/test/e2e/framework/pv/pv.go @@ -34,6 +34,7 @@ import ( "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/types" clientset "k8s.io/client-go/kubernetes" + "k8s.io/kubernetes/pkg/volume/util" "k8s.io/kubernetes/test/e2e/framework" e2eskipper "k8s.io/kubernetes/test/e2e/framework/skipper" ) @@ -45,14 +46,6 @@ const ( // VolumeSelectorKey is the key for volume selector. VolumeSelectorKey = "e2e-pv-pool" - // isDefaultStorageClassAnnotation represents a StorageClass annotation that - // marks a class as the default StorageClass - isDefaultStorageClassAnnotation = "storageclass.kubernetes.io/is-default-class" - - // betaIsDefaultStorageClassAnnotation is the beta version of IsDefaultStorageClassAnnotation. - // TODO: remove Beta when no longer used - betaIsDefaultStorageClassAnnotation = "storageclass.beta.kubernetes.io/is-default-class" - // volumeGidAnnotationKey is the of the annotation on the PersistentVolume // object that specifies a supplemental GID. // it is copied from k8s.io/kubernetes/pkg/volume/util VolumeGidAnnotationKey @@ -826,7 +819,7 @@ func GetDefaultStorageClassName(c clientset.Interface) (string, error) { } var scName string for _, sc := range list.Items { - if isDefaultAnnotation(sc.ObjectMeta) { + if util.IsDefaultAnnotation(sc.ObjectMeta) { if len(scName) != 0 { return "", fmt.Errorf("Multiple default storage classes found: %q and %q", scName, sc.Name) } @@ -840,20 +833,6 @@ func GetDefaultStorageClassName(c clientset.Interface) (string, error) { return scName, nil } -// isDefaultAnnotation returns a boolean if the default storage class -// annotation is set -// TODO: remove Beta when no longer needed -func isDefaultAnnotation(obj metav1.ObjectMeta) bool { - if obj.Annotations[isDefaultStorageClassAnnotation] == "true" { - return true - } - if obj.Annotations[betaIsDefaultStorageClassAnnotation] == "true" { - return true - } - - return false -} - // SkipIfNoDefaultStorageClass skips tests if no default SC can be found. func SkipIfNoDefaultStorageClass(c clientset.Interface) { _, err := GetDefaultStorageClassName(c)