-
Notifications
You must be signed in to change notification settings - Fork 813
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
Publish Arm64 compatible images #2125
Conversation
This ensures the image can be built with plain `docker build`
context: images/${{ matrix.image }} | ||
platforms: linux/amd64,linux/arm64 | ||
push: true | ||
tags: ghcr.io/${{ github.repository_owner }}/z2jh-${{ matrix.image }}:dev |
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.
Ohhh wow how nice and clean these steps are.
We have some complexity if we want to use this instead of build args and a dynamic tag based on chartpress.yaml and git inspection logic in chartpress. This looks great though.
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.
This is just a quick and easy way to expose the built images for download. I should have a chartpress PR to replace some of this soon 😸
Remaining requirements:
|
@consideRatio Do you want me to keep this new workflow? Or get rid of it and add the flags to:
|
I'm not sure, I'd like the idea of having the logic to publish images in a single workflow file I think. What is your take? |
Just realised the
|
Thinking about this a bit more now...
I'm very afraid of duplicating the test suite and really want to avoid breaking DRY in this case. I'm hoping we can have a arm64 based (self-hosted / circle-ci hosted) github actions runner or similar. If we assume that is plausible to accomplish, I'd be leaning towards:
|
Our test-chart workflow as it is already contains a build through chartpress, and it won't rebuild if it finds an existing image - but, the existing image will be in docker.io/jupyterhub/k8s-... rather than a local registry. Hmm... I would be positive to have a dedicated build job if we don't just build in that dedicated build job AND the test-chart job that also builds images unless they are existing already. I guess the ideal outcome given the complexity tradeoff doesn't make it non-ideal would be to:
|
@manics nice work on this! The CHP images are published like a charm and I love the new actions job to extract relevant tags! |
I've updated the publish workflow to (hopefully) build and push the multiple architectures, and removed the push flag from the docker build workflow. I don't think configuring circle-ci is too bad. The aim of the arm64 workflow will be to test the images not the helm chart, so we can skip most of the complexity. I've made a start in https://github.com/manics/zero-to-jupyterhub-k8s/blob/arm64-circleci/.circleci/config.yml Removing pebble and network policies removes a lot of the complex setup. It'll require a flag in the tests to allow non-https requests to be made, but I think that's worthwhile anyway since it makes it easier to developers to test locally. If we want we could use this as an opportunity to test some other config that's not already tests. |
Wieee nice work on this @manics! What an effort! |
Part 1 of #2119 (comment): Minor changes to get Docker images building on ARM64
hub
:psycopg2-binary
is not available as a precompiled binary, solibpq-dev
needs to be installed for compilation. This increases the build time and the image size. An alternative would be to omit PostgreSQL support from the arm64 image, what do you think?secret-sync
:tini
for the correct architecture needs to be downloaded.This contains a temporary workflow just for testing, this will be removed. In the meantime you can pull the test images from
docker pull ghcr.io/manics/z2jh-{hub,image-awaiter,network-tools,secret-sync}:dev
. Not yet tested in a deployment