From a714e9e4855f93053206555f5810f6942acb7a8d Mon Sep 17 00:00:00 2001 From: Peter Schuurman Date: Thu, 23 Feb 2023 16:22:56 -0800 Subject: [PATCH] Fix validation.go to validate without StatefulSetStartOrdinal feature gate check. Adds test case to validate regression fix of validation failing when spec.ordinals set and feature gate disabled --- pkg/apis/apps/validation/validation.go | 20 ++++--------- pkg/apis/apps/validation/validation_test.go | 33 ++++++++++++++++----- 2 files changed, 31 insertions(+), 22 deletions(-) diff --git a/pkg/apis/apps/validation/validation.go b/pkg/apis/apps/validation/validation.go index 027ac8923b1..14c287c3963 100644 --- a/pkg/apis/apps/validation/validation.go +++ b/pkg/apis/apps/validation/validation.go @@ -28,11 +28,9 @@ import ( "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/apimachinery/pkg/util/validation" "k8s.io/apimachinery/pkg/util/validation/field" - utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/kubernetes/pkg/apis/apps" api "k8s.io/kubernetes/pkg/apis/core" apivalidation "k8s.io/kubernetes/pkg/apis/core/validation" - "k8s.io/kubernetes/pkg/features" ) // ValidateStatefulSetName can be used to check whether the given StatefulSet name is valid. @@ -130,11 +128,9 @@ func ValidateStatefulSetSpec(spec *apps.StatefulSetSpec, fldPath *field.Path, op allErrs = append(allErrs, apivalidation.ValidateNonnegativeField(int64(spec.Replicas), fldPath.Child("replicas"))...) allErrs = append(allErrs, apivalidation.ValidateNonnegativeField(int64(spec.MinReadySeconds), fldPath.Child("minReadySeconds"))...) - if utilfeature.DefaultFeatureGate.Enabled(features.StatefulSetStartOrdinal) { - if spec.Ordinals != nil { - replicaStartOrdinal := spec.Ordinals.Start - allErrs = append(allErrs, apivalidation.ValidateNonnegativeField(int64(replicaStartOrdinal), fldPath.Child("ordinals.start"))...) - } + if spec.Ordinals != nil { + replicaStartOrdinal := spec.Ordinals.Start + allErrs = append(allErrs, apivalidation.ValidateNonnegativeField(int64(replicaStartOrdinal), fldPath.Child("ordinals.start"))...) } if spec.Selector == nil { @@ -189,17 +185,11 @@ func ValidateStatefulSetUpdate(statefulSet, oldStatefulSet *apps.StatefulSet, op 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 - if utilfeature.DefaultFeatureGate.Enabled(features.StatefulSetStartOrdinal) { - newStatefulSetClone.Spec.Ordinals = oldStatefulSet.Spec.Ordinals // +k8s:verify-mutation:reason=clone - } + newStatefulSetClone.Spec.Ordinals = oldStatefulSet.Spec.Ordinals // +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.StatefulSetStartOrdinal) { - allErrs = append(allErrs, field.Forbidden(field.NewPath("spec"), "updates to statefulset spec for fields other than 'replicas', 'ordinals', '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', 'persistentVolumeClaimRetentionPolicy' and 'minReadySeconds' are forbidden")) - } + allErrs = append(allErrs, field.Forbidden(field.NewPath("spec"), "updates to statefulset spec for fields other than 'replicas', 'ordinals', 'template', 'updateStrategy', 'persistentVolumeClaimRetentionPolicy' and 'minReadySeconds' are forbidden")) } return allErrs diff --git a/pkg/apis/apps/validation/validation_test.go b/pkg/apis/apps/validation/validation_test.go index 9a3b3181a21..b245ac5ae2d 100644 --- a/pkg/apis/apps/validation/validation_test.go +++ b/pkg/apis/apps/validation/validation_test.go @@ -86,8 +86,6 @@ func TestValidateStatefulSet(t *testing.T) { const enableStatefulSetAutoDeletePVC = "[enable StatefulSetAutoDeletePVC]" - const enableStatefulSetStartOrdinal = "[enable StatefulSetStartOrdinal]" - type testCase struct { name string set apps.StatefulSet @@ -196,7 +194,7 @@ func TestValidateStatefulSet(t *testing.T) { }, }, { - name: "ordinals.start positive value " + enableStatefulSetStartOrdinal, + name: "ordinals.start positive value", set: apps.StatefulSet{ ObjectMeta: metav1.ObjectMeta{Name: "abc-123", Namespace: metav1.NamespaceDefault}, Spec: apps.StatefulSetSpec{ @@ -682,7 +680,7 @@ func TestValidateStatefulSet(t *testing.T) { }, }, { - name: "invalid ordinals.start " + enableStatefulSetStartOrdinal, + name: "invalid ordinals.start ", set: apps.StatefulSet{ ObjectMeta: metav1.ObjectMeta{Name: "abc-123", Namespace: metav1.NamespaceDefault}, Spec: apps.StatefulSetSpec{ @@ -714,9 +712,6 @@ func TestValidateStatefulSet(t *testing.T) { if strings.Contains(name, enableStatefulSetAutoDeletePVC) { defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.StatefulSetAutoDeletePVC, true)() } - if strings.Contains(name, enableStatefulSetStartOrdinal) { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.StatefulSetStartOrdinal, true)() - } errs := ValidateStatefulSet(&testCase.set, pod.GetValidationOptionsFromPodTemplate(&testCase.set.Spec.Template, nil)) wantErrs := testCase.errs @@ -1211,6 +1206,30 @@ func TestValidateStatefulSetUpdate(t *testing.T) { }, }, }, + { + name: "update existing instance with .spec.ordinals.start", + old: apps.StatefulSet{ + ObjectMeta: metav1.ObjectMeta{Name: "abc.123.example", Namespace: metav1.NamespaceDefault}, + Spec: apps.StatefulSetSpec{ + PodManagementPolicy: apps.OrderedReadyPodManagement, + Selector: &metav1.LabelSelector{MatchLabels: validLabels}, + Template: validPodTemplate.Template, + UpdateStrategy: apps.StatefulSetUpdateStrategy{Type: apps.RollingUpdateStatefulSetStrategyType}, + }, + }, + update: apps.StatefulSet{ + ObjectMeta: metav1.ObjectMeta{Name: "abc.123.example", Namespace: metav1.NamespaceDefault}, + Spec: apps.StatefulSetSpec{ + PodManagementPolicy: apps.OrderedReadyPodManagement, + Selector: &metav1.LabelSelector{MatchLabels: validLabels}, + Template: validPodTemplate.Template, + UpdateStrategy: apps.StatefulSetUpdateStrategy{Type: apps.RollingUpdateStatefulSetStrategyType}, + Ordinals: &apps.StatefulSetOrdinals{ + Start: 3, + }, + }, + }, + }, } errorCases := []testCase{