What happened:
A batch handed to UpdateWorkloadPriority for an owner that names no WorkloadPriorityClass can write the workloads that were never following one.
classifyWorkloadsForPriorityUpdate (pkg/controller/jobframework/reconciler.go) sorts the batch by comparing each workload's class name against the owner's. With no label on the owner, that name is "", and a workload with no priorityClassRef reads as naming the same empty class, so it lands in sameClassName. A sibling in the same batch that does hold a WorkloadPriorityClass reference lands in needsClassChange, which is what makes the helper resolve at all. applyResolvedPriority then writes both groups, so the unreferenced workloads are given whatever the fallback resolved to.
Their value did not come from the label. It came from a Pod PriorityClass or the cluster default, and this reconciliation is not supposed to touch it — #13781 and #8480 exclude those workloads on purpose.
What you expected to happen:
A workload with no priorityClassRef under an owner that names no class is left alone, whatever a sibling in the same batch is doing.
How to reproduce it (as minimally and precisely as possible):
The batch has to hold both shapes at once, so it needs a caller that passes several workloads:
- A Job with no
kueue.x-k8s.io/priority-class label.
- One Workload for it with no
priorityClassRef, carrying a value from a Pod PriorityClass.
- A second Workload in the same batch holding a
WorkloadPriorityClass reference.
- Call
UpdateWorkloadPriority with both.
The second one transitions, and the first is written along with it.
The path that reaches this today is reconciler.go's workload-slice branch, which since #13780 hands the helper the replacement and the quota-reserved slice it waits behind. Whether a real slice pair can hold those two shapes at once is the first thing to establish; I have not shown that it can.
Anything else we need to know?:
I had a guard for this in #13904 and took it back out. That PR is about resolving a WorkloadPriorityClass once per reconcile for a LeaderWorkerSet, and after narrowing it to the label path the guard is no longer load-bearing there: the no-label path hands the helper one workload at a time, and a single workload with no reference produces an empty needsClassChange, so nothing is written either way. Removing it left every LeaderWorkerSet test passing.
That left it as a change to shared behaviour for other callers, sitting in a PR about something else, so it belongs here instead. The guard was:
if jobPriorityClassName == "" && workload.HasNoPriority(wl) {
continue
}placed beside the existing quota-reserved skip in classifyWorkloadsForPriorityUpdate, with a case in TestUpdateWorkloadPriority named "leaves the unreferenced workloads of an unlabelled owner alone while a sibling transitions". Whether that is the right fix is worth deciding here rather than inheriting it.
Environment:
- Kubernetes version (use
kubectl version): N/A, read from the source
- Kueue version (use
git describe --tags --dirty --always): v0.20.0-devel
- Cloud provider or hardware configuration: N/A
- OS (e.g:
cat /etc/os-release): N/A
- Kernel (e.g.
uname -a): N/A
- Install tools: N/A
- Others: N/A
This issue was written in part with the assistance of generative AI.
I verified the classifier and the callers against the tree myself.
What happened:
A batch handed to
UpdateWorkloadPriorityfor an owner that names noWorkloadPriorityClasscan write the workloads that were never following one.classifyWorkloadsForPriorityUpdate(pkg/controller/jobframework/reconciler.go) sorts the batch by comparing each workload's class name against the owner's. With no label on the owner, that name is"", and a workload with nopriorityClassRefreads as naming the same empty class, so it lands insameClassName. A sibling in the same batch that does hold aWorkloadPriorityClassreference lands inneedsClassChange, which is what makes the helper resolve at all.applyResolvedPrioritythen writes both groups, so the unreferenced workloads are given whatever the fallback resolved to.Their value did not come from the label. It came from a Pod
PriorityClassor the cluster default, and this reconciliation is not supposed to touch it — #13781 and #8480 exclude those workloads on purpose.What you expected to happen:
A workload with no
priorityClassRefunder an owner that names no class is left alone, whatever a sibling in the same batch is doing.How to reproduce it (as minimally and precisely as possible):
The batch has to hold both shapes at once, so it needs a caller that passes several workloads:
kueue.x-k8s.io/priority-classlabel.priorityClassRef, carrying a value from a PodPriorityClass.WorkloadPriorityClassreference.UpdateWorkloadPrioritywith both.The second one transitions, and the first is written along with it.
The path that reaches this today is
reconciler.go's workload-slice branch, which since #13780 hands the helper the replacement and the quota-reserved slice it waits behind. Whether a real slice pair can hold those two shapes at once is the first thing to establish; I have not shown that it can.Anything else we need to know?:
I had a guard for this in #13904 and took it back out. That PR is about resolving a
WorkloadPriorityClassonce per reconcile for a LeaderWorkerSet, and after narrowing it to the label path the guard is no longer load-bearing there: the no-label path hands the helper one workload at a time, and a single workload with no reference produces an emptyneedsClassChange, so nothing is written either way. Removing it left every LeaderWorkerSet test passing.That left it as a change to shared behaviour for other callers, sitting in a PR about something else, so it belongs here instead. The guard was:
placed beside the existing quota-reserved skip in
classifyWorkloadsForPriorityUpdate, with a case inTestUpdateWorkloadPrioritynamed "leaves the unreferenced workloads of an unlabelled owner alone while a sibling transitions". Whether that is the right fix is worth deciding here rather than inheriting it.Environment:
kubectl version): N/A, read from the sourcegit describe --tags --dirty --always): v0.20.0-develcat /etc/os-release): N/Auname -a): N/AThis issue was written in part with the assistance of generative AI.
I verified the classifier and the callers against the tree myself.