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

Add Docker Buildx for multi-arch image building. #57

Merged
merged 3 commits into from
Dec 24, 2020

Conversation

davidjameshowell
Copy link
Contributor

This should resolve #52. I am using Docker BuildX with QEMU to cross build images for all major platforms. Node has availability on all these images, so we all amd64, arm64, arm v7, arm v6 are all supported.

I chose to build on tags, since I see you tag releases but ultimately reference the git sha of the tagged commit to ensure immutability.

Let me know your thoughts on this!

@claabs
Copy link
Owner

claabs commented Dec 23, 2020

Looks good. I agree with the SHA tags, the releases are only to notify people if they're watching releases on Github.

The only issue I see is due to the fact that we're using an hjson package release from Github releases (https://github.com/claabs/epicgames-freegames-node/blob/master/Dockerfile#L36).
I'll need to either find it in Alpine APK, add it to Alpine APK, or parameterize the Dockerfile for arch.

Edit: or I could just use the CLI for this, since we have Node installed already: https://www.npmjs.com/package/json5

Copy link
Owner

@claabs claabs left a comment

Choose a reason for hiding this comment

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

I just pushed the hjson fix. Here's some changes I'd like before merging.

.github/workflows/build_docker_image.yml Outdated Show resolved Hide resolved
.github/workflows/build_docker_image.yml Outdated Show resolved Hide resolved
@davidjameshowell
Copy link
Contributor Author

Sounds good. I wasn't sure how you want to handle builds in the sense of per commit or per tag. Thought it would be a good baseline. Will update PR with effective changes.

@davidjameshowell
Copy link
Contributor Author

The only other question I have is if you want to replace the tags username with your actual username to have it clearly displayed in the build log. Otherwise, it will asterisks it. Not a huge deal, more of a quality of life change for readability,

@claabs
Copy link
Owner

claabs commented Dec 24, 2020

Yep, username is fine so we can eliminate the asterisks

…fied Docker Hub repo name to match production.
@davidjameshowell
Copy link
Contributor Author

Updated to hardcode username in as well as fixed the repo name on Docker Hub to what it currently is (missed that on the original review update).

@claabs claabs merged commit 8d7bba6 into claabs:master Dec 24, 2020
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.

ARM Docker Image Support?
2 participants