Skip to content

Commit 6ef1a1f

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request kubernetes#130725 from jpbetz/replication-controller-minimums-to-declarative
Migrate to declarative validation: ReplicationController spec.replicas and spec.minReadySeconds fields
2 parents ba6acfd + f7296b3 commit 6ef1a1f

File tree

11 files changed

+239
-9
lines changed

11 files changed

+239
-9
lines changed

‎api/openapi-spec/v3/api__v1_openapi.json‎

Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎pkg/apis/core/v1/defaults.go‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -60,10 +60,7 @@ func SetDefaults_ReplicationController(obj *v1.ReplicationController) {
6060
obj.Labels = labels
6161
}
6262
}
63-
if obj.Spec.Replicas == nil {
64-
obj.Spec.Replicas = new(int32)
65-
*obj.Spec.Replicas = 1
66-
}
63+
// obj.Spec.Replicas is defaulted declaratively
6764
}
6865
func SetDefaults_Volume(obj *v1.Volume) {
6966
if ptr.AllPtrFieldsNil(&obj.VolumeSource) {

‎pkg/apis/core/v1/zz_generated.defaults.go‎

Lines changed: 4 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎pkg/apis/core/v1/zz_generated.validations.go‎

Lines changed: 78 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎pkg/apis/core/validation/validation.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6319,12 +6319,12 @@ func ValidatePodTemplateSpecForRC(template *core.PodTemplateSpec, selectorMap ma
63196319
// ValidateReplicationControllerSpec tests if required fields in the replication controller spec are set.
63206320
func ValidateReplicationControllerSpec(spec, oldSpec *core.ReplicationControllerSpec, fldPath *field.Path, opts PodValidationOptions) field.ErrorList {
63216321
allErrs := field.ErrorList{}
6322-
allErrs = append(allErrs, ValidateNonnegativeField(int64(spec.MinReadySeconds), fldPath.Child("minReadySeconds"))...)
6322+
allErrs = append(allErrs, ValidateNonnegativeField(int64(spec.MinReadySeconds), fldPath.Child("minReadySeconds")).MarkCoveredByDeclarative()...)
63236323
allErrs = append(allErrs, ValidateNonEmptySelector(spec.Selector, fldPath.Child("selector"))...)
63246324
if spec.Replicas == nil {
6325-
allErrs = append(allErrs, field.Required(fldPath.Child("replicas"), ""))
6325+
allErrs = append(allErrs, field.Required(fldPath.Child("replicas"), "").MarkCoveredByDeclarative())
63266326
} else {
6327-
allErrs = append(allErrs, ValidateNonnegativeField(int64(*spec.Replicas), fldPath.Child("replicas"))...)
6327+
allErrs = append(allErrs, ValidateNonnegativeField(int64(*spec.Replicas), fldPath.Child("replicas")).MarkCoveredByDeclarative()...)
63286328
}
63296329
allErrs = append(allErrs, ValidatePodTemplateSpecForRC(spec.Template, spec.Selector, fldPath.Child("template"), opts)...)
63306330
return allErrs

‎pkg/apis/core/validation/validation_test.go‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16755,6 +16755,16 @@ func TestValidateReplicationControllerUpdate(t *testing.T) {
1675516755
update: mkValidReplicationController(func(rc *core.ReplicationController) {
1675616756
rc.Spec.Replicas = ptr.To[int32](3)
1675716757
}),
16758+
}, {
16759+
old: mkValidReplicationController(func(rc *core.ReplicationController) {}),
16760+
update: mkValidReplicationController(func(rc *core.ReplicationController) {
16761+
rc.Spec.MinReadySeconds = 0
16762+
}),
16763+
}, {
16764+
old: mkValidReplicationController(func(rc *core.ReplicationController) {}),
16765+
update: mkValidReplicationController(func(rc *core.ReplicationController) {
16766+
rc.Spec.MinReadySeconds = 3
16767+
}),
1675816768
}, {
1675916769
old: mkValidReplicationController(func(rc *core.ReplicationController) {}),
1676016770
update: mkValidReplicationController(func(rc *core.ReplicationController) {
@@ -16831,6 +16841,15 @@ func TestValidateReplicationControllerUpdate(t *testing.T) {
1683116841
field.Required(field.NewPath("spec.replicas"), ""),
1683216842
},
1683316843
},
16844+
"negative minReadySeconds": {
16845+
old: mkValidReplicationController(func(rc *core.ReplicationController) {}),
16846+
update: mkValidReplicationController(func(rc *core.ReplicationController) {
16847+
rc.Spec.MinReadySeconds = -1
16848+
}),
16849+
expectedErrs: field.ErrorList{
16850+
field.Invalid(field.NewPath("spec.minReadySeconds"), nil, "").WithOrigin("minimum"),
16851+
},
16852+
},
1683416853
}
1683516854
for k, tc := range errorCases {
1683616855
t.Run(k, func(t *testing.T) {
@@ -16872,6 +16891,9 @@ func TestValidateReplicationController(t *testing.T) {
1687216891
mkValidReplicationController(func(rc *core.ReplicationController) { rc.Spec.Replicas = ptr.To[int32](0) }),
1687316892
mkValidReplicationController(func(rc *core.ReplicationController) { rc.Spec.Replicas = ptr.To[int32](1) }),
1687416893
mkValidReplicationController(func(rc *core.ReplicationController) { rc.Spec.Replicas = ptr.To[int32](100) }),
16894+
mkValidReplicationController(func(rc *core.ReplicationController) { rc.Spec.MinReadySeconds = 0 }),
16895+
mkValidReplicationController(func(rc *core.ReplicationController) { rc.Spec.MinReadySeconds = 1 }),
16896+
mkValidReplicationController(func(rc *core.ReplicationController) { rc.Spec.MinReadySeconds = 100 }),
1687516897
}
1687616898
for _, tc := range successCases {
1687716899
if errs := ValidateReplicationController(&tc, PodValidationOptions{}); len(errs) != 0 {
@@ -16919,6 +16941,12 @@ func TestValidateReplicationController(t *testing.T) {
1691916941
field.Invalid(field.NewPath("spec.replicas"), nil, "").WithOrigin("minimum"),
1692016942
},
1692116943
},
16944+
"negative minReadySeconds": {
16945+
input: mkValidReplicationController(func(rc *core.ReplicationController) { rc.Spec.MinReadySeconds = -1 }),
16946+
expectedErrs: field.ErrorList{
16947+
field.Invalid(field.NewPath("spec.minReadySeconds"), nil, "").WithOrigin("minimum"),
16948+
},
16949+
},
1692216950
"nil replicas": {
1692316951
input: mkValidReplicationController(func(rc *core.ReplicationController) { rc.Spec.Replicas = nil }),
1692416952
expectedErrs: field.ErrorList{

‎pkg/generated/openapi/zz_generated.openapi.go‎

Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎pkg/registry/core/replicationcontroller/declarative_validation_test.go‎

Lines changed: 107 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,10 +40,50 @@ func TestDeclarativeValidateForDeclarative(t *testing.T) {
4040
input api.ReplicationController
4141
expectedErrs field.ErrorList
4242
}{
43+
// baseline
4344
"empty resource": {
4445
input: mkValidReplicationController(),
4546
},
46-
// TODO: Add test cases
47+
// spec.replicas
48+
"nil replicas": {
49+
input: mkValidReplicationController(func(rc *api.ReplicationController) {
50+
rc.Spec.Replicas = nil
51+
}),
52+
expectedErrs: field.ErrorList{
53+
field.Required(field.NewPath("spec.replicas"), ""),
54+
},
55+
},
56+
"0 replicas": {
57+
input: mkValidReplicationController(setSpecReplicas(0)),
58+
},
59+
"1 replicas": {
60+
input: mkValidReplicationController(setSpecReplicas(1)),
61+
},
62+
"positive replicas": {
63+
input: mkValidReplicationController(setSpecReplicas(100)),
64+
},
65+
"negative replicas": {
66+
input: mkValidReplicationController(setSpecReplicas(-1)),
67+
expectedErrs: field.ErrorList{
68+
field.Invalid(field.NewPath("spec.replicas"), nil, "").WithOrigin("minimum"),
69+
},
70+
},
71+
// spec.minReadySeconds
72+
"0 minReadySeconds": {
73+
input: mkValidReplicationController(setSpecMinReadySeconds(0)),
74+
},
75+
"1 minReadySeconds": {
76+
input: mkValidReplicationController(setSpecMinReadySeconds(1)),
77+
},
78+
"positive minReadySeconds": {
79+
input: mkValidReplicationController(setSpecMinReadySeconds(100)),
80+
},
81+
"negative minReadySeconds": {
82+
input: mkValidReplicationController(setSpecMinReadySeconds(-1)),
83+
expectedErrs: field.ErrorList{
84+
field.Invalid(field.NewPath("spec.minReadySeconds"), nil, "").WithOrigin("minimum"),
85+
},
86+
},
4787
}
4888
for k, tc := range testCases {
4989
t.Run(k, func(t *testing.T) {
@@ -90,7 +130,60 @@ func TestValidateUpdateForDeclarative(t *testing.T) {
90130
update api.ReplicationController
91131
expectedErrs field.ErrorList
92132
}{
93-
// TODO: Add test cases
133+
// baseline
134+
"baseline": {
135+
old: mkValidReplicationController(),
136+
update: mkValidReplicationController(),
137+
},
138+
// spec.replicas
139+
"nil replicas": {
140+
old: mkValidReplicationController(),
141+
update: mkValidReplicationController(func(rc *api.ReplicationController) {
142+
rc.Spec.Replicas = nil
143+
}),
144+
expectedErrs: field.ErrorList{
145+
field.Required(field.NewPath("spec.replicas"), ""),
146+
},
147+
},
148+
"0 replicas": {
149+
old: mkValidReplicationController(),
150+
update: mkValidReplicationController(setSpecReplicas(0)),
151+
},
152+
"1 replicas": {
153+
old: mkValidReplicationController(),
154+
update: mkValidReplicationController(setSpecReplicas(1)),
155+
},
156+
"positive replicas": {
157+
old: mkValidReplicationController(),
158+
update: mkValidReplicationController(setSpecReplicas(100)),
159+
},
160+
"negative replicas": {
161+
old: mkValidReplicationController(),
162+
update: mkValidReplicationController(setSpecReplicas(-1)),
163+
expectedErrs: field.ErrorList{
164+
field.Invalid(field.NewPath("spec.replicas"), nil, "").WithOrigin("minimum"),
165+
},
166+
},
167+
// spec.minReadySeconds
168+
"0 minReadySeconds": {
169+
old: mkValidReplicationController(),
170+
update: mkValidReplicationController(setSpecMinReadySeconds(0)),
171+
},
172+
"1 minReadySeconds": {
173+
old: mkValidReplicationController(),
174+
update: mkValidReplicationController(setSpecMinReadySeconds(1)),
175+
},
176+
"positive minReadySeconds": {
177+
old: mkValidReplicationController(),
178+
update: mkValidReplicationController(setSpecMinReadySeconds(3)),
179+
},
180+
"negative minReadySeconds": {
181+
old: mkValidReplicationController(),
182+
update: mkValidReplicationController(setSpecMinReadySeconds(-1)),
183+
expectedErrs: field.ErrorList{
184+
field.Invalid(field.NewPath("spec.minReadySeconds"), nil, "").WithOrigin("minimum"),
185+
},
186+
},
94187
}
95188
for k, tc := range testCases {
96189
t.Run(k, func(t *testing.T) {
@@ -164,3 +257,15 @@ func mkValidReplicationController(tweaks ...func(rc *api.ReplicationController))
164257
}
165258
return rc
166259
}
260+
261+
func setSpecReplicas(val int32) func(rc *api.ReplicationController) {
262+
return func(rc *api.ReplicationController) {
263+
rc.Spec.Replicas = ptr.To(val)
264+
}
265+
}
266+
267+
func setSpecMinReadySeconds(val int32) func(rc *api.ReplicationController) {
268+
return func(rc *api.ReplicationController) {
269+
rc.Spec.MinReadySeconds = val
270+
}
271+
}

‎staging/src/k8s.io/api/core/v1/generated.proto‎

Lines changed: 6 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎staging/src/k8s.io/api/core/v1/types.go‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5107,12 +5107,18 @@ type ReplicationControllerSpec struct {
51075107
// Defaults to 1.
51085108
// More info: https://kubernetes.io/docs/concepts/workloads/controllers/replicationcontroller#what-is-a-replicationcontroller
51095109
// +optional
5110+
// +k8s:optional
5111+
// +default=1
5112+
// +k8s:minimum=0
51105113
Replicas *int32 `json:"replicas,omitempty" protobuf:"varint,1,opt,name=replicas"`
51115114

51125115
// Minimum number of seconds for which a newly created pod should be ready
51135116
// without any of its container crashing, for it to be considered available.
51145117
// Defaults to 0 (pod will be considered available as soon as it is ready)
51155118
// +optional
5119+
// +k8s:optional
5120+
// +default=0
5121+
// +k8s:minimum=0
51165122
MinReadySeconds int32 `json:"minReadySeconds,omitempty" protobuf:"varint,4,opt,name=minReadySeconds"`
51175123

51185124
// Selector is a label query over pods that should match the Replicas count.

0 commit comments

Comments
 (0)