diff --git a/pkg/scheduler/algorithm/predicates/predicates.go b/pkg/scheduler/algorithm/predicates/predicates.go index 6a0bd20766b..1337293f61d 100644 --- a/pkg/scheduler/algorithm/predicates/predicates.go +++ b/pkg/scheduler/algorithm/predicates/predicates.go @@ -940,16 +940,18 @@ func PodFitsHost(pod *v1.Pod, meta PredicateMetadata, nodeInfo *schedulernodeinf // NodeLabelChecker contains information to check node labels for a predicate. type NodeLabelChecker struct { - labels []string - presence bool + // presentLabels should be present for the node to be considered a fit for hosting the pod + presentLabels []string + // absentLabels should be absent for the node to be considered a fit for hosting the pod + absentLabels []string } // NewNodeLabelPredicate creates a predicate which evaluates whether a pod can fit based on the // node labels which match a filter that it requests. -func NewNodeLabelPredicate(labels []string, presence bool) FitPredicate { +func NewNodeLabelPredicate(presentLabels []string, absentLabels []string) FitPredicate { labelChecker := &NodeLabelChecker{ - labels: labels, - presence: presence, + presentLabels: presentLabels, + absentLabels: absentLabels, } return labelChecker.CheckNodeLabelPresence } @@ -972,15 +974,21 @@ func (n *NodeLabelChecker) CheckNodeLabelPresence(pod *v1.Pod, meta PredicateMet return false, nil, fmt.Errorf("node not found") } - var exists bool nodeLabels := labels.Set(node.Labels) - for _, label := range n.labels { - exists = nodeLabels.Has(label) - if (exists && !n.presence) || (!exists && n.presence) { - return false, []PredicateFailureReason{ErrNodeLabelPresenceViolated}, nil + check := func(labels []string, presence bool) bool { + for _, label := range labels { + exists := nodeLabels.Has(label) + if (exists && !presence) || (!exists && presence) { + return false + } } + return true } - return true, nil, nil + if check(n.presentLabels, true) && check(n.absentLabels, false) { + return true, nil, nil + } + + return false, []PredicateFailureReason{ErrNodeLabelPresenceViolated}, nil } // ServiceAffinity defines a struct used for creating service affinity predicates. diff --git a/pkg/scheduler/algorithm/predicates/predicates_test.go b/pkg/scheduler/algorithm/predicates/predicates_test.go index ec61bcb2160..f1a69baad76 100644 --- a/pkg/scheduler/algorithm/predicates/predicates_test.go +++ b/pkg/scheduler/algorithm/predicates/predicates_test.go @@ -1658,47 +1658,41 @@ func TestPodFitsSelector(t *testing.T) { func TestNodeLabelPresence(t *testing.T) { label := map[string]string{"foo": "bar", "bar": "foo"} tests := []struct { - pod *v1.Pod - labels []string - presence bool - fits bool - name string + pod *v1.Pod + presentLabels []string + absentLabels []string + fits bool + name string }{ { - labels: []string{"baz"}, - presence: true, - fits: false, - name: "label does not match, presence true", + presentLabels: []string{"baz"}, + fits: false, + name: "label does not match, presence true", }, { - labels: []string{"baz"}, - presence: false, - fits: true, - name: "label does not match, presence false", + absentLabels: []string{"baz"}, + fits: true, + name: "label does not match, presence false", }, { - labels: []string{"foo", "baz"}, - presence: true, - fits: false, - name: "one label matches, presence true", + presentLabels: []string{"foo", "baz"}, + fits: false, + name: "one label matches, presence true", }, { - labels: []string{"foo", "baz"}, - presence: false, - fits: false, - name: "one label matches, presence false", + absentLabels: []string{"foo", "baz"}, + fits: false, + name: "one label matches, presence false", }, { - labels: []string{"foo", "bar"}, - presence: true, - fits: true, - name: "all labels match, presence true", + presentLabels: []string{"foo", "bar"}, + fits: true, + name: "all labels match, presence true", }, { - labels: []string{"foo", "bar"}, - presence: false, - fits: false, - name: "all labels match, presence false", + absentLabels: []string{"foo", "bar"}, + fits: false, + name: "all labels match, presence false", }, } expectedFailureReasons := []PredicateFailureReason{ErrNodeLabelPresenceViolated} @@ -1709,7 +1703,7 @@ func TestNodeLabelPresence(t *testing.T) { nodeInfo := schedulernodeinfo.NewNodeInfo() nodeInfo.SetNode(&node) - labelChecker := NodeLabelChecker{test.labels, test.presence} + labelChecker := NodeLabelChecker{test.presentLabels, test.absentLabels} fits, reasons, err := labelChecker.CheckNodeLabelPresence(test.pod, GetPredicateMetadata(test.pod, nil), nodeInfo) if err != nil { t.Errorf("unexpected error: %v", err) diff --git a/pkg/scheduler/algorithm_factory.go b/pkg/scheduler/algorithm_factory.go index e431f0e4bac..90096aba6f6 100644 --- a/pkg/scheduler/algorithm_factory.go +++ b/pkg/scheduler/algorithm_factory.go @@ -287,17 +287,23 @@ func RegisterCustomFitPredicate(policy schedulerapi.PredicatePolicy, args *plugi } } else if policy.Argument.LabelsPresence != nil { // map LabelPresence policy to ConfigProducerArgs that's used to configure the NodeLabel plugin. - args.NodeLabelArgs = &nodelabel.Args{ - Labels: policy.Argument.LabelsPresence.Labels, - Presence: policy.Argument.LabelsPresence.Presence, + if args.NodeLabelArgs == nil { + args.NodeLabelArgs = &nodelabel.Args{} } - predicateFactory = func(args PluginFactoryArgs) predicates.FitPredicate { + if policy.Argument.LabelsPresence.Presence { + args.NodeLabelArgs.PresentLabels = append(args.NodeLabelArgs.PresentLabels, policy.Argument.LabelsPresence.Labels...) + } else { + args.NodeLabelArgs.AbsentLabels = append(args.NodeLabelArgs.AbsentLabels, policy.Argument.LabelsPresence.Labels...) + } + predicateFactory = func(_ PluginFactoryArgs) predicates.FitPredicate { return predicates.NewNodeLabelPredicate( - policy.Argument.LabelsPresence.Labels, - policy.Argument.LabelsPresence.Presence, + args.NodeLabelArgs.PresentLabels, + args.NodeLabelArgs.AbsentLabels, ) } - // We do not allow specifying the name for custom plugins, see #83472 + // We use the NodeLabel plugin name for all NodeLabel custom priorities. + // It may get called multiple times but we essentially only register one instance of NodeLabel predicate. + // This name is then used to find the registered plugin and run the plugin instead of the predicate. name = nodelabel.Name } } else if predicateFactory, ok = fitPredicateMap[policy.Name]; ok { diff --git a/pkg/scheduler/factory_test.go b/pkg/scheduler/factory_test.go index eae6cf2b0f4..7c8fcdb0ce6 100644 --- a/pkg/scheduler/factory_test.go +++ b/pkg/scheduler/factory_test.go @@ -92,6 +92,7 @@ func TestCreateFromConfig(t *testing.T) { "predicates" : [ {"name" : "TestZoneAffinity", "argument" : {"serviceAffinity" : {"labels" : ["zone"]}}}, {"name" : "TestRequireZone", "argument" : {"labelsPresence" : {"labels" : ["zone"], "presence" : true}}}, + {"name" : "TestNoFooLabel", "argument" : {"labelsPresence" : {"labels" : ["foo"], "presence" : false}}}, {"name" : "PredicateOne"}, {"name" : "PredicateTwo"} ], @@ -123,7 +124,7 @@ func TestCreateFromConfig(t *testing.T) { if err != nil { t.Errorf("Failed to marshal %+v: %v", nodeLabelConfig, err) } - want := `{"Name":"NodeLabel","Args":{"labels":["zone"],"presence":true}}` + want := `{"Name":"NodeLabel","Args":{"presentLabels":["zone"],"absentLabels":["foo"]}}` if string(encoding) != want { t.Errorf("Config for NodeLabel plugin mismatch. got: %v, want: %v", string(encoding), want) } diff --git a/pkg/scheduler/framework/plugins/nodelabel/node_label.go b/pkg/scheduler/framework/plugins/nodelabel/node_label.go index 909ffcab393..65da4622fee 100644 --- a/pkg/scheduler/framework/plugins/nodelabel/node_label.go +++ b/pkg/scheduler/framework/plugins/nodelabel/node_label.go @@ -18,6 +18,7 @@ package nodelabel import ( "context" + "fmt" v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/runtime" @@ -32,11 +33,24 @@ const Name = "NodeLabel" // Args holds the args that are used to configure the plugin. type Args struct { - // The list of labels that identify node "groups" - // All of the labels should be either present (or absent) for the node to be considered a fit for hosting the pod - Labels []string `json:"labels,omitempty"` - // The boolean flag that indicates whether the labels should be present or absent from the node - Presence bool `json:"presence,omitempty"` + // PresentLabels should be present for the node to be considered a fit for hosting the pod + PresentLabels []string `json:"presentLabels,omitempty"` + // AbsentLabels should be absent for the node to be considered a fit for hosting the pod + AbsentLabels []string `json:"absentLabels,omitempty"` +} + +// validateArgs validates that PresentLabels and AbsentLabels do not conflict. +func validateArgs(args *Args) error { + presentLabels := make(map[string]struct{}, len(args.PresentLabels)) + for _, l := range args.PresentLabels { + presentLabels[l] = struct{}{} + } + for _, l := range args.AbsentLabels { + if _, ok := presentLabels[l]; ok { + return fmt.Errorf("detecting at least one label (e.g., %q) that exist in both the present and absent label list: %+v", l, args) + } + } + return nil } // New initializes a new plugin and returns it. @@ -45,8 +59,11 @@ func New(plArgs *runtime.Unknown, _ framework.FrameworkHandle) (framework.Plugin if err := framework.DecodeInto(plArgs, args); err != nil { return nil, err } + if err := validateArgs(args); err != nil { + return nil, err + } return &NodeLabel{ - predicate: predicates.NewNodeLabelPredicate(args.Labels, args.Presence), + predicate: predicates.NewNodeLabelPredicate(args.PresentLabels, args.AbsentLabels), }, nil } diff --git a/pkg/scheduler/framework/plugins/nodelabel/node_label_test.go b/pkg/scheduler/framework/plugins/nodelabel/node_label_test.go index 2b057fffeb9..71764ac8d3a 100644 --- a/pkg/scheduler/framework/plugins/nodelabel/node_label_test.go +++ b/pkg/scheduler/framework/plugins/nodelabel/node_label_test.go @@ -27,42 +27,71 @@ import ( schedulernodeinfo "k8s.io/kubernetes/pkg/scheduler/nodeinfo" ) -func TestNodeLabelPresence(t *testing.T) { - label := map[string]string{"foo": "bar", "bar": "foo"} +func TestValidateNodeLabelArgs(t *testing.T) { + // "bar" exists in both present and absent labels therefore validatio should fail. + args := &runtime.Unknown{Raw: []byte(`{"presentLabels" : ["foo", "bar"], "absentLabels" : ["bar", "baz"]}`)} + _, err := New(args, nil) + if err == nil { + t.Fatal("Plugin initialization should fail.") + } +} + +func TestNodeLabelFilter(t *testing.T) { + label := map[string]string{"foo": "any value", "bar": "any value"} + var pod *v1.Pod tests := []struct { name string - pod *v1.Pod rawArgs string res framework.Code }{ { - name: "label does not match, presence true", - rawArgs: `{"labels" : ["baz"], "presence" : true}`, + name: "present label does not match", + rawArgs: `{"presentLabels" : ["baz"]}`, res: framework.UnschedulableAndUnresolvable, }, { - name: "label does not match, presence false", - rawArgs: `{"labels" : ["baz"], "presence" : false}`, + name: "absent label does not match", + rawArgs: `{"absentLabels" : ["baz"]}`, res: framework.Success, }, { - name: "one label matches, presence true", - rawArgs: `{"labels" : ["foo", "baz"], "presence" : true}`, + name: "one of two present labels matches", + rawArgs: `{"presentLabels" : ["foo", "baz"]}`, res: framework.UnschedulableAndUnresolvable, }, { - name: "one label matches, presence false", - rawArgs: `{"labels" : ["foo", "baz"], "presence" : false}`, + name: "one of two absent labels matches", + rawArgs: `{"absentLabels" : ["foo", "baz"]}`, res: framework.UnschedulableAndUnresolvable, }, { - name: "all labels match, presence true", - rawArgs: `{"labels" : ["foo", "bar"], "presence" : true}`, + name: "all present abels match", + rawArgs: `{"presentLabels" : ["foo", "bar"]}`, res: framework.Success, }, { - name: "all labels match, presence false", - rawArgs: `{"labels" : ["foo", "bar"], "presence" : false}`, + name: "all absent labels match", + rawArgs: `{"absentLabels" : ["foo", "bar"]}`, + res: framework.UnschedulableAndUnresolvable, + }, + { + name: "both present and absent label matches", + rawArgs: `{"presentLabels" : ["foo"], "absentLabels" : ["bar"]}`, + res: framework.UnschedulableAndUnresolvable, + }, + { + name: "neither present nor absent label matches", + rawArgs: `{"presentLabels" : ["foz"], "absentLabels" : ["baz"]}`, + res: framework.UnschedulableAndUnresolvable, + }, + { + name: "present label matches and absent label doesn't match", + rawArgs: `{"presentLabels" : ["foo"], "absentLabels" : ["baz"]}`, + res: framework.Success, + }, + { + name: "present label doesn't match and absent label matches", + rawArgs: `{"presentLabels" : ["foz"], "absentLabels" : ["bar"]}`, res: framework.UnschedulableAndUnresolvable, }, } @@ -79,7 +108,7 @@ func TestNodeLabelPresence(t *testing.T) { t.Fatalf("Failed to create plugin: %v", err) } - status := p.(framework.FilterPlugin).Filter(context.TODO(), nil, test.pod, nodeInfo) + status := p.(framework.FilterPlugin).Filter(context.TODO(), nil, pod, nodeInfo) if status.Code() != test.res { t.Errorf("Status mismatch. got: %v, want: %v", status.Code(), test.res) } diff --git a/test/integration/scheduler_perf/scheduler_bench_test.go b/test/integration/scheduler_perf/scheduler_bench_test.go index 703c48a3904..ab473a2c5bb 100644 --- a/test/integration/scheduler_perf/scheduler_bench_test.go +++ b/test/integration/scheduler_perf/scheduler_bench_test.go @@ -48,7 +48,7 @@ var ( // BenchmarkScheduling benchmarks the scheduling rate when the cluster has // various quantities of nodes and scheduled pods. -func BenchmarkScheduling(b *testing.B) { +func BenchmarkSchedulingV(b *testing.B) { tests := []struct{ nodes, existingPods, minPods int }{ {nodes: 100, existingPods: 0, minPods: 100}, {nodes: 100, existingPods: 1000, minPods: 100},