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

feat: build nvidia images in main repo #319

Merged
merged 14 commits into from
Aug 29, 2023
Merged

Conversation

bsherman
Copy link
Contributor

@bsherman bsherman commented Aug 29, 2023

This adds the nvidia build scripts to the repo and reconfigures Containerfile to build both FOO-main and FOO-nvidia images per matrix provided build-args. Workflow matrix was modified to support this use case, including changes to variables and tag generation.

Reviewers: please double-check the generated tags look correct per image. I did review and put some effort into this, but would appreciate extra eyes.

Changes which were not technically required:

  • "main" build.sh, post-install.sh, packages.json renamed (with main- prefix) to make things more consistent and easier to follow for future
  • switched to COPY instead of ADD in Containerfile for compliance with best practices
  • added retry logic to curl in github-release-install.sh to avoid some observed intermittent errors

This adds the nvidia build scripts to the repo and reconfigures
Containerfile to build both a main and nvidia image per provided
build-args. Workflow is also adapted with a matrix to support this use
case. "main" build and post-install scripts renamed to make things more
consistent and easier to follow for future dev. COPY used instead of ADD
in Containerfile for best practice compliance.
@bsherman bsherman linked an issue Aug 29, 2023 that may be closed by this pull request
@bsherman bsherman marked this pull request as ready for review August 29, 2023 21:15
@bsherman
Copy link
Contributor Author

After merging, we must disable actions on the https://github.com/ublue-os/nvidia/ repo, and probably post a message to the README and/or archive the repo.

@castrojo
Copy link
Member

iirc archiving the repo will turn off the actions in one step.

@bsherman
Copy link
Contributor Author

iirc archiving the repo will turn off the actions in one step.

Yes but do we want a message on the readme or no?

@castrojo
Copy link
Member

Yeah, a signpost would help, I can work on that. You keep going on this.

@bsherman
Copy link
Contributor Author

You keep going on this.

I already added main repo as "Admin" to the container packages (*-nvidia) so the pushes should succeed.

I think this is ready as soon as y'all approve.

@bsherman bsherman added this pull request to the merge queue Aug 29, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Aug 29, 2023
Copy link
Member

@p5 p5 left a comment

Choose a reason for hiding this comment

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

LGTM 🤞

Copy link
Member

@p5 p5 Aug 29, 2023

Choose a reason for hiding this comment

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

(Unrelated)
It might be a good idea to bundle these Nvidia install and post-install scripts into the akmods image instead of requiring downstream consumers to copy, paste and keep them maintained.

We can keep them up-to-date in the akmods repo, then if people are consuming the akmods repo rather than main images, they have the scripts bundled with the image to call.

Send the horse. Full attack.
@bsherman bsherman merged commit 40e6c5d into main Aug 29, 2023
0 of 39 checks passed
@bsherman bsherman deleted the build-nvidia-images-in-main branch August 29, 2023 23:10
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.

Move *-nvidia image build workflow into main repo
3 participants