diff --git a/pkg/apis/extensions/v1beta1/defaults.go b/pkg/apis/extensions/v1beta1/defaults.go index 5373c23d810..46d4c3785b0 100644 --- a/pkg/apis/extensions/v1beta1/defaults.go +++ b/pkg/apis/extensions/v1beta1/defaults.go @@ -86,7 +86,14 @@ func addDefaultingFuncs(scheme *runtime.Scheme) { labels := obj.Spec.Template.Labels // TODO: support templates defined elsewhere when we support them in the API if labels != nil { - if obj.Spec.Selector == nil { + // if an autoselector is requested, we'll build the selector later with controller-uid and job-name + autoSelector := bool(obj.Spec.AutoSelector != nil && *obj.Spec.AutoSelector) + + // otherwise, we are using a manual selector + manualSelector := !autoSelector + + // and default behavior for an unspecified manual selector is to use the pod template labels + if manualSelector && obj.Spec.Selector == nil { obj.Spec.Selector = &LabelSelector{ MatchLabels: labels, } diff --git a/pkg/apis/extensions/v1beta1/defaults_test.go b/pkg/apis/extensions/v1beta1/defaults_test.go index 760df336056..f02b2c0d446 100644 --- a/pkg/apis/extensions/v1beta1/defaults_test.go +++ b/pkg/apis/extensions/v1beta1/defaults_test.go @@ -374,45 +374,107 @@ func TestSetDefaultJobParallelismAndCompletions(t *testing.T) { } func TestSetDefaultJobSelector(t *testing.T) { - expected := &Job{ - Spec: JobSpec{ - Selector: &LabelSelector{ - MatchLabels: map[string]string{"job": "selector"}, - }, - Completions: newInt32(1), - Parallelism: newInt32(1), - }, - } - tests := []*Job{ - // selector set explicitly, completions and parallelism - default + tests := []struct { + original *Job + expectedSelector *LabelSelector + }{ + // selector set explicitly, nil autoSelector { - Spec: JobSpec{ - Selector: &LabelSelector{ - MatchLabels: map[string]string{"job": "selector"}, - }, - }, - }, - // selector from template labels, completions and parallelism - default - { - Spec: JobSpec{ - Template: v1.PodTemplateSpec{ - ObjectMeta: v1.ObjectMeta{ - Labels: map[string]string{"job": "selector"}, + original: &Job{ + Spec: JobSpec{ + Selector: &LabelSelector{ + MatchLabels: map[string]string{"job": "selector"}, }, }, }, + expectedSelector: &LabelSelector{ + MatchLabels: map[string]string{"job": "selector"}, + }, + }, + // selector set explicitly, autoSelector=true + { + original: &Job{ + Spec: JobSpec{ + Selector: &LabelSelector{ + MatchLabels: map[string]string{"job": "selector"}, + }, + AutoSelector: newBool(true), + }, + }, + expectedSelector: &LabelSelector{ + MatchLabels: map[string]string{"job": "selector"}, + }, + }, + // selector set explicitly, autoSelector=false + { + original: &Job{ + Spec: JobSpec{ + Selector: &LabelSelector{ + MatchLabels: map[string]string{"job": "selector"}, + }, + AutoSelector: newBool(false), + }, + }, + expectedSelector: &LabelSelector{ + MatchLabels: map[string]string{"job": "selector"}, + }, + }, + // selector from template labels + { + original: &Job{ + Spec: JobSpec{ + Template: v1.PodTemplateSpec{ + ObjectMeta: v1.ObjectMeta{ + Labels: map[string]string{"job": "selector"}, + }, + }, + }, + }, + expectedSelector: &LabelSelector{ + MatchLabels: map[string]string{"job": "selector"}, + }, + }, + // selector from template labels, autoSelector=false + { + original: &Job{ + Spec: JobSpec{ + Template: v1.PodTemplateSpec{ + ObjectMeta: v1.ObjectMeta{ + Labels: map[string]string{"job": "selector"}, + }, + }, + AutoSelector: newBool(false), + }, + }, + expectedSelector: &LabelSelector{ + MatchLabels: map[string]string{"job": "selector"}, + }, + }, + // selector not copied from template labels, autoSelector=true + { + original: &Job{ + Spec: JobSpec{ + Template: v1.PodTemplateSpec{ + ObjectMeta: v1.ObjectMeta{ + Labels: map[string]string{"job": "selector"}, + }, + }, + AutoSelector: newBool(true), + }, + }, + expectedSelector: nil, }, } - for _, original := range tests { - obj2 := roundTrip(t, runtime.Object(original)) + for i, testcase := range tests { + obj2 := roundTrip(t, runtime.Object(testcase.original)) got, ok := obj2.(*Job) if !ok { - t.Errorf("unexpected object: %v", got) + t.Errorf("%d: unexpected object: %v", i, got) t.FailNow() } - if !reflect.DeepEqual(got.Spec.Selector, expected.Spec.Selector) { - t.Errorf("got different selectors %#v %#v", got.Spec.Selector, expected.Spec.Selector) + if !reflect.DeepEqual(got.Spec.Selector, testcase.expectedSelector) { + t.Errorf("%d: got different selectors %#v %#v", i, got.Spec.Selector, testcase.expectedSelector) } } } @@ -661,3 +723,9 @@ func newString(val string) *string { *p = val return p } + +func newBool(val bool) *bool { + b := new(bool) + *b = val + return b +} diff --git a/pkg/apis/extensions/validation/validation_test.go b/pkg/apis/extensions/validation/validation_test.go index 818a6966173..e485a99614a 100644 --- a/pkg/apis/extensions/validation/validation_test.go +++ b/pkg/apis/extensions/validation/validation_test.go @@ -980,7 +980,7 @@ func TestValidateJob(t *testing.T) { MatchLabels: map[string]string{"a": "b"}, } validGeneratedSelector := &unversioned.LabelSelector{ - MatchLabels: map[string]string{"collection-uid": "1a2b3c"}, + MatchLabels: map[string]string{"controller-uid": "1a2b3c", "job-name": "myjob"}, } validPodTemplateSpecForManual := api.PodTemplateSpec{ ObjectMeta: api.ObjectMeta{ @@ -1023,7 +1023,7 @@ func TestValidateJob(t *testing.T) { }, Spec: extensions.JobSpec{ Selector: validGeneratedSelector, - ManualSelector: newBool(true), + ManualSelector: newBool(false), Template: validPodTemplateSpecForGenerated, }, },