-
-
Notifications
You must be signed in to change notification settings - Fork 26
Use Zstd compression for published images to reduce their size and speedup decompression #220
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -171,12 +171,30 @@ jobs: | |
| rm -rf /tmp/.docker-cache | ||
| mv /tmp/.docker-cache-new /tmp/.docker-cache | ||
| - name: 🚀 Push | ||
| # yamllint disable rule:line-length | ||
| run: | | ||
| docker push \ | ||
| "ghcr.io/${{ github.repository_owner }}/${{ needs.information.outputs.slug }}/${{ matrix.architecture }}:${{ needs.information.outputs.environment }}" | ||
| docker push \ | ||
| "ghcr.io/${{ github.repository_owner }}/${{ needs.information.outputs.slug }}/${{ matrix.architecture }}:${{ needs.information.outputs.version }}" | ||
| uses: docker/build-push-action@v6.18.0 | ||
| with: | ||
| outputs: type=image,oci-mediatypes=true,compression=zstd,push=true | ||
| # yamllint disable rule:line-length | ||
| tags: | | ||
| ghcr.io/${{ github.repository_owner }}/${{ needs.information.outputs.slug }}/${{ matrix.architecture }}:${{ needs.information.outputs.environment }} | ||
| ghcr.io/${{ github.repository_owner }}/${{ needs.information.outputs.slug }}/${{ matrix.architecture }}:${{ needs.information.outputs.version }} | ||
| # yamllint enable rule:line-length | ||
| context: ${{ needs.information.outputs.target }} | ||
|
Comment on lines
+174
to
+182
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We do not need to build it again, this step only pushes out the tags.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, but I need to invoke
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe you want to revise your decision about splitting build and push? That would remove lots of copy-pasting. Is it really a common case when build succeeds but pushing fails and you still want to save the cache?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
We can use docker directly.
Nope. I explained the reasoning earlier. I think think being able to really on cache is better than the speed bump added in this PR. It is a nice improvement, but not something I think is important enough to drop other things for. In the end, if this really was the best and critical option, it would have been the default in Docker.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In that case, I am going to friendly decline this contribution. Honestly, the juice is not worth the squeeze imho. Especially, since it doesn’t solve an actual issue we are experiencing. Nevertheless, thanks for being willing to contribute. ../Frenck |
||
| file: ${{ needs.information.outputs.target }}/Dockerfile | ||
| cache-from: | | ||
| type=local,src=/tmp/.docker-cache | ||
| ghcr.io/${{ github.repository_owner }}/${{ needs.information.outputs.slug }}/${{ matrix.architecture }}:edge | ||
| cache-to: type=local,mode=max,dest=/tmp/.docker-cache-new | ||
| platforms: ${{ steps.flags.outputs.platform }} | ||
| build-args: | | ||
| BUILD_ARCH=${{ matrix.architecture }} | ||
| BUILD_DATE=${{ steps.flags.outputs.date }} | ||
| BUILD_DESCRIPTION=${{ needs.information.outputs.description }} | ||
| BUILD_FROM=${{ steps.flags.outputs.from }} | ||
| BUILD_NAME=${{ needs.information.outputs.name }} | ||
| BUILD_REF=${{ github.sha }} | ||
| BUILD_REPOSITORY=${{ github.repository }} | ||
| BUILD_VERSION=${{ needs.information.outputs.version }} | ||
|
|
||
| publish-edge: | ||
| name: 📢 Publish to edge repository | ||
|
|
||
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'm not following why this part needs to go?
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.
Because push is done through
outputs:parameter todocker/build-push-actionstep.