Repository navigation
feat(kubernetes): enable GPU passthrough out-of-the-box (NvLinkDisable + gpu=on) - #2780
Conversation
cozystack passes individual GPUs into tenant worker VMs, but HAMi's hami-device-plugin DaemonSet carries nodeSelector gpu=on and stays at DESIRED=0 until the GPU nodes are labeled, so nvidia.com/gpu is never advertised. Append --node-labels=gpu=on to the kubelet args of every node-group that declares gpus, so GPU nodes register with the label out of the box. Non-GPU node-groups are unaffected. Co-Authored-By: Claude <[email protected]> Signed-off-by: Andrei Kvapil <[email protected]>
When a single SXM GPU (e.g. H200) is passed through into a tenant worker VM without the NVSwitches, the nvidia driver waits forever for an NVLink fabric that can never come up (Fabric State: In Progress). Every CUDA call then fails with 'system not yet initialized' and the nvidia-operator-validator crashloops. Loading the kernel module with NVreg_NvLinkDisable=1 flips Fabric State to N/A so CUDA initializes — correct for per-GPU passthrough, which has no in-VM NVLink/NVSwitch peering anyway. The upstream gpu-operator chart only references an existing ConfigMap by name via driver.kernelModuleConfig.name and never creates it. Render that ConfigMap from the gpu-operator wrapper chart (opt-in via kernelModuleConfig.create) so it lands in the tenant cozy-gpu-operator namespace alongside the operator, and wire the tenant HelmRelease defaults to enable it and point the driver at it. Remains overridable via addons.gpuOperator.valuesOverride. Co-Authored-By: Claude <[email protected]> Signed-off-by: Andrei Kvapil <[email protected]>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR adds conditional GPU node labeling during kubeadm join and introduces optional GPU operator kernel-module configuration via Helm values and a conditional ConfigMap template, plus tests for both behaviors. ChangesGPU Operator Features
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 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 |
github-actions
Bot
added
area/kubernetes
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 streamlines GPU passthrough for tenant Kubernetes clusters by automating necessary configuration defaults. It addresses common initialization issues caused by missing NVSwitches in virtualized environments and ensures that device plugins are correctly triggered through automatic node labeling. These changes reduce the manual overhead for operators while maintaining flexibility through existing override mechanisms. 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
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces support for disabling NVLink on GPU passthrough configurations in tenant Kubernetes clusters. It adds a wrapper ConfigMap in the gpu-operator system package to apply the NVreg_NvLinkDisable=1 kernel module parameter, and updates the kubernetes app package to label GPU node-groups with gpu=on and default the GPU operator configuration. A review comment highlights a potential mismatch if a user overrides the driver's kernel module config name without overriding the wrapper's config name, suggesting a dynamic default in the Helm template to prevent this issue.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| {{- if .Values.kernelModuleConfig.create }} | ||
| apiVersion: v1 | ||
| kind: ConfigMap | ||
| metadata: | ||
| name: {{ .Values.kernelModuleConfig.name }} |
There was a problem hiding this comment.
There is a potential mismatch between the ConfigMap name created by this template and the name expected by the GPU operator driver if a user overrides gpu-operator.driver.kernelModuleConfig.name but does not also override kernelModuleConfig.name.
To prevent this, we can dynamically default kernelModuleConfig.name to the value of gpu-operator.driver.kernelModuleConfig.name if it is set.
{{- if .Values.kernelModuleConfig.create }}
{{- $gpuOperator := index .Values "gpu-operator" | default dict -}}
{{- $driver := index $gpuOperator "driver" | default dict -}}
{{- $kmc := index $driver "kernelModuleConfig" | default dict -}}
{{- $name := .Values.kernelModuleConfig.name | default $kmc.name | default "nvidia-kernel-module-params" -}}
apiVersion: v1
kind: ConfigMap
metadata:
name: {{ $name }}![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/system/gpu-operator/templates/kernel-module-config.yaml (1)
1-13: ⚡ Quick winAdd a wrapper-chart unittest for this template.
The provided suite only checks the HelmRelease values, so regressions in the actual ConfigMap rendering (
create,name, or thenvidia.confpayload) would slip through until deployment. A small test inpackages/system/gpu-operator/tests/would lock down this new chart contract.🤖 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 `@packages/system/gpu-operator/templates/kernel-module-config.yaml` around lines 1 - 13, Add a wrapper-chart unit test under packages/system/gpu-operator/tests that renders the kernel-module-config.yaml template and asserts the ConfigMap is created only when .Values.kernelModuleConfig.create is true, that metadata.name matches .Values.kernelModuleConfig.name and namespace is the Release.Namespace, that labels include app.kubernetes.io/managed-by and app.kubernetes.io/name: gpu-operator, and that the data.nvidia.conf payload contains the rendered .Values.kernelModuleConfig.content; test both create=true and create=false cases and one case verifying content indentation/line breaks to catch regression in nvidia.conf rendering.
🤖 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.
Inline comments:
In `@packages/apps/kubernetes/templates/helmreleases/gpu-operator.yaml`:
- Around line 19-24: The two ConfigMap name values diverge because the wrapper
uses kernelModuleConfig.name while the upstream chart uses
gpu-operator.driver.kernelModuleConfig.name and mergeOverwrite can override one
side independently; fix by normalizing to a single source of truth or explicitly
propagating the chosen name into both keys after values are merged — for
example, decide which key is authoritative (e.g.,
gpu-operator.driver.kernelModuleConfig.name) and in the wrapper's values merge
step or templates copy that effective value into kernelModuleConfig.name so both
kernelModuleConfig.name and gpu-operator.driver.kernelModuleConfig.name always
contain the same string.
---
Nitpick comments:
In `@packages/system/gpu-operator/templates/kernel-module-config.yaml`:
- Around line 1-13: Add a wrapper-chart unit test under
packages/system/gpu-operator/tests that renders the kernel-module-config.yaml
template and asserts the ConfigMap is created only when
.Values.kernelModuleConfig.create is true, that metadata.name matches
.Values.kernelModuleConfig.name and namespace is the Release.Namespace, that
labels include app.kubernetes.io/managed-by and app.kubernetes.io/name:
gpu-operator, and that the data.nvidia.conf payload contains the rendered
.Values.kernelModuleConfig.content; test both create=true and create=false cases
and one case verifying content indentation/line breaks to catch regression in
nvidia.conf rendering.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3cc066ec-3ddd-43fe-8e1e-4c1cadfd53d0
📒 Files selected for processing (6)
packages/apps/kubernetes/templates/cluster.yamlpackages/apps/kubernetes/templates/helmreleases/gpu-operator.yamlpackages/apps/kubernetes/tests/gpu_node_labels_test.yamlpackages/apps/kubernetes/tests/gpu_operator_hami_test.yamlpackages/system/gpu-operator/templates/kernel-module-config.yamlpackages/system/gpu-operator/values.yaml
| kernelModuleConfig: | ||
| create: true | ||
| gpu-operator: | ||
| driver: | ||
| kernelModuleConfig: | ||
| name: nvidia-kernel-module-params |
There was a problem hiding this comment.
Keep the two kernel-module ConfigMap names in sync.
The wrapper chart reads kernelModuleConfig.name, but the upstream chart reads gpu-operator.driver.kernelModuleConfig.name. Because these are separate trees, mergeOverwrite lets a user override one side without updating the other. The new override test makes this reachable: the driver can look for my-custom-kmc while the wrapper still renders nvidia-kernel-module-params, so the override breaks at runtime.
Please normalize this to a single source of truth, or copy the effective name into both paths after merging.
🤖 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 `@packages/apps/kubernetes/templates/helmreleases/gpu-operator.yaml` around
lines 19 - 24, The two ConfigMap name values diverge because the wrapper uses
kernelModuleConfig.name while the upstream chart uses
gpu-operator.driver.kernelModuleConfig.name and mergeOverwrite can override one
side independently; fix by normalizing to a single source of truth or explicitly
propagating the chosen name into both keys after values are merged — for
example, decide which key is authoritative (e.g.,
gpu-operator.driver.kernelModuleConfig.name) and in the wrapper's values merge
step or templates copy that effective value into kernelModuleConfig.name so both
kernelModuleConfig.name and gpu-operator.driver.kernelModuleConfig.name always
contain the same string.
…rence The wrapper chart named the kernel-module ConfigMap from its own kernelModuleConfig.name key, while the NVIDIA driver mounts the ConfigMap referenced by gpu-operator.driver.kernelModuleConfig.name. The two keys defaulted to the same value but were never linked, so overriding only the driver reference left the driver pointing at a ConfigMap that is never rendered and the NVreg_NvLinkDisable=1 parameters were silently dropped. Derive the ConfigMap name from the driver's effective reference, falling back to the wrapper key when unset, so the rendered ConfigMap and the driver reference can no longer diverge. Add helm-unittest coverage for the rendered name, namespace, data key, and the create=false no-op. Assisted-By: Claude <[email protected]> Signed-off-by: Aleksei Sviridkin <[email protected]>
Aleksei Sviridkin (lexfrei)
left a comment
Aleksei Sviridkin (lexfrei)
left a comment
There was a problem hiding this comment.
LGTM. The core mechanism is correct: NVreg_NvLinkDisable=1 is the right setting for single-SXM-GPU passthrough (no NVSwitch in the VM), the nvidia.conf format matches what the driver reads, the ConfigMap lands in the operator namespace, and gpu=on matches HAMi's device-plugin nodeSelector default.
I pushed one fix on top. The kernel-module ConfigMap was named from kernelModuleConfig.name while the driver references gpu-operator.driver.kernelModuleConfig.name — two unlinked keys. Overriding only the driver reference (which the new test exercises) left the driver pointing at a ConfigMap that is never rendered, silently dropping the parameters. The ConfigMap name now derives from the driver reference, with helm-unittest covering the rendered name, namespace, data key, and the create=false no-op.
Non-blocking, fine to defer:
- packages/system/hami/README.md still lists gpu=on as a manual prerequisite; it is now applied automatically for node-groups that declare GPUs.
- NVreg_NvLinkDisable=1 is now the default for every tenant GPU cluster — correct for single-GPU passthrough, overridable via valuesOverride for multi-GPU NVLink topologies.
2 participants
What this PR does
Makes GPU passthrough work out-of-the-box for tenant Kubernetes clusters (the
kubernetesapp), so operators no longer have to hand-fix it. Two defaults:NVreg_NvLinkDisable=1.gpu=onautomatically.Root cause (validated empirically on a live cluster)
cozystack passes through individual SXM GPUs (e.g. H200) into tenant worker VMs without the NVSwitches. Such a GPU's driver then waits for an NVLink fabric that can never come up — there is no NVSwitch and fabricmanager has nothing to manage — so
nvidia-smireportsFabric State: In Progressindefinitely. As a result every CUDA call fails withsystem not yet initialized,nvidia-operator-validator(cuda-validation) crashloops, and no GPUs are advertised.Loading the kernel module with
NVreg_NvLinkDisable=1flipsFabric StatetoN/A, CUDA initializes, and the validators pass. This is the correct setting for per-GPU passthrough because there is no in-VM NVLink/NVSwitch peering — the GPU is alone in the VM, so there is nothing to disable that the workload needs.The second issue: HAMi's
hami-device-pluginDaemonSet hasnodeSelector: gpu=onand stays atDESIRED=0because GPU worker nodes lack that label, sonvidia.com/gpuis never advertised. Labeling the GPU nodesgpu=onlets the device plugin schedule and advertise the GPUs.How it works
gpu=onlabel: node-groups inpackages/apps/kubernetes/templates/cluster.yamlthat declaregpusget--node-labels=gpu=onappended to their kubelet args (in theKubeadmConfigTemplatejoinConfiguration). Non-GPU node-groups are untouched.NVreg_NvLinkDisable=1: the upstream gpu-operator chart only references an existing ConfigMap by name (driver.kernelModuleConfig.name) and never creates it. The gpu-operator wrapper chart (packages/system/gpu-operator) now renders that ConfigMap (opt-in viakernelModuleConfig.create) so it deploys into the tenantcozy-gpu-operatornamespace alongside the operator. The tenant gpu-operator HelmRelease defaults enable it and pointdriver.kernelModuleConfig.nameat it.Both defaults remain overridable: a user setting
addons.gpuOperator.valuesOverridestill wins via the existingmergeOverwrite(defaults, overrides)in the HelmRelease template.Screenshots
Not applicable — no UI changes.
Release note
Summary by CodeRabbit
New Features
Tests