-
Notifications
You must be signed in to change notification settings - Fork 353
Add envvar for libcuda.so parent dir to CDI spec #1030
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
7b58a3e
to
5349a84
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces a LIBCUDA_SO_PARENT_DIRECTORY_CONTAINER_PATH environment variable to the generated CDI specification and implements the required EnvVars interface across multiple discoverer types.
- Adds an EnvVars() method to several discoverers returning empty environment variable lists where appropriate
- Updates the driver library discoverer to extract and report the libcuda.so parent directory
- Adjusts tests to validate that the new environment variable is being injected into container edits
Reviewed Changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
pkg/nvcdi/workarounds-device-folder-permissions.go | Adds EnvVars() method returning nil, consistent with other discoverers |
pkg/nvcdi/driver-nvml.go | Updates NewDriverLibraryDiscoverer and getVersionLibs to extract the libcuda.so parent directory and append the environment variable |
internal/platform-support/dgpu/nvsandboxutils.go; internal/platform-support/dgpu/by-path-hooks.go | Implements EnvVars() returning nil, maintaining interface consistency |
internal/edits/edits.go | Adds processing of EnvVars into container edits |
internal/discover/* | Updates Discover implementations (none, list, hooks, first-valid, envvar, cache, discover_mock, discover) to support the new EnvVars() interface method |
cmd/nvidia-ctk/cdi/generate/generate_test.go; cmd/nvidia-ctk-installer/container/toolkit/toolkit_test.go | Updates tests to verify injection of LIBCUDA_SO_PARENT_DIRECTORY_CONTAINER_PATH in container edits |
@@ -22,6 +22,12 @@ type Device struct { | |||
Path string | |||
} | |||
|
|||
// EnvVar represents a discovered environment variable. | |||
type EnvVar struct { | |||
Name string |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am more of a Key/Value mind, but not a blocker.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Variables are defined with name-value pairs: NAME=any string as value. The variable name is usually in capital letters. Anything that follows the equal-sign is considered the variable's value until the terminating line feed character. Variables can be defined ad hoc in a terminal by writing the appropriate command. In Bash this would be export MYVAL="Hello world". In this case the variable stays defined until the end of the terminal session.
5349a84
to
2751299
Compare
This change adds environment variables to the Discover interface. Signed-off-by: Evan Lezar <[email protected]>
This change adds a LIBCUDA_SO_PARENT_DIRECTORY_CONTAINER_PATH envvar to a generated CDI specification. This reports where the `libcuda.so.*` libraries will be injected into the container. Signed-off-by: Evan Lezar <[email protected]>
2751299
to
d2ca583
Compare
Pull Request Test Coverage Report for Build 15585739792Details
💛 - Coveralls |
This change adds a LIBCUDA_SO_PARENT_DIRECTORY_CONTAINER_PATH envvar to a generated CDI specification. This reports where the
libcuda.so.*
libraries will be injected into the container.Fixes #1028