Promote minReadySeconds to GA

This commit is contained in:
Ravi Gudimetla 2022-06-30 18:56:34 -04:00
parent d2cea9475b
commit 9144250a92
10 changed files with 95 additions and 290 deletions

View File

@ -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, ValidatePersistentVolumeClaimRetentionPolicy(spec.PersistentVolumeClaimRetentionPolicy, fldPath.Child("persistentVolumeClaimRetentionPolicy"))...)
allErrs = append(allErrs, apivalidation.ValidateNonnegativeField(int64(spec.Replicas), fldPath.Child("replicas"))...) 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 { if spec.Selector == nil {
allErrs = append(allErrs, field.Required(fldPath.Child("selector"), "")) allErrs = append(allErrs, field.Required(fldPath.Child("selector"), ""))
} else { } else {
@ -178,16 +177,11 @@ func ValidateStatefulSetUpdate(statefulSet, oldStatefulSet *apps.StatefulSet, op
newStatefulSetClone.Spec.Replicas = oldStatefulSet.Spec.Replicas // +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.Template = oldStatefulSet.Spec.Template // +k8s:verify-mutation:reason=clone
newStatefulSetClone.Spec.UpdateStrategy = oldStatefulSet.Spec.UpdateStrategy // +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.MinReadySeconds = oldStatefulSet.Spec.MinReadySeconds // +k8s:verify-mutation:reason=clone
}
newStatefulSetClone.Spec.PersistentVolumeClaimRetentionPolicy = oldStatefulSet.Spec.PersistentVolumeClaimRetentionPolicy // +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 !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")) 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"))
}
} }
return allErrs 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.ReadyReplicas), fieldPath.Child("readyReplicas"))...)
allErrs = append(allErrs, apivalidation.ValidateNonnegativeField(int64(status.CurrentReplicas), fieldPath.Child("currentReplicas"))...) allErrs = append(allErrs, apivalidation.ValidateNonnegativeField(int64(status.CurrentReplicas), fieldPath.Child("currentReplicas"))...)
allErrs = append(allErrs, apivalidation.ValidateNonnegativeField(int64(status.UpdatedReplicas), fieldPath.Child("updatedReplicas"))...) 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 { if status.ObservedGeneration != nil {
allErrs = append(allErrs, apivalidation.ValidateNonnegativeField(int64(*status.ObservedGeneration), fieldPath.Child("observedGeneration"))...) 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 { if status.UpdatedReplicas > status.Replicas {
allErrs = append(allErrs, field.Invalid(fieldPath.Child("updatedReplicas"), status.UpdatedReplicas, msg)) allErrs = append(allErrs, field.Invalid(fieldPath.Child("updatedReplicas"), status.UpdatedReplicas, msg))
} }
if utilfeature.DefaultFeatureGate.Enabled(features.StatefulSetMinReadySeconds) {
if status.AvailableReplicas > status.Replicas { if status.AvailableReplicas > status.Replicas {
allErrs = append(allErrs, field.Invalid(fieldPath.Child("availableReplicas"), status.AvailableReplicas, msg)) allErrs = append(allErrs, field.Invalid(fieldPath.Child("availableReplicas"), status.AvailableReplicas, msg))
} }
if status.AvailableReplicas > status.ReadyReplicas { if status.AvailableReplicas > status.ReadyReplicas {
allErrs = append(allErrs, field.Invalid(fieldPath.Child("availableReplicas"), status.AvailableReplicas, "cannot be greater than readyReplicas")) allErrs = append(allErrs, field.Invalid(fieldPath.Child("availableReplicas"), status.AvailableReplicas, "cannot be greater than status.readyReplicas"))
} }
}
return allErrs return allErrs
} }

View File

@ -691,38 +691,27 @@ func generateStatefulSetSpec(minSeconds int32) *apps.StatefulSetSpec {
func TestValidateStatefulSetMinReadySeconds(t *testing.T) { func TestValidateStatefulSetMinReadySeconds(t *testing.T) {
testCases := map[string]struct { testCases := map[string]struct {
ss *apps.StatefulSetSpec ss *apps.StatefulSetSpec
enableMinReadySeconds bool
expectErr bool expectErr bool
}{ }{
"valid : minReadySeconds enabled, zero": { "valid : minReadySeconds enabled, zero": {
ss: generateStatefulSetSpec(0), ss: generateStatefulSetSpec(0),
enableMinReadySeconds: true,
expectErr: false, expectErr: false,
}, },
"invalid : minReadySeconds enabled, negative": { "invalid : minReadySeconds enabled, negative": {
ss: generateStatefulSetSpec(-1), ss: generateStatefulSetSpec(-1),
enableMinReadySeconds: true,
expectErr: true, expectErr: true,
}, },
"valid : minReadySeconds enabled, very large value": { "valid : minReadySeconds enabled, very large value": {
ss: generateStatefulSetSpec(2147483647), ss: generateStatefulSetSpec(2147483647),
enableMinReadySeconds: true,
expectErr: false, expectErr: false,
}, },
"invalid : minReadySeconds enabled, large negative": { "invalid : minReadySeconds enabled, large negative": {
ss: generateStatefulSetSpec(-2147483648), ss: generateStatefulSetSpec(-2147483648),
enableMinReadySeconds: true,
expectErr: true, expectErr: true,
}, },
"valid : minReadySeconds disabled, we don't validate anything": {
ss: generateStatefulSetSpec(-2147483648),
enableMinReadySeconds: false,
expectErr: false,
},
} }
for tcName, tc := range testCases { for tcName, tc := range testCases {
t.Run(tcName, func(t *testing.T) { 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"), errs := ValidateStatefulSetSpec(tc.ss, field.NewPath("spec", "minReadySeconds"),
corevalidation.PodValidationOptions{}) corevalidation.PodValidationOptions{})
if tc.expectErr && len(errs) == 0 { if tc.expectErr && len(errs) == 0 {
@ -745,7 +734,6 @@ func TestValidateStatefulSetStatus(t *testing.T) {
currentReplicas int32 currentReplicas int32
updatedReplicas int32 updatedReplicas int32
availableReplicas int32 availableReplicas int32
enableMinReadySeconds bool
observedGeneration *int64 observedGeneration *int64
collisionCount *int32 collisionCount *int32
expectedErr bool expectedErr bool
@ -839,7 +827,6 @@ func TestValidateStatefulSetStatus(t *testing.T) {
currentReplicas: 2, currentReplicas: 2,
availableReplicas: int32(-1), availableReplicas: int32(-1),
expectedErr: true, expectedErr: true,
enableMinReadySeconds: true,
}, },
{ {
name: "invalid: available replicas greater than replicas", name: "invalid: available replicas greater than replicas",
@ -848,7 +835,6 @@ func TestValidateStatefulSetStatus(t *testing.T) {
currentReplicas: 2, currentReplicas: 2,
availableReplicas: int32(4), availableReplicas: int32(4),
expectedErr: true, expectedErr: true,
enableMinReadySeconds: true,
}, },
{ {
name: "invalid: available replicas greater than ready replicas", name: "invalid: available replicas greater than ready replicas",
@ -857,40 +843,11 @@ func TestValidateStatefulSetStatus(t *testing.T) {
currentReplicas: 2, currentReplicas: 2,
availableReplicas: int32(3), availableReplicas: int32(3),
expectedErr: true, 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,
}, },
} }
for _, test := range tests { for _, test := range tests {
t.Run(test.name, func(t *testing.T) { t.Run(test.name, func(t *testing.T) {
defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.StatefulSetMinReadySeconds, test.enableMinReadySeconds)()
status := apps.StatefulSetStatus{ status := apps.StatefulSetStatus{
Replicas: test.replicas, Replicas: test.replicas,
ReadyReplicas: test.readyReplicas, ReadyReplicas: test.readyReplicas,

View File

@ -29,7 +29,6 @@ import (
"k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/labels"
utilruntime "k8s.io/apimachinery/pkg/util/runtime" utilruntime "k8s.io/apimachinery/pkg/util/runtime"
"k8s.io/apimachinery/pkg/util/wait" "k8s.io/apimachinery/pkg/util/wait"
utilfeature "k8s.io/apiserver/pkg/util/feature"
appsinformers "k8s.io/client-go/informers/apps/v1" appsinformers "k8s.io/client-go/informers/apps/v1"
coreinformers "k8s.io/client-go/informers/core/v1" coreinformers "k8s.io/client-go/informers/core/v1"
clientset "k8s.io/client-go/kubernetes" clientset "k8s.io/client-go/kubernetes"
@ -43,7 +42,6 @@ import (
podutil "k8s.io/kubernetes/pkg/api/v1/pod" podutil "k8s.io/kubernetes/pkg/api/v1/pod"
"k8s.io/kubernetes/pkg/controller" "k8s.io/kubernetes/pkg/controller"
"k8s.io/kubernetes/pkg/controller/history" "k8s.io/kubernetes/pkg/controller/history"
"k8s.io/kubernetes/pkg/features"
"k8s.io/klog/v2" "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 // 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 // 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. // 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) 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. // Add a second to avoid milliseconds skew in AddAfter.
// See https://github.com/kubernetes/kubernetes/issues/39785#issuecomment-279959133 for more info. // 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) 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 // 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) ssc.enqueueSSAfter(set, time.Duration(set.Spec.MinReadySeconds)*time.Second)
} }

View File

@ -298,14 +298,10 @@ func (ssc *defaultStatefulSetControl) updateStatefulSet(
if isRunningAndReady(pods[i]) { if isRunningAndReady(pods[i]) {
status.ReadyReplicas++ status.ReadyReplicas++
// count the number of running and available replicas // count the number of running and available replicas
if utilfeature.DefaultFeatureGate.Enabled(features.StatefulSetMinReadySeconds) {
if isRunningAndAvailable(pods[i], set.Spec.MinReadySeconds) { if isRunningAndAvailable(pods[i], set.Spec.MinReadySeconds) {
status.AvailableReplicas++ status.AvailableReplicas++
} }
} else {
// If the featuregate is not enabled, all the ready replicas should be considered as available replicas
status.AvailableReplicas = status.ReadyReplicas
}
} }
// count the number of current and update replicas // 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. // 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 // We must ensure that all for each Pod, when we create it, all of its predecessors, with respect to its
// ordinal, are Available. // ordinal, are Available.
// TODO: Since available is superset of Ready, once we have this featuregate enabled by default, we can remove the if !isRunningAndAvailable(replicas[i], set.Spec.MinReadySeconds) && monotonic {
// isRunningAndReady block as only Available pods should be brought down.
if utilfeature.DefaultFeatureGate.Enabled(features.StatefulSetMinReadySeconds) && !isRunningAndAvailable(replicas[i], set.Spec.MinReadySeconds) && monotonic {
klog.V(4).InfoS("StatefulSet is waiting for Pod to be Available", klog.V(4).InfoS("StatefulSet is waiting for Pod to be Available",
"statefulSet", klog.KObj(set), "pod", klog.KObj(replicas[i])) "statefulSet", klog.KObj(set), "pod", klog.KObj(replicas[i]))
return &status, nil return &status, nil
@ -525,9 +519,7 @@ func (ssc *defaultStatefulSetControl) updateStatefulSet(
return &status, nil return &status, nil
} }
// if we are in monotonic mode and the condemned target is not the first unhealthy Pod, block. // 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 if !isRunningAndAvailable(condemned[target], set.Spec.MinReadySeconds) && monotonic && condemned[target] != firstUnhealthyPod {
// 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 {
klog.V(4).InfoS("StatefulSet is waiting for Pod to be Available prior to scale down", klog.V(4).InfoS("StatefulSet is waiting for Pod to be Available prior to scale down",
"statefulSet", klog.KObj(set), "pod", klog.KObj(firstUnhealthyPod)) "statefulSet", klog.KObj(set), "pod", klog.KObj(firstUnhealthyPod))
return &status, nil return &status, nil

View File

@ -2054,33 +2054,21 @@ func TestStatefulSetAvailability(t *testing.T) {
inputSTS *apps.StatefulSet inputSTS *apps.StatefulSet
expectedActiveReplicas int32 expectedActiveReplicas int32
readyDuration time.Duration readyDuration time.Duration
minReadySecondsFeaturegateEnabled bool
}{ }{
{
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", name: "replicas running for required time, when minReadySeconds is enabled",
inputSTS: setMinReadySeconds(newStatefulSet(1), int32(3600)), inputSTS: setMinReadySeconds(newStatefulSet(1), int32(3600)),
readyDuration: -120 * time.Minute, readyDuration: -120 * time.Minute,
expectedActiveReplicas: int32(1), expectedActiveReplicas: int32(1),
minReadySecondsFeaturegateEnabled: true,
}, },
{ {
name: "replicas not running for required time, when minReadySeconds is enabled", name: "replicas not running for required time, when minReadySeconds is enabled",
inputSTS: setMinReadySeconds(newStatefulSet(1), int32(3600)), inputSTS: setMinReadySeconds(newStatefulSet(1), int32(3600)),
readyDuration: -30 * time.Minute, readyDuration: -30 * time.Minute,
expectedActiveReplicas: int32(0), expectedActiveReplicas: int32(0),
minReadySecondsFeaturegateEnabled: true,
}, },
} }
for _, test := range tests { for _, test := range tests {
defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.StatefulSetMinReadySeconds, test.minReadySecondsFeaturegateEnabled)()
set := test.inputSTS set := test.inputSTS
client := fake.NewSimpleClientset(set) client := fake.NewSimpleClientset(set)
spc, _, ssc := setupController(client) spc, _, ssc := setupController(client)

View File

@ -24,13 +24,10 @@ import (
apps "k8s.io/api/apps/v1" apps "k8s.io/api/apps/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors" apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime"
utilfeature "k8s.io/apiserver/pkg/util/feature"
"k8s.io/client-go/kubernetes/fake" "k8s.io/client-go/kubernetes/fake"
appslisters "k8s.io/client-go/listers/apps/v1" appslisters "k8s.io/client-go/listers/apps/v1"
core "k8s.io/client-go/testing" core "k8s.io/client-go/testing"
"k8s.io/client-go/tools/cache" "k8s.io/client-go/tools/cache"
featuregatetesting "k8s.io/component-base/featuregate/testing"
"k8s.io/kubernetes/pkg/features"
) )
func TestStatefulSetUpdaterUpdatesSetStatus(t *testing.T) { func TestStatefulSetUpdaterUpdatesSetStatus(t *testing.T) {
@ -128,7 +125,6 @@ func TestStatefulSetStatusUpdaterUpdateReplicasConflictFailure(t *testing.T) {
} }
func TestStatefulSetStatusUpdaterGetAvailableReplicas(t *testing.T) { func TestStatefulSetStatusUpdaterGetAvailableReplicas(t *testing.T) {
defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.StatefulSetMinReadySeconds, true)()
set := newStatefulSet(3) set := newStatefulSet(3)
status := apps.StatefulSetStatus{ObservedGeneration: 1, Replicas: 2, AvailableReplicas: 3} status := apps.StatefulSetStatus{ObservedGeneration: 1, Replicas: 2, AvailableReplicas: 3}
fakeClient := &fake.Clientset{} fakeClient := &fake.Clientset{}

View File

@ -733,6 +733,7 @@ const (
// kep: https://kep.k8s.io/2607 // kep: https://kep.k8s.io/2607
// alpha: v1.22 // alpha: v1.22
// beta: v1.23 // beta: v1.23
// GA: v1.25
// StatefulSetMinReadySeconds allows minReadySeconds to be respected by StatefulSet controller // StatefulSetMinReadySeconds allows minReadySeconds to be respected by StatefulSet controller
StatefulSetMinReadySeconds featuregate.Feature = "StatefulSetMinReadySeconds" StatefulSetMinReadySeconds featuregate.Feature = "StatefulSetMinReadySeconds"
@ -995,7 +996,7 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS
StatefulSetAutoDeletePVC: {Default: false, PreRelease: featuregate.Alpha}, 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 SuspendJob: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.26

View File

@ -116,12 +116,6 @@ func (statefulSetStrategy) PrepareForUpdate(ctx context.Context, obj, old runtim
// newSvc.Spec.MyFeature = nil // newSvc.Spec.MyFeature = nil
// } // }
func dropStatefulSetDisabledFields(newSS *apps.StatefulSet, oldSS *apps.StatefulSet) { 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 !utilfeature.DefaultFeatureGate.Enabled(features.StatefulSetAutoDeletePVC) {
if oldSS == nil || oldSS.Spec.PersistentVolumeClaimRetentionPolicy == nil { if oldSS == nil || oldSS.Spec.PersistentVolumeClaimRetentionPolicy == nil {
newSS.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. // Validate validates a new StatefulSet.
func (statefulSetStrategy) Validate(ctx context.Context, obj runtime.Object) field.ErrorList { func (statefulSetStrategy) Validate(ctx context.Context, obj runtime.Object) field.ErrorList {
statefulSet := obj.(*apps.StatefulSet) statefulSet := obj.(*apps.StatefulSet)

View File

@ -85,8 +85,7 @@ func TestStatefulSetStrategy(t *testing.T) {
Status: apps.StatefulSetStatus{Replicas: 4}, Status: apps.StatefulSetStatus{Replicas: 4},
} }
Strategy.PrepareForUpdate(ctx, validPs, ps) Strategy.PrepareForUpdate(ctx, validPs, ps)
t.Run("when minReadySeconds feature gate is enabled", func(t *testing.T) { t.Run("StatefulSet minReadySeconds field validations on creation and updation", func(t *testing.T) {
defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.StatefulSetMinReadySeconds, true)()
// Test creation // Test creation
ps := &apps.StatefulSet{ ps := &apps.StatefulSet{
ObjectMeta: metav1.ObjectMeta{Name: "abc", Namespace: metav1.NamespaceDefault}, 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.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{ validPs = &apps.StatefulSet{
ObjectMeta: metav1.ObjectMeta{Name: ps.Name, Namespace: ps.Namespace, ResourceVersion: "1", Generation: 1}, ObjectMeta: metav1.ObjectMeta{Name: ps.Name, Namespace: ps.Namespace, ResourceVersion: "1", Generation: 1},
@ -380,71 +332,31 @@ func getMaxUnavailable(maxUnavailable int) *int {
func TestDropStatefulSetDisabledFields(t *testing.T) { func TestDropStatefulSetDisabledFields(t *testing.T) {
testCases := []struct { testCases := []struct {
name string name string
enableMinReadySeconds bool
enableMaxUnavailable bool enableMaxUnavailable bool
ss *apps.StatefulSet ss *apps.StatefulSet
oldSS *apps.StatefulSet oldSS *apps.StatefulSet
expectedSS *apps.StatefulSet expectedSS *apps.StatefulSet
}{ }{
{
name: "no minReadySeconds, no update",
enableMinReadySeconds: false,
ss: &apps.StatefulSet{},
oldSS: nil,
expectedSS: &apps.StatefulSet{},
},
{
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: "no minReadySeconds, oldSS field set to 100, no update",
enableMinReadySeconds: false,
ss: generateStatefulSetWithMinReadySeconds(2000),
oldSS: generateStatefulSetWithMinReadySeconds(100),
expectedSS: generateStatefulSetWithMinReadySeconds(2000),
},
{
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", name: "set minReadySeconds, no update",
enableMinReadySeconds: true,
ss: generateStatefulSetWithMinReadySeconds(10), ss: generateStatefulSetWithMinReadySeconds(10),
oldSS: generateStatefulSetWithMinReadySeconds(20), oldSS: generateStatefulSetWithMinReadySeconds(20),
expectedSS: generateStatefulSetWithMinReadySeconds(10), expectedSS: generateStatefulSetWithMinReadySeconds(10),
}, },
{ {
name: "set minReadySeconds, oldSS field set to nil", name: "set minReadySeconds, oldSS field set to nil",
enableMinReadySeconds: true,
ss: generateStatefulSetWithMinReadySeconds(10), ss: generateStatefulSetWithMinReadySeconds(10),
oldSS: nil, oldSS: nil,
expectedSS: generateStatefulSetWithMinReadySeconds(10), expectedSS: generateStatefulSetWithMinReadySeconds(10),
}, },
{ {
name: "set minReadySeconds, oldSS field is set to 0", name: "set minReadySeconds, oldSS field is set to 0",
enableMinReadySeconds: true,
ss: generateStatefulSetWithMinReadySeconds(10), ss: generateStatefulSetWithMinReadySeconds(10),
oldSS: generateStatefulSetWithMinReadySeconds(0), oldSS: generateStatefulSetWithMinReadySeconds(0),
expectedSS: generateStatefulSetWithMinReadySeconds(10), expectedSS: generateStatefulSetWithMinReadySeconds(10),
}, },
{ {
name: "MaxUnavailable not enabled, field not used", name: "MaxUnavailable not enabled, field not used",
enableMaxUnavailable: false,
ss: makeStatefulSetWithMaxUnavailable(nil), ss: makeStatefulSetWithMaxUnavailable(nil),
oldSS: nil, oldSS: nil,
expectedSS: makeStatefulSetWithMaxUnavailable(nil), expectedSS: makeStatefulSetWithMaxUnavailable(nil),
@ -481,7 +393,6 @@ func TestDropStatefulSetDisabledFields(t *testing.T) {
for _, tc := range testCases { for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) { 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.MaxUnavailableStatefulSet, tc.enableMaxUnavailable)()
defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.StatefulSetMinReadySeconds, tc.enableMinReadySeconds)()
old := tc.oldSS.DeepCopy() old := tc.oldSS.DeepCopy()
dropStatefulSetDisabledFields(tc.ss, tc.oldSS) dropStatefulSetDisabledFields(tc.ss, tc.oldSS)

View File

@ -245,26 +245,16 @@ func TestStatefulSetAvailable(t *testing.T) {
totalReplicas int32 totalReplicas int32
readyReplicas int32 readyReplicas int32
activeReplicas 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, totalReplicas: 4,
readyReplicas: 3, readyReplicas: 3,
activeReplicas: 2, 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 { for _, test := range tests {
t.Run(test.name, func(t *testing.T) { t.Run(test.name, func(t *testing.T) {
defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.StatefulSetMinReadySeconds, test.enabled)()
closeFn, rm, informers, c := scSetup(t) closeFn, rm, informers, c := scSetup(t)
defer closeFn() defer closeFn()
ns := framework.CreateNamespaceOrDie(c, "test-available-pods", t) ns := framework.CreateNamespaceOrDie(c, "test-available-pods", t)