-
Notifications
You must be signed in to change notification settings - Fork 138
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
docker: Add arm64 builds to Dockerfile and CI #513
base: main
Are you sure you want to change the base?
Conversation
cache-from: type=local,src=/tmp/.build-cache | ||
cache-to: type=local,dest=/tmp/.build-cache,mode=max | ||
|
||
# WORKAROUND: |
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.
In future, could we work around this with a registry in the runner ? We should really run tests on both docker images :-)
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.
Yes this would resolve the issue. It also makes it really easy for a reviewer to test a PR's content within a container. With a proper retention policy the containers can be cleaned up afterwards. Alternatively a hook on PR close/merge could do the cleanup.
Probably something which can be linked with go-debos/test-containers#4
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.
I created #514 to track the unit tests not being run on arm64. I don't think we should block on that for merging this PR.
We only have amd64 runners, I think. |
Convert Dockerfile so multiple target architecturebuilds are possible. Also modify the CI pipeline, in order to built a single multi-platform container. Note that at the current time-being docker cannot export multi-platform containers without an intermediate registry. Thus only the amd64 image is exported for the test stage. Nevertheless the arm64 image is built in the pipeline and also published later to the registry. Co-authored-by: Thomas Böhler <t.boehler@kunbus.com> Signed-off-by: Nicolai Buchwitz <n.buchwitz@kunbus.com>
d77997b
to
a118f4d
Compare
GitHub deprecated the v1 comands of compose in their images, so all jobs needs to be migrated to v2 (eg. replace docker-compose with docker compose). This will fix the unit test job. See [1] for more details. [1] https://github.com/orgs/community/discussions/116610 Signed-off-by: Nicolai Buchwitz <nb@tipi-net.de>
853e91a
to
ef7a3ba
Compare
All of the review feedback should have been addressed with the latest patch series. I was also able to figure out, why the unit tests failed earlier. Turns out this was unrelated to my modifications and more an issue with GitHub switching over from v1 to v2 compose api. Fixed this in another commit. CI works as expected (in my fork): https://github.com/nbuchwitz/debos/actions/runs/10537327064 |
The failed job in https://github.com/go-debos/debos/actions/runs/10537327383/job/29218707965?pr=513#step:6:3778 looks like the arch server was unresponsive during curl run. Could you please restart the job? |
Follow-up of @tboehler1 's #505 as suggested by @obbardc
Convert Dockerfile so multiple target architecturebuilds are possible. Also modify the CI pipeline, in order to built a single multi-platform container.
Note that at the current time-being docker cannot export multi-platform containers without an intermediate registry. Thus only the amd64 image is exported for the test stage. Nevertheless the arm64 image is built in the pipeline and also published later to the registry.
If the go-debos project has access to GitHub large runners, the pipeline could be reworked in order to build the container on a native arm runner. This would bring a significant speed improvement and also would allow for testing the arm container. But for know this solution should be sufficient as it tests the build on both architectures and runs the test suite on amd64.