Skip to content

Use single image for deb and rpm-based systems #602

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

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

elezar
Copy link
Member

@elezar elezar commented Jul 12, 2024

This change moves to a single ubi8-based base image for the NVIDIA Container Toolkit contianer. This simplifies the deployment of the toolkit in that users don't have to select a specific image based on their host distribution.

In this implementation, the contents of the deb and rpm packages are stored at a location in the image and copied to the relevant location on the host (as in the past). The following envvironment variables can be used to control the behaviour:

  • TOOLKIT_PACKAGE_TYPE (one of auto (default), deb, or rpm) specifies the packaging system used on the host machine. If this is default, the toolking installer attempts to determine the host packaging system and defaults to deb if this fails.
  • TOOLKIT_SOURCE_ROOT (default = "") specifies the root for the files to be installed. If this is set to "" the path is calculated as /artifacts/{{ .ToolkitPackageType }} where ToolkitPackageType is the resolved value of TOOLKIT_PACKAGE_TYPE.

@elezar elezar changed the title TOFIX: packages in image Allow single image to be used for deb and rpm-based systems Aug 28, 2024
@elezar elezar force-pushed the use-ubi8-image branch 5 times, most recently from 4242ba2 to 5d42fbb Compare September 4, 2024 13:39
@elezar elezar force-pushed the use-ubi8-image branch 2 times, most recently from 4da1db4 to ba65c11 Compare March 17, 2025 09:38
@elezar elezar changed the title Allow single image to be used for deb and rpm-based systems Use single image for deb and rpm-based systems Apr 17, 2025
@elezar elezar force-pushed the use-ubi8-image branch 4 times, most recently from 4b50ca0 to 5952a98 Compare April 23, 2025 12:03
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 simplifies the NVIDIA Container Toolkit installer by consolidating deb and rpm-based images into a single ubi8-based image and updating the corresponding flag names and package resolution logic.

  • Renamed the source root flag to "toolkit-source-root" and added a new "toolkit-package-type" flag.
  • Introduced functions to resolve the package type based on host system binaries.
  • Added a new file to encapsulate package-type definitions.

Reviewed Changes

Copilot reviewed 5 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
cmd/nvidia-ctk-installer/toolkit/toolkit.go Added a newline for readability.
cmd/nvidia-ctk-installer/toolkit/installer/package-type.go Introduced a new file for package-type definitions.
cmd/nvidia-ctk-installer/toolkit/installer/installer.go Refactored toolkitInstaller initialization to set a default sourceRoot.
cmd/nvidia-ctk-installer/main_test.go Updated test flag to use "toolkit-source-root".
cmd/nvidia-ctk-installer/main.go Updated flag names, added package type flag, and implemented source root resolution logic.
Files not reviewed (2)
  • deployments/container/Dockerfile.ubi8: Language not supported
  • deployments/container/Makefile: Language not supported

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 some comments, will do a second review once PR becomes ready for review

Name: "source-root",
Value: "/",
Usage: "The folder where the required toolkit artifacts can be found",
Name: "toolkit-source-root",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we need to update documentation on this flag name change?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is a new flag and is not publicly accessible. It is internal only.

@@ -0,0 +1,19 @@
/*
Copy link
Collaborator

Choose a reason for hiding this comment

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

What's the purpose of this file?

Copy link
Member Author

Choose a reason for hiding this comment

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

It should be removed.


locator := lookup.NewExecutableLocator(a.logger, hostRoot)
if candidates, err := locator.Locate("/usr/bin/rpm"); err == nil && len(candidates) > 0 {
return "rpm", nil
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we have rpm and deb as pkg consts so is safer to use the var instead of a string.

COPY --from=debpackages /artifacts/deb /artifacts/deb
COPY --from=build /artifacts/bin /artifacts/build

FROM nvcr.io/nvidia/cuda:12.8.1-base-ubi8
Copy link
Member Author

Choose a reason for hiding this comment

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

@tariq1890 this could be distroless if required.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ack

@elezar elezar force-pushed the use-ubi8-image branch from 5952a98 to c20024a Compare May 6, 2025 18:37
This change swithces to using a single image for the NVIDIA Container Toolkit contianer.
Here the contents of the architecture-specific deb and rpm packages are extracted
to a known root. These contents can then be installed using the updated installation
mechanism which has been updated to detect the source root based on the packaging type.

Signed-off-by: Evan Lezar <[email protected]>
@elezar elezar force-pushed the use-ubi8-image branch from c20024a to 119a2df Compare May 7, 2025 15:16
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.

3 participants