remarks2

kubernetes-sigs · Apr 15, 2024 · 7ca5892 · 7ca5892
1 parent 43f350b
commit 7ca5892
Show file tree

Hide file tree

Showing 2 changed files with 6 additions and 97 deletions.
diff --git a/pkg/scheduler/preemption/preemption.go b/pkg/scheduler/preemption/preemption.go
@@ -117,9 +117,9 @@ func (p *Preemptor) GetTargets(wl workload.Info, assignment flavorassigner.Assig
 		return minimalPreemptions(wlReq, cq, assignment, snapshot, resPerFlv, candidates, true, allowBorrowingBelowPriority)
 	}
 
-	// If the target queue is exhausted in any requested resource, then we skip
-	// the attempt of preemptions in the cohort, but without borrowing.
-	if !isQueueExhaustedInAnyRequestedResource(wlReq, cq) {
+	// Only try preemptions in the cohort, without borrowing, if the target clusterqueue is still
+	// under nominal quota for all resources.
+	if queueUnderNominalInAllRequestedResources(wlReq, cq) {
 		if targets := minimalPreemptions(wlReq, cq, assignment, snapshot, resPerFlv, candidates, false, nil); len(targets) > 0 {
 			return targets
 		}
@@ -396,8 +396,7 @@ func workloadFits(wlReq cache.FlavorResourceQuantities, cq *cache.ClusterQueue,
 	return true
 }
 
-// is queue exhausted for any requested resource
-func isQueueExhaustedInAnyRequestedResource(wlReq cache.FlavorResourceQuantities, cq *cache.ClusterQueue) bool {
+func queueUnderNominalInAllRequestedResources(wlReq cache.FlavorResourceQuantities, cq *cache.ClusterQueue) bool {
 	for _, rg := range cq.ResourceGroups {
 		for _, flvQuotas := range rg.Flavors {
 			flvReq, found := wlReq[flvQuotas.Name]
@@ -408,12 +407,12 @@ func isQueueExhaustedInAnyRequestedResource(wlReq cache.FlavorResourceQuantities
 			cqResUsage := cq.Usage[flvQuotas.Name]
 			for rName := range flvReq {
 				if cqResUsage[rName] >= flvQuotas.Resources[rName].Nominal {
-					return true
+					return false
 				}
 			}
 		}
 	}
-	return false
+	return true
 }
 
 // candidatesOrdering criteria:

diff --git a/pkg/scheduler/scheduler_test.go b/pkg/scheduler/scheduler_test.go
@@ -1492,96 +1492,6 @@ func TestSchedule(t *testing.T) {
 				"eng-beta/b3":  *utiltesting.MakeAdmission("other-beta").Assignment(corev1.ResourceCPU, "on-demand", "1").Obj(),
 			},
 		},
-		"minimal preemptions when target queue is exhausted in one resource, but not the other": {
-			additionalClusterQueues: []kueue.ClusterQueue{
-				*utiltesting.MakeClusterQueue("other-alpha").
-					Cohort("other").
-					Preemption(kueue.ClusterQueuePreemption{
-						ReclaimWithinCohort: kueue.PreemptionPolicyAny,
-						WithinClusterQueue:  kueue.PreemptionPolicyLowerPriority,
-					}).
-					ResourceGroup(
-						*utiltesting.MakeFlavorQuotas("on-demand").
-							Resource(corev1.ResourceCPU, "2").Resource(corev1.ResourceMemory, "2").Obj(),
-					).
-					Obj(),
-				*utiltesting.MakeClusterQueue("other-beta").
-					Cohort("other").
-					ResourceGroup(
-						*utiltesting.MakeFlavorQuotas("on-demand").
-							Resource(corev1.ResourceCPU, "2").Resource(corev1.ResourceMemory, "2").Obj(),
-					).
-					Obj(),
-				*utiltesting.MakeClusterQueue("other-gamma").
-					Cohort("other").
-					ResourceGroup(
-						*utiltesting.MakeFlavorQuotas("on-demand").
-							Resource(corev1.ResourceCPU, "2").Resource(corev1.ResourceMemory, "2").Obj(),
-					).
-					Obj(),
-			},
-			additionalLocalQueues: []kueue.LocalQueue{
-				*utiltesting.MakeLocalQueue("other", "eng-alpha").ClusterQueue("other-alpha").Obj(),
-				*utiltesting.MakeLocalQueue("other", "eng-beta").ClusterQueue("other-beta").Obj(),
-				*utiltesting.MakeLocalQueue("other", "eng-gamma").ClusterQueue("other-gamma").Obj(),
-			},
-			workloads: []kueue.Workload{
-				*utiltesting.MakeWorkload("a1", "eng-alpha").
-					Priority(-2).
-					Queue("other").
-					Request(corev1.ResourceCPU, "1").
-					ReserveQuota(utiltesting.MakeAdmission("other-alpha").Assignment(corev1.ResourceCPU, "on-demand", "1").Obj()).
-					Obj(),
-				*utiltesting.MakeWorkload("a2", "eng-alpha").
-					Priority(-2).
-					Queue("other").
-					Request(corev1.ResourceCPU, "1").
-					ReserveQuota(utiltesting.MakeAdmission("other-alpha").Assignment(corev1.ResourceCPU, "on-demand", "1").Obj()).
-					Obj(),
-				*utiltesting.MakeWorkload("a3", "eng-alpha").
-					Priority(-1).
-					Queue("other").
-					Request(corev1.ResourceCPU, "1").
-					ReserveQuota(utiltesting.MakeAdmission("other-alpha").Assignment(corev1.ResourceCPU, "on-demand", "1").Obj()).
-					Obj(),
-				*utiltesting.MakeWorkload("b1", "eng-beta").
-					Priority(0).
-					Queue("other").
-					Request(corev1.ResourceCPU, "1").
-					ReserveQuota(utiltesting.MakeAdmission("other-beta").Assignment(corev1.ResourceCPU, "on-demand", "1").Obj()).
-					Obj(),
-				*utiltesting.MakeWorkload("b2", "eng-beta").
-					Priority(0).
-					Queue("other").
-					Request(corev1.ResourceCPU, "1").
-					ReserveQuota(utiltesting.MakeAdmission("other-beta").Assignment(corev1.ResourceCPU, "on-demand", "1").Obj()).
-					Obj(),
-				*utiltesting.MakeWorkload("b3", "eng-beta").
-					Priority(0).
-					Queue("other").
-					Request(corev1.ResourceCPU, "1").
-					ReserveQuota(utiltesting.MakeAdmission("other-beta").Assignment(corev1.ResourceCPU, "on-demand", "1").Obj()).
-					Obj(),
-				*utiltesting.MakeWorkload("incoming", "eng-alpha").
-					Priority(0).
-					Queue("other").
-					Request(corev1.ResourceCPU, "2").
-					Request(corev1.ResourceMemory, "2").
-					Obj(),
-			},
-			wantPreempted: sets.New("eng-alpha/a1", "eng-alpha/a2"),
-			wantLeft: map[string][]string{
-				"other-alpha": {"eng-alpha/incoming"},
-			},
-			wantAssignments: map[string]kueue.Admission{
-				"eng-alpha/a1": *utiltesting.MakeAdmission("other-alpha").Assignment(corev1.ResourceCPU, "on-demand", "1").Obj(),
-				"eng-alpha/a2": *utiltesting.MakeAdmission("other-alpha").Assignment(corev1.ResourceCPU, "on-demand", "1").Obj(),
-				"eng-alpha/a3": *utiltesting.MakeAdmission("other-alpha").Assignment(corev1.ResourceCPU, "on-demand", "1").Obj(),
-				"eng-beta/b1":  *utiltesting.MakeAdmission("other-beta").Assignment(corev1.ResourceCPU, "on-demand", "1").Obj(),
-				"eng-beta/b2":  *utiltesting.MakeAdmission("other-beta").Assignment(corev1.ResourceCPU, "on-demand", "1").Obj(),
-				"eng-beta/b3":  *utiltesting.MakeAdmission("other-beta").Assignment(corev1.ResourceCPU, "on-demand", "1").Obj(),
-			},
-		},
 		"A workload is only eligible to do preemptions if it fits fully within nominal quota": {
 			additionalClusterQueues: []kueue.ClusterQueue{
 				*utiltesting.MakeClusterQueue("other-alpha").