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

Build and publish Docker Hub image for amd64 and arm64 #304

Merged
merged 6 commits into from
Apr 12, 2021

Conversation

manics
Copy link
Member

@manics manics commented Apr 10, 2021

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 and linux/arm64. The large diff is due to me renaming npm-publish.yml to publish.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.

Copy link
Member

@consideRatio consideRatio left a 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 !! ❤️

.github/workflows/npm-publish.yml Outdated Show resolved Hide resolved
.github/workflows/npm-publish.yml Outdated Show resolved Hide resolved
.github/workflows/npm-publish.yml Outdated Show resolved Hide resolved
@consideRatio
Copy link
Member

consideRatio commented Apr 11, 2021

Thanks for the comments!

I've...

  • added the DOCKERHUB_USERNAME and DOCKERHUB_TOKEN and made our bot account have a dedicated access token for this repo
  • given the bot account read/write permissions to the image named jupyterhub/configurable-http-proxy
  • disabled the hub.docker.com's autobuild functionality so that we will rely on this workflow to do the job for us

@consideRatio
Copy link
Member

consideRatio commented Apr 11, 2021

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?

  • Action point: take a decision to move the action to jupyterhub org or not

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.

  • Action point: reach agreement to cut a version: 4.3.2 decided!

Copy link
Member

@consideRatio consideRatio left a comment

Choose a reason for hiding this comment

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

Lgtm!

Copy link
Member

@minrk minrk left a 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.

@consideRatio
Copy link
Member

@manics I consider this ready for merge at this point then! Go for a self-merge if you think it's ready as well!

@manics
Copy link
Member Author

manics commented Apr 12, 2021

Let me see if I can move the action to jupyterhub first....

@consideRatio
Copy link
Member

@manics nice deal! I'll make a changelog and cut a release once we merge this

.github/workflows/publish.yml Outdated Show resolved Hide resolved
.github/workflows/publish.yml Outdated Show resolved Hide resolved
@manics manics marked this pull request as ready for review April 12, 2021 11:11
@manics manics changed the title WIP: Build and publish Docker Hub image for amd64 and arm64 Build and publish Docker Hub image for amd64 and arm64 Apr 12, 2021
@consideRatio consideRatio merged commit f5baffe into jupyterhub:main Apr 12, 2021
@welcome
Copy link

welcome bot commented Apr 12, 2021

Congrats on your first merged pull request in this project! 🎉
congrats
Thank you for contributing, we are very proud of you! ❤️

@consideRatio
Copy link
Member

image

Beuatiful!!!

@manics manics deleted the docker-buildx branch April 12, 2021 17:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants