diff --git a/hack/.linted_packages b/hack/.linted_packages index 38f2eb0535e..dc49c89bc29 100644 --- a/hack/.linted_packages +++ b/hack/.linted_packages @@ -65,6 +65,8 @@ pkg/apis/authentication/install pkg/apis/authorization/install pkg/apis/autoscaling/install pkg/apis/batch/install +pkg/apis/batch/v1 +pkg/apis/batch/v2alpha1 pkg/apis/certificates/install pkg/apis/componentconfig/install pkg/apis/extensions/install diff --git a/pkg/apis/batch/v1/defaults.go b/pkg/apis/batch/v1/defaults.go index 5b029d09a0c..5b8e10a2e4f 100644 --- a/pkg/apis/batch/v1/defaults.go +++ b/pkg/apis/batch/v1/defaults.go @@ -39,4 +39,8 @@ func SetDefaults_Job(obj *Job) { obj.Spec.Parallelism = new(int32) *obj.Spec.Parallelism = 1 } + labels := obj.Spec.Template.Labels + if labels != nil && len(obj.Labels) == 0 { + obj.Labels = labels + } } diff --git a/pkg/apis/batch/v1/defaults_test.go b/pkg/apis/batch/v1/defaults_test.go new file mode 100644 index 00000000000..e655e685870 --- /dev/null +++ b/pkg/apis/batch/v1/defaults_test.go @@ -0,0 +1,222 @@ +/* +Copyright 2016 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package v1_test + +import ( + "reflect" + "testing" + + "k8s.io/kubernetes/pkg/api" + _ "k8s.io/kubernetes/pkg/api/install" + "k8s.io/kubernetes/pkg/api/v1" + _ "k8s.io/kubernetes/pkg/apis/batch/install" + . "k8s.io/kubernetes/pkg/apis/batch/v1" + "k8s.io/kubernetes/pkg/runtime" +) + +func TestSetDefaultJob(t *testing.T) { + defaultLabels := map[string]string{"default": "default"} + tests := map[string]struct { + original *Job + expected *Job + expectLabels bool + }{ + "both unspecified -> sets both to 1": { + original: &Job{ + Spec: JobSpec{ + Template: v1.PodTemplateSpec{ + ObjectMeta: v1.ObjectMeta{Labels: defaultLabels}, + }, + }, + }, + expected: &Job{ + Spec: JobSpec{ + Completions: newInt32(1), + Parallelism: newInt32(1), + }, + }, + expectLabels: true, + }, + "both unspecified -> sets both to 1 and no default labels": { + original: &Job{ + ObjectMeta: v1.ObjectMeta{ + Labels: map[string]string{"mylabel": "myvalue"}, + }, + Spec: JobSpec{ + Template: v1.PodTemplateSpec{ + ObjectMeta: v1.ObjectMeta{Labels: defaultLabels}, + }, + }, + }, + expected: &Job{ + Spec: JobSpec{ + Completions: newInt32(1), + Parallelism: newInt32(1), + }, + }, + }, + "WQ: Parallelism explicitly 0 and completions unset -> no change": { + original: &Job{ + Spec: JobSpec{ + Parallelism: newInt32(0), + Template: v1.PodTemplateSpec{ + ObjectMeta: v1.ObjectMeta{Labels: defaultLabels}, + }, + }, + }, + expected: &Job{ + Spec: JobSpec{ + Parallelism: newInt32(0), + }, + }, + expectLabels: true, + }, + "WQ: Parallelism explicitly 2 and completions unset -> no change": { + original: &Job{ + Spec: JobSpec{ + Parallelism: newInt32(2), + Template: v1.PodTemplateSpec{ + ObjectMeta: v1.ObjectMeta{Labels: defaultLabels}, + }, + }, + }, + expected: &Job{ + Spec: JobSpec{ + Parallelism: newInt32(2), + }, + }, + expectLabels: true, + }, + "Completions explicitly 2 and parallelism unset -> parallelism is defaulted": { + original: &Job{ + Spec: JobSpec{ + Completions: newInt32(2), + Template: v1.PodTemplateSpec{ + ObjectMeta: v1.ObjectMeta{Labels: defaultLabels}, + }, + }, + }, + expected: &Job{ + Spec: JobSpec{ + Completions: newInt32(2), + Parallelism: newInt32(1), + }, + }, + expectLabels: true, + }, + "Both set -> no change": { + original: &Job{ + Spec: JobSpec{ + Completions: newInt32(10), + Parallelism: newInt32(11), + Template: v1.PodTemplateSpec{ + ObjectMeta: v1.ObjectMeta{Labels: defaultLabels}, + }, + }, + }, + expected: &Job{ + Spec: JobSpec{ + Completions: newInt32(10), + Parallelism: newInt32(11), + Template: v1.PodTemplateSpec{ + ObjectMeta: v1.ObjectMeta{Labels: defaultLabels}, + }, + }, + }, + expectLabels: true, + }, + "Both set, flipped -> no change": { + original: &Job{ + Spec: JobSpec{ + Completions: newInt32(11), + Parallelism: newInt32(10), + Template: v1.PodTemplateSpec{ + ObjectMeta: v1.ObjectMeta{Labels: defaultLabels}, + }, + }, + }, + expected: &Job{ + Spec: JobSpec{ + Completions: newInt32(11), + Parallelism: newInt32(10), + }, + }, + expectLabels: true, + }, + } + + for name, test := range tests { + original := test.original + expected := test.expected + obj2 := roundTrip(t, runtime.Object(original)) + actual, ok := obj2.(*Job) + if !ok { + t.Errorf("%s: unexpected object: %v", name, actual) + t.FailNow() + } + if (actual.Spec.Completions == nil) != (expected.Spec.Completions == nil) { + t.Errorf("%s: got different *completions than expected: %v %v", name, actual.Spec.Completions, expected.Spec.Completions) + } + if actual.Spec.Completions != nil && expected.Spec.Completions != nil { + if *actual.Spec.Completions != *expected.Spec.Completions { + t.Errorf("%s: got different completions than expected: %d %d", name, *actual.Spec.Completions, *expected.Spec.Completions) + } + } + if (actual.Spec.Parallelism == nil) != (expected.Spec.Parallelism == nil) { + t.Errorf("%s: got different *Parallelism than expected: %v %v", name, actual.Spec.Parallelism, expected.Spec.Parallelism) + } + if actual.Spec.Parallelism != nil && expected.Spec.Parallelism != nil { + if *actual.Spec.Parallelism != *expected.Spec.Parallelism { + t.Errorf("%s: got different parallelism than expected: %d %d", name, *actual.Spec.Parallelism, *expected.Spec.Parallelism) + } + } + if test.expectLabels != reflect.DeepEqual(actual.Labels, actual.Spec.Template.Labels) { + if test.expectLabels { + t.Errorf("%s: expected: %v, got: %v", name, actual.Spec.Template.Labels, actual.Labels) + } else { + t.Errorf("%s: unexpected equality: %v", name, actual.Labels) + } + } + + } +} + +func roundTrip(t *testing.T, obj runtime.Object) runtime.Object { + data, err := runtime.Encode(api.Codecs.LegacyCodec(SchemeGroupVersion), obj) + if err != nil { + t.Errorf("%v\n %#v", err, obj) + return nil + } + obj2, err := runtime.Decode(api.Codecs.UniversalDecoder(), data) + if err != nil { + t.Errorf("%v\nData: %s\nSource: %#v", err, string(data), obj) + return nil + } + obj3 := reflect.New(reflect.TypeOf(obj).Elem()).Interface().(runtime.Object) + err = api.Scheme.Convert(obj2, obj3, nil) + if err != nil { + t.Errorf("%v\nSource: %#v", err, obj2) + return nil + } + return obj3 +} + +func newInt32(val int32) *int32 { + p := new(int32) + *p = val + return p +} diff --git a/pkg/apis/batch/v2alpha1/defaults.go b/pkg/apis/batch/v2alpha1/defaults.go index 9a594f16fd5..1c06e66a4ea 100644 --- a/pkg/apis/batch/v2alpha1/defaults.go +++ b/pkg/apis/batch/v2alpha1/defaults.go @@ -40,6 +40,10 @@ func SetDefaults_Job(obj *Job) { obj.Spec.Parallelism = new(int32) *obj.Spec.Parallelism = 1 } + labels := obj.Spec.Template.Labels + if labels != nil && len(obj.Labels) == 0 { + obj.Labels = labels + } } func SetDefaults_ScheduledJob(obj *ScheduledJob) { diff --git a/pkg/apis/batch/v2alpha1/defaults_test.go b/pkg/apis/batch/v2alpha1/defaults_test.go new file mode 100644 index 00000000000..0e3b6e62511 --- /dev/null +++ b/pkg/apis/batch/v2alpha1/defaults_test.go @@ -0,0 +1,222 @@ +/* +Copyright 2016 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package v2alpha1_test + +import ( + "reflect" + "testing" + + "k8s.io/kubernetes/pkg/api" + _ "k8s.io/kubernetes/pkg/api/install" + "k8s.io/kubernetes/pkg/api/v1" + _ "k8s.io/kubernetes/pkg/apis/batch/install" + . "k8s.io/kubernetes/pkg/apis/batch/v2alpha1" + "k8s.io/kubernetes/pkg/runtime" +) + +func TestSetDefaultJob(t *testing.T) { + defaultLabels := map[string]string{"default": "default"} + tests := map[string]struct { + original *Job + expected *Job + expectLabels bool + }{ + "both unspecified -> sets both to 1": { + original: &Job{ + Spec: JobSpec{ + Template: v1.PodTemplateSpec{ + ObjectMeta: v1.ObjectMeta{Labels: defaultLabels}, + }, + }, + }, + expected: &Job{ + Spec: JobSpec{ + Completions: newInt32(1), + Parallelism: newInt32(1), + }, + }, + expectLabels: true, + }, + "both unspecified -> sets both to 1 and no default labels": { + original: &Job{ + ObjectMeta: v1.ObjectMeta{ + Labels: map[string]string{"mylabel": "myvalue"}, + }, + Spec: JobSpec{ + Template: v1.PodTemplateSpec{ + ObjectMeta: v1.ObjectMeta{Labels: defaultLabels}, + }, + }, + }, + expected: &Job{ + Spec: JobSpec{ + Completions: newInt32(1), + Parallelism: newInt32(1), + }, + }, + }, + "WQ: Parallelism explicitly 0 and completions unset -> no change": { + original: &Job{ + Spec: JobSpec{ + Parallelism: newInt32(0), + Template: v1.PodTemplateSpec{ + ObjectMeta: v1.ObjectMeta{Labels: defaultLabels}, + }, + }, + }, + expected: &Job{ + Spec: JobSpec{ + Parallelism: newInt32(0), + }, + }, + expectLabels: true, + }, + "WQ: Parallelism explicitly 2 and completions unset -> no change": { + original: &Job{ + Spec: JobSpec{ + Parallelism: newInt32(2), + Template: v1.PodTemplateSpec{ + ObjectMeta: v1.ObjectMeta{Labels: defaultLabels}, + }, + }, + }, + expected: &Job{ + Spec: JobSpec{ + Parallelism: newInt32(2), + }, + }, + expectLabels: true, + }, + "Completions explicitly 2 and parallelism unset -> parallelism is defaulted": { + original: &Job{ + Spec: JobSpec{ + Completions: newInt32(2), + Template: v1.PodTemplateSpec{ + ObjectMeta: v1.ObjectMeta{Labels: defaultLabels}, + }, + }, + }, + expected: &Job{ + Spec: JobSpec{ + Completions: newInt32(2), + Parallelism: newInt32(1), + }, + }, + expectLabels: true, + }, + "Both set -> no change": { + original: &Job{ + Spec: JobSpec{ + Completions: newInt32(10), + Parallelism: newInt32(11), + Template: v1.PodTemplateSpec{ + ObjectMeta: v1.ObjectMeta{Labels: defaultLabels}, + }, + }, + }, + expected: &Job{ + Spec: JobSpec{ + Completions: newInt32(10), + Parallelism: newInt32(11), + Template: v1.PodTemplateSpec{ + ObjectMeta: v1.ObjectMeta{Labels: defaultLabels}, + }, + }, + }, + expectLabels: true, + }, + "Both set, flipped -> no change": { + original: &Job{ + Spec: JobSpec{ + Completions: newInt32(11), + Parallelism: newInt32(10), + Template: v1.PodTemplateSpec{ + ObjectMeta: v1.ObjectMeta{Labels: defaultLabels}, + }, + }, + }, + expected: &Job{ + Spec: JobSpec{ + Completions: newInt32(11), + Parallelism: newInt32(10), + }, + }, + expectLabels: true, + }, + } + + for name, test := range tests { + original := test.original + expected := test.expected + obj2 := roundTrip(t, runtime.Object(original)) + actual, ok := obj2.(*Job) + if !ok { + t.Errorf("%s: unexpected object: %v", name, actual) + t.FailNow() + } + if (actual.Spec.Completions == nil) != (expected.Spec.Completions == nil) { + t.Errorf("%s: got different *completions than expected: %v %v", name, actual.Spec.Completions, expected.Spec.Completions) + } + if actual.Spec.Completions != nil && expected.Spec.Completions != nil { + if *actual.Spec.Completions != *expected.Spec.Completions { + t.Errorf("%s: got different completions than expected: %d %d", name, *actual.Spec.Completions, *expected.Spec.Completions) + } + } + if (actual.Spec.Parallelism == nil) != (expected.Spec.Parallelism == nil) { + t.Errorf("%s: got different *Parallelism than expected: %v %v", name, actual.Spec.Parallelism, expected.Spec.Parallelism) + } + if actual.Spec.Parallelism != nil && expected.Spec.Parallelism != nil { + if *actual.Spec.Parallelism != *expected.Spec.Parallelism { + t.Errorf("%s: got different parallelism than expected: %d %d", name, *actual.Spec.Parallelism, *expected.Spec.Parallelism) + } + } + if test.expectLabels != reflect.DeepEqual(actual.Labels, actual.Spec.Template.Labels) { + if test.expectLabels { + t.Errorf("%s: expected: %v, got: %v", name, actual.Spec.Template.Labels, actual.Labels) + } else { + t.Errorf("%s: unexpected equality: %v", name, actual.Labels) + } + } + + } +} + +func roundTrip(t *testing.T, obj runtime.Object) runtime.Object { + data, err := runtime.Encode(api.Codecs.LegacyCodec(SchemeGroupVersion), obj) + if err != nil { + t.Errorf("%v\n %#v", err, obj) + return nil + } + obj2, err := runtime.Decode(api.Codecs.UniversalDecoder(), data) + if err != nil { + t.Errorf("%v\nData: %s\nSource: %#v", err, string(data), obj) + return nil + } + obj3 := reflect.New(reflect.TypeOf(obj).Elem()).Interface().(runtime.Object) + err = api.Scheme.Convert(obj2, obj3, nil) + if err != nil { + t.Errorf("%v\nSource: %#v", err, obj2) + return nil + } + return obj3 +} + +func newInt32(val int32) *int32 { + p := new(int32) + *p = val + return p +} diff --git a/pkg/apis/batch/validation/validation.go b/pkg/apis/batch/validation/validation.go index cdbdafc5b85..24d97a53134 100644 --- a/pkg/apis/batch/validation/validation.go +++ b/pkg/apis/batch/validation/validation.go @@ -49,14 +49,8 @@ func ValidateGeneratedSelector(obj *batch.Job) field.ErrorList { allErrs = append(allErrs, field.Required(field.NewPath("metadata").Child("uid"), "")) } - // If somehow uid was unset then we would get "controller-uid=" as the selector - // which is bad. - if obj.ObjectMeta.UID == "" { - allErrs = append(allErrs, field.Required(field.NewPath("metadata").Child("uid"), "")) - } - // If selector generation was requested, then expected labels must be - // present on pod template, and much match job's uid and name. The + // present on pod template, and must match job's uid and name. The // generated (not-manual) selectors/labels ensure no overlap with other // controllers. The manual mode allows orphaning, adoption, // backward-compatibility, and experimentation with new diff --git a/pkg/apis/extensions/v1beta1/defaults_test.go b/pkg/apis/extensions/v1beta1/defaults_test.go index 4a90f178215..b6d9747af65 100644 --- a/pkg/apis/extensions/v1beta1/defaults_test.go +++ b/pkg/apis/extensions/v1beta1/defaults_test.go @@ -280,15 +280,39 @@ func TestSetDefaultDeployment(t *testing.T) { } } -func TestSetDefaultJobParallelismAndCompletions(t *testing.T) { - tests := []struct { - original *Job - expected *Job +func TestSetDefaultJob(t *testing.T) { + defaultLabels := map[string]string{"default": "default"} + tests := map[string]struct { + original *Job + expected *Job + expectLabels bool }{ - // both unspecified -> sets both to 1 - { + "both unspecified -> sets both to 1": { original: &Job{ - Spec: JobSpec{}, + Spec: JobSpec{ + Template: v1.PodTemplateSpec{ + ObjectMeta: v1.ObjectMeta{Labels: defaultLabels}, + }, + }, + }, + expected: &Job{ + Spec: JobSpec{ + Completions: newInt32(1), + Parallelism: newInt32(1), + }, + }, + expectLabels: true, + }, + "both unspecified -> sets both to 1 and no default labels": { + original: &Job{ + ObjectMeta: v1.ObjectMeta{ + Labels: map[string]string{"mylabel": "myvalue"}, + }, + Spec: JobSpec{ + Template: v1.PodTemplateSpec{ + ObjectMeta: v1.ObjectMeta{Labels: defaultLabels}, + }, + }, }, expected: &Job{ Spec: JobSpec{ @@ -297,11 +321,13 @@ func TestSetDefaultJobParallelismAndCompletions(t *testing.T) { }, }, }, - // WQ: Parallelism explicitly 0 and completions unset -> no change - { + "WQ: Parallelism explicitly 0 and completions unset -> no change": { original: &Job{ Spec: JobSpec{ Parallelism: newInt32(0), + Template: v1.PodTemplateSpec{ + ObjectMeta: v1.ObjectMeta{Labels: defaultLabels}, + }, }, }, expected: &Job{ @@ -309,12 +335,15 @@ func TestSetDefaultJobParallelismAndCompletions(t *testing.T) { Parallelism: newInt32(0), }, }, + expectLabels: true, }, - // WQ: Parallelism explicitly 2 and completions unset -> no change - { + "WQ: Parallelism explicitly 2 and completions unset -> no change": { original: &Job{ Spec: JobSpec{ Parallelism: newInt32(2), + Template: v1.PodTemplateSpec{ + ObjectMeta: v1.ObjectMeta{Labels: defaultLabels}, + }, }, }, expected: &Job{ @@ -322,12 +351,15 @@ func TestSetDefaultJobParallelismAndCompletions(t *testing.T) { Parallelism: newInt32(2), }, }, + expectLabels: true, }, - // Completions explicitly 2 and parallelism unset -> parallelism is defaulted - { + "Completions explicitly 2 and parallelism unset -> parallelism is defaulted": { original: &Job{ Spec: JobSpec{ Completions: newInt32(2), + Template: v1.PodTemplateSpec{ + ObjectMeta: v1.ObjectMeta{Labels: defaultLabels}, + }, }, }, expected: &Job{ @@ -336,28 +368,37 @@ func TestSetDefaultJobParallelismAndCompletions(t *testing.T) { Parallelism: newInt32(1), }, }, + expectLabels: true, }, - // Both set -> no change - { + "Both set -> no change": { original: &Job{ Spec: JobSpec{ Completions: newInt32(10), Parallelism: newInt32(11), + Template: v1.PodTemplateSpec{ + ObjectMeta: v1.ObjectMeta{Labels: defaultLabels}, + }, }, }, expected: &Job{ Spec: JobSpec{ Completions: newInt32(10), Parallelism: newInt32(11), + Template: v1.PodTemplateSpec{ + ObjectMeta: v1.ObjectMeta{Labels: defaultLabels}, + }, }, }, + expectLabels: true, }, - // Both set, flipped -> no change - { + "Both set, flipped -> no change": { original: &Job{ Spec: JobSpec{ Completions: newInt32(11), Parallelism: newInt32(10), + Template: v1.PodTemplateSpec{ + ObjectMeta: v1.ObjectMeta{Labels: defaultLabels}, + }, }, }, expected: &Job{ @@ -366,32 +407,40 @@ func TestSetDefaultJobParallelismAndCompletions(t *testing.T) { Parallelism: newInt32(10), }, }, + expectLabels: true, }, } - for _, tc := range tests { - original := tc.original - expected := tc.expected + for name, test := range tests { + original := test.original + expected := test.expected obj2 := roundTrip(t, runtime.Object(original)) - got, ok := obj2.(*Job) + actual, ok := obj2.(*Job) if !ok { - t.Errorf("unexpected object: %v", got) + t.Errorf("%s: unexpected object: %v", name, actual) t.FailNow() } - if (got.Spec.Completions == nil) != (expected.Spec.Completions == nil) { - t.Errorf("got different *completions than expected: %v %v", got.Spec.Completions, expected.Spec.Completions) + if (actual.Spec.Completions == nil) != (expected.Spec.Completions == nil) { + t.Errorf("%s: got different *completions than expected: %v %v", name, actual.Spec.Completions, expected.Spec.Completions) } - if got.Spec.Completions != nil && expected.Spec.Completions != nil { - if *got.Spec.Completions != *expected.Spec.Completions { - t.Errorf("got different completions than expected: %d %d", *got.Spec.Completions, *expected.Spec.Completions) + if actual.Spec.Completions != nil && expected.Spec.Completions != nil { + if *actual.Spec.Completions != *expected.Spec.Completions { + t.Errorf("%s: got different completions than expected: %d %d", name, *actual.Spec.Completions, *expected.Spec.Completions) } } - if (got.Spec.Parallelism == nil) != (expected.Spec.Parallelism == nil) { - t.Errorf("got different *Parallelism than expected: %v %v", got.Spec.Parallelism, expected.Spec.Parallelism) + if (actual.Spec.Parallelism == nil) != (expected.Spec.Parallelism == nil) { + t.Errorf("%s: got different *Parallelism than expected: %v %v", name, actual.Spec.Parallelism, expected.Spec.Parallelism) } - if got.Spec.Parallelism != nil && expected.Spec.Parallelism != nil { - if *got.Spec.Parallelism != *expected.Spec.Parallelism { - t.Errorf("got different parallelism than expected: %d %d", *got.Spec.Parallelism, *expected.Spec.Parallelism) + if actual.Spec.Parallelism != nil && expected.Spec.Parallelism != nil { + if *actual.Spec.Parallelism != *expected.Spec.Parallelism { + t.Errorf("%s: got different parallelism than expected: %d %d", name, *actual.Spec.Parallelism, *expected.Spec.Parallelism) + } + } + if test.expectLabels != reflect.DeepEqual(actual.Labels, actual.Spec.Template.Labels) { + if test.expectLabels { + t.Errorf("%s: expected: %v, got: %v", name, actual.Spec.Template.Labels, actual.Labels) + } else { + t.Errorf("%s: unexpected equality: %v", name, actual.Labels) } } }