From 8bfc99475f1136d418444776200bf2c6c27963b0 Mon Sep 17 00:00:00 2001 From: "Da K. Ma" Date: Sat, 21 Apr 2018 12:21:18 +0800 Subject: [PATCH] Added MatchFields to NodeSelectorTerm. Signed-off-by: Da K. Ma --- pkg/apis/core/helper/helpers.go | 32 +++ pkg/apis/core/types.go | 7 +- pkg/apis/core/v1/helper/helpers.go | 66 +++++ pkg/apis/core/v1/helper/helpers_test.go | 230 ++++++++++++++++++ pkg/apis/core/validation/validation.go | 38 +++ pkg/apis/core/validation/validation_test.go | 84 +++++++ staging/src/k8s.io/api/core/v1/types.go | 11 +- .../apimachinery/pkg/fields/selector.go | 21 ++ 8 files changed, 484 insertions(+), 5 deletions(-) diff --git a/pkg/apis/core/helper/helpers.go b/pkg/apis/core/helper/helpers.go index 252ea2c9c77..7f0523579ef 100644 --- a/pkg/apis/core/helper/helpers.go +++ b/pkg/apis/core/helper/helpers.go @@ -412,6 +412,38 @@ func NodeSelectorRequirementsAsSelector(nsm []core.NodeSelectorRequirement) (lab return selector, nil } +// NodeSelectorRequirementsAsFieldSelector converts the []NodeSelectorRequirement core type into a struct that implements +// fields.Selector. +func NodeSelectorRequirementsAsFieldSelector(nsm []core.NodeSelectorRequirement) (fields.Selector, error) { + if len(nsm) == 0 { + return fields.Nothing(), nil + } + + selectors := []fields.Selector{} + for _, expr := range nsm { + switch expr.Operator { + case core.NodeSelectorOpIn: + if len(expr.Values) != 1 { + return nil, fmt.Errorf("unexpected number of value (%d) for node field selector operator %q", + len(expr.Values), expr.Operator) + } + selectors = append(selectors, fields.OneTermEqualSelector(expr.Key, expr.Values[0])) + + case core.NodeSelectorOpNotIn: + if len(expr.Values) != 1 { + return nil, fmt.Errorf("unexpected number of value (%d) for node field selector operator %q", + len(expr.Values), expr.Operator) + } + selectors = append(selectors, fields.OneTermNotEqualSelector(expr.Key, expr.Values[0])) + + default: + return nil, fmt.Errorf("%q is not a valid node field selector operator", expr.Operator) + } + } + + return fields.AndSelectors(selectors...), nil +} + // GetTolerationsFromPodAnnotations gets the json serialized tolerations data from Pod.Annotations // and converts it to the []Toleration type in core. func GetTolerationsFromPodAnnotations(annotations map[string]string) ([]core.Toleration, error) { diff --git a/pkg/apis/core/types.go b/pkg/apis/core/types.go index 292565bc56e..68383100c00 100644 --- a/pkg/apis/core/types.go +++ b/pkg/apis/core/types.go @@ -2105,10 +2105,13 @@ type NodeSelector struct { NodeSelectorTerms []NodeSelectorTerm } -// A null or empty node selector term matches no objects. +// A null or empty node selector term matches no objects. The requirements of +// them are ANDed. type NodeSelectorTerm struct { - //Required. A list of node selector requirements. The requirements are ANDed. + // A list of node selector requirements by node's labels. MatchExpressions []NodeSelectorRequirement + // A list of node selector requirements by node's fields. + MatchFields []NodeSelectorRequirement } // A node selector requirement is a selector that contains values, a key, and an operator diff --git a/pkg/apis/core/v1/helper/helpers.go b/pkg/apis/core/v1/helper/helpers.go index c469a956612..c7e2c2d0c7a 100644 --- a/pkg/apis/core/v1/helper/helpers.go +++ b/pkg/apis/core/v1/helper/helpers.go @@ -23,6 +23,7 @@ import ( "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" + "k8s.io/apimachinery/pkg/fields" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/selection" "k8s.io/apimachinery/pkg/util/validation" @@ -246,6 +247,71 @@ func NodeSelectorRequirementsAsSelector(nsm []v1.NodeSelectorRequirement) (label return selector, nil } +// NodeSelectorRequirementsAsFieldSelector converts the []NodeSelectorRequirement core type into a struct that implements +// fields.Selector. +func NodeSelectorRequirementsAsFieldSelector(nsm []v1.NodeSelectorRequirement) (fields.Selector, error) { + if len(nsm) == 0 { + return fields.Nothing(), nil + } + + selectors := []fields.Selector{} + for _, expr := range nsm { + switch expr.Operator { + case v1.NodeSelectorOpIn: + if len(expr.Values) != 1 { + return nil, fmt.Errorf("unexpected number of value (%d) for node field selector operator %q", + len(expr.Values), expr.Operator) + } + selectors = append(selectors, fields.OneTermEqualSelector(expr.Key, expr.Values[0])) + + case v1.NodeSelectorOpNotIn: + if len(expr.Values) != 1 { + return nil, fmt.Errorf("unexpected number of value (%d) for node field selector operator %q", + len(expr.Values), expr.Operator) + } + selectors = append(selectors, fields.OneTermNotEqualSelector(expr.Key, expr.Values[0])) + + default: + return nil, fmt.Errorf("%q is not a valid node field selector operator", expr.Operator) + } + } + + return fields.AndSelectors(selectors...), nil +} + +// MatchNodeSelectorTerms checks whether the node labels and fields match node selector terms in ORed; +// nil or empty term matches no objects. +func MatchNodeSelectorTerms( + nodeSelectorTerms []v1.NodeSelectorTerm, + nodeLabels labels.Set, + nodeFields fields.Set, +) bool { + for _, req := range nodeSelectorTerms { + // nil or empty term selects no objects + if len(req.MatchExpressions) == 0 && len(req.MatchFields) == 0 { + continue + } + + if len(req.MatchExpressions) != 0 { + labelSelector, err := NodeSelectorRequirementsAsSelector(req.MatchExpressions) + if err != nil || !labelSelector.Matches(nodeLabels) { + continue + } + } + + if len(req.MatchFields) != 0 { + fieldSelector, err := NodeSelectorRequirementsAsFieldSelector(req.MatchFields) + if err != nil || !fieldSelector.Matches(nodeFields) { + continue + } + } + + return true + } + + return false +} + // AddOrUpdateTolerationInPodSpec tries to add a toleration to the toleration list in PodSpec. // Returns true if something was updated, false otherwise. func AddOrUpdateTolerationInPodSpec(spec *v1.PodSpec, toleration *v1.Toleration) bool { diff --git a/pkg/apis/core/v1/helper/helpers_test.go b/pkg/apis/core/v1/helper/helpers_test.go index 44a1c504208..682c96dc09c 100644 --- a/pkg/apis/core/v1/helper/helpers_test.go +++ b/pkg/apis/core/v1/helper/helpers_test.go @@ -25,6 +25,7 @@ import ( apiequality "k8s.io/apimachinery/pkg/api/equality" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/fields" "k8s.io/apimachinery/pkg/labels" ) @@ -562,3 +563,232 @@ func TestSysctlsFromPodAnnotation(t *testing.T) { } } } + +func TestMatchNodeSelectorTerms(t *testing.T) { + type args struct { + nodeSelectorTerms []v1.NodeSelectorTerm + nodeLabels labels.Set + nodeFields fields.Set + } + + tests := []struct { + name string + args args + want bool + }{ + { + name: "nil terms", + args: args{ + nodeSelectorTerms: nil, + nodeLabels: nil, + nodeFields: nil, + }, + want: false, + }, + { + name: "node label matches matchExpressions terms", + args: args{ + nodeSelectorTerms: []v1.NodeSelectorTerm{ + { + MatchExpressions: []v1.NodeSelectorRequirement{{ + Key: "label_1", + Operator: v1.NodeSelectorOpIn, + Values: []string{"label_1_val"}, + }}, + }, + }, + nodeLabels: map[string]string{"label_1": "label_1_val"}, + nodeFields: nil, + }, + want: true, + }, + { + name: "node field matches matchFields terms", + args: args{ + nodeSelectorTerms: []v1.NodeSelectorTerm{ + { + MatchFields: []v1.NodeSelectorRequirement{{ + Key: "metadata.name", + Operator: v1.NodeSelectorOpIn, + Values: []string{"host_1"}, + }}, + }, + }, + nodeLabels: nil, + nodeFields: map[string]string{ + "metadata.name": "host_1", + }, + }, + want: true, + }, + { + name: "invalid node field requirement", + args: args{ + nodeSelectorTerms: []v1.NodeSelectorTerm{ + { + MatchFields: []v1.NodeSelectorRequirement{{ + Key: "metadata.name", + Operator: v1.NodeSelectorOpIn, + Values: []string{"host_1, host_2"}, + }}, + }, + }, + nodeLabels: nil, + nodeFields: map[string]string{ + "metadata.name": "host_1", + }, + }, + want: false, + }, + { + name: "fieldSelectorTerm with node labels", + args: args{ + nodeSelectorTerms: []v1.NodeSelectorTerm{ + { + MatchFields: []v1.NodeSelectorRequirement{{ + Key: "metadata.name", + Operator: v1.NodeSelectorOpIn, + Values: []string{"host_1"}, + }}, + }, + }, + nodeLabels: map[string]string{ + "metadata.name": "host_1", + }, + nodeFields: nil, + }, + want: false, + }, + { + name: "labelSelectorTerm with node fields", + args: args{ + nodeSelectorTerms: []v1.NodeSelectorTerm{ + { + MatchExpressions: []v1.NodeSelectorRequirement{{ + Key: "metadata.name", + Operator: v1.NodeSelectorOpIn, + Values: []string{"host_1"}, + }}, + }, + }, + nodeLabels: nil, + nodeFields: map[string]string{ + "metadata.name": "host_1", + }, + }, + want: false, + }, + { + name: "labelSelectorTerm and fieldSelectorTerm was set, but only node fields", + args: args{ + nodeSelectorTerms: []v1.NodeSelectorTerm{ + { + MatchExpressions: []v1.NodeSelectorRequirement{{ + Key: "label_1", + Operator: v1.NodeSelectorOpIn, + Values: []string{"label_1_val"}, + }}, + MatchFields: []v1.NodeSelectorRequirement{{ + Key: "metadata.name", + Operator: v1.NodeSelectorOpIn, + Values: []string{"host_1"}, + }}, + }, + }, + nodeLabels: nil, + nodeFields: map[string]string{ + "metadata.name": "host_1", + }, + }, + want: false, + }, + { + name: "labelSelectorTerm and fieldSelectorTerm was set, both node fields and labels (both matched)", + args: args{ + nodeSelectorTerms: []v1.NodeSelectorTerm{ + { + MatchExpressions: []v1.NodeSelectorRequirement{{ + Key: "label_1", + Operator: v1.NodeSelectorOpIn, + Values: []string{"label_1_val"}, + }}, + MatchFields: []v1.NodeSelectorRequirement{{ + Key: "metadata.name", + Operator: v1.NodeSelectorOpIn, + Values: []string{"host_1"}, + }}, + }, + }, + nodeLabels: map[string]string{ + "label_1": "label_1_val", + }, + nodeFields: map[string]string{ + "metadata.name": "host_1", + }, + }, + want: true, + }, + { + name: "labelSelectorTerm and fieldSelectorTerm was set, both node fields and labels (one mismatched)", + args: args{ + nodeSelectorTerms: []v1.NodeSelectorTerm{ + { + MatchExpressions: []v1.NodeSelectorRequirement{{ + Key: "label_1", + Operator: v1.NodeSelectorOpIn, + Values: []string{"label_1_val"}, + }}, + MatchFields: []v1.NodeSelectorRequirement{{ + Key: "metadata.name", + Operator: v1.NodeSelectorOpIn, + Values: []string{"host_1"}, + }}, + }, + }, + nodeLabels: map[string]string{ + "label_1": "label_1_val-failed", + }, + nodeFields: map[string]string{ + "metadata.name": "host_1", + }, + }, + want: false, + }, + { + name: "multi-selector was set, both node fields and labels (one mismatched)", + args: args{ + nodeSelectorTerms: []v1.NodeSelectorTerm{ + { + MatchExpressions: []v1.NodeSelectorRequirement{{ + Key: "label_1", + Operator: v1.NodeSelectorOpIn, + Values: []string{"label_1_val"}, + }}, + }, + { + MatchFields: []v1.NodeSelectorRequirement{{ + Key: "metadata.name", + Operator: v1.NodeSelectorOpIn, + Values: []string{"host_1"}, + }}, + }, + }, + nodeLabels: map[string]string{ + "label_1": "label_1_val-failed", + }, + nodeFields: map[string]string{ + "metadata.name": "host_1", + }, + }, + want: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := MatchNodeSelectorTerms(tt.args.nodeSelectorTerms, tt.args.nodeLabels, tt.args.nodeFields); got != tt.want { + t.Errorf("MatchNodeSelectorTermsORed() = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/pkg/apis/core/validation/validation.go b/pkg/apis/core/validation/validation.go index bcef7e324c5..fcb242a1960 100644 --- a/pkg/apis/core/validation/validation.go +++ b/pkg/apis/core/validation/validation.go @@ -2997,7 +2997,40 @@ func ValidateNodeSelectorRequirement(rq core.NodeSelectorRequirement, fldPath *f default: allErrs = append(allErrs, field.Invalid(fldPath.Child("operator"), rq.Operator, "not a valid selector operator")) } + allErrs = append(allErrs, unversionedvalidation.ValidateLabelName(rq.Key, fldPath.Child("key"))...) + + return allErrs +} + +var nodeFieldSelectorValidators = map[string]func(string, bool) []string{ + core.ObjectNameField: ValidateNodeName, +} + +// ValidateNodeFieldSelectorRequirement tests that the specified NodeSelectorRequirement fields has valid data +func ValidateNodeFieldSelectorRequirement(req core.NodeSelectorRequirement, fldPath *field.Path) field.ErrorList { + allErrs := field.ErrorList{} + + switch req.Operator { + case core.NodeSelectorOpIn, core.NodeSelectorOpNotIn: + if len(req.Values) != 1 { + allErrs = append(allErrs, field.Required(fldPath.Child("values"), + "must be only one value when `operator` is 'In' or 'NotIn' for node field selector")) + } + default: + allErrs = append(allErrs, field.Invalid(fldPath.Child("operator"), req.Operator, "not a valid selector operator")) + } + + if vf, found := nodeFieldSelectorValidators[req.Key]; !found { + allErrs = append(allErrs, field.Invalid(fldPath.Child("key"), req.Key, "not a valid field selector key")) + } else { + for i, v := range req.Values { + for _, msg := range vf(v, false) { + allErrs = append(allErrs, field.Invalid(fldPath.Child("values").Index(i), v, msg)) + } + } + } + return allErrs } @@ -3008,6 +3041,11 @@ func ValidateNodeSelectorTerm(term core.NodeSelectorTerm, fldPath *field.Path) f for j, req := range term.MatchExpressions { allErrs = append(allErrs, ValidateNodeSelectorRequirement(req, fldPath.Child("matchExpressions").Index(j))...) } + + for j, req := range term.MatchFields { + allErrs = append(allErrs, ValidateNodeFieldSelectorRequirement(req, fldPath.Child("matchFields").Index(j))...) + } + return allErrs } diff --git a/pkg/apis/core/validation/validation_test.go b/pkg/apis/core/validation/validation_test.go index 8d15e052ee8..c6120142aa2 100644 --- a/pkg/apis/core/validation/validation_test.go +++ b/pkg/apis/core/validation/validation_test.go @@ -6048,6 +6048,13 @@ func TestValidatePod(t *testing.T) { Values: []string{"value1", "value2"}, }, }, + MatchFields: []core.NodeSelectorRequirement{ + { + Key: "metadata.name", + Operator: core.NodeSelectorOpIn, + Values: []string{"host1"}, + }, + }, }, }, }, @@ -6515,6 +6522,83 @@ func TestValidatePod(t *testing.T) { }), }, }, + "invalid node field selector requirement in node affinity, more values for field selector": { + expectedError: "spec.affinity.nodeAffinity.requiredDuringSchedulingIgnoredDuringExecution.nodeSelectorTerms[0].matchFields[0].values", + spec: core.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "123", + Namespace: "ns", + }, + Spec: validPodSpec(&core.Affinity{ + NodeAffinity: &core.NodeAffinity{ + RequiredDuringSchedulingIgnoredDuringExecution: &core.NodeSelector{ + NodeSelectorTerms: []core.NodeSelectorTerm{ + { + MatchFields: []core.NodeSelectorRequirement{ + { + Key: "metadata.name", + Operator: core.NodeSelectorOpIn, + Values: []string{"host1", "host2"}, + }, + }, + }, + }, + }, + }, + }), + }, + }, + "invalid node field selector requirement in node affinity, invalid operator": { + expectedError: "spec.affinity.nodeAffinity.requiredDuringSchedulingIgnoredDuringExecution.nodeSelectorTerms[0].matchFields[0].operator", + spec: core.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "123", + Namespace: "ns", + }, + Spec: validPodSpec(&core.Affinity{ + NodeAffinity: &core.NodeAffinity{ + RequiredDuringSchedulingIgnoredDuringExecution: &core.NodeSelector{ + NodeSelectorTerms: []core.NodeSelectorTerm{ + { + MatchFields: []core.NodeSelectorRequirement{ + { + Key: "metadata.name", + Operator: core.NodeSelectorOpExists, + }, + }, + }, + }, + }, + }, + }), + }, + }, + "invalid node field selector requirement in node affinity, invalid key": { + expectedError: "spec.affinity.nodeAffinity.requiredDuringSchedulingIgnoredDuringExecution.nodeSelectorTerms[0].matchFields[0].key", + spec: core.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "123", + Namespace: "ns", + }, + Spec: validPodSpec(&core.Affinity{ + NodeAffinity: &core.NodeAffinity{ + RequiredDuringSchedulingIgnoredDuringExecution: &core.NodeSelector{ + NodeSelectorTerms: []core.NodeSelectorTerm{ + { + MatchFields: []core.NodeSelectorRequirement{ + { + Key: "metadata.namespace", + Operator: core.NodeSelectorOpIn, + Values: []string{"ns1"}, + }, + }, + }, + }, + }, + }, + }), + }, + }, "invalid preferredSchedulingTerm in node affinity, weight should be in range 1-100": { expectedError: "must be in the range 1-100", spec: core.Pod{ diff --git a/staging/src/k8s.io/api/core/v1/types.go b/staging/src/k8s.io/api/core/v1/types.go index 28be9abfebd..cb9077d6003 100644 --- a/staging/src/k8s.io/api/core/v1/types.go +++ b/staging/src/k8s.io/api/core/v1/types.go @@ -2320,10 +2320,15 @@ type NodeSelector struct { NodeSelectorTerms []NodeSelectorTerm `json:"nodeSelectorTerms" protobuf:"bytes,1,rep,name=nodeSelectorTerms"` } -// A null or empty node selector term matches no objects. +// A null or empty node selector term matches no objects. The requirements of +// them are ANDed. type NodeSelectorTerm struct { - //Required. A list of node selector requirements. The requirements are ANDed. - MatchExpressions []NodeSelectorRequirement `json:"matchExpressions" protobuf:"bytes,1,rep,name=matchExpressions"` + // A list of node selector requirements by node's labels. + // +optional + MatchExpressions []NodeSelectorRequirement `json:"matchExpressions,omitempty" protobuf:"bytes,1,rep,name=matchExpressions"` + // A list of node selector requirements by node's fields. + // +optional + MatchFields []NodeSelectorRequirement `json:"matchFields,omitempty" protobuf:"bytes,2,rep,name=matchFields"` } // A node selector requirement is a selector that contains values, a key, and an operator diff --git a/staging/src/k8s.io/apimachinery/pkg/fields/selector.go b/staging/src/k8s.io/apimachinery/pkg/fields/selector.go index 3785d8c2f74..e3e4453b64f 100644 --- a/staging/src/k8s.io/apimachinery/pkg/fields/selector.go +++ b/staging/src/k8s.io/apimachinery/pkg/fields/selector.go @@ -55,6 +55,21 @@ type Selector interface { DeepCopySelector() Selector } +type nothingSelector struct{} + +func (n nothingSelector) Matches(_ Fields) bool { return false } +func (n nothingSelector) Empty() bool { return false } +func (n nothingSelector) String() string { return "" } +func (n nothingSelector) Requirements() Requirements { return nil } +func (n nothingSelector) DeepCopySelector() Selector { return n } +func (n nothingSelector) RequiresExactMatch(field string) (value string, found bool) { return "", false } +func (n nothingSelector) Transform(fn TransformFunc) (Selector, error) { return n, nil } + +// Nothing returns a selector that matches no fields +func Nothing() Selector { + return nothingSelector{} +} + // Everything returns a selector that matches all fields. func Everything() Selector { return andTerm{} @@ -449,6 +464,12 @@ func OneTermEqualSelector(k, v string) Selector { return &hasTerm{field: k, value: v} } +// OneTermNotEqualSelector returns an object that matches objects where one field/field does not equal one value. +// Cannot return an error. +func OneTermNotEqualSelector(k, v string) Selector { + return ¬HasTerm{field: k, value: v} +} + // AndSelectors creates a selector that is the logical AND of all the given selectors func AndSelectors(selectors ...Selector) Selector { return andTerm(selectors)