docker
Composite Docker action
Hi guys,
this is not really an issue, but some feedback that you hopefully consider: I'm aware I probably sound like a whiny crybaby, but please believe me that I admire your efforts... but seriously: with v2 you raped a good working Github action.
And yes yes, you're probably thinking: Why don't you fix it then yourself? But I'm already managing two dozen repositories for the FAForever project and I just need an easy and working CI infrastructure. [And v1 is still available, so technically no problem.]
Why do I use this action? I prefer Github actions because they are easier to read and simplified building blocks over writing it in a shell script, even if they are a few (!) lines longer
And v1 of this action is the perfect solution for the workflow I need (with some Github if expression magic):
- name: Build and push Docker images
if: github.ref == 'refs/heads/develop' || startsWith(github.ref, 'refs/tags')
uses: docker/[email protected]
with:
username: ${{ secrets.DOCKER_USERNAME }}
password: ${{ secrets.DOCKER_PASSWORD }}
repository: myproject/myrepo
tag_with_ref: true
What has changed? With v2 I need to split it off in 3 steps (+ an additional one considering #100):
- one step declaring the tag I want to push
- one step for a login (yes, if I want to push I ALWAYS want to login)
- one step for the build & push
Eventually I end up with 3x the lines of code that I had before. No thanks, I rather stick with the previous version or go back to manual shell commands.
Here are some ideas to improve on v2:
- I do understand that it makes sense to have a login action if you want to pull images from a protected repository. But why does this have to affect the build-and-push step? If already logged in for other reason, this is the special case not the default, so my proposal would be to either ignore this case (double login then) or to offer an login opt-out config flag instead.
- I do understand that the
tag_with_refflag was quirky and limits some scenarios like pushing multiple tags and I actually like that there is another way now to configure this now. But was it necessary to drop the existing flag? Maybe you can consider bringing it back under a more meaningful name. - Looking at the upgrade guide it seems that
contextandfileare mandatory now (didn't test it actually) but then again: why would I need to declare what is the 99% default case? So if it's not mandatory I'd to remove it from the upgrade document, and if it is mandatory I'd suggest to offer default settings for a classic Dockerfile build
I'd love to see a very powerful action as it is now, with sensible default settings preconfigured for the simple use cases.
Love you guys! Keep up the good work! :heart:
Hi @Brutus5000, thanks for your feedback!
I do understand that it makes sense to have a login action if you want to pull images from a protected repository. But why does this have to affect the build-and-push step? If already logged in for other reason, this is the special case not the default, so my proposal would be to either ignore this case (double login then) or to offer an login opt-out config flag instead.
Actually it makes sense to have a dedicated login action because you can define tags affecting multiple registries which was not possible in v1. I'll give you an example with an image that will be published on both Docker Hub and GHCR:
# v2
steps:
-
name: Checkout
uses: actions/checkout@v2
-
name: Set up Docker Buildx
uses: docker/setup-buildx-action@v1
-
name: Login to DockerHub
uses: docker/login-action@v1
with:
username: ${{ secrets.DOCKERHUB_USERNAME }}
password: ${{ secrets.DOCKERHUB_TOKEN }}
-
name: Login to GitHub Container Registry
uses: docker/login-action@v1
with:
registry: ghcr.io
username: ${{ github.repository_owner }}
password: ${{ secrets.CR_PAT }}
-
name: Build and push
uses: docker/build-push-action@v2
with:
context: .
push: true
tags: |
user/app:latest
ghcr.io/${{ github.repository_owner }}/app:latest
# v1
steps:
-
name: Checkout code
uses: actions/checkout@v2
-
name: Build and push to Docker Hub
uses: docker/build-push-action@v1
with:
username: ${{ secrets.DOCKERHUB_USERNAME }}
password: ${{ secrets.DOCKERHUB_TOKEN }}
repository: user/app
tags: latest
-
name: Build and push to GHCR
uses: docker/build-push-action@v1
with:
username: ${{ github.repository_owner }}
password: ${{ secrets.CR_PAT }}
repository: ghcr.io/${{ github.repository_owner }}/app
tags: latest
As you can see here you must duplicate as many times the number of registries as you wish to use for v1. So you don't have as much flexibility on the use of tags and also build will be done x times the number of registries. The fact of having a dedicated action for the connection also allows to reuse it in other actions that need to connect to a registry.
And finally, a significant point concerns certain registries that use specific authentication methods through their own action. This is for example the case for AWS:
name: ci
on:
push:
branches: master
jobs:
login:
runs-on: ubuntu-latest
steps:
-
name: Configure AWS Credentials
uses: aws-actions/configure-aws-credentials@v1
with:
aws-access-key-id: ${{ secrets.AWS_ACCESS_KEY_ID }}
aws-secret-access-key: ${{ secrets.AWS_SECRET_ACCESS_KEY }}
aws-region: <region>
-
name: Login to ECR
uses: docker/login-action@v1
with:
registry: <aws-account-number>.dkr.ecr.<region>.amazonaws.com
I do understand that the
tag_with_refflag was quirky and limits some scenarios like pushing multiple tags and I actually like that there is another way now to configure this now. But was it necessary to drop the existing flag? Maybe you can consider bringing it back under a more meaningful name.
I'm currently working on an action called Docker meta allowing a management of tags and labels similar to the old flags used in v1. #206 was created to integrate this action as an example.
Looking at the upgrade guide it seems that context and file are mandatory now (didn't test it actually) but then again: why would I need to declare what is the 99% default case? So if it's not mandatory I'd to remove it from the upgrade document, and if it is mandatory I'd suggest to offer default settings for a classic Dockerfile build
Context and file are not mandatory. The default Git context is used if no context is defined. This allows you to stop using the step actions/checkout. This has been added in the upgrade notes for an easy and reproducible transition for users coming from v1.
@Brutus5000
But I'm already managing two dozen repositories for the FAForever project and I just need an easy and working CI infrastructure.
About this I think a Composite action would fit perfectly for you. But atm it's not possible to use actions within actions with composite (see actions/runner#646). What you can do is sharing workflows with your organization.
I'm currently working on an action called Docker meta allowing a management of tags and labels similar to the old flags used in v1. #206 was created to integrate this action as an example.
But this is one of the many issues with v2: you keep making new actions (more complexity) when what users want is something simple out-of-the-box with optional complexity. And even those options should be presented simply.
As an example: why am I leveraging multiple caches with opaque language? It's starting to look like a DSL. A key-value of cache: yes should be all that is needed. This
cache-from: type=registry,ref=user/app:latest
cache-to: type=inline
and
cache-from: type=local,src=/tmp/.buildx-cache
cache-to: type=local,dest=/tmp/.buildx-cache
mean nothing to the uninitiated. If that is the language used behind the scenes, don't present that to the user!
You've taken something that needed a few tweaks and turned it into something for people to spend hours pouring over documentation for the right recipe of incantations that result in a successful build.
@crazy-max Thanks for your suggestions I will look into them.
Regarding the other points: As I said, I do get the point of a dedicated login step. However, for me the question is: Do we need to drop the login in the build-and-push just because there is a dedicated login step now? Why not make it dependent on the presence or absence of the username and password env variables? Pushing to more than one docker registry sounds like a very rare edge case to me. It's good to support it (I guess especially to support your enterprise customers), but why force the 99% of the people who just live with one registry to split up login and push? Is there any drawback with offering both ways for login?
I understand there are edge cases where you want more controls, but v2 is a serious regression in usability over v1 ... to the point where I've been holding updates and even downgrading some to get the simple reliable easy to configure v1 back. If this keeps going this way I'll be looking for alternative actions altogether because it's just silly to have such complex workflows with multiple things to bump for such simple integrated tasks. As the OP noted, the advanced features should have been added while keeping sensible defaults in place for simpler use cases.
@worldofgeese @Brutus5000 @alerque
A single (or more concise) step as it was the case for v1 will be possible when GitHub will have implemented nested actions through a composite action (actions/runner#646). Here is an example of a composite action named for example crazy-max/docker-action@v1:
# https://github.com/crazy-max/docker-action/blob/v1/action.yml
name: 'My Docker action'
description: 'Docker AIO action'
author: 'crazy-max'
inputs:
registry:
description: 'Server address of the Docker registry'
required: false
image:
description: 'Docker image'
required: true
username:
description: 'Username to log in to a Docker registry'
required: false
password:
description: 'Password or PAT to log in to a Docker registry'
required: false
push:
description: 'Push'
required: false
default: 'false'
runs:
using: 'composite'
steps:
-
name: Checkout
uses: actions/checkout@v2
-
name: Docker meta
id: docker_meta
uses: crazy-max/ghaction-docker-meta@v1
with:
images: ${{ inputs.image }}
-
name: Set up QEMU
uses: docker/setup-qemu-action@v1
-
name: Set up Docker Buildx
uses: docker/setup-buildx-action@v1
-
name: Login to DockerHub
if: github.event_name != 'pull_request' && inputs.username != '' && inputs.password != ''
uses: docker/login-action@v1
with:
username: ${{ inputs.username }}
password: ${{ inputs.password }}
-
name: Build and push
uses: docker/build-push-action@v2
with:
context: .
push: ${{ inputs.push }}
tags: ${{ steps.docker_meta.outputs.tags }}
labels: ${{ steps.docker_meta.outputs.labels }}
And a possible workflow to use it:
name: ci
on:
push:
jobs:
ci:
runs-on: ubuntu-latest
steps:
-
name: Docker build
uses: crazy-max/docker-action@v1
with:
registry: ghcr.io # optional for Docker Hub
image: name/app
username: ${{ secrets.GHCR_USERNAME }}
password: ${{ secrets.GHCR_PAT }}
push: ${{ github.event_name != 'pull_request' }}
@CrazyMax I'll look forward to a v3 of this action (or a new one entirely) that works like that and brings some sanity to simpler projects. For now v1 still functions nicely. :wink:
I'll look forward to a v3 of this action (or a new one entirely) that works like that and brings some sanity to simpler projects. For now v1 still functions nicely.
Is there an issue open specifically to track that?
Thanks. I think my question was unclear though.
You described crazy-max/docker-action@v1 as a way of using https://github.com/actions/runner/issues/646 to create a composite action in your own repo. I'm asking whether this repo (or the docker org in general) will host such an action when 646 is completed.
Thanks. I think my question was unclear though.
You described
crazy-max/docker-action@v1as a way of using actions/runner#646 to create a composite action in your own repo. I'm asking whether this repo (or thedockerorg in general) will host such an action when 646 is completed.
It might be a good idea for the community to put together a "docker-build-push-simple-action" as a stopgap while waiting for https://github.com/actions/runner/issues/646. Once that's complete, if Docker decides to create a new action for simple use cases, the community-maintained stopgap could be deprecated in favor of the new Docker-maintained solution. If Docker decides not to create such an action, the community-maintained action could be maintained as long as demand for it existed.
Composite actions have supported runs.steps[*].uses since late 2021. I didn't see any official support for a composite action yet, so I put one together at https://github.com/benfrisbie/docker-tag-build-push-action.
It doesn't support all of the inputs and outputs from docker/login-action, docker/metadata-action, and docker/build-push-action. However, it supports most of the main ones that I needed for my personal use. I plan on adding support for the rest of them soon.
Anyways, I'd welcome some feedback on it! If there is interest to get it moved into the docker organization I'd love to discuss that as well.
