ci(docker): cache buildx layers between master builds - #800
Conversation
NuSkooler
left a comment
There was a problem hiding this comment.
Good idea and the master-only reasoning holds. A couple of things to weigh first, inline.
| cache-from: type=gha | ||
| cache-to: type=gha,mode=max |
There was a problem hiding this comment.
The reasoning here is right -- this workflow is push: [master] plus workflow_dispatch, so the cache really is only ever written by master, and the QEMU arm legs are exactly the part worth caching.
Two things before it goes in.
mode=max shares a 10 GB budget with the workflows that gate every PR. It caches every intermediate layer across three platforms, native rebuilds included, which adds up quickly. That storage is the repo-wide GitHub Actions cache, and test.yml and lint.yml both use actions/setup-node with cache: npm, as do the two website workflows. Eviction is LRU across the whole repo, so a large Docker cache can quietly evict the npm caches that keep PR checks fast -- trading a slow master build for slow checks on every pull request.
I would start at mode=min and see what the arm legs actually save before reaching for max.
The actions underneath this are old. The workflow pairs setup-qemu-action@v3 and setup-buildx-action@v3 with build-push-action@v3 (2022) and login-action@v2. Which cache protocol type=gha ends up speaking depends on the buildx/BuildKit version behind it, so bumping build-push-action belongs in this PR rather than layering a cache config onto an old action.
And one that is not really yours to fix here: this workflow has no pull_request trigger, so nothing in this PR is exercised before merge -- the first real run is on master, in the job that publishes :latest. Worth a manual workflow_dispatch run off this branch to confirm the cache engages at all. A separate PR adding a pull_request trigger that builds without pushing would give Docker changes the same safety net the rest of the tree already has.
|
Heads up before you spend more time on this: I have a change for #811 ready that adds a It also undercuts the comment here. "Nothing here runs on a pull request, so the cache is only ever written by master" stops being true, and that matters more than the wording: with So once #811 is in, this probably wants: cache-from: type=gha
cache-to: ${{ github.event_name != 'pull_request' && 'type=gha,mode=max' || '' }}which lets a pull request warm off master's cache without writing one back. That also makes Happy either way on sequencing: I can hold #811 until this lands and rebase mine instead, if you'd rather not redo yours. |
50caf65 to
c5620a6
Compare
c5620a6 to
78da869
Compare
…) (#819) * build(docker): keep .git and the website's node_modules out of the build context COPY . /enigma-bbs copies the whole checkout. CI checks out fresh, but a local build on a working copy shipped .git and, once the docs site had been built, website/node_modules and website/dist -- around 460 MB that nothing at runtime reads. Exclude them, and use **/node_modules so a nested one is caught wherever it appears. * build(docker): build native modules in a stage that does not ship (#814) The image installed build-essential, python3, libssl-dev, git and curl, ran npm ci, and then apt-get removed the toolchain in a later RUN. A layer can only add to an image, so the removal recorded a whiteout over bytes that were still pulled: four fifths of the published image was the layer that installed the toolchain. Split into a build stage that owns the toolchain and npm ci, and a runtime stage that installs only the archivers and lrzsz, installs pm2, and copies node_modules out of the build stage. Both stages start from the same base image and neither pins --platform, which is what makes copying the compiled native modules sound (#794); the Dockerfile says so where the COPY --from lands. git stays in the build stage because the lockfile resolves one dependency from a git URL. Two things the old file did on the side are gone. It ran pm2 start main.js at build time, which really launched the BBS: the image carried a pm2 daemon's state under /root/.pm2 and, on a checkout with a config, sqlite databases under /enigma-bbs/db. The entrypoint runs pm2-runtime and needs neither. And it installed dos2unix to fix one file's line endings and removed it again; sed does that now. The workflow's cache-to moves from mode=min to mode=max. min exports only the final image's layers, and the npm ci layer -- where the QEMU native rebuilds live -- now belongs to the discarded build stage, so with min it would be rebuilt on every master push. This is the question #800 left open. test/docker_image.test.js parses the Dockerfile into stages and pins the shape: toolchain only in the build stage, no apt-get remove in the runtime stage, node_modules copied across before the source tree, no BBS started during the build, mode=max on master and no cache written from a pull request. test/live/docker_image.live.js builds the image when ENIGMA_DOCKER_LIVE=1 and checks it from inside -- the copied native modules load, sexyz runs, the archivers are present -- and reads the layer history, since apt-get remove hides gcc from a running container just fine and only the history shows whether the toolchain layer shipped. Measured locally on amd64, the image goes from 1.70 GB to 686 MB. The old figure includes the local build context leak fixed in the previous commit; without it, the old image is roughly 1.1 GB.
No description provided.