Skip to content

Mount host /lib/modules for precompiled drivers on SUSE - #2784

Open
abrarshivani wants to merge 1 commit into
NVIDIA:mainfrom
abrarshivani:fix/precompiled-lib-modules-mount
Open

Mount host /lib/modules for precompiled drivers on SUSE#2784
abrarshivani wants to merge 1 commit into
NVIDIA:mainfrom
abrarshivani:fix/precompiled-lib-modules-mount

Conversation

@abrarshivani

Copy link
Copy Markdown
Contributor

Description

Fixes #2776.

getDriverAdditionalConfigs builds the lib-modules volume and mount inside the
if !cr.Spec.UsePrecompiledDrivers() branch, and the block itself is guarded by
if cr.Spec.UsePrecompiledDrivers() && (pool.osRelease == "sles" || pool.osRelease == "sl-micro").
The inner condition can never hold in that branch, so an NVIDIADriver with
usePrecompiled: true on 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 once
gives 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 else already
says 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.go sits before the early return for precompiled
drivers 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-modules handling in either driver path, so the unreachable code
described 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

  • No secrets, sensitive information, or unrelated changes
  • Lint checks passing (make lint)
  • Generated assets in-sync (make validate-generated-assets)
  • Go mod artifacts in-sync (make validate-modules)
  • Test cases are added for new code paths

Testing

Added TestDriverPrecompiledLibModules, a table-driven test covering sles and
sl-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-modules hostPath volume and nvidia-driver-ctr for the mount, so it
covers both the helper and the template threading AdditionalConfigs into the
pod.

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
AdditionalConfigs mounts from the driver container in
manifests/state-driver/0500_daemonset.yaml also fails the test, which the
helper-level assertions alone would not have caught.

go test ./internal/... ./controllers/... passes, along with go vet, gofmt
and -race. Lint and the generated-asset check were run with GOOS=linux,
since the vendored filepath-securejoin/pathrs-lite is linux-only and breaks
the typecheck of cmd/nvidia-validator on macOS. Regenerating the CRDs produces
no diff.

Comment thread internal/state/driver_volumes.go Outdated
@tariq1890

Copy link
Copy Markdown
Contributor

Thanks for working on this @abrarshivani !

This will need to be backported to release-26.7

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
abrarshivani force-pushed the fix/precompiled-lib-modules-mount branch from 24119ce to 77d1673 Compare August 21, 2026 23:57
@abrarshivani

Copy link
Copy Markdown
Contributor Author

@tariq1890 Sure, will backport this change once this PR is checked in.

@tariq1890

Copy link
Copy Markdown
Contributor

/cherry-pick release-26.7

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Unreachable code prevents lib-modules volume mount when using precompiled drivers.

2 participants