diff --git a/examples/examples_test.go b/examples/examples_test.go index fa77d676a49..c31109f424e 100644 --- a/examples/examples_test.go +++ b/examples/examples_test.go @@ -32,7 +32,9 @@ import ( "k8s.io/kubernetes/pkg/apis/extensions" expvalidation "k8s.io/kubernetes/pkg/apis/extensions/validation" "k8s.io/kubernetes/pkg/capabilities" + "k8s.io/kubernetes/pkg/registry/job" "k8s.io/kubernetes/pkg/runtime" + "k8s.io/kubernetes/pkg/types" "k8s.io/kubernetes/pkg/util/validation/field" "k8s.io/kubernetes/pkg/util/yaml" schedulerapi "k8s.io/kubernetes/plugin/pkg/scheduler/api" @@ -111,7 +113,10 @@ func validateObject(obj runtime.Object) (errors field.ErrorList) { if t.Namespace == "" { t.Namespace = api.NamespaceDefault } - errors = expvalidation.ValidateJob(t) + // Job needs generateSelector called before validation, and job.Validate does this. + // See: https://github.com/kubernetes/kubernetes/issues/20951#issuecomment-187787040 + t.ObjectMeta.UID = types.UID("fakeuid") + errors = job.Strategy.Validate(nil, t) case *extensions.Ingress: if t.Namespace == "" { t.Namespace = api.NamespaceDefault diff --git a/pkg/api/testing/fuzzer.go b/pkg/api/testing/fuzzer.go index 092802d111f..1807dd5c004 100644 --- a/pkg/api/testing/fuzzer.go +++ b/pkg/api/testing/fuzzer.go @@ -162,6 +162,11 @@ func FuzzerFor(t *testing.T, version unversioned.GroupVersion, src rand.Source) parallelism := int(c.Rand.Int31()) j.Completions = &completions j.Parallelism = ¶llelism + if c.Rand.Int31()%2 == 0 { + j.ManualSelector = newBool(true) + } else { + j.ManualSelector = nil + } }, func(j *api.List, c fuzz.Continue) { c.FuzzNoCustom(j) // fuzz self without calling this function again @@ -411,3 +416,9 @@ func FuzzerFor(t *testing.T, version unversioned.GroupVersion, src rand.Source) ) return f } + +func newBool(val bool) *bool { + p := new(bool) + *p = val + return p +} diff --git a/pkg/apis/batch/v1/defaults.go b/pkg/apis/batch/v1/defaults.go index e4e592b9681..759ab0fb6de 100644 --- a/pkg/apis/batch/v1/defaults.go +++ b/pkg/apis/batch/v1/defaults.go @@ -23,18 +23,6 @@ import ( func addDefaultingFuncs(scheme *runtime.Scheme) { scheme.AddDefaultingFuncs( func(obj *Job) { - 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 { - obj.Spec.Selector = &LabelSelector{ - MatchLabels: labels, - } - } - if len(obj.Labels) == 0 { - obj.Labels = labels - } - } // For a non-parallel job, you can leave both `.spec.completions` and // `.spec.parallelism` unset. When both are unset, both are defaulted to 1. if obj.Spec.Completions == nil && obj.Spec.Parallelism == nil { diff --git a/pkg/apis/batch/v1/types.go b/pkg/apis/batch/v1/types.go index 87e960104b1..9dfe0d3dcf9 100644 --- a/pkg/apis/batch/v1/types.go +++ b/pkg/apis/batch/v1/types.go @@ -63,6 +63,7 @@ type JobSpec struct { // pod signals the success of all pods, and allows parallelism to have any positive // value. Setting to 1 means that parallelism is limited to 1 and the success of that // pod signals the success of the job. + // More info: http://releases.k8s.io/HEAD/docs/user-guide/jobs.md Completions *int32 `json:"completions,omitempty"` // Optional duration in seconds relative to the startTime that the job may be active @@ -70,9 +71,22 @@ type JobSpec struct { ActiveDeadlineSeconds *int64 `json:"activeDeadlineSeconds,omitempty"` // Selector is a label query over pods that should match the pod count. + // Normally, the system sets this field for you. // More info: http://releases.k8s.io/HEAD/docs/user-guide/labels.md#label-selectors Selector *LabelSelector `json:"selector,omitempty"` + // ManualSelector controls generation of pod labels and pod selectors. + // Leave `manualSelector` unset unless you are certain what you are doing. + // When false or unset, the system pick labels unique to this job + // and appends those labels to the pod template. When true, + // the user is responsible for picking unique labels and specifying + // the selector. Failure to pick a unique label may cause this + // and other jobs to not function correctly. However, You may see + // `manualSelector=true` in jobs that were created with the old `extensions/v1beta1` + // API. + // More info: http://releases.k8s.io/HEAD/docs/design/selector-generation.md + ManualSelector *bool `json:"manualSelector,omitempty"` + // Template is the object that describes the pod that will be created when // executing a job. // More info: http://releases.k8s.io/HEAD/docs/user-guide/jobs.md diff --git a/pkg/apis/extensions/types.go b/pkg/apis/extensions/types.go index 6b46cb6074e..15377083e98 100644 --- a/pkg/apis/extensions/types.go +++ b/pkg/apis/extensions/types.go @@ -535,7 +535,10 @@ type JobSpec struct { Parallelism *int `json:"parallelism,omitempty"` // Completions specifies the desired number of successfully finished pods the - // job should be run with. When unset, any pod exiting signals the job to complete. + // job should be run with. Setting to nil means that the success of any + // pod signals the success of all pods, and allows parallelism to have any positive + // value. Setting to 1 means that parallelism is limited to 1 and the success of that + // pod signals the success of the job. Completions *int `json:"completions,omitempty"` // Optional duration in seconds relative to the startTime that the job may be active @@ -543,8 +546,20 @@ type JobSpec struct { ActiveDeadlineSeconds *int64 `json:"activeDeadlineSeconds,omitempty"` // Selector is a label query over pods that should match the pod count. + // Normally, the system sets this field for you. Selector *unversioned.LabelSelector `json:"selector,omitempty"` + // ManualSelector controls generation of pod labels and pod selectors. + // Leave `manualSelector` unset unless you are certain what you are doing. + // When false or unset, the system pick labels unique to this job + // and appends those labels to the pod template. When true, + // the user is responsible for picking unique labels and specifying + // the selector. Failure to pick a unique label may cause this + // and other jobs to not function correctly. However, You may see + // `manualSelector=true` in jobs that were created with the old `extensions/v1beta1` + // API. + ManualSelector *bool `json:"manualSelector,omitempty"` + // Template is the object that describes the pod that will be created when // executing a job. Template api.PodTemplateSpec `json:"template"` diff --git a/pkg/apis/extensions/v1beta1/types.go b/pkg/apis/extensions/v1beta1/types.go index 9de6cb1f395..19895d90eb2 100644 --- a/pkg/apis/extensions/v1beta1/types.go +++ b/pkg/apis/extensions/v1beta1/types.go @@ -543,6 +543,7 @@ type JobSpec struct { // pod signals the success of all pods, and allows parallelism to have any positive // value. Setting to 1 means that parallelism is limited to 1 and the success of that // pod signals the success of the job. + // More info: http://releases.k8s.io/HEAD/docs/user-guide/jobs.md Completions *int32 `json:"completions,omitempty"` // Optional duration in seconds relative to the startTime that the job may be active @@ -550,9 +551,17 @@ type JobSpec struct { ActiveDeadlineSeconds *int64 `json:"activeDeadlineSeconds,omitempty"` // Selector is a label query over pods that should match the pod count. + // Normally, the system sets this field for you. // More info: http://releases.k8s.io/HEAD/docs/user-guide/labels.md#label-selectors Selector *LabelSelector `json:"selector,omitempty"` + // AutoSelector controls generation of pod labels and pod selectors. + // It was not present in the original extensions/v1beta1 Job definition, but exists + // to allow conversion from batch/v1 Jobs, where it corresponds to, but has the opposite + // meaning as, ManualSelector. + // More info: http://releases.k8s.io/HEAD/docs/design/selector-generation.md + AutoSelector *bool `json:"autoSelector,omitempty"` + // Template is the object that describes the pod that will be created when // executing a job. // More info: http://releases.k8s.io/HEAD/docs/user-guide/jobs.md diff --git a/pkg/apis/extensions/validation/validation.go b/pkg/apis/extensions/validation/validation.go index 157dc87dacc..764a5fc3ab9 100644 --- a/pkg/apis/extensions/validation/validation.go +++ b/pkg/apis/extensions/validation/validation.go @@ -379,9 +379,62 @@ func ValidateThirdPartyResourceData(obj *extensions.ThirdPartyResourceData) fiel return allErrs } +// TODO: generalize for other controller objects that will follow the same pattern, such as ReplicaSet and DaemonSet, and +// move to new location. Replace extensions.Job with an interface. +// +// ValidateGeneratedSelector validates that the generated selector on a controller object match the controller object +// metadata, and the labels on the pod template are as generated. +func ValidateGeneratedSelector(obj *extensions.Job) field.ErrorList { + allErrs := field.ErrorList{} + if obj.Spec.ManualSelector != nil && *obj.Spec.ManualSelector { + return allErrs + } + + if obj.Spec.Selector == nil { + return allErrs // This case should already have been checked in caller. No need for more errors. + } + + // 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 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 + // generated (not-manual) selectors/labels ensure no overlap with other + // controllers. The manual mode allows orphaning, adoption, + // backward-compatibility, and experimentation with new + // labeling/selection schemes. Automatic selector generation should + // have placed certain labels on the pod, but this could have failed if + // the user added coflicting labels. Validate that the expected + // generated ones are there. + + allErrs = append(allErrs, apivalidation.ValidateHasLabel(obj.Spec.Template.ObjectMeta, field.NewPath("spec").Child("template").Child("metadata"), "controller-uid", string(obj.UID))...) + allErrs = append(allErrs, apivalidation.ValidateHasLabel(obj.Spec.Template.ObjectMeta, field.NewPath("spec").Child("template").Child("metadata"), "job-name", string(obj.Name))...) + expectedLabels := make(map[string]string) + expectedLabels["controller-uid"] = string(obj.UID) + expectedLabels["job-name"] = string(obj.Name) + // Whether manually or automatically generated, the selector of the job must match the pods it will produce. + if selector, err := unversioned.LabelSelectorAsSelector(obj.Spec.Selector); err == nil { + if !selector.Matches(labels.Set(expectedLabels)) { + allErrs = append(allErrs, field.Invalid(field.NewPath("spec").Child("selector"), obj.Spec.Selector, "`selector` not auto-generated")) + } + } + + return allErrs +} + func ValidateJob(job *extensions.Job) field.ErrorList { // Jobs and rcs have the same name validation allErrs := apivalidation.ValidateObjectMeta(&job.ObjectMeta, true, apivalidation.ValidateReplicationControllerName, field.NewPath("metadata")) + allErrs = append(allErrs, ValidateGeneratedSelector(job)...) allErrs = append(allErrs, ValidateJobSpec(&job.Spec, field.NewPath("spec"))...) return allErrs } @@ -404,6 +457,7 @@ func ValidateJobSpec(spec *extensions.JobSpec, fldPath *field.Path) field.ErrorL allErrs = append(allErrs, unversionedvalidation.ValidateLabelSelector(spec.Selector, fldPath.Child("selector"))...) } + // Whether manually or automatically generated, the selector of the job must match the pods it will produce. if selector, err := unversioned.LabelSelectorAsSelector(spec.Selector); err == nil { labels := labels.Set(spec.Template.Labels) if !selector.Matches(labels) { diff --git a/pkg/apis/extensions/validation/validation_test.go b/pkg/apis/extensions/validation/validation_test.go index c421bdd404f..ff88893b91f 100644 --- a/pkg/apis/extensions/validation/validation_test.go +++ b/pkg/apis/extensions/validation/validation_test.go @@ -25,6 +25,7 @@ import ( "k8s.io/kubernetes/pkg/api/unversioned" "k8s.io/kubernetes/pkg/apis/extensions" "k8s.io/kubernetes/pkg/controller/podautoscaler" + "k8s.io/kubernetes/pkg/types" "k8s.io/kubernetes/pkg/util/intstr" ) @@ -975,12 +976,15 @@ func TestValidateDeploymentRollback(t *testing.T) { } func TestValidateJob(t *testing.T) { - validSelector := &unversioned.LabelSelector{ + validManualSelector := &unversioned.LabelSelector{ MatchLabels: map[string]string{"a": "b"}, } - validPodTemplateSpec := api.PodTemplateSpec{ + validGeneratedSelector := &unversioned.LabelSelector{ + MatchLabels: map[string]string{"collection-uid": "1a2b3c"}, + } + validPodTemplateSpecForManual := api.PodTemplateSpec{ ObjectMeta: api.ObjectMeta{ - Labels: validSelector.MatchLabels, + Labels: validManualSelector.MatchLabels, }, Spec: api.PodSpec{ RestartPolicy: api.RestartPolicyOnFailure, @@ -988,21 +992,45 @@ func TestValidateJob(t *testing.T) { Containers: []api.Container{{Name: "abc", Image: "image", ImagePullPolicy: "IfNotPresent"}}, }, } - successCases := []extensions.Job{ - { + validPodTemplateSpecForGenerated := api.PodTemplateSpec{ + ObjectMeta: api.ObjectMeta{ + Labels: validGeneratedSelector.MatchLabels, + }, + Spec: api.PodSpec{ + RestartPolicy: api.RestartPolicyOnFailure, + DNSPolicy: api.DNSClusterFirst, + Containers: []api.Container{{Name: "abc", Image: "image", ImagePullPolicy: "IfNotPresent"}}, + }, + } + successCases := map[string]extensions.Job{ + "manual selector": { ObjectMeta: api.ObjectMeta{ Name: "myjob", Namespace: api.NamespaceDefault, + UID: types.UID("1a2b3c"), }, Spec: extensions.JobSpec{ - Selector: validSelector, - Template: validPodTemplateSpec, + Selector: validManualSelector, + ManualSelector: newBool(true), + Template: validPodTemplateSpecForManual, + }, + }, + "generated selector": { + ObjectMeta: api.ObjectMeta{ + Name: "myjob", + Namespace: api.NamespaceDefault, + UID: types.UID("1a2b3c"), + }, + Spec: extensions.JobSpec{ + Selector: validGeneratedSelector, + ManualSelector: newBool(true), + Template: validPodTemplateSpecForGenerated, }, }, } - for _, successCase := range successCases { - if errs := ValidateJob(&successCase); len(errs) != 0 { - t.Errorf("expected success: %v", errs) + for k, v := range successCases { + if errs := ValidateJob(&v); len(errs) != 0 { + t.Errorf("expected success for %s: %v", k, errs) } } negative := -1 @@ -1012,51 +1040,59 @@ func TestValidateJob(t *testing.T) { ObjectMeta: api.ObjectMeta{ Name: "myjob", Namespace: api.NamespaceDefault, + UID: types.UID("1a2b3c"), }, Spec: extensions.JobSpec{ - Parallelism: &negative, - Selector: validSelector, - Template: validPodTemplateSpec, + Parallelism: &negative, + ManualSelector: newBool(true), + Template: validPodTemplateSpecForGenerated, }, }, "spec.completions:must be greater than or equal to 0": { ObjectMeta: api.ObjectMeta{ Name: "myjob", Namespace: api.NamespaceDefault, + UID: types.UID("1a2b3c"), }, Spec: extensions.JobSpec{ - Completions: &negative, - Selector: validSelector, - Template: validPodTemplateSpec, + Completions: &negative, + Selector: validManualSelector, + ManualSelector: newBool(true), + Template: validPodTemplateSpecForGenerated, }, }, "spec.activeDeadlineSeconds:must be greater than or equal to 0": { ObjectMeta: api.ObjectMeta{ Name: "myjob", Namespace: api.NamespaceDefault, + UID: types.UID("1a2b3c"), }, Spec: extensions.JobSpec{ ActiveDeadlineSeconds: &negative64, - Selector: validSelector, - Template: validPodTemplateSpec, + Selector: validManualSelector, + ManualSelector: newBool(true), + Template: validPodTemplateSpecForGenerated, }, }, "spec.selector:Required value": { ObjectMeta: api.ObjectMeta{ Name: "myjob", Namespace: api.NamespaceDefault, + UID: types.UID("1a2b3c"), }, Spec: extensions.JobSpec{ - Template: validPodTemplateSpec, + Template: validPodTemplateSpecForGenerated, }, }, "spec.template.metadata.labels: Invalid value: {\"y\":\"z\"}: `selector` does not match template `labels`": { ObjectMeta: api.ObjectMeta{ Name: "myjob", Namespace: api.NamespaceDefault, + UID: types.UID("1a2b3c"), }, Spec: extensions.JobSpec{ - Selector: validSelector, + Selector: validManualSelector, + ManualSelector: newBool(true), Template: api.PodTemplateSpec{ ObjectMeta: api.ObjectMeta{ Labels: map[string]string{"y": "z"}, @@ -1069,16 +1105,39 @@ func TestValidateJob(t *testing.T) { }, }, }, + "spec.template.metadata.labels: Invalid value: {\"controller-uid\":\"4d5e6f\"}: `selector` does not match template `labels`": { + ObjectMeta: api.ObjectMeta{ + Name: "myjob", + Namespace: api.NamespaceDefault, + UID: types.UID("1a2b3c"), + }, + Spec: extensions.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"}}, + }, + }, + }, + }, "spec.template.spec.restartPolicy: Unsupported value": { ObjectMeta: api.ObjectMeta{ Name: "myjob", Namespace: api.NamespaceDefault, + UID: types.UID("1a2b3c"), }, Spec: extensions.JobSpec{ - Selector: validSelector, + Selector: validManualSelector, + ManualSelector: newBool(true), Template: api.PodTemplateSpec{ ObjectMeta: api.ObjectMeta{ - Labels: validSelector.MatchLabels, + Labels: validManualSelector.MatchLabels, }, Spec: api.PodSpec{ RestartPolicy: api.RestartPolicyAlways, @@ -2070,3 +2129,9 @@ func TestValidatePodSecurityPolicy(t *testing.T) { } } } + +func newBool(val bool) *bool { + p := new(bool) + *p = val + return p +} diff --git a/pkg/kubectl/run.go b/pkg/kubectl/run.go index e0ed8ada89f..8cf4325b9c2 100644 --- a/pkg/kubectl/run.go +++ b/pkg/kubectl/run.go @@ -269,6 +269,7 @@ func (JobV1Beta1) Generate(genericParams map[string]interface{}) (runtime.Object Selector: &unversioned.LabelSelector{ MatchLabels: labels, }, + ManualSelector: newBool(true), Template: api.PodTemplateSpec{ ObjectMeta: api.ObjectMeta{ Labels: labels, @@ -607,3 +608,9 @@ func parseEnvs(envArray []string) ([]api.EnvVar, error) { } return envs, nil } + +func newBool(val bool) *bool { + p := new(bool) + *p = val + return p +} diff --git a/pkg/kubectl/run_test.go b/pkg/kubectl/run_test.go index 27a366c418b..46ca5984eb9 100644 --- a/pkg/kubectl/run_test.go +++ b/pkg/kubectl/run_test.go @@ -749,6 +749,7 @@ func TestGenerateJob(t *testing.T) { Selector: &unversioned.LabelSelector{ MatchLabels: map[string]string{"foo": "bar", "baz": "blah"}, }, + ManualSelector: newBool(true), Template: api.PodTemplateSpec{ ObjectMeta: api.ObjectMeta{ Labels: map[string]string{"foo": "bar", "baz": "blah"}, diff --git a/pkg/registry/job/etcd/etcd_test.go b/pkg/registry/job/etcd/etcd_test.go index 623125e8a66..9f52ccf8a19 100644 --- a/pkg/registry/job/etcd/etcd_test.go +++ b/pkg/registry/job/etcd/etcd_test.go @@ -53,6 +53,7 @@ func validNewJob() *extensions.Job { Selector: &unversioned.LabelSelector{ MatchLabels: map[string]string{"a": "b"}, }, + ManualSelector: newBool(true), Template: api.PodTemplateSpec{ ObjectMeta: api.ObjectMeta{ Labels: map[string]string{"a": "b"}, @@ -165,3 +166,9 @@ func TestWatch(t *testing.T) { } // TODO: test update /status + +func newBool(val bool) *bool { + p := new(bool) + *p = val + return p +} diff --git a/pkg/registry/job/strategy.go b/pkg/registry/job/strategy.go index 807dc078a42..aef9eebf205 100644 --- a/pkg/registry/job/strategy.go +++ b/pkg/registry/job/strategy.go @@ -21,6 +21,7 @@ import ( "strconv" "k8s.io/kubernetes/pkg/api" + "k8s.io/kubernetes/pkg/api/unversioned" "k8s.io/kubernetes/pkg/apis/extensions" "k8s.io/kubernetes/pkg/apis/extensions/validation" "k8s.io/kubernetes/pkg/fields" @@ -60,9 +61,63 @@ func (jobStrategy) PrepareForUpdate(obj, old runtime.Object) { // Validate validates a new job. func (jobStrategy) Validate(ctx api.Context, obj runtime.Object) field.ErrorList { job := obj.(*extensions.Job) + // TODO: move UID generation earlier and do this in defaulting logic? + if job.Spec.ManualSelector == nil || *job.Spec.ManualSelector == false { + generateSelector(job) + } return validation.ValidateJob(job) } +// generateSelector adds a selector to a job and labels to its template +// which can be used to uniquely identify the pods created by that job, +// if the user has requested this behavior. +func generateSelector(obj *extensions.Job) { + if obj.Spec.Template.Labels == nil { + obj.Spec.Template.Labels = make(map[string]string) + } + // The job-name label is unique except in cases that are expected to be + // quite uncommon, and is more user friendly than uid. So, we add it as + // a label. + _, found := obj.Spec.Template.Labels["job-name"] + if found { + // User asked us to not automatically generate a selector and labels, + // but set a possibly conflicting value. If there is a conflict, + // we will reject in validation. + } else { + obj.Spec.Template.Labels["job-name"] = string(obj.ObjectMeta.Name) + } + // The controller-uid label makes the pods that belong to this job + // only match this job. + _, found = obj.Spec.Template.Labels["controller-uid"] + if found { + // User asked us to automatically generate a selector and labels, + // but set a possibly conflicting value. If there is a conflict, + // we will reject in validation. + } else { + obj.Spec.Template.Labels["controller-uid"] = string(obj.ObjectMeta.UID) + } + // Select the controller-uid label. This is sufficient for uniqueness. + if obj.Spec.Selector == nil { + obj.Spec.Selector = &unversioned.LabelSelector{} + } + if obj.Spec.Selector.MatchLabels == nil { + obj.Spec.Selector.MatchLabels = make(map[string]string) + } + if _, found := obj.Spec.Selector.MatchLabels["controller-uid"]; !found { + obj.Spec.Selector.MatchLabels["controller-uid"] = string(obj.ObjectMeta.UID) + } + // If the user specified matchLabel controller-uid=$WRONGUID, then it should fail + // in validation, either because the selector does not match the pod template + // (controller-uid=$WRONGUID does not match controller-uid=$UID, which we applied + // above, or we will reject in validation because the template has the wrong + // labels. +} + +// TODO: generalize generateSelector so it can work for other controller +// objects such as ReplicaSet. Can use pkg/api/meta to generically get the +// UID, but need some way to generically access the selector and pod labels +// fields. + // Canonicalize normalizes the object after validation. func (jobStrategy) Canonicalize(obj runtime.Object) { } diff --git a/pkg/registry/job/strategy_test.go b/pkg/registry/job/strategy_test.go index 924e363d1ad..13eb6c58279 100644 --- a/pkg/registry/job/strategy_test.go +++ b/pkg/registry/job/strategy_test.go @@ -17,6 +17,7 @@ limitations under the License. package job import ( + "reflect" "testing" "k8s.io/kubernetes/pkg/api" @@ -25,8 +26,15 @@ import ( "k8s.io/kubernetes/pkg/api/unversioned" "k8s.io/kubernetes/pkg/apis/extensions" "k8s.io/kubernetes/pkg/labels" + "k8s.io/kubernetes/pkg/types" ) +func newBool(a bool) *bool { + r := new(bool) + *r = a + return r +} + func TestJobStrategy(t *testing.T) { ctx := api.NewDefaultContext() if !Strategy.NamespaceScoped() { @@ -55,8 +63,9 @@ func TestJobStrategy(t *testing.T) { Namespace: api.NamespaceDefault, }, Spec: extensions.JobSpec{ - Selector: validSelector, - Template: validPodTemplateSpec, + Selector: validSelector, + Template: validPodTemplateSpec, + ManualSelector: newBool(true), }, Status: extensions.JobStatus{ Active: 11, @@ -93,6 +102,56 @@ func TestJobStrategy(t *testing.T) { } } +func TestJobStrategyWithGeneration(t *testing.T) { + ctx := api.NewDefaultContext() + + theUID := types.UID("1a2b3c4d5e6f7g8h9i0k") + + validPodTemplateSpec := api.PodTemplateSpec{ + Spec: api.PodSpec{ + RestartPolicy: api.RestartPolicyOnFailure, + DNSPolicy: api.DNSClusterFirst, + Containers: []api.Container{{Name: "abc", Image: "image", ImagePullPolicy: "IfNotPresent"}}, + }, + } + job := &extensions.Job{ + ObjectMeta: api.ObjectMeta{ + Name: "myjob2", + Namespace: api.NamespaceDefault, + UID: theUID, + }, + Spec: extensions.JobSpec{ + Selector: nil, + Template: validPodTemplateSpec, + }, + } + + Strategy.PrepareForCreate(job) + errs := Strategy.Validate(ctx, job) + if len(errs) != 0 { + t.Errorf("Unexpected error validating %v", errs) + } + + // Validate the stuff that validation should have validated. + if job.Spec.Selector == nil { + t.Errorf("Selector not generated") + } + expectedLabels := make(map[string]string) + expectedLabels["controller-uid"] = string(theUID) + if !reflect.DeepEqual(job.Spec.Selector.MatchLabels, expectedLabels) { + t.Errorf("Expected label selector not generated") + } + if job.Spec.Template.ObjectMeta.Labels == nil { + t.Errorf("Expected template labels not generated") + } + if v, ok := job.Spec.Template.ObjectMeta.Labels["job-name"]; !ok || v != "myjob2" { + t.Errorf("Expected template labels not present") + } + if v, ok := job.Spec.Template.ObjectMeta.Labels["controller-uid"]; !ok || v != string(theUID) { + t.Errorf("Expected template labels not present: ok: %v, v: %v", ok, v) + } +} + func TestJobStatusStrategy(t *testing.T) { ctx := api.NewDefaultContext() if !StatusStrategy.NamespaceScoped() { diff --git a/test/e2e/job.go b/test/e2e/job.go index bf20cf6a22b..9c9a8b02153 100644 --- a/test/e2e/job.go +++ b/test/e2e/job.go @@ -204,8 +204,9 @@ func newTestJob(behavior, name string, rPol api.RestartPolicy, parallelism, comp Name: name, }, Spec: extensions.JobSpec{ - Parallelism: ¶llelism, - Completions: &completions, + Parallelism: ¶llelism, + Completions: &completions, + ManualSelector: newBool(true), Template: api.PodTemplateSpec{ ObjectMeta: api.ObjectMeta{ Labels: map[string]string{jobSelectorKey: name}, @@ -312,3 +313,9 @@ func waitForJobFail(c *client.Client, ns, jobName string) error { return false, nil }) } + +func newBool(val bool) *bool { + p := new(bool) + *p = val + return p +} diff --git a/test/integration/master_test.go b/test/integration/master_test.go index 1a19d0b2b61..8c62236d14a 100644 --- a/test/integration/master_test.go +++ b/test/integration/master_test.go @@ -204,26 +204,15 @@ var jobV1 string = ` "kind": "Job", "apiVersion": "batch/v1", "metadata": { - "name": "pi", - "labels": { - "app": "pi" - } + "name": "pi" }, "spec": { "parallelism": 1, "completions": 1, - "selector": { - "matchLabels": { - "app": "pi" - } - }, "template": { "metadata": { "name": "pi", - "creationTimestamp": null, - "labels": { - "app": "pi" - } + "creationTimestamp": null }, "spec": { "containers": [