-
Notifications
You must be signed in to change notification settings - Fork 129
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 multi-architecture images #197
Conversation
# A workflow run is made up of one or more jobs that can run sequentially or in parallel | ||
|
||
env: | ||
CI: true |
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.
annotation: This is temporary until we get the whole test suite refactored. For now we skip the image build step in the test script since this pipeline ensures that the correct images are available.
.github/workflows/main.yml
Outdated
uses: actions/upload-artifact@v3 | ||
with: | ||
name: oss | ||
path: /tmp/oss.tar |
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.
annotation: The other two images use this as a base. Therefore we export the built image for use in:
- The tests for this image
- The build process for the
latest-njs
andunprivileged
images
- name: Set up Docker Buildx | ||
uses: docker/setup-buildx-action@v3 | ||
with: | ||
driver: docker |
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.
annotation:
If we use the docker-container
driver that buildx uses by default, a docker load
does not get the image in a context where the build process can access it. In order to do this you must use a local registry as detailed in this guide: https://docs.docker.com/build/ci/github-actions/named-contexts/#using-with-a-container-builder
tags: | | ||
ghcr.io/${{ github.repository }}/nginx-oss-s3-gateway:latest-${{ steps.date.outputs.date }} | ||
ghcr.io/${{ github.repository }}/nginx-oss-s3-gateway:latest | ||
nginxinc/nginx-s3-gateway:latest-${{ steps.date.outputs.date }} | ||
nginxinc/nginx-s3-gateway:latest |
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.
annotation:
This is nice, the plugin lets you log in to both registries then pushes tot the correct one based on the tag.
context: . | ||
push: true | ||
platforms: linux/amd64,linux/arm64 | ||
provenance: false |
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.
annotation:
This is a little workaround for the github packages repo. The provenance
file is a docker thing not an OCI thing so you'll get an unknown/unknown
architecture listed in gh packages if you don't set this.
ref: https://docs.docker.com/build/attestations/slsa-provenance/
If we want these to go to dockerhub, we could also separate the builds again but I think the simplicity of having them both push is more desireable.
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.
Looks good. I'm looking forward to seeing this merged.
.github/workflows/main.yml
Outdated
uses: actions/download-artifact@v3 | ||
with: | ||
name: oss | ||
path: /tmp |
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 would check to see if github actions has defined an environment variable for the temp path. Generally, it isn't great to assume that /tmp
is the system temp directory. Yes, I know it is in this case and this is a nit.
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.
Thank you for this note. I discovered ${{ runner.temp }}
which serves the purpose well.
.github/workflows/main.yml
Outdated
uses: actions/upload-artifact@v3 | ||
with: | ||
name: unprivileged | ||
path: /tmp/unprivileged.tar |
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.
Refresh my memory if the uploaded artifacts eventually get cleaned up. I have a vague memory that they do.
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's a default setting for the repo which I believe is 30 days. However based on this comment I'm going to update the retention explicitly since so it's obvious from the script.
retention-days: 1
One day is the minimum which is really too long for this but should serve us fine.
Also worth noting that artifacts are only accessible from the workflow in which they were created so there's no chance of us pulling an old artifact which I was worried about initially.
…200) The original pull request #197 had an issue where the final build and push could not find the base image locally. This change corrects the issue but introducing a local registry to the final build push step as described in https://docs.docker.com/build/ci/github-actions/named-contexts/#using-with-a-container-builder since the `docker-container` driver-based build can't load from the local docker environment.
What
Allows us to push multi-architecture builds for
linux/amd64
andlinux/arm64
on merge to master. Fixes: #196The flow looks like this:
Tests are performed by building the relevant image and tagging it with the image name the tests expect. This follows the original implementation in
test.sh
. This will likely change to an explicit image name specification as we move the test suite in to Javascript but it's being maintained for now in the interest of simplicity.Goals
Notes
unprivileged
andlatest-njs
images build off the baseoss
image, there were some strange moves that had to be done to make that work without rebuilding the base image every time. Specifically, we had to setsetup-buildx-action
to run indocker
driver mode so we can simplyload
the base image. Otherwise we would have had to use a local repository. However, indocker
modeupload-artifact
doesn't like the file produced so we have to save the file again.