From 01be91b9db91ac40d51c13792823d653c54a3f59 Mon Sep 17 00:00:00 2001
From: mszacillo <mszacillo@bloomberg.net>
Date: Mon, 8 Jul 2024 17:14:19 -0400
Subject: [PATCH] GroupClusters should sort by score and availableReplica count

Signed-off-by: mszacillo <mszacillo@bloomberg.net>
---
 .../core/spreadconstraint/group_clusters.go   |  9 +++-
 .../core/spreadconstraint/util_test.go        | 50 +++++++++++++++++--
 2 files changed, 53 insertions(+), 6 deletions(-)

diff --git a/pkg/scheduler/core/spreadconstraint/group_clusters.go b/pkg/scheduler/core/spreadconstraint/group_clusters.go
index 1e880abf2c72..893239416855 100644
--- a/pkg/scheduler/core/spreadconstraint/group_clusters.go
+++ b/pkg/scheduler/core/spreadconstraint/group_clusters.go
@@ -17,6 +17,8 @@ limitations under the License.
 package spreadconstraint
 
 import (
+	"k8s.io/utils/ptr"
+
 	clusterv1alpha1 "github.com/karmada-io/karmada/pkg/apis/cluster/v1alpha1"
 	policyv1alpha1 "github.com/karmada-io/karmada/pkg/apis/policy/v1alpha1"
 	workv1alpha2 "github.com/karmada-io/karmada/pkg/apis/work/v1alpha2"
@@ -143,7 +145,12 @@ func (info *GroupClustersInfo) generateClustersInfo(clustersScore framework.Clus
 		info.Clusters[i].AvailableReplicas += int64(rbSpec.AssignedReplicasForCluster(clustersReplica.Name))
 	}
 
-	sortClusters(info.Clusters)
+	sortClusters(info.Clusters, func(i *ClusterDetailInfo, j *ClusterDetailInfo) *bool {
+		if i.AvailableReplicas != j.AvailableReplicas {
+			return ptr.To(i.AvailableReplicas > j.AvailableReplicas)
+		}
+		return nil
+	})
 }
 
 func (info *GroupClustersInfo) generateZoneInfo(spreadConstraints []policyv1alpha1.SpreadConstraint) {
diff --git a/pkg/scheduler/core/spreadconstraint/util_test.go b/pkg/scheduler/core/spreadconstraint/util_test.go
index a3fae85cf9a6..0a140b32b81c 100644
--- a/pkg/scheduler/core/spreadconstraint/util_test.go
+++ b/pkg/scheduler/core/spreadconstraint/util_test.go
@@ -20,6 +20,8 @@ import (
 	"reflect"
 	"testing"
 
+	"k8s.io/utils/ptr"
+
 	policyv1alpha1 "github.com/karmada-io/karmada/pkg/apis/policy/v1alpha1"
 )
 
@@ -75,9 +77,10 @@ func TestIsSpreadConstraintExisted(t *testing.T) {
 
 func Test_sortClusters(t *testing.T) {
 	tests := []struct {
-		name  string
-		infos []ClusterDetailInfo
-		want  []ClusterDetailInfo
+		name            string
+		infos           []ClusterDetailInfo
+		want            []ClusterDetailInfo
+		compareFunction func(*ClusterDetailInfo, *ClusterDetailInfo) *bool
 	}{
 		{
 			name: "different scores",
@@ -103,7 +106,7 @@ func Test_sortClusters(t *testing.T) {
 			},
 		},
 		{
-			name: "same score",
+			name: "same score, default to sort by name",
 			infos: []ClusterDetailInfo{
 				{
 					Name:              "b",
@@ -129,10 +132,47 @@ func Test_sortClusters(t *testing.T) {
 				},
 			},
 		},
+		{
+			name: "same score, sort by availableReplicas",
+			infos: []ClusterDetailInfo{
+				{
+					Name:              "a",
+					Score:             1,
+					AvailableReplicas: 5,
+				},
+				{
+					Name:              "b",
+					Score:             1,
+					AvailableReplicas: 10,
+				},
+			},
+			want: []ClusterDetailInfo{
+				{
+					Name:              "b",
+					Score:             1,
+					AvailableReplicas: 10,
+				},
+				{
+					Name:              "a",
+					Score:             1,
+					AvailableReplicas: 5,
+				},
+			},
+			compareFunction: func(i *ClusterDetailInfo, j *ClusterDetailInfo) *bool {
+				if i.AvailableReplicas != j.AvailableReplicas {
+					return ptr.To(i.AvailableReplicas > j.AvailableReplicas)
+				}
+				return nil
+			},
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			sortClusters(tt.infos)
+			if tt.compareFunction != nil {
+				sortClusters(tt.infos, tt.compareFunction)
+			} else {
+				sortClusters(tt.infos)
+			}
 			if !reflect.DeepEqual(tt.infos, tt.want) {
 				t.Errorf("sortClusters() = %v, want %v", tt.infos, tt.want)
 			}