Repository: kubernetes-sigs/kueue
Pull request #271 (2022-06-08 16:59:22)

cherry-pick: Fix number of pending workloads in a BestEffortFIFO ClusterQueue
This is a cherry-pick of #238


Patch:
diff --git a/CHANGELOG/CHANGELOG-0.1.md b/CHANGELOG/CHANGELOG-0.1.md
new file mode 100644
index 000000000..1312a48a9
--- /dev/null
+++ b/CHANGELOG/CHANGELOG-0.1.md
@@ -0,0 +1,5 @@
+## v0.1.1
+
+Changes since `v0.1.0`:
+
+- Fixed number of pending workloads in a BestEffortFIFO ClusterQueue.
\ No newline at end of file
diff --git a/pkg/queue/cluster_queue_best_effort_fifo.go b/pkg/queue/cluster_queue_best_effort_fifo.go
index 98153cd4d..704368205 100644
--- a/pkg/queue/cluster_queue_best_effort_fifo.go
+++ b/pkg/queue/cluster_queue_best_effort_fifo.go
@@ -112,3 +112,7 @@ func (cq *ClusterQueueBestEffortFIFO) QueueInadmissibleWorkloads() bool {
 	cq.inadmissibleWorkloads = make(map[string]*workload.Info)
 	return true
 }
+
+func (cq *ClusterQueueBestEffortFIFO) Pending() int32 {
+	return cq.ClusterQueueImpl.Pending() + int32(len(cq.inadmissibleWorkloads))
+}


Test patch:
diff --git a/pkg/queue/cluster_queue_best_effort_fifo_test.go b/pkg/queue/cluster_queue_best_effort_fifo_test.go
index fcd38e06e..a60061f4f 100644
--- a/pkg/queue/cluster_queue_best_effort_fifo_test.go
+++ b/pkg/queue/cluster_queue_best_effort_fifo_test.go
@@ -41,55 +41,61 @@ func TestClusterQueueBestEffortFIFO(t *testing.T) {
 	updatedWorkloads[1] = workloads[1].DeepCopy()
 	updatedWorkloads[1].Spec.QueueName = "q1"
 
-	tests := []struct {
-		name                       string
+	tests := map[string]struct {
 		workloadsToAdd             []*kueue.Workload
 		inadmissibleWorkloadsToAdd []*workload.Info
 		workloadsToUpdate          []*kueue.Workload
 		workloadsToDelete          []*kueue.Workload
 		queueInadmissibleWorkloads bool
-		wantWorkloads              sets.String
+		wantActiveWorkloads        sets.String
+		wantPending                int32
 	}{
-		{
-			name:                       "add, update, delete workload",
+		"add, update, delete workload": {
 			workloadsToAdd:             []*kueue.Workload{workloads[0], workloads[1]},
 			inadmissibleWorkloadsToAdd: []*workload.Info{},
 			workloadsToUpdate:          []*kueue.Workload{updatedWorkloads[0]},
 			workloadsToDelete:          []*kueue.Workload{workloads[0]},
-			queueInadmissibleWorkloads: false,
-			wantWorkloads:              sets.NewString(workloads[1].Name),
+			wantActiveWorkloads:        sets.NewString(workloads[1].Name),
+			wantPending:                1,
 		},
-		{
-			name:                       "update inadmissible workload",
+		"re-queue inadmissible workload": {
 			workloadsToAdd:             []*kueue.Workload{workloads[0]},
 			inadmissibleWorkloadsToAdd: []*workload.Info{workload.NewInfo(workloads[1])},
-			workloadsToUpdate:          []*kueue.Workload{updatedWorkloads[1]},
+			workloadsToUpdate:          []*kueue.Workload{},
 			workloadsToDelete:          []*kueue.Workload{},
-			queueInadmissibleWorkloads: false,
-			wantWorkloads:              sets.NewString(workloads[0].Name, workloads[1].Name),
+			wantActiveWorkloads:        sets.NewString(workloads[0].Name),
+			wantPending:                2,
 		},
-		{
-			name:                       "re-queue inadmissible workload",
+		"re-queue inadmissible workload and flush": {
 			workloadsToAdd:             []*kueue.Workload{workloads[0]},
 			inadmissibleWorkloadsToAdd: []*workload.Info{workload.NewInfo(workloads[1])},
 			workloadsToUpdate:          []*kueue.Workload{},
 			workloadsToDelete:          []*kueue.Workload{},
 			queueInadmissibleWorkloads: true,
-			wantWorkloads:              sets.NewString(workloads[0].Name, workloads[1].Name),
+			wantActiveWorkloads:        sets.NewString(workloads[0].Name, workloads[1].Name),
+			wantPending:                2,
+		},
+		"update inadmissible workload": {
+			workloadsToAdd:             []*kueue.Workload{workloads[0]},
+			inadmissibleWorkloadsToAdd: []*workload.Info{workload.NewInfo(workloads[1])},
+			workloadsToUpdate:          []*kueue.Workload{updatedWorkloads[1]},
+			workloadsToDelete:          []*kueue.Workload{},
+			wantActiveWorkloads:        sets.NewString(workloads[0].Name, workloads[1].Name),
+			wantPending:                2,
 		},
-		{
-			name:                       "delete inadmissible workload",
+		"delete inadmissible workload": {
 			workloadsToAdd:             []*kueue.Workload{workloads[0]},
 			inadmissibleWorkloadsToAdd: []*workload.Info{workload.NewInfo(workloads[1])},
 			workloadsToUpdate:          []*kueue.Workload{},
 			workloadsToDelete:          []*kueue.Workload{workloads[1]},
 			queueInadmissibleWorkloads: true,
-			wantWorkloads:              sets.NewString(workloads[0].Name),
+			wantActiveWorkloads:        sets.NewString(workloads[0].Name),
+			wantPending:                1,
 		},
 	}
 
-	for _, test := range tests {
-		t.Run(test.name, func(t *testing.T) {
+	for name, test := range tests {
+		t.Run(name, func(t *testing.T) {
 			cq, err := newClusterQueueBestEffortFIFO(clusterQueue)
 			if err != nil {
 				t.Fatalf("Failed creating ClusterQueue %v", err)
@@ -116,9 +122,12 @@ func TestClusterQueueBestEffortFIFO(t *testing.T) {
 			}
 
 			gotWorkloads, _ := cq.Dump()
-			if diff := cmp.Diff(test.wantWorkloads, gotWorkloads); diff != "" {
+			if diff := cmp.Diff(test.wantActiveWorkloads, gotWorkloads); diff != "" {
 				t.Errorf("Unexpected items in cluster foo (-want,+got):\n%s", diff)
 			}
+			if got := cq.Pending(); got != test.wantPending {
+				t.Errorf("Got %d pending workloads, want %d", got, test.wantPending)
+			}
 		})
 	}
 }