Skip to content

ci: build and push image for main branch #222

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 7 commits into from
Jun 10, 2024
Merged
Show file tree
Hide file tree
Changes from 5 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
49 changes: 48 additions & 1 deletion .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,12 @@ permissions:
contents: read
deployments: none
issues: none
packages: none
pull-requests: none
repository-projects: none
security-events: none
statuses: none
# Necessary to push docker images to ghcr.io.
packages: write

# Cancel in-progress runs for pull requests when developers push
# additional changes
Expand Down Expand Up @@ -67,3 +68,49 @@ jobs:

- name: Check format
run: ./scripts/check_fmt.sh
build:
runs-on: ubuntu-latest
steps:
- name: Checkout
uses: actions/checkout@v4
with:
# Needed to get older tags
fetch-depth: 0
Comment on lines +77 to +78
Copy link
Member

Choose a reason for hiding this comment

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

we probably don't need the tags if we're just building a preview image on main?

Copy link
Member Author

@matifali matifali Jun 6, 2024

Choose a reason for hiding this comment

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

Yes. The benefit of the version tag is to use any previous image. It's a choice we should make. If we only tag with main then we will only have a single tagged image pointing to the tip of the main.


- uses: actions/setup-go@v5
with:
go-version: "~1.22"

- name: Login to GitHub Container Registry
if: github.event_name == 'push' && github.ref == 'refs/heads/main'
uses: docker/login-action@v2
with:
registry: ghcr.io
username: ${{ github.actor }}
password: ${{ secrets.GITHUB_TOKEN }}

# do not push images for pull requests
- name: Build
if: github.event_name == 'pull_request'
run: |
VERSION=$(./scripts/version.sh)-dev-$(git rev-parse --short HEAD)
BASE=ghcr.io/coder/envbuilder-preview

./scripts/build.sh \
--arch=amd64 \
--base=$BASE \
--tag=$VERSION
Comment on lines +99 to +102
Copy link
Member Author

Choose a reason for hiding this comment

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

I am not building a multi-arch here, as this step only ensures that Dockerfile builds fine.

Copy link
Member

Choose a reason for hiding this comment

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

we should probably still build multi-arch as some changes may only cause issues with certain architectures

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok. Our build script uses --load when --push is not specified, which cannot be used for multi arch images.
docker/buildx#59

I didn't look deep if we actually needed to load the built images during local development otherwise I can just remove --load and build a multi arch image.

While calling the script locally we only build with $current arch so --load works fine.


- name: Build and Push
if: github.ref == 'refs/heads/main'
run: |
VERSION=$(./scripts/version.sh)-dev-$(git rev-parse --short HEAD)
BASE=ghcr.io/coder/envbuilder-preview

./scripts/build.sh \
--arch=amd64 \
--arch=arm64 \
--arch=arm \
--base=$BASE \
--tag=$VERSION \
--push
3 changes: 1 addition & 2 deletions scripts/build.sh
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ if [ -z "$BUILDER_EXISTS" ]; then
docker buildx create --use --platform=linux/arm64,linux/amd64,linux/arm/v7 --name $BUILDER_NAME
else
echo "Builder $BUILDER_NAME already exists. Using it."
docker buildx use $BUILDER_NAME
fi

# Ensure the builder is bootstrapped and ready to use
Expand All @@ -63,7 +62,7 @@ else
args+=( --load )
fi

docker buildx build "${args[@]}" -t $base:$tag -t $base:latest -f Dockerfile .
docker buildx build --builder $BUILDER_NAME "${args[@]}" -t $base:$tag -t $base:latest -f Dockerfile .
Copy link
Member Author

Choose a reason for hiding this comment

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

Instead of setting the builder as a default builder, use it explicitly to avoid changing the developer's local builder configuration.

Copy link
Member

Choose a reason for hiding this comment

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

could also tag as $base:main

Copy link
Member Author

@matifali matifali Jun 6, 2024

Choose a reason for hiding this comment

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

Yes thought about this but will require hacking the build script to work differently for tagged releases. Currently the script has a hardcoded $base:latest so I opted for changing the $base.

Copy link
Member

Choose a reason for hiding this comment

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

fair, not blocking


# Check if archs contains the current. If so, then output a message!
if [[ -z "${CI:-}" ]] && [[ " ${archs[@]} " =~ " ${current} " ]]; then
Expand Down