ScheduledJob validation & defaults

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
+	}
+}

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
+}

pkg/apis/batch/validation/validation_test.go

@@ -26,16 +26,35 @@ import (
 	"k8s.io/kubernetes/pkg/types"
 )
 
-func TestValidateJob(t *testing.T) {
+func getValidManualSelector() *unversioned.LabelSelector {
-	validManualSelector := &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,
+			Labels: selector.MatchLabels,
 		},
 		Spec: api.PodSpec{
 			RestartPolicy: api.RestartPolicyOnFailure,
@@ -43,16 +62,14 @@ func TestValidateJob(t *testing.T) {
 			Containers:    []api.Container{{Name: "abc", Image: "image", ImagePullPolicy: "IfNotPresent"}},
 		},
 	}
-	validPodTemplateSpecForGenerated := api.PodTemplateSpec{
-		ObjectMeta: api.ObjectMeta{
-			Labels: validGeneratedSelector.MatchLabels,
-		},
-		Spec: api.PodSpec{
-			RestartPolicy: api.RestartPolicyOnFailure,
-			DNSPolicy:     api.DNSClusterFirst,
-			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{
@@ -74,7 +91,6 @@ func TestValidateJob(t *testing.T) {
 			},
 			Spec: batch.JobSpec{
 				Selector: validGeneratedSelector,
-				ManualSelector: newBool(false),
 				Template: validPodTemplateSpecForGenerated,
 			},
 		},
@@ -95,7 +111,7 @@ func TestValidateJob(t *testing.T) {
 			},
 			Spec: batch.JobSpec{
 				Parallelism: &negative,
-				ManualSelector: newBool(true),
+				Selector:    validGeneratedSelector,
 				Template:    validPodTemplateSpecForGenerated,
 			},
 		},
@@ -107,8 +123,7 @@ func TestValidateJob(t *testing.T) {
 			},
 			Spec: batch.JobSpec{
 				Completions: &negative,
-				Selector:       validManualSelector,
+				Selector:    validGeneratedSelector,
-				ManualSelector: newBool(true),
 				Template:    validPodTemplateSpecForGenerated,
 			},
 		},
@@ -120,8 +135,7 @@ func TestValidateJob(t *testing.T) {
 			},
 			Spec: batch.JobSpec{
 				ActiveDeadlineSeconds: &negative64,
-				Selector:              validManualSelector,
+				Selector:              validGeneratedSelector,
-				ManualSelector:        newBool(true),
 				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