feat(gha): add docker push github action#1116
feat(gha): add docker push github action#1116georgettica wants to merge 1 commit intocrate-ci:masterfrom
Conversation
|
this is not tested fully, but I am hopeful this just works. |
|
@epage , this is my "throw this and check if it sticks" I added the manual trigger if you want to play with it manually, before pushing to continuus-deployment if you want me to integrate with other github actions I can spend time on this too |
|
As this is a completely new workflow to me, posting an Action is insufficient. I specifically asked for documentation. I need to understand the workflow and the requirements it places on me. |
|
|
||
| steps: | ||
| - name: Checkout code | ||
| uses: actions/checkout@v4 |
There was a problem hiding this comment.
Maybe only fetch 1 depth is sufficient.
There was a problem hiding this comment.
did not find a way to change it to the way you want. I saw the default fetch-depth is 1
| - name: Build and push Docker image | ||
| uses: docker/build-push-action@v6 | ||
| with: | ||
| context: . | ||
| push: true | ||
| tags: ghcr.io/${{ github.repository_owner }}/${{ github.event.repository.name }}:${{ steps.get_tag.outputs.tag }} |
There was a problem hiding this comment.
We also need to build multi-architecture images alongside the CLI release
There was a problem hiding this comment.
ill do it along the weekend 🍾
There was a problem hiding this comment.
my weekend was not very useful, so found time to push this now
|
oh sorry @epage , I will create the documentation on how this workflow works in addition to the changes requested in the review |
|
@weijiany care to check on your machine? ( I use podman so I just replaced it with docker hoping it just works ) |
|
It is worth noting ChatGPT is a good friend of mine, so he is a co-author. I hope this is ok by you ❤️ |
Hi @georgettica, it works well. ✅ BTW, I am not the repository's maintainer; I am simply a user who wishes to directly utilize the image. |
| tags: ghcr.io/${{ github.repository_owner }}/${{ github.event.repository.name }}:${{ steps.get_tag.outputs.tag }} | ||
| # platforms: linux/arm64/v8,darwin/amd64,linux/amd64,windows/amd64,linux/amd64 # was what I was aiming for | ||
| # but locally I only got to these 3 (specifically linux/arm64/v8 but yeah) | ||
| platforms: linux/arm64,linux/amd64 |
There was a problem hiding this comment.
TBH, I didn't work with on Github action to build multi-arch. But in some pipeline tool that I have used, a question is needn't we to install qemu before building image?’
There was a problem hiding this comment.
That makes sense. I can add it, but I am adding the required tools inside the docker, so I hope it "just works".
|
PS, I accidentally had one of the tools twice: |
|
@weijiany, I guess the only other thing I want to ask so we can move this PR forward is your thoughts and opinions on the documentation I cobbled up. |
For me, the doc is quite clear, possibly because I've reviewed your changes by going through the GitHub Actions documentation. 😅 |
|
Well, at least it's clear. I hope it'll be ok once the actual review comes around |
|
@epage, can I get 👀 ? (sorry for pinging, I don't want this PR to go to waste) |
|
You are in my queue of prs and issues to get to. Be sure the commits are all atomic and organized for how you want me to review and merge this. |
91f9eb2 to
0a1a113
Compare
|
I made them into one big blob, as they weren't atomic and some were patch fixes. I am ok with all the PR not passing as it's one atomic change. |
Pull Request Test Coverage Report for Build 11464483713Details
💛 - Coveralls |
|
I'm looking forward to this landing, because that'd make it much, much easier to install COPY --from=ghcr.io/crate-ci/typos:latest /typos /bin/instead of installing I see your commit message failed the linter; should be easy to fix: git commit --amend --message 'feat(gha): Add docker push github action'
git force --push |
0a1a113 to
f629cac
Compare
|
took me a while, but let's hope this passes 🤞 |
|
@mjpieters I would be really happy if this works, lets see though |
| release: | ||
| types: [published] |
There was a problem hiding this comment.
Building of binaries is kicked off from a tag, rather than release. Should we do that here as well?
typos/.github/workflows/post-release.yml
Lines 16 to 21 in 0d9e0c2
| id: get_tag | ||
| run: | | ||
| if [ "${{ github.event_name }}" == "release" ]; then | ||
| echo "tag=${GITHUB_REF#refs/tags/}" >> $GITHUB_OUTPUT |
There was a problem hiding this comment.
If we do this off of a tag, could we simplify this to
typos/.github/workflows/post-release.yml
Line 43 in 0d9e0c2
| # platforms: linux/arm64/v8,darwin/amd64,linux/amd64,windows/amd64,linux/amd64 # was what I was aiming for | ||
| # but locally I only got to these 3 (specifically linux/arm64/v8 but yeah) | ||
| platforms: linux/arm64,linux/amd64 |
There was a problem hiding this comment.
This feels like a during-development note and I'd like it resolved one way or the other
There was a problem hiding this comment.
My PC is MacOS, so I don't have much wiggle room for testing.
I would give it a go if I had more computers with different platforms.
| @@ -1,11 +1,82 @@ | |||
| ARG DEBIAN_DIST=bullseye | |||
| # syntax=docker/dockerfile:1.10 | |||
There was a problem hiding this comment.
This is a complete rewrite of the dockerfile
Can any of this be broken out into smaller (complete) steps (ie commits) to make this easier to follow along?
There was a problem hiding this comment.
Sure, I can do that, sorry for the mess. This process is more complex for me so I opted to give one big commit and break it into tiny bits if required (mostly because my work was based inside.
There was a problem hiding this comment.
@epage you are correct, but the reason I did this is to allow compiling the code to multiple architectures based on TARGETPLATFORM arg (filled by the --platform flag in docker build)
| @@ -0,0 +1,219 @@ | |||
| # Build and Push Docker Image Workflow Documentation | |||
There was a problem hiding this comment.
When I said that I wanted it documented, I meant a write up in the issue or PR so I could understand the expectations behind this. Unsure how much of this will be useful in a living document. If there is, it likely more belongs in CONTRIBUTING.md
There was a problem hiding this comment.
sure, I will move it there, as moving documentation is easier than rewriting.
There was a problem hiding this comment.
Based on the understanding from #1116 (comment), I don't think we need to keep this documentation.
| - **GitHub Permissions**: The `GITHUB_TOKEN` provided by GitHub Actions must have the necessary permissions (default settings typically suffice). | ||
| - **Understanding of Docker and GHCR**: Basic knowledge of Docker image building and pushing to GHCR will be beneficial. |
There was a problem hiding this comment.
This was the core of what I was wanting to understand.
If I get the gist of it, instead of us pushing to docker hub, we are pushing to github's container registry which allows us to just use GH_TOKEN without having to setup any count, register secrets, and/or do any manual per-release action.
Is that right?
There was a problem hiding this comment.
yes, that is the gist.
I think GH_TOKEN is if you run it manually, so most times the action won't need even permissions to run.
|
Worth noting to all I am not quick with PR modifications, but I hope this will soon go through another cycle (and I will ping all of you when it is) |
|
seems this PR is getting a lot of traction and I am not getting enough time to sit on it. if anyone wants to grab what I have so far and push it across the finish line be my guest |
fixes #427