Skip to content

Commit 14783b8

Browse files
atiratreeatiratree
committed
add validation, field disablement and tests
1 parent f7c46df commit 14783b8

File tree

6 files changed

+276
-56
lines changed

6 files changed

+276
-56
lines changed

‎pkg/apis/apps/validation/validation.go‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -606,6 +606,9 @@ func ValidateDeploymentStatus(status *apps.DeploymentStatus, fldPath *field.Path
606606
allErrs = append(allErrs, apivalidation.ValidateNonnegativeField(int64(status.ReadyReplicas), fldPath.Child("readyReplicas"))...)
607607
allErrs = append(allErrs, apivalidation.ValidateNonnegativeField(int64(status.AvailableReplicas), fldPath.Child("availableReplicas"))...)
608608
allErrs = append(allErrs, apivalidation.ValidateNonnegativeField(int64(status.UnavailableReplicas), fldPath.Child("unavailableReplicas"))...)
609+
if status.TerminatingReplicas != nil {
610+
allErrs = append(allErrs, apivalidation.ValidateNonnegativeField(int64(*status.TerminatingReplicas), fldPath.Child("terminatingReplicas"))...)
611+
}
609612
if status.CollisionCount != nil {
610613
allErrs = append(allErrs, apivalidation.ValidateNonnegativeField(int64(*status.CollisionCount), fldPath.Child("collisionCount"))...)
611614
}
@@ -702,6 +705,9 @@ func ValidateReplicaSetStatus(status apps.ReplicaSetStatus, fldPath *field.Path)
702705
allErrs = append(allErrs, apivalidation.ValidateNonnegativeField(int64(status.ReadyReplicas), fldPath.Child("readyReplicas"))...)
703706
allErrs = append(allErrs, apivalidation.ValidateNonnegativeField(int64(status.AvailableReplicas), fldPath.Child("availableReplicas"))...)
704707
allErrs = append(allErrs, apivalidation.ValidateNonnegativeField(int64(status.ObservedGeneration), fldPath.Child("observedGeneration"))...)
708+
if status.TerminatingReplicas != nil {
709+
allErrs = append(allErrs, apivalidation.ValidateNonnegativeField(int64(*status.TerminatingReplicas), fldPath.Child("terminatingReplicas"))...)
710+
}
705711
msg := "cannot be greater than status.replicas"
706712
if status.FullyLabeledReplicas > status.Replicas {
707713
allErrs = append(allErrs, field.Invalid(fldPath.Child("fullyLabeledReplicas"), status.FullyLabeledReplicas, msg))

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

Lines changed: 112 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -2334,59 +2334,84 @@ func TestValidateDeploymentStatus(t *testing.T) {
23342334
tests := []struct {
23352335
name string
23362336

2337-
replicas int32
2338-
updatedReplicas int32
2339-
readyReplicas int32
2340-
availableReplicas int32
2341-
observedGeneration int64
2342-
collisionCount *int32
2337+
replicas int32
2338+
updatedReplicas int32
2339+
readyReplicas int32
2340+
availableReplicas int32
2341+
terminatingReplicas *int32
2342+
observedGeneration int64
2343+
collisionCount *int32
23432344

23442345
expectedErr bool
23452346
}{{
2346-
name: "valid status",
2347-
replicas: 3,
2348-
updatedReplicas: 3,
2349-
readyReplicas: 2,
2350-
availableReplicas: 1,
2351-
observedGeneration: 2,
2352-
expectedErr: false,
2353-
}, {
2354-
name: "invalid replicas",
2355-
replicas: -1,
2356-
updatedReplicas: 2,
2357-
readyReplicas: 2,
2358-
availableReplicas: 1,
2359-
observedGeneration: 2,
2360-
expectedErr: true,
2361-
}, {
2362-
name: "invalid updatedReplicas",
2363-
replicas: 2,
2364-
updatedReplicas: -1,
2365-
readyReplicas: 2,
2366-
availableReplicas: 1,
2367-
observedGeneration: 2,
2368-
expectedErr: true,
2369-
}, {
2370-
name: "invalid readyReplicas",
2371-
replicas: 3,
2372-
readyReplicas: -1,
2373-
availableReplicas: 1,
2374-
observedGeneration: 2,
2375-
expectedErr: true,
2376-
}, {
2377-
name: "invalid availableReplicas",
2378-
replicas: 3,
2379-
readyReplicas: 3,
2380-
availableReplicas: -1,
2381-
observedGeneration: 2,
2382-
expectedErr: true,
2383-
}, {
2384-
name: "invalid observedGeneration",
2385-
replicas: 3,
2386-
readyReplicas: 3,
2387-
availableReplicas: 3,
2388-
observedGeneration: -1,
2389-
expectedErr: true,
2347+
name: "valid status",
2348+
replicas: 3,
2349+
updatedReplicas: 3,
2350+
readyReplicas: 2,
2351+
availableReplicas: 1,
2352+
terminatingReplicas: nil,
2353+
observedGeneration: 2,
2354+
expectedErr: false,
2355+
}, {
2356+
name: "valid status with terminating replicas",
2357+
replicas: 3,
2358+
updatedReplicas: 3,
2359+
readyReplicas: 2,
2360+
availableReplicas: 1,
2361+
terminatingReplicas: ptr.To[int32](5),
2362+
observedGeneration: 2,
2363+
expectedErr: false,
2364+
}, {
2365+
name: "invalid replicas",
2366+
replicas: -1,
2367+
updatedReplicas: 2,
2368+
readyReplicas: 2,
2369+
availableReplicas: 1,
2370+
terminatingReplicas: nil,
2371+
observedGeneration: 2,
2372+
expectedErr: true,
2373+
}, {
2374+
name: "invalid updatedReplicas",
2375+
replicas: 2,
2376+
updatedReplicas: -1,
2377+
readyReplicas: 2,
2378+
availableReplicas: 1,
2379+
terminatingReplicas: nil,
2380+
observedGeneration: 2,
2381+
expectedErr: true,
2382+
}, {
2383+
name: "invalid readyReplicas",
2384+
replicas: 3,
2385+
readyReplicas: -1,
2386+
availableReplicas: 1,
2387+
terminatingReplicas: nil,
2388+
observedGeneration: 2,
2389+
expectedErr: true,
2390+
}, {
2391+
name: "invalid availableReplicas",
2392+
replicas: 3,
2393+
readyReplicas: 3,
2394+
availableReplicas: -1,
2395+
terminatingReplicas: nil,
2396+
observedGeneration: 2,
2397+
expectedErr: true,
2398+
}, {
2399+
name: "invalid terminatingReplicas",
2400+
replicas: 3,
2401+
updatedReplicas: 3,
2402+
readyReplicas: 2,
2403+
availableReplicas: 1,
2404+
terminatingReplicas: ptr.To[int32](-1),
2405+
observedGeneration: 2,
2406+
expectedErr: true,
2407+
}, {
2408+
name: "invalid observedGeneration",
2409+
replicas: 3,
2410+
readyReplicas: 3,
2411+
availableReplicas: 3,
2412+
terminatingReplicas: nil,
2413+
observedGeneration: -1,
2414+
expectedErr: true,
23902415
}, {
23912416
name: "updatedReplicas greater than replicas",
23922417
replicas: 3,
@@ -2427,12 +2452,13 @@ func TestValidateDeploymentStatus(t *testing.T) {
24272452

24282453
for _, test := range tests {
24292454
status := apps.DeploymentStatus{
2430-
Replicas: test.replicas,
2431-
UpdatedReplicas: test.updatedReplicas,
2432-
ReadyReplicas: test.readyReplicas,
2433-
AvailableReplicas: test.availableReplicas,
2434-
ObservedGeneration: test.observedGeneration,
2435-
CollisionCount: test.collisionCount,
2455+
Replicas: test.replicas,
2456+
UpdatedReplicas: test.updatedReplicas,
2457+
ReadyReplicas: test.readyReplicas,
2458+
AvailableReplicas: test.availableReplicas,
2459+
TerminatingReplicas: test.terminatingReplicas,
2460+
ObservedGeneration: test.observedGeneration,
2461+
CollisionCount: test.collisionCount,
24362462
}
24372463

24382464
errs := ValidateDeploymentStatus(&status, field.NewPath("status"))
@@ -2735,6 +2761,7 @@ func TestValidateReplicaSetStatus(t *testing.T) {
27352761
fullyLabeledReplicas int32
27362762
readyReplicas int32
27372763
availableReplicas int32
2764+
terminatingReplicas *int32
27382765
observedGeneration int64
27392766

27402767
expectedErr bool
@@ -2744,6 +2771,16 @@ func TestValidateReplicaSetStatus(t *testing.T) {
27442771
fullyLabeledReplicas: 3,
27452772
readyReplicas: 2,
27462773
availableReplicas: 1,
2774+
terminatingReplicas: nil,
2775+
observedGeneration: 2,
2776+
expectedErr: false,
2777+
}, {
2778+
name: "valid status with terminating replicas",
2779+
replicas: 3,
2780+
fullyLabeledReplicas: 3,
2781+
readyReplicas: 2,
2782+
availableReplicas: 1,
2783+
terminatingReplicas: ptr.To[int32](5),
27472784
observedGeneration: 2,
27482785
expectedErr: false,
27492786
}, {
@@ -2752,6 +2789,7 @@ func TestValidateReplicaSetStatus(t *testing.T) {
27522789
fullyLabeledReplicas: 3,
27532790
readyReplicas: 2,
27542791
availableReplicas: 1,
2792+
terminatingReplicas: nil,
27552793
observedGeneration: 2,
27562794
expectedErr: true,
27572795
}, {
@@ -2760,6 +2798,7 @@ func TestValidateReplicaSetStatus(t *testing.T) {
27602798
fullyLabeledReplicas: -1,
27612799
readyReplicas: 2,
27622800
availableReplicas: 1,
2801+
terminatingReplicas: nil,
27632802
observedGeneration: 2,
27642803
expectedErr: true,
27652804
}, {
@@ -2768,6 +2807,7 @@ func TestValidateReplicaSetStatus(t *testing.T) {
27682807
fullyLabeledReplicas: 3,
27692808
readyReplicas: -1,
27702809
availableReplicas: 1,
2810+
terminatingReplicas: nil,
27712811
observedGeneration: 2,
27722812
expectedErr: true,
27732813
}, {
@@ -2776,6 +2816,16 @@ func TestValidateReplicaSetStatus(t *testing.T) {
27762816
fullyLabeledReplicas: 3,
27772817
readyReplicas: 3,
27782818
availableReplicas: -1,
2819+
terminatingReplicas: nil,
2820+
observedGeneration: 2,
2821+
expectedErr: true,
2822+
}, {
2823+
name: "invalid terminatingReplicas",
2824+
replicas: 3,
2825+
fullyLabeledReplicas: 3,
2826+
readyReplicas: 2,
2827+
availableReplicas: 1,
2828+
terminatingReplicas: ptr.To[int32](-1),
27792829
observedGeneration: 2,
27802830
expectedErr: true,
27812831
}, {
@@ -2784,6 +2834,7 @@ func TestValidateReplicaSetStatus(t *testing.T) {
27842834
fullyLabeledReplicas: 3,
27852835
readyReplicas: 3,
27862836
availableReplicas: 3,
2837+
terminatingReplicas: nil,
27872838
observedGeneration: -1,
27882839
expectedErr: true,
27892840
}, {
@@ -2792,6 +2843,7 @@ func TestValidateReplicaSetStatus(t *testing.T) {
27922843
fullyLabeledReplicas: 4,
27932844
readyReplicas: 3,
27942845
availableReplicas: 3,
2846+
terminatingReplicas: nil,
27952847
observedGeneration: 1,
27962848
expectedErr: true,
27972849
}, {
@@ -2800,6 +2852,7 @@ func TestValidateReplicaSetStatus(t *testing.T) {
28002852
fullyLabeledReplicas: 3,
28012853
readyReplicas: 4,
28022854
availableReplicas: 3,
2855+
terminatingReplicas: nil,
28032856
observedGeneration: 1,
28042857
expectedErr: true,
28052858
}, {
@@ -2808,6 +2861,7 @@ func TestValidateReplicaSetStatus(t *testing.T) {
28082861
fullyLabeledReplicas: 3,
28092862
readyReplicas: 3,
28102863
availableReplicas: 4,
2864+
terminatingReplicas: nil,
28112865
observedGeneration: 1,
28122866
expectedErr: true,
28132867
}, {
@@ -2816,6 +2870,7 @@ func TestValidateReplicaSetStatus(t *testing.T) {
28162870
fullyLabeledReplicas: 3,
28172871
readyReplicas: 2,
28182872
availableReplicas: 3,
2873+
terminatingReplicas: nil,
28192874
observedGeneration: 1,
28202875
expectedErr: true,
28212876
},
@@ -2827,6 +2882,7 @@ func TestValidateReplicaSetStatus(t *testing.T) {
28272882
FullyLabeledReplicas: test.fullyLabeledReplicas,
28282883
ReadyReplicas: test.readyReplicas,
28292884
AvailableReplicas: test.availableReplicas,
2885+
TerminatingReplicas: test.terminatingReplicas,
28302886
ObservedGeneration: test.observedGeneration,
28312887
}
28322888

‎pkg/registry/apps/deployment/strategy.go‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,10 +31,12 @@ import (
3131
genericapirequest "k8s.io/apiserver/pkg/endpoints/request"
3232
"k8s.io/apiserver/pkg/registry/rest"
3333
"k8s.io/apiserver/pkg/storage/names"
34+
utilfeature "k8s.io/apiserver/pkg/util/feature"
3435
"k8s.io/kubernetes/pkg/api/legacyscheme"
3536
"k8s.io/kubernetes/pkg/api/pod"
3637
"k8s.io/kubernetes/pkg/apis/apps"
3738
appsvalidation "k8s.io/kubernetes/pkg/apis/apps/validation"
39+
"k8s.io/kubernetes/pkg/features"
3840
"sigs.k8s.io/structured-merge-diff/v4/fieldpath"
3941
)
4042

@@ -192,6 +194,7 @@ func (deploymentStatusStrategy) PrepareForUpdate(ctx context.Context, obj, old r
192194
oldDeployment := old.(*apps.Deployment)
193195
newDeployment.Spec = oldDeployment.Spec
194196
newDeployment.Labels = oldDeployment.Labels
197+
dropDisabledStatusFields(&newDeployment.Status, &oldDeployment.Status)
195198
}
196199

197200
// ValidateUpdate is the default update validation for an end user updating status
@@ -203,3 +206,11 @@ func (deploymentStatusStrategy) ValidateUpdate(ctx context.Context, obj, old run
203206
func (deploymentStatusStrategy) WarningsOnUpdate(ctx context.Context, obj, old runtime.Object) []string {
204207
return nil
205208
}
209+
210+
// dropDisabledStatusFields removes disabled fields from the deployment status.
211+
func dropDisabledStatusFields(deploymentStatus, oldDeploymentStatus *apps.DeploymentStatus) {
212+
if !utilfeature.DefaultFeatureGate.Enabled(features.DeploymentPodReplacementPolicy) &&
213+
(oldDeploymentStatus == nil || oldDeploymentStatus.TerminatingReplicas == nil) {
214+
deploymentStatus.TerminatingReplicas = nil
215+
}
216+
}

0 commit comments

Comments
 (0)