Merge pull request #130725 from jpbetz/replication-controller-minimums-to-declarative

Migrate to declarative validation: ReplicationController spec.replicas and spec.minReadySeconds fields

api/openapi-spec/v3/api__v1_openapi.json

@@ -6523,11 +6523,13 @@
         "description": "ReplicationControllerSpec is the specification of a replication controller.",
         "properties": {
           "minReadySeconds": {
+            "default": 0,
             "description": "Minimum number of seconds for which a newly created pod should be ready without any of its container crashing, for it to be considered available. Defaults to 0 (pod will be considered available as soon as it is ready)",
             "format": "int32",
             "type": "integer"
           },
           "replicas": {
+            "default": 1,
             "description": "Replicas is the number of desired replicas. This is a pointer to distinguish between explicit zero and unspecified. Defaults to 1. More info: https://kubernetes.io/docs/concepts/workloads/controllers/replicationcontroller#what-is-a-replicationcontroller",
             "format": "int32",
             "type": "integer"

pkg/apis/core/v1/defaults.go

@@ -60,10 +60,7 @@ func SetDefaults_ReplicationController(obj *v1.ReplicationController) {
 			obj.Labels = labels
 		}
 	}
-	if obj.Spec.Replicas == nil {
+	// obj.Spec.Replicas is defaulted declaratively
-		obj.Spec.Replicas = new(int32)
-		*obj.Spec.Replicas = 1
-	}
 }
 func SetDefaults_Volume(obj *v1.Volume) {
 	if ptr.AllPtrFieldsNil(&obj.VolumeSource) {

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

@@ -878,6 +878,10 @@ func SetObjectDefaults_PodTemplateList(in *corev1.PodTemplateList) {
 
 func SetObjectDefaults_ReplicationController(in *corev1.ReplicationController) {
 	SetDefaults_ReplicationController(in)
+	if in.Spec.Replicas == nil {
+		var ptrVar1 int32 = 1
+		in.Spec.Replicas = &ptrVar1
+	}
 	if in.Spec.Template != nil {
 		SetDefaults_PodSpec(&in.Spec.Template.Spec)
 		for i := range in.Spec.Template.Spec.Volumes {

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

@@ -22,7 +22,15 @@ limitations under the License.
 package v1
 
 import (
+	context "context"
+	fmt "fmt"
+
+	corev1 "k8s.io/api/core/v1"
+	operation "k8s.io/apimachinery/pkg/api/operation"
+	safe "k8s.io/apimachinery/pkg/api/safe"
+	validate "k8s.io/apimachinery/pkg/api/validate"
 	runtime "k8s.io/apimachinery/pkg/runtime"
+	field "k8s.io/apimachinery/pkg/util/validation/field"
 )
 
 func init() { localSchemeBuilder.Register(RegisterValidations) }
@@ -30,5 +38,75 @@ func init() { localSchemeBuilder.Register(RegisterValidations) }
 // RegisterValidations adds validation functions to the given scheme.
 // Public to allow building arbitrary schemes.
 func RegisterValidations(scheme *runtime.Scheme) error {
+	scheme.AddValidationFunc((*corev1.ReplicationController)(nil), func(ctx context.Context, op operation.Operation, obj, oldObj interface{}, subresources ...string) field.ErrorList {
+		if len(subresources) == 0 {
+			return Validate_ReplicationController(ctx, op, nil /* fldPath */, obj.(*corev1.ReplicationController), safe.Cast[*corev1.ReplicationController](oldObj))
+		}
+		if len(subresources) == 1 && subresources[0] == "status" {
+			return nil // corev1.ReplicationControllerStatus has no validation
+		}
+		return field.ErrorList{field.InternalError(nil, fmt.Errorf("no validation found for %T, subresources: %v", obj, subresources))}
+	})
+	scheme.AddValidationFunc((*corev1.ReplicationControllerList)(nil), func(ctx context.Context, op operation.Operation, obj, oldObj interface{}, subresources ...string) field.ErrorList {
+		if len(subresources) == 0 {
+			return Validate_ReplicationControllerList(ctx, op, nil /* fldPath */, obj.(*corev1.ReplicationControllerList), safe.Cast[*corev1.ReplicationControllerList](oldObj))
+		}
+		return field.ErrorList{field.InternalError(nil, fmt.Errorf("no validation found for %T, subresources: %v", obj, subresources))}
+	})
 	return nil
 }
+
+func Validate_ReplicationController(ctx context.Context, op operation.Operation, fldPath *field.Path, obj, oldObj *corev1.ReplicationController) (errs field.ErrorList) {
+	// field corev1.ReplicationController.TypeMeta has no validation
+	// field corev1.ReplicationController.ObjectMeta has no validation
+
+	// field corev1.ReplicationController.Spec
+	errs = append(errs,
+		func(fldPath *field.Path, obj, oldObj *corev1.ReplicationControllerSpec) (errs field.ErrorList) {
+			errs = append(errs, Validate_ReplicationControllerSpec(ctx, op, fldPath, obj, oldObj)...)
+			return
+		}(fldPath.Child("spec"), &obj.Spec, safe.Field(oldObj, func(oldObj *corev1.ReplicationController) *corev1.ReplicationControllerSpec { return &oldObj.Spec }))...)
+
+	// field corev1.ReplicationController.Status has no validation
+	return errs
+}
+
+func Validate_ReplicationControllerList(ctx context.Context, op operation.Operation, fldPath *field.Path, obj, oldObj *corev1.ReplicationControllerList) (errs field.ErrorList) {
+	// field corev1.ReplicationControllerList.TypeMeta has no validation
+	// field corev1.ReplicationControllerList.ListMeta has no validation
+
+	// field corev1.ReplicationControllerList.Items
+	errs = append(errs,
+		func(fldPath *field.Path, obj, oldObj []corev1.ReplicationController) (errs field.ErrorList) {
+			errs = append(errs, validate.EachSliceVal(ctx, op, fldPath, obj, oldObj, nil, Validate_ReplicationController)...)
+			return
+		}(fldPath.Child("items"), obj.Items, safe.Field(oldObj, func(oldObj *corev1.ReplicationControllerList) []corev1.ReplicationController { return oldObj.Items }))...)
+
+	return errs
+}
+
+func Validate_ReplicationControllerSpec(ctx context.Context, op operation.Operation, fldPath *field.Path, obj, oldObj *corev1.ReplicationControllerSpec) (errs field.ErrorList) {
+	// field corev1.ReplicationControllerSpec.Replicas
+	errs = append(errs,
+		func(fldPath *field.Path, obj, oldObj *int32) (errs field.ErrorList) {
+			// optional fields with default values are effectively required
+			if e := validate.RequiredPointer(ctx, op, fldPath, obj, oldObj); len(e) != 0 {
+				errs = append(errs, e...)
+				return // do not proceed
+			}
+			errs = append(errs, validate.Minimum(ctx, op, fldPath, obj, oldObj, 0)...)
+			return
+		}(fldPath.Child("replicas"), obj.Replicas, safe.Field(oldObj, func(oldObj *corev1.ReplicationControllerSpec) *int32 { return oldObj.Replicas }))...)
+
+	// field corev1.ReplicationControllerSpec.MinReadySeconds
+	errs = append(errs,
+		func(fldPath *field.Path, obj, oldObj *int32) (errs field.ErrorList) {
+			errs = append(errs, validate.Minimum(ctx, op, fldPath, obj, oldObj, 0)...)
+			// optional value-type fields with zero-value defaults are purely documentation
+			return
+		}(fldPath.Child("minReadySeconds"), &obj.MinReadySeconds, safe.Field(oldObj, func(oldObj *corev1.ReplicationControllerSpec) *int32 { return &oldObj.MinReadySeconds }))...)
+
+	// field corev1.ReplicationControllerSpec.Selector has no validation
+	// field corev1.ReplicationControllerSpec.Template has no validation
+	return errs
+}

pkg/apis/core/validation/validation.go

@@ -6319,12 +6319,12 @@ func ValidatePodTemplateSpecForRC(template *core.PodTemplateSpec, selectorMap ma
 // ValidateReplicationControllerSpec tests if required fields in the replication controller spec are set.
 func ValidateReplicationControllerSpec(spec, oldSpec *core.ReplicationControllerSpec, fldPath *field.Path, opts PodValidationOptions) field.ErrorList {
 	allErrs := field.ErrorList{}
-	allErrs = append(allErrs, ValidateNonnegativeField(int64(spec.MinReadySeconds), fldPath.Child("minReadySeconds"))...)
+	allErrs = append(allErrs, ValidateNonnegativeField(int64(spec.MinReadySeconds), fldPath.Child("minReadySeconds")).MarkCoveredByDeclarative()...)
 	allErrs = append(allErrs, ValidateNonEmptySelector(spec.Selector, fldPath.Child("selector"))...)
 	if spec.Replicas == nil {
-		allErrs = append(allErrs, field.Required(fldPath.Child("replicas"), ""))
+		allErrs = append(allErrs, field.Required(fldPath.Child("replicas"), "").MarkCoveredByDeclarative())
 	} else {
-		allErrs = append(allErrs, ValidateNonnegativeField(int64(*spec.Replicas), fldPath.Child("replicas"))...)
+		allErrs = append(allErrs, ValidateNonnegativeField(int64(*spec.Replicas), fldPath.Child("replicas")).MarkCoveredByDeclarative()...)
 	}
 	allErrs = append(allErrs, ValidatePodTemplateSpecForRC(spec.Template, spec.Selector, fldPath.Child("template"), opts)...)
 	return allErrs

pkg/apis/core/validation/validation_test.go

@@ -16755,6 +16755,16 @@ func TestValidateReplicationControllerUpdate(t *testing.T) {
 		update: mkValidReplicationController(func(rc *core.ReplicationController) {
 			rc.Spec.Replicas = ptr.To[int32](3)
 		}),
+	}, {
+		old: mkValidReplicationController(func(rc *core.ReplicationController) {}),
+		update: mkValidReplicationController(func(rc *core.ReplicationController) {
+			rc.Spec.MinReadySeconds = 0
+		}),
+	}, {
+		old: mkValidReplicationController(func(rc *core.ReplicationController) {}),
+		update: mkValidReplicationController(func(rc *core.ReplicationController) {
+			rc.Spec.MinReadySeconds = 3
+		}),
 	}, {
 		old: mkValidReplicationController(func(rc *core.ReplicationController) {}),
 		update: mkValidReplicationController(func(rc *core.ReplicationController) {
@@ -16831,6 +16841,15 @@ func TestValidateReplicationControllerUpdate(t *testing.T) {
 				field.Required(field.NewPath("spec.replicas"), ""),
 			},
 		},
+		"negative minReadySeconds": {
+			old: mkValidReplicationController(func(rc *core.ReplicationController) {}),
+			update: mkValidReplicationController(func(rc *core.ReplicationController) {
+				rc.Spec.MinReadySeconds = -1
+			}),
+			expectedErrs: field.ErrorList{
+				field.Invalid(field.NewPath("spec.minReadySeconds"), nil, "").WithOrigin("minimum"),
+			},
+		},
 	}
 	for k, tc := range errorCases {
 		t.Run(k, func(t *testing.T) {
@@ -16872,6 +16891,9 @@ func TestValidateReplicationController(t *testing.T) {
 		mkValidReplicationController(func(rc *core.ReplicationController) { rc.Spec.Replicas = ptr.To[int32](0) }),
 		mkValidReplicationController(func(rc *core.ReplicationController) { rc.Spec.Replicas = ptr.To[int32](1) }),
 		mkValidReplicationController(func(rc *core.ReplicationController) { rc.Spec.Replicas = ptr.To[int32](100) }),
+		mkValidReplicationController(func(rc *core.ReplicationController) { rc.Spec.MinReadySeconds = 0 }),
+		mkValidReplicationController(func(rc *core.ReplicationController) { rc.Spec.MinReadySeconds = 1 }),
+		mkValidReplicationController(func(rc *core.ReplicationController) { rc.Spec.MinReadySeconds = 100 }),
 	}
 	for _, tc := range successCases {
 		if errs := ValidateReplicationController(&tc, PodValidationOptions{}); len(errs) != 0 {
@@ -16919,6 +16941,12 @@ func TestValidateReplicationController(t *testing.T) {
 				field.Invalid(field.NewPath("spec.replicas"), nil, "").WithOrigin("minimum"),
 			},
 		},
+		"negative minReadySeconds": {
+			input: mkValidReplicationController(func(rc *core.ReplicationController) { rc.Spec.MinReadySeconds = -1 }),
+			expectedErrs: field.ErrorList{
+				field.Invalid(field.NewPath("spec.minReadySeconds"), nil, "").WithOrigin("minimum"),
+			},
+		},
 		"nil replicas": {
 			input: mkValidReplicationController(func(rc *core.ReplicationController) { rc.Spec.Replicas = nil }),
 			expectedErrs: field.ErrorList{

pkg/generated/openapi/zz_generated.openapi.go

@@ -30098,6 +30098,7 @@ func schema_k8sio_api_core_v1_ReplicationControllerSpec(ref common.ReferenceCall
 					"replicas": {
 						SchemaProps: spec.SchemaProps{
 							Description: "Replicas is the number of desired replicas. This is a pointer to distinguish between explicit zero and unspecified. Defaults to 1. More info: https://kubernetes.io/docs/concepts/workloads/controllers/replicationcontroller#what-is-a-replicationcontroller",
+							Default:     1,
 							Type:        []string{"integer"},
 							Format:      "int32",
 						},
@@ -30105,6 +30106,7 @@ func schema_k8sio_api_core_v1_ReplicationControllerSpec(ref common.ReferenceCall
 					"minReadySeconds": {
 						SchemaProps: spec.SchemaProps{
 							Description: "Minimum number of seconds for which a newly created pod should be ready without any of its container crashing, for it to be considered available. Defaults to 0 (pod will be considered available as soon as it is ready)",
+							Default:     0,
 							Type:        []string{"integer"},
 							Format:      "int32",
 						},

pkg/registry/core/replicationcontroller/declarative_validation_test.go

@@ -40,10 +40,50 @@ func TestDeclarativeValidateForDeclarative(t *testing.T) {
 		input        api.ReplicationController
 		expectedErrs field.ErrorList
 	}{
+		// baseline
 		"empty resource": {
 			input: mkValidReplicationController(),
 		},
-		// TODO: Add test cases
+		// spec.replicas
+		"nil replicas": {
+			input: mkValidReplicationController(func(rc *api.ReplicationController) {
+				rc.Spec.Replicas = nil
+			}),
+			expectedErrs: field.ErrorList{
+				field.Required(field.NewPath("spec.replicas"), ""),
+			},
+		},
+		"0 replicas": {
+			input: mkValidReplicationController(setSpecReplicas(0)),
+		},
+		"1 replicas": {
+			input: mkValidReplicationController(setSpecReplicas(1)),
+		},
+		"positive replicas": {
+			input: mkValidReplicationController(setSpecReplicas(100)),
+		},
+		"negative replicas": {
+			input: mkValidReplicationController(setSpecReplicas(-1)),
+			expectedErrs: field.ErrorList{
+				field.Invalid(field.NewPath("spec.replicas"), nil, "").WithOrigin("minimum"),
+			},
+		},
+		// spec.minReadySeconds
+		"0 minReadySeconds": {
+			input: mkValidReplicationController(setSpecMinReadySeconds(0)),
+		},
+		"1 minReadySeconds": {
+			input: mkValidReplicationController(setSpecMinReadySeconds(1)),
+		},
+		"positive minReadySeconds": {
+			input: mkValidReplicationController(setSpecMinReadySeconds(100)),
+		},
+		"negative minReadySeconds": {
+			input: mkValidReplicationController(setSpecMinReadySeconds(-1)),
+			expectedErrs: field.ErrorList{
+				field.Invalid(field.NewPath("spec.minReadySeconds"), nil, "").WithOrigin("minimum"),
+			},
+		},
 	}
 	for k, tc := range testCases {
 		t.Run(k, func(t *testing.T) {
@@ -90,7 +130,60 @@ func TestValidateUpdateForDeclarative(t *testing.T) {
 		update       api.ReplicationController
 		expectedErrs field.ErrorList
 	}{
-		// TODO: Add test cases
+		// baseline
+		"baseline": {
+			old:    mkValidReplicationController(),
+			update: mkValidReplicationController(),
+		},
+		// spec.replicas
+		"nil replicas": {
+			old: mkValidReplicationController(),
+			update: mkValidReplicationController(func(rc *api.ReplicationController) {
+				rc.Spec.Replicas = nil
+			}),
+			expectedErrs: field.ErrorList{
+				field.Required(field.NewPath("spec.replicas"), ""),
+			},
+		},
+		"0 replicas": {
+			old:    mkValidReplicationController(),
+			update: mkValidReplicationController(setSpecReplicas(0)),
+		},
+		"1 replicas": {
+			old:    mkValidReplicationController(),
+			update: mkValidReplicationController(setSpecReplicas(1)),
+		},
+		"positive replicas": {
+			old:    mkValidReplicationController(),
+			update: mkValidReplicationController(setSpecReplicas(100)),
+		},
+		"negative replicas": {
+			old:    mkValidReplicationController(),
+			update: mkValidReplicationController(setSpecReplicas(-1)),
+			expectedErrs: field.ErrorList{
+				field.Invalid(field.NewPath("spec.replicas"), nil, "").WithOrigin("minimum"),
+			},
+		},
+		// spec.minReadySeconds
+		"0 minReadySeconds": {
+			old:    mkValidReplicationController(),
+			update: mkValidReplicationController(setSpecMinReadySeconds(0)),
+		},
+		"1 minReadySeconds": {
+			old:    mkValidReplicationController(),
+			update: mkValidReplicationController(setSpecMinReadySeconds(1)),
+		},
+		"positive minReadySeconds": {
+			old:    mkValidReplicationController(),
+			update: mkValidReplicationController(setSpecMinReadySeconds(3)),
+		},
+		"negative minReadySeconds": {
+			old:    mkValidReplicationController(),
+			update: mkValidReplicationController(setSpecMinReadySeconds(-1)),
+			expectedErrs: field.ErrorList{
+				field.Invalid(field.NewPath("spec.minReadySeconds"), nil, "").WithOrigin("minimum"),
+			},
+		},
 	}
 	for k, tc := range testCases {
 		t.Run(k, func(t *testing.T) {
@@ -164,3 +257,15 @@ func mkValidReplicationController(tweaks ...func(rc *api.ReplicationController))
 	}
 	return rc
 }
+
+func setSpecReplicas(val int32) func(rc *api.ReplicationController) {
+	return func(rc *api.ReplicationController) {
+		rc.Spec.Replicas = ptr.To(val)
+	}
+}
+
+func setSpecMinReadySeconds(val int32) func(rc *api.ReplicationController) {
+	return func(rc *api.ReplicationController) {
+		rc.Spec.MinReadySeconds = val
+	}
+}

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

@@ -5085,12 +5085,18 @@ message ReplicationControllerSpec {
   // Defaults to 1.
   // More info: https://kubernetes.io/docs/concepts/workloads/controllers/replicationcontroller#what-is-a-replicationcontroller
   // +optional
+  // +k8s:optional
+  // +default=1
+  // +k8s:minimum=0
   optional int32 replicas = 1;
 
   // Minimum number of seconds for which a newly created pod should be ready
   // without any of its container crashing, for it to be considered available.
   // Defaults to 0 (pod will be considered available as soon as it is ready)
   // +optional
+  // +k8s:optional
+  // +default=0
+  // +k8s:minimum=0
   optional int32 minReadySeconds = 4;
 
   // Selector is a label query over pods that should match the Replicas count.

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

@@ -5107,12 +5107,18 @@ type ReplicationControllerSpec struct {
 	// Defaults to 1.
 	// More info: https://kubernetes.io/docs/concepts/workloads/controllers/replicationcontroller#what-is-a-replicationcontroller
 	// +optional
+	// +k8s:optional
+	// +default=1
+	// +k8s:minimum=0
 	Replicas *int32 `json:"replicas,omitempty" protobuf:"varint,1,opt,name=replicas"`
 
 	// Minimum number of seconds for which a newly created pod should be ready
 	// without any of its container crashing, for it to be considered available.
 	// Defaults to 0 (pod will be considered available as soon as it is ready)
 	// +optional
+	// +k8s:optional
+	// +default=0
+	// +k8s:minimum=0
 	MinReadySeconds int32 `json:"minReadySeconds,omitempty" protobuf:"varint,4,opt,name=minReadySeconds"`
 
 	// Selector is a label query over pods that should match the Replicas count.

staging/src/k8s.io/client-go/applyconfigurations/internal/internal.go

@@ -7646,9 +7646,11 @@ var schemaYAML = typed.YAMLObject(`types:
     - name: minReadySeconds
       type:
         scalar: numeric
+      default: 0
     - name: replicas
       type:
         scalar: numeric
+      default: 1
     - name: selector
       type:
         map: