build message in scheduler & update tests (#825)

Azure · May 24, 2024 · 1d87d24 · 1d87d24
1 parent af2840b
commit 1d87d24
Show file tree

Hide file tree

Showing 4 changed files with 54 additions and 59 deletions.
diff --git a/pkg/controllers/clusterresourceplacement/placement_status.go b/pkg/controllers/clusterresourceplacement/placement_status.go
@@ -47,12 +47,6 @@ const (
 	ResourceScheduleSucceededReason = "ScheduleSucceeded"
 	// ResourceScheduleFailedReason is the reason string of placement condition when the scheduler failed to schedule the selected resources.
 	ResourceScheduleFailedReason = "ScheduleFailed"
-
-	// ResourcePlacementStatus schedule condition message formats
-	resourcePlacementConditionScheduleFailedMessageFormat             = "%s is not selected: %s"
-	resourcePlacementConditionScheduleFailedWithScoreMessageFormat    = "%s is not selected with clusterScore %+v: %s"
-	resourcePlacementConditionScheduleSucceededMessageFormat          = "Successfully scheduled resources for placement in %s: %s"
-	resourcePlacementConditionScheduleSucceededWithScoreMessageFormat = "Successfully scheduled resources for placement in %s (affinity score: %d, topology spread score: %d): %s"
 )
 
 // setResourceConditions sets the resource related conditions by looking at the bindings and work, excluding the scheduled condition.
@@ -91,13 +85,10 @@ func (r *Reconciler) setResourceConditions(ctx context.Context, crp *fleetv1beta
 			Status:             metav1.ConditionTrue,
 			Type:               string(fleetv1beta1.ResourceScheduledConditionType),
 			Reason:             condition.ScheduleSucceededReason,
-			Message:            fmt.Sprintf(resourcePlacementConditionScheduleSucceededMessageFormat, c.ClusterName, c.Reason),
+			Message:            c.Reason,
 			ObservedGeneration: crp.Generation,
 		}
 		rps.ClusterName = c.ClusterName
-		if c.ClusterScore != nil {
-			scheduledCondition.Message = fmt.Sprintf(resourcePlacementConditionScheduleSucceededWithScoreMessageFormat, c.ClusterName, *c.ClusterScore.AffinityScore, *c.ClusterScore.TopologySpreadScore, c.Reason)
-		}
 		oldConditions, ok := oldResourcePlacementStatusMap[c.ClusterName]
 		if ok {
 			// update the lastTransitionTime considering the existing condition status instead of overwriting
@@ -136,12 +127,10 @@ func (r *Reconciler) setResourceConditions(ctx context.Context, crp *fleetv1beta
 			Status:             metav1.ConditionFalse,
 			Type:               string(fleetv1beta1.ResourceScheduledConditionType),
 			Reason:             ResourceScheduleFailedReason,
-			Message:            fmt.Sprintf(resourcePlacementConditionScheduleFailedMessageFormat, unselected[i].ClusterName, unselected[i].Reason),
+			Message:            unselected[i].Reason,
 			ObservedGeneration: crp.Generation,
 		}
-		if unselected[i].ClusterScore != nil {
-			scheduledCondition.Message = fmt.Sprintf(resourcePlacementConditionScheduleFailedWithScoreMessageFormat, unselected[i].ClusterName, unselected[i].ClusterScore, unselected[i].Reason)
-		}
+
 		meta.SetStatusCondition(&rp.Conditions, scheduledCondition)
 		placementStatuses = append(placementStatuses, rp)
 		klog.V(2).InfoS("Populated the resource placement status for the unscheduled cluster", "clusterResourcePlacement", klog.KObj(crp), "cluster", unselected[i].ClusterName)

diff --git a/pkg/scheduler/framework/framework.go b/pkg/scheduler/framework/framework.go
@@ -40,9 +40,9 @@ const (
 
 	// The reasons to use for scheduling decisions.
 	pickedByPolicyReason                  = "picked by scheduling policy"
-	pickFixedInvalidClusterReasonTemplate = "cluster is not eligible for resource placement yet: %s"
-	pickFixedNotFoundClusterReason        = "specified cluster is not found"
-	notPickedByScoreReason                = "cluster does not score high enough"
+	pickFixedInvalidClusterReasonTemplate = "cluster \"%s\" is not eligible for resource placement yet: %s"
+	pickFixedNotFoundClusterReason        = "specified cluster \"%s\" is not found"
+	notPickedByScoreReason                = "cluster \"%s\" does not score high enough (affinity score: %d, topology spread score: %d)"
 
 	// FullyScheduledReason is the reason string of placement condition when the placement is scheduled.
 	FullyScheduledReason = "SchedulingPolicyFulfilled"

diff --git a/pkg/scheduler/framework/framework_test.go b/pkg/scheduler/framework/framework_test.go
@@ -1286,7 +1286,7 @@ func TestCrossReferencePickedClustersAndDeDupBindings(t *testing.T) {
 								AffinityScore:       &affinityScore1,
 								TopologySpreadScore: &topologySpreadScore1,
 							},
-							Reason: pickedByPolicyReason,
+							Reason: fmt.Sprintf(resourceScheduleSucceededWithScoreMessageFormat, clusterName1, affinityScore1, topologySpreadScore1, pickedByPolicyReason),
 						},
 					},
 				},
@@ -1308,7 +1308,7 @@ func TestCrossReferencePickedClustersAndDeDupBindings(t *testing.T) {
 								AffinityScore:       &affinityScore2,
 								TopologySpreadScore: &topologySpreadScore2,
 							},
-							Reason: pickedByPolicyReason,
+							Reason: fmt.Sprintf(resourceScheduleSucceededWithScoreMessageFormat, clusterName2, affinityScore2, topologySpreadScore2, pickedByPolicyReason),
 						},
 					},
 				},
@@ -1330,7 +1330,7 @@ func TestCrossReferencePickedClustersAndDeDupBindings(t *testing.T) {
 								AffinityScore:       &affinityScore3,
 								TopologySpreadScore: &topologySpreadScore3,
 							},
-							Reason: pickedByPolicyReason,
+							Reason: fmt.Sprintf(resourceScheduleSucceededWithScoreMessageFormat, clusterName3, affinityScore3, topologySpreadScore3, pickedByPolicyReason),
 						},
 					},
 				},
@@ -1397,7 +1397,7 @@ func TestCrossReferencePickedClustersAndDeDupBindings(t *testing.T) {
 									AffinityScore:       &affinityScore1,
 									TopologySpreadScore: &topologySpreadScore1,
 								},
-								Reason: pickedByPolicyReason,
+								Reason: fmt.Sprintf(resourceScheduleSucceededWithScoreMessageFormat, clusterName1, affinityScore1, topologySpreadScore1, pickedByPolicyReason),
 							},
 						},
 					},
@@ -1426,7 +1426,7 @@ func TestCrossReferencePickedClustersAndDeDupBindings(t *testing.T) {
 									AffinityScore:       &affinityScore2,
 									TopologySpreadScore: &topologySpreadScore2,
 								},
-								Reason: pickedByPolicyReason,
+								Reason: fmt.Sprintf(resourceScheduleSucceededWithScoreMessageFormat, clusterName2, affinityScore2, topologySpreadScore2, pickedByPolicyReason),
 							},
 						},
 					},
@@ -1455,7 +1455,7 @@ func TestCrossReferencePickedClustersAndDeDupBindings(t *testing.T) {
 									AffinityScore:       &affinityScore3,
 									TopologySpreadScore: &topologySpreadScore3,
 								},
-								Reason: pickedByPolicyReason,
+								Reason: fmt.Sprintf(resourceScheduleSucceededWithScoreMessageFormat, clusterName3, affinityScore3, topologySpreadScore3, pickedByPolicyReason),
 							},
 						},
 					},
@@ -1522,7 +1522,7 @@ func TestCrossReferencePickedClustersAndDeDupBindings(t *testing.T) {
 								AffinityScore:       &affinityScore3,
 								TopologySpreadScore: &topologySpreadScore3,
 							},
-							Reason: pickedByPolicyReason,
+							Reason: fmt.Sprintf(resourceScheduleSucceededWithScoreMessageFormat, clusterName3, affinityScore3, topologySpreadScore3, pickedByPolicyReason),
 						},
 					},
 				},
@@ -1544,7 +1544,7 @@ func TestCrossReferencePickedClustersAndDeDupBindings(t *testing.T) {
 									AffinityScore:       &affinityScore1,
 									TopologySpreadScore: &topologySpreadScore1,
 								},
-								Reason: pickedByPolicyReason,
+								Reason: fmt.Sprintf(resourceScheduleSucceededWithScoreMessageFormat, clusterName1, affinityScore1, topologySpreadScore1, pickedByPolicyReason),
 							},
 						},
 					},
@@ -1573,7 +1573,7 @@ func TestCrossReferencePickedClustersAndDeDupBindings(t *testing.T) {
 									AffinityScore:       &affinityScore2,
 									TopologySpreadScore: &topologySpreadScore2,
 								},
-								Reason: pickedByPolicyReason,
+								Reason: fmt.Sprintf(resourceScheduleSucceededWithScoreMessageFormat, clusterName2, affinityScore2, topologySpreadScore2, pickedByPolicyReason),
 							},
 						},
 					},
@@ -1631,7 +1631,7 @@ func TestCrossReferencePickedClustersAndDeDupBindings(t *testing.T) {
 								AffinityScore:       &affinityScore1,
 								TopologySpreadScore: &topologySpreadScore1,
 							},
-							Reason: pickedByPolicyReason,
+							Reason: fmt.Sprintf(resourceScheduleSucceededWithScoreMessageFormat, clusterName1, affinityScore1, topologySpreadScore1, pickedByPolicyReason),
 						},
 					},
 				},
@@ -1653,7 +1653,7 @@ func TestCrossReferencePickedClustersAndDeDupBindings(t *testing.T) {
 								AffinityScore:       &affinityScore2,
 								TopologySpreadScore: &topologySpreadScore2,
 							},
-							Reason: pickedByPolicyReason,
+							Reason: fmt.Sprintf(resourceScheduleSucceededWithScoreMessageFormat, clusterName2, affinityScore2, topologySpreadScore2, pickedByPolicyReason),
 						},
 					},
 				},
@@ -1675,7 +1675,7 @@ func TestCrossReferencePickedClustersAndDeDupBindings(t *testing.T) {
 								AffinityScore:       &affinityScore3,
 								TopologySpreadScore: &topologySpreadScore3,
 							},
-							Reason: pickedByPolicyReason,
+							Reason: fmt.Sprintf(resourceScheduleSucceededWithScoreMessageFormat, clusterName3, affinityScore3, topologySpreadScore3, pickedByPolicyReason),
 						},
 					},
 				},
@@ -1718,7 +1718,7 @@ func TestCrossReferencePickedClustersAndDeDupBindings(t *testing.T) {
 								AffinityScore:       &affinityScore1,
 								TopologySpreadScore: &topologySpreadScore1,
 							},
-							Reason: pickedByPolicyReason,
+							Reason: fmt.Sprintf(resourceScheduleSucceededWithScoreMessageFormat, clusterName1, affinityScore1, topologySpreadScore1, pickedByPolicyReason),
 						},
 					},
 				},
@@ -1740,7 +1740,7 @@ func TestCrossReferencePickedClustersAndDeDupBindings(t *testing.T) {
 								AffinityScore:       &affinityScore3,
 								TopologySpreadScore: &topologySpreadScore3,
 							},
-							Reason: pickedByPolicyReason,
+							Reason: fmt.Sprintf(resourceScheduleSucceededWithScoreMessageFormat, clusterName3, affinityScore3, topologySpreadScore3, pickedByPolicyReason),
 						},
 					},
 				},
@@ -1763,7 +1763,7 @@ func TestCrossReferencePickedClustersAndDeDupBindings(t *testing.T) {
 									AffinityScore:       &affinityScore2,
 									TopologySpreadScore: &topologySpreadScore2,
 								},
-								Reason: pickedByPolicyReason,
+								Reason: fmt.Sprintf(resourceScheduleSucceededWithScoreMessageFormat, clusterName2, affinityScore2, topologySpreadScore2, pickedByPolicyReason),
 							},
 						},
 					},
@@ -1825,7 +1825,7 @@ func TestCrossReferencePickedClustersAndDeDupBindings(t *testing.T) {
 								AffinityScore:       &affinityScore3,
 								TopologySpreadScore: &topologySpreadScore3,
 							},
-							Reason: pickedByPolicyReason,
+							Reason: fmt.Sprintf(resourceScheduleSucceededWithScoreMessageFormat, clusterName3, affinityScore3, topologySpreadScore3, pickedByPolicyReason),
 						},
 					},
 				},
@@ -1847,7 +1847,7 @@ func TestCrossReferencePickedClustersAndDeDupBindings(t *testing.T) {
 									AffinityScore:       &affinityScore1,
 									TopologySpreadScore: &topologySpreadScore1,
 								},
-								Reason: pickedByPolicyReason,
+								Reason: fmt.Sprintf(resourceScheduleSucceededWithScoreMessageFormat, clusterName1, affinityScore1, topologySpreadScore1, pickedByPolicyReason),
 							},
 						},
 					},
@@ -1877,7 +1877,7 @@ func TestCrossReferencePickedClustersAndDeDupBindings(t *testing.T) {
 									AffinityScore:       &affinityScore2,
 									TopologySpreadScore: &topologySpreadScore2,
 								},
-								Reason: pickedByPolicyReason,
+								Reason: fmt.Sprintf(resourceScheduleSucceededWithScoreMessageFormat, clusterName2, affinityScore2, topologySpreadScore2, pickedByPolicyReason),
 							},
 						},
 					},
@@ -2428,7 +2428,7 @@ func TestUpdatePolicySnapshotStatusFromBindings(t *testing.T) {
 				{
 					ClusterName: anotherClusterName,
 					Selected:    false,
-					Reason:      notPickedByScoreReason,
+					Reason:      fmt.Sprintf(notPickedByScoreReason, anotherClusterName, affinityScore3, topologySpreadScore3),
 					ClusterScore: &placementv1beta1.ClusterScore{
 						AffinityScore:       &affinityScore3,
 						TopologySpreadScore: &topologySpreadScore3,
@@ -2502,7 +2502,7 @@ func TestUpdatePolicySnapshotStatusFromBindings(t *testing.T) {
 						AffinityScore:       &affinityScore3,
 						TopologySpreadScore: &topologySpreadScore3,
 					},
-					Reason: notPickedByScoreReason,
+					Reason: fmt.Sprintf(notPickedByScoreReason, altClusterName, affinityScore3, topologySpreadScore3),
 				},
 				{
 					ClusterName: anotherClusterName,
@@ -2642,7 +2642,7 @@ func TestUpdatePolicySnapshotStatusFromBindings(t *testing.T) {
 				{
 					ClusterName: altClusterName,
 					Selected:    false,
-					Reason:      notPickedByScoreReason,
+					Reason:      fmt.Sprintf(notPickedByScoreReason, altClusterName, affinityScore2, topologySpreadScore2),
 					ClusterScore: &placementv1beta1.ClusterScore{
 						AffinityScore:       &affinityScore2,
 						TopologySpreadScore: &topologySpreadScore2,
@@ -3300,7 +3300,7 @@ func TestNewSchedulingDecisionsFromBindings(t *testing.T) {
 						TopologySpreadScore: &topologySpreadScore3,
 						AffinityScore:       &affinityScore3,
 					},
-					Reason: notPickedByScoreReason,
+					Reason: fmt.Sprintf(notPickedByScoreReason, anotherClusterName, affinityScore3, topologySpreadScore3),
 				},
 			},
 		},
@@ -3367,7 +3367,7 @@ func TestNewSchedulingDecisionsFromBindings(t *testing.T) {
 						TopologySpreadScore: &topologySpreadScore3,
 						AffinityScore:       &affinityScore3,
 					},
-					Reason: notPickedByScoreReason,
+					Reason: fmt.Sprintf(notPickedByScoreReason, anotherClusterName, affinityScore3, topologySpreadScore3),
 				},
 				{
 					ClusterName: altClusterName,
@@ -6080,12 +6080,12 @@ func TestUpdatePolicySnapshotStatusForPickFixedPlacementType(t *testing.T) {
 				{
 					ClusterName: clusterName,
 					Selected:    true,
-					Reason:      pickedByPolicyReason,
+					Reason:      fmt.Sprintf(resourceScheduleSucceededMessageFormat, clusterName, pickedByPolicyReason),
 				},
 				{
 					ClusterName: altClusterName,
 					Selected:    true,
-					Reason:      pickedByPolicyReason,
+					Reason:      fmt.Sprintf(resourceScheduleSucceededMessageFormat, altClusterName, pickedByPolicyReason),
 				},
 			},
 			wantCondition: newScheduledCondition(policyWithNoStatus, metav1.ConditionTrue, FullyScheduledReason, fmt.Sprintf(fullyScheduledMessage, 2)),
@@ -6116,12 +6116,12 @@ func TestUpdatePolicySnapshotStatusForPickFixedPlacementType(t *testing.T) {
 				{
 					ClusterName: clusterName,
 					Selected:    false,
-					Reason:      fmt.Sprintf(pickFixedInvalidClusterReasonTemplate, invalidClusterDummyReason),
+					Reason:      fmt.Sprintf(pickFixedInvalidClusterReasonTemplate, clusterName, invalidClusterDummyReason),
 				},
 				{
 					ClusterName: altClusterName,
 					Selected:    false,
-					Reason:      fmt.Sprintf(pickFixedInvalidClusterReasonTemplate, invalidClusterDummyReason),
+					Reason:      fmt.Sprintf(pickFixedInvalidClusterReasonTemplate, altClusterName, invalidClusterDummyReason),
 				},
 			},
 			wantCondition: newScheduledCondition(policyWithNoStatus, metav1.ConditionFalse, NotFullyScheduledReason, fmt.Sprintf(notFullyScheduledMessage, 0)),
@@ -6138,12 +6138,12 @@ func TestUpdatePolicySnapshotStatusForPickFixedPlacementType(t *testing.T) {
 				{
 					ClusterName: clusterName,
 					Selected:    false,
-					Reason:      pickFixedNotFoundClusterReason,
+					Reason:      fmt.Sprintf(pickFixedNotFoundClusterReason, clusterName),
 				},
 				{
 					ClusterName: altClusterName,
 					Selected:    false,
-					Reason:      pickFixedNotFoundClusterReason,
+					Reason:      fmt.Sprintf(pickFixedNotFoundClusterReason, altClusterName),
 				},
 			},
 			wantCondition: newScheduledCondition(policyWithNoStatus, metav1.ConditionFalse, NotFullyScheduledReason, fmt.Sprintf(notFullyScheduledMessage, 0)),
@@ -6176,17 +6176,17 @@ func TestUpdatePolicySnapshotStatusForPickFixedPlacementType(t *testing.T) {
 				{
 					ClusterName: clusterName,
 					Selected:    true,
-					Reason:      pickedByPolicyReason,
+					Reason:      fmt.Sprintf(resourceScheduleSucceededMessageFormat, clusterName, pickedByPolicyReason),
 				},
 				{
 					ClusterName: altClusterName,
 					Selected:    false,
-					Reason:      fmt.Sprintf(pickFixedInvalidClusterReasonTemplate, invalidClusterDummyReason),
+					Reason:      fmt.Sprintf(pickFixedInvalidClusterReasonTemplate, altClusterName, invalidClusterDummyReason),
 				},
 				{
 					ClusterName: anotherClusterName,
 					Selected:    false,
-					Reason:      pickFixedNotFoundClusterReason,
+					Reason:      fmt.Sprintf(pickFixedNotFoundClusterReason, anotherClusterName),
 				},
 			},
 			wantCondition: newScheduledCondition(policyWithNoStatus, metav1.ConditionFalse, NotFullyScheduledReason, fmt.Sprintf(notFullyScheduledMessage, 1)),
@@ -6217,12 +6217,12 @@ func TestUpdatePolicySnapshotStatusForPickFixedPlacementType(t *testing.T) {
 				{
 					ClusterName: clusterName,
 					Selected:    true,
-					Reason:      pickedByPolicyReason,
+					Reason:      fmt.Sprintf(resourceScheduleSucceededMessageFormat, clusterName, pickedByPolicyReason),
 				},
 				{
 					ClusterName: altClusterName,
 					Selected:    true,
-					Reason:      pickedByPolicyReason,
+					Reason:      fmt.Sprintf(resourceScheduleSucceededMessageFormat, altClusterName, pickedByPolicyReason),
 				},
 			},
 			wantCondition: newScheduledCondition(policyWithNoStatus, metav1.ConditionTrue, FullyScheduledReason, fmt.Sprintf(fullyScheduledMessage, 2)),