github/kubernetes
1
0
Fork 0
mirror of https://github.com/k3s-io/kubernetes.git synced 2025-08-13 22:05:59 +00:00
Code Issues Projects Releases Wiki Activity

Merge pull request #118102 from RomanBednar/retro-sc-assignment-ga

Browse Source 
graduate RetroactiveDefaultStorageClass feature to GA in 1.28
This commit is contained in:
Kubernetes Prow Robot 2023-06-27 20:46:32 -07:00 committed by GitHub
commit 960830bc66
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 55 additions and 106 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

23
pkg/apis/core/validation/validation.go
View File

@ -2019,18 +2019,15 @@ type PersistentVolumeClaimSpecValidationOptions struct {
AllowReadWriteOncePod bool AllowReadWriteOncePod bool
// Allow users to recover from previously failing expansion operation // Allow users to recover from previously failing expansion operation
EnableRecoverFromExpansionFailure bool EnableRecoverFromExpansionFailure bool
// Allow assigning StorageClass to unbound PVCs retroactively
EnableRetroactiveDefaultStorageClass bool
// Allow to validate the label value of the label selector // Allow to validate the label value of the label selector
AllowInvalidLabelValueInSelector bool AllowInvalidLabelValueInSelector bool
} }
func ValidationOptionsForPersistentVolumeClaim(pvc, oldPvc *core.PersistentVolumeClaim) PersistentVolumeClaimSpecValidationOptions { func ValidationOptionsForPersistentVolumeClaim(pvc, oldPvc *core.PersistentVolumeClaim) PersistentVolumeClaimSpecValidationOptions {
opts := PersistentVolumeClaimSpecValidationOptions{ opts := PersistentVolumeClaimSpecValidationOptions{
AllowReadWriteOncePod: utilfeature.DefaultFeatureGate.Enabled(features.ReadWriteOncePod), AllowReadWriteOncePod: utilfeature.DefaultFeatureGate.Enabled(features.ReadWriteOncePod),
EnableRecoverFromExpansionFailure: utilfeature.DefaultFeatureGate.Enabled(features.RecoverVolumeExpansionFailure), EnableRecoverFromExpansionFailure: utilfeature.DefaultFeatureGate.Enabled(features.RecoverVolumeExpansionFailure),
EnableRetroactiveDefaultStorageClass: utilfeature.DefaultFeatureGate.Enabled(features.RetroactiveDefaultStorageClass), AllowInvalidLabelValueInSelector: false,
AllowInvalidLabelValueInSelector: false,
} }
if oldPvc == nil { if oldPvc == nil {
// If there's no old PVC, use the options based solely on feature enablement // If there's no old PVC, use the options based solely on feature enablement
@ -2286,16 +2283,14 @@ func validateStorageClassUpgradeFromAnnotation(oldAnnotations, newAnnotations ma
// Provide an upgrade path from PVC with nil storage class. We allow update of // 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: // StorageClassName only if following four conditions are met at the same time:
// 1. RetroactiveDefaultStorageClass FeatureGate is enabled // 1. The new pvc's StorageClassName is not nil
// 2. The new pvc's StorageClassName is not nil // 2. The old pvc's StorageClassName is nil
// 3. The old pvc's StorageClassName is nil // 3. The old pvc either does not have beta annotation set, or the beta annotation matches new pvc's StorageClassName
// 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 { func validateStorageClassUpgradeFromNil(oldAnnotations map[string]string, oldScName, newScName *string, opts PersistentVolumeClaimSpecValidationOptions) bool {
oldAnnotation, oldAnnotationExist := oldAnnotations[core.BetaStorageClassAnnotation] oldAnnotation, oldAnnotationExist := oldAnnotations[core.BetaStorageClassAnnotation]
return opts.EnableRetroactiveDefaultStorageClass /* condition 1 */ && return newScName != nil /* condition 1 */ &&
newScName != nil /* condition 2 */ && oldScName == nil /* condition 2 */ &&
oldScName == nil /* condition 3 */ && (!oldAnnotationExist || *newScName == oldAnnotation) /* condition 3 */
(!oldAnnotationExist || *newScName == oldAnnotation) /* condition 4 */
} }
var resizeStatusSet = sets.NewString(string(core.PersistentVolumeClaimNoExpansionInProgress), var resizeStatusSet = sets.NewString(string(core.PersistentVolumeClaimNoExpansionInProgress),

106
pkg/apis/core/validation/validation_test.go
View File

@ -2420,11 +2420,10 @@ func TestValidatePersistentVolumeClaimUpdate(t *testing.T) {
}) })
scenarios := map[string]struct { scenarios := map[string]struct {
isExpectedFailure bool isExpectedFailure bool
oldClaim *core.PersistentVolumeClaim oldClaim *core.PersistentVolumeClaim
newClaim *core.PersistentVolumeClaim newClaim *core.PersistentVolumeClaim
enableRecoverFromExpansion bool enableRecoverFromExpansion bool
enableRetroactiveDefaultStorageClass bool
}{ }{
"valid-update-volumeName-only": { "valid-update-volumeName-only": {
isExpectedFailure: false, isExpectedFailure: false,
@ -2536,67 +2535,34 @@ func TestValidatePersistentVolumeClaimUpdate(t *testing.T) {
newClaim: validClaimStorageClassInSpec, newClaim: validClaimStorageClassInSpec,
}, },
"valid-upgrade-nil-storage-class-spec-to-spec": { "valid-upgrade-nil-storage-class-spec-to-spec": {
isExpectedFailure: false, isExpectedFailure: false,
oldClaim: validClaimStorageClassNil, oldClaim: validClaimStorageClassNil,
newClaim: validClaimStorageClassInSpec, 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": { "invalid-upgrade-not-nil-storage-class-spec-to-spec": {
isExpectedFailure: true, isExpectedFailure: true,
oldClaim: validClaimStorageClassInSpec, oldClaim: validClaimStorageClassInSpec,
newClaim: validClaimStorageClassInSpecChanged, newClaim: validClaimStorageClassInSpecChanged,
enableRetroactiveDefaultStorageClass: true,
// Feature enablement must not allow non nil value change.
}, },
"invalid-upgrade-to-nil-storage-class-spec-to-spec": { "invalid-upgrade-to-nil-storage-class-spec-to-spec": {
isExpectedFailure: true, isExpectedFailure: true,
oldClaim: validClaimStorageClassInSpec, oldClaim: validClaimStorageClassInSpec,
newClaim: validClaimStorageClassNil, 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": { "valid-upgrade-storage-class-annotation-and-nil-spec-to-spec-retro": {
isExpectedFailure: false, isExpectedFailure: false,
oldClaim: validClaimStorageClassInAnnotationAndNilInSpec, oldClaim: validClaimStorageClassInAnnotationAndNilInSpec,
newClaim: validClaimStorageClassInAnnotationAndSpec, 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": { "invalid-upgrade-storage-class-annotation-and-spec-to-spec-retro": {
isExpectedFailure: true, isExpectedFailure: true,
oldClaim: validClaimStorageClassInAnnotationAndSpec, oldClaim: validClaimStorageClassInAnnotationAndSpec,
newClaim: validClaimStorageClassInSpecChanged, 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": { "invalid-upgrade-storage-class-annotation-and-no-spec": {
isExpectedFailure: true, isExpectedFailure: true,
oldClaim: validClaimStorageClassInAnnotationAndNilInSpec, oldClaim: validClaimStorageClassInAnnotationAndNilInSpec,
newClaim: validClaimStorageClassInSpecChanged, 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": { "invalid-upgrade-storage-class-annotation-to-spec": {
isExpectedFailure: true, isExpectedFailure: true,
@ -2662,7 +2628,6 @@ func TestValidatePersistentVolumeClaimUpdate(t *testing.T) {
for name, scenario := range scenarios { for name, scenario := range scenarios {
t.Run(name, func(t *testing.T) { t.Run(name, func(t *testing.T) {
defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.RecoverVolumeExpansionFailure, scenario.enableRecoverFromExpansion)() 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.oldClaim.ResourceVersion = "1"
scenario.newClaim.ResourceVersion = "1" scenario.newClaim.ResourceVersion = "1"
opts := ValidationOptionsForPersistentVolumeClaim(scenario.newClaim, scenario.oldClaim) opts := ValidationOptionsForPersistentVolumeClaim(scenario.newClaim, scenario.oldClaim)
@ -2687,45 +2652,40 @@ func TestValidationOptionsForPersistentVolumeClaim(t *testing.T) {
oldPvc: nil, oldPvc: nil,
enableReadWriteOncePod: true, enableReadWriteOncePod: true,
expectValidationOpts: PersistentVolumeClaimSpecValidationOptions{ expectValidationOpts: PersistentVolumeClaimSpecValidationOptions{
AllowReadWriteOncePod: true, AllowReadWriteOncePod: true,
EnableRecoverFromExpansionFailure: false, EnableRecoverFromExpansionFailure: false,
EnableRetroactiveDefaultStorageClass: true,
}, },
}, },
"rwop allowed because feature enabled": { "rwop allowed because feature enabled": {
oldPvc: pvcWithAccessModes([]core.PersistentVolumeAccessMode{core.ReadWriteOnce}), oldPvc: pvcWithAccessModes([]core.PersistentVolumeAccessMode{core.ReadWriteOnce}),
enableReadWriteOncePod: true, enableReadWriteOncePod: true,
expectValidationOpts: PersistentVolumeClaimSpecValidationOptions{ expectValidationOpts: PersistentVolumeClaimSpecValidationOptions{
AllowReadWriteOncePod: true, AllowReadWriteOncePod: true,
EnableRecoverFromExpansionFailure: false, EnableRecoverFromExpansionFailure: false,
EnableRetroactiveDefaultStorageClass: true,
}, },
}, },
"rwop not allowed because not used and feature disabled": { "rwop not allowed because not used and feature disabled": {
oldPvc: pvcWithAccessModes([]core.PersistentVolumeAccessMode{core.ReadWriteOnce}), oldPvc: pvcWithAccessModes([]core.PersistentVolumeAccessMode{core.ReadWriteOnce}),
enableReadWriteOncePod: false, enableReadWriteOncePod: false,
expectValidationOpts: PersistentVolumeClaimSpecValidationOptions{ expectValidationOpts: PersistentVolumeClaimSpecValidationOptions{
AllowReadWriteOncePod: false, AllowReadWriteOncePod: false,
EnableRecoverFromExpansionFailure: false, EnableRecoverFromExpansionFailure: false,
EnableRetroactiveDefaultStorageClass: true,
}, },
}, },
"rwop allowed because used and feature enabled": { "rwop allowed because used and feature enabled": {
oldPvc: pvcWithAccessModes([]core.PersistentVolumeAccessMode{core.ReadWriteOncePod}), oldPvc: pvcWithAccessModes([]core.PersistentVolumeAccessMode{core.ReadWriteOncePod}),
enableReadWriteOncePod: true, enableReadWriteOncePod: true,
expectValidationOpts: PersistentVolumeClaimSpecValidationOptions{ expectValidationOpts: PersistentVolumeClaimSpecValidationOptions{
AllowReadWriteOncePod: true, AllowReadWriteOncePod: true,
EnableRecoverFromExpansionFailure: false, EnableRecoverFromExpansionFailure: false,
EnableRetroactiveDefaultStorageClass: true,
}, },
}, },
"rwop allowed because used and feature disabled": { "rwop allowed because used and feature disabled": {
oldPvc: pvcWithAccessModes([]core.PersistentVolumeAccessMode{core.ReadWriteOncePod}), oldPvc: pvcWithAccessModes([]core.PersistentVolumeAccessMode{core.ReadWriteOncePod}),
enableReadWriteOncePod: false, enableReadWriteOncePod: false,
expectValidationOpts: PersistentVolumeClaimSpecValidationOptions{ expectValidationOpts: PersistentVolumeClaimSpecValidationOptions{
AllowReadWriteOncePod: true, AllowReadWriteOncePod: true,
EnableRecoverFromExpansionFailure: false, EnableRecoverFromExpansionFailure: false,
EnableRetroactiveDefaultStorageClass: true,
}, },
}, },
} }
@ -2736,7 +2696,7 @@ func TestValidationOptionsForPersistentVolumeClaim(t *testing.T) {
opts := ValidationOptionsForPersistentVolumeClaim(nil, tc.oldPvc) opts := ValidationOptionsForPersistentVolumeClaim(nil, tc.oldPvc)
if opts != tc.expectValidationOpts { if opts != tc.expectValidationOpts {
t.Errorf("Expected opts: %+v, received: %+v", opts, tc.expectValidationOpts) t.Errorf("Expected opts: %+v, received: %+v", tc.expectValidationOpts, opts)
} }
}) })
} }

22
pkg/controller/volume/persistentvolume/pv_controller.go
View File

@ -349,18 +349,16 @@ func (ctrl *PersistentVolumeController) syncUnboundClaim(ctx context.Context, cl
// No PV could be found // No PV could be found
// OBSERVATION: pvc is "Pending", will retry // OBSERVATION: pvc is "Pending", will retry
if utilfeature.DefaultFeatureGate.Enabled(features.RetroactiveDefaultStorageClass) { logger.V(4).Info("Attempting to assign storage class to unbound PersistentVolumeClaim", "PVC", klog.KObj(claim))
logger.V(4).Info("FeatureGate is enabled, attempting to assign storage class to unbound PersistentVolumeClaim", "featureGate", features.RetroactiveDefaultStorageClass, "PVC", klog.KObj(claim)) updated, err := ctrl.assignDefaultStorageClass(ctx, claim)
updated, err := ctrl.assignDefaultStorageClass(ctx, claim) if err != nil {
if err != nil { metrics.RecordRetroactiveStorageClassMetric(false)
metrics.RecordRetroactiveStorageClassMetric(false) return fmt.Errorf("can't update PersistentVolumeClaim[%q]: %w", claimToClaimKey(claim), err)
return fmt.Errorf("can't update PersistentVolumeClaim[%q]: %w", claimToClaimKey(claim), err) }
} if updated {
if updated { logger.V(4).Info("PersistentVolumeClaim update successful, restarting claim sync", "PVC", klog.KObj(claim))
logger.V(4).Info("PersistentVolumeClaim update successful, restarting claim sync", "PVC", klog.KObj(claim)) metrics.RecordRetroactiveStorageClassMetric(true)
metrics.RecordRetroactiveStorageClassMetric(true) return nil
return nil
}
} }
switch { switch {

2
pkg/controller/volume/persistentvolume/pv_controller_test.go
View File

@ -754,8 +754,6 @@ func TestModifyDeletionFinalizers(t *testing.T) {
} }
func TestRetroactiveStorageClassAssignment(t *testing.T) { func TestRetroactiveStorageClassAssignment(t *testing.T) {
// Enable RetroactiveDefaultStorageClass feature gate.
defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.RetroactiveDefaultStorageClass, true)()
tests := []struct { tests := []struct {
storageClasses []*storagev1.StorageClass storageClasses []*storagev1.StorageClass
tests []controllerTest tests []controllerTest

4
pkg/features/kube_features.go
View File

@ -668,6 +668,8 @@ const (
// owner: @RomanBednar // owner: @RomanBednar
// kep: https://kep.k8s.io/3333 // kep: https://kep.k8s.io/3333
// alpha: v1.25 // alpha: v1.25
// beta: 1.26
// stable: v1.28
// //
// Allow assigning StorageClass to unbound PVCs retroactively // Allow assigning StorageClass to unbound PVCs retroactively
RetroactiveDefaultStorageClass featuregate.Feature = "RetroactiveDefaultStorageClass" RetroactiveDefaultStorageClass featuregate.Feature = "RetroactiveDefaultStorageClass"
@ -1026,7 +1028,7 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS
RecoverVolumeExpansionFailure: {Default: false, PreRelease: featuregate.Alpha}, RecoverVolumeExpansionFailure: {Default: false, PreRelease: featuregate.Alpha},
RetroactiveDefaultStorageClass: {Default: true, PreRelease: featuregate.Beta}, RetroactiveDefaultStorageClass: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.29
RotateKubeletServerCertificate: {Default: true, PreRelease: featuregate.Beta}, RotateKubeletServerCertificate: {Default: true, PreRelease: featuregate.Beta},

4
test/integration/volume/persistent_volumes_test.go
View File

@ -19,9 +19,6 @@ package volume
import ( import (
"context" "context"
"fmt" "fmt"
utilfeature "k8s.io/apiserver/pkg/util/feature"
featuregatetesting "k8s.io/component-base/featuregate/testing"
"k8s.io/kubernetes/pkg/features"
"math/rand" "math/rand"
"os" "os"
"strconv" "strconv"
@ -1045,7 +1042,6 @@ func TestPersistentVolumeMultiPVsDiffAccessModes(t *testing.T) {
// assignment and binding of PVCs with storage class name set to nil or "" with // assignment and binding of PVCs with storage class name set to nil or "" with
// and without presence of a default SC. // and without presence of a default SC.
func TestRetroactiveStorageClassAssignment(t *testing.T) { func TestRetroactiveStorageClassAssignment(t *testing.T) {
defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.RetroactiveDefaultStorageClass, true)()
s := kubeapiservertesting.StartTestServerOrDie(t, nil, []string{"--disable-admission-plugins=DefaultStorageClass"}, framework.SharedEtcd()) s := kubeapiservertesting.StartTestServerOrDie(t, nil, []string{"--disable-admission-plugins=DefaultStorageClass"}, framework.SharedEtcd())
defer s.TearDownFn() defer s.TearDownFn()
namespaceName := "retro-pvc-sc" namespaceName := "retro-pvc-sc"