Skip to content

Simplify docker build and push workflow #76

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

Merged
merged 3 commits into from
Jan 26, 2022

Conversation

Xerkus
Copy link
Member

@Xerkus Xerkus commented Jan 24, 2022

Q A
Documentation no
Bugfix no
BC Break no
New Feature no
RFC no
QA yes

Description

Workflow is updated and simplified to use single step to build container also using github actions cache for build cache export.

Same step also pushes the image when triggered from release event.

Signed-off-by: Aleksei Khudiakov <[email protected]>
Signed-off-by: Aleksei Khudiakov <[email protected]>
if: ${{ github.event_name == 'pull_request' && ! env.DOCKER_USER }}
images: ghcr.io/laminas/laminas-continuous-integration
tags: |
type=semver,pattern={{version}}
Copy link
Member Author

Choose a reason for hiding this comment

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

{{version}} is shorthand for {{major}}.{{minor}}.{{patch}}
It uses tag ref associated with release to extract version info.

Copy link
Member

Choose a reason for hiding this comment

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

Checking the docs, type=semver seems wrong, as it expects the tag to be v1.2.3, while we tag 1.2.3: https://github.com/docker/metadata-action#typesemver

https://github.com/docker/metadata-action#typepep440 seems to be more connected

Copy link
Member Author

Choose a reason for hiding this comment

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

semver is correct. v prefix is dropped and ignored

Comment on lines 22 to +28
- name: Login to GitHub Container Registry
if: ${{ env.DOCKER_USER }}
if: ${{ github.event_name == 'release' }}
uses: docker/login-action@v1
with:
registry: ghcr.io
username: ${{ env.DOCKER_USER }}
password: ${{ secrets.CONTAINER_PAT }}

- name: Build and push for release
if: ${{ github.event_name == 'release' }}
uses: docker/build-push-action@v2
with:
context: .
file: ./Dockerfile
platforms: linux/amd64
pull: true
push: true
tags: ${{ join(fromJSON(steps.tags.outputs.tags), ',') }}
cache-from: type=registry,ref=ghcr.io/laminas/laminas-continuous-integration-action:build-cache
cache-to: type=registry,ref=ghcr.io/laminas/laminas-continuous-integration-action:build-cache,mode=max
username: ${{ github.repository_owner }}
password: ${{ secrets.GITHUB_TOKEN }}
Copy link
Member Author

Choose a reason for hiding this comment

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

Actions token works for GHCR because package has this repository added with write access for actions.

Link for reference for TSC members: https://github.com/orgs/laminas/packages/container/laminas-continuous-integration/settings

if: ${{ github.event_name == 'pull_request' && ! env.DOCKER_USER }}
images: ghcr.io/laminas/laminas-continuous-integration
tags: |
type=semver,pattern={{version}}
Copy link
Member

Choose a reason for hiding this comment

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

Checking the docs, type=semver seems wrong, as it expects the tag to be v1.2.3, while we tag 1.2.3: https://github.com/docker/metadata-action#typesemver

https://github.com/docker/metadata-action#typepep440 seems to be more connected

uses: docker/build-push-action@v2
- name: Docker meta
id: docker_meta
uses: docker/metadata-action@v3
Copy link
Member

Choose a reason for hiding this comment

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

Given that for us, git tag === image tag, why not skip this action completely, and just get the tag from github.ref_name + github.ref_type === 'tag' ?

That would remove he need for this third-party to be involved.

Copy link
Member Author

Choose a reason for hiding this comment

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

why not skip this action completely

It have sanity checks and validations in place. Action that was used here failed immediately when used with ref for PR.

push: ${{ github.event_name == 'release' }}
tags: |
${{ steps.docker_meta.outputs.tags }}
labels: ${{ steps.docker_meta.outputs.labels }}
Copy link
Member

Choose a reason for hiding this comment

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

Is this fetched from the LABEL blocks in Dockerfile? If so, I can totally see the value in using the docker/metadata-action, although it's weird that these aren't simply fetched from the LABEL blocks in this step directly 🤔

Copy link
Member Author

Choose a reason for hiding this comment

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

No, those are populated from action context, you can see output in log for metadata step:

Docker tags
  ghcr.io/laminas/laminas-continuous-integration:pr-76
Docker labels
  org.opencontainers.image.title=laminas-continuous-integration-action
  org.opencontainers.image.description=GitHub Action for running a QA check
  org.opencontainers.image.url=https://github.com/laminas/laminas-continuous-integration-action
  org.opencontainers.image.source=https://github.com/laminas/laminas-continuous-integration-action
  org.opencontainers.image.version=pr-76
  org.opencontainers.image.created=2022-01-25T00:22:31.500Z
  org.opencontainers.image.revision=2b0bbcc594923f10ac377486321ba0e1fd8cc366
  org.opencontainers.image.licenses=BSD-3-Clause

Copy link
Member Author

Choose a reason for hiding this comment

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

LABEL in dockerfile are already applied and those don't need to be specified at runtime

@Ocramius Ocramius added this to the 1.17.0 milestone Jan 25, 2022
@Ocramius Ocramius self-assigned this Jan 26, 2022
Copy link
Member

@Ocramius Ocramius left a comment

Choose a reason for hiding this comment

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

Rolling a release - let's see how this one works :D

@Ocramius Ocramius merged commit bfaac1c into laminas:1.17.x Jan 26, 2022
@Xerkus
Copy link
Member Author

Xerkus commented Jan 26, 2022

docker meta step for the release:

Docker tags
  ghcr.io/laminas/laminas-continuous-integration:1.17.0
Docker labels
  org.opencontainers.image.title=laminas-continuous-integration-action
  org.opencontainers.image.description=GitHub Action for running a QA check
  org.opencontainers.image.url=https://github.com/laminas/laminas-continuous-integration-action
  org.opencontainers.image.source=https://github.com/laminas/laminas-continuous-integration-action
  org.opencontainers.image.version=1.17.0
  org.opencontainers.image.created=2022-01-26T12:23:55.897Z
  org.opencontainers.image.revision=bfaac1c74bdea35dc0ec4c028f5f125d8a8b08f8
  org.opencontainers.image.licenses=BSD-3-Clause

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.

2 participants