diff --git a/pkg/apis/apps/validation/validation.go b/pkg/apis/apps/validation/validation.go index 7429847c41f..a8e06a2bfa6 100644 --- a/pkg/apis/apps/validation/validation.go +++ b/pkg/apis/apps/validation/validation.go @@ -129,9 +129,8 @@ func ValidateStatefulSetSpec(spec *apps.StatefulSetSpec, fldPath *field.Path, op allErrs = append(allErrs, ValidatePersistentVolumeClaimRetentionPolicy(spec.PersistentVolumeClaimRetentionPolicy, fldPath.Child("persistentVolumeClaimRetentionPolicy"))...) allErrs = append(allErrs, apivalidation.ValidateNonnegativeField(int64(spec.Replicas), fldPath.Child("replicas"))...) - if utilfeature.DefaultFeatureGate.Enabled(features.StatefulSetMinReadySeconds) { - allErrs = append(allErrs, apivalidation.ValidateNonnegativeField(int64(spec.MinReadySeconds), fldPath.Child("minReadySeconds"))...) - } + allErrs = append(allErrs, apivalidation.ValidateNonnegativeField(int64(spec.MinReadySeconds), fldPath.Child("minReadySeconds"))...) + if spec.Selector == nil { allErrs = append(allErrs, field.Required(fldPath.Child("selector"), "")) } else { @@ -175,19 +174,14 @@ func ValidateStatefulSetUpdate(statefulSet, oldStatefulSet *apps.StatefulSet, op // statefulset updates aren't super common and general updates are likely to be touching spec, so we'll do this // deep copy right away. This avoids mutating our inputs newStatefulSetClone := statefulSet.DeepCopy() - newStatefulSetClone.Spec.Replicas = oldStatefulSet.Spec.Replicas // +k8s:verify-mutation:reason=clone - newStatefulSetClone.Spec.Template = oldStatefulSet.Spec.Template // +k8s:verify-mutation:reason=clone - newStatefulSetClone.Spec.UpdateStrategy = oldStatefulSet.Spec.UpdateStrategy // +k8s:verify-mutation:reason=clone - if utilfeature.DefaultFeatureGate.Enabled(features.StatefulSetMinReadySeconds) { - newStatefulSetClone.Spec.MinReadySeconds = oldStatefulSet.Spec.MinReadySeconds // +k8s:verify-mutation:reason=clone - } + newStatefulSetClone.Spec.Replicas = oldStatefulSet.Spec.Replicas // +k8s:verify-mutation:reason=clone + newStatefulSetClone.Spec.Template = oldStatefulSet.Spec.Template // +k8s:verify-mutation:reason=clone + newStatefulSetClone.Spec.UpdateStrategy = oldStatefulSet.Spec.UpdateStrategy // +k8s:verify-mutation:reason=clone + newStatefulSetClone.Spec.MinReadySeconds = oldStatefulSet.Spec.MinReadySeconds // +k8s:verify-mutation:reason=clone + newStatefulSetClone.Spec.PersistentVolumeClaimRetentionPolicy = oldStatefulSet.Spec.PersistentVolumeClaimRetentionPolicy // +k8s:verify-mutation:reason=clone if !apiequality.Semantic.DeepEqual(newStatefulSetClone.Spec, oldStatefulSet.Spec) { - if utilfeature.DefaultFeatureGate.Enabled(features.StatefulSetMinReadySeconds) { - allErrs = append(allErrs, field.Forbidden(field.NewPath("spec"), "updates to statefulset spec for fields other than 'replicas', 'template', 'updateStrategy', 'persistentVolumeClaimRetentionPolicy' and 'minReadySeconds' are forbidden")) - } else { - allErrs = append(allErrs, field.Forbidden(field.NewPath("spec"), "updates to statefulset spec for fields other than 'replicas', 'template', 'updateStrategy' and 'persistentVolumeClaimRetentionPolicy' are forbidden")) - } + allErrs = append(allErrs, field.Forbidden(field.NewPath("spec"), "updates to statefulset spec for fields other than 'replicas', 'template', 'updateStrategy', 'persistentVolumeClaimRetentionPolicy' and 'minReadySeconds' are forbidden")) } return allErrs @@ -201,9 +195,7 @@ func ValidateStatefulSetStatus(status *apps.StatefulSetStatus, fieldPath *field. allErrs = append(allErrs, apivalidation.ValidateNonnegativeField(int64(status.ReadyReplicas), fieldPath.Child("readyReplicas"))...) allErrs = append(allErrs, apivalidation.ValidateNonnegativeField(int64(status.CurrentReplicas), fieldPath.Child("currentReplicas"))...) allErrs = append(allErrs, apivalidation.ValidateNonnegativeField(int64(status.UpdatedReplicas), fieldPath.Child("updatedReplicas"))...) - if utilfeature.DefaultFeatureGate.Enabled(features.StatefulSetMinReadySeconds) { - allErrs = append(allErrs, apivalidation.ValidateNonnegativeField(int64(status.AvailableReplicas), fieldPath.Child("availableReplicas"))...) - } + allErrs = append(allErrs, apivalidation.ValidateNonnegativeField(int64(status.AvailableReplicas), fieldPath.Child("availableReplicas"))...) if status.ObservedGeneration != nil { allErrs = append(allErrs, apivalidation.ValidateNonnegativeField(int64(*status.ObservedGeneration), fieldPath.Child("observedGeneration"))...) } @@ -221,15 +213,12 @@ func ValidateStatefulSetStatus(status *apps.StatefulSetStatus, fieldPath *field. if status.UpdatedReplicas > status.Replicas { allErrs = append(allErrs, field.Invalid(fieldPath.Child("updatedReplicas"), status.UpdatedReplicas, msg)) } - if utilfeature.DefaultFeatureGate.Enabled(features.StatefulSetMinReadySeconds) { - if status.AvailableReplicas > status.Replicas { - allErrs = append(allErrs, field.Invalid(fieldPath.Child("availableReplicas"), status.AvailableReplicas, msg)) - } - if status.AvailableReplicas > status.ReadyReplicas { - allErrs = append(allErrs, field.Invalid(fieldPath.Child("availableReplicas"), status.AvailableReplicas, "cannot be greater than readyReplicas")) - } + if status.AvailableReplicas > status.Replicas { + allErrs = append(allErrs, field.Invalid(fieldPath.Child("availableReplicas"), status.AvailableReplicas, msg)) + } + if status.AvailableReplicas > status.ReadyReplicas { + allErrs = append(allErrs, field.Invalid(fieldPath.Child("availableReplicas"), status.AvailableReplicas, "cannot be greater than status.readyReplicas")) } - return allErrs } diff --git a/pkg/apis/apps/validation/validation_test.go b/pkg/apis/apps/validation/validation_test.go index 2e31dd6b84a..d33d86ff12c 100644 --- a/pkg/apis/apps/validation/validation_test.go +++ b/pkg/apis/apps/validation/validation_test.go @@ -690,39 +690,28 @@ func generateStatefulSetSpec(minSeconds int32) *apps.StatefulSetSpec { // TestValidateStatefulSetMinReadySeconds tests the StatefulSet Spec's minReadySeconds field func TestValidateStatefulSetMinReadySeconds(t *testing.T) { testCases := map[string]struct { - ss *apps.StatefulSetSpec - enableMinReadySeconds bool - expectErr bool + ss *apps.StatefulSetSpec + expectErr bool }{ "valid : minReadySeconds enabled, zero": { - ss: generateStatefulSetSpec(0), - enableMinReadySeconds: true, - expectErr: false, + ss: generateStatefulSetSpec(0), + expectErr: false, }, "invalid : minReadySeconds enabled, negative": { - ss: generateStatefulSetSpec(-1), - enableMinReadySeconds: true, - expectErr: true, + ss: generateStatefulSetSpec(-1), + expectErr: true, }, "valid : minReadySeconds enabled, very large value": { - ss: generateStatefulSetSpec(2147483647), - enableMinReadySeconds: true, - expectErr: false, + ss: generateStatefulSetSpec(2147483647), + expectErr: false, }, "invalid : minReadySeconds enabled, large negative": { - ss: generateStatefulSetSpec(-2147483648), - enableMinReadySeconds: true, - expectErr: true, - }, - "valid : minReadySeconds disabled, we don't validate anything": { - ss: generateStatefulSetSpec(-2147483648), - enableMinReadySeconds: false, - expectErr: false, + ss: generateStatefulSetSpec(-2147483648), + expectErr: true, }, } for tcName, tc := range testCases { t.Run(tcName, func(t *testing.T) { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.StatefulSetMinReadySeconds, tc.enableMinReadySeconds)() errs := ValidateStatefulSetSpec(tc.ss, field.NewPath("spec", "minReadySeconds"), corevalidation.PodValidationOptions{}) if tc.expectErr && len(errs) == 0 { @@ -739,16 +728,15 @@ func TestValidateStatefulSetStatus(t *testing.T) { observedGenerationMinusOne := int64(-1) collisionCountMinusOne := int32(-1) tests := []struct { - name string - replicas int32 - readyReplicas int32 - currentReplicas int32 - updatedReplicas int32 - availableReplicas int32 - enableMinReadySeconds bool - observedGeneration *int64 - collisionCount *int32 - expectedErr bool + name string + replicas int32 + readyReplicas int32 + currentReplicas int32 + updatedReplicas int32 + availableReplicas int32 + observedGeneration *int64 + collisionCount *int32 + expectedErr bool }{ { name: "valid status", @@ -833,64 +821,33 @@ func TestValidateStatefulSetStatus(t *testing.T) { expectedErr: true, }, { - name: "invalid: number of available replicas", - replicas: 3, - readyReplicas: 3, - currentReplicas: 2, - availableReplicas: int32(-1), - expectedErr: true, - enableMinReadySeconds: true, + name: "invalid: number of available replicas", + replicas: 3, + readyReplicas: 3, + currentReplicas: 2, + availableReplicas: int32(-1), + expectedErr: true, }, { - name: "invalid: available replicas greater than replicas", - replicas: 3, - readyReplicas: 3, - currentReplicas: 2, - availableReplicas: int32(4), - expectedErr: true, - enableMinReadySeconds: true, + name: "invalid: available replicas greater than replicas", + replicas: 3, + readyReplicas: 3, + currentReplicas: 2, + availableReplicas: int32(4), + expectedErr: true, }, { - name: "invalid: available replicas greater than ready replicas", - replicas: 3, - readyReplicas: 2, - currentReplicas: 2, - availableReplicas: int32(3), - expectedErr: true, - enableMinReadySeconds: true, - }, - { - name: "minReadySeconds flag not set, no validation: number of available replicas", - replicas: 3, - readyReplicas: 3, - currentReplicas: 2, - availableReplicas: int32(-1), - expectedErr: false, - enableMinReadySeconds: false, - }, - { - name: "minReadySeconds flag not set, no validation: available replicas greater than replicas", - replicas: 3, - readyReplicas: 3, - currentReplicas: 2, - availableReplicas: int32(4), - expectedErr: false, - enableMinReadySeconds: false, - }, - { - name: "minReadySeconds flag not set, no validation: available replicas greater than ready replicas", - replicas: 3, - readyReplicas: 2, - currentReplicas: 2, - availableReplicas: int32(3), - expectedErr: false, - enableMinReadySeconds: false, + name: "invalid: available replicas greater than ready replicas", + replicas: 3, + readyReplicas: 2, + currentReplicas: 2, + availableReplicas: int32(3), + expectedErr: true, }, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.StatefulSetMinReadySeconds, test.enableMinReadySeconds)() status := apps.StatefulSetStatus{ Replicas: test.replicas, ReadyReplicas: test.readyReplicas, diff --git a/pkg/controller/statefulset/stateful_set.go b/pkg/controller/statefulset/stateful_set.go index 7ae634f29fa..7c1b5c287c2 100644 --- a/pkg/controller/statefulset/stateful_set.go +++ b/pkg/controller/statefulset/stateful_set.go @@ -29,7 +29,6 @@ import ( "k8s.io/apimachinery/pkg/labels" utilruntime "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/apimachinery/pkg/util/wait" - utilfeature "k8s.io/apiserver/pkg/util/feature" appsinformers "k8s.io/client-go/informers/apps/v1" coreinformers "k8s.io/client-go/informers/core/v1" clientset "k8s.io/client-go/kubernetes" @@ -43,7 +42,6 @@ import ( podutil "k8s.io/kubernetes/pkg/api/v1/pod" "k8s.io/kubernetes/pkg/controller" "k8s.io/kubernetes/pkg/controller/history" - "k8s.io/kubernetes/pkg/features" "k8s.io/klog/v2" ) @@ -232,7 +230,7 @@ func (ssc *StatefulSetController) updatePod(old, cur interface{}) { // TODO: MinReadySeconds in the Pod will generate an Available condition to be added in // the Pod status which in turn will trigger a requeue of the owning replica set thus // having its status updated with the newly available replica. - if utilfeature.DefaultFeatureGate.Enabled(features.StatefulSetMinReadySeconds) && !podutil.IsPodReady(oldPod) && podutil.IsPodReady(curPod) && set.Spec.MinReadySeconds > 0 { + if !podutil.IsPodReady(oldPod) && podutil.IsPodReady(curPod) && set.Spec.MinReadySeconds > 0 { klog.V(2).Infof("StatefulSet %s will be enqueued after %ds for availability check", set.Name, set.Spec.MinReadySeconds) // Add a second to avoid milliseconds skew in AddAfter. // See https://github.com/kubernetes/kubernetes/issues/39785#issuecomment-279959133 for more info. @@ -485,7 +483,7 @@ func (ssc *StatefulSetController) syncStatefulSet(ctx context.Context, set *apps } klog.V(4).Infof("Successfully synced StatefulSet %s/%s successful", set.Namespace, set.Name) // One more sync to handle the clock skew. This is also helping in requeuing right after status update - if utilfeature.DefaultFeatureGate.Enabled(features.StatefulSetMinReadySeconds) && set.Spec.MinReadySeconds > 0 && status != nil && status.AvailableReplicas != *set.Spec.Replicas { + if set.Spec.MinReadySeconds > 0 && status != nil && status.AvailableReplicas != *set.Spec.Replicas { ssc.enqueueSSAfter(set, time.Duration(set.Spec.MinReadySeconds)*time.Second) } diff --git a/pkg/controller/statefulset/stateful_set_control.go b/pkg/controller/statefulset/stateful_set_control.go index d911ca5f0d2..cc281c0a710 100644 --- a/pkg/controller/statefulset/stateful_set_control.go +++ b/pkg/controller/statefulset/stateful_set_control.go @@ -298,14 +298,10 @@ func (ssc *defaultStatefulSetControl) updateStatefulSet( if isRunningAndReady(pods[i]) { status.ReadyReplicas++ // count the number of running and available replicas - if utilfeature.DefaultFeatureGate.Enabled(features.StatefulSetMinReadySeconds) { - if isRunningAndAvailable(pods[i], set.Spec.MinReadySeconds) { - status.AvailableReplicas++ - } - } else { - // If the featuregate is not enabled, all the ready replicas should be considered as available replicas - status.AvailableReplicas = status.ReadyReplicas + if isRunningAndAvailable(pods[i], set.Spec.MinReadySeconds) { + status.AvailableReplicas++ } + } // count the number of current and update replicas @@ -462,9 +458,7 @@ func (ssc *defaultStatefulSetControl) updateStatefulSet( // If we have a Pod that has been created but is not available we can not make progress. // We must ensure that all for each Pod, when we create it, all of its predecessors, with respect to its // ordinal, are Available. - // TODO: Since available is superset of Ready, once we have this featuregate enabled by default, we can remove the - // isRunningAndReady block as only Available pods should be brought down. - if utilfeature.DefaultFeatureGate.Enabled(features.StatefulSetMinReadySeconds) && !isRunningAndAvailable(replicas[i], set.Spec.MinReadySeconds) && monotonic { + if !isRunningAndAvailable(replicas[i], set.Spec.MinReadySeconds) && monotonic { klog.V(4).InfoS("StatefulSet is waiting for Pod to be Available", "statefulSet", klog.KObj(set), "pod", klog.KObj(replicas[i])) return &status, nil @@ -525,9 +519,7 @@ func (ssc *defaultStatefulSetControl) updateStatefulSet( return &status, nil } // if we are in monotonic mode and the condemned target is not the first unhealthy Pod, block. - // TODO: Since available is superset of Ready, once we have this featuregate enabled by default, we can remove the - // isRunningAndReady block as only Available pods should be brought down. - if utilfeature.DefaultFeatureGate.Enabled(features.StatefulSetMinReadySeconds) && !isRunningAndAvailable(condemned[target], set.Spec.MinReadySeconds) && monotonic && condemned[target] != firstUnhealthyPod { + if !isRunningAndAvailable(condemned[target], set.Spec.MinReadySeconds) && monotonic && condemned[target] != firstUnhealthyPod { klog.V(4).InfoS("StatefulSet is waiting for Pod to be Available prior to scale down", "statefulSet", klog.KObj(set), "pod", klog.KObj(firstUnhealthyPod)) return &status, nil diff --git a/pkg/controller/statefulset/stateful_set_control_test.go b/pkg/controller/statefulset/stateful_set_control_test.go index cda8f435d6d..7b85fb9ad06 100644 --- a/pkg/controller/statefulset/stateful_set_control_test.go +++ b/pkg/controller/statefulset/stateful_set_control_test.go @@ -2050,37 +2050,25 @@ func TestStatefulSetControlRollback(t *testing.T) { func TestStatefulSetAvailability(t *testing.T) { tests := []struct { - name string - inputSTS *apps.StatefulSet - expectedActiveReplicas int32 - readyDuration time.Duration - minReadySecondsFeaturegateEnabled bool + name string + inputSTS *apps.StatefulSet + expectedActiveReplicas int32 + readyDuration time.Duration }{ { - name: "replicas not running for required time, still will be available," + - " when minReadySeconds is disabled", - inputSTS: setMinReadySeconds(newStatefulSet(1), int32(3600)), - readyDuration: 0 * time.Minute, - expectedActiveReplicas: int32(1), - minReadySecondsFeaturegateEnabled: false, + name: "replicas running for required time, when minReadySeconds is enabled", + inputSTS: setMinReadySeconds(newStatefulSet(1), int32(3600)), + readyDuration: -120 * time.Minute, + expectedActiveReplicas: int32(1), }, { - name: "replicas running for required time, when minReadySeconds is enabled", - inputSTS: setMinReadySeconds(newStatefulSet(1), int32(3600)), - readyDuration: -120 * time.Minute, - expectedActiveReplicas: int32(1), - minReadySecondsFeaturegateEnabled: true, - }, - { - name: "replicas not running for required time, when minReadySeconds is enabled", - inputSTS: setMinReadySeconds(newStatefulSet(1), int32(3600)), - readyDuration: -30 * time.Minute, - expectedActiveReplicas: int32(0), - minReadySecondsFeaturegateEnabled: true, + name: "replicas not running for required time, when minReadySeconds is enabled", + inputSTS: setMinReadySeconds(newStatefulSet(1), int32(3600)), + readyDuration: -30 * time.Minute, + expectedActiveReplicas: int32(0), }, } for _, test := range tests { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.StatefulSetMinReadySeconds, test.minReadySecondsFeaturegateEnabled)() set := test.inputSTS client := fake.NewSimpleClientset(set) spc, _, ssc := setupController(client) diff --git a/pkg/controller/statefulset/stateful_set_status_updater_test.go b/pkg/controller/statefulset/stateful_set_status_updater_test.go index 22e608fcbb9..83c51bd873a 100644 --- a/pkg/controller/statefulset/stateful_set_status_updater_test.go +++ b/pkg/controller/statefulset/stateful_set_status_updater_test.go @@ -24,13 +24,10 @@ import ( apps "k8s.io/api/apps/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime" - utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/client-go/kubernetes/fake" appslisters "k8s.io/client-go/listers/apps/v1" core "k8s.io/client-go/testing" "k8s.io/client-go/tools/cache" - featuregatetesting "k8s.io/component-base/featuregate/testing" - "k8s.io/kubernetes/pkg/features" ) func TestStatefulSetUpdaterUpdatesSetStatus(t *testing.T) { @@ -128,7 +125,6 @@ func TestStatefulSetStatusUpdaterUpdateReplicasConflictFailure(t *testing.T) { } func TestStatefulSetStatusUpdaterGetAvailableReplicas(t *testing.T) { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.StatefulSetMinReadySeconds, true)() set := newStatefulSet(3) status := apps.StatefulSetStatus{ObservedGeneration: 1, Replicas: 2, AvailableReplicas: 3} fakeClient := &fake.Clientset{} diff --git a/pkg/features/kube_features.go b/pkg/features/kube_features.go index 31d73f3512c..7217bc588b6 100644 --- a/pkg/features/kube_features.go +++ b/pkg/features/kube_features.go @@ -733,6 +733,7 @@ const ( // kep: https://kep.k8s.io/2607 // alpha: v1.22 // beta: v1.23 + // GA: v1.25 // StatefulSetMinReadySeconds allows minReadySeconds to be respected by StatefulSet controller StatefulSetMinReadySeconds featuregate.Feature = "StatefulSetMinReadySeconds" @@ -995,7 +996,7 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS StatefulSetAutoDeletePVC: {Default: false, PreRelease: featuregate.Alpha}, - StatefulSetMinReadySeconds: {Default: true, PreRelease: featuregate.Beta}, + StatefulSetMinReadySeconds: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.27 SuspendJob: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.26 diff --git a/pkg/registry/apps/statefulset/strategy.go b/pkg/registry/apps/statefulset/strategy.go index 66dd8d014e5..1a7d2386d7e 100644 --- a/pkg/registry/apps/statefulset/strategy.go +++ b/pkg/registry/apps/statefulset/strategy.go @@ -116,12 +116,6 @@ func (statefulSetStrategy) PrepareForUpdate(ctx context.Context, obj, old runtim // newSvc.Spec.MyFeature = nil // } func dropStatefulSetDisabledFields(newSS *apps.StatefulSet, oldSS *apps.StatefulSet) { - if !utilfeature.DefaultFeatureGate.Enabled(features.StatefulSetMinReadySeconds) { - if !minReadySecondsFieldsInUse(oldSS) { - newSS.Spec.MinReadySeconds = int32(0) - } - } - if !utilfeature.DefaultFeatureGate.Enabled(features.StatefulSetAutoDeletePVC) { if oldSS == nil || oldSS.Spec.PersistentVolumeClaimRetentionPolicy == nil { newSS.Spec.PersistentVolumeClaimRetentionPolicy = nil @@ -134,17 +128,6 @@ func dropStatefulSetDisabledFields(newSS *apps.StatefulSet, oldSS *apps.Stateful } } -// minReadySecondsFieldsInUse returns true if fields related to StatefulSet minReadySeconds are set and -// are greater than 0 -func minReadySecondsFieldsInUse(ss *apps.StatefulSet) bool { - if ss == nil { - return false - } else if ss.Spec.MinReadySeconds >= 0 { - return true - } - return false -} - // Validate validates a new StatefulSet. func (statefulSetStrategy) Validate(ctx context.Context, obj runtime.Object) field.ErrorList { statefulSet := obj.(*apps.StatefulSet) diff --git a/pkg/registry/apps/statefulset/strategy_test.go b/pkg/registry/apps/statefulset/strategy_test.go index 887ec686ebe..ea9ae728c35 100644 --- a/pkg/registry/apps/statefulset/strategy_test.go +++ b/pkg/registry/apps/statefulset/strategy_test.go @@ -85,8 +85,7 @@ func TestStatefulSetStrategy(t *testing.T) { Status: apps.StatefulSetStatus{Replicas: 4}, } Strategy.PrepareForUpdate(ctx, validPs, ps) - t.Run("when minReadySeconds feature gate is enabled", func(t *testing.T) { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.StatefulSetMinReadySeconds, true)() + t.Run("StatefulSet minReadySeconds field validations on creation and updation", func(t *testing.T) { // Test creation ps := &apps.StatefulSet{ ObjectMeta: metav1.ObjectMeta{Name: "abc", Namespace: metav1.NamespaceDefault}, @@ -137,53 +136,6 @@ func TestStatefulSetStrategy(t *testing.T) { t.Errorf("expected minReadySeconds to not be changed %v", errs) } }) - t.Run("when minReadySeconds feature gate is disabled, the minReadySeconds should not be updated", - func(t *testing.T) { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.StatefulSetMinReadySeconds, false)() - // Test creation - ps := &apps.StatefulSet{ - ObjectMeta: metav1.ObjectMeta{Name: "abc", Namespace: metav1.NamespaceDefault}, - Spec: apps.StatefulSetSpec{ - PodManagementPolicy: apps.OrderedReadyPodManagement, - Selector: &metav1.LabelSelector{MatchLabels: validSelector}, - Template: validPodTemplate.Template, - UpdateStrategy: apps.StatefulSetUpdateStrategy{Type: apps.RollingUpdateStatefulSetStrategyType}, - MinReadySeconds: int32(-1), - }, - } - Strategy.PrepareForCreate(ctx, ps) - errs := Strategy.Validate(ctx, ps) - if len(errs) != 0 { - t.Errorf("StatefulSet creation should not have any issues but found %v", errs) - } - if ps.Spec.MinReadySeconds != 0 { - t.Errorf("if the StatefulSet is created with invalid value we expect it to be defaulted to 0 "+ - "but got %v", ps.Spec.MinReadySeconds) - } - - // Test Updation - validPs := &apps.StatefulSet{ - ObjectMeta: metav1.ObjectMeta{Name: ps.Name, Namespace: ps.Namespace, ResourceVersion: "1", Generation: 1}, - Spec: apps.StatefulSetSpec{ - PodManagementPolicy: apps.OrderedReadyPodManagement, - Selector: ps.Spec.Selector, - Template: validPodTemplate.Template, - UpdateStrategy: apps.StatefulSetUpdateStrategy{Type: apps.RollingUpdateStatefulSetStrategyType}, - MinReadySeconds: newMinReadySeconds, - }, - Status: apps.StatefulSetStatus{Replicas: 4}, - } - Strategy.PrepareForUpdate(ctx, validPs, ps) - errs = Strategy.ValidateUpdate(ctx, validPs, ps) - if len(errs) == 0 { - t.Errorf("updating only spec.Replicas is allowed on a statefulset: %v", errs) - } - expectedUpdateErrorString := "spec: Forbidden: updates to statefulset spec for fields other than" + - " 'replicas', 'template', 'updateStrategy' and 'persistentVolumeClaimRetentionPolicy' are forbidden" - if errs[0].Error() != expectedUpdateErrorString { - t.Errorf("expected error string %v", errs[0].Error()) - } - }) validPs = &apps.StatefulSet{ ObjectMeta: metav1.ObjectMeta{Name: ps.Name, Namespace: ps.Namespace, ResourceVersion: "1", Generation: 1}, @@ -379,75 +331,35 @@ func getMaxUnavailable(maxUnavailable int) *int { // TestDropStatefulSetDisabledFields tests if the drop functionality is working fine or not func TestDropStatefulSetDisabledFields(t *testing.T) { testCases := []struct { - name string - enableMinReadySeconds bool - enableMaxUnavailable bool - ss *apps.StatefulSet - oldSS *apps.StatefulSet - expectedSS *apps.StatefulSet + name string + enableMaxUnavailable bool + ss *apps.StatefulSet + oldSS *apps.StatefulSet + expectedSS *apps.StatefulSet }{ { - name: "no minReadySeconds, no update", - enableMinReadySeconds: false, - ss: &apps.StatefulSet{}, - oldSS: nil, - expectedSS: &apps.StatefulSet{}, + name: "set minReadySeconds, no update", + ss: generateStatefulSetWithMinReadySeconds(10), + oldSS: generateStatefulSetWithMinReadySeconds(20), + expectedSS: generateStatefulSetWithMinReadySeconds(10), }, { - name: "no minReadySeconds, irrespective of the current value, set to default value of 0", - enableMinReadySeconds: false, - ss: generateStatefulSetWithMinReadySeconds(2000), - oldSS: nil, - expectedSS: &apps.StatefulSet{Spec: apps.StatefulSetSpec{MinReadySeconds: int32(0)}}, + name: "set minReadySeconds, oldSS field set to nil", + ss: generateStatefulSetWithMinReadySeconds(10), + oldSS: nil, + expectedSS: generateStatefulSetWithMinReadySeconds(10), }, { - name: "no minReadySeconds, oldSS field set to 100, no update", - enableMinReadySeconds: false, - ss: generateStatefulSetWithMinReadySeconds(2000), - oldSS: generateStatefulSetWithMinReadySeconds(100), - expectedSS: generateStatefulSetWithMinReadySeconds(2000), + name: "set minReadySeconds, oldSS field is set to 0", + ss: generateStatefulSetWithMinReadySeconds(10), + oldSS: generateStatefulSetWithMinReadySeconds(0), + expectedSS: generateStatefulSetWithMinReadySeconds(10), }, { - name: "no minReadySeconds, oldSS field set to -1(invalid value), update to zero", - enableMinReadySeconds: false, - ss: generateStatefulSetWithMinReadySeconds(2000), - oldSS: generateStatefulSetWithMinReadySeconds(-1), - expectedSS: generateStatefulSetWithMinReadySeconds(0), - }, - { - name: "no minReadySeconds, oldSS field set to 0, no update", - enableMinReadySeconds: false, - ss: generateStatefulSetWithMinReadySeconds(2000), - oldSS: generateStatefulSetWithMinReadySeconds(0), - expectedSS: generateStatefulSetWithMinReadySeconds(2000), - }, - { - name: "set minReadySeconds, no update", - enableMinReadySeconds: true, - ss: generateStatefulSetWithMinReadySeconds(10), - oldSS: generateStatefulSetWithMinReadySeconds(20), - expectedSS: generateStatefulSetWithMinReadySeconds(10), - }, - { - name: "set minReadySeconds, oldSS field set to nil", - enableMinReadySeconds: true, - ss: generateStatefulSetWithMinReadySeconds(10), - oldSS: nil, - expectedSS: generateStatefulSetWithMinReadySeconds(10), - }, - { - name: "set minReadySeconds, oldSS field is set to 0", - enableMinReadySeconds: true, - ss: generateStatefulSetWithMinReadySeconds(10), - oldSS: generateStatefulSetWithMinReadySeconds(0), - expectedSS: generateStatefulSetWithMinReadySeconds(10), - }, - { - name: "MaxUnavailable not enabled, field not used", - enableMaxUnavailable: false, - ss: makeStatefulSetWithMaxUnavailable(nil), - oldSS: nil, - expectedSS: makeStatefulSetWithMaxUnavailable(nil), + name: "MaxUnavailable not enabled, field not used", + ss: makeStatefulSetWithMaxUnavailable(nil), + oldSS: nil, + expectedSS: makeStatefulSetWithMaxUnavailable(nil), }, { name: "MaxUnavailable not enabled, field used in new, not in old", @@ -481,7 +393,6 @@ func TestDropStatefulSetDisabledFields(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.MaxUnavailableStatefulSet, tc.enableMaxUnavailable)() - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.StatefulSetMinReadySeconds, tc.enableMinReadySeconds)() old := tc.oldSS.DeepCopy() dropStatefulSetDisabledFields(tc.ss, tc.oldSS) diff --git a/test/integration/statefulset/statefulset_test.go b/test/integration/statefulset/statefulset_test.go index 5c26be3f45b..f83d7da600b 100644 --- a/test/integration/statefulset/statefulset_test.go +++ b/test/integration/statefulset/statefulset_test.go @@ -245,26 +245,16 @@ func TestStatefulSetAvailable(t *testing.T) { totalReplicas int32 readyReplicas int32 activeReplicas int32 - enabled bool }{ { - name: "When feature gate is enabled, only certain replicas would become active", + name: "only certain replicas would become active", totalReplicas: 4, readyReplicas: 3, activeReplicas: 2, - enabled: true, - }, - { - name: "When feature gate is disabled, all the ready replicas would become active", - totalReplicas: 4, - readyReplicas: 3, - activeReplicas: 3, - enabled: false, }, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.StatefulSetMinReadySeconds, test.enabled)() closeFn, rm, informers, c := scSetup(t) defer closeFn() ns := framework.CreateNamespaceOrDie(c, "test-available-pods", t)