diff --git a/CHANGELOG/CHANGELOG-0.1.md b/CHANGELOG/CHANGELOG-0.1.md index c902064707..7f1abd017d 100644 --- a/CHANGELOG/CHANGELOG-0.1.md +++ b/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. diff --git a/pkg/queue/cluster_queue_best_effort_fifo.go b/pkg/queue/cluster_queue_best_effort_fifo.go index 1a75d03ab6..cad711f00f 100644 --- a/pkg/queue/cluster_queue_best_effort_fifo.go +++ b/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, diff --git a/pkg/queue/cluster_queue_best_effort_fifo_test.go b/pkg/queue/cluster_queue_best_effort_fifo_test.go index d7b28f7112..55a8a4d6be 100644 --- a/pkg/queue/cluster_queue_best_effort_fifo_test.go +++ b/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") + } +}