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

Setup the Docker GH Action for Matrix Building #2644

Merged
merged 1 commit into from
Nov 16, 2023

Conversation

misilot
Copy link
Contributor

@misilot misilot commented Nov 16, 2023

This PR enables building of the amd64 and arm64 images simultaneously.

References

Logic borrowed from https://docs.docker.com/build/ci/github-actions/multi-platform/#distribute-build-across-multiple-runners

Description

Once both images finish, the manifest is sent to Docker Hub, allowing for a single image that has both the amd64/arm64 images.

This will allow for faster building of the amd64 image, and being sent to Docker Hub, while the arm64 image is still being built.

Instructions for Reviewers

Please add a more detailed description of the changes made by your PR. At a minimum, providing a bulleted list of changes in your PR is helpful to reviewers.

List of changes in this PR:

  • Creates a matrix for the target arch (amd64, arc64).
  • Merges the two docker images and pushes the manifest to Docker Hub at completion

Include guidance for how to test or review your PR. This may include: steps to reproduce a bug, screenshots or description of a new feature, or reasons behind specific changes.

Checklist

This checklist provides a reminder of what we are going to look for when reviewing your PR. You need not complete this checklist prior to creating your PR (draft PRs are always welcome). If you are unsure about an item in the checklist, don't hesitate to ask. We're here to help!

  • My PR is small in size (e.g. less than 1,000 lines of code, not including comments & specs/tests), or I have provided reasons as to why that's not possible.
  • My PR passes ESLint validation using yarn lint
  • My PR doesn't introduce circular dependencies (verified via yarn check-circ-deps)
  • My PR includes TypeDoc comments for all new (or modified) public methods and classes. It also includes TypeDoc for large or complex private methods.
  • My PR passes all specs/tests and includes new/updated specs or tests based on the Code Testing Guide.
  • If my PR includes new libraries/dependencies (in package.json), I've made sure their licenses align with the DSpace BSD License based on the Licensing of Contributions documentation.
  • If my PR includes new features or configurations, I've provided basic technical documentation in the PR itself.
  • If my PR fixes an issue ticket, I've linked them together.

This change enables building of the amd64 and arm64 images simultaneously.

Once both images finish, the manifest is sent to Docker Hub, allowing for a single image that has both the amd64/arm64 images.
@tdonohue tdonohue added code task 1 APPROVAL pull request only requires a single approval to merge port to dspace-7_x This PR needs to be ported to `dspace-7_x` branch for next bug-fix release labels Nov 16, 2023
@tdonohue
Copy link
Member

tdonohue commented Nov 16, 2023

Thanks @misilot ! I found this looks to be borrowed from https://docs.docker.com/build/ci/github-actions/multi-platform/#distribute-build-across-multiple-runners . I went searching around as I was finding the logic of the "merge" step a bit difficult to follow. But I see that's the recommended approach from Docker themselves.

It's a shame this is so complex to achieve, but I agree that the benefits are wonderful. I'll see if I can get this merged soon so that we can test it out. We'll also need to create a version of this for DSpace/DSpace. I might be able to help with that if I find time (and you don't beat me to it).

@misilot
Copy link
Contributor Author

misilot commented Nov 16, 2023

@tdonohue yup, I think that is where I borrowed from! It was a while go that I found it.

@tdonohue tdonohue added the backend: Docker related to DSpace deployment via Docker label Nov 16, 2023
Copy link
Member

@tdonohue tdonohue left a comment

Choose a reason for hiding this comment

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

👍 Thanks @misilot ! Looks good and makes sense now that I look at the Docker recommendations (see links above). The only way to fully test it is to merge it. So, I'm going to merge it now. (I was already planning on doing some minor GitHub Actions cleanup tomorrow to prep for 8.0, so this is good timing!)

@tdonohue tdonohue merged commit b0deccc into DSpace:main Nov 16, 2023
13 checks passed
@tdonohue tdonohue added this to the 8.0 milestone Nov 16, 2023
@dspace-bot
Copy link
Contributor

Successfully created backport PR for dspace-7_x:

@tdonohue
Copy link
Member

Ported to dspace-7_x in #2655

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1 APPROVAL pull request only requires a single approval to merge backend: Docker related to DSpace deployment via Docker code task
Projects
No open projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

3 participants