Merge pull request #52533 from soltysh/cronjob_beta_defaults

Automatic merge from submit-queue (batch tested with PRs 52500, 52533). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>..

Cronjob beta defaults

**What this PR does / why we need it**:
I forget to set the defaults in beta for (successful|failed)JobsHistoryLimit so doing this now. It's already too late for 1.8 so this goes into 1.9.

**Release note**:
```release-note
Set defaults for successfulJobsHistoryLimit (3) and failedJobsHistoryLimit (1) in batch/v1beta1.CronJobs
```

api/openapi-spec/swagger.json

@@ -63113,7 +63113,7 @@
       "type": "string"
      },
      "failedJobsHistoryLimit": {
-      "description": "The number of failed finished jobs to retain. This is a pointer to distinguish between explicit zero and not specified.",
+      "description": "The number of failed finished jobs to retain. This is a pointer to distinguish between explicit zero and not specified. Defaults to 1.",
       "type": "integer",
       "format": "int32"
      },
@@ -63131,7 +63131,7 @@
       "format": "int64"
      },
      "successfulJobsHistoryLimit": {
-      "description": "The number of successful finished jobs to retain. This is a pointer to distinguish between explicit zero and not specified.",
+      "description": "The number of successful finished jobs to retain. This is a pointer to distinguish between explicit zero and not specified. Defaults to 3.",
       "type": "integer",
       "format": "int32"
      },

api/swagger-spec/batch_v1beta1.json

@@ -1497,12 +1497,12 @@
      "successfulJobsHistoryLimit": {
       "type": "integer",
       "format": "int32",
-      "description": "The number of successful finished jobs to retain. This is a pointer to distinguish between explicit zero and not specified."
+      "description": "The number of successful finished jobs to retain. This is a pointer to distinguish between explicit zero and not specified. Defaults to 3."
      },
      "failedJobsHistoryLimit": {
       "type": "integer",
       "format": "int32",
-      "description": "The number of failed finished jobs to retain. This is a pointer to distinguish between explicit zero and not specified."
+      "description": "The number of failed finished jobs to retain. This is a pointer to distinguish between explicit zero and not specified. Defaults to 1."
      }
     }
    },

docs/api-reference/batch/v1beta1/definitions.html

@@ -3428,14 +3428,14 @@ When an object is created, the system will populate this list with the current s
 </tr>
 <tr>
 <td class="tableblock halign-left valign-top"><p class="tableblock">successfulJobsHistoryLimit</p></td>
-<td class="tableblock halign-left valign-top"><p class="tableblock">The number of successful finished jobs to retain. This is a pointer to distinguish between explicit zero and not specified.</p></td>
+<td class="tableblock halign-left valign-top"><p class="tableblock">The number of successful finished jobs to retain. This is a pointer to distinguish between explicit zero and not specified. Defaults to 3.</p></td>
 <td class="tableblock halign-left valign-top"><p class="tableblock">false</p></td>
 <td class="tableblock halign-left valign-top"><p class="tableblock">integer (int32)</p></td>
 <td class="tableblock halign-left valign-top"></td>
 </tr>
 <tr>
 <td class="tableblock halign-left valign-top"><p class="tableblock">failedJobsHistoryLimit</p></td>
-<td class="tableblock halign-left valign-top"><p class="tableblock">The number of failed finished jobs to retain. This is a pointer to distinguish between explicit zero and not specified.</p></td>
+<td class="tableblock halign-left valign-top"><p class="tableblock">The number of failed finished jobs to retain. This is a pointer to distinguish between explicit zero and not specified. Defaults to 1.</p></td>
 <td class="tableblock halign-left valign-top"><p class="tableblock">false</p></td>
 <td class="tableblock halign-left valign-top"><p class="tableblock">integer (int32)</p></td>
 <td class="tableblock halign-left valign-top"></td>

pkg/apis/batch/fuzzer/fuzzer.go

@@ -53,14 +53,10 @@ var Funcs = func(codecs runtimeserializer.CodecFactory) []interface{} {
 			sds := int64(c.RandUint64())
 			sj.StartingDeadlineSeconds = &sds
 			sj.Schedule = c.RandString()
-			if hasSuccessLimit := c.RandBool(); hasSuccessLimit {
+			successfulJobsHistoryLimit := int32(c.Rand.Int31())
-				successfulJobsHistoryLimit := int32(c.Rand.Int31())
+			sj.SuccessfulJobsHistoryLimit = &successfulJobsHistoryLimit
-				sj.SuccessfulJobsHistoryLimit = &successfulJobsHistoryLimit
+			failedJobsHistoryLimit := int32(c.Rand.Int31())
-			}
+			sj.FailedJobsHistoryLimit = &failedJobsHistoryLimit
-			if hasFailedLimit := c.RandBool(); hasFailedLimit {
-				failedJobsHistoryLimit := int32(c.Rand.Int31())
-				sj.FailedJobsHistoryLimit = &failedJobsHistoryLimit
-			}
 		},
 		func(cp *batch.ConcurrencyPolicy, c fuzz.Continue) {
 			policies := []batch.ConcurrencyPolicy{batch.AllowConcurrent, batch.ForbidConcurrent, batch.ReplaceConcurrent}

pkg/apis/batch/v1beta1/defaults.go

@@ -32,4 +32,12 @@ func SetDefaults_CronJob(obj *batchv1beta1.CronJob) {
 	if obj.Spec.Suspend == nil {
 		obj.Spec.Suspend = new(bool)
 	}
+	if obj.Spec.SuccessfulJobsHistoryLimit == nil {
+		obj.Spec.SuccessfulJobsHistoryLimit = new(int32)
+		*obj.Spec.SuccessfulJobsHistoryLimit = 3
+	}
+	if obj.Spec.FailedJobsHistoryLimit == nil {
+		obj.Spec.FailedJobsHistoryLimit = new(int32)
+		*obj.Spec.FailedJobsHistoryLimit = 1
+	}
 }

pkg/apis/batch/v1beta1/defaults_test.go

@@ -38,22 +38,28 @@ func TestSetDefaultCronJob(t *testing.T) {
 			original: &batchv1beta1.CronJob{},
 			expected: &batchv1beta1.CronJob{
 				Spec: batchv1beta1.CronJobSpec{
-					ConcurrencyPolicy: batchv1beta1.AllowConcurrent,
+					ConcurrencyPolicy:          batchv1beta1.AllowConcurrent,
-					Suspend:           newBool(false),
+					Suspend:                    newBool(false),
+					SuccessfulJobsHistoryLimit: newInt32(3),
+					FailedJobsHistoryLimit:     newInt32(1),
 				},
 			},
 		},
 		"set fields should not be defaulted": {
 			original: &batchv1beta1.CronJob{
 				Spec: batchv1beta1.CronJobSpec{
-					ConcurrencyPolicy: batchv1beta1.ForbidConcurrent,
+					ConcurrencyPolicy:          batchv1beta1.ForbidConcurrent,
-					Suspend:           newBool(true),
+					Suspend:                    newBool(true),
+					SuccessfulJobsHistoryLimit: newInt32(5),
+					FailedJobsHistoryLimit:     newInt32(5),
 				},
 			},
 			expected: &batchv1beta1.CronJob{
 				Spec: batchv1beta1.CronJobSpec{
-					ConcurrencyPolicy: batchv1beta1.ForbidConcurrent,
+					ConcurrencyPolicy:          batchv1beta1.ForbidConcurrent,
-					Suspend:           newBool(true),
+					Suspend:                    newBool(true),
+					SuccessfulJobsHistoryLimit: newInt32(5),
+					FailedJobsHistoryLimit:     newInt32(5),
 				},
 			},
 		},
@@ -74,6 +80,12 @@ func TestSetDefaultCronJob(t *testing.T) {
 		if *actual.Spec.Suspend != *expected.Spec.Suspend {
 			t.Errorf("%s: got different suspend than expected: %v %v", name, *actual.Spec.Suspend, *expected.Spec.Suspend)
 		}
+		if *actual.Spec.SuccessfulJobsHistoryLimit != *expected.Spec.SuccessfulJobsHistoryLimit {
+			t.Errorf("%s: got different successfulJobsHistoryLimit than expected: %v %v", name, *actual.Spec.SuccessfulJobsHistoryLimit, *expected.Spec.SuccessfulJobsHistoryLimit)
+		}
+		if *actual.Spec.FailedJobsHistoryLimit != *expected.Spec.FailedJobsHistoryLimit {
+			t.Errorf("%s: got different failedJobsHistoryLimit than expected: %v %v", name, *actual.Spec.FailedJobsHistoryLimit, *expected.Spec.FailedJobsHistoryLimit)
+		}
 	}
 }
 
@@ -102,3 +114,9 @@ func newBool(val bool) *bool {
 	*p = val
 	return p
 }
+
+func newInt32(val int32) *int32 {
+	p := new(int32)
+	*p = val
+	return p
+}

staging/src/k8s.io/api/batch/v1beta1/generated.proto

@@ -85,11 +85,13 @@ message CronJobSpec {
 
   // The number of successful finished jobs to retain.
   // This is a pointer to distinguish between explicit zero and not specified.
+  // Defaults to 3.
   // +optional
   optional int32 successfulJobsHistoryLimit = 6;
 
   // The number of failed finished jobs to retain.
   // This is a pointer to distinguish between explicit zero and not specified.
+  // Defaults to 1.
   // +optional
   optional int32 failedJobsHistoryLimit = 7;
 }

staging/src/k8s.io/api/batch/v1beta1/types.go

@@ -114,11 +114,13 @@ type CronJobSpec struct {
 
 	// The number of successful finished jobs to retain.
 	// This is a pointer to distinguish between explicit zero and not specified.
+	// Defaults to 3.
 	// +optional
 	SuccessfulJobsHistoryLimit *int32 `json:"successfulJobsHistoryLimit,omitempty" protobuf:"varint,6,opt,name=successfulJobsHistoryLimit"`
 
 	// The number of failed finished jobs to retain.
 	// This is a pointer to distinguish between explicit zero and not specified.
+	// Defaults to 1.
 	// +optional
 	FailedJobsHistoryLimit *int32 `json:"failedJobsHistoryLimit,omitempty" protobuf:"varint,7,opt,name=failedJobsHistoryLimit"`
 }

staging/src/k8s.io/api/batch/v1beta1/types_swagger_doc_generated.go

@@ -55,8 +55,8 @@ var map_CronJobSpec = map[string]string{
 	"concurrencyPolicy":          "Specifies how to treat concurrent executions of a Job. Defaults to Allow.",
 	"suspend":                    "This flag tells the controller to suspend subsequent executions, it does not apply to already started executions.  Defaults to false.",
 	"jobTemplate":                "Specifies the job that will be created when executing a CronJob.",
-	"successfulJobsHistoryLimit": "The number of successful finished jobs to retain. This is a pointer to distinguish between explicit zero and not specified.",
+	"successfulJobsHistoryLimit": "The number of successful finished jobs to retain. This is a pointer to distinguish between explicit zero and not specified. Defaults to 3.",
-	"failedJobsHistoryLimit":     "The number of failed finished jobs to retain. This is a pointer to distinguish between explicit zero and not specified.",
+	"failedJobsHistoryLimit":     "The number of failed finished jobs to retain. This is a pointer to distinguish between explicit zero and not specified. Defaults to 1.",
 }
 
 func (CronJobSpec) SwaggerDoc() map[string]string {