-
Notifications
You must be signed in to change notification settings - Fork 131
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
Build and publish Docker Hub image for amd64 and arm64 #304
Conversation
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.
Excellent to have you working on this @manics !! ❤️
Thanks for the comments! I've...
|
I'd personally be happy to introduce the GitHub action to calculate the tags to the jupyterhub org btw. @minrk perhaps you can comment on that as well?
I suggest we release CHP 4.4.0 when this is included as well, we have since current 4.3.1 bumped some npm production level dependencies, and i consider this a non-breaking enhancement to associate with a new version motivating a minor version bump.
|
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.
Lgtm!
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.
👍 great! Also happy to trigger a release just for the CI, though I think 4.3.2 is appropriate, as there are no user-facing changes since 4.3.1.
@manics I consider this ready for merge at this point then! Go for a self-merge if you think it's ready as well! |
Let me see if I can move the action to jupyterhub first.... |
@manics nice deal! I'll make a changelog and cut a release once we merge this |
Z2JH requires the Docker image from this repo:
https://github.com/jupyterhub/zero-to-jupyterhub-k8s/blob/08faa6f197e478ca28ce23d083287d37096cbb36/jupyterhub/values.yaml#L191
This PR modifies the npm-publish.yml GitHub workflow to also build the Docker image using Docker buildx for
linux/amd64
andlinux/arm64
. The large diff is due to me renamingnpm-publish.yml
topublish.yml
, the individual commit diffs are clearer. As we've done with other repos I've followed the policy of always running the publish workflow steps for all pushes, with only the actual push/publish step guarded by the tag conditional.This requires the following Docker Hub secrets:
{{ secrets.DOCKERHUB_USERNAME }}
{{ secrets.DOCKERHUB_TOKEN }}
I presume the image is currently built using a Docker Hub automated build, so that will need to be disabled before this is merged (EDIT: done). An additional advantage of doing the build/push in a GitHub workflow instead of with the automated Docker Hub build is that we can easily push to other registries in future.
This uses a new action
manics/action-major-minor-tag-calculator
(currently in an intermediate GitHub org for various boring reasons but it could probably move to JupyterHub if there's agreement 😄) to generate the tags needed for the Docker image from the GitHub tag. E.g.1.2.3
is expanded to Docker tags[1.2.3, 1.2, 1, latest]
unless this is a backported tag in which case the newer tags aren't updated.