validation: Handle presence of api introduced

When the StatefulSetMinReadySeconds feature gate is disabled,
the registry and validation must properly handle dropping the
minReadySeconds and AvailableReplicas fields
This commit is contained in:
ravisantoshgudimetla 2021-04-05 17:08:53 -04:00
parent d11cc95376
commit f3ab955018
4 changed files with 393 additions and 15 deletions

View File

@ -109,6 +109,9 @@ func ValidateStatefulSetSpec(spec *apps.StatefulSetSpec, fldPath *field.Path, op
}
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"))...)
}
if spec.Selector == nil {
allErrs = append(allErrs, field.Required(fldPath.Child("selector"), ""))
} else {
@ -152,11 +155,21 @@ func ValidateStatefulSetUpdate(statefulSet, oldStatefulSet *apps.StatefulSet) fi
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
}
if !apiequality.Semantic.DeepEqual(newStatefulSetClone.Spec, oldStatefulSet.Spec) {
allErrs = append(allErrs, field.Forbidden(field.NewPath("spec"), "updates to statefulset spec for fields other than 'replicas', 'template', and 'updateStrategy' are forbidden"))
if utilfeature.DefaultFeatureGate.Enabled(features.StatefulSetMinReadySeconds) {
allErrs = append(allErrs, field.Forbidden(field.NewPath("spec"), "updates to statefulset spec for fields other than 'replicas', 'template', 'minReadySeconds' and 'updateStrategy' are forbidden"))
} else {
allErrs = append(allErrs, field.Forbidden(field.NewPath("spec"), "updates to statefulset spec for fields other than 'replicas', 'template' and 'updateStrategy' are forbidden"))
}
}
allErrs = append(allErrs, apivalidation.ValidateNonnegativeField(int64(statefulSet.Spec.Replicas), field.NewPath("spec", "replicas"))...)
if utilfeature.DefaultFeatureGate.Enabled(features.StatefulSetMinReadySeconds) {
allErrs = append(allErrs, apivalidation.ValidateNonnegativeField(int64(statefulSet.Spec.MinReadySeconds), field.NewPath("spec", "minReadySeconds"))...)
}
return allErrs
}
@ -168,6 +181,9 @@ 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"))...)
}
if status.ObservedGeneration != nil {
allErrs = append(allErrs, apivalidation.ValidateNonnegativeField(int64(*status.ObservedGeneration), fieldPath.Child("observedGeneration"))...)
}
@ -185,6 +201,14 @@ 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"))
}
}
return allErrs
}

View File

@ -386,18 +386,94 @@ func TestValidateStatefulSet(t *testing.T) {
}
}
// generateStatefulSetSpec generates a valid StatefulSet spec
func generateStatefulSetSpec(minSeconds int32) *apps.StatefulSetSpec {
labels := map[string]string{"a": "b"}
podTemplate := api.PodTemplate{
Template: api.PodTemplateSpec{
ObjectMeta: metav1.ObjectMeta{
Labels: labels,
},
Spec: api.PodSpec{
RestartPolicy: api.RestartPolicyAlways,
DNSPolicy: api.DNSClusterFirst,
Containers: []api.Container{{Name: "abc", Image: "image", ImagePullPolicy: "IfNotPresent"}},
},
},
}
ss := &apps.StatefulSetSpec{
PodManagementPolicy: "OrderedReady",
Selector: &metav1.LabelSelector{MatchLabels: labels},
Template: podTemplate.Template,
Replicas: 3,
UpdateStrategy: apps.StatefulSetUpdateStrategy{Type: apps.RollingUpdateStatefulSetStrategyType},
MinReadySeconds: minSeconds,
}
return ss
}
// TestValidateStatefulSetMinReadySeconds tests the StatefulSet Spec's minReadySeconds field
func TestValidateStatefulSetMinReadySeconds(t *testing.T) {
testCases := map[string]struct {
ss *apps.StatefulSetSpec
enableMinReadySeconds bool
expectErr bool
}{
"valid : minReadySeconds enabled, zero": {
ss: generateStatefulSetSpec(0),
enableMinReadySeconds: true,
expectErr: false,
},
"invalid : minReadySeconds enabled, negative": {
ss: generateStatefulSetSpec(-1),
enableMinReadySeconds: true,
expectErr: true,
},
"valid : minReadySeconds enabled, very large value": {
ss: generateStatefulSetSpec(2147483647),
enableMinReadySeconds: true,
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,
},
}
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 {
t.Errorf("Unexpected success")
}
if !tc.expectErr && len(errs) != 0 {
t.Errorf("Unexpected error(s): %v", errs)
}
})
}
}
func TestValidateStatefulSetStatus(t *testing.T) {
observedGenerationMinusOne := int64(-1)
collisionCountMinusOne := int32(-1)
tests := []struct {
name string
replicas int32
readyReplicas int32
currentReplicas int32
updatedReplicas int32
observedGeneration *int64
collisionCount *int32
expectedErr bool
name string
replicas int32
readyReplicas int32
currentReplicas int32
updatedReplicas int32
availableReplicas int32
enableMinReadySeconds bool
observedGeneration *int64
collisionCount *int32
expectedErr bool
}{
{
name: "valid status",
@ -481,10 +557,65 @@ func TestValidateStatefulSetStatus(t *testing.T) {
updatedReplicas: 4,
expectedErr: true,
},
{
name: "invalid: number of available replicas",
replicas: 3,
readyReplicas: 3,
currentReplicas: 2,
availableReplicas: int32(-1),
expectedErr: true,
enableMinReadySeconds: 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 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,
},
}
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,
@ -492,6 +623,7 @@ func TestValidateStatefulSetStatus(t *testing.T) {
UpdatedReplicas: test.updatedReplicas,
ObservedGeneration: test.observedGeneration,
CollisionCount: test.collisionCount,
AvailableReplicas: test.availableReplicas,
}
errs := ValidateStatefulSetStatus(&status, field.NewPath("status"))

View File

@ -18,6 +18,8 @@ package statefulset
import (
"context"
utilfeature "k8s.io/apiserver/pkg/util/feature"
"k8s.io/kubernetes/pkg/features"
appsv1beta1 "k8s.io/api/apps/v1beta1"
appsv1beta2 "k8s.io/api/apps/v1beta2"
@ -84,7 +86,7 @@ func (statefulSetStrategy) PrepareForCreate(ctx context.Context, obj runtime.Obj
statefulSet.Status = apps.StatefulSetStatus{}
statefulSet.Generation = 1
dropStatefulSetDisabledFields(statefulSet, nil)
pod.DropDisabledTemplateFields(&statefulSet.Spec.Template, nil)
}
@ -95,6 +97,7 @@ func (statefulSetStrategy) PrepareForUpdate(ctx context.Context, obj, old runtim
// Update is not allowed to set status
newStatefulSet.Status = oldStatefulSet.Status
dropStatefulSetDisabledFields(newStatefulSet, oldStatefulSet)
pod.DropDisabledTemplateFields(&newStatefulSet.Spec.Template, &oldStatefulSet.Spec.Template)
// Any changes to the spec increment the generation number, any changes to the
@ -103,7 +106,31 @@ func (statefulSetStrategy) PrepareForUpdate(ctx context.Context, obj, old runtim
if !apiequality.Semantic.DeepEqual(oldStatefulSet.Spec, newStatefulSet.Spec) {
newStatefulSet.Generation = oldStatefulSet.Generation + 1
}
}
// dropStatefulSetDisabledFields drops fields that are not used if their associated feature gates
// are not enabled.
// The typical pattern is:
// if !utilfeature.DefaultFeatureGate.Enabled(features.MyFeature) && !myFeatureInUse(oldSvc) {
// 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)
}
}
}
// 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.

View File

@ -17,13 +17,18 @@ limitations under the License.
package statefulset
import (
"reflect"
"testing"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/diff"
genericapirequest "k8s.io/apiserver/pkg/endpoints/request"
"k8s.io/apiserver/pkg/registry/rest"
utilfeature "k8s.io/apiserver/pkg/util/feature"
featuregatetesting "k8s.io/component-base/featuregate/testing"
"k8s.io/kubernetes/pkg/apis/apps"
api "k8s.io/kubernetes/pkg/apis/core"
"k8s.io/kubernetes/pkg/features"
)
func TestStatefulSetStrategy(t *testing.T) {
@ -67,7 +72,7 @@ func TestStatefulSetStrategy(t *testing.T) {
if len(errs) != 0 {
t.Errorf("unexpected error validating %v", errs)
}
newMinReadySeconds := int32(50)
// Just Spec.Replicas is allowed to change
validPs := &apps.StatefulSet{
ObjectMeta: metav1.ObjectMeta{Name: ps.Name, Namespace: ps.Namespace, ResourceVersion: "1", Generation: 1},
@ -76,14 +81,110 @@ func TestStatefulSetStrategy(t *testing.T) {
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 spec.Replicas is allowed on a statefulset: %v", errs)
}
t.Run("when minReadySeconds feature gate is enabled", func(t *testing.T) {
defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.StatefulSetMinReadySeconds, true)()
// 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("expected failure when MinReadySeconds is not positive number but got no error %v", errs)
}
expectedCreateErrorString := "spec.minReadySeconds: Invalid value: -1: must be greater than or equal to 0"
if errs[0].Error() != expectedCreateErrorString {
t.Errorf("mismatched error string %v", errs[0].Error())
}
// Test updation
newMinReadySeconds := int32(50)
// Just Spec.Replicas is allowed to change
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 spec.Replicas and minReadySeconds is allowed on a statefulset: %v", errs)
}
invalidPs := ps
invalidPs.Spec.MinReadySeconds = int32(-1)
Strategy.PrepareForUpdate(ctx, validPs, invalidPs)
errs = Strategy.ValidateUpdate(ctx, validPs, ps)
if len(errs) != 0 {
t.Errorf("updating spec.Replicas and minReadySeconds is allowed on a statefulset: %v", errs)
}
if validPs.Spec.MinReadySeconds != newMinReadySeconds {
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' and 'updateStrategy' are forbidden"
if errs[0].Error() != expectedUpdateErrorString {
t.Errorf("expected error string %v", errs[0].Error())
}
})
validPs.Spec.Selector = &metav1.LabelSelector{MatchLabels: map[string]string{"a": "bar"}}
Strategy.PrepareForUpdate(ctx, validPs, ps)
@ -204,3 +305,97 @@ func TestStatefulSetStatusStrategy(t *testing.T) {
t.Errorf("unexpected error %v", errs)
}
}
// generateStatefulSetWithMinReadySeconds generates a StatefulSet with min values
func generateStatefulSetWithMinReadySeconds(minReadySeconds int32) *apps.StatefulSet {
return &apps.StatefulSet{
Spec: apps.StatefulSetSpec{
MinReadySeconds: minReadySeconds,
},
}
}
// TestDropStatefulSetDisabledFields tests if the drop functionality is working fine or not
func TestDropStatefulSetDisabledFields(t *testing.T) {
testCases := []struct {
name string
enableMinReadySeconds 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: "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",
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),
},
}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.StatefulSetMinReadySeconds, tc.enableMinReadySeconds)()
old := tc.oldSS.DeepCopy()
dropStatefulSetDisabledFields(tc.ss, tc.oldSS)
// old obj should never be changed
if !reflect.DeepEqual(tc.oldSS, old) {
t.Fatalf("old ds changed: %v", diff.ObjectReflectDiff(tc.oldSS, old))
}
if !reflect.DeepEqual(tc.ss, tc.expectedSS) {
t.Fatalf("unexpected ds spec: %v", diff.ObjectReflectDiff(tc.expectedSS, tc.ss))
}
})
}
}