Merge pull request #110894 from yuanchen8911/prefilter

Consolidate PreFilter and Filter reason messages for scheduler

pkg/scheduler/framework/types.go

@@ -213,7 +213,9 @@ type WeightedAffinityTerm struct {
 type Diagnosis struct {
 	NodeToStatusMap      NodeToStatusMap
 	UnschedulablePlugins sets.String
-	// PostFilterMsg records the messages returned from PostFilterPlugins.
+	// PreFilterMsg records the messages returned from PreFilter plugins.
+	PreFilterMsg string
+	// PostFilterMsg records the messages returned from PostFilter plugins.
 	PostFilterMsg string
 }
 
@@ -224,10 +226,15 @@ type FitError struct {
 	Diagnosis   Diagnosis
 }
 
-// NoNodeAvailableMsg is used to format message when no nodes available.
+const (
-const NoNodeAvailableMsg = "0/%v nodes are available"
+	// NoNodeAvailableMsg is used to format message when no nodes available.
+	NoNodeAvailableMsg = "0/%v nodes are available"
+	// SeparatorFormat is used to separate PreFilterMsg, FilterMsg and PostFilterMsg.
+	SeparatorFormat = " %v."
+)
 
-// Error returns detailed information of why the pod failed to fit on each node
+// Error returns detailed information of why the pod failed to fit on each node.
+// A message format is "0/X nodes are available: <PreFilterMsg>. <FilterMsg>. <PostFilterMsg>."
 func (f *FitError) Error() string {
 	reasons := make(map[string]int)
 	for _, status := range f.Diagnosis.NodeToStatusMap {
@@ -236,6 +243,12 @@ func (f *FitError) Error() string {
 		}
 	}
 
+	reasonMsg := fmt.Sprintf(NoNodeAvailableMsg+":", f.NumAllNodes)
+	// Add the messages from PreFilter plugins to reasonMsg.
+	preFilterMsg := f.Diagnosis.PreFilterMsg
+	if preFilterMsg != "" {
+		reasonMsg += fmt.Sprintf(SeparatorFormat, preFilterMsg)
+	}
 	sortReasonsHistogram := func() []string {
 		var reasonStrings []string
 		for k, v := range reasons {
@@ -244,10 +257,14 @@ func (f *FitError) Error() string {
 		sort.Strings(reasonStrings)
 		return reasonStrings
 	}
-	reasonMsg := fmt.Sprintf(NoNodeAvailableMsg+": %v.", f.NumAllNodes, strings.Join(sortReasonsHistogram(), ", "))
+	sortedFilterMsg := sortReasonsHistogram()
+	if len(sortedFilterMsg) != 0 {
+		reasonMsg += fmt.Sprintf(SeparatorFormat, strings.Join(sortedFilterMsg, ", "))
+	}
+	// Add the messages from PostFilter plugins to reasonMsg.
 	postFilterMsg := f.Diagnosis.PostFilterMsg
 	if postFilterMsg != "" {
-		reasonMsg += " " + postFilterMsg
+		reasonMsg += fmt.Sprintf(SeparatorFormat, postFilterMsg)
 	}
 	return reasonMsg
 }

pkg/scheduler/framework/types_test.go

@@ -1373,3 +1373,88 @@ func TestGetNamespacesFromPodAffinityTerm(t *testing.T) {
 		})
 	}
 }
+
+func TestFitError_Error(t *testing.T) {
+	tests := []struct {
+		name          string
+		pod           *v1.Pod
+		numAllNodes   int
+		diagnosis     Diagnosis
+		wantReasonMsg string
+	}{
+		{
+			name:        "nodes failed Prefilter plugin",
+			numAllNodes: 3,
+			diagnosis: Diagnosis{
+				PreFilterMsg: "Node(s) failed PreFilter plugin FalsePreFilter",
+			},
+			wantReasonMsg: "0/3 nodes are available: Node(s) failed PreFilter plugin FalsePreFilter.",
+		},
+		{
+			name:        "nodes failed one Filter plugin with an empty PostFilterMsg",
+			numAllNodes: 3,
+			diagnosis: Diagnosis{
+				PreFilterMsg: "",
+				NodeToStatusMap: NodeToStatusMap{
+					"node1": NewStatus(Unschedulable, "Node(s) failed Filter plugin FalseFilter-1"),
+					"node2": NewStatus(Unschedulable, "Node(s) failed Filter plugin FalseFilter-1"),
+					"node3": NewStatus(Unschedulable, "Node(s) failed Filter plugin FalseFilter-1"),
+				},
+			},
+			wantReasonMsg: "0/3 nodes are available: 3 Node(s) failed Filter plugin FalseFilter-1.",
+		},
+		{
+			name:        "nodes failed one Filter plugin with a non-empty PostFilterMsg",
+			numAllNodes: 3,
+			diagnosis: Diagnosis{
+				PreFilterMsg: "",
+				NodeToStatusMap: NodeToStatusMap{
+					"node1": NewStatus(Unschedulable, "Node(s) failed Filter plugin FalseFilter-1"),
+					"node2": NewStatus(Unschedulable, "Node(s) failed Filter plugin FalseFilter-1"),
+					"node3": NewStatus(Unschedulable, "Node(s) failed Filter plugin FalseFilter-1"),
+				},
+				PostFilterMsg: "Error running PostFilter plugin FailedPostFilter",
+			},
+			wantReasonMsg: "0/3 nodes are available: 3 Node(s) failed Filter plugin FalseFilter-1. Error running PostFilter plugin FailedPostFilter.",
+		},
+		{
+			name:        "nodes failed two Filter plugins with an empty PostFilterMsg",
+			numAllNodes: 3,
+			diagnosis: Diagnosis{
+				PreFilterMsg: "",
+				NodeToStatusMap: NodeToStatusMap{
+					"node1": NewStatus(Unschedulable, "Node(s) failed Filter plugin FalseFilter-1"),
+					"node2": NewStatus(Unschedulable, "Node(s) failed Filter plugin FalseFilter-1"),
+					"node3": NewStatus(Unschedulable, "Node(s) failed Filter plugin FalseFilter-2"),
+				},
+			},
+			wantReasonMsg: "0/3 nodes are available: 1 Node(s) failed Filter plugin FalseFilter-2, 2 Node(s) failed Filter plugin FalseFilter-1.",
+		},
+		{
+			name:        "nodes failed two Filter plugins with a non-empty PostFilterMsg",
+			numAllNodes: 3,
+			diagnosis: Diagnosis{
+				PreFilterMsg: "",
+				NodeToStatusMap: NodeToStatusMap{
+					"node1": NewStatus(Unschedulable, "Node(s) failed Filter plugin FalseFilter-1"),
+					"node2": NewStatus(Unschedulable, "Node(s) failed Filter plugin FalseFilter-1"),
+					"node3": NewStatus(Unschedulable, "Node(s) failed Filter plugin FalseFilter-2"),
+				},
+				PostFilterMsg: "Error running PostFilter plugin FailedPostFilter",
+			},
+			wantReasonMsg: "0/3 nodes are available: 1 Node(s) failed Filter plugin FalseFilter-2, 2 Node(s) failed Filter plugin FalseFilter-1. Error running PostFilter plugin FailedPostFilter.",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			f := &FitError{
+				Pod:         tt.pod,
+				NumAllNodes: tt.numAllNodes,
+				Diagnosis:   tt.diagnosis,
+			}
+			if gotReasonMsg := f.Error(); gotReasonMsg != tt.wantReasonMsg {
+				t.Errorf("Error() = Got: %v Want: %v", gotReasonMsg, tt.wantReasonMsg)
+			}
+		})
+	}
+}

pkg/scheduler/schedule_one.go

@@ -398,11 +398,9 @@ func (sched *Scheduler) findNodesThatFitPod(ctx context.Context, fwk framework.F
 		if !s.IsUnschedulable() {
 			return nil, diagnosis, s.AsError()
 		}
-		// All nodes will have the same status. Some non trivial refactoring is
+		// Record the messages from PreFilter in Diagnosis.PreFilterMsg.
-		// needed to avoid this copy.
+		diagnosis.PreFilterMsg = s.Message()
-		for _, n := range allNodes {
+		klog.V(5).InfoS("Status after running PreFilter plugins for pod", "pod", klog.KObj(pod), "status", s)
-			diagnosis.NodeToStatusMap[n.Node().Name] = s
-		}
 		// Status satisfying IsUnschedulable() gets injected into diagnosis.UnschedulablePlugins.
 		if s.FailedPlugin() != "" {
 			diagnosis.UnschedulablePlugins.Insert(s.FailedPlugin())

pkg/scheduler/schedule_one_test.go

@@ -1694,10 +1694,8 @@ func TestSchedulerSchedulePod(t *testing.T) {
 				Pod:         st.MakePod().Name("ignore").UID("ignore").PVC("unknownPVC").Obj(),
 				NumAllNodes: 2,
 				Diagnosis: framework.Diagnosis{
-					NodeToStatusMap: framework.NodeToStatusMap{
+					NodeToStatusMap:      framework.NodeToStatusMap{},
-						"node1": framework.NewStatus(framework.UnschedulableAndUnresolvable, `persistentvolumeclaim "unknownPVC" not found`).WithFailedPlugin(volumebinding.Name),
+					PreFilterMsg:         `persistentvolumeclaim "unknownPVC" not found`,
-						"node2": framework.NewStatus(framework.UnschedulableAndUnresolvable, `persistentvolumeclaim "unknownPVC" not found`).WithFailedPlugin(volumebinding.Name),
-					},
 					UnschedulablePlugins: sets.NewString(volumebinding.Name),
 				},
 			},
@@ -1718,10 +1716,8 @@ func TestSchedulerSchedulePod(t *testing.T) {
 				Pod:         st.MakePod().Name("ignore").UID("ignore").Namespace(v1.NamespaceDefault).PVC("existingPVC").Obj(),
 				NumAllNodes: 2,
 				Diagnosis: framework.Diagnosis{
-					NodeToStatusMap: framework.NodeToStatusMap{
+					NodeToStatusMap:      framework.NodeToStatusMap{},
-						"node1": framework.NewStatus(framework.UnschedulableAndUnresolvable, `persistentvolumeclaim "existingPVC" is being deleted`).WithFailedPlugin(volumebinding.Name),
+					PreFilterMsg:         `persistentvolumeclaim "existingPVC" is being deleted`,
-						"node2": framework.NewStatus(framework.UnschedulableAndUnresolvable, `persistentvolumeclaim "existingPVC" is being deleted`).WithFailedPlugin(volumebinding.Name),
-					},
 					UnschedulablePlugins: sets.NewString(volumebinding.Name),
 				},
 			},
@@ -1878,10 +1874,8 @@ func TestSchedulerSchedulePod(t *testing.T) {
 				Pod:         st.MakePod().Name("test-prefilter").UID("test-prefilter").Obj(),
 				NumAllNodes: 2,
 				Diagnosis: framework.Diagnosis{
-					NodeToStatusMap: framework.NodeToStatusMap{
+					NodeToStatusMap:      framework.NodeToStatusMap{},
-						"1": framework.NewStatus(framework.UnschedulableAndUnresolvable, "injected unschedulable status").WithFailedPlugin("FakePreFilter"),
+					PreFilterMsg:         "injected unschedulable status",
-						"2": framework.NewStatus(framework.UnschedulableAndUnresolvable, "injected unschedulable status").WithFailedPlugin("FakePreFilter"),
-					},
 					UnschedulablePlugins: sets.NewString("FakePreFilter"),
 				},
 			},
@@ -1948,12 +1942,9 @@ func TestSchedulerSchedulePod(t *testing.T) {
 				Pod:         st.MakePod().Name("test-prefilter").UID("test-prefilter").Obj(),
 				NumAllNodes: 3,
 				Diagnosis: framework.Diagnosis{
-					NodeToStatusMap: framework.NodeToStatusMap{
+					NodeToStatusMap:      framework.NodeToStatusMap{},
-						"node1": framework.NewStatus(framework.Unschedulable, "node(s) didn't satisfy plugin(s) [FakePreFilter2 FakePreFilter3] simultaneously"),
-						"node2": framework.NewStatus(framework.Unschedulable, "node(s) didn't satisfy plugin(s) [FakePreFilter2 FakePreFilter3] simultaneously"),
-						"node3": framework.NewStatus(framework.Unschedulable, "node(s) didn't satisfy plugin(s) [FakePreFilter2 FakePreFilter3] simultaneously"),
-					},
 					UnschedulablePlugins: sets.String{},
+					PreFilterMsg:         "node(s) didn't satisfy plugin(s) [FakePreFilter2 FakePreFilter3] simultaneously",
 				},
 			},
 		},
@@ -1977,10 +1968,9 @@ func TestSchedulerSchedulePod(t *testing.T) {
 				Pod:         st.MakePod().Name("test-prefilter").UID("test-prefilter").Obj(),
 				NumAllNodes: 1,
 				Diagnosis: framework.Diagnosis{
-					NodeToStatusMap: framework.NodeToStatusMap{
+					NodeToStatusMap:      framework.NodeToStatusMap{},
-						"node1": framework.NewStatus(framework.Unschedulable, "node(s) didn't satisfy plugin FakePreFilter2"),
-					},
 					UnschedulablePlugins: sets.String{},
+					PreFilterMsg:         "node(s) didn't satisfy plugin FakePreFilter2",
 				},
 			},
 		},