A pending workload that's deleted is still listed in the visibility API pendingworkloads results

[astefanutti]

What happened:

If a pending workload is deleted, querying the visibility API stills reports the workload.

What you expected to happen:

The workload should not be listed in the pendingworkloads sub-resource after it's deleted.

How to reproduce it (as minimally and precisely as possible):

1. Create a workload such that it's going to be pending
2. Delete the job / workload
3. Query the visibility API

Anything else we need to know?:

Once the operator Pod is deleted / has restarted, the results are corrected.

Environment:

• Kubernetes version (use kubectl version): v1.25.3
• Kueue version (use git describe --tags --dirty --always): v0.6.0-devel-146-ged81667f-dirty

[mimowo]

Interesting, because the visibility API serves on-demand the data directly from the in-memory cache used by scheduler, which suggests the cache itself is not updated. Did you have a chance to check if the PendingWorkloads counter is correct:

kueue/apis/kueue/v1beta1/clusterqueue_types.go

Line 230 in 1adca15

PendingWorkloads int32 `json:"pendingWorkloads"`

?

[astefanutti]

Interesting, because the visibility API serves on-demand the data directly from the in-memory cache used by scheduler, which suggests the cache itself is not updated.

Right, that'd be my understanding as well. I had traced it down to:

kueue/pkg/controller/core/workload_controller.go

Lines 406 to 408 in 1adca15

	if workload.HasQuotaReservation(wl) {
	r.queues.DeleteWorkload(wl)
	}

For a pending workload, workload.HasQuotaReservation(wl) returns false, which skips the cache update.

Naively, I'd think that if statement could / should be removed. But I wanted to understand why it's been added in the first place, and if it's correct / safe to remove it.

[mimowo]

/cc @alculquicondor

[alculquicondor]

oh shoot, this is a bug introduced here: 57c714d#diff-60dd240c20adbd6a189d018d1c216c2d296730f446c341d8bf449fa6657964ffR242

it went from admission==nil to IsWorkloadAdmitted.

I think removing the if (always clear from queues) should be fine too.

And please add an integration test.

[astefanutti]

/assign