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

Simplify standalone installer #765

Merged
merged 8 commits into from
Jan 22, 2025

Conversation

elezar
Copy link
Member

@elezar elezar commented Oct 30, 2024

This PR starts refactoring the toolkit installation process from the toolkit contianer.

The changes are non-functional and limited to:

  • Removing unused code
  • Using a logger.Interface for logging instead of the top-level logger

The goal is to gradually move towards allowing #602 to be used for a single container image.

@elezar elezar self-assigned this Oct 30, 2024
@elezar elezar requested a review from tariq1890 October 30, 2024 15:36
@elezar elezar force-pushed the use-logger-in-toolkit-install branch from 2d685f7 to dbed2e9 Compare October 30, 2024 15:38
@elezar elezar requested review from klueska and cdesiniotis November 1, 2024 11:07
@elezar elezar marked this pull request as draft November 1, 2024 14:56
@elezar
Copy link
Member Author

elezar commented Nov 1, 2024

I've pulled out the addition of tests to #768. I would rather merge those first.

@elezar elezar force-pushed the use-logger-in-toolkit-install branch from dbed2e9 to a71dcc2 Compare November 6, 2024 00:00
@elezar elezar force-pushed the use-logger-in-toolkit-install branch from a71dcc2 to 2a5d5c4 Compare January 16, 2025 15:41
@elezar elezar marked this pull request as ready for review January 16, 2025 15:41
@elezar elezar changed the title Use logger in toolkit install Simplify standalone installer Jan 16, 2025
@elezar elezar force-pushed the use-logger-in-toolkit-install branch 3 times, most recently from 255d550 to ba98e64 Compare January 16, 2025 16:37
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.

just 2 nits, not blockers.
It would be good to add to the README a small section on how to use the installer from a container using docker/podman run

cmd/nvidia-ctk-installer/main.go Show resolved Hide resolved
@elezar
Copy link
Member Author

elezar commented Jan 21, 2025

It would be good to add to the README a small section on how to use the installer from a container using docker/podman run

This is not (currently) a supported installation mechanism.

This change moves the containerized installer from nvidia-toolkit to
cmd/nvidia-ctk-installer to allow for its use in CI.

Signed-off-by: Evan Lezar <[email protected]>
@elezar elezar force-pushed the use-logger-in-toolkit-install branch from ba98e64 to 6bd292e Compare January 21, 2025 13:40
@elezar elezar merged commit c22f3bd into NVIDIA:main Jan 22, 2025
10 checks passed
@elezar elezar deleted the use-logger-in-toolkit-install branch January 22, 2025 13:42
OddBloke added a commit to wolfi-dev/os that referenced this pull request Jan 23, 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