Skip to content
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

Use GitHub Actions for build/pull/tag/push #46

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
108 changes: 91 additions & 17 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,24 +5,98 @@ jobs:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
- name: Set CACHE_DATE
run: echo "CACHE_DATE=$(date --utc +%Y%m%dT%H%M%SZ)" >> $GITHUB_ENV
- run: ./devel/pull
- run: ./devel/build
- if: ${{ github.event_name != 'pull_request' }}

- uses: docker/setup-buildx-action@v1
id: builder-image-builder

- uses: docker/setup-buildx-action@v1
id: base-image-builder

- name: Show builder-image-builder name
run: echo ${{ steps.builder-image-builder.outputs.name }}

- name: Show base-image-builder name
run: echo ${{ steps.base-image-builder.outputs.name }}

# Set CACHE_DATE in your environment to force layers after our custom cache
# point to be re-built. See the ARG CACHE_DATE line in the Dockerfile for more
# information.
- name: Set CACHE_DATE and GIT_REVISION env vars
run: |
echo "CACHE_DATE=$(date --utc +%Y%m%dT%H%M%SZ)" >> $GITHUB_ENV
echo "GIT_REVISION=$(git describe --tags --abbrev=40 --always --dirty || true)" >> $GITHUB_ENV
Comment on lines +26 to +27
Copy link
Member

Choose a reason for hiding this comment

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

I like to do | tee -a instead of >> so that the values are visible in the workflow logs.


# Use toJSON to store boolean values in a string
# and fromJSON to retrieve as boolean later on.
- name: Set common variables
run: |
echo "BASE_IMAGE_NAME=nextstrain/base" >> $GITHUB_ENV
echo "BUILDER_IMAGE_NAME=nextstrain/base-builder" >> $GITHUB_ENV
echo "TAG_AND_PUSH=${{ toJSON(github.event_name != 'pull_request') }}" >> $GITHUB_ENV
echo "ON_DEFAULT_BRANCH=${{ toJSON(github.ref_type == 'branch' && github.ref_name == github.event.repository.default_branch) }}" >> $GITHUB_ENV

- if: ${{ fromJSON(env.TAG_AND_PUSH) && fromJSON(env.ON_DEFAULT_BRANCH) }}
name: Set tag name (default branch)
run: echo "TAG_NAME=build-$CACHE_DATE" >> $GITHUB_ENV

# From `man docker-image-tag`: The tag name must be valid ASCII and may
# contain lowercase and uppercase letters, digits, underscores, periods
# and hyphens.
# TODO: parameterize this as workflow input
- if: ${{ fromJSON(env.TAG_AND_PUSH) && !fromJSON(env.ON_DEFAULT_BRANCH) }}
name: Set tag name (non-default branch)
run: echo "TAG_NAME=branch-${GITHUB_REF_NAME//[^A-Za-z0-9._-]/-}" >> $GITHUB_ENV

- if: ${{ fromJSON(env.TAG_AND_PUSH) }}
name: Set tags to push
run: |
maybe_base_image_latest_tag=${{ fromJSON(env.ON_DEFAULT_BRANCH) && ',$BASE_IMAGE_NAME:latest' || '' }}
echo "BASE_IMAGE_TAGS=$BASE_IMAGE_NAME:$TAG_NAME$maybe_base_image_latest_tag" >> $GITHUB_ENV
maybe_builder_image_latest_tag=${{ fromJSON(env.ON_DEFAULT_BRANCH) && ',$BUILDER_IMAGE_NAME:latest' || '' }}
echo "BUILDER_IMAGE_TAGS=$BUILDER_IMAGE_NAME:$TAG_NAME$maybe_builder_image_latest_tag" >> $GITHUB_ENV
Comment on lines +29 to +56
Copy link
Member

Choose a reason for hiding this comment

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

This is all pretty dense and hard for me to follow given the mix of interpolation at different levels, JSON serde, and conditionals. The motivation for defining and using all these is because the build-push-actions needed to be parameterized instead of themselves being in a conditional like with the modular devel/tag + devel/push commands?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I couldn't figure out a better way since GitHub Actions provides limited support for conditional "code paths" here. It would be either some variant of this (with comments to explain) or several copies of build-push-action that are mostly the same except for push, tags, and step condition.


- if: ${{ fromJSON(env.TAG_AND_PUSH) }}
uses: docker/login-action@v1
with:
username: ${{ secrets.DOCKER_LOGIN }}
password: ${{ secrets.DOCKER_PASSWORD }}
- if: ${{ github.event_name != 'pull_request' && github.ref_type == 'branch' && github.ref_name == github.event.repository.default_branch }}
run: |
./devel/tag build-$CACHE_DATE
./devel/push latest build-$CACHE_DATE
- if: ${{ github.event_name != 'pull_request' && github.ref_type == 'branch' && github.ref_name != github.event.repository.default_branch }}
# From `man docker-image-tag`: The tag name must be valid ASCII and may
# contain lowercase and uppercase letters, digits, underscores, periods
# and hyphens.
run: |
tag=branch-${GITHUB_REF_NAME//[^A-Za-z0-9._-]/-}
./devel/tag $tag
./devel/push $tag

# The nextstrain/base Dockerfile is a multi-stage with both a "builder" target
# and a main target. To enable proper caching via --cache-from we need both
# these images available to pull layers from. This means pulling both in at
# the start and pushing both up at the end.

# Calling `docker run nextstrain/base` will still only pull down the small base
# image rather than pulling down the larger nextstrain/base-builder image.
Comment on lines +64 to +70
Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure if isolated builders are needed here though. It might be more efficient to have one build and push to both base and builder images, if that's even possible?

Or, to cut some time, build the two images in parallel using 2 jobs in this workflow.

Copy link
Member

Choose a reason for hiding this comment

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

I believe one of the features of buildx is better support for multi-stage builds, such that you can do just build operation for a multi-stage build but still push up tagged images for each stage (instead of having to explicitly build each stage yourself just so you can push each later).

I'm not sure how building both stages in parallel would work since the second stage relies on the first?

Copy link
Member

Choose a reason for hiding this comment

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

Ah, this better support appears to be (1, 2) in the form of exporting cached layers via type=registry with mode=max (meaning "all intermediate layers"), but you have to use a separate image tag/name (unlike type=inline, but that only supports mode=min which means "final layers").

- name: Build${{ fromJSON(env.TAG_AND_PUSH) && '/tag/push' || '' }} ${{ env.BUILDER_IMAGE_NAME }}
uses: docker/build-push-action@v2
with:
target: builder
builder: ${{ steps.builder-image-builder.outputs.name }}
platforms: linux/amd64
context: .
pull: true
push: ${{ fromJSON(env.TAG_AND_PUSH) }}
tags: ${{ fromJSON(env.TAG_AND_PUSH) && env.BUILDER_IMAGE_TAGS || null }}
cache-from: |
type=registry,ref=${{ env.BUILDER_IMAGE_NAME }}
type=registry,ref=${{ env.BASE_IMAGE_NAME }}
Comment on lines +81 to +83
Copy link
Member

Choose a reason for hiding this comment

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

The lack of cache-to here might mean that the subsequent step can't use the just-built first stage when TAG_AND_PUSH is false (and would have to re-download it with TAG_AND_PUSH is true).

build-args: |
CACHE_DATE
GIT_REVISION

- name: Build${{ fromJSON(env.TAG_AND_PUSH) && '/tag/push' || '' }} ${{ env.BASE_IMAGE_NAME }}
uses: docker/build-push-action@v2
with:
builder: ${{ steps.base-image-builder.outputs.name }}
platforms: linux/amd64
context: .
pull: true
push: ${{ fromJSON(env.TAG_AND_PUSH) }}
tags: ${{ fromJSON(env.TAG_AND_PUSH) && env.BASE_IMAGE_TAGS || null }}
cache-from: |
type=registry,ref=${{ env.BUILDER_IMAGE_NAME }}
type=registry,ref=${{ env.BASE_IMAGE_NAME }}
Comment on lines +97 to +99
Copy link
Member

Choose a reason for hiding this comment

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

Similarly, the lack of non-registry sources in this cache-from here might mean that this step can't use the just-built first stage from the previous step when TAG_AND_PUSH is false (and would have to re-download it with TAG_AND_PUSH is true).

build-args: |
CACHE_DATE
GIT_REVISION
37 changes: 0 additions & 37 deletions devel/build

This file was deleted.

18 changes: 0 additions & 18 deletions devel/pull

This file was deleted.

18 changes: 0 additions & 18 deletions devel/push

This file was deleted.

16 changes: 0 additions & 16 deletions devel/tag

This file was deleted.