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

This commit is contained in:
Peter Schuurman 2023-02-23 16:22:56 -08:00
parent 35f3fc59c1
commit a714e9e485
2 changed files with 31 additions and 22 deletions

View File

@ -28,11 +28,9 @@ import (
"k8s.io/apimachinery/pkg/util/intstr" "k8s.io/apimachinery/pkg/util/intstr"
"k8s.io/apimachinery/pkg/util/validation" "k8s.io/apimachinery/pkg/util/validation"
"k8s.io/apimachinery/pkg/util/validation/field" "k8s.io/apimachinery/pkg/util/validation/field"
utilfeature "k8s.io/apiserver/pkg/util/feature"
"k8s.io/kubernetes/pkg/apis/apps" "k8s.io/kubernetes/pkg/apis/apps"
api "k8s.io/kubernetes/pkg/apis/core" api "k8s.io/kubernetes/pkg/apis/core"
apivalidation "k8s.io/kubernetes/pkg/apis/core/validation" 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. // 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.Replicas), fldPath.Child("replicas"))...)
allErrs = append(allErrs, apivalidation.ValidateNonnegativeField(int64(spec.MinReadySeconds), fldPath.Child("minReadySeconds"))...) allErrs = append(allErrs, apivalidation.ValidateNonnegativeField(int64(spec.MinReadySeconds), fldPath.Child("minReadySeconds"))...)
if utilfeature.DefaultFeatureGate.Enabled(features.StatefulSetStartOrdinal) { if spec.Ordinals != nil {
if spec.Ordinals != nil { replicaStartOrdinal := spec.Ordinals.Start
replicaStartOrdinal := spec.Ordinals.Start allErrs = append(allErrs, apivalidation.ValidateNonnegativeField(int64(replicaStartOrdinal), fldPath.Child("ordinals.start"))...)
allErrs = append(allErrs, apivalidation.ValidateNonnegativeField(int64(replicaStartOrdinal), fldPath.Child("ordinals.start"))...)
}
} }
if spec.Selector == nil { 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.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
newStatefulSetClone.Spec.MinReadySeconds = oldStatefulSet.Spec.MinReadySeconds // +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 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.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"))
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"))
}
} }
return allErrs return allErrs

View File

@ -86,8 +86,6 @@ func TestValidateStatefulSet(t *testing.T) {
const enableStatefulSetAutoDeletePVC = "[enable StatefulSetAutoDeletePVC]" const enableStatefulSetAutoDeletePVC = "[enable StatefulSetAutoDeletePVC]"
const enableStatefulSetStartOrdinal = "[enable StatefulSetStartOrdinal]"
type testCase struct { type testCase struct {
name string name string
set apps.StatefulSet 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{ set: apps.StatefulSet{
ObjectMeta: metav1.ObjectMeta{Name: "abc-123", Namespace: metav1.NamespaceDefault}, ObjectMeta: metav1.ObjectMeta{Name: "abc-123", Namespace: metav1.NamespaceDefault},
Spec: apps.StatefulSetSpec{ Spec: apps.StatefulSetSpec{
@ -682,7 +680,7 @@ func TestValidateStatefulSet(t *testing.T) {
}, },
}, },
{ {
name: "invalid ordinals.start " + enableStatefulSetStartOrdinal, name: "invalid ordinals.start ",
set: apps.StatefulSet{ set: apps.StatefulSet{
ObjectMeta: metav1.ObjectMeta{Name: "abc-123", Namespace: metav1.NamespaceDefault}, ObjectMeta: metav1.ObjectMeta{Name: "abc-123", Namespace: metav1.NamespaceDefault},
Spec: apps.StatefulSetSpec{ Spec: apps.StatefulSetSpec{
@ -714,9 +712,6 @@ func TestValidateStatefulSet(t *testing.T) {
if strings.Contains(name, enableStatefulSetAutoDeletePVC) { if strings.Contains(name, enableStatefulSetAutoDeletePVC) {
defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.StatefulSetAutoDeletePVC, true)() 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)) errs := ValidateStatefulSet(&testCase.set, pod.GetValidationOptionsFromPodTemplate(&testCase.set.Spec.Template, nil))
wantErrs := testCase.errs 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{ errorCases := []testCase{