Skip to content

Refactor the CUDA container spec #1961

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Sep 19, 2024
Merged

Refactor the CUDA container spec #1961

merged 1 commit into from
Sep 19, 2024

Conversation

fabiendupont
Copy link
Contributor

@fabiendupont fabiendupont commented Aug 2, 2024 •
edited
Loading

This change modifies the Containerfile for NVIDIA CUDA to:

  • Use CentOS Stream 9 as the default base image. UBI9 also works
  • Install the NVIDIA packages from NVIDIA repository for exact CUDA version
  • Add NVIDIA scripts from nvcr.io/nvidia/cuda image for copyrights...
  • Install the Python packages in a Python 3.11 virtualenv
  • Install instructlab package from PyPI
  • Allow specifying the instructlab version at build time
  • Make the final container rootless

Resolves #899
Replaces #932
Replaces #994

@mergify mergify bot added the container Affects containization aspects label Aug 2, 2024
@fabiendupont fabiendupont force-pushed the update-cuda-container branch from e058e69 to 24c9ace Compare August 2, 2024 16:12
bcrochet
bcrochet reviewed Aug 2, 2024
View reviewed changes
@fabiendupont fabiendupont force-pushed the update-cuda-container branch from 24c9ace to 90b3cfb Compare August 5, 2024 07:45
bcrochet
bcrochet reviewed Aug 5, 2024
View reviewed changes
Copy link
Contributor

@bcrochet bcrochet left a comment

Choose a reason for hiding this comment

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

What I've found so far. I'll be running a build shortly to test.

@nathan-weinberg
Copy link
Member

@fabiendupont did you mean "replaces #992"? seems unrelated

@mergify mergify bot added the ci-failure PR has at least one CI failure label Aug 5, 2024
@fabiendupont
Copy link
Contributor Author

@fabiendupont did you mean "replaces #992"? seems unrelated

Thanks. Reference updated.

@fabiendupont fabiendupont force-pushed the update-cuda-container branch from 90b3cfb to 7142e6a Compare August 6, 2024 06:31
@mergify mergify bot added ci-failure PR has at least one CI failure and removed ci-failure PR has at least one CI failure labels Aug 6, 2024
bcrochet
bcrochet suggested changes Aug 7, 2024
View reviewed changes
Copy link
Contributor

@bcrochet bcrochet left a comment

Choose a reason for hiding this comment

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

Just a missing letter...

@fabiendupont fabiendupont force-pushed the update-cuda-container branch from 7142e6a to a21a108 Compare August 7, 2024 11:52
@mergify mergify bot added ci-failure PR has at least one CI failure and removed ci-failure PR has at least one CI failure labels Aug 7, 2024
@fabiendupont fabiendupont force-pushed the update-cuda-container branch from a21a108 to 9584d23 Compare August 7, 2024 13:04
@mergify mergify bot removed the ci-failure PR has at least one CI failure label Aug 7, 2024
@fabiendupont fabiendupont force-pushed the update-cuda-container branch from 9584d23 to 369baf3 Compare August 19, 2024 07:30
@nathan-weinberg nathan-weinberg requested a review from tiran August 19, 2024 19:35
@mergify mergify bot added the ci-failure PR has at least one CI failure label Aug 19, 2024
@seroyer seroyer mentioned this pull request Sep 16, 2024
Closed
6 tasks
@mergify mergify bot added ci-failure PR has at least one CI failure and removed ci-failure PR has at least one CI failure labels Sep 17, 2024
nathan-weinberg
nathan-weinberg approved these changes Sep 18, 2024
View reviewed changes
Copy link
Member

@nathan-weinberg nathan-weinberg left a comment

Choose a reason for hiding this comment

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

one nit but otherwise LGTM

@mergify mergify bot added the one-approval PR has one approval from a maintainer label Sep 18, 2024
@mergify mergify bot added ci-failure PR has at least one CI failure and removed ci-failure PR has at least one CI failure labels Sep 18, 2024
@mergify mergify bot removed the ci-failure PR has at least one CI failure label Sep 18, 2024
@nathan-weinberg nathan-weinberg requested review from a team and removed request for tiran September 19, 2024 13:24
markstur
markstur requested changes Sep 19, 2024
View reviewed changes
Copy link
Member

@markstur markstur left a comment

Choose a reason for hiding this comment

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

typo on var name

@fabiendupont
Refactor the CUDA container spec
69f9113 
This change modifies the Containerfile for NVIDIA CUDA to:
- Use CentOS Stream 9 as the default base image. UBI9 also works
- Install the NVIDIA packages from NVIDIA repository for exact CUDA version
- Add NVIDIA scripts from `nvcr.io/nvidia/cuda` image for copyrights...
- Install the Python packages in a Python 3.11 virtualenv
- Install instructlab package from PyPI
- Allow specifying the instructlab version at build time
- Make the final container rootless

Resolves instructlab#899
Replaces instructlab#992
Replaces instructlab#994

Signed-off-by: Fabien Dupont <fdupont@redhat.com>
@mergify mergify bot merged commit e189f4e into instructlab:main Sep 19, 2024
9 checks passed
@mergify mergify bot removed the one-approval PR has one approval from a maintainer label Sep 19, 2024
@ktdreyer ktdreyer mentioned this pull request May 1, 2025
Closed
@compi-migui compi-migui mentioned this pull request May 30, 2025
Merged
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@markstur markstur markstur approved these changes

@nathan-weinberg nathan-weinberg nathan-weinberg approved these changes

+1 more reviewer

@bcrochet bcrochet bcrochet requested changes

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
container Affects containization aspects
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Container ships build dependencies
4 participants
@fabiendupont @nathan-weinberg @bcrochet @markstur