Add option to enable CDI via NVIDIA-CTK#319
Merged
ArangoGutierrez merged 1 commit intoNVIDIA:mainfrom Mar 20, 2025
Merged
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds the option to enable the Container Device Interface (CDI) for NVIDIA-CTK by introducing a new boolean parameter and updating the command-line configuration as well as the API types.
- Updated container-toolkit template to include the "--cdi.enable" flag.
- Added a new boolean field "EnableCDI" in both the provisioner and API types.
- Modified the constructor logic for ContainerToolkit to conditionally set EnableCDI based on the environment.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| pkg/provisioner/templates/container-toolkit.go | Updated the runtime configuration command and added EnableCDI field; updated initialization logic with a TODO regarding the default value. |
| api/holodeck/v1alpha1/types.go | Added the EnableCDI boolean field to the NVIDIAContainerToolkit type. |
Comments suppressed due to low confidence (1)
pkg/provisioner/templates/container-toolkit.go:40
- Verify that the boolean value passed via '{{.EnableCDI}}' correctly translates to the expected string value required by NVIDIA-CTK. Consider explicitly converting the boolean to the proper flag format if needed.
sudo nvidia-ctk runtime configure --runtime={{.ContainerRuntime}} --set-as-default --cdi.enable={{.EnableCDI}}
42e1df7 to
d2b1402
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR adds an option to enable the Container Device Interface (CDI) via NVIDIA-CTK.
- Updates the container runtime configuration command to include the new --cdi.enable flag.
- Introduces an EnableCDI field in both the provisioner template and the NVIDIAContainerToolkit CRD.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| pkg/provisioner/templates/container-toolkit.go | Adds the EnableCDI field and updates the CLI command with the new flag. |
| api/holodeck/v1alpha1/types.go | Updates the CRD definition by adding an optional EnableCDI boolean field. |
Comments suppressed due to low confidence (2)
pkg/provisioner/templates/container-toolkit.go:40
- Verify that the addition of the '--cdi.enable' flag is intentional, and double-check that the template execution in the Execute function is not duplicated unintentionally, as duplicate execution may lead to unexpected behavior.
sudo nvidia-ctk runtime configure --runtime={{.ContainerRuntime}} --set-as-default --cdi.enable={{.EnableCDI}}
pkg/provisioner/templates/container-toolkit.go:62
- Consider adding test cases to ensure that the EnableCDI flag is correctly set and reflected in the container toolkit configuration when specified in the environment.
if env.Spec.NVIDIAContainerToolkit.EnableCDI {
d2b1402 to
063640a
Compare
ArangoGutierrez
commented
Mar 20, 2025
elezar
reviewed
Mar 20, 2025
elezar
reviewed
Mar 20, 2025
elezar
reviewed
Mar 20, 2025
063640a to
bb8d580
Compare
bb8d580 to
6aa1ba1
Compare
elezar
reviewed
Mar 20, 2025
Signed-off-by: Carlos Eduardo Arango Gutierrez <eduardoa@nvidia.com>
6aa1ba1 to
5429d0a
Compare
elezar
approved these changes
Mar 20, 2025
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.
This pull request introduces changes to enable the Container Device Interface (CDI) plugin in the NVIDIA Container Toolkit. The key changes involve adding a new configuration option for CDI in the
NVIDIAContainerToolkitstruct and updating the container runtime configuration script to support this option.Configuration enhancements:
api/holodeck/v1alpha1/types.go: Added a new boolean fieldEnableCDIto theNVIDIAContainerToolkitstruct to enable the CDI plugin.Runtime configuration updates:
pkg/provisioner/templates/container-toolkit.go: Modified the container runtime configuration script to include the--enable-cdiflag based on the newEnableCDIfield.pkg/provisioner/templates/container-toolkit.go: Updated theContainerToolkitstruct and its initialization logic to include theEnableCDIfield, with a default value offalse.