Skip to content
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

Refactor toolkit installer #990

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

elezar
Copy link
Member

@elezar elezar commented Mar 13, 2025

These changes refactor the toolkit installer used from the toolkit contianer.

The goal is to prepare for ##602 where a single container image is used for the toolkit instead of multiple container images.

@elezar elezar force-pushed the refactor-toolkit-installer branch from b1c8a90 to 15d6c5e Compare March 13, 2025 14:19
@ArangoGutierrez ArangoGutierrez requested a review from Copilot March 13, 2025 15:37
Copy link

@Copilot Copilot AI left a 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 refactors the toolkit installer to prepare for a future change where a single container image is used for the toolkit. Key changes include new implementations for collecting libraries and executables, creating wrappers for executables, centralizing artifact discovery, and removing deprecated container/toolkit files.

Reviewed Changes

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

Show a summary per file
File Description
cmd/nvidia-ctk-installer/toolkit/installer/executables_test.go Adds tests for verifying the wrapper rendering of executables.
cmd/nvidia-ctk-installer/toolkit/installer/libraries.go Implements library collection logic with symlink creation.
cmd/nvidia-ctk-installer/toolkit/installer/installer.go Provides the main installer logic including file copying and wrapper creation.
cmd/nvidia-ctk-installer/toolkit/installer/executables.go Implements executable collection and wrapper file generation via templates.
cmd/nvidia-ctk-installer/toolkit/installer/artifact-root.go Introduces an artifact root abstraction for locating libraries and executables.
cmd/nvidia-ctk-installer/toolkit/configure.go Installs and configures the NVIDIA toolkit configuration file.
Other files (including mocks, directory creation, and tests) Update mocks and tests to support the refactored installer logic while cleaning up deprecated code.

Copy link
Collaborator

@ArangoGutierrez ArangoGutierrez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First pass looks good. will give it a try

@elezar elezar force-pushed the refactor-toolkit-installer branch 3 times, most recently from dee62ca to 9660137 Compare March 14, 2025 12:55
elezar added 3 commits March 17, 2025 15:23
Since this is running in a contianer the contents of the
/etc/nvidia-container-runtime/config.toml file is equivalent to the
default config. This change makes it explicit.

Signed-off-by: Evan Lezar <[email protected]>
@elezar elezar force-pushed the refactor-toolkit-installer branch from 9660137 to e6ee2d7 Compare March 17, 2025 14:00
Signed-off-by: Evan Lezar <[email protected]>
@elezar elezar force-pushed the refactor-toolkit-installer branch from e6ee2d7 to c575025 Compare March 17, 2025 14:05
@elezar elezar added this to the v1.18.0 milestone Mar 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants