Mount host /lib/modules for precompiled drivers on SUSE - #2784
Open
abrarshivani wants to merge 1 commit into
Open
Mount host /lib/modules for precompiled drivers on SUSE#2784abrarshivani wants to merge 1 commit into
abrarshivani wants to merge 1 commit into
Conversation
abrarshivani
requested review from
cdesiniotis,
karthikvetrivel,
rahulait,
rajathagasthya,
shivamerla and
tariq1890
as code owners
August 21, 2026 23:29
tariq1890
reviewed
Aug 21, 2026
Contributor
|
Thanks for working on this @abrarshivani ! This will need to be backported to |
getDriverAdditionalConfigs placed the lib-modules volume and mount inside the "if !cr.Spec.UsePrecompiledDrivers()" branch, guarded by a check for UsePrecompiledDrivers. That inner condition can never hold there, so an NVIDIADriver with usePrecompiled: true on sles or sl-micro rendered a driver pod without /run/host/lib/modules. Move the mount out to its own block after the non-precompiled branch. The comment above it goes as well, since the guard already says precompiled and SUSE and the volume literals already say host module tree. The equivalent code in the ClusterPolicy path sits before the early return for precompiled drivers and works as intended, so only the NVIDIADriver path was affected. That is also why this went unnoticed: the tests added with the mount only covered the ClusterPolicy path. Add a table-driven test that renders the DaemonSet and asserts the volume on the pod spec and the mount on nvidia-driver-ctr. Going through the render catches a template change that stopped threading AdditionalConfigs into the pod as well as the Go-side bug. Fixes NVIDIA#2776 Signed-off-by: Abrar Shivani <ashivani@nvidia.com>
abrarshivani
force-pushed
the
fix/precompiled-lib-modules-mount
branch
from
August 21, 2026 23:57
24119ce to
77d1673
Compare
Contributor
Author
|
@tariq1890 Sure, will backport this change once this PR is checked in. |
tariq1890
approved these changes
Aug 22, 2026
Contributor
|
/cherry-pick release-26.7 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fixes #2776.
getDriverAdditionalConfigsbuilds thelib-modulesvolume and mount inside theif !cr.Spec.UsePrecompiledDrivers()branch, and the block itself is guarded byif cr.Spec.UsePrecompiledDrivers() && (pool.osRelease == "sles" || pool.osRelease == "sl-micro").The inner condition can never hold in that branch, so an NVIDIADriver with
usePrecompiled: trueon sles or sl-micro renders a driver pod without/run/host/lib/modules.Rather than repeat the predicate, the mount now hangs off the non-precompiled
branch as an
else. The two paths are mutually exclusive, and saying so oncegives precompiled-only logic an obvious home, which is what went wrong here in
the first place. The comment above the mount goes with it: the
elsealreadysays precompiled, the guard already says SUSE, and the volume literals already
say host module tree.
This only affects the NVIDIADriver path. The equivalent code in
controllers/object_controls.gosits before the early return for precompileddrivers and works as intended. That is also why it went unnoticed, since the
tests that came in with a7323f5 only covered the ClusterPolicy path.
One note for triage: the issue lists v24.9.2 under Environment, but v24.9.2 has
no
lib-moduleshandling in either driver path, so the unreachable codedescribed in the report cannot exist there. The line numbers quoted in the issue
match current main. A v24.9.2 user on precompiled SUSE does see the same missing
mount, but because the feature had not landed yet rather than because of this
bug.
Checklist
make lint)make validate-generated-assets)make validate-modules)Testing
Added
TestDriverPrecompiledLibModules, a table-driven test covering sles andsl-micro with precompiled drivers, plus sles without them and ubuntu with them
as negative cases. Each case renders the DaemonSet and checks the pod spec for
the
lib-moduleshostPath volume andnvidia-driver-ctrfor the mount, so itcovers both the helper and the template threading
AdditionalConfigsinto thepod.
The two SUSE cases fail against the current code with "expected a lib-modules
volume on the driver pod spec" and pass with the fix. Removing the
AdditionalConfigsmounts from the driver container inmanifests/state-driver/0500_daemonset.yamlalso fails the test, which thehelper-level assertions alone would not have caught.
go test ./internal/... ./controllers/...passes, along withgo vet, gofmtand
-race. Lint and the generated-asset check were run withGOOS=linux,since the vendored
filepath-securejoin/pathrs-liteis linux-only and breaksthe typecheck of
cmd/nvidia-validatoron macOS. Regenerating the CRDs producesno diff.