-
Notifications
You must be signed in to change notification settings - Fork 101
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
chore: build & publish multi-platform image(linux/arm64, linux/adm64) Docker images #1528
base: master
Are you sure you want to change the base?
Conversation
I tested in OS and Linux, and I was able to pull the docker image with the right architecture locally. Used:
Working Github action example https://github.com/MobilityData/gtfs-validator/actions/runs/5464582100 |
✅ Rule acceptance tests passed. |
✅ Rule acceptance tests passed. |
fi | ||
echo "Set DOCKER_TAGS=${DOCKER_TAGS}" | ||
|
||
echo ::set-output name=version::${AXION_VERSION} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
::set-output is deprecated.
Here is the suggestion for replacement: https://github.blog/changelog/2022-10-11-github-actions-deprecating-save-state-and-set-output-commands/
But they say they postponed removal because of significant use. So I guess it's up to you to change it or not.
echo ::set-output name=tags::${DOCKER_TAGS} | ||
echo ::set-output name=created::$(date -u +'%Y-%m-%dT%H:%M:%SZ') | ||
- name: Build Docker container image | ||
uses: docker/build-push-action@v3 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you explain why it seems to build an image twice, one here and once in "Push Docker container image" below. The two command lines to build the images are identical (obtained form the logs) except for the --push
runs-on: ubuntu-latest | ||
strategy: | ||
matrix: | ||
platform: [amd64 , arm64] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem running this action (see https://github.com/MobilityData/gtfs-validator/actions/runs/5833255963)
I suspect it's because with the matrix we build 2 images but give them the same tag (ghcr.io/mobilitydata/gtfs-validator:latest, see line 95 below).
Since it appears the matrix strategy runs in parallel for the 2 platforms, this could cause a race condition and it will fail or not depending on the time it takes to create and put the images in the local registry.
I suggest you add a suffix to the DOCKER_TAGS variable on line 95 below. Something like:
DOCKER_TAGS="${DOCKER_IMAGE}.${{ matrix.platform }}:latest"
And see what happens.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tried it and the problem is still present. Although I think it would not hurt to keep this fix.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to run the arm64 version on an arm64 platform, or use emulation.
Summary:
Alternative solution for #1367. This PR addresses the issue related with no supported multi-platform docker with load parameter (PR comment, docker/buildx#59).
Core changes
gradle:7-jdk11-alpine
docker builder image witheclipse-temurin:11
asgradle:7-jdk11-alpine
don't support arm64 architecture.Expected behavior:
A multi-platform docker image is published.
Please make sure these boxes are checked before submitting your pull request - thanks!
gradle test
to make sure you didn't break anything