diff --git a/pkg/apis/core/v1/helper/BUILD b/pkg/apis/core/v1/helper/BUILD index 79483ac3d22..0a5c0649606 100644 --- a/pkg/apis/core/v1/helper/BUILD +++ b/pkg/apis/core/v1/helper/BUILD @@ -15,7 +15,6 @@ go_test( "//staging/src/k8s.io/apimachinery/pkg/api/equality:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/api/resource:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", - "//staging/src/k8s.io/apimachinery/pkg/fields:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/labels:go_default_library", ], ) @@ -31,6 +30,7 @@ go_library( "//staging/src/k8s.io/apimachinery/pkg/fields:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/labels:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/selection:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/util/errors:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/validation:go_default_library", ], ) diff --git a/pkg/apis/core/v1/helper/helpers.go b/pkg/apis/core/v1/helper/helpers.go index ca45fd8ec40..0d737e0ade3 100644 --- a/pkg/apis/core/v1/helper/helpers.go +++ b/pkg/apis/core/v1/helper/helpers.go @@ -26,6 +26,7 @@ import ( "k8s.io/apimachinery/pkg/fields" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/selection" + apierrors "k8s.io/apimachinery/pkg/util/errors" "k8s.io/apimachinery/pkg/util/validation" "k8s.io/kubernetes/pkg/apis/core/helper" ) @@ -232,9 +233,9 @@ func containsAccessMode(modes []v1.PersistentVolumeAccessMode, mode v1.Persisten return false } -// NodeSelectorRequirementsAsSelector converts the []NodeSelectorRequirement api type into a struct that implements +// nodeSelectorRequirementsAsSelector converts the []NodeSelectorRequirement api type into a struct that implements // labels.Selector. -func NodeSelectorRequirementsAsSelector(nsm []v1.NodeSelectorRequirement) (labels.Selector, error) { +func nodeSelectorRequirementsAsSelector(nsm []v1.NodeSelectorRequirement) (labels.Selector, error) { if len(nsm) == 0 { return labels.Nothing(), nil } @@ -266,9 +267,9 @@ func NodeSelectorRequirementsAsSelector(nsm []v1.NodeSelectorRequirement) (label return selector, nil } -// NodeSelectorRequirementsAsFieldSelector converts the []NodeSelectorRequirement core type into a struct that implements +// nodeSelectorRequirementsAsFieldSelector converts the []NodeSelectorRequirement core type into a struct that implements // fields.Selector. -func NodeSelectorRequirementsAsFieldSelector(nsm []v1.NodeSelectorRequirement) (fields.Selector, error) { +func nodeSelectorRequirementsAsFieldSelector(nsm []v1.NodeSelectorRequirement) (fields.Selector, error) { if len(nsm) == 0 { return fields.Nothing(), nil } @@ -315,34 +316,45 @@ func NodeSelectorRequirementKeysExistInNodeSelectorTerms(reqs []v1.NodeSelectorR // 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 { + node *v1.Node, + nodeSelector *v1.NodeSelector, +) (bool, error) { + if node == nil { + return false, nil + } + var errors []error + for _, req := range nodeSelector.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) { + labelSelector, err := nodeSelectorRequirementsAsSelector(req.MatchExpressions) + if err != nil { + errors = append(errors, err) + continue + } + if labelSelector == nil || !labelSelector.Matches(labels.Set(node.Labels)) { continue } } - if len(req.MatchFields) != 0 { - fieldSelector, err := NodeSelectorRequirementsAsFieldSelector(req.MatchFields) - if err != nil || !fieldSelector.Matches(nodeFields) { + if len(req.MatchFields) != 0 && len(node.Name) > 0 { + fieldSelector, err := nodeSelectorRequirementsAsFieldSelector(req.MatchFields) + if err != nil { + errors = append(errors, err) + continue + } + if fieldSelector == nil || !fieldSelector.Matches(fields.Set{"metadata.name": node.Name}) { continue } } - return true + return true, nil } - return false + return false, apierrors.NewAggregate(errors) } // TopologySelectorRequirementsAsSelector converts the []TopologySelectorLabelRequirement api type into a struct diff --git a/pkg/apis/core/v1/helper/helpers_test.go b/pkg/apis/core/v1/helper/helpers_test.go index c418e82b4fb..6063a9950a5 100644 --- a/pkg/apis/core/v1/helper/helpers_test.go +++ b/pkg/apis/core/v1/helper/helpers_test.go @@ -25,7 +25,6 @@ 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" ) @@ -298,7 +297,7 @@ func TestNodeSelectorRequirementsAsSelector(t *testing.T) { } for i, tc := range tc { - out, err := NodeSelectorRequirementsAsSelector(tc.in) + out, err := nodeSelectorRequirementsAsSelector(tc.in) if err == nil && tc.expectErr { t.Errorf("[%v]expected error but got none.", i) } @@ -594,9 +593,8 @@ func TestGetAvoidPodsFromNode(t *testing.T) { func TestMatchNodeSelectorTerms(t *testing.T) { type args struct { - nodeSelectorTerms []v1.NodeSelectorTerm - nodeLabels labels.Set - nodeFields fields.Set + nodeSelector *v1.NodeSelector + node *v1.Node } tests := []struct { @@ -607,16 +605,15 @@ func TestMatchNodeSelectorTerms(t *testing.T) { { name: "nil terms", args: args{ - nodeSelectorTerms: nil, - nodeLabels: nil, - nodeFields: nil, + nodeSelector: nil, + node: nil, }, want: false, }, { name: "node label matches matchExpressions terms", args: args{ - nodeSelectorTerms: []v1.NodeSelectorTerm{ + nodeSelector: &v1.NodeSelector{NodeSelectorTerms: []v1.NodeSelectorTerm{ { MatchExpressions: []v1.NodeSelectorRequirement{{ Key: "label_1", @@ -624,16 +621,15 @@ func TestMatchNodeSelectorTerms(t *testing.T) { Values: []string{"label_1_val"}, }}, }, - }, - nodeLabels: map[string]string{"label_1": "label_1_val"}, - nodeFields: nil, + }}, + node: &v1.Node{ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{"label_1": "label_1_val"}}}, }, want: true, }, { name: "node field matches matchFields terms", args: args{ - nodeSelectorTerms: []v1.NodeSelectorTerm{ + nodeSelector: &v1.NodeSelector{NodeSelectorTerms: []v1.NodeSelectorTerm{ { MatchFields: []v1.NodeSelectorRequirement{{ Key: "metadata.name", @@ -641,18 +637,15 @@ func TestMatchNodeSelectorTerms(t *testing.T) { Values: []string{"host_1"}, }}, }, - }, - nodeLabels: nil, - nodeFields: map[string]string{ - "metadata.name": "host_1", - }, + }}, + node: &v1.Node{ObjectMeta: metav1.ObjectMeta{Name: "host_1"}}, }, want: true, }, { name: "invalid node field requirement", args: args{ - nodeSelectorTerms: []v1.NodeSelectorTerm{ + nodeSelector: &v1.NodeSelector{NodeSelectorTerms: []v1.NodeSelectorTerm{ { MatchFields: []v1.NodeSelectorRequirement{{ Key: "metadata.name", @@ -660,18 +653,15 @@ func TestMatchNodeSelectorTerms(t *testing.T) { Values: []string{"host_1, host_2"}, }}, }, - }, - nodeLabels: nil, - nodeFields: map[string]string{ - "metadata.name": "host_1", - }, + }}, + node: &v1.Node{ObjectMeta: metav1.ObjectMeta{Name: "host_1"}}, }, want: false, }, { name: "fieldSelectorTerm with node labels", args: args{ - nodeSelectorTerms: []v1.NodeSelectorTerm{ + nodeSelector: &v1.NodeSelector{NodeSelectorTerms: []v1.NodeSelectorTerm{ { MatchFields: []v1.NodeSelectorRequirement{{ Key: "metadata.name", @@ -679,18 +669,17 @@ func TestMatchNodeSelectorTerms(t *testing.T) { Values: []string{"host_1"}, }}, }, - }, - nodeLabels: map[string]string{ + }}, + node: &v1.Node{ObjectMeta: metav1.ObjectMeta{Name: "not_host_1", Labels: map[string]string{ "metadata.name": "host_1", - }, - nodeFields: nil, + }}}, }, want: false, }, { name: "labelSelectorTerm with node fields", args: args{ - nodeSelectorTerms: []v1.NodeSelectorTerm{ + nodeSelector: &v1.NodeSelector{NodeSelectorTerms: []v1.NodeSelectorTerm{ { MatchExpressions: []v1.NodeSelectorRequirement{{ Key: "metadata.name", @@ -698,18 +687,15 @@ func TestMatchNodeSelectorTerms(t *testing.T) { Values: []string{"host_1"}, }}, }, - }, - nodeLabels: nil, - nodeFields: map[string]string{ - "metadata.name": "host_1", - }, + }}, + node: &v1.Node{ObjectMeta: metav1.ObjectMeta{Name: "host_1"}}, }, want: false, }, { name: "labelSelectorTerm and fieldSelectorTerm was set, but only node fields", args: args{ - nodeSelectorTerms: []v1.NodeSelectorTerm{ + nodeSelector: &v1.NodeSelector{NodeSelectorTerms: []v1.NodeSelectorTerm{ { MatchExpressions: []v1.NodeSelectorRequirement{{ Key: "label_1", @@ -722,18 +708,15 @@ func TestMatchNodeSelectorTerms(t *testing.T) { Values: []string{"host_1"}, }}, }, - }, - nodeLabels: nil, - nodeFields: map[string]string{ - "metadata.name": "host_1", - }, + }}, + node: &v1.Node{ObjectMeta: metav1.ObjectMeta{Name: "host_1"}}, }, want: false, }, { name: "labelSelectorTerm and fieldSelectorTerm was set, both node fields and labels (both matched)", args: args{ - nodeSelectorTerms: []v1.NodeSelectorTerm{ + nodeSelector: &v1.NodeSelector{NodeSelectorTerms: []v1.NodeSelectorTerm{ { MatchExpressions: []v1.NodeSelectorRequirement{{ Key: "label_1", @@ -745,21 +728,19 @@ func TestMatchNodeSelectorTerms(t *testing.T) { Operator: v1.NodeSelectorOpIn, Values: []string{"host_1"}, }}, - }, - }, - nodeLabels: map[string]string{ - "label_1": "label_1_val", - }, - nodeFields: map[string]string{ - "metadata.name": "host_1", + }}, }, + node: &v1.Node{ObjectMeta: metav1.ObjectMeta{Name: "host_1", + Labels: map[string]string{ + "label_1": "label_1_val", + }}}, }, want: true, }, { name: "labelSelectorTerm and fieldSelectorTerm was set, both node fields and labels (one mismatched)", args: args{ - nodeSelectorTerms: []v1.NodeSelectorTerm{ + nodeSelector: &v1.NodeSelector{NodeSelectorTerms: []v1.NodeSelectorTerm{ { MatchExpressions: []v1.NodeSelectorRequirement{{ Key: "label_1", @@ -772,20 +753,18 @@ func TestMatchNodeSelectorTerms(t *testing.T) { Values: []string{"host_1"}, }}, }, - }, - nodeLabels: map[string]string{ - "label_1": "label_1_val-failed", - }, - nodeFields: map[string]string{ - "metadata.name": "host_1", - }, + }}, + node: &v1.Node{ObjectMeta: metav1.ObjectMeta{Name: "host_1", + Labels: map[string]string{ + "label_1": "label_1_val-failed", + }}}, }, want: false, }, { name: "multi-selector was set, both node fields and labels (one mismatched)", args: args{ - nodeSelectorTerms: []v1.NodeSelectorTerm{ + nodeSelector: &v1.NodeSelector{NodeSelectorTerms: []v1.NodeSelectorTerm{ { MatchExpressions: []v1.NodeSelectorRequirement{{ Key: "label_1", @@ -800,13 +779,11 @@ func TestMatchNodeSelectorTerms(t *testing.T) { Values: []string{"host_1"}, }}, }, - }, - nodeLabels: map[string]string{ - "label_1": "label_1_val-failed", - }, - nodeFields: map[string]string{ - "metadata.name": "host_1", - }, + }}, + node: &v1.Node{ObjectMeta: metav1.ObjectMeta{Name: "host_1", + Labels: map[string]string{ + "label_1": "label_1_val-failed", + }}}, }, want: true, }, @@ -814,7 +791,7 @@ func TestMatchNodeSelectorTerms(t *testing.T) { 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 { + if got, _ := MatchNodeSelectorTerms(tt.args.node, tt.args.nodeSelector); got != tt.want { t.Errorf("MatchNodeSelectorTermsORed() = %v, want %v", got, tt.want) } }) @@ -822,33 +799,31 @@ func TestMatchNodeSelectorTerms(t *testing.T) { } // TestMatchNodeSelectorTermsStateless ensures MatchNodeSelectorTerms() -// is invoked in a "stateless" manner, i.e. nodeSelectorTerms should NOT +// is invoked in a "stateless" manner, i.e. nodeSelector should NOT // be deeply modified after invoking func TestMatchNodeSelectorTermsStateless(t *testing.T) { type args struct { - nodeSelectorTerms []v1.NodeSelectorTerm - nodeLabels labels.Set - nodeFields fields.Set + nodeSelector *v1.NodeSelector + node *v1.Node } tests := []struct { name string args args - want []v1.NodeSelectorTerm + want *v1.NodeSelector }{ { name: "nil terms", args: args{ - nodeSelectorTerms: nil, - nodeLabels: nil, - nodeFields: nil, + nodeSelector: nil, + node: nil, }, want: nil, }, { name: "nodeLabels: preordered matchExpressions and nil matchFields", args: args{ - nodeSelectorTerms: []v1.NodeSelectorTerm{ + nodeSelector: &v1.NodeSelector{NodeSelectorTerms: []v1.NodeSelectorTerm{ { MatchExpressions: []v1.NodeSelectorRequirement{{ Key: "label_1", @@ -856,11 +831,10 @@ func TestMatchNodeSelectorTermsStateless(t *testing.T) { Values: []string{"label_1_val", "label_2_val"}, }}, }, - }, - nodeLabels: map[string]string{"label_1": "label_1_val"}, - nodeFields: nil, + }}, + node: &v1.Node{ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{"label_1": "label_1_val"}}}, }, - want: []v1.NodeSelectorTerm{ + want: &v1.NodeSelector{NodeSelectorTerms: []v1.NodeSelectorTerm{ { MatchExpressions: []v1.NodeSelectorRequirement{{ Key: "label_1", @@ -868,12 +842,12 @@ func TestMatchNodeSelectorTermsStateless(t *testing.T) { Values: []string{"label_1_val", "label_2_val"}, }}, }, - }, + }}, }, { name: "nodeLabels: unordered matchExpressions and nil matchFields", args: args{ - nodeSelectorTerms: []v1.NodeSelectorTerm{ + nodeSelector: &v1.NodeSelector{NodeSelectorTerms: []v1.NodeSelectorTerm{ { MatchExpressions: []v1.NodeSelectorRequirement{{ Key: "label_1", @@ -881,11 +855,10 @@ func TestMatchNodeSelectorTermsStateless(t *testing.T) { Values: []string{"label_2_val", "label_1_val"}, }}, }, - }, - nodeLabels: map[string]string{"label_1": "label_1_val"}, - nodeFields: nil, + }}, + node: &v1.Node{ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{"label_1": "label_1_val"}}}, }, - want: []v1.NodeSelectorTerm{ + want: &v1.NodeSelector{NodeSelectorTerms: []v1.NodeSelectorTerm{ { MatchExpressions: []v1.NodeSelectorRequirement{{ Key: "label_1", @@ -893,12 +866,12 @@ func TestMatchNodeSelectorTermsStateless(t *testing.T) { Values: []string{"label_2_val", "label_1_val"}, }}, }, - }, + }}, }, { name: "nodeFields: nil matchExpressions and preordered matchFields", args: args{ - nodeSelectorTerms: []v1.NodeSelectorTerm{ + nodeSelector: &v1.NodeSelector{NodeSelectorTerms: []v1.NodeSelectorTerm{ { MatchFields: []v1.NodeSelectorRequirement{{ Key: "metadata.name", @@ -906,13 +879,10 @@ func TestMatchNodeSelectorTermsStateless(t *testing.T) { Values: []string{"host_1", "host_2"}, }}, }, - }, - nodeLabels: nil, - nodeFields: map[string]string{ - "metadata.name": "host_1", - }, + }}, + node: &v1.Node{ObjectMeta: metav1.ObjectMeta{Name: "host_1"}}, }, - want: []v1.NodeSelectorTerm{ + want: &v1.NodeSelector{NodeSelectorTerms: []v1.NodeSelectorTerm{ { MatchFields: []v1.NodeSelectorRequirement{{ Key: "metadata.name", @@ -920,12 +890,12 @@ func TestMatchNodeSelectorTermsStateless(t *testing.T) { Values: []string{"host_1", "host_2"}, }}, }, - }, + }}, }, { name: "nodeFields: nil matchExpressions and unordered matchFields", args: args{ - nodeSelectorTerms: []v1.NodeSelectorTerm{ + nodeSelector: &v1.NodeSelector{NodeSelectorTerms: []v1.NodeSelectorTerm{ { MatchFields: []v1.NodeSelectorRequirement{{ Key: "metadata.name", @@ -933,13 +903,10 @@ func TestMatchNodeSelectorTermsStateless(t *testing.T) { Values: []string{"host_2", "host_1"}, }}, }, - }, - nodeLabels: nil, - nodeFields: map[string]string{ - "metadata.name": "host_1", - }, + }}, + node: &v1.Node{ObjectMeta: metav1.ObjectMeta{Name: "host_1"}}, }, - want: []v1.NodeSelectorTerm{ + want: &v1.NodeSelector{NodeSelectorTerms: []v1.NodeSelectorTerm{ { MatchFields: []v1.NodeSelectorRequirement{{ Key: "metadata.name", @@ -947,12 +914,12 @@ func TestMatchNodeSelectorTermsStateless(t *testing.T) { Values: []string{"host_2", "host_1"}, }}, }, - }, + }}, }, { name: "nodeLabels and nodeFields: ordered matchExpressions and ordered matchFields", args: args{ - nodeSelectorTerms: []v1.NodeSelectorTerm{ + nodeSelector: &v1.NodeSelector{NodeSelectorTerms: []v1.NodeSelectorTerm{ { MatchExpressions: []v1.NodeSelectorRequirement{{ Key: "label_1", @@ -965,15 +932,13 @@ func TestMatchNodeSelectorTermsStateless(t *testing.T) { Values: []string{"host_1", "host_2"}, }}, }, - }, - nodeLabels: map[string]string{ - "label_1": "label_1_val", - }, - nodeFields: map[string]string{ - "metadata.name": "host_1", - }, + }}, + node: &v1.Node{ObjectMeta: metav1.ObjectMeta{Name: "host_1", + Labels: map[string]string{ + "label_1": "label_1_val", + }}}, }, - want: []v1.NodeSelectorTerm{ + want: &v1.NodeSelector{NodeSelectorTerms: []v1.NodeSelectorTerm{ { MatchExpressions: []v1.NodeSelectorRequirement{{ Key: "label_1", @@ -986,12 +951,12 @@ func TestMatchNodeSelectorTermsStateless(t *testing.T) { Values: []string{"host_1", "host_2"}, }}, }, - }, + }}, }, { name: "nodeLabels and nodeFields: ordered matchExpressions and unordered matchFields", args: args{ - nodeSelectorTerms: []v1.NodeSelectorTerm{ + nodeSelector: &v1.NodeSelector{NodeSelectorTerms: []v1.NodeSelectorTerm{ { MatchExpressions: []v1.NodeSelectorRequirement{{ Key: "label_1", @@ -1004,15 +969,13 @@ func TestMatchNodeSelectorTermsStateless(t *testing.T) { Values: []string{"host_2", "host_1"}, }}, }, - }, - nodeLabels: map[string]string{ - "label_1": "label_1_val", - }, - nodeFields: map[string]string{ - "metadata.name": "host_1", - }, + }}, + node: &v1.Node{ObjectMeta: metav1.ObjectMeta{Name: "host_1", + Labels: map[string]string{ + "label_1": "label_1_val", + }}}, }, - want: []v1.NodeSelectorTerm{ + want: &v1.NodeSelector{NodeSelectorTerms: []v1.NodeSelectorTerm{ { MatchExpressions: []v1.NodeSelectorRequirement{{ Key: "label_1", @@ -1025,12 +988,12 @@ func TestMatchNodeSelectorTermsStateless(t *testing.T) { Values: []string{"host_2", "host_1"}, }}, }, - }, + }}, }, { name: "nodeLabels and nodeFields: unordered matchExpressions and ordered matchFields", args: args{ - nodeSelectorTerms: []v1.NodeSelectorTerm{ + nodeSelector: &v1.NodeSelector{NodeSelectorTerms: []v1.NodeSelectorTerm{ { MatchExpressions: []v1.NodeSelectorRequirement{{ Key: "label_1", @@ -1043,15 +1006,13 @@ func TestMatchNodeSelectorTermsStateless(t *testing.T) { Values: []string{"host_1", "host_2"}, }}, }, - }, - nodeLabels: map[string]string{ - "label_1": "label_1_val", - }, - nodeFields: map[string]string{ - "metadata.name": "host_1", - }, + }}, + node: &v1.Node{ObjectMeta: metav1.ObjectMeta{Name: "host_1", + Labels: map[string]string{ + "label_1": "label_1_val", + }}}, }, - want: []v1.NodeSelectorTerm{ + want: &v1.NodeSelector{NodeSelectorTerms: []v1.NodeSelectorTerm{ { MatchExpressions: []v1.NodeSelectorRequirement{{ Key: "label_1", @@ -1064,12 +1025,12 @@ func TestMatchNodeSelectorTermsStateless(t *testing.T) { Values: []string{"host_1", "host_2"}, }}, }, - }, + }}, }, { name: "nodeLabels and nodeFields: unordered matchExpressions and unordered matchFields", args: args{ - nodeSelectorTerms: []v1.NodeSelectorTerm{ + nodeSelector: &v1.NodeSelector{NodeSelectorTerms: []v1.NodeSelectorTerm{ { MatchExpressions: []v1.NodeSelectorRequirement{{ Key: "label_1", @@ -1082,15 +1043,13 @@ func TestMatchNodeSelectorTermsStateless(t *testing.T) { Values: []string{"host_2", "host_1"}, }}, }, - }, - nodeLabels: map[string]string{ - "label_1": "label_1_val", - }, - nodeFields: map[string]string{ - "metadata.name": "host_1", - }, + }}, + node: &v1.Node{ObjectMeta: metav1.ObjectMeta{Name: "host_1", + Labels: map[string]string{ + "label_1": "label_1_val", + }}}, }, - want: []v1.NodeSelectorTerm{ + want: &v1.NodeSelector{NodeSelectorTerms: []v1.NodeSelectorTerm{ { MatchExpressions: []v1.NodeSelectorRequirement{{ Key: "label_1", @@ -1103,16 +1062,16 @@ func TestMatchNodeSelectorTermsStateless(t *testing.T) { Values: []string{"host_2", "host_1"}, }}, }, - }, + }}, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - MatchNodeSelectorTerms(tt.args.nodeSelectorTerms, tt.args.nodeLabels, tt.args.nodeFields) - if !apiequality.Semantic.DeepEqual(tt.args.nodeSelectorTerms, tt.want) { - // fail when tt.args.nodeSelectorTerms is deeply modified - t.Errorf("MatchNodeSelectorTerms() got = %v, want %v", tt.args.nodeSelectorTerms, tt.want) + MatchNodeSelectorTerms(tt.args.node, tt.args.nodeSelector) + if !apiequality.Semantic.DeepEqual(tt.args.nodeSelector, tt.want) { + // fail when tt.args.nodeSelector is deeply modified + t.Errorf("MatchNodeSelectorTerms() got = %v, want %v", tt.args.nodeSelector, tt.want) } }) } diff --git a/pkg/scheduler/framework/plugins/helper/BUILD b/pkg/scheduler/framework/plugins/helper/BUILD index 1a13368ef9b..07ab13eeeb7 100644 --- a/pkg/scheduler/framework/plugins/helper/BUILD +++ b/pkg/scheduler/framework/plugins/helper/BUILD @@ -14,7 +14,6 @@ go_library( "//pkg/scheduler/framework:go_default_library", "//staging/src/k8s.io/api/core/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", - "//staging/src/k8s.io/apimachinery/pkg/fields:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/labels:go_default_library", "//staging/src/k8s.io/client-go/listers/apps/v1:go_default_library", "//staging/src/k8s.io/client-go/listers/core/v1:go_default_library", diff --git a/pkg/scheduler/framework/plugins/helper/node_affinity.go b/pkg/scheduler/framework/plugins/helper/node_affinity.go index 6d02fc75472..d013f534f1c 100644 --- a/pkg/scheduler/framework/plugins/helper/node_affinity.go +++ b/pkg/scheduler/framework/plugins/helper/node_affinity.go @@ -18,7 +18,6 @@ package helper import ( "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/fields" "k8s.io/apimachinery/pkg/labels" v1helper "k8s.io/kubernetes/pkg/apis/core/v1/helper" ) @@ -61,8 +60,7 @@ func PodMatchesNodeSelectorAndAffinityTerms(pod *v1.Pod, node *v1.Node) bool { // Match node selector for requiredDuringSchedulingIgnoredDuringExecution. if nodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution != nil { - nodeSelectorTerms := nodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution.NodeSelectorTerms - nodeAffinityMatches = nodeAffinityMatches && nodeMatchesNodeSelectorTerms(node, nodeSelectorTerms) + nodeAffinityMatches = nodeAffinityMatches && nodeMatchesNodeSelectorTerms(node, nodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution) } } @@ -71,8 +69,8 @@ func PodMatchesNodeSelectorAndAffinityTerms(pod *v1.Pod, node *v1.Node) bool { // nodeMatchesNodeSelectorTerms checks if a node's labels satisfy a list of node selector terms, // terms are ORed, and an empty list of terms will match nothing. -func nodeMatchesNodeSelectorTerms(node *v1.Node, nodeSelectorTerms []v1.NodeSelectorTerm) bool { - return v1helper.MatchNodeSelectorTerms(nodeSelectorTerms, node.Labels, fields.Set{ - "metadata.name": node.Name, - }) +func nodeMatchesNodeSelectorTerms(node *v1.Node, nodeSelector *v1.NodeSelector) bool { + // TODO(@alculquicondor, #95738): parse this error earlier in the plugin so we only need to do it once per pod + matches, _ := v1helper.MatchNodeSelectorTerms(node, nodeSelector) + return matches } diff --git a/pkg/scheduler/framework/plugins/nodeaffinity/BUILD b/pkg/scheduler/framework/plugins/nodeaffinity/BUILD index 1c412890395..51fc205f819 100644 --- a/pkg/scheduler/framework/plugins/nodeaffinity/BUILD +++ b/pkg/scheduler/framework/plugins/nodeaffinity/BUILD @@ -10,7 +10,6 @@ go_library( "//pkg/scheduler/framework:go_default_library", "//pkg/scheduler/framework/plugins/helper:go_default_library", "//staging/src/k8s.io/api/core/v1:go_default_library", - "//staging/src/k8s.io/apimachinery/pkg/labels:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library", ], ) diff --git a/pkg/scheduler/framework/plugins/nodeaffinity/node_affinity.go b/pkg/scheduler/framework/plugins/nodeaffinity/node_affinity.go index be23945ca4c..6dabcd2496d 100644 --- a/pkg/scheduler/framework/plugins/nodeaffinity/node_affinity.go +++ b/pkg/scheduler/framework/plugins/nodeaffinity/node_affinity.go @@ -21,7 +21,6 @@ import ( "fmt" v1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" v1helper "k8s.io/kubernetes/pkg/apis/core/v1/helper" "k8s.io/kubernetes/pkg/scheduler/framework" @@ -88,12 +87,12 @@ func (pl *NodeAffinity) Score(ctx context.Context, state *framework.CycleState, } // TODO: Avoid computing it for all nodes if this becomes a performance problem. - nodeSelector, err := v1helper.NodeSelectorRequirementsAsSelector(preferredSchedulingTerm.Preference.MatchExpressions) + matches, err := v1helper.MatchNodeSelectorTerms(node, &v1.NodeSelector{NodeSelectorTerms: []v1.NodeSelectorTerm{preferredSchedulingTerm.Preference}}) if err != nil { return 0, framework.AsStatus(err) } - if nodeSelector.Matches(labels.Set(node.Labels)) { + if matches { count += int64(preferredSchedulingTerm.Weight) } } diff --git a/pkg/volume/util/BUILD b/pkg/volume/util/BUILD index d0095a9abf9..e08e17b6a3f 100644 --- a/pkg/volume/util/BUILD +++ b/pkg/volume/util/BUILD @@ -33,7 +33,6 @@ go_library( "//staging/src/k8s.io/apimachinery/pkg/api/resource:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/unstructured:go_default_library", - "//staging/src/k8s.io/apimachinery/pkg/labels:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/types:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library", diff --git a/pkg/volume/util/util.go b/pkg/volume/util/util.go index 26f15608a90..a4538769f9f 100644 --- a/pkg/volume/util/util.go +++ b/pkg/volume/util/util.go @@ -35,7 +35,6 @@ import ( storage "k8s.io/api/storage/v1" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/labels" apiruntime "k8s.io/apimachinery/pkg/runtime" utypes "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/sets" @@ -165,18 +164,20 @@ func GetClassForVolume(kubeClient clientset.Interface, pv *v1.PersistentVolume) // CheckNodeAffinity looks at the PV node affinity, and checks if the node has the same corresponding labels // This ensures that we don't mount a volume that doesn't belong to this node func CheckNodeAffinity(pv *v1.PersistentVolume, nodeLabels map[string]string) error { - return checkVolumeNodeAffinity(pv, nodeLabels) + return checkVolumeNodeAffinity(pv, &v1.Node{ObjectMeta: metav1.ObjectMeta{Labels: nodeLabels}}) } -func checkVolumeNodeAffinity(pv *v1.PersistentVolume, nodeLabels map[string]string) error { +func checkVolumeNodeAffinity(pv *v1.PersistentVolume, node *v1.Node) error { if pv.Spec.NodeAffinity == nil { return nil } if pv.Spec.NodeAffinity.Required != nil { - terms := pv.Spec.NodeAffinity.Required.NodeSelectorTerms + terms := pv.Spec.NodeAffinity.Required klog.V(10).Infof("Match for Required node selector terms %+v", terms) - if !v1helper.MatchNodeSelectorTerms(terms, labels.Set(nodeLabels), nil) { + if matches, err := v1helper.MatchNodeSelectorTerms(node, terms); err != nil { + return err + } else if !matches { return fmt.Errorf("no matching NodeSelectorTerms") } }