Conversation
| ./scripts/build.sh \ | ||
| --arch=amd64 \ | ||
| --base=$BASE \ | ||
| --tag=$VERSION |
There was a problem hiding this comment.
I am not building a multi-arch here, as this step only ensures that Dockerfile builds fine.
There was a problem hiding this comment.
we should probably still build multi-arch as some changes may only cause issues with certain architectures
There was a problem hiding this comment.
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.
| 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 . |
There was a problem hiding this comment.
Instead of setting the builder as a default builder, use it explicitly to avoid changing the developer's local builder configuration.
| 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 . |
There was a problem hiding this comment.
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.
dannykopping
left a comment
There was a problem hiding this comment.
LGTM
To what extent has this workflow been tested?
Has the workflow been linted?
| # Needed to get older tags | ||
| fetch-depth: 0 |
There was a problem hiding this comment.
we probably don't need the tags if we're just building a preview image on main?
There was a problem hiding this comment.
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.
|
@dannykopping I have tested the workflow except the push image step, but will pass this through the linter. Thanks for calling it. |
|
@matifali is this good to merge? |
|
Yes. Once the CI passes it should be good to merge. |
This pull request updates the
ciworkflow to build and push a Docker image for themainbranch.Image is pushed to
ghcr.io/coder/envbuilder-previewto not pollute the manually tagged versions atghcr.io/coder/envbuilderThis will help test the features that are under development.
Each image is tagged as
latestand$(./scripts/version.sh)-dev-$(git rev-parse --short HEAD)e.g.,ghcr.io/coder/envbuilder-preview:latestghcr.io/coder/envbuilder-preview:v0.2.9-dev-ab86184Alternatively, we can push
ghcr.io/coder/envbuilder:mainto the same package.