Repository navigation
docs(gpu-operator): document passthrough variant host-driver prerequisite - #2767
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughDocumentation updated to cover passthrough and vGPU modes: adds a "Passthrough variant: host preparation" section describing host-driver detection, a clean-host workaround, FORCE_REINSTALL limitation, and updates the OS support table and vGPU/bundle/KubeVirt guidance. ChangesGPU Documentation: Passthrough and vGPU
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
83c656c to
4b5601f
Compare
4b5601f to
6c00af5
Compare
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses an undocumented limitation in the cozystack.gpu-operator where the passthrough variant fails if an NVIDIA driver is already present on the host. By providing clear documentation on the root cause, symptoms, and a safe cleanup procedure, this change enables operators to successfully deploy the operator on standard Linux distributions while avoiding common pitfalls like accidental removal of essential RDMA/DOCA components. Highlights
New Features🧠 You can now enable Memory (public preview) to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on Gemini (@gemini-code-assist) comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request updates the docs/gpu-vgpu.md documentation to add a comprehensive guide on host preparation for the GPU Operator's passthrough (default) variant. It explains the issue where pre-installed host drivers prevent the operator from binding GPUs to vfio-pci, details the symptoms, and provides a workaround to purge the host NVIDIA stack and blacklist the modules. It also updates the Host OS compatibility table to reflect these requirements. The review feedback suggests several improvements to the documentation's shell commands, including using command -v to check for nvidia-smi, explicitly purging nvidia-persistenced, and blacklisting nvidia_peermem on converged GPU + RDMA hosts.
| lsmod | grep -E '^(nvidia|nouveau)' | ||
|
|
||
| # Self-evident verification: prints "ok: gone" on success. | ||
| test -e /usr/bin/nvidia-smi && echo "STILL PRESENT — purge incomplete" || echo "ok: gone" |
There was a problem hiding this comment.
Using test -e /usr/bin/nvidia-smi only checks a specific hardcoded path. It is more robust to check if nvidia-smi is available anywhere in the system's PATH using command -v.
| test -e /usr/bin/nvidia-smi && echo "STILL PRESENT — purge incomplete" || echo "ok: gone" | |
| command -v nvidia-smi &>/dev/null && echo "STILL PRESENT — purge incomplete" || echo "ok: gone" |
| sudo apt purge \ | ||
| 'nvidia-driver-*' 'nvidia-dkms-*' 'nvidia-kernel-*' \ | ||
| 'nvidia-firmware-*' 'nvidia-utils-*' \ | ||
| 'nvidia-modprobe' 'nvidia-settings' 'nvidia-prime' \ | ||
| 'libnvidia-cfg1-*' 'libnvidia-compute-*' 'libnvidia-decode-*' \ | ||
| 'libnvidia-encode-*' 'libnvidia-extra-*' 'libnvidia-fbc1-*' \ | ||
| 'libnvidia-gl-*' \ | ||
| 'cuda-toolkit-*' 'cuda-drivers' 'cuda-drivers-*' |
There was a problem hiding this comment.
On systems where the NVIDIA host driver was installed, the nvidia-persistenced daemon might also be running and installed. It is recommended to explicitly purge nvidia-persistenced to ensure a completely clean host.
| sudo apt purge \ | |
| 'nvidia-driver-*' 'nvidia-dkms-*' 'nvidia-kernel-*' \ | |
| 'nvidia-firmware-*' 'nvidia-utils-*' \ | |
| 'nvidia-modprobe' 'nvidia-settings' 'nvidia-prime' \ | |
| 'libnvidia-cfg1-*' 'libnvidia-compute-*' 'libnvidia-decode-*' \ | |
| 'libnvidia-encode-*' 'libnvidia-extra-*' 'libnvidia-fbc1-*' \ | |
| 'libnvidia-gl-*' \ | |
| 'cuda-toolkit-*' 'cuda-drivers' 'cuda-drivers-*' | |
| sudo apt purge \ | |
| 'nvidia-driver-*' 'nvidia-dkms-*' 'nvidia-kernel-*' \ | |
| 'nvidia-firmware-*' 'nvidia-utils-*' \ | |
| 'nvidia-modprobe' 'nvidia-settings' 'nvidia-prime' 'nvidia-persistenced' \ | |
| 'libnvidia-cfg1-*' 'libnvidia-compute-*' 'libnvidia-decode-*' \ | |
| 'libnvidia-encode-*' 'libnvidia-extra-*' 'libnvidia-fbc1-*' \ | |
| 'libnvidia-gl-*' \ | |
| 'cuda-toolkit-*' 'cuda-drivers' 'cuda-drivers-*' |
| sudo tee /etc/modprobe.d/blacklist-nvidia.conf > /dev/null <<'EOF' | ||
| blacklist nouveau | ||
| blacklist nvidia | ||
| blacklist nvidia_drm | ||
| blacklist nvidia_modeset | ||
| blacklist nvidia_uvm | ||
| EOF |
There was a problem hiding this comment.
On converged GPU + RDMA hosts (which are explicitly mentioned in the guide), the nvidia_peermem module is often loaded for GPUDirect RDMA/Storage. It is safer to also blacklist nvidia_peermem to prevent any automatic loading of NVIDIA-related modules.
| sudo tee /etc/modprobe.d/blacklist-nvidia.conf > /dev/null <<'EOF' | |
| blacklist nouveau | |
| blacklist nvidia | |
| blacklist nvidia_drm | |
| blacklist nvidia_modeset | |
| blacklist nvidia_uvm | |
| EOF | |
| sudo tee /etc/modprobe.d/blacklist-nvidia.conf > /dev/null <<'EOF' | |
| blacklist nouveau | |
| blacklist nvidia | |
| blacklist nvidia_drm | |
| blacklist nvidia_modeset | |
| blacklist nvidia_uvm | |
| blacklist nvidia_peermem | |
| EOF |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
docs/gpu-vgpu.md (2)
331-331: 💤 Low valueConsider clarifying the usr-merge patch requirement.
The reference to "a
nvidia-driverpatch for usr-merge" may be unclear to operators when Ubuntu 26.04 releases. If the patch requirement is known, consider adding a brief inline explanation or a link to tracking documentation. If details aren't finalized, adding "(details pending)" would set expectations.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/gpu-vgpu.md` at line 331, Update the table cell that currently reads "a `nvidia-driver` patch for usr-merge" so it clarifies the usr-merge requirement: either add a short inline explanation of what the patch does and why it’s required (e.g., how usr-merge changes paths that the driver touches) or append a link to tracking documentation/issue and, if specifics aren’t finalized, add "(details pending)"; target the exact phrase "nvidia-driver patch for usr-merge" in docs/gpu-vgpu.md and keep the note concise.
33-36: ⚡ Quick winConsider breaking the overview into shorter sentences.
The current paragraph is a single 300+ word sentence with multiple nested parentheses covering detection flow, flag behavior, workarounds, and Talos exceptions. While technically accurate, it's difficult to scan during incident response. Breaking it into 3-4 focused sentences would improve clarity without changing content.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/gpu-vgpu.md` around lines 33 - 36, The paragraph is a single very long sentence that bundles multiple concepts and is hard to scan; break it into 3–4 focused sentences that separate: (1) the detection flow (mention the k8s-driver-manager init container run of uninstall_driver and the Go method (*DriverManager).isHostDriver that chroots to run nvidia-smi), (2) the observed behavior on detection (log Host driver detected: <ver>, node label nvidia.com/gpu.deploy.driver=pre-installed, and crashloop of nvidia-sandbox-validator due to device not bound to vfio-pci), and (3) the force/mitigation notes (that FORCE_REINSTALL/--force-reinstall does not bypass isHostDriver, the only current workaround is removing the host NVIDIA stack, and the upcoming container variant and Talos exception). Keep wording concise, preserve all technical symbols (uninstall_driver, (*DriverManager).isHostDriver, nvidia-smi, FORCE_REINSTALL/--force-reinstall, vfio-manager, nvidia-sandbox-validator) and the references/links as-is.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@docs/gpu-vgpu.md`:
- Line 331: Update the table cell that currently reads "a `nvidia-driver` patch
for usr-merge" so it clarifies the usr-merge requirement: either add a short
inline explanation of what the patch does and why it’s required (e.g., how
usr-merge changes paths that the driver touches) or append a link to tracking
documentation/issue and, if specifics aren’t finalized, add "(details pending)";
target the exact phrase "nvidia-driver patch for usr-merge" in docs/gpu-vgpu.md
and keep the note concise.
- Around line 33-36: The paragraph is a single very long sentence that bundles
multiple concepts and is hard to scan; break it into 3–4 focused sentences that
separate: (1) the detection flow (mention the k8s-driver-manager init container
run of uninstall_driver and the Go method (*DriverManager).isHostDriver that
chroots to run nvidia-smi), (2) the observed behavior on detection (log Host
driver detected: <ver>, node label nvidia.com/gpu.deploy.driver=pre-installed,
and crashloop of nvidia-sandbox-validator due to device not bound to vfio-pci),
and (3) the force/mitigation notes (that FORCE_REINSTALL/--force-reinstall does
not bypass isHostDriver, the only current workaround is removing the host NVIDIA
stack, and the upcoming container variant and Talos exception). Keep wording
concise, preserve all technical symbols (uninstall_driver,
(*DriverManager).isHostDriver, nvidia-smi, FORCE_REINSTALL/--force-reinstall,
vfio-manager, nvidia-sandbox-validator) and the references/links as-is.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 148737e0-d761-4fd4-ba9a-0317dd9041fb
📒 Files selected for processing (1)
docs/gpu-vgpu.md
6c00af5 to
f30f047
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
docs/gpu-vgpu.md (2)
61-76: ⚡ Quick winConsider clarifying the comment placement.
The comment block (lines 62-67) describes "load-bearing kernel pieces" and "userspace driver pieces", but it's visually separated from the actual
apt purgecommand that starts on line 68. Operators skimming the code might miss the connection.♻️ Suggested adjustment
Move the comment immediately before the
sudo apt purgecommand to improve clarity:-# nvidia-dkms-* and nvidia-kernel-* are the load-bearing kernel -# pieces — without them DKMS rebuilds nvidia.ko on the next reboot -# and the blacklist below is bypassed by any explicit modprobe. -# libnvidia-* is intentionally split into the userspace driver -# pieces only — a bare 'libnvidia-*' glob also matches DOCA-OFED -# shipped libnvidia packages on converged GPU + RDMA hosts. sudo apt purge \ + # nvidia-dkms-* and nvidia-kernel-* are the load-bearing kernel + # pieces — without them DKMS rebuilds nvidia.ko on the next reboot + # and the blacklist below is bypassed by any explicit modprobe. 'nvidia-driver-*' 'nvidia-dkms-*' 'nvidia-kernel-*' \ 'nvidia-firmware-*' 'nvidia-utils-*' \ 'nvidia-modprobe' 'nvidia-settings' 'nvidia-prime' \ + # libnvidia-* is intentionally split into the userspace driver + # pieces only — a bare 'libnvidia-*' glob also matches DOCA-OFED + # shipped libnvidia packages on converged GPU + RDMA hosts. 'libnvidia-cfg1-*' 'libnvidia-compute-*' 'libnvidia-decode-*' \ 'libnvidia-encode-*' 'libnvidia-extra-*' 'libnvidia-fbc1-*' \ 'libnvidia-gl-*' \Alternatively, keep the current structure but add a blank line to visually separate the explanation from the workaround introduction.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/gpu-vgpu.md` around lines 61 - 76, The explanatory comment about "nvidia-dkms-*" and "libnvidia-*" should be visually associated with the apt purge command; move the comment block so it appears immediately above the sudo apt purge line (the bash block that begins with sudo apt purge 'nvidia-driver-*' ...) or, at minimum, add a blank line between the explanation and the following paragraphs so readers immediately see the relation between the comment and the apt purge command.
35-35: ⚡ Quick winBreak this paragraph into multiple focused sections for readability.
This single paragraph exceeds 1000 characters and covers multiple distinct concepts: the upstream
isHostDriverbehavior, theFORCE_REINSTALLlimitation, the upcomingcontainervariant, Talos compatibility, and cross-references to other docs. Operators diagnosing a live issue will struggle to extract the key points quickly.♻️ Suggested restructure
Consider breaking this into logical sections:
-The `default` (passthrough) variant assumes the GPU is **owned by the host kernel's `vfio-pci` driver and nothing else**. The chart's `vfio-manager` DaemonSet runs the upstream NVIDIA `k8s-driver-manager` init container with the `uninstall_driver` subcommand; that path calls the Go method `(*DriverManager).isHostDriver`, which runs `chroot /host nvidia-smi --query-gpu=driver_version --format=csv,noheader` and treats any non-empty stdout as "host driver present" (file-existence is not pre-tested — if `nvidia-smi` is missing, the chroot exec errors and `isHostDriver` returns false, which is the intended path on a clean host: the operator then proceeds with the uninstall flow and `vfio-manager` binds `vfio-pci` as designed). On a positive detection the binary logs `Host driver detected: <ver>`, labels the node `nvidia.com/gpu.deploy.driver=pre-installed`, exits, and `nvidia-sandbox-validator` then crashloops with `device not bound to 'vfio-pci'`. `k8s-driver-manager` v0.10.0 exposes a `FORCE_REINSTALL` / `--force-reinstall` env+flag pair (visible in the binary's strings table), but it gates a later "same-config already loaded" branch inside `uninstallDriver`, not the `isHostDriver` short-circuit at the top — operators who set it and expect a bypass will report a false bug. There is currently no opt-out for the `isHostDriver` guard itself. The only mitigation available on `main` today is to remove the host NVIDIA stack before enabling the variant (the "Clean-host workaround" section below). An upcoming `container` variant of `cozystack.gpu-operator` is tracked in [`#2766`](https://github.com/cozystack/cozystack/pull/2766) — once merged it will give operators a no-purge path that keeps the host driver and exposes GPUs to pods rather than VMs — but it is not part of the current package and is **not** a usable workaround until it lands. Talos is unaffected because the Talos image ships only the `vfio-pci` extension; this section applies to Linux distributions where you installed the host driver yourself (typically Ubuntu / Debian / RHEL with `apt install nvidia-driver-*` or equivalent). See also [`packages/system/gpu-operator/examples/README.md`](../packages/system/gpu-operator/examples/README.md) for the native-pod-workload reference flow that predates the `container` variant. +The `default` (passthrough) variant assumes the GPU is **owned by the host kernel's `vfio-pci` driver and nothing else**. + +**How the operator detects a pre-installed host driver:** +The chart's `vfio-manager` DaemonSet runs the upstream NVIDIA `k8s-driver-manager` init container with the `uninstall_driver` subcommand. That path calls the Go method `(*DriverManager).isHostDriver`, which runs `chroot /host nvidia-smi --query-gpu=driver_version --format=csv,noheader` and treats any non-empty stdout as "host driver present". File existence is not pre-tested — if `nvidia-smi` is missing, the chroot exec errors and `isHostDriver` returns false, which is the intended path on a clean host. On a positive detection the binary logs `Host driver detected: <ver>`, labels the node `nvidia.com/gpu.deploy.driver=pre-installed`, exits, and `nvidia-sandbox-validator` then crashloops with `device not bound to 'vfio-pci'`. + +**`FORCE_REINSTALL` does not bypass this check:** +`k8s-driver-manager` v0.10.0 exposes a `FORCE_REINSTALL` / `--force-reinstall` env+flag pair (visible in the binary's strings table), but it gates a later "same-config already loaded" branch inside `uninstallDriver`, not the `isHostDriver` short-circuit at the top — operators who set it and expect a bypass will report a false bug. There is currently no opt-out for the `isHostDriver` guard itself. + +**Current mitigation:** +The only workaround available on `main` today is to remove the host NVIDIA stack before enabling the variant (see "Clean-host workaround" below). + +**Future alternatives (not yet available):** +An upcoming `container` variant of `cozystack.gpu-operator` is tracked in [`#2766`](https://github.com/cozystack/cozystack/pull/2766) — once merged it will give operators a no-purge path that keeps the host driver and exposes GPUs to pods rather than VMs — but it is not part of the current package and is **not** a usable workaround until it lands. + +**Scope:** +Talos is unaffected because the Talos image ships only the `vfio-pci` extension. This section applies to Linux distributions where you installed the host driver yourself (typically Ubuntu / Debian / RHEL with `apt install nvidia-driver-*` or equivalent). See also [`packages/system/gpu-operator/examples/README.md`](../packages/system/gpu-operator/examples/README.md) for the native-pod-workload reference flow that predates the `container` variant.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/gpu-vgpu.md` at line 35, The paragraph is too long and conflates multiple topics; split it into focused sections such as: (1) "isHostDriver behavior" explaining the chroot nvidia-smi check and that (*DriverManager).isHostDriver short-circuits on any stdout (mention nvidia-smi and chroot /host), (2) "FORCE_REINSTALL limitation" noting FORCE_REINSTALL / --force-reinstall only affects uninstallDriver's later branch and does not bypass isHostDriver, (3) "Current mitigation" describing the Clean-host workaround and vfio-manager behavior (mention vfio-manager, nvidia-sandbox-validator crashloop and vfio-pci bind), and (4) "Future option and scope" covering the upcoming container variant (reference cozystack PR `#2766`) and Talos exemption; move the cross-reference to packages/system/gpu-operator/examples/README.md into the "Future option" or "Native-pod-workload" note. Keep each section to 1–3 sentences and use the unique symbols isHostDriver, uninstallDriver, FORCE_REINSTALL, nvidia-smi, vfio-manager, nvidia-sandbox-validator, vfio-pci, and PR `#2766` to anchor explanations.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@docs/gpu-vgpu.md`:
- Around line 61-76: The explanatory comment about "nvidia-dkms-*" and
"libnvidia-*" should be visually associated with the apt purge command; move the
comment block so it appears immediately above the sudo apt purge line (the bash
block that begins with sudo apt purge 'nvidia-driver-*' ...) or, at minimum, add
a blank line between the explanation and the following paragraphs so readers
immediately see the relation between the comment and the apt purge command.
- Line 35: The paragraph is too long and conflates multiple topics; split it
into focused sections such as: (1) "isHostDriver behavior" explaining the chroot
nvidia-smi check and that (*DriverManager).isHostDriver short-circuits on any
stdout (mention nvidia-smi and chroot /host), (2) "FORCE_REINSTALL limitation"
noting FORCE_REINSTALL / --force-reinstall only affects uninstallDriver's later
branch and does not bypass isHostDriver, (3) "Current mitigation" describing the
Clean-host workaround and vfio-manager behavior (mention vfio-manager,
nvidia-sandbox-validator crashloop and vfio-pci bind), and (4) "Future option
and scope" covering the upcoming container variant (reference cozystack PR
`#2766`) and Talos exemption; move the cross-reference to
packages/system/gpu-operator/examples/README.md into the "Future option" or
"Native-pod-workload" note. Keep each section to 1–3 sentences and use the
unique symbols isHostDriver, uninstallDriver, FORCE_REINSTALL, nvidia-smi,
vfio-manager, nvidia-sandbox-validator, vfio-pci, and PR `#2766` to anchor
explanations.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: dde2945f-8864-41b6-bb95-8a6a0524246f
📒 Files selected for processing (1)
docs/gpu-vgpu.md
f30f047 to
e9f831c
Compare
myasnikovdaniil
left a comment
There was a problem hiding this comment.
Docs-only change documenting a real, previously-undocumented failure mode of the gpu-operator default (vfio-pci passthrough) variant: when the host already has an apt-installed NVIDIA driver, the upstream k8s-driver-manager init container in the nvidia-vfio-manager DaemonSet detects it via chroot /host nvidia-smi, labels the node nvidia.com/gpu.deploy.driver=pre-installed, and bails — so vfio-pci never binds and nvidia-sandbox-validator crashloops. The PR adds symptom recognition, a clean-host purge workaround, an OS-table update, and a Known-limitation section.
I verified the core technical claims against the upstream NVIDIA/k8s-driver-manager v0.10.0 Go source (cmd/driver-manager/main.go) and the vendored chart: the isHostDriver chroot-nvidia-smi check, the non-empty-stdout logic, exec-error→false on clean hosts, the pre-installed label, the preflight_check entrypoint, and — notably — that FORCE_REINSTALL only gates shouldSkipUninstall() and not the isHostDriver short-circuit. All accurate. Version pin matches (charts/gpu-operator/values.yaml → k8s-driver-manager:v0.10.0, chart v26.3.1), the cozy-gpu-operator namespace is correct, and all four cross-refs are live (#2763 open, #2766 open/unmerged, upstream #191 open, examples/README.md exists). Unusually well-researched for a docs PR.
Three substantive issues remain, all verifiable from the same upstream source the doc cites: (1) "will not retry on their own" is wrong — the init container exits non-zero and crash-loops; (2) the workaround leaves the stale pre-installed label behind, which the doc itself uses as diagnostic evidence; (3) the headline apt purge can abort with zero effect on a host without the CUDA repo. Details inline. All are small edits — happy to re-approve once addressed.
| sudo reboot | ||
| ``` | ||
|
|
||
| After the reboot, confirm the host is clean and kick the stuck DaemonSet pods so they re-create against the cleaned state — they have already labeled the node and will not retry on their own: |
There was a problem hiding this comment.
Factual error — verified against upstream v0.10.0 source. uninstallDriver() returns fmt.Errorf("driver is pre-installed on host") right after the labeling step (cmd/driver-manager/main.go#L272-L281), i.e. the init container exits non-zero. kubelet keeps restarting it under crash-loop backoff, so it does retry on its own — and the reboot in this workaround recreates the pod sandbox and re-runs init containers anyway. The kubectl delete is still worth keeping (it skips the up-to-5-minute backoff wait), but the stated justification is wrong.
| After the reboot, confirm the host is clean and kick the stuck DaemonSet pods so they re-create against the cleaned state — they have already labeled the node and will not retry on their own: | |
| After the reboot, confirm the host is clean. The init container exits non-zero after detecting a host driver, so both DaemonSet pods sit in `Init:CrashLoopBackOff`; they *will* retry on their own, but deleting them skips the up-to-5-minute backoff window: |
| kubectl -n cozy-gpu-operator get pods -o name \ | ||
| | grep -E '^pod/(nvidia-vfio-manager|nvidia-sandbox-validator)-' \ | ||
| | xargs -r kubectl -n cozy-gpu-operator delete | ||
| ``` |
There was a problem hiding this comment.
Stale label left behind — verified against upstream source. disableContainerizedDriver() sets nvidia.com/gpu.deploy.driver=pre-installed, but the success path rescheduleGPUOperatorComponents() only restores the operand labels (validator / toolkit / device-plugin / gfd / dcgm / mig / nvsm / sandbox-validator / sandbox-device-plugin / vgpu-device-manager) — nvidia.com/gpu.deploy.driver is not in that list and is never reset by the binary. So after a successful recovery the node keeps pre-installed indefinitely. That's the exact label the Symptom section tells operators to use as evidence, and it would disable the containerized-driver DaemonSet if the node ever switches to the container workload (directly relevant once #2766 lands). Suggest adding a cleanup step:
# The init container set this label and nothing resets it on the success
# path; clear it so it does not mislead future debugging.
kubectl label node <NODE> nvidia.com/gpu.deploy.driver-(Worth a quick runtime check that the gpu-operator controller doesn't reconcile it back/away on its own — see the verification note in the review.)
| # libnvidia-* is intentionally split into the userspace driver | ||
| # pieces only — a bare 'libnvidia-*' glob also matches DOCA-OFED | ||
| # shipped libnvidia packages on converged GPU + RDMA hosts. | ||
| sudo apt purge \ |
There was a problem hiding this comment.
The headline remediation can abort with zero effect on a common host shape. apt/apt-get treat * args as regexes matched against the whole package cache, and a pattern (or a plain name like cuda-drivers) that matches nothing in the cache is a fatal E: Unable to locate package — the entire transaction aborts and nothing is purged. cuda-toolkit-* / cuda-drivers / cuda-drivers-* only exist in the cache when NVIDIA's CUDA repo is configured; on a stock Ubuntu 24.04 host with just nvidia-driver-580-open from the Ubuntu archive (the very host this doc was verified against), this command fails outright. Note also these are substring regexes, not shell globs — broader than they look, which interacts with the DOCA caution above.
Suggest either a dry-run-first instruction (sudo apt-get -s purge … | grep -E '^(Purg|Remv)', review, drop patterns apt can't locate), or build the list from installed packages, which sidesteps both the zero-match abort and the regex over-match:
dpkg-query -W -f '${Package}\n' 'nvidia-*' 'libnvidia-*' 'cuda-*' 2>/dev/null(dpkg-query patterns are true globs over installed packages.)
| blacklist nvidia_uvm | ||
| EOF | ||
|
|
||
| sudo update-initramfs -u |
There was a problem hiding this comment.
update-initramfs -u only rebuilds the running kernel's initramfs. If the node boots a different (e.g. just-upgraded) kernel after this reboot, its initramfs won't carry the blacklist and the host can re-claim the GPU. Use -k all:
| sudo update-initramfs -u | |
| sudo update-initramfs -u -k all |
| lsmod | grep -E '^(nvidia|nouveau)' | ||
|
|
||
| # Self-evident verification: prints "ok: gone" on success. | ||
| test -e /usr/bin/nvidia-smi && echo "STILL PRESENT — purge incomplete" || echo "ok: gone" |
There was a problem hiding this comment.
isHostDriver runs chroot /host nvidia-smi resolved via PATH inside the chroot, not pinned to /usr/bin. So test -e /usr/bin/nvidia-smi misses /usr/local/bin/nvidia-smi (typical leftover of a .run-installer or CUDA-toolkit install), and the apt purge above doesn't cover .run-installed drivers at all. Match what the operator actually does:
| test -e /usr/bin/nvidia-smi && echo "STILL PRESENT — purge incomplete" || echo "ok: gone" | |
| command -v nvidia-smi && echo "STILL PRESENT — purge incomplete" || echo "ok: gone" |
Also worth adding dkms status | grep -i nvidia (a leftover DKMS module rebuilds nvidia.ko on the next kernel update and the blacklist is bypassed by an explicit modprobe), and one sentence: if the driver was installed with NVIDIA's .run installer rather than apt, run nvidia-uninstall instead of the purge.
|
|
||
| The `default` (passthrough) variant assumes the GPU is **owned by the host kernel's `vfio-pci` driver and nothing else**. The chart's `vfio-manager` DaemonSet runs the upstream NVIDIA `k8s-driver-manager` init container with the `uninstall_driver` subcommand; that path calls the Go method `(*DriverManager).isHostDriver`, which runs `chroot /host nvidia-smi --query-gpu=driver_version --format=csv,noheader` and treats any non-empty stdout as "host driver present" (file-existence is not pre-tested — if `nvidia-smi` is missing, the chroot exec errors and `isHostDriver` returns false, which is the intended path on a clean host: the operator then proceeds with the uninstall flow and `vfio-manager` binds `vfio-pci` as designed). On a positive detection the binary logs `Host driver detected: <ver>`, labels the node `nvidia.com/gpu.deploy.driver=pre-installed`, exits, and `nvidia-sandbox-validator` then crashloops with `device not bound to 'vfio-pci'`. `k8s-driver-manager` v0.10.0 exposes a `FORCE_REINSTALL` / `--force-reinstall` env+flag pair (visible in the binary's strings table), but it gates a later "same-config already loaded" branch inside `uninstallDriver`, not the `isHostDriver` short-circuit at the top — operators who set it and expect a bypass will report a false bug. There is currently no opt-out for the `isHostDriver` guard itself. The only mitigation available on `main` today is to remove the host NVIDIA stack before enabling the variant (the "Clean-host workaround" section below). An upcoming `container` variant of `cozystack.gpu-operator` is tracked in [#2766](https://github.com/cozystack/cozystack/pull/2766) — once merged it will give operators a no-purge path that keeps the host driver and exposes GPUs to pods rather than VMs — but it is not part of the current package and is **not** a usable workaround until it lands. Talos is unaffected because the Talos image ships only the `vfio-pci` extension; this section applies to Linux distributions where you installed the host driver yourself (typically Ubuntu / Debian / RHEL with `apt install nvidia-driver-*` or equivalent). See also [`packages/system/gpu-operator/examples/README.md`](../packages/system/gpu-operator/examples/README.md) for the native-pod-workload reference flow that predates the `container` variant. | ||
|
|
||
| ### Symptom |
There was a problem hiding this comment.
Minor: the Symptom section quotes logs but not the first thing an operator sees in kubectl get pods — nvidia-vfio-manager in Init:Error / Init:CrashLoopBackOff. Per the upstream code path the init container exits non-zero (it doesn't complete quietly), so adding one line about the pod state would help, since that's what people grep for first.
e9f831c to
10044eb
Compare
|
Thanks — all three blocking items plus the inline corrections are addressed in the latest push. I re-verified every claim against the v0.10.0 source you cited.
Inline corrections also applied:
Heads-up for whoever merges second: this and #2766 both edit the OS-support table in |
Andrei Kvapil (kvaps)
left a comment
There was a problem hiding this comment.
LGTM. Docs-only, and every correction from the earlier review is in: init-container retry behavior fixed, stale nvidia.com/gpu.deploy.driver label cleanup added, the apt purge 'cuda-*' footgun replaced with a dpkg-query -W approach (no aborted transaction on hosts without the CUDA repo), update-initramfs -u -k all, command -v nvidia-smi, and the Init:Error symptom line. Technically thorough. CI green.
Note: shares docs/gpu-vgpu.md with #2766/#2768 — expect a docs rebase on whichever merges last.
…site The vfio-manager DaemonSet's k8s-driver-manager init container exits non-zero and labels the node pre-installed whenever it finds a host NVIDIA driver, leaving vfio-pci unbound and the sandbox validator in CrashLoopBackOff. Upstream offers no override knob, so the only working path today is a clean host. Document the symptom (including the Init:CrashLoopBackOff pod state), a build-from-installed purge plus module-blacklist workaround (with the autoremove-vs-DOCA warning and the apt zero-match-abort caveat), the post-reboot verification, and clearing the stale pre-installed label that the operator's success path never resets. Assisted-By: Claude <[email protected]> Signed-off-by: Aleksei Sviridkin <[email protected]>
b2d3842 to
cd36b0d
Compare
What this PR does
Documents an undocumented prerequisite of the
cozystack.gpu-operatordefault(passthrough) variant: it assumes the GPU is not already claimed by a host-installed NVIDIA driver. The chart'svfio-managerDaemonSet runs the upstream NVIDIAk8s-driver-managerinit container, which is hard-coded to exit non-zero and label the nodenvidia.com/gpu.deploy.driver=pre-installedthe momentchroot /host nvidia-smireturns a driver version (aPATHlookup, not a fixed file path). There is no env-var override ink8s-driver-managerv0.10.0, so today the only working path is a clean host.The new "Passthrough variant: host preparation" section in
docs/gpu-vgpu.mdcovers:lspcievidence) so operators can recognise it without spelunking.dpkg-query, so a zero-match pattern can't silently abort the wholeapttransaction the way a hardcoded'cuda-*'pattern does on a host without the CUDA repo),/etc/modprobe.d/blacklist-nvidia.conf(incl.nvidia_peermem),update-initramfs -k all, reboot. Warns thatautoremoveis dangerous on converged GPU + NVIDIA DOCA / Mellanox hosts because of the sharednvidia-prefix.nvidiamodules loaded, nonvidia-smionPATH, no leftover NVIDIA DKMS module) and a label-cleanup step — the DaemonSet pods crash-loop and retry on their own, but the operator's success path never resets thepre-installedlabel, so it must be cleared manually.containervariant of the package).Linked to #2763.
Release note
Summary by CodeRabbit