DEV: use docker bake to build images#1066
Conversation
Replace the ruby auto_build.rb script with [docker bake](https://docs.docker.com/build/bake/introduction/) Use build cache to push image tags directly with `docker bake --push` rather than specifying tags in `build.yml` Unifies the way all dockerfile-based images are built. Allows for image groups to be built/pushed at once. Bake targets are able to handle target variants, and tags accordingly across all version and tags (version+arch), simplifying `build.yml` Renames stable-* images to esr-* Handle image building differences: Test builds --- Test images have two distinct bake targets: at build time (built at -base) and when pushing `test` jobs. base test build jobs are built from local images, before base images are pushed. Both amd64 and arm64 variants are built. The `test` job depends on images to get pushed and depends on the finalized pushed multiarch tags, and the test image is only built with the amd64 variant. Dev builds --- Update the way base templates are copied into the dev image - rather than an external process copying and mutating the template, copy the existing template straight from ../templates (via build context) and mutate within the dev dockerfile. add explicit allow when bulding dev images a dev build mounts up templates from the parent directory to setup postgres and redis from the template skeletons. buildx may complain about insecure capabilities without this. Previously copying and editing of templates was done in the ruby build script directly. Parameters --- Add parameters to many settings, allowing for the github actions to be run on forks, or against different environments. Eases testing outside of the main docker repositories when developing against discourse_docker. Github forks would be able to build images themselves, allowing for personal docker repositories to be used, or potentially separate into official stage or testing environments by way of GitHub environments. Add an option to skip tests via vars, making it easier to test changes. All GitHub environments should then define following parameters: * `AMD64_RUNNER` - Github actions runner for AMD64 image versions * `ARM64_RUNNER` - Github actions runner for ARM64 image versions * `BASE_REPO` - Docker repository for discourse base image builds * `DEV_REPO` - Docker repository for discourse dev image builds * `TEST_REPO` - Docker repository for discourse test image builds * `SETUP_WIZARD_REPO` - Docker repository for discourse setup wizard image builds * `WEB_ONLY_REPO` - Docker repository for discourse web-only image builds * `DOCKERHUB_USERNAME` - docker registry username * `DOCKERHUB_PASSWORD` - docker registry password * `SKIP_TESTS` - Flag to skip tests. Set to 1 to skip full tests before images pushes. Allows time for builds to complete on github-hosted runners.
|
This cleans up a lot of Introducing github environments:
Test branch: https://github.com/featheredtoast/discourse_docker/tree/refs/heads/docker-bake-test Test repositories: |
parameterize the branch to be used for image pushing via IMAGE_PUSH_BRANCH
add local_discourse/* defaults to variables. add default arch to be `amd64,arm64`.
Building without a timestamp env var should build an image with no timestamp tagged. Allows for easier local builds.
That's a breaking change, right? Tbh, I don't think we should be publishing any tags like esr/stable/release. Instead, we should be maintaining tags for each currently-supported release, and then people should be responsible for picking the one they want and updating when they want. That way, it can work well with our new overlapping support policy. So for the purposes of this PR, maybe we keep it named |
| push: | ||
| branches: | ||
| - main |
There was a problem hiding this comment.
We should keep the main restriction, right?
There was a problem hiding this comment.
It's intentional. I'm parameterizing main to get this repository easier to test, and skipping workflows that do not match vars.IMAGE_PUSH_BRANCH (IE, main).
There's unfortunately no way to do something like:
push:
branches:
- ${{ vars.IMAGE_PUSH_BRANCH }}
so the if blocks are the best we have.
Doing this allows us to have a future where we can build/push in different parameters, such as setting up test docker repos to iterate on changes outside of the main branch
| echo "timestamp=$timestamp" >> $GITHUB_OUTPUT | ||
|
|
||
| base: | ||
| if: ${{ github.event_name == 'schedule' || github.event_name == 'pull_request' || (github.event_name == 'push' && github.ref == format('refs/heads/{0}', vars.IMAGE_PUSH_BRANCH)) }} |
There was a problem hiding this comment.
Is this working correctly? seems like all the CI steps were skipped on the most recent push to this PR 👀
There was a problem hiding this comment.
The discourse_docker env vars need to be updated for the proposed github actions workflow to run properly. I can update those in the repo so the workflows can run properly.
| run: | | ||
| ruby auto_build.rb discourse_dev_${{ matrix.arch }} | ||
| docker tag discourse/discourse_dev:build_${{ matrix.arch }} discourse/discourse_dev:${{ env.TIMESTAMP }}-${{ matrix.arch }} | ||
| docker buildx bake dev --push --allow=fs.read=../templates |
There was a problem hiding this comment.
Should this step be gated on main? Otherwise we're gonna be pushing images from PR CI?
There was a problem hiding this comment.
Good catch - updated 👍
| if: ${{ github.event_name == 'schedule' || github.event_name == 'pull_request' || (github.event_name == 'push' && github.ref == format('refs/heads/{0}', vars.IMAGE_PUSH_BRANCH)) }} | ||
| runs-on: ${{ vars.AMD64_RUNNER }} | ||
| timeout-minutes: 60 | ||
| needs: base |
There was a problem hiding this comment.
Not related to your PR, but we should probably make this:
| needs: base | |
| needs: push_multi_arch_manifests |
otherwise there's a race condition where it'll test the older version of the images.
(which also makes me think... maybe this job shouldn't run at all on PRs? Kinda silly to test the released images, and not the ones built from the current PR...)
| ## Building new images | ||
|
|
||
| To build a new image, just run `ruby auto_build.rb image-name`. The build process will build a local image with a predefined tag. | ||
| To build a new image, just run `docker bake {target}`. The build process will build a local image with predefined tags. |
There was a problem hiding this comment.
| To build a new image, just run `docker bake {target}`. The build process will build a local image with predefined tags. | |
| To build a new image, just run `docker buildx bake {target}`. The build process will build a local image with predefined tags. |
?
And the same for all the other references in the readme?
There was a problem hiding this comment.
As I understand it docker bake is an alias for docker buildx bake (similar to docker build and friends). I documented the friendly name by default.
|
I asked claude to make a diff of the published tags before/after this change. Seems like there are a lot of new ones being published: discourse/base
# --- per-arch tags ---
- 2.0.T-slim-{arch}
+ 2.0.T-slim-main-{arch} # renamed: gained -main qualifier
+ 2.0.T-slim-esr-{arch} # NEW (old built stable slim but never pushed it)
+ slim-main-{arch} # NEW (plain, non-timestamped)
+ slim-esr-{arch} # NEW
+ web-only-main-{arch} # NEW (plain)
+ web-only-esr-{arch} # NEW (plain)
+ release-main-{arch} # NEW (plain)
+ release-esr-{arch} # NEW (plain)
- 2.0.T-web-only-stable-{arch}
+ 2.0.T-web-only-esr-{arch} # rename stable→esr
- 2.0.T-stable-{arch}
+ 2.0.T-esr-{arch} # rename stable→esr
2.0.T-web-only-main-{arch} # unchanged
2.0.T-main-{arch} # unchanged
aarch64 # unchanged
# --- multi-arch manifests (user-facing) ---
- web-only-stable
+ web-only-esr # rename
- 2.0.T-web-only-stable
+ 2.0.T-web-only-esr # rename
- 2.0.T-stable
+ 2.0.T-esr # rename
- release-stable
+ release-esr # rename
slim # unchanged
2.0.T-slim # unchanged
web-only # unchanged
2.0.T-web-only # unchanged
release # unchanged
2.0.T # unchanged
discourse/discourse_dev
+ release-{arch} # NEW (plain, non-timestamped per-arch)
T-{arch} # unchanged
release # unchanged (manifest)
T # unchanged (manifest)
discourse/setup-wizard — no change
release-{arch} # unchanged
release # unchanged (manifest)
discourse/discourse_test — no change
slim, slim-browsers, release # unchangedis that intentional? |
updating tags is deferred
|
For the new branches, there's no good way for bake to omit pushing branches in the list unfortunately, so I'm proposing pushing those out for sake of simplicity. That said, it's not ideal. Would really be nice to have a way to delete branches programmatically though. Docker's experimental hub-tool was able to do it, but was archived this year, so maybe there's traction to get it added at some point. Some thoughts to do tagging more elegantly:
|
|
Probably the most attractive option re: consolidating tag names is to cache to another image registry (ghcr? internal?) It also means we aren't tightly dependent on github-specific bits. Then we can build separately, cache the run, and then when we're ready to push, do that for all images. There is also a lot of opportunity for parallel builds, reusing the cache. I'll get a POC out soon. |
Replace the ruby auto_build.rb script with docker bake
Use build cache to push image tags directly with
docker bake --pushrather than specifying tags inbuild.ymlUnifies the way all dockerfile-based images are built.
Allows for image groups to be built/pushed at once.
Bake targets are able to handle target variants, and tags accordingly across all version and tags (version+arch), simplifying
build.ymlRenames stable-* images to esr-*
Handle image building differences:
Test builds
Test images have two distinct bake targets: at build time (built at -base) and when pushing
testjobs. base test build jobs are built from local images, before base images are pushed. Both amd64 and arm64 variants are built. Thetestjob depends on images to get pushed and depends on the finalized pushed multiarch tags, and the test image is only built with the amd64 variant.Dev builds
Update the way base templates are copied into the dev image - rather than an external process copying and mutating the template, copy the existing template straight from ../templates (via build context) and mutate within the dev dockerfile.
add explicit allow when bulding dev images
a dev build mounts up templates from the parent directory to setup postgres and redis from the template skeletons.
buildx may complain about insecure capabilities without this.
Previously copying and editing of templates was done in the ruby build script directly.
Parameters
Add parameters to many settings, allowing for the github actions to be run on forks, or against different environments. Eases testing outside of the main docker repositories when developing against discourse_docker.
Github forks would be able to build images themselves, allowing for personal docker repositories to be used, or potentially separate into official stage or testing environments by way of GitHub environments.
Add an option to skip tests via vars, making it easier to test changes.
All GitHub environments should then define following parameters:
AMD64_RUNNER- Github actions runner for AMD64 image versionsARM64_RUNNER- Github actions runner for ARM64 image versionsBASE_REPO- Docker repository for discourse base image buildsDEV_REPO- Docker repository for discourse dev image buildsTEST_REPO- Docker repository for discourse test image buildsSETUP_WIZARD_REPO- Docker repository for discourse setup wizard image buildsWEB_ONLY_REPO- Docker repository for discourse web-only image buildsDOCKERHUB_USERNAME- docker registry usernameDOCKERHUB_PASSWORD- docker registry passwordSKIP_TESTS- Flag to skip tests. Set to 1 to skip full tests before images pushes.IMAGE_PUSH_BRANCH- the environment branch to trigger image pushes for (eg,main).Allows time for builds to complete on github-hosted runners.