diff --git a/pkg/scheduler/algorithm/predicates/predicates.go b/pkg/scheduler/algorithm/predicates/predicates.go index 9a3fb1b534d..60f96c40f55 100644 --- a/pkg/scheduler/algorithm/predicates/predicates.go +++ b/pkg/scheduler/algorithm/predicates/predicates.go @@ -939,59 +939,6 @@ func PodFitsHost(pod *v1.Pod, meta Metadata, nodeInfo *schedulernodeinfo.NodeInf return false, []PredicateFailureReason{ErrPodNotMatchHostName}, nil } -// NodeLabelChecker contains information to check node labels for a predicate. -type NodeLabelChecker struct { - // 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(presentLabels []string, absentLabels []string) FitPredicate { - labelChecker := &NodeLabelChecker{ - presentLabels: presentLabels, - absentLabels: absentLabels, - } - return labelChecker.CheckNodeLabelPresence -} - -// CheckNodeLabelPresence checks whether all of the specified labels exists on a node or not, regardless of their value -// If "presence" is false, then returns false if any of the requested labels matches any of the node's labels, -// otherwise returns true. -// If "presence" is true, then returns false if any of the requested labels does not match any of the node's labels, -// otherwise returns true. -// -// Consider the cases where the nodes are placed in regions/zones/racks and these are identified by labels -// In some cases, it is required that only nodes that are part of ANY of the defined regions/zones/racks be selected -// -// Alternately, eliminating nodes that have a certain label, regardless of value, is also useful -// A node may have a label with "retiring" as key and the date as the value -// and it may be desirable to avoid scheduling new pods on this node. -func (n *NodeLabelChecker) CheckNodeLabelPresence(pod *v1.Pod, meta Metadata, nodeInfo *schedulernodeinfo.NodeInfo) (bool, []PredicateFailureReason, error) { - node := nodeInfo.Node() - if node == nil { - return false, nil, fmt.Errorf("node not found") - } - - nodeLabels := labels.Set(node.Labels) - check := func(labels []string, presence bool) bool { - for _, label := range labels { - exists := nodeLabels.Has(label) - if (exists && !presence) || (!exists && presence) { - return false - } - } - return true - } - if check(n.presentLabels, true) && check(n.absentLabels, false) { - return true, nil, nil - } - - return false, []PredicateFailureReason{ErrNodeLabelPresenceViolated}, nil -} - // PodFitsHostPorts is a wrapper around PodFitsHostPortsPredicate. This is needed until // we are able to get rid of the FitPredicate function signature. // TODO(#85822): remove this function once predicate registration logic is deleted. diff --git a/pkg/scheduler/algorithm/predicates/predicates_test.go b/pkg/scheduler/algorithm/predicates/predicates_test.go index 1d633c1ddfa..4766e71521e 100644 --- a/pkg/scheduler/algorithm/predicates/predicates_test.go +++ b/pkg/scheduler/algorithm/predicates/predicates_test.go @@ -1651,69 +1651,6 @@ func TestPodFitsSelector(t *testing.T) { } } -func TestNodeLabelPresence(t *testing.T) { - label := map[string]string{"foo": "bar", "bar": "foo"} - tests := []struct { - pod *v1.Pod - presentLabels []string - absentLabels []string - fits bool - name string - }{ - { - presentLabels: []string{"baz"}, - fits: false, - name: "label does not match, presence true", - }, - { - absentLabels: []string{"baz"}, - fits: true, - name: "label does not match, presence false", - }, - { - presentLabels: []string{"foo", "baz"}, - fits: false, - name: "one label matches, presence true", - }, - { - absentLabels: []string{"foo", "baz"}, - fits: false, - name: "one label matches, presence false", - }, - { - presentLabels: []string{"foo", "bar"}, - fits: true, - name: "all labels match, presence true", - }, - { - absentLabels: []string{"foo", "bar"}, - fits: false, - name: "all labels match, presence false", - }, - } - expectedFailureReasons := []PredicateFailureReason{ErrNodeLabelPresenceViolated} - - for _, test := range tests { - t.Run(test.name, func(t *testing.T) { - node := v1.Node{ObjectMeta: metav1.ObjectMeta{Labels: label}} - nodeInfo := schedulernodeinfo.NewNodeInfo() - nodeInfo.SetNode(&node) - - labelChecker := NodeLabelChecker{test.presentLabels, test.absentLabels} - fits, reasons, err := labelChecker.CheckNodeLabelPresence(test.pod, nil, nodeInfo) - if err != nil { - t.Errorf("unexpected error: %v", err) - } - if !fits && !reflect.DeepEqual(reasons, expectedFailureReasons) { - t.Errorf("unexpected failure reasons: %v, want: %v", reasons, expectedFailureReasons) - } - if fits != test.fits { - t.Errorf("expected: %v got %v", test.fits, fits) - } - }) - } -} - func newPodWithPort(hostPorts ...int) *v1.Pod { networkPorts := []v1.ContainerPort{} for _, port := range hostPorts { diff --git a/pkg/scheduler/algorithm_factory.go b/pkg/scheduler/algorithm_factory.go index abccb5c72a6..a3d99cbdd55 100644 --- a/pkg/scheduler/algorithm_factory.go +++ b/pkg/scheduler/algorithm_factory.go @@ -278,10 +278,7 @@ func RegisterCustomFitPredicate(policy schedulerapi.PredicatePolicy, pluginArgs pluginArgs.NodeLabelArgs.AbsentLabels = append(pluginArgs.NodeLabelArgs.AbsentLabels, policy.Argument.LabelsPresence.Labels...) } predicateFactory = func(_ AlgorithmFactoryArgs) predicates.FitPredicate { - return predicates.NewNodeLabelPredicate( - pluginArgs.NodeLabelArgs.PresentLabels, - pluginArgs.NodeLabelArgs.AbsentLabels, - ) + return nil } } } else if predicateFactory, ok = fitPredicateMap[policyName]; ok { diff --git a/pkg/scheduler/framework/plugins/nodelabel/node_label.go b/pkg/scheduler/framework/plugins/nodelabel/node_label.go index fdf60878294..aa8f0d242d2 100644 --- a/pkg/scheduler/framework/plugins/nodelabel/node_label.go +++ b/pkg/scheduler/framework/plugins/nodelabel/node_label.go @@ -71,16 +71,14 @@ func New(plArgs *runtime.Unknown, handle framework.FrameworkHandle) (framework.P return nil, err } return &NodeLabel{ - handle: handle, - predicate: predicates.NewNodeLabelPredicate(args.PresentLabels, args.AbsentLabels), - Args: args, + handle: handle, + Args: args, }, nil } // NodeLabel checks whether a pod can fit based on the node labels which match a filter that it requests. type NodeLabel struct { - handle framework.FrameworkHandle - predicate predicates.FitPredicate + handle framework.FrameworkHandle Args } @@ -93,10 +91,31 @@ func (pl *NodeLabel) Name() string { } // Filter invoked at the filter extension point. +// It checks whether all of the specified labels exists on a node or not, regardless of their value +// +// Consider the cases where the nodes are placed in regions/zones/racks and these are identified by labels +// In some cases, it is required that only nodes that are part of ANY of the defined regions/zones/racks be selected +// +// Alternately, eliminating nodes that have a certain label, regardless of value, is also useful +// A node may have a label with "retiring" as key and the date as the value +// and it may be desirable to avoid scheduling new pods on this node. func (pl *NodeLabel) Filter(ctx context.Context, _ *framework.CycleState, pod *v1.Pod, nodeInfo *nodeinfo.NodeInfo) *framework.Status { - // Note that NodeLabelPredicate doesn't use predicate metadata, hence passing nil here. - _, reasons, err := pl.predicate(pod, nil, nodeInfo) - return migration.PredicateResultToFrameworkStatus(reasons, err) + node := nodeInfo.Node() + nodeLabels := labels.Set(node.Labels) + check := func(labels []string, presence bool) bool { + for _, label := range labels { + exists := nodeLabels.Has(label) + if (exists && !presence) || (!exists && presence) { + return false + } + } + return true + } + if check(pl.PresentLabels, true) && check(pl.AbsentLabels, false) { + return nil + } + + return migration.PredicateResultToFrameworkStatus([]predicates.PredicateFailureReason{predicates.ErrNodeLabelPresenceViolated}, nil) } // Score invoked at the score extension point.