Repository navigation
feat(kueue): let a deployment describe its queues and their dispatch - #917
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
There was a problem hiding this comment.
Code Review
This pull request parameterizes the Kueue CPU and GPU queue templates to dynamically render resource flavors, node labels, preemption settings, and resource quotas based on Helm values. It also introduces a new multikueue.yaml template to dynamically generate MultiKueue resources, and updates values.yaml with default queue configurations. Feedback on these changes focuses on preventing potential nil pointer dereference errors in the Go templates by safely guarding nested field accesses (such as .flavors and .multikueue) using safe defaults and with blocks. Additionally, it is recommended to simplify the formatting of coveredResources by using the built-in toJson function instead of manual loops.
5e3fba6 to
3d2a138
Compare
97aa6ca to
8ef4801
Compare
3d2a138 to
e434905
Compare
1a26629 to
9c33b65
Compare
The chart ships one flavor per queue, no node labels, and a quota far larger than any cluster. A fresh install therefore admits every workload the moment it arrives, which is the right behaviour for getting started and the wrong one for a cluster with finite accelerators: a queue that never fills never queues, and priority only ever orders workloads that are waiting. This makes the shape of both queues a value rather than a constant. A deployment lists one flavor per class of node its jobs can land on, each with the node labels that select it and the real quota of that class, and may set whether a higher-priority workload preempts a running lower-priority one. MultiKueue dispatch is rendered the same way. A deployment that names worker clusters gets the `MultiKueueConfig`, `MultiKueueCluster`, and `AdmissionCheck` objects for them, on the GPU queue and the CPU queue alike. A deployment that names none gets none, so the default install is unchanged and the manager keeps running the work itself. Defaults preserve today's behaviour: no node labels, the same oversized quota, preemption off, and no dispatch. Quotas are quantity strings rather than bare numbers, because YAML reads a large bare number as a float, which renders in scientific notation and is not a quantity Kubernetes accepts. Sent with ❤️ from Eka Robotics.
9c33b65 to
366e8fc
Compare
6179966
into
eka/kueue-node-bounded-requests
Makes the shape of both Kueue queues a value rather than a constant: their flavors and quota, whether preemption is on, and which worker clusters they dispatch to.
Today the chart ships one flavor per queue, no node labels, and a quota larger than any cluster, so a fresh install admits every workload the moment it arrives. That is the right behaviour for getting started and the wrong one for a cluster with finite accelerators, because a queue that never fills never queues, and priority only ever orders workloads that are waiting. A deployment can now list one flavor per class of node its jobs can land on, each with the labels that select it and the real count of that class in the fleet.
It also closes a gap in MultiKueue. The chart renders dispatch objects for the GPU queue only, so a CPU job on a manager cluster is admitted and then has nowhere to run.
MultiKueueConfig,MultiKueueCluster, andAdmissionCheckare now rendered from values for both queues.Defaults reproduce current behaviour exactly, and an install naming no worker clusters renders no MultiKueue objects at all.
This is three commits squashed into one, and I am happy to split it into the queue configuration, the MultiKueue rendering, and the CPU queue dispatch if that is easier to review or adopt.
Based on #916 rather than
main, only because both add to the samequeueskey invalues.yaml. It does not otherwise depend on it.Tested with
helm lintand the chart configuration tests, with the renderedClusterQueueobjects checked bykubectl apply --dry-run=serveragainst a live cluster.Sent with ❤️ from Eka Robotics.