diff --git a/pkg/apis/batch/fuzzer/fuzzer.go b/pkg/apis/batch/fuzzer/fuzzer.go index 367ddf59c94..832de7d2f66 100644 --- a/pkg/apis/batch/fuzzer/fuzzer.go +++ b/pkg/apis/batch/fuzzer/fuzzer.go @@ -45,11 +45,7 @@ var Funcs = func(codecs runtimeserializer.CodecFactory) []interface{} { j.Completions = &completions j.Parallelism = ¶llelism j.BackoffLimit = &backoffLimit - if c.Rand.Int31()%2 == 0 { - j.ManualSelector = pointer.Bool(true) - } else { - j.ManualSelector = nil - } + j.ManualSelector = pointer.Bool(c.RandBool()) mode := batch.NonIndexedCompletion if c.RandBool() { mode = batch.IndexedCompletion diff --git a/pkg/apis/batch/v1/defaults.go b/pkg/apis/batch/v1/defaults.go index 7b6d585cd71..3a0033fb38d 100644 --- a/pkg/apis/batch/v1/defaults.go +++ b/pkg/apis/batch/v1/defaults.go @@ -79,6 +79,9 @@ func SetDefaults_Job(obj *batchv1.Job) { } } } + if obj.Spec.ManualSelector == nil { + obj.Spec.ManualSelector = utilpointer.Bool(false) + } } func SetDefaults_CronJob(obj *batchv1.CronJob) { diff --git a/pkg/apis/batch/v1/defaults_test.go b/pkg/apis/batch/v1/defaults_test.go index 1da8c4eaeb8..6c5fe47269a 100644 --- a/pkg/apis/batch/v1/defaults_test.go +++ b/pkg/apis/batch/v1/defaults_test.go @@ -98,6 +98,7 @@ func TestSetDefaultJob(t *testing.T) { BackoffLimit: pointer.Int32(6), CompletionMode: completionModePtr(batchv1.NonIndexedCompletion), Suspend: pointer.Bool(false), + ManualSelector: pointer.Bool(false), PodFailurePolicy: &batchv1.PodFailurePolicy{ Rules: []batchv1.PodFailurePolicyRule{ { @@ -166,6 +167,7 @@ func TestSetDefaultJob(t *testing.T) { CompletionMode: completionModePtr(batchv1.NonIndexedCompletion), Suspend: pointer.Bool(false), PodReplacementPolicy: podReplacementPtr(batchv1.Failed), + ManualSelector: pointer.Bool(false), PodFailurePolicy: &batchv1.PodFailurePolicy{ Rules: []batchv1.PodFailurePolicyRule{ { @@ -198,6 +200,7 @@ func TestSetDefaultJob(t *testing.T) { CompletionMode: completionModePtr(batchv1.NonIndexedCompletion), Suspend: pointer.Bool(false), PodReplacementPolicy: podReplacementPtr(batchv1.TerminatingOrFailed), + ManualSelector: pointer.Bool(false), }, }, expectLabels: true, @@ -218,6 +221,7 @@ func TestSetDefaultJob(t *testing.T) { BackoffLimit: pointer.Int32(6), CompletionMode: completionModePtr(batchv1.NonIndexedCompletion), Suspend: pointer.Bool(false), + ManualSelector: pointer.Bool(false), }, }, expectLabels: true, @@ -237,6 +241,7 @@ func TestSetDefaultJob(t *testing.T) { BackoffLimit: pointer.Int32(6), CompletionMode: completionModePtr(batchv1.NonIndexedCompletion), Suspend: pointer.Bool(false), + ManualSelector: pointer.Bool(false), }, }, expectLabels: true, @@ -257,6 +262,7 @@ func TestSetDefaultJob(t *testing.T) { BackoffLimit: pointer.Int32(6), CompletionMode: completionModePtr(batchv1.NonIndexedCompletion), Suspend: pointer.Bool(true), + ManualSelector: pointer.Bool(false), }, }, expectLabels: true, @@ -279,6 +285,7 @@ func TestSetDefaultJob(t *testing.T) { BackoffLimit: pointer.Int32(6), CompletionMode: completionModePtr(batchv1.NonIndexedCompletion), Suspend: pointer.Bool(false), + ManualSelector: pointer.Bool(false), }, }, }, @@ -297,6 +304,7 @@ func TestSetDefaultJob(t *testing.T) { BackoffLimit: pointer.Int32(6), CompletionMode: completionModePtr(batchv1.NonIndexedCompletion), Suspend: pointer.Bool(false), + ManualSelector: pointer.Bool(false), }, }, expectLabels: true, @@ -316,6 +324,7 @@ func TestSetDefaultJob(t *testing.T) { BackoffLimit: pointer.Int32(6), CompletionMode: completionModePtr(batchv1.NonIndexedCompletion), Suspend: pointer.Bool(false), + ManualSelector: pointer.Bool(false), }, }, expectLabels: true, @@ -336,6 +345,7 @@ func TestSetDefaultJob(t *testing.T) { BackoffLimit: pointer.Int32(6), CompletionMode: completionModePtr(batchv1.NonIndexedCompletion), Suspend: pointer.Bool(false), + ManualSelector: pointer.Bool(false), }, }, expectLabels: true, @@ -356,6 +366,7 @@ func TestSetDefaultJob(t *testing.T) { BackoffLimit: pointer.Int32(5), CompletionMode: completionModePtr(batchv1.NonIndexedCompletion), Suspend: pointer.Bool(false), + ManualSelector: pointer.Bool(false), }, }, expectLabels: true, @@ -369,6 +380,7 @@ func TestSetDefaultJob(t *testing.T) { CompletionMode: completionModePtr(batchv1.NonIndexedCompletion), Suspend: pointer.Bool(false), PodReplacementPolicy: podReplacementPtr(batchv1.TerminatingOrFailed), + ManualSelector: pointer.Bool(false), Template: v1.PodTemplateSpec{ ObjectMeta: metav1.ObjectMeta{Labels: defaultLabels}, }, @@ -382,6 +394,7 @@ func TestSetDefaultJob(t *testing.T) { CompletionMode: completionModePtr(batchv1.NonIndexedCompletion), Suspend: pointer.Bool(false), PodReplacementPolicy: podReplacementPtr(batchv1.TerminatingOrFailed), + ManualSelector: pointer.Bool(false), Template: v1.PodTemplateSpec{ ObjectMeta: metav1.ObjectMeta{Labels: defaultLabels}, }, @@ -398,6 +411,7 @@ func TestSetDefaultJob(t *testing.T) { CompletionMode: completionModePtr(batchv1.IndexedCompletion), Suspend: pointer.Bool(true), PodReplacementPolicy: podReplacementPtr(batchv1.Failed), + ManualSelector: pointer.Bool(true), Template: v1.PodTemplateSpec{ ObjectMeta: metav1.ObjectMeta{Labels: defaultLabels}, }, @@ -411,6 +425,7 @@ func TestSetDefaultJob(t *testing.T) { CompletionMode: completionModePtr(batchv1.IndexedCompletion), Suspend: pointer.Bool(true), PodReplacementPolicy: podReplacementPtr(batchv1.Failed), + ManualSelector: pointer.Bool(true), }, }, expectLabels: true, @@ -424,6 +439,7 @@ func TestSetDefaultJob(t *testing.T) { CompletionMode: completionModePtr(batchv1.IndexedCompletion), Template: validPodTemplateSpec, Suspend: pointer.Bool(true), + ManualSelector: pointer.Bool(false), }, }, expected: &batchv1.Job{ @@ -435,6 +451,7 @@ func TestSetDefaultJob(t *testing.T) { CompletionMode: completionModePtr(batchv1.IndexedCompletion), Template: validPodTemplateSpec, Suspend: pointer.Bool(true), + ManualSelector: pointer.Bool(false), }, }, expectLabels: true, @@ -449,6 +466,7 @@ func TestSetDefaultJob(t *testing.T) { CompletionMode: completionModePtr(batchv1.IndexedCompletion), Template: validPodTemplateSpec, Suspend: pointer.Bool(true), + ManualSelector: pointer.Bool(true), }, }, expected: &batchv1.Job{ @@ -460,6 +478,7 @@ func TestSetDefaultJob(t *testing.T) { CompletionMode: completionModePtr(batchv1.IndexedCompletion), Template: validPodTemplateSpec, Suspend: pointer.Bool(true), + ManualSelector: pointer.Bool(true), }, }, expectLabels: true, @@ -500,6 +519,9 @@ func TestSetDefaultJob(t *testing.T) { if diff := cmp.Diff(expected.Spec.PodReplacementPolicy, actual.Spec.PodReplacementPolicy); diff != "" { t.Errorf("Unexpected PodReplacementPolicy (-want,+got):\n%s", diff) } + if diff := cmp.Diff(expected.Spec.ManualSelector, actual.Spec.ManualSelector); diff != "" { + t.Errorf("Unexpected ManualSelector (-want,+got):\n%s", diff) + } }) } } diff --git a/pkg/registry/batch/job/storage/storage_test.go b/pkg/registry/batch/job/storage/storage_test.go index 765b3179509..9894b57a3b3 100644 --- a/pkg/registry/batch/job/storage/storage_test.go +++ b/pkg/registry/batch/job/storage/storage_test.go @@ -17,6 +17,7 @@ limitations under the License. package storage import ( + "k8s.io/utils/pointer" "testing" batchv1 "k8s.io/api/batch/v1" @@ -137,9 +138,10 @@ func TestCreate(t *testing.T) { // invalid (empty selector) &batch.Job{ Spec: batch.JobSpec{ - Completions: validJob.Spec.Completions, - Selector: &metav1.LabelSelector{}, - Template: validJob.Spec.Template, + ManualSelector: pointer.Bool(false), + Completions: validJob.Spec.Completions, + Selector: &metav1.LabelSelector{}, + Template: validJob.Spec.Template, }, }, ) diff --git a/pkg/registry/batch/job/strategy.go b/pkg/registry/batch/job/strategy.go index 75e253857c1..71582bed98c 100644 --- a/pkg/registry/batch/job/strategy.go +++ b/pkg/registry/batch/job/strategy.go @@ -163,8 +163,7 @@ func (jobStrategy) PrepareForUpdate(ctx context.Context, obj, old runtime.Object // Validate validates a new job. func (jobStrategy) Validate(ctx context.Context, obj runtime.Object) field.ErrorList { job := obj.(*batch.Job) - // TODO: move UID generation earlier and do this in defaulting logic? - if job.Spec.ManualSelector == nil || *job.Spec.ManualSelector == false { + if !*job.Spec.ManualSelector { generateSelector(job) } opts := validationOptionsForJob(job, nil) diff --git a/pkg/registry/batch/job/strategy_test.go b/pkg/registry/batch/job/strategy_test.go index c2ee060b4a7..aa7abac20f2 100644 --- a/pkg/registry/batch/job/strategy_test.go +++ b/pkg/registry/batch/job/strategy_test.go @@ -1320,7 +1320,8 @@ func TestJobStrategy_Validate(t *testing.T) { job: &batch.Job{ ObjectMeta: validObjectMeta, Spec: batch.JobSpec{ - Selector: nil, + Selector: nil, + ManualSelector: pointer.Bool(false), Template: api.PodTemplateSpec{ ObjectMeta: metav1.ObjectMeta{ Labels: validSelector.MatchLabels, @@ -1331,7 +1332,8 @@ func TestJobStrategy_Validate(t *testing.T) { wantJob: &batch.Job{ ObjectMeta: validObjectMeta, Spec: batch.JobSpec{ - Selector: &metav1.LabelSelector{MatchLabels: map[string]string{batch.ControllerUidLabel: string(theUID)}}, + Selector: &metav1.LabelSelector{MatchLabels: map[string]string{batch.ControllerUidLabel: string(theUID)}}, + ManualSelector: pointer.Bool(false), Template: api.PodTemplateSpec{ ObjectMeta: metav1.ObjectMeta{ Labels: validSelector.MatchLabels, @@ -1344,7 +1346,8 @@ func TestJobStrategy_Validate(t *testing.T) { job: &batch.Job{ ObjectMeta: validObjectMeta, Spec: batch.JobSpec{ - Selector: nil, + Selector: nil, + ManualSelector: pointer.Bool(false), Template: api.PodTemplateSpec{ Spec: validPodSpec, }}, @@ -1352,7 +1355,8 @@ func TestJobStrategy_Validate(t *testing.T) { wantJob: &batch.Job{ ObjectMeta: validObjectMeta, Spec: batch.JobSpec{ - Selector: &metav1.LabelSelector{MatchLabels: map[string]string{batch.ControllerUidLabel: string(theUID)}}, + Selector: &metav1.LabelSelector{MatchLabels: map[string]string{batch.ControllerUidLabel: string(theUID)}}, + ManualSelector: pointer.Bool(false), Template: api.PodTemplateSpec{ ObjectMeta: metav1.ObjectMeta{ Labels: validLabels, @@ -1365,7 +1369,8 @@ func TestJobStrategy_Validate(t *testing.T) { job: &batch.Job{ ObjectMeta: validObjectMeta, Spec: batch.JobSpec{ - Selector: nil, + Selector: nil, + ManualSelector: pointer.Bool(false), Template: api.PodTemplateSpec{ ObjectMeta: metav1.ObjectMeta{ Labels: labelsWithNonBatch, @@ -1376,7 +1381,8 @@ func TestJobStrategy_Validate(t *testing.T) { wantJob: &batch.Job{ ObjectMeta: validObjectMeta, Spec: batch.JobSpec{ - Selector: &metav1.LabelSelector{MatchLabels: map[string]string{batch.ControllerUidLabel: string(theUID)}}, + Selector: &metav1.LabelSelector{MatchLabels: map[string]string{batch.ControllerUidLabel: string(theUID)}}, + ManualSelector: pointer.Bool(false), Template: api.PodTemplateSpec{ ObjectMeta: metav1.ObjectMeta{ Labels: labelsWithNonBatch, @@ -1419,7 +1425,8 @@ func TestJobStrategy_Validate(t *testing.T) { job: &batch.Job{ ObjectMeta: validObjectMeta, Spec: batch.JobSpec{ - Selector: nil, + Selector: nil, + ManualSelector: pointer.Bool(false), Template: api.PodTemplateSpec{ ObjectMeta: metav1.ObjectMeta{ Labels: validSelector.MatchLabels, @@ -1435,7 +1442,8 @@ func TestJobStrategy_Validate(t *testing.T) { wantJob: &batch.Job{ ObjectMeta: validObjectMeta, Spec: batch.JobSpec{ - Selector: &metav1.LabelSelector{MatchLabels: map[string]string{batch.ControllerUidLabel: string(theUID)}}, + Selector: &metav1.LabelSelector{MatchLabels: map[string]string{batch.ControllerUidLabel: string(theUID)}}, + ManualSelector: pointer.Bool(false), Template: api.PodTemplateSpec{ ObjectMeta: metav1.ObjectMeta{ Labels: labelsWithNonBatch, @@ -1453,7 +1461,8 @@ func TestJobStrategy_Validate(t *testing.T) { job: &batch.Job{ ObjectMeta: validObjectMeta, Spec: batch.JobSpec{ - Selector: nil, + Selector: nil, + ManualSelector: pointer.Bool(false), Template: api.PodTemplateSpec{ ObjectMeta: metav1.ObjectMeta{ Labels: validSelector.MatchLabels, @@ -1470,7 +1479,8 @@ func TestJobStrategy_Validate(t *testing.T) { wantJob: &batch.Job{ ObjectMeta: validObjectMeta, Spec: batch.JobSpec{ - Selector: &metav1.LabelSelector{MatchLabels: map[string]string{batch.ControllerUidLabel: string(theUID)}}, + Selector: &metav1.LabelSelector{MatchLabels: map[string]string{batch.ControllerUidLabel: string(theUID)}}, + ManualSelector: pointer.Bool(false), Template: api.PodTemplateSpec{ ObjectMeta: metav1.ObjectMeta{ Labels: labelsWithNonBatch,