diff --git a/pkg/apis/batch/fuzzer/fuzzer.go b/pkg/apis/batch/fuzzer/fuzzer.go index cb4b751f6d1..07a8cc1ed59 100644 --- a/pkg/apis/batch/fuzzer/fuzzer.go +++ b/pkg/apis/batch/fuzzer/fuzzer.go @@ -36,8 +36,10 @@ var Funcs = func(codecs runtimeserializer.CodecFactory) []interface{} { c.FuzzNoCustom(j) // fuzz self without calling this function again completions := int32(c.Rand.Int31()) parallelism := int32(c.Rand.Int31()) + backoffLimit := int32(c.Rand.Int31()) j.Completions = &completions j.Parallelism = ¶llelism + j.BackoffLimit = &backoffLimit if c.Rand.Int31()%2 == 0 { j.ManualSelector = newBool(true) } else { diff --git a/pkg/apis/batch/types.go b/pkg/apis/batch/types.go index 7b450d4ffd1..be2b692f296 100644 --- a/pkg/apis/batch/types.go +++ b/pkg/apis/batch/types.go @@ -109,6 +109,16 @@ type JobSpec struct { // +optional ActiveDeadlineSeconds *int64 + // Optional number of retries before marking this job failed. + // Defaults to 6 + // +optional + BackoffLimit *int32 + + // TODO enabled it when https://github.com/kubernetes/kubernetes/issues/28486 has been fixed + // Optional number of failed pods to retain. + // +optional + // FailedPodsLimit *int32 + // A label query over pods that should match the pod count. // Normally, the system sets this field for you. // +optional diff --git a/pkg/apis/batch/v1/conversion.go b/pkg/apis/batch/v1/conversion.go index 55215676cc1..941b61d4a70 100644 --- a/pkg/apis/batch/v1/conversion.go +++ b/pkg/apis/batch/v1/conversion.go @@ -53,6 +53,7 @@ func Convert_batch_JobSpec_To_v1_JobSpec(in *batch.JobSpec, out *batchv1.JobSpec out.Parallelism = in.Parallelism out.Completions = in.Completions out.ActiveDeadlineSeconds = in.ActiveDeadlineSeconds + out.BackoffLimit = in.BackoffLimit out.Selector = in.Selector if in.ManualSelector != nil { out.ManualSelector = new(bool) @@ -71,6 +72,7 @@ func Convert_v1_JobSpec_To_batch_JobSpec(in *batchv1.JobSpec, out *batch.JobSpec out.Parallelism = in.Parallelism out.Completions = in.Completions out.ActiveDeadlineSeconds = in.ActiveDeadlineSeconds + out.BackoffLimit = in.BackoffLimit out.Selector = in.Selector if in.ManualSelector != nil { out.ManualSelector = new(bool) diff --git a/pkg/apis/batch/v1/defaults.go b/pkg/apis/batch/v1/defaults.go index 0d2af9e6aca..72e7e2a9d5b 100644 --- a/pkg/apis/batch/v1/defaults.go +++ b/pkg/apis/batch/v1/defaults.go @@ -38,6 +38,10 @@ func SetDefaults_Job(obj *batchv1.Job) { obj.Spec.Parallelism = new(int32) *obj.Spec.Parallelism = 1 } + if obj.Spec.BackoffLimit == nil { + obj.Spec.BackoffLimit = new(int32) + *obj.Spec.BackoffLimit = 6 + } labels := obj.Spec.Template.Labels if labels != nil && len(obj.Labels) == 0 { obj.Labels = labels diff --git a/pkg/apis/batch/v1/defaults_test.go b/pkg/apis/batch/v1/defaults_test.go index d1d48b1666f..f19ac542a5c 100644 --- a/pkg/apis/batch/v1/defaults_test.go +++ b/pkg/apis/batch/v1/defaults_test.go @@ -38,7 +38,7 @@ func TestSetDefaultJob(t *testing.T) { expected *batchv1.Job expectLabels bool }{ - "both unspecified -> sets both to 1": { + "All unspecified -> sets all to default values": { original: &batchv1.Job{ Spec: batchv1.JobSpec{ Template: v1.PodTemplateSpec{ @@ -48,13 +48,14 @@ func TestSetDefaultJob(t *testing.T) { }, expected: &batchv1.Job{ Spec: batchv1.JobSpec{ - Completions: newInt32(1), - Parallelism: newInt32(1), + Completions: newInt32(1), + Parallelism: newInt32(1), + BackoffLimit: newInt32(6), }, }, expectLabels: true, }, - "both unspecified -> sets both to 1 and no default labels": { + "All unspecified -> all integers are defaulted and no default labels": { original: &batchv1.Job{ ObjectMeta: metav1.ObjectMeta{ Labels: map[string]string{"mylabel": "myvalue"}, @@ -67,12 +68,13 @@ func TestSetDefaultJob(t *testing.T) { }, expected: &batchv1.Job{ Spec: batchv1.JobSpec{ - Completions: newInt32(1), - Parallelism: newInt32(1), + Completions: newInt32(1), + Parallelism: newInt32(1), + BackoffLimit: newInt32(6), }, }, }, - "WQ: Parallelism explicitly 0 and completions unset -> no change": { + "WQ: Parallelism explicitly 0 and completions unset -> BackoffLimit is defaulted": { original: &batchv1.Job{ Spec: batchv1.JobSpec{ Parallelism: newInt32(0), @@ -83,12 +85,13 @@ func TestSetDefaultJob(t *testing.T) { }, expected: &batchv1.Job{ Spec: batchv1.JobSpec{ - Parallelism: newInt32(0), + Parallelism: newInt32(0), + BackoffLimit: newInt32(6), }, }, expectLabels: true, }, - "WQ: Parallelism explicitly 2 and completions unset -> no change": { + "WQ: Parallelism explicitly 2 and completions unset -> BackoffLimit is defaulted": { original: &batchv1.Job{ Spec: batchv1.JobSpec{ Parallelism: newInt32(2), @@ -99,12 +102,13 @@ func TestSetDefaultJob(t *testing.T) { }, expected: &batchv1.Job{ Spec: batchv1.JobSpec{ - Parallelism: newInt32(2), + Parallelism: newInt32(2), + BackoffLimit: newInt32(6), }, }, expectLabels: true, }, - "Completions explicitly 2 and parallelism unset -> parallelism is defaulted": { + "Completions explicitly 2 and others unset -> parallelism and BackoffLimit are defaulted": { original: &batchv1.Job{ Spec: batchv1.JobSpec{ Completions: newInt32(2), @@ -115,17 +119,17 @@ func TestSetDefaultJob(t *testing.T) { }, expected: &batchv1.Job{ Spec: batchv1.JobSpec{ - Completions: newInt32(2), - Parallelism: newInt32(1), + Completions: newInt32(2), + Parallelism: newInt32(1), + BackoffLimit: newInt32(6), }, }, expectLabels: true, }, - "Both set -> no change": { + "BackoffLimit explicitly 5 and others unset -> parallelism and completions are defaulted": { original: &batchv1.Job{ Spec: batchv1.JobSpec{ - Completions: newInt32(10), - Parallelism: newInt32(11), + BackoffLimit: newInt32(5), Template: v1.PodTemplateSpec{ ObjectMeta: metav1.ObjectMeta{Labels: defaultLabels}, }, @@ -133,20 +137,19 @@ func TestSetDefaultJob(t *testing.T) { }, expected: &batchv1.Job{ Spec: batchv1.JobSpec{ - Completions: newInt32(10), - Parallelism: newInt32(11), - Template: v1.PodTemplateSpec{ - ObjectMeta: metav1.ObjectMeta{Labels: defaultLabels}, - }, + Completions: newInt32(1), + Parallelism: newInt32(1), + BackoffLimit: newInt32(5), }, }, expectLabels: true, }, - "Both set, flipped -> no change": { + "All set -> no change": { original: &batchv1.Job{ Spec: batchv1.JobSpec{ - Completions: newInt32(11), - Parallelism: newInt32(10), + Completions: newInt32(8), + Parallelism: newInt32(9), + BackoffLimit: newInt32(10), Template: v1.PodTemplateSpec{ ObjectMeta: metav1.ObjectMeta{Labels: defaultLabels}, }, @@ -154,8 +157,32 @@ func TestSetDefaultJob(t *testing.T) { }, expected: &batchv1.Job{ Spec: batchv1.JobSpec{ - Completions: newInt32(11), - Parallelism: newInt32(10), + Completions: newInt32(8), + Parallelism: newInt32(9), + BackoffLimit: newInt32(10), + Template: v1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{Labels: defaultLabels}, + }, + }, + }, + expectLabels: true, + }, + "All set, flipped -> no change": { + original: &batchv1.Job{ + Spec: batchv1.JobSpec{ + Completions: newInt32(11), + Parallelism: newInt32(10), + BackoffLimit: newInt32(9), + Template: v1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{Labels: defaultLabels}, + }, + }, + }, + expected: &batchv1.Job{ + Spec: batchv1.JobSpec{ + Completions: newInt32(11), + Parallelism: newInt32(10), + BackoffLimit: newInt32(9), }, }, expectLabels: true, @@ -171,22 +198,11 @@ func TestSetDefaultJob(t *testing.T) { t.Errorf("%s: unexpected object: %v", name, actual) t.FailNow() } - if (actual.Spec.Completions == nil) != (expected.Spec.Completions == nil) { - t.Errorf("%s: got different *completions than expected: %v %v", name, actual.Spec.Completions, expected.Spec.Completions) - } - if actual.Spec.Completions != nil && expected.Spec.Completions != nil { - if *actual.Spec.Completions != *expected.Spec.Completions { - t.Errorf("%s: got different completions than expected: %d %d", name, *actual.Spec.Completions, *expected.Spec.Completions) - } - } - if (actual.Spec.Parallelism == nil) != (expected.Spec.Parallelism == nil) { - t.Errorf("%s: got different *Parallelism than expected: %v %v", name, actual.Spec.Parallelism, expected.Spec.Parallelism) - } - if actual.Spec.Parallelism != nil && expected.Spec.Parallelism != nil { - if *actual.Spec.Parallelism != *expected.Spec.Parallelism { - t.Errorf("%s: got different parallelism than expected: %d %d", name, *actual.Spec.Parallelism, *expected.Spec.Parallelism) - } - } + + validateDefaultInt32(t, name, "Completions", actual.Spec.Completions, expected.Spec.Completions) + validateDefaultInt32(t, name, "Parallelism", actual.Spec.Parallelism, expected.Spec.Parallelism) + validateDefaultInt32(t, name, "BackoffLimit", actual.Spec.BackoffLimit, expected.Spec.BackoffLimit) + if test.expectLabels != reflect.DeepEqual(actual.Labels, actual.Spec.Template.Labels) { if test.expectLabels { t.Errorf("%s: expected: %v, got: %v", name, actual.Spec.Template.Labels, actual.Labels) @@ -198,6 +214,17 @@ func TestSetDefaultJob(t *testing.T) { } } +func validateDefaultInt32(t *testing.T, name string, field string, actual *int32, expected *int32) { + if (actual == nil) != (expected == nil) { + t.Errorf("%s: got different *%s than expected: %v %v", name, field, actual, expected) + } + if actual != nil && expected != nil { + if *actual != *expected { + t.Errorf("%s: got different %s than expected: %d %d", name, field, *actual, *expected) + } + } +} + func roundTrip(t *testing.T, obj runtime.Object) runtime.Object { data, err := runtime.Encode(api.Codecs.LegacyCodec(SchemeGroupVersion), obj) if err != nil { diff --git a/pkg/apis/batch/validation/validation.go b/pkg/apis/batch/validation/validation.go index 77ab204e4a7..c736c10fbd4 100644 --- a/pkg/apis/batch/validation/validation.go +++ b/pkg/apis/batch/validation/validation.go @@ -113,6 +113,9 @@ func validateJobSpec(spec *batch.JobSpec, fldPath *field.Path) field.ErrorList { if spec.ActiveDeadlineSeconds != nil { allErrs = append(allErrs, apivalidation.ValidateNonnegativeField(int64(*spec.ActiveDeadlineSeconds), fldPath.Child("activeDeadlineSeconds"))...) } + if spec.BackoffLimit != nil { + allErrs = append(allErrs, apivalidation.ValidateNonnegativeField(int64(*spec.BackoffLimit), fldPath.Child("backoffLimit"))...) + } allErrs = append(allErrs, apivalidation.ValidatePodTemplateSpec(&spec.Template, fldPath.Child("template"))...) if spec.Template.Spec.RestartPolicy != api.RestartPolicyOnFailure && diff --git a/staging/src/k8s.io/api/batch/v1/types.go b/staging/src/k8s.io/api/batch/v1/types.go index 7124e0719f0..4f3b83e8a8d 100644 --- a/staging/src/k8s.io/api/batch/v1/types.go +++ b/staging/src/k8s.io/api/batch/v1/types.go @@ -77,11 +77,21 @@ type JobSpec struct { // +optional Completions *int32 `json:"completions,omitempty" protobuf:"varint,2,opt,name=completions"` - // Optional duration in seconds relative to the startTime that the job may be active + // Specifies the duration in seconds relative to the startTime that the job may be active // before the system tries to terminate it; value must be positive integer // +optional ActiveDeadlineSeconds *int64 `json:"activeDeadlineSeconds,omitempty" protobuf:"varint,3,opt,name=activeDeadlineSeconds"` + // Specifies the number of retries before marking this job failed. + // Defaults to 6 + // +optional + BackoffLimit *int32 `json:"backoffLimit,omitempty" protobuf:"varint,7,opt,name=backoffLimit"` + + // TODO enabled it when https://github.com/kubernetes/kubernetes/issues/28486 has been fixed + // Optional number of failed pods to retain. + // +optional + // FailedPodsLimit *int32 `json:"failedPodsLimit,omitempty" protobuf:"varint,9,opt,name=failedPodsLimit"` + // A label query over pods that should match the pod count. // Normally, the system sets this field for you. // More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/#label-selectors diff --git a/test/e2e_federation/job.go b/test/e2e_federation/job.go index 71ad152ba4b..d84a8bb2821 100644 --- a/test/e2e_federation/job.go +++ b/test/e2e_federation/job.go @@ -225,6 +225,7 @@ func verifyJob(fedJob, localJob *batchv1.Job) bool { localJob.Spec.ManualSelector = fedJob.Spec.ManualSelector localJob.Spec.Completions = fedJob.Spec.Completions localJob.Spec.Parallelism = fedJob.Spec.Parallelism + localJob.Spec.BackoffLimit = fedJob.Spec.BackoffLimit return fedutil.ObjectMetaAndSpecEquivalent(fedJob, localJob) }