Repository navigation
docs(design-proposals): add GPUDevice proposal - #99
Aleksei Sviridkin (lexfrei) wants to merge 32 commits into
Conversation
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars.
 |
3846877 to
684efa3
Compare
IvanHunters
left a comment
IvanHunters
left a comment
There was a problem hiding this comment.
LGTM.
Well-formed proposal that follows the template section for section, and the parts I stress-tested hold together: Lost/rebind never double-assigns a device across a node outage, the vGPU default-deny stays consistent across sections 4, 5 and 7, both webhooks fail closed, and forbidden is enforced in layers rather than on one check. The empty ## Decisions section is correct, the template asks for it to be left empty in the first PR. Live CI is green (DCO, CodeRabbit). Sound enough to proceed.
Four non-blocking notes, none needed before merge, left as inline comments.
|
|
||
| ## Security | ||
|
|
||
| - Tenant access. Tenants with the `admin` role or above manage `GPUDevice` in their namespaces; lower roles read it. Every authenticated user can read `GPUClass`, through a ClusterRoleBinding to `system:authenticated`, because tenant roles are bound per namespace and cannot grant a cluster-scoped kind. Only platform administrators write `GPUClass`. Tenants cannot read `GPUNode`. |
There was a problem hiding this comment.
[MINOR] GPUClass.status is world-readable, so cluster-wide GPU inventory is too
GPUClass is cluster-scoped and bound to system:authenticated, and its status carries capacity, bound, free, lost and maxFreePerNode per class. The catalog goal needs free and maxFreePerNode visible to a tenant, but this also exposes the hoster's total GPU count and live per-node utilization to every authenticated tenant user, including one who is a competitor. The gpuclass option source in section 5 is already tenant-scoped and is where a portal reads these numbers. Worth deciding whether the raw per-class/per-node counts belong on a world-readable object or only behind that source.
There was a problem hiding this comment.
IvanHunters The counts are out of the status now. GPUClass.status keeps only free and maxFreePerNode, and its conditions name no nodes and carry no counts. Capacity, bound units and the Lost count went to controller metrics for platform monitoring, and per-node occupancy stays in GPUNode, which tenants can't read. A tenant can still watch free over time and guess how busy a class is. Ordering needs those two numbers, so the text says that openly. A class-metrics test checks that none of the hidden counts shows up in the status.
|
|
||
| ### 5. Usage accounting, catalog and licensing | ||
|
|
||
| Cozystack does not bill. It keeps usage records (`Workload`) that an external billing system reads. Each `GPUDevice` gets one with `kind: gpu-device` and resources `gpu.cozystack.io/<class>: <count>` while it is `Bound` or `Attached`, next to the records Cozystack already keeps for CPU, memory and storage. The GPU controller writes the record itself, under a `WorkloadMonitor` it owns in the tenant's namespace, whose selector matches no object, so tenant labels cannot drop a device from the usage records. Like other `Workload` records it has no time fields: a held interval is the record's lifetime, which a billing system observes by watching and reconciles by sampling. A virt-launcher pod does not report GPUs its devices cover a second time. |
There was a problem hiding this comment.
[MINOR] Null-selector WorkloadMonitor assumes the reconciler leaves foreign Workloads alone
The usage record is written by the GPU controller under a WorkloadMonitor whose selector matches no object, so that a tenant cannot drop a device from billing by relabeling. This holds only if the WorkloadMonitor controller does not prune Workloads that fall outside its selector. If it reconcile-deletes them, every GPU usage record disappears. Confirm the reconciler's behavior for a controller-written Workload under a non-matching monitor, and add it to the usage-record tests, which today assert presence and no double-count but not survival against the monitor's own reconcile.
There was a problem hiding this comment.
IvanHunters It survives. The Workload controller deletes a record only when its workloads.cozystack.io/monitor label is missing or names a monitor that no longer exists (internal/controller/workload_controller.go). The WorkloadMonitor controller never deletes records. Section 5 says this now. The selector is a reserved label key that admission refuses on every object in a tenant namespace, not an empty selector, which MatchingLabels would treat as match-all. The usage-record tests also check that the record survives reconciles of both controllers.
| When every consumer has moved, the hoster sets `gpu.rawResourceNames: forbidden`. Preconditions: no Talos GPU pools in the cluster, ComputePlane GPU groups included (a hoster without a passthrough worker image keeps passthrough Talos pools and raw names), no `gn1.*` GPU instance types in use, and no raw GPU quota keys left. With `forbidden`: | ||
|
|
||
| - an admission policy refuses GPU entries (section 3) on every VMI created without a device reference, on every path; | ||
| - the same policy refuses every pod created in a tenant namespace that requests a GPU resource name, unless it is a virt-launcher pod created by KubeVirt's virt-controller service account with an owner reference to a VirtualMachineInstance. Tenants cannot forge such a pod, since no tenant role can create pods or VMIs, so app charts that copy free-form `resources` into pods cannot request GPUs either; |
There was a problem hiding this comment.
[MINOR] The forbidden guarantee leans on an RBAC invariant worth pinning with a test
The non-VM half of forbidden is stated to be safe because no tenant role can create Pods or VMIs. The real gate is the SA-plus-ownerRef check on virt-launcher pods, which does hold regardless, so this is about making the premise explicit rather than a hole. Confirm the Testing line for the raw-name policy exercises a tenant super-admin attempting a direct Pod and a direct VMI that request a GPU resource name, since that is the tier the prose singles out.
There was a problem hiding this comment.
IvanHunters The premise holds against the tenant roles in cozystack. cozy:tenant:super-admin gets * on kubevirt.io virtualmachines and on apps.cozystack.io, and none of the roles aggregated into it can create pods or VMIs. The admission test line in Testing now covers a tenant super-admin whose direct Pod or VMI is refused by RBAC, whose VirtualMachine with a GPU entry and no device reference gets its VMI refused, and a pod requesting a GPU resource name from any creator other than virt-controller.
| - on a `NotReady` node, attached devices go to `Lost` as well, which drops them from the usage records, and are rebound once their VMIs reach a final phase or are removed from the API; if the node returns while the VMI still runs, the device goes back to `Attached` there; | ||
| - a VM started on such a node after the condition is set is refused (section 3), and the controller deletes the unscheduled VMI of a device that no longer fits, so a VM restarted during the grace period meets the same refusal; either way its device becomes unattached and can move. | ||
|
|
||
| Devices in `Lost` are rebound before new orders when capacity returns, without blocking smaller orders behind a large one. `GPUClass.status.lost` shows how many GPUs are waiting. A rebind never fails a budget check, since a `Lost` device already counts. |
There was a problem hiding this comment.
[NIT] Lost rebind reads unconditional here but is conditional for a live VMI
"Devices in Lost are rebound before new orders when capacity returns" reads as always, but the preceding paragraph qualifies that a device that went Lost because its node was NotReady, while its VMI is still live, waits for the VMI to reach a final phase before rebinding to a new node (otherwise it reattaches on the returning node). A half-clause pointing back at that case would stop a reader taking the two as contradictory.
There was a problem hiding this comment.
IvanHunters Done. The sentence now ends with "a device whose VMI is still live waits until that VMI is final or removed (above)".
65c90dd to
6ce81f3
Compare
|
IvanHunters GPUClass stays readable by everyone, because the offer is meant to be public and tenants name a class when they order, also from kubectl or GitOps. Its status now has only The A direct Pod or VMI from a tenant super-admin never reaches the policy, since RBAC already refuses both. So the test covers that, plus the paths that do reach the policy: a VirtualMachine with a GPU entry and no device reference, and a GPU pod from any creator other than virt-controller. The The push dropped your approval, could you take another look? |
Timofei Larkin (lllamnyp)
left a comment
Timofei Larkin (lllamnyp)
left a comment
There was a problem hiding this comment.
Thanks for this, and for the upstream groundwork behind it (NVIDIA/kubevirt-gpu-device-plugin#192, the vGPU SR-IOV POC, the KubeVirt source references). The problem list is the clearest statement of the GPU gaps we have. Before line-level review I'd like to discuss the shape, because a new API group is close to permanent.
- Please split the host layer (section 6) into its own proposal and land it first. It fixes two of the six problems (one model in both modes, profiles across reboots) on its own, needs no tenant API, and is the decision you asked reviewers to look at first. Today it's Rollout step 4, after all the API work.
- The DRA rejection rests on a premise I don't think holds. The resourceclaim controller keeps non-Pod
reservedForentries ("We don't know how to check this entry, so we just keep it") and deallocates only when the last consumer leaves (controller.go, v1.37.0). KubeVirt VEP 344 ("VM-Scoped Persistent ResourceClaims",persistWhenStopped) builds exactly the stopped-VM hold on that. DRA also brings quota charged at claim creation (<class>.deviceclass.resource.k8s.io/devices), per-device inventory (ResourceSlice), exact-device allocation, and KubeVirt wiring (GPUsWithDRA/HostDevicesWithDRA, beta in v1.9). The gaps that remain (no vGPU in the NVIDIA DRA driver yet, alpha VFIO passthrough, VEP 344 unmerged) are timing gaps. I'd like the proposal to engage with VEP 344 directly, and to say whetherGPUDeviceis defined to map 1:1 onto aResourceClaim. Otherwise the binder becomes a second allocator once thedrasource arrives. - The binder is most of the cost, and the guarantee it buys is soft on most clusters. The hold is hard only under
gpu.rawResourceNames: forbidden, which needs no Talos GPU pools and no ComputePlane GPU groups. Talos is the only worker OS on main today, and ComputePlane has no path. Could Goals say plainly that the guarantee is unavailable until then? Could Alternatives also weigh a cluster-wide per-class reservation for VMs (a count per tenant per class, like a capacity reservation)? It avoids node binding, moves and the pod pin, at the cost of fragmentation for multi-GPU sets. - The product question I'd like maintainers to settle first: must vGPU holds ship before DRA can carry vGPU? If not, a DRA-native design for passthrough first is much smaller. If yes, I'd prefer the smallest interim that doesn't put an allocator into the tenant API.
Smaller points are inline.
|
|
||
| ## Overview | ||
|
|
||
| A tenant who wants a GPU today writes a device-plugin resource name such as `nvidia.com/AD102GL_L40S` into `gpus` of a VM or a worker pool and hopes one is free. Nothing says what each name is, nothing holds a GPU while the VM is off, and an order that does not fit is accepted and then sits in `Pending`. A hoster selling GPUs needs the opposite: a catalog of GPU models and modes, GPUs a tenant takes within a quota and keeps until they release them, the same handling for VMs and managed Kubernetes worker pools, and a usage record of who held what and when, which the hoster's external billing system reads. |
There was a problem hiding this comment.
Small precision point: the gpu option source already describes each name with "N available on " (pkg/registry/core/option/providers.go). What's missing is mode, profile and a display name, not availability. Worth adjusting so the problem statement doesn't overstate the gap.
There was a problem hiding this comment.
Timofei Larkin (@lllamnyp) The problem statement now says the gpu source shows how many of each name the nodes advertise and where, and that what's missing is the model, mode and profile behind a name.
|
|
||
| How NVIDIA's vGPU host driver exposes vGPUs depends on the GPU, the host OS and the driver branch. Older setups use mediated devices (mdev). SR-IOV GPUs on current releases, such as Ubuntu 24.04 with the vGPU 17 or newer branch that Cozystack's guide uses, get VFs, and each VF gets a profile when `current_vgpu_type` is written in its sysfs. KubeVirt v1.9, which Cozystack bundles, advertises such VFs through virt-handler's own device plugin ([kubevirt/kubevirt#16890](https://github.com/kubevirt/kubevirt/pull/16890)), matched by PCI ID; on L40S every VF reports the PF's ID. On GPU Operator nodes nothing in Cozystack writes VF profiles; the `cozystack-nvidia-vgpu-restore` boot unit from cozystack/ansible-cozystack#70 covers only hosts with a host-installed driver. | ||
|
|
||
| Both consumers take GPUs the same way: `vm-instance` and `kubernetes-nodes` read `gpus` as a list of `{name: <resource name>}`, one entry per GPU; the `gn1.*` instance types carry such entries of their own. Tenant quotas become a namespace `ResourceQuota`, which refuses a GPU only when virt-launcher's pod is created, and the usage records (`Workload`) show a GPU only while a VM runs. |
There was a problem hiding this comment.
gn1.* carry {name: gpu1, deviceName: nvidia.com/A400} in KubeVirt's own shape (packages/system/kubevirt-instancetypes/templates/extra-instancetypes.yaml), and nvidia.com/A400 isn't in either default permitted table. They're also marked deprecated. Maybe migration can simply say they are dropped.
There was a problem hiding this comment.
Timofei Larkin (@lllamnyp) Context now describes the gn1.* entries in KubeVirt's shape and notes that nvidia.com/A400 is in neither default permitted table. Section 7 gives them no class to migrate to: a VM on one moves to an instance type without GPUs, and a Talos pool on one takes the Kairos path.
|
|
||
| A `GPUDevice` with `count: 2` is two GPUs on one node attached together; GPUs that move between VMs separately are separate devices. How many GPUs of a class one VM may take is the hoster's `maxCount` on the class. For vGPU classes it follows NVIDIA's multi-vGPU matrix for the board and the profile series. Compute (C-series) profiles follow the [NVIDIA AI Enterprise matrix](https://docs.nvidia.com/ai-enterprise/release-8/latest/infra-software/vgpu/features/multi-vgpu-support.html), which allows several vGPU for Compute devices per VM on L40S under the Linux with KVM hypervisors it supports (section 6), so the `l40s-24c` class above sets `maxCount: 2`. Graphics (Q-series) profiles follow the vGPU 20 release notes for Linux with KVM, which allow several Q-series vGPUs per VM on L40S and offer no C-series profiles. For passthrough classes it is at most the GPUs per node. NVIDIA requires all vGPUs in one VM to share board type and series. Cozystack keeps it simpler and stricter: a VM takes vGPU devices of one class only, although NVIDIA would also allow, say, A40-48Q next to A40-16Q. `maxCount` defaults to 1 for vGPU classes; on a passthrough class, unset means no limit beyond the GPUs of one node. Admission refuses a device above `maxCount`. The spec is immutable, so a change is a new order. | ||
|
|
||
| The inventory is written by a node agent from sysfs and from kubelet's pod-resources API, and goes away with its `Node`. Ownership is exact for running GPUs: a `usedBy` pod leads to its VM, and the VM's devices name it in `status.attachedTo`. A held but not running reservation is a count on a node, not a specific card, because with a device plugin kubelet picks the physical device when the pod starts. PCI addresses stay in `GPUNode`, which tenants cannot read. |
There was a problem hiding this comment.
This is the crux for me. The binder's guarantee is node capacity, not a device, so GPUNode can attribute only running GPUs, and the binder has to reconcile its counts against kubelet's allocatable and raw-name pods. With a DRA backend the allocation names the exact device. That makes me hesitant to fix the device-plugin semantics into a v1alpha1 tenant API that is meant to survive the switch.
There was a problem hiding this comment.
Timofei Larkin (@lllamnyp) The device-plugin coupling sits in what Bound means and in the binder picking the node, not in what a tenant writes or reads. Section 1 now has a table of what Bound holds per source: count free units on one node under devicePlugin, one ResourceClaim and, once it is allocated at the first start, its devices under dra. It also says a tenant sees a class, a count, a phase and a node, never the card, so the API stays the same when an offer moves to a dra class. Under dra the binder does not allocate at all, see line 340.
|
|
||
| A device leaves `Attached` when its VMI reaches a final phase (`Succeeded` or `Failed`) or is removed from the API, whichever comes first. A deletion timestamp alone is not enough: on an isolated node the domain of a VMI being deleted can still hold the GPU, while a final VMI no longer runs its domain. KubeVirt v1.9.0 deletes a final VMI before it creates the VM's next one ([`vm.go`](https://github.com/kubevirt/kubevirt/blob/v1.9.0/pkg/virt-controller/watch/vm/vm.go)), so a new VMI never finds its device attached to the old one. | ||
|
|
||
| A device keeps its node while the `Node` exists, matches the class, has not lost capacity and has no pending layout change (section 6); otherwise an unattached device that no longer fits there moves to another node of its class, or to `Lost` when none has room. When a device's node is deleted, the controller also deletes the device's unscheduled VMI, so the device becomes unattached and moves. KubeVirt cannot live-migrate a VM with a passthrough GPU or an SR-IOV VF, so pinning adds no cost in normal operation. Cordoning or draining a node does not move a device, and the device stays in the usage records: a drained VM waits for the same node, and the device carries a condition naming the cordon. |
There was a problem hiding this comment.
This reverses cozystack/cozystack#4403 for GPU VMs. Since the switch to LiveMigrateIfPossible, a drain evicts a non-migratable VM and it restarts elsewhere. Here it waits for the cordoned node for the whole maintenance window while the tenant is billed. Could an unattached device (its VMI evicted) move when another node of the class has room, keeping the hold, with "stay" as an opt-in class policy?
There was a problem hiding this comment.
Timofei Larkin (@lllamnyp) Moving is now the default. A class has onCordon: Move | Stay, Move by default. Under Move an unattached device on a cordoned node moves to another node of its class when one has room and keeps its hold, and a device still there when a VMI is created is moved first. The devices one VM names move as a set. So after a drain the VM starts elsewhere, as cozystack/cozystack#4403 intended. Stay is for a hoster who wants VMs back on the same hardware. The pod pin is now a required node affinity, because a nodeName pin would let kubelet start the pod on the cordoned node.
| - is on a different node than the VMI's other devices; | ||
| - is a vGPU device next to vGPU devices of another class, or brings the VMI's GPUs of its class above `maxCount`. | ||
|
|
||
| A GPU entry is any `gpus` entry, any GPU resource name in the VMI's `spec.domain.resources`, and any `hostDevices` entry whose device name is a `GPUClass` resource name, a name under `nvidia.com/`, or the name of an NVIDIA (`10DE`) or mdev entry of the KubeVirt `permittedHostDevices` table, on the VMI or from its instance type. A non-NVIDIA GPU without a class does not count, and neither does an FPGA or a NIC of another vendor; an NVIDIA NIC passed through under an `nvidia.com/` name does count and is refused under `forbidden`. The webhook also refuses a referencing VMI that carries GPU entries of its own, from any source, including an instance type or a VM written directly by a tenant super-admin. A VM with a GPU device therefore cannot also take an NVIDIA NIC, such as ConnectX or BlueField, passed through under an `nvidia.com/` name. A refused VMI leaves its devices unattached, so they move by the rules of section 2. Both webhooks see only VMIs and pods carrying the chart's reference label and fail closed, so an outage blocks the start of VMs with device references and nothing else. |
There was a problem hiding this comment.
This blocks GPUDirect RDMA: training VMs routinely take a ConnectX or BlueField VF alongside the GPU, and NVIDIA's network operator advertises those under nvidia.com/. Could the "GPU entry" definition key off the resource names of existing GPUClass objects plus an explicit hoster list, rather than the nvidia.com/ prefix and PCI vendor 10DE? That would also stop the enforcement depending on the vendor.
There was a problem hiding this comment.
Timofei Larkin (@lllamnyp) The prefix is gone. A GPU entry is now a resource name of a GPUClass or one in the hoster list gpu.resourceNames, which defaults to the 10DE and mdev entries of the permitted table plus the names the platform's GPU device plugins advertise. So a ConnectX or BlueField VF under nvidia.com/hostdev goes next to a GPU device, which today is refused even without forbidden. The prefix protected nothing anyway, since KubeVirt passes only names from permittedHostDevices.
|
|
||
| A `vm-instance` entry has exactly one of `name` or `device`, and a `kubernetes-nodes` entry exactly one of `name` or `deviceSelector`. A list holds only `name` entries or only references, and a pool has at most one `deviceSelector`, whose devices must share a class and a `count`; the webhook checks this at each attach, since device labels can change after the pool is admitted. The new entries are KubeVirt-only; Proxmox pools keep refusing `gpus`. ComputePlane is not yet an equal consumer: the module does not let a tenant pick the worker OS, so its GPU groups are Talos pools on `name` entries, and moving them to Kairos is separate work. | ||
|
|
||
| Kairos pools take GPUs only through `deviceSelector`, from classes of both modes. The chart refuses `name` entries on them. The pool's mode, `kairos.gpuMode`, must match the class, and the chart sets it as the `gpu.cozystack.io/mode` label on the worker VMI template ([Choosing the worker image by mode](../kairos-vgpu-workers/README.md#choosing-the-worker-image-by-mode)). At VMI creation the webhook refuses a device whose class `mode` differs from that label, and a pool reference on a VMI without the label. The `kubernetes-nodes` chart, from Rollout step 2, fails to render a `deviceSelector` on a pool with `os: talos`, the field cozystack/cozystack#4911 adds, so the pool's HelmRelease fails before any machine is created. A pool never runs more GPU workers than it has matching devices. Admission on `KubernetesNodes` refuses a `maxReplicas` above that number; the chart default is 10, so a pool with fewer devices sets it. A pool without spare devices rolls one worker at a time: surge machines are created at once, their VMIs are refused until an old worker frees a device, and a surge machine that waits past `nodeStartupTimeout` is replaced by MachineHealthCheck. Talos GPU pools, existing and new, keep working as today on `name` entries, and `GPUDevice` does not extend them. |
There was a problem hiding this comment.
So an autoscaled GPU pool must pre-hold, and pay for, maxReplicas devices, and under forbidden there is no pay-while-running path for VMs or pools at all. That's a meaningful product change. Could the proposal state it as a consequence in User-facing changes? Could it also consider an ephemeral device bound to a VM's or worker's lifetime, the way DRA separates ResourceClaimTemplate from ResourceClaim?
There was a problem hiding this comment.
Timofei Larkin (@lllamnyp) User-facing changes now states it: a held device is billed while held, an autoscaled pool on deviceSelector pays for every device its selector matches, and under forbidden nothing runs on GPUs without a device. The ephemeral device is in section 3 as deviceTemplate, next to held devices. The webhook creates a GPUDevice from it at VMI creation, and the controller deletes it once the VMI is final or gone, like a claim made from a ResourceClaimTemplate, so a pool pays for running workers only. The cost is that the refusal comes at start, not at order time.
| - A child's key only adds a limit; it never lifts an ancestor's limit or a missing grant. | ||
| - A raw GPU key (section 7) on the tenant or any ancestor for a class's resource name blocks orders of that class until the tenant that carries the raw key also has the class key or, for a passthrough class, `total`. While the raw key exists, it also caps those classes' devices together with the raw-name VMs it covers, so an existing GPU limit is never bypassed. | ||
| - For a passthrough class, a tenant with no GPU key of either kind anywhere above it is limited only by capacity. | ||
| - A vGPU class is an exception to the general Cozystack quota semantics, where a missing key means no limit: a tenant may order a vGPU class only if the tenant and every ancestor declare a non-zero key for that class. A missing class key means not allowed, and `total` alone does not allow it, because it names no mode, so each vGPU class is granted explicitly at every level, from the root tenant down. A class key on the root tenant caps that class for the whole cluster, so the hoster sets it to at least the class's capacity from the controller's metrics (section 5), and raises it when nodes are added, unless a cluster-wide cap is intended. Removing a grant or setting it to `0` refuses new orders; devices already held keep counting and can still move or be rebound, while a tenant's Kairos pools lose their vGPU entitlement only once no vGPU class stays granted, as `kairos-vgpu-workers` describes. Raw vGPU names in VMs stay as open as today until a raw key limits them or `forbidden` is set (section 7). |
There was a problem hiding this comment.
Three concerns. (a) The vGPU default-deny mixes entitlement (may this tenant use vGPU at all) with budget (how many). That's why total alone doesn't allow it, and why the root tenant's class key must be raised whenever nodes are added. Could entitlement be its own field, on the class or the tenant, with the rationale stated (licensing, I assume)? (b) These rules diverge from today's pool and carve-out semantics and are explicitly temporary until #48, so tenants migrate twice. (c) The opening paragraph of this section says a tenant with only GPU keys gets no ResourceQuota or LimitRange. Today any key renders both (packages/apps/tenant/templates/quota.yaml), so please list that as a template change.
There was a problem hiding this comment.
Timofei Larkin (@lllamnyp) On (c): section 4 now says it's a tenant chart change, since the chart renders both objects for any resourceQuotas key today.
There was a problem hiding this comment.
Timofei Larkin (@lllamnyp) On (a), entitlement is now a flag on the Tenant, gpu.vgpu. The parent sets it, and it holds only when the parent's holds, so no tenant can grant itself vGPU. The chart puts the effective value on the namespace as a label. The reason is licensing: a vGPU license is counted per unit, which is a budget, but getting the guest driver and the license token at all is yes or no. Class keys and total now only count, so total works for vGPU and the root key no longer tracks capacity. It is not on GPUClass, because every user can read the class and a tenant list there would publish the hoster's customers.
On (b), the key names don't change, so tenants don't rewrite keys twice. The real gap was that #48 as written covers neither GPUs nor entitlement. Section 4 is now GPUDevice's own rule, the declaration gate skips GPU keys, and the #48 rollout step applies only if its gate takes over subtree caps for GPU keys.
|
|
||
| Until the phase ships, a cluster keeps the modes its variant gives it today, one per model for models whose whole GPU and VFs share a PCI ID, and vGPU classes keep the experimental status `docs/gpu-vgpu.md` gives the `vgpu` variant: the hoster writes profiles and node labels by hand. On clusters without a layout, which includes the `default` variant and Talos hosts, node labels stay hand-written. | ||
|
|
||
| The device plugin is the decision reviewers should look at first. Cozystack ships NVIDIA's kubevirt-gpu-device-plugin with #192 applied downstream, as the sandbox device plugin of the `vgpu` variant, until #192 merges upstream or upstream takes another approach. #192 adds discovery of vGPU VFs on the vendor-specific VFIO framework and advertises them with one resource name per profile, next to the passthrough names the plugin already advertises for GPUs bound to `vfio-pci`. One plugin then serves both modes of every model under distinct names, so the per-model limit of the problem list goes away. The branch is validated on hardware. Its upstream review has had no maintainer response since 2026-07-10, which is why it ships as a downstream patch rather than waiting. On a card whose VFs all carry a profile, the plugin can read profile names only through NVML, so the platform runs it as its own DaemonSet with the host driver's `libnvidia-ml.so.1` and the `/dev/nvidia*` nodes mounted, privileged; the GPU Operator's DaemonSet has no field for those mounts. If #192 merges in another form, or is rejected and NVIDIA's DRA path covers vGPU instead, only this phase changes. KubeVirt's built-in plugin was the other candidate (Alternatives). |
There was a problem hiding this comment.
On the question you raised: carrying #192 downstream seems a reasonable bridge to me, given the NVIDIA review said the approach makes sense and the built-in plugin can't give each profile its own name. I'd still like it in its own proposal with exit criteria (#192 merged, or DRA vGPU in kubernetes-sigs/dra-driver-nvidia-gpu). I'd also like Alternatives to record why NVIDIA vgpu-device-manager can't be the profile writer, since its upstream VF support was closed.
There was a problem hiding this comment.
Timofei Larkin (@lllamnyp) Section 6 now says when the downstream patch leaves. That happens when NVIDIA/kubevirt-gpu-device-plugin#192 merges upstream in any form, or when the NVIDIA DRA driver serves vGPU VFs to KubeVirt (kubernetes-sigs/dra-driver-nvidia-gpu#1402). Alternatives has the vgpu-device-manager entry. At v0.5.1 it writes mdev profiles only and has no current_vgpu_type path, and the VF pull requests were closed unmerged, NVIDIA/vgpu-device-manager#139 as not planned and NVIDIA/vgpu-device-manager#195 after NVIDIA pointed at gaps on the KubeVirt side and had no resources for that test round.
I kept the host layer in this proposal and in its rollout place. Shipped first, it would put the first vGPU tenants on raw names and raw quota keys, which is the population section 7 then has to migrate.
|
|
||
| Quota keys move too. A raw GPU key is a quota key on a GPU resource name, such as `nvidia.com/AD102GL_L40S`. A device-backed VM's pod still requests that name, so while a tenant migrates, its raw key covers both kinds of VM; after its last raw-name VM has moved, the key is replaced by `gpu.cozystack.io/*` keys. A tenant whose raw-name vGPU VMs run without a class key can order their devices only after the class key is granted on every level down to that tenant, by the hoster on the root tenant and by each parent's administrator on its child, at least for what those VMs use today, and on each ancestor for its whole subtree. | ||
|
|
||
| When every consumer has moved, the hoster sets `gpu.rawResourceNames: forbidden`. Preconditions: no Talos GPU pools in the cluster, ComputePlane GPU groups included (a hoster without a passthrough worker image keeps passthrough Talos pools and raw names), no `gn1.*` GPU instance types in use, and no raw GPU quota keys left. With `forbidden`: |
There was a problem hiding this comment.
Given these preconditions, most current clusters can't reach forbidden until Kairos pools (cozystack/cozystack#4911, still a draft) and ComputePlane on Kairos both land. Please state that timeline in Goals. "The hold is a hard guarantee" reads as the default outcome, but it's the end state.
There was a problem hiding this comment.
Timofei Larkin (@lllamnyp) Goals now says the hard hold is the end state, not the default. forbidden waits for Kairos pools (cozystack/cozystack#4911) and for ComputePlane on Kairos workers, and until then a cluster with Talos GPU pools or ComputePlane GPU groups keeps raw names and has no guaranteed hold.
|
|
||
| ## Alternatives considered | ||
|
|
||
| DRA as the whole API, with `DeviceClass` as the catalog and `ResourceClaim` as the tenant object. Rejected for now. An allocation exists only while a pod reserves its claim, so a GPU held for a stopped VM, which hosters charge for, cannot be expressed, and NVIDIA's DRA driver does not serve vGPU to KubeVirt yet. DRA is the planned backend under `GPUClass` (section 8), not the tenant API: tenants keep `GPUDevice` when the backend changes. |
There was a problem hiding this comment.
See the top-level comment: a non-Pod reservedFor entry keeps a claim allocated, and KubeVirt VEP 344 relies on that. Please also note that DRA quota is charged at claim creation, which covers "refuse at order time" for budgets. The honest remaining gaps seem to be vGPU support, VEP 344 status, and holding before the first boot.
There was a problem hiding this comment.
Timofei Larkin (@lllamnyp) The old DRA rejection was wrong and is rewritten. Under a dra source a GPUDevice maps one to one onto a ResourceClaim created at order time. The scheduler allocates it, the GPU controller keeps it through its own reservedFor entry, and the binder does not allocate, so there is no second allocator. Section 8 lists the gaps you named, plus two more: passthrough in the NVIDIA DRA driver is still alpha, and DRA quota is per namespace, without subtree caps.
40bf77f to
5e38d44
Compare
A tenant can only ask for a GPU by a device-plugin resource name, cannot hold one while a VM is stopped, and learns that an order does not fit only when virt-launcher stays Pending. Propose a hoster-defined GPU catalog (GPUClass), tenant-held GPU reservations that VMs and Kairos worker pools attach the same way (GPUDevice), a per-node inventory of physical GPUs and VFs (GPUNode), and usage records that an external billing system reads. Cover the host layer under them as a replaceable phase: passthrough and vGPU nodes in one cluster, declarative vGPU profiles, and NVIDIA/kubevirt-gpu-device-plugin#192 carried as a downstream patch until upstream settles. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la>
5e38d44 to
278c6fd
Compare
|
Timofei Larkin (@lllamnyp) On the four shape questions, with details in the inline threads:
|
Andrei Kvapil (kvaps)
left a comment
Andrei Kvapil (kvaps)
left a comment
There was a problem hiding this comment.
LGTM as of 278c6fd, with one ask before merge.
We decided to postpone Kairos worker pools until there is a separate proposal for moving tenant pools to Kairos (see my comment on #101). This proposal still depends on Kairos in three places: Kairos as a vGPU host OS in section 6, deviceSelector and deviceTemplate only for Kairos pools in Rollout step 2, and "no Talos GPU pools" as a precondition for forbidden. Talos can do passthrough, both on hosts and in worker pools. Could you drop Kairos hosts here, and let Talos pools take deviceSelector and deviceTemplate with passthrough classes? Then step 2 doesn't wait for cozystack/cozystack#4911, and forbidden only needs no raw-name pools.
nits, up to you:
- The VMI webhook writes attachments, asks the binder to move devices within the admission timeout and creates template devices, all fail closed. Wouldn't it be simpler to keep the webhook to check, record and pin, and let the controller create template devices on the VM start request?
GPUDevicewithcount: 2is a claim for units on a node rather than a device, soGPUClaimfrom your open questions reads better to me.- "KubeVirt v1.9.0 deletes a final VMI before it creates the VM's next one" holds for
AlwaysandRerunOnFailureonly (vm.go:987-1011, 1049-1053). - Monitor-made
Workloadrecords are also removed by the ownerReference GC. Worth saying which object owns the GPU record. - #192 reads profile names from sysfs
creatable_vgpu_typesfirst, NVML is the fallback for a fully allocated card. - "On L40S every VF reports the PF's ID": is there a source or
lspci -nnoutput for it?
Building the first step of GPUDevice settled several questions the proposal left open or answered differently: how admission handles a class KubeVirt does not permit, what runs on a cluster without GPUs for VMs, how the usage-monitor label is reserved, what GPUNode health means, which taints count, when a Stay pin gives way, who can order template devices, and when the controller gets its host-layer rights. Record each one next to the proposal, so the reasoning survives the proposal text being brought in line. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la>
Several sentences of the proposal now contradict what the decision records settled, or leave out the rule the records add: the taint rule read as a whole prefix, the usage-monitor label as refused on every update, and the Security section as granting the controller its node rights and tenant admins their writes on every cluster. Change those sentences and point each one at its record, so a reader of the proposal does not get the old rule. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la>
The failure cases did not say what happens to a VM pinned by a Stay device to a cordoned node when that node then fails, and section 3 alone reads as if such a VM waits on the dead node indefinitely. Describe the implemented behaviour: the pin holds through NotReady and gives way once the node has lost capacity, after which the set moves or goes to Lost as any other set does. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la>
…tails Section 4 and the rollback rules explained why GPU keys stay out of the tenant quota pools by walking through how pool membership treats undeclared keys. The rule itself is what the design needs: GPU keys stay out of the pool and carve-out model, so a budget covers every descendant, and raw keys keep following the pools. The Stay-pin sentence in section 3 now says "no template devices still to place", which is the condition the implementation checks. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la>
A GPUClass toleration is copied into every virt-launcher pod of the class, so a toleration that pod validation refuses would make each of those pods unadmittable. The API therefore admits only the Exists and Equal operators and pod-valid fields. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la>
A VMI cannot carry a GPU condition, since KubeVirt's admission guards writes to it, so the reason a set waits reaches the VMI as a Warning event while the claims keep the condition. A set placed after a WaitForFirstConsumer volume bound is limited to the nodes that the volume's node affinity allows. A set of template claims only loses its claims with the failed VMI, so the controller keeps its rejected nodes in memory against the VM, and a restart forgets them. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la>
|
I took the rest of the review in 173fc65, with #100 and #101 following the rename and the class-named pool. Two points I'd keep are at the end. The VMI webhook now only checks and records, and The GPU The I'd keep holding a GPU for a stopped VM in this proposal. Your reason for the split is that the hold is soft until The hold doesn't have to be in the first v1alpha1, though. I'd do one proposal with two rollout steps. Step 1 is the order checked against budget and capacity, For vGPU I'd keep the flag. In Cozystack a missing quota key means no limit. A |
|
Hi! We removed Sorry for the extra work, I wanted to mention this now so it doesn't come up late in review. |
The controller's pod update and delete rights sit in its ClusterRole, like its write on Secrets, because tenant namespaces appear at runtime, so the Security section and DR 0008 now scope them to every namespace. DR 0004 described allocatable as summed over healthy functions, with an unhealthy function alone counting as lost capacity. The controller sums allocatable over every function carrying the resource name, caps it at the kubelet's node allocatable, and the agent counts an IOMMU group device once, on its lowest-addressed GPU function. DR 0012 now states, as the README does, that rejected nodes of a template-only set live in controller memory and do not survive a restart. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la>
…, in 0012 KubeVirt's admission guards VMI writes, so the reason reaches the VMI as an event, as the README already states. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la>
The GPU admission webhooks read the KubeVirt table, GPUClass, GPUClaim, GPUNode, namespaces and VMIs from the API server, not from the controller's informer cache. The controller runs two replicas that all serve admission while only the leader binds, so a replica whose cache lags could admit what should be refused. Decision 0001 still said the permitted table is read through the cache. Correct it and give the reason. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la>
Andrei Kvapil (kvaps)
left a comment
Andrei Kvapil (kvaps)
left a comment
There was a problem hiding this comment.
Thanks, this is a big step. Kairos is out, the class carries tolerations, the usage record is owned by its claim and the label policy is gone, and the scheduling gate is much better than doing the work inside admission. One thing left before LGTM as of 694eceb: the two rollout steps for the hold are only in your comment, the text still has the binder, onCordon and Lost in Rollout step 1. I'm fine with one proposal if the text says it: which fields and phases exist in step 1, what a claim entry means there without the binder, and step 2 as its own section with its own decision record. Then the hold enters the API only when we get to it.
Agreed on the vGPU flag: it's a yes or no, not a quota, and a Tenant field stamped as a namespace label is the same pattern as namespace.cozystack.io/etcd and the other module labels. Could it move out of section 4 into its own section with a decision record, and say when to revisit it, for example once #101 stops copying Secrets into tenant namespaces?
nit: #111 removed Status and Date from proposal headers.
Andrei Kvapil (kvaps)
left a comment
Andrei Kvapil (kvaps)
left a comment
There was a problem hiding this comment.
LGTM as of 694eceb. The two notes from my comment above, the rollout steps for the hold and a separate section for the vGPU flag, are up to you, not blocking.
|
One more API-shape question, which I should have raised in my first review: the proposal introduces a new API group, My concern is less the group name than what it's built around. Kubernetes treats CPU and memory as counts: a pod asks for
So above the host layer, a tenant needs "two L40S-24C", which is a count of a named resource. Going through what the proposal adds on top of that:
That leaves the hold, which is the only part that needs a tenant-owned object. As Andrei Kvapil (@kvaps) noted, the binder, moves, What I'd suggest:
If you see a requirement that only a dedicated group can carry, I'd like to see it written down: a short section or decision record saying why a new group rather than |
Proposal status and dates now come from the PR and Git history. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la>
No separate API group, no hold and no GPUNode in step 1, holding as a GPU-specific step 2 until DRA can carry it, and the vGPU flag as an entitlement apart from budgets. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la>
…step Step 1 has no tenant object: a gpus entry names a GPUClass in cozystack.io, budgets are pool keys checked at write through #48 with an interim check until then, and usage is the launcher pod's Workload. Holding, with GPUClaim, the binder and the launcher pod gate, moves to its own step 2 section, and the vGPU flag gets its own section. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la>
Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la>
Without a binder, a drained, dead or failing node in step 1 leaves a VM on a class entry to its run strategy and to any node with a free unit. Say so, keep the raw-key cap on held claims and the layout rule of the step 2 binder, place the tolerations webhook in step 1, and test the class metrics and the two-class taint rule. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la>
…KubeVirt Whether KubeVirt creates a next VMI depends on how the previous one ended: a start-failure backoff applies only to a VMI that never ran, and a VMI the controller deletes goes Failed first. One table at the top of Failure and edge cases holds this, with KubeVirt v1.9.0 line references, and every case states an outcome only where its runStrategy produces one. Step 2 also leaves nodes with a pending layout change out of free. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la>
…able A VMI that the controller deletes before it ran counts as a start failure, so an Always VM waits the backoff, and during the backoff the VM shows CrashLoopBackOff, as a Once VM does without retrying. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la>
…able A refused pinned pod, or a VMI the controller deletes, costs a start-failure backoff only where the VM's runStrategy creates a next VMI. Record 0012 and the cases that restart a VM after its claims moved now refer to the runStrategy table for that, and the refused-VMI backoff is named as the VM controller's per-item backoff. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la>
… outcome appears Each sentence that states what a runStrategy does now names the situation of the runStrategy table it relies on, or points at the table, so no outcome is restated without its case. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la>
… gaps The consumer proposals need only GPU classes, so the Rollout lists them in step 1, ahead of #48 and of holding. Restarts after an eviction are conditional on the runStrategy, a directly written VirtualMachine can also be missing from the usage records until super-admins lose that write access, and maxCount stays editable in step 1 while step 2 freezes it for a class in use. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la>
"Held claim" as a noun collided with "holds GPUs", which only Bound and Attached claims do, and left the phase table, the refusal list and the usage-record sentences unclear about template claims. An ordered claim is a GPUClaim the tenant orders; a template claim holds GPUs in the same phases but has no budget or usage record of its own. A class being deleted keeps serving its Bound, Attached and Lost ordered claims, and refuses a Pending one, which would never be bound. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la>
A token baked into a shared guest image reaches every tenant that can clone it. GPUClass gains a license with the hoster's token Secret and the FeatureType; the GPU controller copies the Secret only into entitled namespaces, and the VMI webhook gives each VMI of a licensed vGPU class a Secret volume, so VMs and pool workers get the license the same way and rotation is a VMI restart. The vGPU flag's revisit condition now points at this copy, and links to the withdrawn Kairos GPU proposal point at the Kairos tenant pool proposal. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la>
Andrei Kvapil (kvaps)
left a comment
Andrei Kvapil (kvaps)
left a comment
There was a problem hiding this comment.
LGTM as of 5a20666. Step 1 without a tenant object, the classes in cozystack.io with DRA-style quota keys, and the license as a secret volume are what I hoped for. Thanks to Timofei Larkin (@lllamnyp) for pushing the shape.
NVIDIA's license system documents no way to revoke a client configuration token, and a running guest renews its lease while the allocation serves the token. Revocation is therefore stated as what it is: clearing the entitlement takes effect at the next start, an immediate cut-off stops the VMs, and rotation moves to a new allocation and disables leasing on the old one. The guest contract now copies the token into a directory on a memory file system named by ClientConfigTokenPath, so a guest restarted without the license disk has no token left. The eviction outcomes per runStrategy and the live-migration limits of GPU VMs move from the GPU VMs proposal into the runStrategy table, which now covers a VMI that succeeded, and Scope and Rollout follow the licensing proposal. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la>
|
Andrei Kvapil (@kvaps) Both notes are in. Step 1 has no tenant object: a The vGPU flag has its own section 6 and record 0017. The license now reaches each VM as a KubeVirt
|
|
Timofei Larkin (@lllamnyp) There is no The rest follows your four points. The host layer is section 2 and ends at one resource name per offer. Classes are hoster-side, On the hold I'd keep it GPU-specific rather than generic. CPU and memory are interchangeable across nodes, while a GPU hold is a whole unit on one node that does not migrate, and that is what the binder, |
Timofei Larkin (lllamnyp)
left a comment
Timofei Larkin (lllamnyp)
left a comment
There was a problem hiding this comment.
LGTM as of 30da976. Thanks, Aleksei Sviridkin (@lexfrei), for taking the shape questions on board this thoroughly. Step 1 without a tenant object, the host layer first and on its own, the kinds in cozystack.io with DRA-style quota keys, budgets as ordinary pool keys under #48, and node allocatable instead of GPUNode all read right to me. 0013 and 0016 record the reasoning well. On holding, I still lean toward a general mechanism, but 0016 makes a fair case for the node-bound GPU hold, and its revisit condition is the right trigger.
Non-blocking notes:
- Implementation. cozystack/cozystack#4998 still creates
gpu.cozystack.iowithGPUClaimandGPUNode. As 0013 says, the stack needs the move tocozystack.ioand the step 1/step 2 split before any of it merges. I'll look for that on the next push. - Step 2 is accepted as intent, not as a design to build as written. Section 9 ships in this proposal, but a lot can change before holding is implemented (VEP 344, vGPU in the DRA driver, #48). I'd expect step 2 to come back for review as its own change, with a decision record, rather than be implemented straight from this text.
- Naming leftovers. The directory is still
gpudeviceand the PR title still says "GPUDevice". Renaming the directory (togpu-classes, say) is cheap now and breaks links later. Record number 0003 is also missing from the sequence. Either is fine to leave if you prefer.
Adds a design proposal for GPUs as a tenant resource. Today a tenant asks for a GPU by a device-plugin resource name, cannot keep one while the VM is stopped, and finds out that an order does not fit only when virt-launcher stays
Pending.The proposal adds three kinds.
GPUClassis the hoster's catalog of offers: model, passthrough or vGPU, profile.GPUDeviceis a GPU a tenant orders within a quota and holds until they delete it, and VMs and Kairos worker pools attach it the same way.GPUNodelists every physical GPU and VF and who uses it. Held devices show up in usage records (Workload) that an external billing system reads; Cozystack charges nothing and stores no prices.It also covers the host layer as its own phase, on Ubuntu or Kairos GPU hosts. A node layout in the
vgpuvariant lets one cluster have passthrough and vGPU nodes of the same model, with vGPU profiles written declaratively and kept across reboots.The decision I'd like reviewers to look at first is the device plugin, in section 6. I propose to ship NVIDIA's kubevirt-gpu-device-plugin with NVIDIA/kubevirt-gpu-device-plugin#192 as a downstream patch until upstream merges it or takes another approach. The plugin sits behind the host-layer boundary: the API, quotas, webhooks and migration use no plugin-specific names or formats, so a different outcome for #192 changes only that phase. KubeVirt's built-in plugin is in Alternatives.
Two related proposals build on this one from the start.
kairos-vgpu-workers(#101) adds Kairos GPU worker pools that attach devices throughdeviceSelector, andvgpu-vms(#100) adds GPU VMs with hoster-supplied guest images that attach them throughdeviceentries. Section 7 covers only the consumers that exist today: VMs withgpus, Talos GPU pools andgn1.*instance types.Before review
decisions/directory, or says below why none is needed.This is a new proposal, not a revision, so no decision record is needed.
DCO
git commit --signoff).