github/kubernetes
1
0
Fork 0
mirror of https://github.com/k3s-io/kubernetes.git synced 2025-07-30 23:15:14 +00:00
Code Issues Projects Releases Wiki Activity

add validation, field disablement and tests

Browse Source
This commit is contained in:
Filip Křepinský 2024-11-04 19:19:17 +01:00
parent f7c46df665
commit 14783b8a9b
6 changed files with 271 additions and 51 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
pkg/apis/apps/validation/validation.go
View File

@ -606,6 +606,9 @@ func ValidateDeploymentStatus(status *apps.DeploymentStatus, fldPath *field.Path
allErrs = append(allErrs, apivalidation.ValidateNonnegativeField(int64(status.ReadyReplicas), fldPath.Child("readyReplicas"))...)
allErrs = append(allErrs, apivalidation.ValidateNonnegativeField(int64(status.AvailableReplicas), fldPath.Child("availableReplicas"))...)
allErrs = append(allErrs, apivalidation.ValidateNonnegativeField(int64(status.UnavailableReplicas), fldPath.Child("unavailableReplicas"))...)
if status.TerminatingReplicas != nil {
allErrs = append(allErrs, apivalidation.ValidateNonnegativeField(int64(*status.TerminatingReplicas), fldPath.Child("terminatingReplicas"))...)
}
if status.CollisionCount != nil {
allErrs = append(allErrs, apivalidation.ValidateNonnegativeField(int64(*status.CollisionCount), fldPath.Child("collisionCount"))...)
}
@ -702,6 +705,9 @@ func ValidateReplicaSetStatus(status apps.ReplicaSetStatus, fldPath *field.Path)
allErrs = append(allErrs, apivalidation.ValidateNonnegativeField(int64(status.ReadyReplicas), fldPath.Child("readyReplicas"))...)
allErrs = append(allErrs, apivalidation.ValidateNonnegativeField(int64(status.AvailableReplicas), fldPath.Child("availableReplicas"))...)
allErrs = append(allErrs, apivalidation.ValidateNonnegativeField(int64(status.ObservedGeneration), fldPath.Child("observedGeneration"))...)
if status.TerminatingReplicas != nil {
allErrs = append(allErrs, apivalidation.ValidateNonnegativeField(int64(*status.TerminatingReplicas), fldPath.Child("terminatingReplicas"))...)
}
msg := "cannot be greater than status.replicas"
if status.FullyLabeledReplicas > status.Replicas {
allErrs = append(allErrs, field.Invalid(fldPath.Child("fullyLabeledReplicas"), status.FullyLabeledReplicas, msg))

158
pkg/apis/apps/validation/validation_test.go
View File

@ -2334,59 +2334,84 @@ func TestValidateDeploymentStatus(t *testing.T) {
tests := []struct {
name string
replicas int32
updatedReplicas int32
readyReplicas int32
availableReplicas int32
observedGeneration int64
collisionCount *int32
replicas int32
updatedReplicas int32
readyReplicas int32
availableReplicas int32
terminatingReplicas *int32
observedGeneration int64
collisionCount *int32
expectedErr bool
}{{
name: "valid status",
replicas: 3,
updatedReplicas: 3,
readyReplicas: 2,
availableReplicas: 1,
observedGeneration: 2,
expectedErr: false,
name: "valid status",
replicas: 3,
updatedReplicas: 3,
readyReplicas: 2,
availableReplicas: 1,
terminatingReplicas: nil,
observedGeneration: 2,
expectedErr: false,
}, {
name: "invalid replicas",
replicas: -1,
updatedReplicas: 2,
readyReplicas: 2,
availableReplicas: 1,
observedGeneration: 2,
expectedErr: true,
name: "valid status with terminating replicas",
replicas: 3,
updatedReplicas: 3,
readyReplicas: 2,
availableReplicas: 1,
terminatingReplicas: ptr.To[int32](5),
observedGeneration: 2,
expectedErr: false,
}, {
name: "invalid updatedReplicas",
replicas: 2,
updatedReplicas: -1,
readyReplicas: 2,
availableReplicas: 1,
observedGeneration: 2,
expectedErr: true,
name: "invalid replicas",
replicas: -1,
updatedReplicas: 2,
readyReplicas: 2,
availableReplicas: 1,
terminatingReplicas: nil,
observedGeneration: 2,
expectedErr: true,
}, {
name: "invalid readyReplicas",
replicas: 3,
readyReplicas: -1,
availableReplicas: 1,
observedGeneration: 2,
expectedErr: true,
name: "invalid updatedReplicas",
replicas: 2,
updatedReplicas: -1,
readyReplicas: 2,
availableReplicas: 1,
terminatingReplicas: nil,
observedGeneration: 2,
expectedErr: true,
}, {
name: "invalid availableReplicas",
replicas: 3,
readyReplicas: 3,
availableReplicas: -1,
observedGeneration: 2,
expectedErr: true,
name: "invalid readyReplicas",
replicas: 3,
readyReplicas: -1,
availableReplicas: 1,
terminatingReplicas: nil,
observedGeneration: 2,
expectedErr: true,
}, {
name: "invalid observedGeneration",
replicas: 3,
readyReplicas: 3,
availableReplicas: 3,
observedGeneration: -1,
expectedErr: true,
name: "invalid availableReplicas",
replicas: 3,
readyReplicas: 3,
availableReplicas: -1,
terminatingReplicas: nil,
observedGeneration: 2,
expectedErr: true,
}, {
name: "invalid terminatingReplicas",
replicas: 3,
updatedReplicas: 3,
readyReplicas: 2,
availableReplicas: 1,
terminatingReplicas: ptr.To[int32](-1),
observedGeneration: 2,
expectedErr: true,
}, {
name: "invalid observedGeneration",
replicas: 3,
readyReplicas: 3,
availableReplicas: 3,
terminatingReplicas: nil,
observedGeneration: -1,
expectedErr: true,
}, {
name: "updatedReplicas greater than replicas",
replicas: 3,
@ -2427,12 +2452,13 @@ func TestValidateDeploymentStatus(t *testing.T) {
for _, test := range tests {
status := apps.DeploymentStatus{
Replicas: test.replicas,
UpdatedReplicas: test.updatedReplicas,
ReadyReplicas: test.readyReplicas,
AvailableReplicas: test.availableReplicas,
ObservedGeneration: test.observedGeneration,
CollisionCount: test.collisionCount,
Replicas: test.replicas,
UpdatedReplicas: test.updatedReplicas,
ReadyReplicas: test.readyReplicas,
AvailableReplicas: test.availableReplicas,
TerminatingReplicas: test.terminatingReplicas,
ObservedGeneration: test.observedGeneration,
CollisionCount: test.collisionCount,
}
errs := ValidateDeploymentStatus(&status, field.NewPath("status"))
@ -2735,6 +2761,7 @@ func TestValidateReplicaSetStatus(t *testing.T) {
fullyLabeledReplicas int32
readyReplicas int32
availableReplicas int32
terminatingReplicas *int32
observedGeneration int64
expectedErr bool
@ -2744,6 +2771,16 @@ func TestValidateReplicaSetStatus(t *testing.T) {
fullyLabeledReplicas: 3,
readyReplicas: 2,
availableReplicas: 1,
terminatingReplicas: nil,
observedGeneration: 2,
expectedErr: false,
}, {
name: "valid status with terminating replicas",
replicas: 3,
fullyLabeledReplicas: 3,
readyReplicas: 2,
availableReplicas: 1,
terminatingReplicas: ptr.To[int32](5),
observedGeneration: 2,
expectedErr: false,
}, {
@ -2752,6 +2789,7 @@ func TestValidateReplicaSetStatus(t *testing.T) {
fullyLabeledReplicas: 3,
readyReplicas: 2,
availableReplicas: 1,
terminatingReplicas: nil,
observedGeneration: 2,
expectedErr: true,
}, {
@ -2760,6 +2798,7 @@ func TestValidateReplicaSetStatus(t *testing.T) {
fullyLabeledReplicas: -1,
readyReplicas: 2,
availableReplicas: 1,
terminatingReplicas: nil,
observedGeneration: 2,
expectedErr: true,
}, {
@ -2768,6 +2807,7 @@ func TestValidateReplicaSetStatus(t *testing.T) {
fullyLabeledReplicas: 3,
readyReplicas: -1,
availableReplicas: 1,
terminatingReplicas: nil,
observedGeneration: 2,
expectedErr: true,
}, {
@ -2776,6 +2816,16 @@ func TestValidateReplicaSetStatus(t *testing.T) {
fullyLabeledReplicas: 3,
readyReplicas: 3,
availableReplicas: -1,
terminatingReplicas: nil,
observedGeneration: 2,
expectedErr: true,
}, {
name: "invalid terminatingReplicas",
replicas: 3,
fullyLabeledReplicas: 3,
readyReplicas: 2,
availableReplicas: 1,
terminatingReplicas: ptr.To[int32](-1),
observedGeneration: 2,
expectedErr: true,
}, {
@ -2784,6 +2834,7 @@ func TestValidateReplicaSetStatus(t *testing.T) {
fullyLabeledReplicas: 3,
readyReplicas: 3,
availableReplicas: 3,
terminatingReplicas: nil,
observedGeneration: -1,
expectedErr: true,
}, {
@ -2792,6 +2843,7 @@ func TestValidateReplicaSetStatus(t *testing.T) {
fullyLabeledReplicas: 4,
readyReplicas: 3,
availableReplicas: 3,
terminatingReplicas: nil,
observedGeneration: 1,
expectedErr: true,
}, {
@ -2800,6 +2852,7 @@ func TestValidateReplicaSetStatus(t *testing.T) {
fullyLabeledReplicas: 3,
readyReplicas: 4,
availableReplicas: 3,
terminatingReplicas: nil,
observedGeneration: 1,
expectedErr: true,
}, {
@ -2808,6 +2861,7 @@ func TestValidateReplicaSetStatus(t *testing.T) {
fullyLabeledReplicas: 3,
readyReplicas: 3,
availableReplicas: 4,
terminatingReplicas: nil,
observedGeneration: 1,
expectedErr: true,
}, {
@ -2816,6 +2870,7 @@ func TestValidateReplicaSetStatus(t *testing.T) {
fullyLabeledReplicas: 3,
readyReplicas: 2,
availableReplicas: 3,
terminatingReplicas: nil,
observedGeneration: 1,
expectedErr: true,
},
@ -2827,6 +2882,7 @@ func TestValidateReplicaSetStatus(t *testing.T) {
FullyLabeledReplicas: test.fullyLabeledReplicas,
ReadyReplicas: test.readyReplicas,
AvailableReplicas: test.availableReplicas,
TerminatingReplicas: test.terminatingReplicas,
ObservedGeneration: test.observedGeneration,
}

11
pkg/registry/apps/deployment/strategy.go
View File

@ -31,10 +31,12 @@ import (
genericapirequest "k8s.io/apiserver/pkg/endpoints/request"
"k8s.io/apiserver/pkg/registry/rest"
"k8s.io/apiserver/pkg/storage/names"
utilfeature "k8s.io/apiserver/pkg/util/feature"
"k8s.io/kubernetes/pkg/api/legacyscheme"
"k8s.io/kubernetes/pkg/api/pod"
"k8s.io/kubernetes/pkg/apis/apps"
appsvalidation "k8s.io/kubernetes/pkg/apis/apps/validation"
"k8s.io/kubernetes/pkg/features"
"sigs.k8s.io/structured-merge-diff/v4/fieldpath"
)
@ -192,6 +194,7 @@ func (deploymentStatusStrategy) PrepareForUpdate(ctx context.Context, obj, old r
oldDeployment := old.(*apps.Deployment)
newDeployment.Spec = oldDeployment.Spec
newDeployment.Labels = oldDeployment.Labels
dropDisabledStatusFields(&newDeployment.Status, &oldDeployment.Status)
}
// 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
func (deploymentStatusStrategy) WarningsOnUpdate(ctx context.Context, obj, old runtime.Object) []string {
return nil
}
// dropDisabledStatusFields removes disabled fields from the deployment status.
func dropDisabledStatusFields(deploymentStatus, oldDeploymentStatus *apps.DeploymentStatus) {
if !utilfeature.DefaultFeatureGate.Enabled(features.DeploymentPodReplacementPolicy) &&
(oldDeploymentStatus == nil || oldDeploymentStatus.TerminatingReplicas == nil) {
deploymentStatus.TerminatingReplicas = nil
}
}

68
pkg/registry/apps/deployment/strategy_test.go
View File

@ -17,6 +17,7 @@ limitations under the License.
package deployment
import (
"k8s.io/utils/ptr"
"reflect"
"testing"
@ -26,9 +27,12 @@ import (
"k8s.io/apimachinery/pkg/util/intstr"
"k8s.io/apimachinery/pkg/util/validation/field"
genericapirequest "k8s.io/apiserver/pkg/endpoints/request"
utilfeature "k8s.io/apiserver/pkg/util/feature"
featuregatetesting "k8s.io/component-base/featuregate/testing"
podtest "k8s.io/kubernetes/pkg/api/pod/testing"
"k8s.io/kubernetes/pkg/apis/apps"
api "k8s.io/kubernetes/pkg/apis/core"
"k8s.io/kubernetes/pkg/features"
)
const (
@ -62,6 +66,70 @@ func TestStatusUpdates(t *testing.T) {
}
}
func TestStatusUpdatesWithDeploymentPodReplacementPolicy(t *testing.T) {
tests := []struct {
name string
enableDeploymentPodReplacementPolicy bool
terminatingReplicas *int32
terminatingReplicasUpdate *int32
expectedTerminatingReplicas *int32
}{
{
name: "should not allow updates when feature gate is disabled",
enableDeploymentPodReplacementPolicy: false,
terminatingReplicas: nil,
terminatingReplicasUpdate: ptr.To[int32](2),
expectedTerminatingReplicas: nil,
},
{
name: "should allow update when the field is in use when feature gate is disabled",
enableDeploymentPodReplacementPolicy: false,
terminatingReplicas: ptr.To[int32](2),
terminatingReplicasUpdate: ptr.To[int32](5),
expectedTerminatingReplicas: ptr.To[int32](5),
},
{
name: "should allow updates when feature gate is enabled",
enableDeploymentPodReplacementPolicy: true,
terminatingReplicas: nil,
terminatingReplicasUpdate: ptr.To[int32](2),
expectedTerminatingReplicas: ptr.To[int32](2),
},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.DeploymentPodReplacementPolicy, tc.enableDeploymentPodReplacementPolicy)
ctx := genericapirequest.NewDefaultContext()
validSelector := map[string]string{"a": "b"}
oldDeploy := newDeploymentWithSelectorLabels(validSelector)
oldDeploy.Spec.Replicas = 3
oldDeploy.Status.Replicas = 3
oldDeploy.Status.TerminatingReplicas = tc.terminatingReplicas
newDeploy := newDeploymentWithSelectorLabels(validSelector)
newDeploy.Spec.Replicas = 3
newDeploy.Status.Replicas = 2
newDeploy.Status.TerminatingReplicas = tc.terminatingReplicasUpdate
StatusStrategy.PrepareForUpdate(ctx, newDeploy, oldDeploy)
if newDeploy.Status.Replicas != 2 {
t.Errorf("ReplicaSet status updates should allow change of replicas: %v", newDeploy.Status.Replicas)
}
if !ptr.Equal(newDeploy.Status.TerminatingReplicas, tc.expectedTerminatingReplicas) {
t.Errorf("ReplicaSet status updates failed, expected terminating pods: %v, got: %v", ptr.Deref(tc.expectedTerminatingReplicas, -1), ptr.Deref(newDeploy.Status.TerminatingReplicas, -1))
}
errs := StatusStrategy.ValidateUpdate(ctx, newDeploy, oldDeploy)
if len(errs) != 0 {
t.Errorf("Unexpected error %v", errs)
}
})
}
}
func newDeployment(labels, annotations map[string]string) *apps.Deployment {
return &apps.Deployment{
ObjectMeta: metav1.ObjectMeta{

11
pkg/registry/apps/replicaset/strategy.go
View File

@ -37,10 +37,12 @@ import (
"k8s.io/apiserver/pkg/registry/rest"
apistorage "k8s.io/apiserver/pkg/storage"
"k8s.io/apiserver/pkg/storage/names"
utilfeature "k8s.io/apiserver/pkg/util/feature"
"k8s.io/kubernetes/pkg/api/legacyscheme"
"k8s.io/kubernetes/pkg/api/pod"
"k8s.io/kubernetes/pkg/apis/apps"
appsvalidation "k8s.io/kubernetes/pkg/apis/apps/validation"
"k8s.io/kubernetes/pkg/features"
"sigs.k8s.io/structured-merge-diff/v4/fieldpath"
)
@ -231,6 +233,7 @@ func (rsStatusStrategy) PrepareForUpdate(ctx context.Context, obj, old runtime.O
oldRS := old.(*apps.ReplicaSet)
// update is not allowed to set spec
newRS.Spec = oldRS.Spec
dropDisabledStatusFields(&newRS.Status, &oldRS.Status)
}
func (rsStatusStrategy) ValidateUpdate(ctx context.Context, obj, old runtime.Object) field.ErrorList {
@ -241,3 +244,11 @@ func (rsStatusStrategy) ValidateUpdate(ctx context.Context, obj, old runtime.Obj
func (rsStatusStrategy) WarningsOnUpdate(ctx context.Context, obj, old runtime.Object) []string {
return nil
}
// dropDisabledStatusFields removes disabled fields from the replica set status.
func dropDisabledStatusFields(rsStatus, oldRSStatus *apps.ReplicaSetStatus) {
if !utilfeature.DefaultFeatureGate.Enabled(features.DeploymentPodReplacementPolicy) &&
(oldRSStatus == nil || oldRSStatus.TerminatingReplicas == nil) {
rsStatus.TerminatingReplicas = nil
}
}

68
pkg/registry/apps/replicaset/strategy_test.go
View File

@ -17,15 +17,19 @@ limitations under the License.
package replicaset
import (
"k8s.io/utils/ptr"
"reflect"
"testing"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/validation/field"
genericapirequest "k8s.io/apiserver/pkg/endpoints/request"
utilfeature "k8s.io/apiserver/pkg/util/feature"
featuregatetesting "k8s.io/component-base/featuregate/testing"
podtest "k8s.io/kubernetes/pkg/api/pod/testing"
"k8s.io/kubernetes/pkg/apis/apps"
api "k8s.io/kubernetes/pkg/apis/core"
"k8s.io/kubernetes/pkg/features"
)
const (
@ -148,6 +152,70 @@ func TestReplicaSetStatusStrategy(t *testing.T) {
}
}
func TestReplicaSetStatusStrategyWithDeploymentPodReplacementPolicy(t *testing.T) {
tests := []struct {
name string
enableDeploymentPodReplacementPolicy bool
terminatingReplicas *int32
terminatingReplicasUpdate *int32
expectedTerminatingReplicas *int32
}{
{
name: "should not allow updates when feature gate is disabled",
enableDeploymentPodReplacementPolicy: false,
terminatingReplicas: nil,
terminatingReplicasUpdate: ptr.To[int32](2),
expectedTerminatingReplicas: nil,
},
{
name: "should allow update when the field is in use when feature gate is disabled",
enableDeploymentPodReplacementPolicy: false,
terminatingReplicas: ptr.To[int32](2),
terminatingReplicasUpdate: ptr.To[int32](5),
expectedTerminatingReplicas: ptr.To[int32](5),
},
{
name: "should allow updates when feature gate is enabled",
enableDeploymentPodReplacementPolicy: true,
terminatingReplicas: nil,
terminatingReplicasUpdate: ptr.To[int32](2),
expectedTerminatingReplicas: ptr.To[int32](2),
},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.DeploymentPodReplacementPolicy, tc.enableDeploymentPodReplacementPolicy)
ctx := genericapirequest.NewDefaultContext()
validSelector := map[string]string{"a": "b"}
oldRS := newReplicaSetWithSelectorLabels(validSelector)
oldRS.Spec.Replicas = 3
oldRS.Status.Replicas = 3
oldRS.Status.TerminatingReplicas = tc.terminatingReplicas
newRS := newReplicaSetWithSelectorLabels(validSelector)
newRS.Spec.Replicas = 3
newRS.Status.Replicas = 2
newRS.Status.TerminatingReplicas = tc.terminatingReplicasUpdate
StatusStrategy.PrepareForUpdate(ctx, newRS, oldRS)
if newRS.Status.Replicas != 2 {
t.Errorf("ReplicaSet status updates should allow change of replicas: %v", newRS.Status.Replicas)
}
if !ptr.Equal(newRS.Status.TerminatingReplicas, tc.expectedTerminatingReplicas) {
t.Errorf("ReplicaSet status updates failed, expected terminating pods: %v, got: %v", ptr.Deref(tc.expectedTerminatingReplicas, -1), ptr.Deref(newRS.Status.TerminatingReplicas, -1))
}
errs := StatusStrategy.ValidateUpdate(ctx, newRS, oldRS)
if len(errs) != 0 {
t.Errorf("Unexpected error %v", errs)
}
})
}
}
func TestSelectorImmutability(t *testing.T) {
tests := []struct {
requestInfo genericapirequest.RequestInfo