Merge pull request #98129 from chendave/expose_status

Expose node status so that external preemption plugins can use it

pkg/scheduler/framework/plugins/defaultpreemption/default_preemption.go

@@ -138,11 +138,23 @@ func (pl *DefaultPreemption) preempt(ctx context.Context, state *framework.Cycle
 	}
 
 	// 2) Find all preemption candidates.
-	candidates, err := pl.FindCandidates(ctx, state, pod, m)
+	candidates, nodeToStautsMap, err := pl.FindCandidates(ctx, state, pod, m)
-	if err != nil || len(candidates) == 0 {
+	if err != nil {
 		return "", err
 	}
 
+	// Return a FitError only when there are no candidates that fit the pod.
+	if len(candidates) == 0 {
+		return "", &framework.FitError{
+			Pod:         pod,
+			NumAllNodes: len(nodeToStautsMap),
+			Diagnosis: framework.Diagnosis{
+				NodeToStatusMap: nodeToStautsMap,
+				// Leave FailedPlugins as nil as it won't be used on moving Pods.
+			},
+		}
+	}
+
 	// 3) Interact with registered Extenders to filter out some candidates if needed.
 	candidates, err = CallExtenders(ph.Extenders(), pod, nodeLister, candidates)
 	if err != nil {
@@ -186,16 +198,15 @@ func (pl *DefaultPreemption) getOffsetAndNumCandidates(numNodes int32) (int32, i
 
 // FindCandidates calculates a slice of preemption candidates.
 // Each candidate is executable to make the given <pod> schedulable.
-func (pl *DefaultPreemption) FindCandidates(ctx context.Context, state *framework.CycleState, pod *v1.Pod, m framework.NodeToStatusMap) ([]Candidate, error) {
+func (pl *DefaultPreemption) FindCandidates(ctx context.Context, state *framework.CycleState, pod *v1.Pod, m framework.NodeToStatusMap) ([]Candidate, framework.NodeToStatusMap, error) {
 	allNodes, err := pl.fh.SnapshotSharedLister().NodeInfos().List()
 	if err != nil {
-		return nil, err
+		return nil, nil, err
 	}
 	if len(allNodes) == 0 {
-		return nil, fmt.Errorf("no nodes available")
+		return nil, nil, fmt.Errorf("no nodes available")
 	}
-
+	potentialNodes, unschedulableNodeStatus := nodesWherePreemptionMightHelp(allNodes, m)
-	potentialNodes := nodesWherePreemptionMightHelp(allNodes, m)
 	if len(potentialNodes) == 0 {
 		klog.V(3).Infof("Preemption will not help schedule pod %v/%v on any node.", pod.Namespace, pod.Name)
 		// In this case, we should clean-up any existing nominated node name of the pod.
@@ -203,12 +214,12 @@ func (pl *DefaultPreemption) FindCandidates(ctx context.Context, state *framewor
 			klog.Errorf("Cannot clear 'NominatedNodeName' field of pod %v/%v: %v", pod.Namespace, pod.Name, err)
 			// We do not return as this error is not critical.
 		}
-		return nil, nil
+		return nil, unschedulableNodeStatus, nil
 	}
 
 	pdbs, err := getPodDisruptionBudgets(pl.pdbLister)
 	if err != nil {
-		return nil, err
+		return nil, nil, err
 	}
 
 	offset, numCandidates := pl.getOffsetAndNumCandidates(int32(len(potentialNodes)))
@@ -220,18 +231,10 @@ func (pl *DefaultPreemption) FindCandidates(ctx context.Context, state *framewor
 		klog.Infof("from a pool of %d nodes (offset: %d, sample %d nodes: %v), ~%d candidates will be chosen", len(potentialNodes), offset, len(sample), sample, numCandidates)
 	}
 	candidates, nodeStatuses := dryRunPreemption(ctx, pl.fh, state, pod, potentialNodes, pdbs, offset, numCandidates)
-	// Return a FitError only when there are no candidates that fit the pod.
+	for node, status := range unschedulableNodeStatus {
-	if len(candidates) == 0 {
+		nodeStatuses[node] = status
-		return candidates, &framework.FitError{
-			Pod:         pod,
-			NumAllNodes: len(potentialNodes),
-			Diagnosis: framework.Diagnosis{
-				NodeToStatusMap: nodeStatuses,
-				// Leave FailedPlugins as nil as it won't be used on moving Pods.
-			},
-		}
 	}
-	return candidates, nil
+	return candidates, nodeStatuses, nil
 }
 
 // PodEligibleToPreemptOthers determines whether this pod should be considered
@@ -268,18 +271,20 @@ func PodEligibleToPreemptOthers(pod *v1.Pod, nodeInfos framework.NodeInfoLister,
 
 // nodesWherePreemptionMightHelp returns a list of nodes with failed predicates
 // that may be satisfied by removing pods from the node.
-func nodesWherePreemptionMightHelp(nodes []*framework.NodeInfo, m framework.NodeToStatusMap) []*framework.NodeInfo {
+func nodesWherePreemptionMightHelp(nodes []*framework.NodeInfo, m framework.NodeToStatusMap) ([]*framework.NodeInfo, framework.NodeToStatusMap) {
 	var potentialNodes []*framework.NodeInfo
+	nodeStatuses := make(framework.NodeToStatusMap)
 	for _, node := range nodes {
 		name := node.Node().Name
-		// We reply on the status by each plugin - 'Unschedulable' or 'UnschedulableAndUnresolvable'
+		// We rely on the status by each plugin - 'Unschedulable' or 'UnschedulableAndUnresolvable'
 		// to determine whether preemption may help or not on the node.
 		if m[name].Code() == framework.UnschedulableAndUnresolvable {
+			nodeStatuses[node.Node().Name] = framework.NewStatus(framework.UnschedulableAndUnresolvable, "Preemption is not helpful for scheduling")
 			continue
 		}
 		potentialNodes = append(potentialNodes, node)
 	}
-	return potentialNodes
+	return potentialNodes, nodeStatuses
 }
 
 type candidateList struct {

pkg/scheduler/framework/plugins/defaultpreemption/default_preemption_test.go

@@ -154,7 +154,7 @@ func TestPostFilter(t *testing.T) {
 				"node1": framework.NewStatus(framework.UnschedulableAndUnresolvable),
 			},
 			wantResult: nil,
-			wantStatus: framework.NewStatus(framework.Unschedulable),
+			wantStatus: framework.NewStatus(framework.Unschedulable, "0/1 nodes are available: 1 Preemption is not helpful for scheduling."),
 		},
 		{
 			name: "pod can be made schedulable on one node",
@@ -231,6 +231,26 @@ func TestPostFilter(t *testing.T) {
 			wantResult: nil,
 			wantStatus: framework.NewStatus(framework.Unschedulable, "0/2 nodes are available: 1 Insufficient cpu, 1 No victims found on node node1 for preemptor pod p."),
 		},
+		{
+			name: "no candidate nodes found with mixed reason, 2 UnschedulableAndUnresolvable nodes and 2 nodes don't have enough CPU resource",
+			pod:  st.MakePod().Name("p").UID("p").Namespace(v1.NamespaceDefault).Priority(highPriority).Req(largeRes).Obj(),
+			pods: []*v1.Pod{
+				st.MakePod().Name("p1").UID("p1").Namespace(v1.NamespaceDefault).Node("node1").Obj(),
+				st.MakePod().Name("p2").UID("p2").Namespace(v1.NamespaceDefault).Node("node2").Obj(),
+			},
+			nodes: []*v1.Node{
+				st.MakeNode().Name("node1").Capacity(nodeRes).Obj(),
+				st.MakeNode().Name("node2").Capacity(nodeRes).Obj(),
+				st.MakeNode().Name("node3").Capacity(nodeRes).Obj(),
+				st.MakeNode().Name("node4").Capacity(nodeRes).Obj(),
+			},
+			filteredNodesStatuses: framework.NodeToStatusMap{
+				"node3": framework.NewStatus(framework.UnschedulableAndUnresolvable),
+				"node4": framework.NewStatus(framework.UnschedulableAndUnresolvable),
+			},
+			wantResult: nil,
+			wantStatus: framework.NewStatus(framework.Unschedulable, "0/4 nodes are available: 2 Insufficient cpu, 2 Preemption is not helpful for scheduling."),
+		},
 	}
 
 	for _, tt := range tests {
@@ -1411,7 +1431,7 @@ func TestNodesWherePreemptionMightHelp(t *testing.T) {
 				ni.SetNode(st.MakeNode().Name(name).Obj())
 				nodeInfos = append(nodeInfos, ni)
 			}
-			nodes := nodesWherePreemptionMightHelp(nodeInfos, tt.nodesStatuses)
+			nodes, _ := nodesWherePreemptionMightHelp(nodeInfos, tt.nodesStatuses)
 			if len(tt.expected) != len(nodes) {
 				t.Errorf("number of nodes is not the same as expected. exptectd: %d, got: %d. Nodes: %v", len(tt.expected), len(nodes), nodes)
 			}