Ensure that libcuda.so is in the ldcache #947

elezar · 2025-02-27T13:07:42Z

These changes add a hook to create soname symlinks (e.g. libcuda.so.1 -> libcuda.so.RM_VERSION) to ensure that the libcuda.so -> libcuda.so.1 -> libcuda.so.RM_VERSION symlink chain exists when the ldcache is updated. This allows libcuda.so to be present in the ldcache when ldconfig is run.

Note that since we're adding a new hook, a generating client such as the k8s-device plugin or the k8s-dra-driver-gpu must be used with a nvidia-cdi-hook binary with a sufficient version.

See https://nvbugspro.nvidia.com/bug/5076022

ArangoGutierrez · 2025-02-27T13:17:40Z

/ok to test

Copilot

PR Overview

This PR adds an end-to-end test to verify that the libcuda symlink chain is properly present in the ldcache when running via Docker with NVIDIA runtime. Key changes include:

Addition of the "strings" import to support string splitting.
New test block that pulls an Ubuntu image and runs a container to inspect the ldcache output for libcuda.
Parsing and validation of the ldcache output to ensure both "libcuda.so" and "libcuda.so.1" are present.

Reviewed Changes

File	Description
tests/e2e/nvidia-container-toolkit_test.go	Added an end-to-end test to validate the presence of libcuda symlink entries in the ldcache

Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.

tests/e2e/nvidia-container-toolkit_test.go

elezar · 2025-02-27T13:32:11Z

cmd/nvidia-cdi-hook/create-soname-symlinks/soname-symlinks.go

+
+	// Create the 'create-soname-symlinks' command
+	c := cli.Command{
+		Name:  "create-soname-symlinks",


@klueska I elected to add a new hook entirely instead of modifying the existing update-ldcache. This is in keeping with "purpose-built hooks" and also means that the hook name can be used to indicate the intent.

ArangoGutierrez

lgtm, just one nit

cmd/nvidia-cdi-hook/create-soname-symlinks/soname-symlinks.go

cdesiniotis

Left one clarifying question, but not a blocker.

cdesiniotis · 2025-03-06T01:16:25Z

cmd/nvidia-cdi-hook/create-soname-symlinks/soname-symlinks.go

+		"-N",
+	)
+	// Explicitly specific the directories to add.
+	args = append(args, dirs...)


Question -- does this create .so symlinks for all libraries present in the specified directories? Does this differ from the behavior of the legacy libnvidia-container implementation, which IIRC would only create the .so symlinks for a small list of libraries (like libcuda.so)?

This is not the .so symlinks, but the SONAME symlinks i.e. libcuda.so.1 -> libcuda.so.RM_VERSION in the case of libcuda. The .so symlinks are created using the "standard" create-symlinks hook.

copy-pr-bot · 2025-03-06T08:30:05Z

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

elezar · 2025-03-06T08:52:34Z

@cdesiniotis I am removing the must-backport label for this. Although this was triggered by a customer request, there is a workaround available and I would rather not backport another new hook to the release-1.17 branch.

jgehrcke · 2025-03-06T10:29:39Z

and I would rather not backport another new hook to the release-1.17 branch.

Sounds good.

jgehrcke · 2025-03-06T10:39:18Z

cmd/nvidia-cdi-hook/cudacompat/cudacompat.go

 	if hostDriverVersion == "" {
 		m.logger.Debugf("Host driver version not specified")
 		return "", nil
 	}
-	if !containerRoot.hasPath(cudaCompatPath) {
+	if !containerRoot.HasPath(cudaCompatPath) {


I have picked a more or less arbitrary place in the code to put the following comment. Having more expressive variable names would make review for me easier. You can postpone looking at this comment until after merge, that's fine with me! In the spirit of learning what we do here however I'd appreciate an answer at some point :)

In my software engineering life I have always deeply cared about file system terminology and operations, and about making related code readable and self-expressive. For example,

I like to distinguish a "relative path" from an "absolute path" via variable name (if possible)

I like to distinguish a file object from a file path via variable name

I like to distinguish a path to a file from a path to a directory (expressing intent, of course these are technically the same)

I like to call things a "base name" for expressing intent a well (when there are no path separators, and this is supposed to be a "file name" or "directory name").

What is containerRoot in canonical unix file path terminology?

Is it guaranteed to point to a directory?

Is it an absolute path?

Is it always just /?

What are some properties about cudaCompatPath that we know/guarantee, that we could also express in the variable name or in the type?

Is it always a relative path (not starting with `/)?

Is it always a path to a directory?

Thanks for calling this out. I think we could do a pass through this and improve readability greatly. There is no rush on this particular PR (I've elected to hold the changes back from the v1.17.5 release) and as such, I think we can address these concerns here instead of as a follow-up.

The containerRootDir is the absolute path on the host filesystem to the root (/) of the container filesystem. It is specified in the OCI Runtime Specification as Root which informs the slightly ambiguous name. The containerRoot variable is the typed representation of this directory that allows us to attached helper functions such as HasPath / Resolve / Glob to it.

The cudaCompatPath is the absolute path to the directory containing the CUDA compat libraries in the container (if it exists). It defined as a constant with value /usr/local/cuda/compat. That is to say, it is always an absolute path to a directory in the container, and we're confirming that this exists, but have to calculate the path to this folder on the host filesystem to do this check.

I think we can address these concerns here instead of as a follow-up.

Thank you for that!

The containerRootDir is the absolute path on the host filesystem to the root (/) of the container filesystem

Is the container filesystem always accessible from within the host filesystem?

It is specified in the OCI Runtime Specification as Root which informs the slightly ambiguous name.

gotcha. Thanks for that background.

The cudaCompatPath is the absolute path to the directory containing the CUDA compat libraries in the container (if it exists).

Thank you for that precision. Now, that is quickly&easily understandable.

but have to calculate the path to this folder on the host filesystem to do this check

meaning that it's always mounted into the container, and never baked into it?

meaning that it's always mounted into the container, and never baked into it?

No, I think there's a misunderstanding. The folder is baked into the container, but the hook is running on the host (in the Container's mount namespace). This means that we need to calculate the host path to the /usr/local/cuda/compat folder to check if it exists.

jgehrcke · 2025-03-06T10:41:26Z

cmd/nvidia-cdi-hook/utils/container-root.go

+)
+
+// A ContainerRoot represents the root filesystem of a container.
+type ContainerRoot string


Oh, it's just a string.

Relates to my previous comment: I'd love to see the intent expressed in the variable name: path to a directory?

And this is probably really just my lack of knowledge: I wonder: when is the container's root file system not located at just /?

I have added som context above. Let me know if it's not clear and I can add more information.

jgehrcke

Thanks for all the work! I understand very little but you have my full emotional support! And I know you will repair everything you destroy. In that sense: approved with honor. 🚀

jgehrcke · 2025-03-21T13:18:22Z

cmd/nvidia-cdi-hook/chmod/chmod.go

@@ -113,15 +113,15 @@ func (m command) run(c *cli.Context, cfg *config) error {
 		return fmt.Errorf("failed to load container state: %v", err)
 	}

-	containerRoot, err := s.GetContainerRoot()
+	containerRoot, err := s.GetContainerRootDirPath()


❤️ Thanks for adding clarity here.

And now that I have read your explanation.. this is a path that is valid in the host filesystem, and points to the container filesystem's root..? :)

Yes. It is the absolute path on the host where the root of the container's filesystem is located.

jgehrcke

Thank you for all the iteration. Let's land this. I have left a few more questions while browsing this, but please don't slow down landing this because of my remarks.

coveralls · 2025-06-17T14:01:16Z

Pull Request Test Coverage Report for Build 15707946920

Details

9 of 233 (3.86%) changed or added relevant lines in 6 files are covered.
27 unchanged lines in 1 file lost coverage.
Overall coverage decreased (-0.3%) to 33.356%

Changes Missing Coverage	Changed/Added Lines	%
cmd/nvidia-cdi-hook/commands/commands.go	1	0.0%
internal/ldconfig/ldconfig_linux.go	2	0.0%
cmd/nvidia-cdi-hook/update-ldcache/update-ldcache.go	12	0.0%
cmd/nvidia-cdi-hook/create-soname-symlinks/soname-symlinks.go	88	0.0%
internal/ldconfig/ldconfig.go	121	0.0%

Files with Coverage Reduction	New Missed Lines	%
internal/discover/hooks.go	27	58.14%

Totals
Change from base Build 15683627970:	-0.3%
Covered Lines:	4336
Relevant Lines:	12999

💛 - Coveralls

coveralls · 2025-06-17T14:01:24Z

Pull Request Test Coverage Report for Build 15849722836

Details

9 of 231 (3.9%) changed or added relevant lines in 6 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage decreased (-0.3%) to 33.387%

Changes Missing Coverage	Changed/Added Lines	%
cmd/nvidia-cdi-hook/commands/commands.go	1	0.0%
internal/ldconfig/ldconfig_linux.go	2	0.0%
cmd/nvidia-cdi-hook/update-ldcache/update-ldcache.go	12	0.0%
cmd/nvidia-cdi-hook/create-soname-symlinks/soname-symlinks.go	88	0.0%
internal/ldconfig/ldconfig.go	119	0.0%

Totals
Change from base Build 15846172189:	-0.3%
Covered Lines:	4377
Relevant Lines:	13110

💛 - Coveralls

refactor

jgehrcke · 2025-06-19T13:14:33Z

internal/discover/ldconfig.go

-	if ldconfig != "" {
-		args = append(args, "--ldconfig-path", ldconfig)
+	if d.ldconfigPath != "" {
+		args = append(args, "--ldconfig-path", d.ldconfigPath)


jgehrcke · 2025-06-19T13:15:49Z

internal/ldconfig/ldconfig.go

+		args = append(args, "-N")
+	}
+
+	// If we did not create the


If we did not create the rest of the sentence, then

jgehrcke · 2025-06-19T13:16:28Z

cmd/nvidia-cdi-hook/create-soname-symlinks/soname-symlinks.go

+}
+
+type options struct {
+	folders       cli.StringSlice


directoryPaths?

agree with Jan-P Directory is a much nicer call to Folder.

These options are filled by the CLI. A convention that I use is that the member matches (with the exception of plurality if required) the primary flag. Since the CLI argument is --folder (and can be repeated), I want to keep this folders.

jgehrcke

Should we stop iterating on minor details, and just land this? :) Thank you, Evan!

ArangoGutierrez

Some quick nits, I feel I lean into more towards directory than folder.
PR Looks good overall just naming convetion discussion and one misSpell

ArangoGutierrez · 2025-06-24T11:28:27Z

cmd/nvidia-cdi-hook/create-soname-symlinks/soname-symlinks.go

+}
+
+type options struct {
+	folders       cli.StringSlice


agree with Jan-P Directory is a much nicer call to Folder.

ArangoGutierrez · 2025-06-24T11:29:42Z

cmd/nvidia-cdi-hook/create-soname-symlinks/soname-symlinks.go

+	c.Flags = []cli.Flag{
+		&cli.StringSliceFlag{
+			Name:        "folder",
+			Usage:       "Specify a folder to generate soname symlinks in",


if we end up deciding for directory over folder on the discussion above, let's make sure we update this string

ArangoGutierrez · 2025-06-24T11:31:32Z

cmd/nvidia-cdi-hook/create-soname-symlinks/soname-symlinks.go

+		&cli.StringSliceFlag{
+			Name:        "folder",
+			Usage:       "Specify a folder to generate soname symlinks in",
+			Destination: &cfg.folders,


ArangoGutierrez · 2025-06-24T11:31:55Z

cmd/nvidia-cdi-hook/create-soname-symlinks/soname-symlinks.go

+	}
+}
+
+// createSonameSymlinks runs ldconfig enusures that soname symlinks are created


Suggested change

// createSonameSymlinks runs ldconfig enusures that soname symlinks are created

// createSonameSymlinks runs ldconfig ensures that soname symlinks are created

ArangoGutierrez · 2025-06-24T11:32:39Z

cmd/nvidia-cdi-hook/create-soname-symlinks/soname-symlinks.go

+//
+// args[0] is the reexec initializer function name
+// args[1] is the path of the ldconfig binary on the host
+// args[2] is the container root directory


we use directory here, I guess this is a good clue on how to define the discussion above.

This directory is different from the other directories that we're talking about.

ArangoGutierrez · 2025-06-24T11:33:34Z

cmd/nvidia-cdi-hook/update-ldcache/update-ldcache.go

 //
 // args[0] is the reexec initializer function name
 // args[1] is the path of the ldconfig binary on the host
 // args[2] is the container root directory
-// The remaining args are folders that need to be added to the ldcache.
+// The remaining args are folders where soname symlinks need to be created.


jeje folders vs dir conversation. I guess is was a nice Nit catch by @jgehrcke

ArangoGutierrez · 2025-06-24T11:34:46Z

internal/ldconfig/ldconfig.go

+}
+
+// CreateSonameSymlinks uses ldconfig to create the soname symlinks in the
+// specified folders.


folder vs dir

ArangoGutierrez · 2025-06-24T11:35:33Z

internal/ldconfig/ldconfig.go

+	// If the ld.so.conf.d directory exists, we create a config file there
+	// containing the required directories, otherwise we add the specified
+	// directories to the ldconfig command directly.


here we use dir but some lines above we use folder

elezar · 2025-06-24T11:40:25Z

Some quick nits, I feel I lean into more towards directory than folder.

Please note that the API for the hook uses --folder as a command line argument and this is not something that I want to change at the moment. I will update the local content to be more consistent though.

This change adds a create-soname-symlinks hook that can be used to ensure that the soname symlinks for injected libraries exist in a container. This is done by calling ldconfig -n -N for the directories containing the injected libraries. This also ensures that libcuda.so is present in the ldcache when the update-ldcache hook is run. Signed-off-by: Evan Lezar <[email protected]>

Signed-off-by: Evan Lezar <[email protected]>

ArangoGutierrez

My comments on folder vs directory have been addressed.
LGTM now

ArangoGutierrez requested review from ArangoGutierrez and Copilot February 27, 2025 13:21

Copilot AI reviewed Feb 27, 2025

View reviewed changes

tests/e2e/nvidia-container-toolkit_test.go Outdated Show resolved Hide resolved

tests/e2e/nvidia-container-toolkit_test.go Outdated Show resolved Hide resolved

elezar force-pushed the ensure-libcuda.so-in-ldcache branch from 6e7e1a8 to 44e6630 Compare February 27, 2025 13:25

elezar commented Feb 27, 2025

View reviewed changes

ArangoGutierrez reviewed Feb 27, 2025

View reviewed changes

cmd/nvidia-cdi-hook/create-soname-symlinks/soname-symlinks.go Outdated Show resolved Hide resolved

elezar linked an issue Feb 27, 2025 that may be closed by this pull request

LDCache does not include libcuda.so #944

Closed

elezar force-pushed the ensure-libcuda.so-in-ldcache branch from 44e6630 to e43da10 Compare February 27, 2025 13:48

elezar added the must-backport The changes in PR need to be backported to at least one stable release branch. label Feb 27, 2025

elezar force-pushed the ensure-libcuda.so-in-ldcache branch 3 times, most recently from daadba9 to 9daa179 Compare February 28, 2025 12:45

elezar requested review from klueska and jgehrcke February 28, 2025 12:52

ArangoGutierrez previously approved these changes Mar 5, 2025

View reviewed changes

cdesiniotis reviewed Mar 6, 2025

View reviewed changes

elezar force-pushed the ensure-libcuda.so-in-ldcache branch from 9daa179 to 035abe0 Compare March 6, 2025 08:30

elezar removed the must-backport The changes in PR need to be backported to at least one stable release branch. label Mar 6, 2025

jgehrcke reviewed Mar 6, 2025

View reviewed changes

jgehrcke approved these changes Mar 6, 2025

View reviewed changes

elezar added this to the v1.18.0 milestone Mar 19, 2025

elezar force-pushed the ensure-libcuda.so-in-ldcache branch 3 times, most recently from 7229201 to bdfaea4 Compare March 20, 2025 12:56

jgehrcke reviewed Mar 21, 2025

View reviewed changes

jgehrcke previously approved these changes Mar 21, 2025

View reviewed changes

ArangoGutierrez assigned elezar May 6, 2025

ArangoGutierrez mentioned this pull request May 13, 2025

Add the ability to disable specific (or all) CDI hooks when generating a CDI specification #1077

Merged

elezar force-pushed the ensure-libcuda.so-in-ldcache branch 5 times, most recently from 66fe6d4 to f653469 Compare June 17, 2025 13:09

elezar force-pushed the ensure-libcuda.so-in-ldcache branch from f653469 to bedd086 Compare June 17, 2025 17:54

elezar requested a review from jgehrcke June 18, 2025 10:10

jgehrcke reviewed Jun 19, 2025

View reviewed changes

jgehrcke approved these changes Jun 19, 2025

View reviewed changes

elezar force-pushed the ensure-libcuda.so-in-ldcache branch from bedd086 to fddc1d3 Compare June 24, 2025 09:22

ArangoGutierrez requested changes Jun 24, 2025

View reviewed changes

elezar added 2 commits June 24, 2025 13:49

[no-relnote] Refactor ldconfig hooks

39975fc

Signed-off-by: Evan Lezar <[email protected]>

elezar force-pushed the ensure-libcuda.so-in-ldcache branch from fddc1d3 to 39975fc Compare June 24, 2025 11:54

elezar requested a review from ArangoGutierrez June 24, 2025 12:10

ArangoGutierrez approved these changes Jun 24, 2025

View reviewed changes

elezar merged commit c95d36d into NVIDIA:main Jun 24, 2025
16 checks passed

elezar deleted the ensure-libcuda.so-in-ldcache branch June 24, 2025 17:55

ereslibre mentioned this pull request Jul 6, 2025

nvidia-container-toolkit: Some cuda libraries do not work without extra LD_LIBRARY_PATH NixOS/nixpkgs#366109

Open

elezar mentioned this pull request Jul 21, 2025

Create .so symlinks for driver libraries in container #326

Merged

	// createSonameSymlinks runs ldconfig enusures that soname symlinks are created
	// createSonameSymlinks runs ldconfig ensures that soname symlinks are created

Ensure that libcuda.so is in the ldcache #947

Ensure that libcuda.so is in the ldcache #947

Uh oh!

Conversation

elezar commented Feb 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ArangoGutierrez commented Feb 27, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

PR Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ArangoGutierrez left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cdesiniotis left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

copy-pr-bot bot commented Mar 6, 2025

Uh oh!

elezar commented Mar 6, 2025

Uh oh!

jgehrcke commented Mar 6, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jgehrcke left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jgehrcke left a comment

Choose a reason for hiding this comment

Uh oh!

coveralls commented Jun 17, 2025

Pull Request Test Coverage Report for Build 15707946920

Details

💛 - Coveralls

Uh oh!

coveralls commented Jun 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Test Coverage Report for Build 15849722836

Details

💛 - Coveralls

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

elezar commented Feb 27, 2025 •

edited

Loading

coveralls commented Jun 17, 2025 •

edited

Loading