Change suspend to be pointer for ScheduledJob and modify validation to forbid setting job selectors

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.

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.

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
 }

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(getValidGeneratedSelector())
-	validPodTemplateSpec := getValidPodTemplateSpecForGenerated(validGeneratedSelector)
+	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{
+						Template:       validPodTemplateSpec,
-							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"}},
-							},
-						},
 					},
 				},
 			},
@@ -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,

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,
-					Template:       validPodTemplateSpec,
-					ManualSelector: newBool(true),
 				},
 			},
 		},
@@ -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,
-					Template:       validPodTemplateSpec,
-					ManualSelector: newBool(true),
 				},
 			},
 		},
@@ -154,9 +138,7 @@ func TestScheduledJobStatusStrategy(t *testing.T) {
 			ConcurrencyPolicy: batch.AllowConcurrent,
 			JobTemplate: batch.JobTemplateSpec{
 				Spec: batch.JobSpec{
-					Selector:       validSelector,
+					Template: validPodTemplateSpec,
-					Template:       validPodTemplateSpec,
-					ManualSelector: newBool(true),
 				},
 			},
 		},