scheduler: Extend ExtenderFilterResult to include UnschedulableAndUnresolvable nodes

pkg/scheduler/core/extender.go

@@ -269,11 +269,13 @@ func convertToNodeNameToMetaVictims(
 
 // Filter based on extender implemented predicate functions. The filtered list is
 // expected to be a subset of the supplied list; otherwise the function returns an error.
-// failedNodesMap optionally contains the list of failed nodes and failure reasons.
+// The failedNodes and failedAndUnresolvableNodes optionally contains the list
+// of failed nodes and failure reasons, except nodes in the latter are
+// unresolvable.
 func (h *HTTPExtender) Filter(
 	pod *v1.Pod,
 	nodes []*v1.Node,
-) ([]*v1.Node, extenderv1.FailedNodesMap, error) {
+) (filteredList []*v1.Node, failedNodes, failedAndUnresolvableNodes extenderv1.FailedNodesMap, err error) {
 	var (
 		result     extenderv1.ExtenderFilterResult
 		nodeList   *v1.NodeList
@@ -287,7 +289,7 @@ func (h *HTTPExtender) Filter(
 	}
 
 	if h.filterVerb == "" {
-		return nodes, extenderv1.FailedNodesMap{}, nil
+		return nodes, extenderv1.FailedNodesMap{}, extenderv1.FailedNodesMap{}, nil
 	}
 
 	if h.nodeCacheCapable {
@@ -310,10 +312,10 @@ func (h *HTTPExtender) Filter(
 	}
 
 	if err := h.send(h.filterVerb, args, &result); err != nil {
-		return nil, nil, err
+		return nil, nil, nil, err
 	}
 	if result.Error != "" {
-		return nil, nil, fmt.Errorf(result.Error)
+		return nil, nil, nil, fmt.Errorf(result.Error)
 	}
 
 	if h.nodeCacheCapable && result.NodeNames != nil {
@@ -322,7 +324,7 @@ func (h *HTTPExtender) Filter(
 			if n, ok := fromNodeName[nodeName]; ok {
 				nodeResult[i] = n
 			} else {
-				return nil, nil, fmt.Errorf(
+				return nil, nil, nil, fmt.Errorf(
 					"extender %q claims a filtered node %q which is not found in the input node list",
 					h.extenderURL, nodeName)
 			}
@@ -334,7 +336,7 @@ func (h *HTTPExtender) Filter(
 		}
 	}
 
-	return nodeResult, result.FailedNodes, nil
+	return nodeResult, result.FailedNodes, result.FailedAndUnresolvableNodes, nil
 }
 
 // Prioritize based on extender implemented priority functions. Weight*priority is added

pkg/scheduler/core/generic_scheduler.go

@@ -345,14 +345,23 @@ func (g *genericScheduler) findNodesThatPassFilters(
 }
 
 func (g *genericScheduler) findNodesThatPassExtenders(pod *v1.Pod, feasibleNodes []*v1.Node, statuses framework.NodeToStatusMap) ([]*v1.Node, error) {
+	// Extenders are called sequentially.
+	// Nodes in original feasibleNodes can be excluded in one extender, and pass on to the next
+	// extender in a decreasing manner.
 	for _, extender := range g.extenders {
 		if len(feasibleNodes) == 0 {
 			break
 		}
 		if !extender.IsInterested(pod) {
 			continue
 		}
-		feasibleList, failedMap, err := extender.Filter(pod, feasibleNodes)
+
+		// Status of failed nodes in failedAndUnresolvableMap will be added or overwritten in <statuses>,
+		// so that the scheduler framework can respect the UnschedulableAndUnresolvable status for
+		// particular nodes, and this may eventually improve preemption efficiency.
+		// Note: users are recommended to configure the extenders that may return UnschedulableAndUnresolvable
+		// status ahead of others.
+		feasibleList, failedMap, failedAndUnresolvableMap, err := extender.Filter(pod, feasibleNodes)
 		if err != nil {
 			if extender.IsIgnorable() {
 				klog.InfoS("Skipping extender as it returned error and has ignorable flag set", "extender", extender, "err", err)
@@ -361,13 +370,28 @@ func (g *genericScheduler) findNodesThatPassExtenders(pod *v1.Pod, feasibleNodes
 			return nil, err
 		}
 
+		for failedNodeName, failedMsg := range failedAndUnresolvableMap {
+			var aggregatedReasons []string
+			if _, found := statuses[failedNodeName]; found {
+				aggregatedReasons = statuses[failedNodeName].Reasons()
+			}
+			aggregatedReasons = append(aggregatedReasons, failedMsg)
+			statuses[failedNodeName] = framework.NewStatus(framework.UnschedulableAndUnresolvable, aggregatedReasons...)
+		}
+
 		for failedNodeName, failedMsg := range failedMap {
+			if _, found := failedAndUnresolvableMap[failedNodeName]; found {
+				// failedAndUnresolvableMap takes precedence over failedMap
+				// note that this only happens if the extender returns the node in both maps
+				continue
+			}
 			if _, found := statuses[failedNodeName]; !found {
 				statuses[failedNodeName] = framework.NewStatus(framework.Unschedulable, failedMsg)
 			} else {
 				statuses[failedNodeName].AppendReason(failedMsg)
 			}
 		}
+
 		feasibleNodes = feasibleList
 	}
 	return feasibleNodes, nil

pkg/scheduler/core/generic_scheduler_test.go

@@ -273,6 +273,180 @@ func TestSelectHost(t *testing.T) {
 	}
 }
 
+func TestFindNodesThatPassExtenders(t *testing.T) {
+	tests := []struct {
+		name                  string
+		extenders             []st.FakeExtender
+		nodes                 []*v1.Node
+		filteredNodesStatuses framework.NodeToStatusMap
+		expectsErr            bool
+		expectedNodes         []*v1.Node
+		expectedStatuses      framework.NodeToStatusMap
+	}{
+		{
+			name: "error",
+			extenders: []st.FakeExtender{
+				{
+					Predicates: []st.FitPredicate{st.ErrorPredicateExtender},
+				},
+			},
+			nodes:                 makeNodeList([]string{"a"}),
+			filteredNodesStatuses: make(framework.NodeToStatusMap),
+			expectsErr:            true,
+		},
+		{
+			name: "success",
+			extenders: []st.FakeExtender{
+				{
+					Predicates: []st.FitPredicate{st.TruePredicateExtender},
+				},
+			},
+			nodes:                 makeNodeList([]string{"a"}),
+			filteredNodesStatuses: make(framework.NodeToStatusMap),
+			expectsErr:            false,
+			expectedNodes:         makeNodeList([]string{"a"}),
+			expectedStatuses:      make(framework.NodeToStatusMap),
+		},
+		{
+			name: "unschedulable",
+			extenders: []st.FakeExtender{
+				{
+					Predicates: []st.FitPredicate{func(pod *v1.Pod, node *v1.Node) *framework.Status {
+						if node.Name == "a" {
+							return framework.NewStatus(framework.Success)
+						}
+						return framework.NewStatus(framework.Unschedulable, fmt.Sprintf("node %q is not allowed", node.Name))
+					}},
+				},
+			},
+			nodes:                 makeNodeList([]string{"a", "b"}),
+			filteredNodesStatuses: make(framework.NodeToStatusMap),
+			expectsErr:            false,
+			expectedNodes:         makeNodeList([]string{"a"}),
+			expectedStatuses: framework.NodeToStatusMap{
+				"b": framework.NewStatus(framework.Unschedulable, fmt.Sprintf("FakeExtender: node %q failed", "b")),
+			},
+		},
+		{
+			name: "unschedulable and unresolvable",
+			extenders: []st.FakeExtender{
+				{
+					Predicates: []st.FitPredicate{func(pod *v1.Pod, node *v1.Node) *framework.Status {
+						if node.Name == "a" {
+							return framework.NewStatus(framework.Success)
+						}
+						if node.Name == "b" {
+							return framework.NewStatus(framework.Unschedulable, fmt.Sprintf("node %q is not allowed", node.Name))
+						}
+						return framework.NewStatus(framework.UnschedulableAndUnresolvable, fmt.Sprintf("node %q is not allowed", node.Name))
+					}},
+				},
+			},
+			nodes:                 makeNodeList([]string{"a", "b", "c"}),
+			filteredNodesStatuses: make(framework.NodeToStatusMap),
+			expectsErr:            false,
+			expectedNodes:         makeNodeList([]string{"a"}),
+			expectedStatuses: framework.NodeToStatusMap{
+				"b": framework.NewStatus(framework.Unschedulable, fmt.Sprintf("FakeExtender: node %q failed", "b")),
+				"c": framework.NewStatus(framework.UnschedulableAndUnresolvable, fmt.Sprintf("FakeExtender: node %q failed and unresolvable", "c")),
+			},
+		},
+		{
+			name: "extender may overwrite the statuses",
+			extenders: []st.FakeExtender{
+				{
+					Predicates: []st.FitPredicate{func(pod *v1.Pod, node *v1.Node) *framework.Status {
+						if node.Name == "a" {
+							return framework.NewStatus(framework.Success)
+						}
+						if node.Name == "b" {
+							return framework.NewStatus(framework.Unschedulable, fmt.Sprintf("node %q is not allowed", node.Name))
+						}
+						return framework.NewStatus(framework.UnschedulableAndUnresolvable, fmt.Sprintf("node %q is not allowed", node.Name))
+					}},
+				},
+			},
+			nodes: makeNodeList([]string{"a", "b", "c"}),
+			filteredNodesStatuses: framework.NodeToStatusMap{
+				"c": framework.NewStatus(framework.Unschedulable, fmt.Sprintf("FakeFilterPlugin: node %q failed", "c")),
+			},
+			expectsErr:    false,
+			expectedNodes: makeNodeList([]string{"a"}),
+			expectedStatuses: framework.NodeToStatusMap{
+				"b": framework.NewStatus(framework.Unschedulable, fmt.Sprintf("FakeExtender: node %q failed", "b")),
+				"c": framework.NewStatus(framework.UnschedulableAndUnresolvable, fmt.Sprintf("FakeFilterPlugin: node %q failed", "c"), fmt.Sprintf("FakeExtender: node %q failed and unresolvable", "c")),
+			},
+		},
+		{
+			name: "multiple extenders",
+			extenders: []st.FakeExtender{
+				{
+					Predicates: []st.FitPredicate{func(pod *v1.Pod, node *v1.Node) *framework.Status {
+						if node.Name == "a" {
+							return framework.NewStatus(framework.Success)
+						}
+						if node.Name == "b" {
+							return framework.NewStatus(framework.Unschedulable, fmt.Sprintf("node %q is not allowed", node.Name))
+						}
+						return framework.NewStatus(framework.UnschedulableAndUnresolvable, fmt.Sprintf("node %q is not allowed", node.Name))
+					}},
+				},
+				{
+					Predicates: []st.FitPredicate{func(pod *v1.Pod, node *v1.Node) *framework.Status {
+						if node.Name == "a" {
+							return framework.NewStatus(framework.Success)
+						}
+						return framework.NewStatus(framework.Unschedulable, fmt.Sprintf("node %q is not allowed", node.Name))
+					}},
+				},
+			},
+			nodes:                 makeNodeList([]string{"a", "b", "c"}),
+			filteredNodesStatuses: make(framework.NodeToStatusMap),
+			expectsErr:            false,
+			expectedNodes:         makeNodeList([]string{"a"}),
+			expectedStatuses: framework.NodeToStatusMap{
+				"b": framework.NewStatus(framework.Unschedulable, fmt.Sprintf("FakeExtender: node %q failed", "b")),
+				"c": framework.NewStatus(framework.UnschedulableAndUnresolvable, fmt.Sprintf("FakeExtender: node %q failed and unresolvable", "c")),
+			},
+		},
+	}
+
+	cmpOpts := []cmp.Option{
+		cmp.Comparer(func(s1 framework.Status, s2 framework.Status) bool {
+			return s1.Code() == s2.Code() && reflect.DeepEqual(s1.Reasons(), s2.Reasons())
+		}),
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			var extenders []framework.Extender
+			for ii := range tt.extenders {
+				extenders = append(extenders, &tt.extenders[ii])
+			}
+			scheduler := &genericScheduler{
+				extenders: extenders,
+			}
+			pod := &v1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "1", UID: types.UID("1")}}
+			got, err := scheduler.findNodesThatPassExtenders(pod, tt.nodes, tt.filteredNodesStatuses)
+			if tt.expectsErr {
+				if err == nil {
+					t.Error("Unexpected non-error")
+				}
+			} else {
+				if err != nil {
+					t.Errorf("Unexpected error: %v", err)
+				}
+				if diff := cmp.Diff(tt.expectedNodes, got); diff != "" {
+					t.Errorf("filtered nodes (-want,+got):\n%s", diff)
+				}
+				if diff := cmp.Diff(tt.expectedStatuses, tt.filteredNodesStatuses, cmpOpts...); diff != "" {
+					t.Errorf("filtered statuses (-want,+got):\n%s", diff)
+				}
+			}
+		})
+	}
+}
+
 func TestGenericScheduler(t *testing.T) {
 	tests := []struct {
 		name            string

pkg/scheduler/factory_test.go

@@ -674,8 +674,8 @@ func (f *fakeExtender) SupportsPreemption() bool {
 	return false
 }
 
-func (f *fakeExtender) Filter(pod *v1.Pod, nodes []*v1.Node) (filteredNodes []*v1.Node, failedNodesMap extenderv1.FailedNodesMap, err error) {
-	return nil, nil, nil
+func (f *fakeExtender) Filter(pod *v1.Pod, nodes []*v1.Node) ([]*v1.Node, extenderv1.FailedNodesMap, extenderv1.FailedNodesMap, error) {
+	return nil, nil, nil, nil
 }
 
 func (f *fakeExtender) Prioritize(

pkg/scheduler/framework/extender.go

@@ -29,9 +29,11 @@ type Extender interface {
 	Name() string
 
 	// Filter based on extender-implemented predicate functions. The filtered list is
-	// expected to be a subset of the supplied list. failedNodesMap optionally contains
-	// the list of failed nodes and failure reasons.
-	Filter(pod *v1.Pod, nodes []*v1.Node) (filteredNodes []*v1.Node, failedNodesMap extenderv1.FailedNodesMap, err error)
+	// expected to be a subset of the supplied list.
+	// The failedNodes and failedAndUnresolvableNodes optionally contains the list
+	// of failed nodes and failure reasons, except nodes in the latter are
+	// unresolvable.
+	Filter(pod *v1.Pod, nodes []*v1.Node) (filteredNodes []*v1.Node, failedNodesMap extenderv1.FailedNodesMap, failedAndUnresolvable extenderv1.FailedNodesMap, err error)
 
 	// Prioritize based on extender-implemented priority functions. The returned scores & weight
 	// are used to compute the weighted score for an extender. The weighted scores are added to