scheduler: refine reservation msg when no owner matched

Signed-off-by: saintube <saintube@foxmail.com>

pkg/scheduler/plugins/reservation/plugin.go

@@ -154,16 +154,15 @@ type stateData struct {
 	preemptible          map[string]corev1.ResourceList
 	preemptibleInRRs     map[string]map[types.UID]corev1.ResourceList
 
-	nodeReservationStates map[string]nodeReservationState
-	preferredNode         string
-	assumed               *frameworkext.ReservationInfo
+	nodeReservationStates    map[string]nodeReservationState
+	nodeReservationDiagnosis map[string]nodeDiagnosisState
+	preferredNode            string
+	assumed                  *frameworkext.ReservationInfo
 }
 
 type nodeReservationState struct {
-	nodeName                 string
-	matched                  []*frameworkext.ReservationInfo // owner and affinity matched, not unschedulable
-	isUnschedulableUnmatched int                             // owner matched but unmatched due to unschedulable
-	affinityUnmatched        int                             // owner matched but unmatched due to affinity
+	nodeName string
+	matched  []*frameworkext.ReservationInfo
 
 	// podRequested represents all Pods(including matched reservation) requested resources
 	// but excluding the already allocated from unmatched reservations
@@ -172,14 +171,22 @@ type nodeReservationState struct {
 	rAllocated *framework.Resource
 }
 
+type nodeDiagnosisState struct {
+	nodeName                 string
+	ownerMatched             int // owner matched
+	isUnschedulableUnmatched int // owner matched but unmatched due to unschedulable
+	affinityUnmatched        int // owner matched but unmatched due to affinity
+}
+
 func (s *stateData) Clone() framework.StateData {
 	ns := &stateData{
-		hasAffinity:           s.hasAffinity,
-		podRequests:           s.podRequests,
-		podRequestsResources:  s.podRequestsResources,
-		nodeReservationStates: s.nodeReservationStates,
-		preferredNode:         s.preferredNode,
-		assumed:               s.assumed,
+		hasAffinity:              s.hasAffinity,
+		podRequests:              s.podRequests,
+		podRequestsResources:     s.podRequestsResources,
+		nodeReservationStates:    s.nodeReservationStates,
+		nodeReservationDiagnosis: s.nodeReservationDiagnosis,
+		preferredNode:            s.preferredNode,
+		assumed:                  s.assumed,
 	}
 	preemptible := map[string]corev1.ResourceList{}
 	for nodeName, returned := range s.preemptible {
@@ -509,12 +516,12 @@ func (pl *Plugin) PostFilter(ctx context.Context, cycleState *framework.CycleSta
 
 func (pl *Plugin) makePostFilterReasons(state *stateData) []string {
 	ownerMatched, affinityUnmatched, isUnSchedulableUnmatched := 0, 0, 0
-	for _, nodeState := range state.nodeReservationStates {
+	for _, nodeState := range state.nodeReservationDiagnosis {
 		isUnSchedulableUnmatched += nodeState.isUnschedulableUnmatched
 		affinityUnmatched += nodeState.affinityUnmatched
-		ownerMatched += len(nodeState.matched) + nodeState.isUnschedulableUnmatched + nodeState.affinityUnmatched
+		ownerMatched += nodeState.ownerMatched
 	}
-	if ownerMatched <= 0 {
+	if ownerMatched <= 0 && !state.hasAffinity {
 		return nil
 	}
 
@@ -524,7 +531,7 @@ func (pl *Plugin) makePostFilterReasons(state *stateData) []string {
 	var b strings.Builder
 	if affinityUnmatched > 0 {
 		b.WriteString(strconv.Itoa(affinityUnmatched))
-		b.WriteString(" Reservation(s) owner match but bad affinity")
+		b.WriteString(" Reservation(s) didn't match affinity rules")
 		reasons = append(reasons, b.String())
 		b.Reset()
 	}
@@ -535,7 +542,7 @@ func (pl *Plugin) makePostFilterReasons(state *stateData) []string {
 		b.Reset()
 	}
 	b.WriteString(strconv.Itoa(ownerMatched))
-	b.WriteString(" Reservation(s) owner matched total")
+	b.WriteString(" Reservation(s) matched owner total")
 	reasons = append(reasons, b.String())
 	return reasons
 }

pkg/scheduler/plugins/reservation/plugin_test.go

@@ -1835,8 +1835,9 @@ func TestFilterReservation(t *testing.T) {
 
 func TestPostFilter(t *testing.T) {
 	type args struct {
-		hasStateData          bool
-		nodeReservationStates map[string]nodeReservationState
+		hasStateData             bool
+		hasAffinity              bool
+		nodeReservationDiagnosis map[string]nodeDiagnosisState
 	}
 	tests := []struct {
 		name  string
@@ -1852,45 +1853,55 @@ func TestPostFilter(t *testing.T) {
 			want:  nil,
 			want1: framework.NewStatus(framework.Unschedulable),
 		},
+		{
+			name: "show reservation owner matched when reservation affinity specified",
+			args: args{
+				hasStateData:             true,
+				hasAffinity:              true,
+				nodeReservationDiagnosis: map[string]nodeDiagnosisState{},
+			},
+			want:  nil,
+			want1: framework.NewStatus(framework.Unschedulable, "0 Reservation(s) matched owner total"),
+		},
 		{
 			name: "show reservation owner matched, unschedulable unmatched",
 			args: args{
 				hasStateData: true,
-				nodeReservationStates: map[string]nodeReservationState{
+				nodeReservationDiagnosis: map[string]nodeDiagnosisState{
 					"test-node-0": {
-						matched:                  make([]*frameworkext.ReservationInfo, 0),
+						ownerMatched:             3,
 						isUnschedulableUnmatched: 3,
 						affinityUnmatched:        0,
 					},
 					"test-node-1": {
-						matched:                  make([]*frameworkext.ReservationInfo, 0),
+						ownerMatched:             1,
 						isUnschedulableUnmatched: 1,
 						affinityUnmatched:        0,
 					},
 				},
 			},
 			want:  nil,
-			want1: framework.NewStatus(framework.Unschedulable, "4 Reservation(s) is unschedulable", "4 Reservation(s) owner matched total"),
+			want1: framework.NewStatus(framework.Unschedulable, "4 Reservation(s) is unschedulable", "4 Reservation(s) matched owner total"),
 		},
 		{
 			name: "show reservation owner matched, unschedulable and affinity unmatched",
 			args: args{
 				hasStateData: true,
-				nodeReservationStates: map[string]nodeReservationState{
+				nodeReservationDiagnosis: map[string]nodeDiagnosisState{
 					"test-node-0": {
-						matched:                  make([]*frameworkext.ReservationInfo, 0),
+						ownerMatched:             3,
 						isUnschedulableUnmatched: 0,
 						affinityUnmatched:        3,
 					},
 					"test-node-1": {
-						matched:                  make([]*frameworkext.ReservationInfo, 0),
+						ownerMatched:             2,
 						isUnschedulableUnmatched: 1,
 						affinityUnmatched:        1,
 					},
 				},
 			},
 			want:  nil,
-			want1: framework.NewStatus(framework.Unschedulable, "4 Reservation(s) owner match but bad affinity", "1 Reservation(s) is unschedulable", "5 Reservation(s) owner matched total"),
+			want1: framework.NewStatus(framework.Unschedulable, "4 Reservation(s) didn't match affinity rules", "1 Reservation(s) is unschedulable", "5 Reservation(s) matched owner total"),
 		},
 	}
 	for _, tt := range tests {
@@ -1899,7 +1910,8 @@ func TestPostFilter(t *testing.T) {
 			cycleState := framework.NewCycleState()
 			if tt.args.hasStateData {
 				cycleState.Write(stateKey, &stateData{
-					nodeReservationStates: tt.args.nodeReservationStates,
+					hasAffinity:              tt.args.hasAffinity,
+					nodeReservationDiagnosis: tt.args.nodeReservationDiagnosis,
 				})
 			}
 			got, got1 := pl.PostFilter(context.TODO(), cycleState, nil, nil)

pkg/scheduler/plugins/reservation/transformer.go

@@ -61,9 +61,10 @@ func (pl *Plugin) prepareMatchReservationState(ctx context.Context, cycleState *
 	}
 	requiredNodeAffinity := nodeaffinity.GetRequiredNodeAffinity(pod)
 
-	var stateIndex int32
+	var stateIndex, diagnosisIndex int32
 	allNodes := pl.reservationCache.listAllNodes()
 	allNodeReservationStates := make([]*nodeReservationState, len(allNodes))
+	allNodeDiagnosisStates := make([]*nodeDiagnosisState, len(allNodes))
 	allPluginToRestoreState := make([]frameworkext.PluginToReservationRestoreStates, len(allNodes))
 
 	isReservedPod := reservationutil.IsReservePod(pod)
@@ -100,7 +101,12 @@ func (pl *Plugin) prepareMatchReservationState(ctx context.Context, cycleState *
 		}
 
 		var unmatched, matched []*frameworkext.ReservationInfo
-		isUnschedulableUnmatched, affinityUnmatched := 0, 0
+		diagnosisState := &nodeDiagnosisState{
+			nodeName:                 node.Name,
+			ownerMatched:             0,
+			isUnschedulableUnmatched: 0,
+			affinityUnmatched:        0,
+		}
 		status := pl.reservationCache.forEachAvailableReservationOnNode(node.Name, func(rInfo *frameworkext.ReservationInfo) (bool, *framework.Status) {
 			if !rInfo.IsAvailable() || rInfo.ParseError != nil {
 				return true, nil
@@ -112,18 +118,24 @@ func (pl *Plugin) prepareMatchReservationState(ctx context.Context, cycleState *
 				return true, nil
 			}
 
-			if !isReservedPod && rInfo.Match(pod) { // owner matched
-				if rInfo.IsUnschedulable() {
-					isUnschedulableUnmatched++
-				} else if !matchReservationAffinity(node, rInfo, reservationAffinity) {
-					affinityUnmatched++
-				} else {
-					matched = append(matched, rInfo.Clone())
-				}
+			isOwnerMatched := rInfo.Match(pod)
+			isUnschedulable := rInfo.IsUnschedulable()
+			isMatchReservationAffinity := matchReservationAffinity(node, rInfo, reservationAffinity)
+			if !isReservedPod && !isUnschedulable && isOwnerMatched && isMatchReservationAffinity {
+				matched = append(matched, rInfo.Clone())
 
 			} else if len(rInfo.AssignedPods) > 0 {
 				unmatched = append(unmatched, rInfo.Clone())
 			}
+			if isOwnerMatched { // count owner-matched diagnosis state
+				diagnosisState.ownerMatched++
+				if isUnschedulable {
+					diagnosisState.isUnschedulableUnmatched++
+				} else if !isMatchReservationAffinity {
+					diagnosisState.affinityUnmatched++
+				}
+			}
+
 			return true, nil
 		})
 		if !status.IsSuccess() {
@@ -133,6 +145,11 @@ func (pl *Plugin) prepareMatchReservationState(ctx context.Context, cycleState *
 			return
 		}
 
+		if diagnosisState.ownerMatched > 0 {
+			idx := atomic.AddInt32(&diagnosisIndex, 1)
+			allNodeDiagnosisStates[idx-1] = diagnosisState
+		}
+
 		if len(matched) == 0 && len(unmatched) == 0 {
 			return
 		}
@@ -189,12 +206,10 @@ func (pl *Plugin) prepareMatchReservationState(ctx context.Context, cycleState *
 		if len(matched) > 0 || len(unmatched) > 0 {
 			index := atomic.AddInt32(&stateIndex, 1)
 			allNodeReservationStates[index-1] = &nodeReservationState{
-				nodeName:                 node.Name,
-				matched:                  matched,
-				isUnschedulableUnmatched: isUnschedulableUnmatched,
-				affinityUnmatched:        affinityUnmatched,
-				podRequested:             podRequested,
-				rAllocated:               framework.NewResource(rAllocated),
+				nodeName:     node.Name,
+				matched:      matched,
+				podRequested: podRequested,
+				rAllocated:   framework.NewResource(rAllocated),
 			}
 			allPluginToRestoreState[index-1] = pluginToRestoreState
 		}
@@ -208,19 +223,22 @@ func (pl *Plugin) prepareMatchReservationState(ctx context.Context, cycleState *
 
 	allNodeReservationStates = allNodeReservationStates[:stateIndex]
 	allPluginToRestoreState = allPluginToRestoreState[:stateIndex]
+	allNodeDiagnosisStates = allNodeDiagnosisStates[:diagnosisIndex]
 
 	podRequests := resourceapi.PodRequests(pod, resourceapi.PodResourcesOptions{})
 	podRequestResources := framework.NewResource(podRequests)
 	state := &stateData{
-		hasAffinity:           reservationAffinity != nil,
-		podRequests:           podRequests,
-		podRequestsResources:  podRequestResources,
-		preemptible:           map[string]corev1.ResourceList{},
-		preemptibleInRRs:      map[string]map[types.UID]corev1.ResourceList{},
-		nodeReservationStates: map[string]nodeReservationState{},
+		hasAffinity:              reservationAffinity != nil,
+		podRequests:              podRequests,
+		podRequestsResources:     podRequestResources,
+		preemptible:              map[string]corev1.ResourceList{},
+		preemptibleInRRs:         map[string]map[types.UID]corev1.ResourceList{},
+		nodeReservationStates:    map[string]nodeReservationState{},
+		nodeReservationDiagnosis: map[string]nodeDiagnosisState{},
 	}
 	pluginToNodeReservationRestoreState := frameworkext.PluginToNodeReservationRestoreStates{}
-	for index, v := range allNodeReservationStates {
+	for index := range allNodeReservationStates {
+		v := allNodeReservationStates[index]
 		state.nodeReservationStates[v.nodeName] = *v
 		for pluginName, pluginState := range allPluginToRestoreState[index] {
 			if pluginState == nil {
@@ -234,6 +252,10 @@ func (pl *Plugin) prepareMatchReservationState(ctx context.Context, cycleState *
 			nodeRestoreStates[v.nodeName] = pluginState
 		}
 	}
+	for i := range allNodeDiagnosisStates {
+		v := allNodeDiagnosisStates[i]
+		state.nodeReservationDiagnosis[v.nodeName] = *v
+	}
 	if extender != nil {
 		status := extender.RunReservationExtensionFinalRestoreReservation(ctx, cycleState, pod, pluginToNodeReservationRestoreState)
 		if !status.IsSuccess() {

pkg/scheduler/plugins/reservation/transformer_test.go

@@ -322,6 +322,14 @@ func TestRestoreReservation(t *testing.T) {
 				rAllocated: framework.NewResource(nil),
 			},
 		},
+		nodeReservationDiagnosis: map[string]nodeDiagnosisState{
+			node.Name: {
+				nodeName:                 node.Name,
+				ownerMatched:             1,
+				affinityUnmatched:        0,
+				isUnschedulableUnmatched: 0,
+			},
+		},
 	}
 	assert.Equal(t, expectedStat, getStateData(cycleState))