Avoid populating job selector from pod template labels in autoSelector case

This commit is contained in:
Jordan Liggitt 2016-03-16 02:23:01 -04:00
parent 49092c0b99
commit 64f51723c8
3 changed files with 106 additions and 31 deletions

View File

@ -86,7 +86,14 @@ func addDefaultingFuncs(scheme *runtime.Scheme) {
labels := obj.Spec.Template.Labels labels := obj.Spec.Template.Labels
// TODO: support templates defined elsewhere when we support them in the API // TODO: support templates defined elsewhere when we support them in the API
if labels != nil { 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{ obj.Spec.Selector = &LabelSelector{
MatchLabels: labels, MatchLabels: labels,
} }

View File

@ -374,26 +374,54 @@ func TestSetDefaultJobParallelismAndCompletions(t *testing.T) {
} }
func TestSetDefaultJobSelector(t *testing.T) { func TestSetDefaultJobSelector(t *testing.T) {
expected := &Job{ tests := []struct {
Spec: JobSpec{ original *Job
Selector: &LabelSelector{ expectedSelector *LabelSelector
MatchLabels: map[string]string{"job": "selector"}, }{
}, // selector set explicitly, nil autoSelector
Completions: newInt32(1),
Parallelism: newInt32(1),
},
}
tests := []*Job{
// selector set explicitly, completions and parallelism - default
{ {
original: &Job{
Spec: JobSpec{ Spec: JobSpec{
Selector: &LabelSelector{ Selector: &LabelSelector{
MatchLabels: map[string]string{"job": "selector"}, MatchLabels: map[string]string{"job": "selector"},
}, },
}, },
}, },
// selector from template labels, completions and parallelism - default 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{ Spec: JobSpec{
Template: v1.PodTemplateSpec{ Template: v1.PodTemplateSpec{
ObjectMeta: v1.ObjectMeta{ ObjectMeta: v1.ObjectMeta{
@ -402,17 +430,51 @@ func TestSetDefaultJobSelector(t *testing.T) {
}, },
}, },
}, },
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 { for i, testcase := range tests {
obj2 := roundTrip(t, runtime.Object(original)) obj2 := roundTrip(t, runtime.Object(testcase.original))
got, ok := obj2.(*Job) got, ok := obj2.(*Job)
if !ok { if !ok {
t.Errorf("unexpected object: %v", got) t.Errorf("%d: unexpected object: %v", i, got)
t.FailNow() t.FailNow()
} }
if !reflect.DeepEqual(got.Spec.Selector, expected.Spec.Selector) { if !reflect.DeepEqual(got.Spec.Selector, testcase.expectedSelector) {
t.Errorf("got different selectors %#v %#v", got.Spec.Selector, expected.Spec.Selector) 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 *p = val
return p return p
} }
func newBool(val bool) *bool {
b := new(bool)
*b = val
return b
}

View File

@ -980,7 +980,7 @@ func TestValidateJob(t *testing.T) {
MatchLabels: map[string]string{"a": "b"}, MatchLabels: map[string]string{"a": "b"},
} }
validGeneratedSelector := &unversioned.LabelSelector{ validGeneratedSelector := &unversioned.LabelSelector{
MatchLabels: map[string]string{"collection-uid": "1a2b3c"}, MatchLabels: map[string]string{"controller-uid": "1a2b3c", "job-name": "myjob"},
} }
validPodTemplateSpecForManual := api.PodTemplateSpec{ validPodTemplateSpecForManual := api.PodTemplateSpec{
ObjectMeta: api.ObjectMeta{ ObjectMeta: api.ObjectMeta{
@ -1023,7 +1023,7 @@ func TestValidateJob(t *testing.T) {
}, },
Spec: extensions.JobSpec{ Spec: extensions.JobSpec{
Selector: validGeneratedSelector, Selector: validGeneratedSelector,
ManualSelector: newBool(true), ManualSelector: newBool(false),
Template: validPodTemplateSpecForGenerated, Template: validPodTemplateSpecForGenerated,
}, },
}, },