From 29c50cdc1adbab0ac70f4ebb2874e6de8062601e Mon Sep 17 00:00:00 2001 From: Mike Danese Date: Wed, 14 Oct 2015 11:03:59 -0700 Subject: [PATCH] plumb PodSelector through the api --- pkg/apis/extensions/helpers.go | 67 +++++++++++++++ pkg/apis/extensions/helpers_test.go | 83 +++++++++++++++++++ pkg/apis/extensions/types.go | 13 +-- pkg/apis/extensions/v1beta1/defaults.go | 6 +- pkg/apis/extensions/v1beta1/defaults_test.go | 8 +- pkg/apis/extensions/v1beta1/types.go | 39 ++++++++- pkg/apis/extensions/validation/validation.go | 45 ++++++++-- .../extensions/validation/validation_test.go | 12 +-- pkg/labels/selector.go | 12 +++ 9 files changed, 262 insertions(+), 23 deletions(-) create mode 100644 pkg/apis/extensions/helpers.go create mode 100644 pkg/apis/extensions/helpers_test.go diff --git a/pkg/apis/extensions/helpers.go b/pkg/apis/extensions/helpers.go new file mode 100644 index 00000000000..23e0abba673 --- /dev/null +++ b/pkg/apis/extensions/helpers.go @@ -0,0 +1,67 @@ +/* +Copyright 2015 The Kubernetes Authors All rights reserved. + +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 extensions + +import ( + "fmt" + + "sort" + + "k8s.io/kubernetes/pkg/labels" + "k8s.io/kubernetes/pkg/util/sets" +) + +// PodSelectorAsSelector converts the PodSelector api type into a struct that implements +// labels.Selector +func PodSelectorAsSelector(ps *PodSelector) (labels.Selector, error) { + if ps == nil { + return labels.Nothing(), nil + } + if len(ps.MatchLabels)+len(ps.MatchExpressions) == 0 { + return labels.Everything(), nil + } + selector := labels.LabelSelector{} + for k, v := range ps.MatchLabels { + req, err := labels.NewRequirement(k, labels.InOperator, sets.NewString(v)) + if err != nil { + return nil, err + } + selector = append(selector, *req) + } + for _, expr := range ps.MatchExpressions { + var op labels.Operator + switch expr.Operator { + case PodSelectorOpIn: + op = labels.InOperator + case PodSelectorOpNotIn: + op = labels.NotInOperator + case PodSelectorOpExists: + op = labels.ExistsOperator + case PodSelectorOpDoesNotExist: + op = labels.DoesNotExistOperator + default: + return nil, fmt.Errorf("%q is not a valid pod selector operator", expr.Operator) + } + req, err := labels.NewRequirement(expr.Key, op, sets.NewString(expr.Values...)) + if err != nil { + return nil, err + } + selector = append(selector, *req) + } + sort.Sort(labels.ByKey(selector)) + return selector, nil +} diff --git a/pkg/apis/extensions/helpers_test.go b/pkg/apis/extensions/helpers_test.go new file mode 100644 index 00000000000..866230bfcc7 --- /dev/null +++ b/pkg/apis/extensions/helpers_test.go @@ -0,0 +1,83 @@ +/* +Copyright 2015 The Kubernetes Authors All rights reserved. + +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 extensions + +import ( + "reflect" + "testing" + + "k8s.io/kubernetes/pkg/labels" +) + +func TestPodSelectorAsSelector(t *testing.T) { + matchLabels := map[string]string{"foo": "bar"} + matchExpressions := []PodSelectorRequirement{{ + Key: "baz", + Operator: PodSelectorOpIn, + Values: []string{"qux", "norf"}, + }} + mustParse := func(s string) labels.Selector { + out, e := labels.Parse(s) + if e != nil { + panic(e) + } + return out + } + tc := []struct { + in *PodSelector + out labels.Selector + expectErr bool + }{ + {in: nil, out: labels.Nothing()}, + {in: &PodSelector{}, out: labels.Everything()}, + { + in: &PodSelector{MatchLabels: matchLabels}, + out: mustParse("foo in (bar)"), + }, + { + in: &PodSelector{MatchExpressions: matchExpressions}, + out: mustParse("baz in (norf,qux)"), + }, + { + in: &PodSelector{MatchLabels: matchLabels, MatchExpressions: matchExpressions}, + out: mustParse("foo in (bar),baz in (norf,qux)"), + }, + { + in: &PodSelector{ + MatchExpressions: []PodSelectorRequirement{{ + Key: "baz", + Operator: PodSelectorOpExists, + Values: []string{"qux", "norf"}, + }}, + }, + expectErr: true, + }, + } + + for i, tc := range tc { + out, err := PodSelectorAsSelector(tc.in) + if err == nil && tc.expectErr { + t.Errorf("[%v]expected error but got none.", i) + } + if err != nil && !tc.expectErr { + t.Errorf("[%v]did not expect error but got: %v", i, err) + } + if !reflect.DeepEqual(out, tc.out) { + t.Errorf("[%v]expected:\n\t%+v\nbut got:\n\t%+v", i, tc.out, out) + } + } +} diff --git a/pkg/apis/extensions/types.go b/pkg/apis/extensions/types.go index 4c7f52f895f..9b73b607622 100644 --- a/pkg/apis/extensions/types.go +++ b/pkg/apis/extensions/types.go @@ -405,7 +405,7 @@ type JobSpec struct { Completions *int `json:"completions,omitempty"` // Selector is a label query over pods that should match the pod count. - Selector map[string]string `json:"selector"` + Selector *PodSelector `json:"selector,omitempty"` // Template is the object that describes the pod that will be created when // executing a job. @@ -661,7 +661,7 @@ type PodSelector struct { MatchExpressions []PodSelectorRequirement `json:"matchExpressions,omitempty"` } -// A pod selector requirement is a selector that contains values, a key and an operator that +// A pod selector requirement is a selector that contains values, a key, and an operator that // relates the key and values. type PodSelectorRequirement struct { // key is the label key that the selector applies to. @@ -669,10 +669,11 @@ type PodSelectorRequirement struct { // operator represents a key's relationship to a set of values. // Valid operators ard In, NotIn, Exists and DoesNotExist. Operator PodSelectorOperator `json:"operator"` - // values is a set of string values. If the operator is In or NotIn, - // the values set must be non-empty. This array is replaced during a - // strategic merge patch. - Values []string `json:"stringValues,omitempty"` + // values is an array of string values. If the operator is In or NotIn, + // the values array must be non-empty. If the operator is Exists or DoesNotExist, + // the values array must be empty. This array is replaced during a strategic + // merge patch. + Values []string `json:"values,omitempty"` } // A pod selector operator is the set of operators that can be used in a selector requirement. diff --git a/pkg/apis/extensions/v1beta1/defaults.go b/pkg/apis/extensions/v1beta1/defaults.go index 07c69158075..e1018ff416b 100644 --- a/pkg/apis/extensions/v1beta1/defaults.go +++ b/pkg/apis/extensions/v1beta1/defaults.go @@ -92,8 +92,10 @@ func addDefaultingFuncs() { labels := obj.Spec.Template.Labels // TODO: support templates defined elsewhere when we support them in the API if labels != nil { - if len(obj.Spec.Selector) == 0 { - obj.Spec.Selector = labels + if obj.Spec.Selector == nil { + obj.Spec.Selector = &PodSelector{ + MatchLabels: labels, + } } if len(obj.Labels) == 0 { obj.Labels = labels diff --git a/pkg/apis/extensions/v1beta1/defaults_test.go b/pkg/apis/extensions/v1beta1/defaults_test.go index 4eb9069b321..baab807924d 100644 --- a/pkg/apis/extensions/v1beta1/defaults_test.go +++ b/pkg/apis/extensions/v1beta1/defaults_test.go @@ -192,7 +192,9 @@ func TestSetDefaultDeployment(t *testing.T) { func TestSetDefaultJob(t *testing.T) { expected := &Job{ Spec: JobSpec{ - Selector: map[string]string{"job": "selector"}, + Selector: &PodSelector{ + MatchLabels: map[string]string{"job": "selector"}, + }, Completions: newInt(1), Parallelism: newInt(1), }, @@ -201,7 +203,9 @@ func TestSetDefaultJob(t *testing.T) { // selector set explicitly, completions and parallelism - default { Spec: JobSpec{ - Selector: map[string]string{"job": "selector"}, + Selector: &PodSelector{ + MatchLabels: map[string]string{"job": "selector"}, + }, }, }, // selector from template labels, completions and parallelism - default diff --git a/pkg/apis/extensions/v1beta1/types.go b/pkg/apis/extensions/v1beta1/types.go index 5b05de54a2f..250c939ba0d 100644 --- a/pkg/apis/extensions/v1beta1/types.go +++ b/pkg/apis/extensions/v1beta1/types.go @@ -412,7 +412,7 @@ type JobSpec struct { // Selector is a label query over pods that should match the pod count. // More info: http://releases.k8s.io/HEAD/docs/user-guide/labels.md#label-selectors - Selector map[string]string `json:"selector,omitempty"` + Selector *PodSelector `json:"selector,omitempty"` // Template is the object that describes the pod that will be created when // executing a job. @@ -657,3 +657,40 @@ type ClusterAutoscalerList struct { Items []ClusterAutoscaler `json:"items"` } + +// A pod selector is a label query over a set of pods. The result of matchLabels and +// matchExpressions are ANDed. An empty pod selector matches all objects. A null +// pod selector matches no objects. +type PodSelector struct { + // matchLabels is a map of {key,value} pairs. A single {key,value} in the matchLabels + // map is equivalent to an element of matchExpressions, whose key field is "key", the + // operator is "In", and the values array contains only "value". The requirements are ANDed. + MatchLabels map[string]string `json:"matchLabels,omitempty"` + // matchExpressions is a list of pod selector requirements. The requirements are ANDed. + MatchExpressions []PodSelectorRequirement `json:"matchExpressions,omitempty"` +} + +// A pod selector requirement is a selector that contains values, a key, and an operator that +// relates the key and values. +type PodSelectorRequirement struct { + // key is the label key that the selector applies to. + Key string `json:"key" patchStrategy:"merge" patchMergeKey:"key"` + // operator represents a key's relationship to a set of values. + // Valid operators ard In, NotIn, Exists and DoesNotExist. + Operator PodSelectorOperator `json:"operator"` + // values is an array of string values. If the operator is In or NotIn, + // the values array must be non-empty. If the operator is Exists or DoesNotExist, + // the values array must be empty. This array is replaced during a strategic + // merge patch. + Values []string `json:"values,omitempty"` +} + +// A pod selector operator is the set of operators that can be used in a selector requirement. +type PodSelectorOperator string + +const ( + PodSelectorOpIn PodSelectorOperator = "In" + PodSelectorOpNotIn PodSelectorOperator = "NotIn" + PodSelectorOpExists PodSelectorOperator = "Exists" + PodSelectorOpDoesNotExist PodSelectorOperator = "DoesNotExist" +) diff --git a/pkg/apis/extensions/validation/validation.go b/pkg/apis/extensions/validation/validation.go index a1b3f96f965..a9ba7d98e01 100644 --- a/pkg/apis/extensions/validation/validation.go +++ b/pkg/apis/extensions/validation/validation.go @@ -345,16 +345,19 @@ func ValidateJobSpec(spec *extensions.JobSpec) errs.ValidationErrorList { if spec.Completions != nil && *spec.Completions < 0 { allErrs = append(allErrs, errs.NewFieldInvalid("completions", spec.Completions, isNegativeErrorMsg)) } - - selector := labels.Set(spec.Selector).AsSelector() - if selector.Empty() { + if spec.Selector == nil { allErrs = append(allErrs, errs.NewFieldRequired("selector")) + } else { + allErrs = append(allErrs, ValidatePodSelector(spec.Selector).Prefix("selector")...) } - labels := labels.Set(spec.Template.Labels) - if !selector.Matches(labels) { - allErrs = append(allErrs, errs.NewFieldInvalid("template.labels", spec.Template.Labels, "selector does not match template")) + if selector, err := extensions.PodSelectorAsSelector(spec.Selector); err == nil { + labels := labels.Set(spec.Template.Labels) + if !selector.Matches(labels) { + allErrs = append(allErrs, errs.NewFieldInvalid("template.metadata.labels", spec.Template.Labels, "selector does not match template")) + } } + allErrs = append(allErrs, apivalidation.ValidatePodTemplateSpec(&spec.Template).Prefix("template")...) if spec.Template.Spec.RestartPolicy != api.RestartPolicyOnFailure && spec.Template.Spec.RestartPolicy != api.RestartPolicyNever { @@ -568,3 +571,33 @@ func ValidateClusterAutoscaler(autoscaler *extensions.ClusterAutoscaler) errs.Va allErrs = append(allErrs, validateClusterAutoscalerSpec(autoscaler.Spec)...) return allErrs } + +func ValidatePodSelector(ps *extensions.PodSelector) errs.ValidationErrorList { + allErrs := errs.ValidationErrorList{} + if ps == nil { + return allErrs + } + allErrs = append(allErrs, apivalidation.ValidateLabels(ps.MatchLabels, "matchLabels")...) + for i, expr := range ps.MatchExpressions { + allErrs = append(allErrs, ValidatePodSelectorRequirement(expr).Prefix(fmt.Sprintf("matchExpressions.[%v]", i))...) + } + return allErrs +} + +func ValidatePodSelectorRequirement(sr extensions.PodSelectorRequirement) errs.ValidationErrorList { + allErrs := errs.ValidationErrorList{} + switch sr.Operator { + case extensions.PodSelectorOpIn, extensions.PodSelectorOpNotIn: + if len(sr.Values) == 0 { + allErrs = append(allErrs, errs.NewFieldInvalid("values", sr.Values, "must be non-empty when operator is In or NotIn")) + } + case extensions.PodSelectorOpExists, extensions.PodSelectorOpDoesNotExist: + if len(sr.Values) > 0 { + allErrs = append(allErrs, errs.NewFieldInvalid("values", sr.Values, "must be empty when operator is Exists or DoesNotExist")) + } + default: + allErrs = append(allErrs, errs.NewFieldInvalid("operator", sr.Operator, "not a valid pod selector operator")) + } + allErrs = append(allErrs, apivalidation.ValidateLabelName(sr.Key, "key")...) + return allErrs +} diff --git a/pkg/apis/extensions/validation/validation_test.go b/pkg/apis/extensions/validation/validation_test.go index ddbfa0f881b..f1654309432 100644 --- a/pkg/apis/extensions/validation/validation_test.go +++ b/pkg/apis/extensions/validation/validation_test.go @@ -192,7 +192,6 @@ func TestValidateDaemonSetStatusUpdate(t *testing.T) { t.Errorf("expected failure: %s", testName) } } - } func TestValidateDaemonSetUpdate(t *testing.T) { @@ -725,10 +724,12 @@ func TestValidateDeployment(t *testing.T) { } func TestValidateJob(t *testing.T) { - validSelector := map[string]string{"a": "b"} + validSelector := &extensions.PodSelector{ + MatchLabels: map[string]string{"a": "b"}, + } validPodTemplateSpec := api.PodTemplateSpec{ ObjectMeta: api.ObjectMeta{ - Labels: validSelector, + Labels: validSelector.MatchLabels, }, Spec: api.PodSpec{ RestartPolicy: api.RestartPolicyOnFailure, @@ -783,11 +784,10 @@ func TestValidateJob(t *testing.T) { Namespace: api.NamespaceDefault, }, Spec: extensions.JobSpec{ - Selector: map[string]string{}, Template: validPodTemplateSpec, }, }, - "spec.template.labels:selector does not match template": { + "spec.template.metadata.labels: invalid value 'map[y:z]', Details: selector does not match template": { ObjectMeta: api.ObjectMeta{ Name: "myjob", Namespace: api.NamespaceDefault, @@ -815,7 +815,7 @@ func TestValidateJob(t *testing.T) { Selector: validSelector, Template: api.PodTemplateSpec{ ObjectMeta: api.ObjectMeta{ - Labels: validSelector, + Labels: validSelector.MatchLabels, }, Spec: api.PodSpec{ RestartPolicy: api.RestartPolicyAlways, diff --git a/pkg/labels/selector.go b/pkg/labels/selector.go index fabf0f1ccd3..8f6a3b5e64e 100644 --- a/pkg/labels/selector.go +++ b/pkg/labels/selector.go @@ -47,6 +47,18 @@ func Everything() Selector { return LabelSelector{} } +type nothingSelector struct{} + +func (n nothingSelector) Matches(_ Labels) bool { return false } +func (n nothingSelector) Empty() bool { return false } +func (n nothingSelector) String() string { return "" } +func (n nothingSelector) Add(_ string, _ Operator, _ []string) Selector { return n } + +// Nothing returns a selector that matches no labels +func Nothing() Selector { + return nothingSelector{} +} + // Operator represents a key's relationship // to a set of values in a Requirement. type Operator string