Add feature gate for priorities sorting in a cohort (#1406)

pkg/features/kube_features.go

@@ -62,6 +62,13 @@ const (
 	//
 	// Enables Kueue visibility on demand
 	VisibilityOnDemand featuregate.Feature = "VisibilityOnDemand"
+
+	// owner: @yaroslava-serdiuk
+	// kep: https://github.com/kubernetes-sigs/kueue/issues/1283
+	// beta: v0.6
+	//
+	// Enable priority sorting within the cohort.
+	PrioritySortingWithinCohort featuregate.Feature = "PrioritySortingWithinCohort"
 )
 
 func init() {
@@ -75,11 +82,12 @@ func init() {
 // Entries are separated from each other with blank lines to avoid sweeping gofmt changes
 // when adding or removing one entry.
 var defaultFeatureGates = map[featuregate.Feature]featuregate.FeatureSpec{
-	PartialAdmission:   {Default: true, PreRelease: featuregate.Beta},
-	QueueVisibility:    {Default: false, PreRelease: featuregate.Alpha},
-	FlavorFungibility:  {Default: true, PreRelease: featuregate.Beta},
-	ProvisioningACC:    {Default: false, PreRelease: featuregate.Alpha},
-	VisibilityOnDemand: {Default: false, PreRelease: featuregate.Alpha},
+	PartialAdmission:            {Default: true, PreRelease: featuregate.Beta},
+	QueueVisibility:             {Default: false, PreRelease: featuregate.Alpha},
+	FlavorFungibility:           {Default: true, PreRelease: featuregate.Beta},
+	ProvisioningACC:             {Default: false, PreRelease: featuregate.Alpha},
+	VisibilityOnDemand:          {Default: false, PreRelease: featuregate.Alpha},
+	PrioritySortingWithinCohort: {Default: true, PreRelease: featuregate.Beta},
 }
 
 func SetFeatureGateDuringTest(tb testing.TB, f featuregate.Feature, value bool) func() {

pkg/scheduler/scheduler.go

@@ -170,7 +170,7 @@ func (s *Scheduler) schedule(ctx context.Context) {
 	// 3. Calculate requirements (resource flavors, borrowing) for admitting workloads.
 	entries := s.nominate(ctx, headWorkloads, snapshot)
 
-	// 4. Sort entries based on borrowing and timestamps.
+	// 4. Sort entries based on borrowing, priorities (if enabled) and timestamps.
 	sort.Sort(entryOrdering(entries))
 
 	// 5. Admit entries, ensuring that no more than one workload gets
@@ -500,14 +500,16 @@ func (e entryOrdering) Less(i, j int) bool {
 		return !aBorrows
 	}
 
-	// 2. Higher priority first.
-	p1 := priority.Priority(a.Obj)
-	p2 := priority.Priority(b.Obj)
-	if p1 != p2 {
-		return p1 > p2
+	// 2. Higher priority first if not disabled.
+	if features.Enabled(features.PrioritySortingWithinCohort) {
+		p1 := priority.Priority(a.Obj)
+		p2 := priority.Priority(b.Obj)
+		if p1 != p2 {
+			return p1 > p2
+		}
 	}
 
-	// 2. FIFO.
+	// 3. FIFO.
 	aComparisonTimestamp := workload.GetQueueOrderTimestamp(a.Obj)
 	bComparisonTimestamp := workload.GetQueueOrderTimestamp(b.Obj)
 	return aComparisonTimestamp.Before(bComparisonTimestamp)

pkg/scheduler/scheduler_test.go

@@ -1208,7 +1208,7 @@ func TestEntryOrdering(t *testing.T) {
 			Info: workload.Info{
 				Obj: &kueue.Workload{ObjectMeta: metav1.ObjectMeta{
 					Name:              "new_high_pri",
-					CreationTimestamp: metav1.NewTime(now.Add(3 * time.Second)),
+					CreationTimestamp: metav1.NewTime(now.Add(4 * time.Second)),
 				}, Spec: kueue.WorkloadSpec{
 					Priority: ptr.To[int32](1),
 				}},
@@ -1232,7 +1232,7 @@ func TestEntryOrdering(t *testing.T) {
 				Obj: &kueue.Workload{
 					ObjectMeta: metav1.ObjectMeta{
 						Name:              "evicted_borrowing",
-						CreationTimestamp: metav1.NewTime(now),
+						CreationTimestamp: metav1.NewTime(now.Add(time.Second)),
 					},
 					Status: kueue.WorkloadStatus{
 						Conditions: []metav1.Condition{
@@ -1273,14 +1273,31 @@ func TestEntryOrdering(t *testing.T) {
 			},
 		},
 	}
-	sort.Sort(entryOrdering(input))
-	order := make([]string, len(input))
-	for i, e := range input {
-		order[i] = e.Obj.Name
-	}
-	wantOrder := []string{"new_high_pri", "old", "recently_evicted", "new", "high_pri_borrowing", "old_borrowing", "evicted_borrowing", "new_borrowing"}
-	if diff := cmp.Diff(wantOrder, order); diff != "" {
-		t.Errorf("Unexpected order (-want,+got):\n%s", diff)
+	for _, tc := range []struct {
+		name            string
+		prioritySorting bool
+		wantOrder       []string
+	}{
+		{
+			name:            "Priority sorting is enabled (default)",
+			prioritySorting: true,
+			wantOrder:       []string{"new_high_pri", "old", "recently_evicted", "new", "high_pri_borrowing", "old_borrowing", "evicted_borrowing", "new_borrowing"},
+		},
+		{
+			name:            "Priority sorting is disabled",
+			prioritySorting: false,
+			wantOrder:       []string{"old", "recently_evicted", "new", "new_high_pri", "old_borrowing", "evicted_borrowing", "high_pri_borrowing", "new_borrowing"},
+		},
+	} {
+		features.SetFeatureGateDuringTest(t, features.PrioritySortingWithinCohort, tc.prioritySorting)
+		sort.Sort(entryOrdering(input))
+		order := make([]string, len(input))
+		for i, e := range input {
+			order[i] = e.Obj.Name
+		}
+		if diff := cmp.Diff(tc.wantOrder, order); diff != "" {
+			t.Errorf("%s: Unexpected order (-want,+got):\n%s", tc.name, diff)
+		}
 	}
 }