update SortJobsByPod func

Signed-off-by: baowj <[email protected]>
koordinator-sh · Oct 20, 2023 · 9c9b46f · 9c9b46f
1 parent 9d3dd0f
commit 9c9b46f
Show file tree

Hide file tree

Showing 3 changed files with 35 additions and 31 deletions.
diff --git a/pkg/descheduler/controllers/migration/arbitrator/arbitrator.go b/pkg/descheduler/controllers/migration/arbitrator/arbitrator.go
@@ -79,6 +79,15 @@ func New(args *config.MigrationControllerArgs, options Options) (Arbitrator, err
 	if err != nil {
 		return nil, err
 	}
+	comparators := []sorter.CompareFn{
+		sorter.KoordinatorPriorityClass,
+		sorter.Priority,
+		sorter.KubernetesQoSClass,
+		sorter.KoordinatorQoSClass,
+		sorter.PodDeletionCost,
+		sorter.EvictionCost,
+		sorter.PodCreationTimestamp,
+	}
 
 	arbitrator := &arbitratorImpl{
 		waitingCollection: map[types.UID]*v1alpha1.PodMigrationJob{},
@@ -88,14 +97,21 @@ func New(args *config.MigrationControllerArgs, options Options) (Arbitrator, err
 				return job.Status.Phase == v1alpha1.PodMigrationJobRunning ||
 					(job.Status.Phase == v1alpha1.PodMigrationJobPending && f.checkJobPassedArbitration(job.UID))
 			}),
-			SortJobsByPod([]sorter.CompareFn{
-				sorter.KoordinatorPriorityClass,
-				sorter.Priority,
-				sorter.KubernetesQoSClass,
-				sorter.KoordinatorQoSClass,
-				sorter.PodDeletionCost,
-				sorter.EvictionCost,
-				sorter.PodCreationTimestamp,
+			SortJobsByPod(func(p1, p2 *corev1.Pod) int {
+				var k int
+				for k = 0; k < len(comparators)-1; k++ {
+					cmpResult := comparators[k](p1, p2)
+					// p1 is less than p2
+					if cmpResult < 0 {
+						return -1
+					}
+					// p1 is greater than p2
+					if cmpResult > 0 {
+						return 1
+					}
+				}
+				cmpResult := comparators[k](p1, p2)
+				return cmpResult
 			}),
 			SortJobsByCreationTime(),
 		},

diff --git a/pkg/descheduler/controllers/migration/arbitrator/sort.go b/pkg/descheduler/controllers/migration/arbitrator/sort.go
@@ -29,7 +29,6 @@ import (
 
 	"github.com/koordinator-sh/koordinator/apis/scheduling/v1alpha1"
 	"github.com/koordinator-sh/koordinator/pkg/descheduler/fieldindex"
-	"github.com/koordinator-sh/koordinator/pkg/descheduler/utils/sorter"
 	utilclient "github.com/koordinator-sh/koordinator/pkg/util/client"
 )
 
@@ -108,19 +107,10 @@ func (ms *MultiSorter) Less(i, j int) bool {
 }
 
 // SortJobsByPod returns a SortFn that sorts PodMigrationJobs by their Pods, including priority, QoS.
-func SortJobsByPod(podSorters []sorter.CompareFn) SortFn {
+func SortJobsByPod(sorter func(p1, p2 *corev1.Pod) int) SortFn {
 	return func(jobs []*v1alpha1.PodMigrationJob, podOfJob map[*v1alpha1.PodMigrationJob]*corev1.Pod) CompareFn {
 		return func(p1, p2 *v1alpha1.PodMigrationJob) int {
-			for _, podSorter := range podSorters {
-				cmpResult := podSorter(podOfJob[p1], podOfJob[p2])
-				if cmpResult < 0 {
-					return -1
-				}
-				if cmpResult > 0 {
-					return 1
-				}
-			}
-			return 0
+			return sorter(podOfJob[p1], podOfJob[p2])
 		}
 	}
 }

diff --git a/pkg/descheduler/controllers/migration/arbitrator/sort_test.go b/pkg/descheduler/controllers/migration/arbitrator/sort_test.go
@@ -40,7 +40,6 @@ import (
 	"github.com/koordinator-sh/koordinator/apis/extension"
 	"github.com/koordinator-sh/koordinator/apis/scheduling/v1alpha1"
 	"github.com/koordinator-sh/koordinator/pkg/descheduler/fieldindex"
-	"github.com/koordinator-sh/koordinator/pkg/descheduler/utils/sorter"
 )
 
 func TestSortJobsByPod(t *testing.T) {
@@ -85,16 +84,15 @@ func TestSortJobsByPod(t *testing.T) {
 				podOrder[pod.Name] = testCase.order[i]
 			}
 
-			fn := SortJobsByPod(
-				[]sorter.CompareFn{func(p1, p2 *corev1.Pod) int {
-					if podOrder[p1.Name] < podOrder[p2.Name] {
-						return -1
-					}
-					if podOrder[p1.Name] > podOrder[p2.Name] {
-						return 1
-					}
-					return 0
-				}})
+			fn := SortJobsByPod(func(p1, p2 *corev1.Pod) int {
+				if podOrder[p1.Name] < podOrder[p2.Name] {
+					return -1
+				}
+				if podOrder[p1.Name] > podOrder[p2.Name] {
+					return 1
+				}
+				return 0
+			})
 			compareFn := fn(jobs, podOfJob)
 			sorterFn := OrderedBy(compareFn)
 			sorterFn.Sort(jobs)