diff --git a/pkg/apis/batch/types.go b/pkg/apis/batch/types.go index 756d4c9d6b..6507c56a67 100644 --- a/pkg/apis/batch/types.go +++ b/pkg/apis/batch/types.go @@ -209,7 +209,7 @@ type ScheduledJobSpec struct { // Suspend flag tells the controller to suspend subsequent executions, it does // not apply to already started executions. Defaults to false. - Suspend bool `json:"suspend"` + Suspend *bool `json:"suspend"` // JobTemplate is the object that describes the job that will be created when // executing a ScheduledJob. diff --git a/pkg/apis/batch/v2alpha1/types.go b/pkg/apis/batch/v2alpha1/types.go index f33dfc1ed6..4575bc804b 100644 --- a/pkg/apis/batch/v2alpha1/types.go +++ b/pkg/apis/batch/v2alpha1/types.go @@ -211,7 +211,7 @@ type ScheduledJobSpec struct { // Suspend flag tells the controller to suspend subsequent executions, it does // not apply to already started executions. Defaults to false. - Suspend bool `json:"suspend" protobuf:"varint,4,opt,name=suspend"` + Suspend *bool `json:"suspend" protobuf:"varint,4,opt,name=suspend"` // JobTemplate is the object that describes the job that will be created when // executing a ScheduledJob. diff --git a/pkg/apis/batch/validation/validation.go b/pkg/apis/batch/validation/validation.go index ca1ea5b9e9..e592ca18dc 100644 --- a/pkg/apis/batch/validation/validation.go +++ b/pkg/apis/batch/validation/validation.go @@ -89,17 +89,8 @@ func ValidateJob(job *batch.Job) field.ErrorList { } func ValidateJobSpec(spec *batch.JobSpec, fldPath *field.Path) field.ErrorList { - allErrs := field.ErrorList{} + allErrs := validateJobSpec(spec, fldPath) - if spec.Parallelism != nil { - allErrs = append(allErrs, apivalidation.ValidateNonnegativeField(int64(*spec.Parallelism), fldPath.Child("parallelism"))...) - } - if spec.Completions != nil { - allErrs = append(allErrs, apivalidation.ValidateNonnegativeField(int64(*spec.Completions), fldPath.Child("completions"))...) - } - if spec.ActiveDeadlineSeconds != nil { - allErrs = append(allErrs, apivalidation.ValidateNonnegativeField(int64(*spec.ActiveDeadlineSeconds), fldPath.Child("activeDeadlineSeconds"))...) - } if spec.Selector == nil { allErrs = append(allErrs, field.Required(fldPath.Child("selector"), "")) } else { @@ -113,6 +104,21 @@ func ValidateJobSpec(spec *batch.JobSpec, fldPath *field.Path) field.ErrorList { allErrs = append(allErrs, field.Invalid(fldPath.Child("template", "metadata", "labels"), spec.Template.Labels, "`selector` does not match template `labels`")) } } + return allErrs +} + +func validateJobSpec(spec *batch.JobSpec, fldPath *field.Path) field.ErrorList { + allErrs := field.ErrorList{} + + if spec.Parallelism != nil { + allErrs = append(allErrs, apivalidation.ValidateNonnegativeField(int64(*spec.Parallelism), fldPath.Child("parallelism"))...) + } + if spec.Completions != nil { + allErrs = append(allErrs, apivalidation.ValidateNonnegativeField(int64(*spec.Completions), fldPath.Child("completions"))...) + } + if spec.ActiveDeadlineSeconds != nil { + allErrs = append(allErrs, apivalidation.ValidateNonnegativeField(int64(*spec.ActiveDeadlineSeconds), fldPath.Child("activeDeadlineSeconds"))...) + } allErrs = append(allErrs, apivalidation.ValidatePodTemplateSpec(&spec.Template, fldPath.Child("template"))...) if spec.Template.Spec.RestartPolicy != api.RestartPolicyOnFailure && @@ -215,7 +221,14 @@ func ValidateJobTemplate(job *batch.JobTemplate) field.ErrorList { } func ValidateJobTemplateSpec(spec *batch.JobTemplateSpec, fldPath *field.Path) field.ErrorList { - // this method should be identical to ValidateJob - allErrs := ValidateJobSpec(&spec.Spec, fldPath.Child("spec")) + allErrs := validateJobSpec(&spec.Spec, fldPath.Child("spec")) + + // jobtemplate will always have the selector automatically generated + if spec.Spec.Selector != nil { + allErrs = append(allErrs, field.Invalid(fldPath.Child("spec", "selector"), spec.Spec.Selector, "`selector` will be auto-generated")) + } + if spec.Spec.ManualSelector != nil && *spec.Spec.ManualSelector { + allErrs = append(allErrs, field.NotSupported(fldPath.Child("spec", "manualSelector"), spec.Spec.ManualSelector, []string{"nil", "false"})) + } return allErrs } diff --git a/pkg/apis/batch/validation/validation_test.go b/pkg/apis/batch/validation/validation_test.go index 2f5fb534db..fa91ae1ec5 100644 --- a/pkg/apis/batch/validation/validation_test.go +++ b/pkg/apis/batch/validation/validation_test.go @@ -306,8 +306,8 @@ func TestValidateJobUpdateStatus(t *testing.T) { func TestValidateScheduledJob(t *testing.T) { validManualSelector := getValidManualSelector() - validGeneratedSelector := getValidGeneratedSelector() - validPodTemplateSpec := getValidPodTemplateSpecForGenerated(validGeneratedSelector) + validPodTemplateSpec := getValidPodTemplateSpecForGenerated(getValidGeneratedSelector()) + validPodTemplateSpec.Labels = map[string]string{} successCases := map[string]batch.ScheduledJob{ "basic scheduled job": { @@ -321,7 +321,6 @@ func TestValidateScheduledJob(t *testing.T) { ConcurrencyPolicy: batch.AllowConcurrent, JobTemplate: batch.JobTemplateSpec{ Spec: batch.JobSpec{ - Selector: validGeneratedSelector, Template: validPodTemplateSpec, }, }, @@ -349,7 +348,6 @@ func TestValidateScheduledJob(t *testing.T) { ConcurrencyPolicy: batch.AllowConcurrent, JobTemplate: batch.JobTemplateSpec{ Spec: batch.JobSpec{ - Selector: validGeneratedSelector, Template: validPodTemplateSpec, }, }, @@ -366,7 +364,6 @@ func TestValidateScheduledJob(t *testing.T) { ConcurrencyPolicy: batch.AllowConcurrent, JobTemplate: batch.JobTemplateSpec{ Spec: batch.JobSpec{ - Selector: validGeneratedSelector, Template: validPodTemplateSpec, }, }, @@ -384,7 +381,6 @@ func TestValidateScheduledJob(t *testing.T) { StartingDeadlineSeconds: &negative64, JobTemplate: batch.JobTemplateSpec{ Spec: batch.JobSpec{ - Selector: validGeneratedSelector, Template: validPodTemplateSpec, }, }, @@ -400,7 +396,6 @@ func TestValidateScheduledJob(t *testing.T) { Schedule: "* * * * * ?", JobTemplate: batch.JobTemplateSpec{ Spec: batch.JobSpec{ - Selector: validGeneratedSelector, Template: validPodTemplateSpec, }, }, @@ -417,7 +412,6 @@ func TestValidateScheduledJob(t *testing.T) { ConcurrencyPolicy: batch.AllowConcurrent, JobTemplate: batch.JobTemplateSpec{ Spec: batch.JobSpec{ - Selector: validGeneratedSelector, Parallelism: &negative, Template: validPodTemplateSpec, }, @@ -437,7 +431,6 @@ func TestValidateScheduledJob(t *testing.T) { Spec: batch.JobSpec{ Completions: &negative, - Selector: validGeneratedSelector, Template: validPodTemplateSpec, }, }, @@ -455,13 +448,12 @@ func TestValidateScheduledJob(t *testing.T) { JobTemplate: batch.JobTemplateSpec{ Spec: batch.JobSpec{ ActiveDeadlineSeconds: &negative64, - Selector: validGeneratedSelector, Template: validPodTemplateSpec, }, }, }, }, - "spec.jobTemplate.spec.selector:Required value": { + "spec.jobTemplate.spec.selector: Invalid value: {\"matchLabels\":{\"a\":\"b\"}}: `selector` will be auto-generated": { ObjectMeta: api.ObjectMeta{ Name: "myscheduledjob", Namespace: api.NamespaceDefault, @@ -472,12 +464,13 @@ func TestValidateScheduledJob(t *testing.T) { ConcurrencyPolicy: batch.AllowConcurrent, JobTemplate: batch.JobTemplateSpec{ Spec: batch.JobSpec{ + Selector: validManualSelector, Template: validPodTemplateSpec, }, }, }, }, - "spec.jobTemplate.spec.template.metadata.labels: Invalid value: {\"y\":\"z\"}: `selector` does not match template `labels`": { + "spec.jobTemplate.spec.manualSelector: Unsupported value": { ObjectMeta: api.ObjectMeta{ Name: "myscheduledjob", Namespace: api.NamespaceDefault, @@ -488,45 +481,8 @@ func TestValidateScheduledJob(t *testing.T) { ConcurrencyPolicy: batch.AllowConcurrent, JobTemplate: batch.JobTemplateSpec{ Spec: batch.JobSpec{ - Selector: validManualSelector, ManualSelector: newBool(true), - Template: api.PodTemplateSpec{ - ObjectMeta: api.ObjectMeta{ - Labels: map[string]string{"y": "z"}, - }, - Spec: api.PodSpec{ - RestartPolicy: api.RestartPolicyOnFailure, - DNSPolicy: api.DNSClusterFirst, - Containers: []api.Container{{Name: "abc", Image: "image", ImagePullPolicy: "IfNotPresent"}}, - }, - }, - }, - }, - }, - }, - "spec.jobTemplate.spec.template.metadata.labels: Invalid value: {\"controller-uid\":\"4d5e6f\"}: `selector` does not match template `labels`": { - ObjectMeta: api.ObjectMeta{ - Name: "myscheduledjob", - Namespace: api.NamespaceDefault, - UID: types.UID("1a2b3c"), - }, - Spec: batch.ScheduledJobSpec{ - Schedule: "* * * * * ?", - ConcurrencyPolicy: batch.AllowConcurrent, - JobTemplate: batch.JobTemplateSpec{ - Spec: batch.JobSpec{ - Selector: validManualSelector, - ManualSelector: newBool(true), - Template: api.PodTemplateSpec{ - ObjectMeta: api.ObjectMeta{ - Labels: map[string]string{"controller-uid": "4d5e6f"}, - }, - Spec: api.PodSpec{ - RestartPolicy: api.RestartPolicyOnFailure, - DNSPolicy: api.DNSClusterFirst, - Containers: []api.Container{{Name: "abc", Image: "image", ImagePullPolicy: "IfNotPresent"}}, - }, - }, + Template: validPodTemplateSpec, }, }, }, @@ -542,12 +498,7 @@ func TestValidateScheduledJob(t *testing.T) { ConcurrencyPolicy: batch.AllowConcurrent, JobTemplate: batch.JobTemplateSpec{ Spec: batch.JobSpec{ - Selector: validManualSelector, - ManualSelector: newBool(true), Template: api.PodTemplateSpec{ - ObjectMeta: api.ObjectMeta{ - Labels: validManualSelector.MatchLabels, - }, Spec: api.PodSpec{ RestartPolicy: api.RestartPolicyAlways, DNSPolicy: api.DNSClusterFirst, diff --git a/pkg/registry/scheduledjob/strategy_test.go b/pkg/registry/scheduledjob/strategy_test.go index b4a6680490..3ace2dd2fc 100644 --- a/pkg/registry/scheduledjob/strategy_test.go +++ b/pkg/registry/scheduledjob/strategy_test.go @@ -41,13 +41,7 @@ func TestScheduledJobStrategy(t *testing.T) { t.Errorf("ScheduledJob should not allow create on update") } - validSelector := &unversioned.LabelSelector{ - MatchLabels: map[string]string{"a": "b"}, - } validPodTemplateSpec := api.PodTemplateSpec{ - ObjectMeta: api.ObjectMeta{ - Labels: validSelector.MatchLabels, - }, Spec: api.PodSpec{ RestartPolicy: api.RestartPolicyOnFailure, DNSPolicy: api.DNSClusterFirst, @@ -64,9 +58,7 @@ func TestScheduledJobStrategy(t *testing.T) { ConcurrencyPolicy: batch.AllowConcurrent, JobTemplate: batch.JobTemplateSpec{ Spec: batch.JobSpec{ - Selector: validSelector, - Template: validPodTemplateSpec, - ManualSelector: newBool(true), + Template: validPodTemplateSpec, }, }, }, @@ -110,13 +102,7 @@ func TestScheduledJobStatusStrategy(t *testing.T) { if StatusStrategy.AllowCreateOnUpdate() { t.Errorf("ScheduledJob should not allow create on update") } - validSelector := &unversioned.LabelSelector{ - MatchLabels: map[string]string{"a": "b"}, - } validPodTemplateSpec := api.PodTemplateSpec{ - ObjectMeta: api.ObjectMeta{ - Labels: validSelector.MatchLabels, - }, Spec: api.PodSpec{ RestartPolicy: api.RestartPolicyOnFailure, DNSPolicy: api.DNSClusterFirst, @@ -135,9 +121,7 @@ func TestScheduledJobStatusStrategy(t *testing.T) { ConcurrencyPolicy: batch.AllowConcurrent, JobTemplate: batch.JobTemplateSpec{ Spec: batch.JobSpec{ - Selector: validSelector, - Template: validPodTemplateSpec, - ManualSelector: newBool(true), + Template: validPodTemplateSpec, }, }, }, @@ -154,9 +138,7 @@ func TestScheduledJobStatusStrategy(t *testing.T) { ConcurrencyPolicy: batch.AllowConcurrent, JobTemplate: batch.JobTemplateSpec{ Spec: batch.JobSpec{ - Selector: validSelector, - Template: validPodTemplateSpec, - ManualSelector: newBool(true), + Template: validPodTemplateSpec, }, }, },