Merge pull request #275 from kerthcet/automated-cherry-pick-of-#248-u…

…pstream-release-0.1

Automated cherry pick of #248: fix: forget to cleanup inadmissibleWorkloads when

CHANGELOG/CHANGELOG-0.1.md

@@ -6,3 +6,5 @@ Changes since `v0.1.0`:
 - Fixed bug in a BestEffortFIFO ClusterQueue where a workload might not be
   retried after a transient error.
 - Fixed requeuing an out-of-date workload when failed to admit it.
+- Fixed bug in a BestEffortFIFO ClusterQueue where unadmissible workloads
+  were not removed from the ClusterQueue when removing the corresponding Queue.

pkg/queue/cluster_queue_best_effort_fifo.go

@@ -75,6 +75,16 @@ func (cq *ClusterQueueBestEffortFIFO) Delete(w *kueue.Workload) {
 	cq.ClusterQueueImpl.Delete(w)
 }
 
+func (cq *ClusterQueueBestEffortFIFO) DeleteFromQueue(q *Queue) {
+	for _, w := range q.items {
+		key := workload.Key(w.Obj)
+		if wl := cq.inadmissibleWorkloads[key]; wl != nil {
+			delete(cq.inadmissibleWorkloads, key)
+		}
+	}
+	cq.ClusterQueueImpl.DeleteFromQueue(q)
+}
+
 // RequeueIfNotPresent inserts a workload that cannot be admitted into
 // ClusterQueue, unless it is already in the queue. If immediate is true,
 // the workload will be pushed back to heap directly. If not,

pkg/queue/cluster_queue_best_effort_fifo_test.go

@@ -136,3 +136,43 @@ func TestClusterQueueBestEffortFIFO(t *testing.T) {
 		})
 	}
 }
+
+func TestDeleteFromQueue(t *testing.T) {
+	cq := utiltesting.MakeClusterQueue("cq").Obj()
+	cqImpl, err := newClusterQueueBestEffortFIFO(cq)
+	if err != nil {
+		t.Fatalf("Failed creating ClusterQueue %v", err)
+	}
+	q := utiltesting.MakeQueue("foo", "").ClusterQueue(cq.Name).Obj()
+	qImpl := newQueue(q)
+	wl1 := utiltesting.MakeWorkload("wl1", "").Queue(q.Name).Obj()
+	wl2 := utiltesting.MakeWorkload("wl2", "").Queue(q.Name).Obj()
+	wl3 := utiltesting.MakeWorkload("wl3", "").Queue(q.Name).Obj()
+	wl4 := utiltesting.MakeWorkload("wl4", "").Queue(q.Name).Obj()
+	admissibleworkloads := []*kueue.Workload{wl1, wl2}
+	inadmissibleWorkloads := []*kueue.Workload{wl3, wl4}
+
+	for _, w := range admissibleworkloads {
+		cqImpl.PushOrUpdate(w)
+		qImpl.AddOrUpdate(w)
+	}
+
+	for _, w := range inadmissibleWorkloads {
+		cqImpl.RequeueIfNotPresent(workload.NewInfo(w), false)
+		qImpl.AddOrUpdate(w)
+	}
+
+	wantPending := len(admissibleworkloads) + len(inadmissibleWorkloads)
+	if pending := cqImpl.Pending(); pending != int32(wantPending) {
+		t.Errorf("clusterQueue's workload number not right, want %v, got %v", wantPending, pending)
+	}
+	fifo := cqImpl.(*ClusterQueueBestEffortFIFO)
+	if len(fifo.inadmissibleWorkloads) != len(inadmissibleWorkloads) {
+		t.Errorf("clusterQueue's workload number in inadmissibleWorkloads not right, want %v, got %v", len(inadmissibleWorkloads), len(fifo.inadmissibleWorkloads))
+	}
+
+	cqImpl.DeleteFromQueue(qImpl)
+	if cqImpl.Pending() != 0 {
+		t.Error("clusterQueue should be empty")
+	}
+}