feat: CDI-based NVIDIA GPU passthrough into gVisor actor containers - #502
feat: CDI-based NVIDIA GPU passthrough into gVisor actor containers#502eliranw wants to merge 3 commits into
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. |
b510ec9 to
2693528
Compare
|
eliranw is there a way to do this without introducing all the new vendored components? (21k lines of code is too much!) |
|
It seems like we decided to commit vendor packages in this repo. I think it’s best to use the cdi package instead of reimplementing it. As far as I understand, the main downside of committing the vendor folder (instead of gitignoring it) is having a large diff. Do you have a different solution? To make it easier to review, I moved the vendor changes into a separate commit. I noticed many other PRs do the same thing - does this approach make sense to you? |
2693528 to
9cf40a5
Compare
|
eliranw it's not whether the vendor-ed stuff is a separate commit or not. just that we should avoid adding stuff if not really required. see for example - main...dims:substrate:gpu-nocdi |
|
thanks for working on this I'd also like to keep imported deps minimal. I started looking at uVM without privileged and we may have a bigger problem there ... ideally I don't want to require a plugin on the host but we need permission to talk to KVM. I may have an answer to that wrinkle but broadly speaking I'm also hoping that even if not implemented yet we can figure out how we'll extend this to uVM later, as I'm also working on closing the existing gap (durable directory support) at higher priority ... |
f896c76 to
5320561
Compare
|
Thanks Davanum Srinivas (@dims) and Benjamin Elder (@BenTheElder). Dropped the vendored CDI as you both suggested and re-tested on a GPU node. I’ve also gated the whole thing to gVisor-only for now so the NVIDIA toolkit mount only lands on gVisor pools, meaning a micro-VM pool that requests a GPU won’t pick up the nvidia toolkit. On uVM - I'll start looking into that |
37f756b to
39d9060
Compare
|
Benjamin Elder (@BenTheElder) Davanum Srinivas (@dims) Omer Yahud (@omeryahud) - I've updated this PR to reflect #496 (unprivileged ateom). I had a few issues getting the GPU CDI hooks working, but right now GPU pools keep exactly the same posture as any other gVisor worker - no user namespace, no procMount: Unmasked, and per-actor cgroup delegation still working. The issue was that only one CDI hook was failing: update-ldcache, which needs the private /proc; skipping just that hook (and writing the SONAME symlinks from DT_SONAME instead) avoids the issue I had. The other CDI hooks run unmodified. I also needed a debian:stable-slim gvisor image to run the cdi generation tool. I did a full test end to end on a cluster with a T4 node workerpool on GKE with different NVIDIA tooling to check compatibility: nvidia-smi, vectorAdd, nbody, and a PyTorch cuBLAS matmul, plus two workers with two GPUs each on one 4-GPU node to check isolation betweek workers on a single node. I've added details in the updated description. |
Signed-off-by: Eliran Wolff <eliranw@nvidia.com>
| // (see nvproxyGlobalArgs). Set once here at boot to avoid a data race on the | ||
| // concurrent RunWorkload path. | ||
| if gpuPresent() { | ||
| runscNvproxy = true |
There was a problem hiding this comment.
I don't like passing arguments in a back channel, let's pass it as a function argument to maybeInjectGPU.
You can leave the check here for logging purposes
| // maybeApplyGPUPodShape shapes a gVisor worker pod that requests a GPU so ateom | ||
| // can inject the GPU into actors via CDI. It mounts the host NVIDIA toolkit | ||
| // (version-matched to the node) — the glibc-based ateom image runs those toolkit | ||
| // binaries directly — and applies the user-namespace + unmasked-/proc posture the |
There was a problem hiding this comment.
Is this correct? A left-over from a previous implementation?
fc0575c to
74bc170
Compare
Generate a per-pod CDI spec with nvidia-ctk, parse it in-tree, and inject the device nodes (major/minor stat-resolved) of the "all" CDI device, the driver- library mounts, and env into each actor's OCI spec. Run the CDI createContainer hooks from the mounted toolkit except update-ldcache, whose ldconfig needs a private /proc mount; stage the SONAME symlinks it would create from each library's ELF DT_SONAME instead. That keeps the GPU worker on the same unprivileged posture as any other gVisor worker (no user namespace, no procMount:Unmasked, cgroup delegation intact). Enable runsc --nvproxy at sandbox creation. Detect GPUs by device-node glob so any assigned index works (multi-GPU). The distroless ateom cannot exec nvidia-ctk, so GPU pools must run a glibc ateom build (WorkerPool.spec.ateomImage). Signed-off-by: Eliran Wolff <eliranw@nvidia.com>
Signed-off-by: Eliran Wolff <eliranw@nvidia.com>
74bc170 to
01a3e03
Compare
4 participants
Summary
atecontrollerpropagates a pool'snvidia.com/gpurequest onto theateomcontainer and mounts the host NVIDIA toolkit read-only (path overridable viaATE_NVIDIA_TOOLKIT_HOST_PATH)ateom-gvisorgenerates a CDI spec withnvidia-ctkand injects the device nodes, driver-library mounts, and env into each actor container's OCI speccreateContainerhooks exceptupdate-ldcachewhich needs a privileged ateom, staging the SONAME symlinks it would create from each library's ELFDT_SONAMErunsc --nvproxyat sandbox creationRequesting
nvidia.com/gpuon the pool is the only configuration needed; a pool that requests N GPUs makes all N usable.Two details of the CDI spec are worth calling out, because getting either wrong fails at runtime rather than at parse time.
nvidia-ctkleavesmajor/minorunset — CDI delegates that to the OCI runtime — so each device node is resolved by stat-ing the host; without it the actor gets0,0char devices and NVML reports it cannot communicate with the driver. And it emits per-index, per-UUID, andalldevices that repeat the same nodes, so onlyallis applied. The spec is plain JSON, soencoding/jsonsuffices and no CDI library is vendored.update-ldcacheis the one hook that cannot run here: itsldconfigunshares a mount namespace and mounts a private/proc, whichmount_too_revealing()rejects under the pod's masked/proc. Permitting it would needprocMount: Unmasked, which Kubernetes only allows withhostUsers: false, and that user namespace breaks the per-actor cgroup delegation from #496. Skipping it avoids the whole chain, so a GPU worker keeps the same posture as any other unprivileged gVisor worker.create-symlinksandenable-cuda-compatstill run unmodified.--nvproxymust be set when the sandbox is created — thepausecontainer, which holds no GPU devices — so runsc's auto-detection never fires on its own; without the flag the GPU subcontainer crashes the sentry on start. GPU detection matches any device index rather than assuming/dev/nvidia0, since a worker sharing a multi-GPU node can be assigned/dev/nvidia2and/dev/nvidia3.GPU pools must set
spec.ateomImageto a glibc build (KO_DEFAULTBASEIMAGE=debian:stable-slim ko build ./cmd/ateom-gvisor) because the distroless default cannot execnvidia-ctk; the default base is unchanged for every other pool.ateletalso has to run on the GPU nodes to restore actors there, so its DaemonSet needs a toleration for whatever taint they carry. Both are documented in the API guide rather than defaulted.Testing
make testenv -u NO_COLOR make verifynvidia-smi,vectorAdd,nbodyat 3.77 TFLOP/s, PyTorch matmul via cuBLAS at 3.5 TFLOP/s (T4 peak FP32 is ~8.1, so no measurable sandbox penalty)Not yet exercised: snapshot/restore of a running GPU actor.
Fixes #627