diff --git a/pkg/apis/batch/v2alpha1/defaults.go b/pkg/apis/batch/v2alpha1/defaults.go index d1b90e33d06..72da797c77f 100644 --- a/pkg/apis/batch/v2alpha1/defaults.go +++ b/pkg/apis/batch/v2alpha1/defaults.go @@ -23,6 +23,7 @@ import ( func addDefaultingFuncs(scheme *runtime.Scheme) { scheme.AddDefaultingFuncs( SetDefaults_Job, + SetDefaults_ScheduledJob, ) } @@ -40,3 +41,9 @@ func SetDefaults_Job(obj *Job) { *obj.Spec.Parallelism = 1 } } + +func SetDefaults_ScheduledJob(obj *ScheduledJob) { + if obj.Spec.ConcurrencyPolicy == "" { + obj.Spec.ConcurrencyPolicy = AllowConcurrent + } +} diff --git a/pkg/apis/batch/validation/validation.go b/pkg/apis/batch/validation/validation.go index d4d18757834..ca1ea5b9e95 100644 --- a/pkg/apis/batch/validation/validation.go +++ b/pkg/apis/batch/validation/validation.go @@ -17,6 +17,8 @@ limitations under the License. package validation import ( + "github.com/robfig/cron" + "k8s.io/kubernetes/pkg/api" "k8s.io/kubernetes/pkg/api/unversioned" unversionedvalidation "k8s.io/kubernetes/pkg/api/unversioned/validation" @@ -155,3 +157,65 @@ func ValidateJobStatusUpdate(status, oldStatus batch.JobStatus) field.ErrorList allErrs = append(allErrs, ValidateJobStatus(&status, field.NewPath("status"))...) return allErrs } + +func ValidateScheduledJob(scheduledJob *batch.ScheduledJob) field.ErrorList { + // ScheduledJobs and rcs have the same name validation + allErrs := apivalidation.ValidateObjectMeta(&scheduledJob.ObjectMeta, true, apivalidation.ValidateReplicationControllerName, field.NewPath("metadata")) + allErrs = append(allErrs, ValidateScheduledJobSpec(&scheduledJob.Spec, field.NewPath("spec"))...) + return allErrs +} + +func ValidateScheduledJobSpec(spec *batch.ScheduledJobSpec, fldPath *field.Path) field.ErrorList { + allErrs := field.ErrorList{} + + if len(spec.Schedule) == 0 { + allErrs = append(allErrs, field.Required(fldPath.Child("schedule"), "")) + } else { + allErrs = append(allErrs, validateScheduleFormat(spec.Schedule, fldPath.Child("schedule"))...) + } + if spec.StartingDeadlineSeconds != nil { + allErrs = append(allErrs, apivalidation.ValidateNonnegativeField(int64(*spec.StartingDeadlineSeconds), fldPath.Child("startingDeadlineSeconds"))...) + } + allErrs = append(allErrs, validateConcurrencyPolicy(&spec.ConcurrencyPolicy, fldPath.Child("concurrencyPolicy"))...) + allErrs = append(allErrs, ValidateJobTemplateSpec(&spec.JobTemplate, fldPath.Child("jobTemplate"))...) + + return allErrs +} + +func validateConcurrencyPolicy(concurrencyPolicy *batch.ConcurrencyPolicy, fldPath *field.Path) field.ErrorList { + allErrs := field.ErrorList{} + switch *concurrencyPolicy { + case batch.AllowConcurrent, batch.ForbidConcurrent, batch.ReplaceConcurrent: + break + case "": + allErrs = append(allErrs, field.Required(fldPath, "")) + default: + validValues := []string{string(batch.AllowConcurrent), string(batch.ForbidConcurrent), string(batch.ReplaceConcurrent)} + allErrs = append(allErrs, field.NotSupported(fldPath, *concurrencyPolicy, validValues)) + } + + return allErrs +} + +func validateScheduleFormat(schedule string, fldPath *field.Path) field.ErrorList { + allErrs := field.ErrorList{} + _, err := cron.Parse(schedule) + if err != nil { + allErrs = append(allErrs, field.Invalid(fldPath, schedule, err.Error())) + } + + return allErrs +} + +func ValidateJobTemplate(job *batch.JobTemplate) field.ErrorList { + // this method should be identical to ValidateJob + allErrs := apivalidation.ValidateObjectMeta(&job.ObjectMeta, true, apivalidation.ValidateReplicationControllerName, field.NewPath("metadata")) + allErrs = append(allErrs, ValidateJobTemplateSpec(&job.Template, field.NewPath("template"))...) + return allErrs +} + +func ValidateJobTemplateSpec(spec *batch.JobTemplateSpec, fldPath *field.Path) field.ErrorList { + // this method should be identical to ValidateJob + allErrs := ValidateJobSpec(&spec.Spec, fldPath.Child("spec")) + return allErrs +} diff --git a/pkg/apis/batch/validation/validation_test.go b/pkg/apis/batch/validation/validation_test.go index 5b59a4c6e19..2f5fb534db2 100644 --- a/pkg/apis/batch/validation/validation_test.go +++ b/pkg/apis/batch/validation/validation_test.go @@ -26,26 +26,35 @@ import ( "k8s.io/kubernetes/pkg/types" ) -func TestValidateJob(t *testing.T) { - validManualSelector := &unversioned.LabelSelector{ +func getValidManualSelector() *unversioned.LabelSelector { + return &unversioned.LabelSelector{ MatchLabels: map[string]string{"a": "b"}, } - validGeneratedSelector := &unversioned.LabelSelector{ +} + +func getValidPodTemplateSpecForManual(selector *unversioned.LabelSelector) api.PodTemplateSpec { + return api.PodTemplateSpec{ + ObjectMeta: api.ObjectMeta{ + Labels: selector.MatchLabels, + }, + Spec: api.PodSpec{ + RestartPolicy: api.RestartPolicyOnFailure, + DNSPolicy: api.DNSClusterFirst, + Containers: []api.Container{{Name: "abc", Image: "image", ImagePullPolicy: "IfNotPresent"}}, + }, + } +} + +func getValidGeneratedSelector() *unversioned.LabelSelector { + return &unversioned.LabelSelector{ MatchLabels: map[string]string{"controller-uid": "1a2b3c", "job-name": "myjob"}, } - validPodTemplateSpecForManual := api.PodTemplateSpec{ +} + +func getValidPodTemplateSpecForGenerated(selector *unversioned.LabelSelector) api.PodTemplateSpec { + return api.PodTemplateSpec{ ObjectMeta: api.ObjectMeta{ - Labels: validManualSelector.MatchLabels, - }, - Spec: api.PodSpec{ - RestartPolicy: api.RestartPolicyOnFailure, - DNSPolicy: api.DNSClusterFirst, - Containers: []api.Container{{Name: "abc", Image: "image", ImagePullPolicy: "IfNotPresent"}}, - }, - } - validPodTemplateSpecForGenerated := api.PodTemplateSpec{ - ObjectMeta: api.ObjectMeta{ - Labels: validGeneratedSelector.MatchLabels, + Labels: selector.MatchLabels, }, Spec: api.PodSpec{ RestartPolicy: api.RestartPolicyOnFailure, @@ -53,6 +62,14 @@ func TestValidateJob(t *testing.T) { Containers: []api.Container{{Name: "abc", Image: "image", ImagePullPolicy: "IfNotPresent"}}, }, } +} + +func TestValidateJob(t *testing.T) { + validManualSelector := getValidManualSelector() + validPodTemplateSpecForManual := getValidPodTemplateSpecForManual(validManualSelector) + validGeneratedSelector := getValidGeneratedSelector() + validPodTemplateSpecForGenerated := getValidPodTemplateSpecForGenerated(validGeneratedSelector) + successCases := map[string]batch.Job{ "manual selector": { ObjectMeta: api.ObjectMeta{ @@ -73,9 +90,8 @@ func TestValidateJob(t *testing.T) { UID: types.UID("1a2b3c"), }, Spec: batch.JobSpec{ - Selector: validGeneratedSelector, - ManualSelector: newBool(false), - Template: validPodTemplateSpecForGenerated, + Selector: validGeneratedSelector, + Template: validPodTemplateSpecForGenerated, }, }, } @@ -94,9 +110,9 @@ func TestValidateJob(t *testing.T) { UID: types.UID("1a2b3c"), }, Spec: batch.JobSpec{ - Parallelism: &negative, - ManualSelector: newBool(true), - Template: validPodTemplateSpecForGenerated, + Parallelism: &negative, + Selector: validGeneratedSelector, + Template: validPodTemplateSpecForGenerated, }, }, "spec.completions:must be greater than or equal to 0": { @@ -106,10 +122,9 @@ func TestValidateJob(t *testing.T) { UID: types.UID("1a2b3c"), }, Spec: batch.JobSpec{ - Completions: &negative, - Selector: validManualSelector, - ManualSelector: newBool(true), - Template: validPodTemplateSpecForGenerated, + Completions: &negative, + Selector: validGeneratedSelector, + Template: validPodTemplateSpecForGenerated, }, }, "spec.activeDeadlineSeconds:must be greater than or equal to 0": { @@ -120,8 +135,7 @@ func TestValidateJob(t *testing.T) { }, Spec: batch.JobSpec{ ActiveDeadlineSeconds: &negative64, - Selector: validManualSelector, - ManualSelector: newBool(true), + Selector: validGeneratedSelector, Template: validPodTemplateSpecForGenerated, }, }, @@ -290,6 +304,276 @@ func TestValidateJobUpdateStatus(t *testing.T) { } } +func TestValidateScheduledJob(t *testing.T) { + validManualSelector := getValidManualSelector() + validGeneratedSelector := getValidGeneratedSelector() + validPodTemplateSpec := getValidPodTemplateSpecForGenerated(validGeneratedSelector) + + successCases := map[string]batch.ScheduledJob{ + "basic scheduled job": { + 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: validGeneratedSelector, + Template: validPodTemplateSpec, + }, + }, + }, + }, + } + for k, v := range successCases { + if errs := ValidateScheduledJob(&v); len(errs) != 0 { + t.Errorf("expected success for %s: %v", k, errs) + } + } + + negative := int32(-1) + negative64 := int64(-1) + + errorCases := map[string]batch.ScheduledJob{ + "spec.schedule: Invalid value": { + ObjectMeta: api.ObjectMeta{ + Name: "myscheduledjob", + Namespace: api.NamespaceDefault, + UID: types.UID("1a2b3c"), + }, + Spec: batch.ScheduledJobSpec{ + Schedule: "error", + ConcurrencyPolicy: batch.AllowConcurrent, + JobTemplate: batch.JobTemplateSpec{ + Spec: batch.JobSpec{ + Selector: validGeneratedSelector, + Template: validPodTemplateSpec, + }, + }, + }, + }, + "spec.schedule: Required value": { + 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: validGeneratedSelector, + Template: validPodTemplateSpec, + }, + }, + }, + }, + "spec.startingDeadlineSeconds:must be greater than or equal to 0": { + ObjectMeta: api.ObjectMeta{ + Name: "myscheduledjob", + Namespace: api.NamespaceDefault, + UID: types.UID("1a2b3c"), + }, + Spec: batch.ScheduledJobSpec{ + Schedule: "* * * * * ?", + ConcurrencyPolicy: batch.AllowConcurrent, + StartingDeadlineSeconds: &negative64, + JobTemplate: batch.JobTemplateSpec{ + Spec: batch.JobSpec{ + Selector: validGeneratedSelector, + Template: validPodTemplateSpec, + }, + }, + }, + }, + "spec.concurrencyPolicy: Required value": { + ObjectMeta: api.ObjectMeta{ + Name: "myscheduledjob", + Namespace: api.NamespaceDefault, + UID: types.UID("1a2b3c"), + }, + Spec: batch.ScheduledJobSpec{ + Schedule: "* * * * * ?", + JobTemplate: batch.JobTemplateSpec{ + Spec: batch.JobSpec{ + Selector: validGeneratedSelector, + Template: validPodTemplateSpec, + }, + }, + }, + }, + "spec.jobTemplate.spec.parallelism:must be greater than or equal to 0": { + 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: validGeneratedSelector, + Parallelism: &negative, + Template: validPodTemplateSpec, + }, + }, + }, + }, + "spec.jobTemplate.spec.completions:must be greater than or equal to 0": { + 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{ + Completions: &negative, + Selector: validGeneratedSelector, + Template: validPodTemplateSpec, + }, + }, + }, + }, + "spec.jobTemplate.spec.activeDeadlineSeconds:must be greater than or equal to 0": { + 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{ + ActiveDeadlineSeconds: &negative64, + Selector: validGeneratedSelector, + Template: validPodTemplateSpec, + }, + }, + }, + }, + "spec.jobTemplate.spec.selector:Required value": { + 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{ + Template: validPodTemplateSpec, + }, + }, + }, + }, + "spec.jobTemplate.spec.template.metadata.labels: Invalid value: {\"y\":\"z\"}: `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{"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"}}, + }, + }, + }, + }, + }, + }, + "spec.jobTemplate.spec.template.spec.restartPolicy: Unsupported value": { + 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: validManualSelector.MatchLabels, + }, + Spec: api.PodSpec{ + RestartPolicy: api.RestartPolicyAlways, + DNSPolicy: api.DNSClusterFirst, + Containers: []api.Container{{Name: "abc", Image: "image", ImagePullPolicy: "IfNotPresent"}}, + }, + }, + }, + }, + }, + }, + } + + for k, v := range errorCases { + errs := ValidateScheduledJob(&v) + if len(errs) == 0 { + t.Errorf("expected failure for %s", k) + } else { + s := strings.Split(k, ":") + err := errs[0] + if err.Field != s[0] || !strings.Contains(err.Error(), s[1]) { + t.Errorf("unexpected error: %v, expected: %s", err, k) + } + } + } +} + func newBool(val bool) *bool { p := new(bool) *p = val