Skip to content
This repository was archived by the owner on Sep 17, 2026. It is now read-only.

Layer-cache the image builds in itest and e2e, keeping images.yml cold - #25

Merged
Peeja merged 1 commit into
mainfrom
claude/ci-cache
Sep 17, 2026
Merged

Peeja merged 1 commit into
mainfrom
claude/ci-cache

Conversation

@Peeja

@Peeja Peeja commented Sep 17, 2026 •

Copy link
Copy Markdown

⏭️ Safe to merge without waiting for CI — now moot: 22/22 green.

It was "no — watch itest and e2e", and that was right: e2e did go red. It turned out to be a pre-existing flake (comment), the one allowed re-run passed, and every check is now green.

check verdict at the time
itest, e2e Load-bearing, and they were the measurement. Correct call.
ci (guards + 13 unit) Skippable — no Go package or guard reads a workflow file.
images ×8 Skippable, and deliberately untouched: it is the canary this change depends on.

What it cost: nothing, but it was nearly wrong in the useful direction — a red e2e on a PR that changes image building is exactly what a regression looks like, and ruling it out took reading main's own history rather than trusting the block.


✅ Measured: the warm build is 23 seconds

The re-run of the flaky e2e job doubles as the first warm measurement, because a pull request writes its own cache scope and a re-run reads it back.

all 8 images
baseline, plain docker build (#19 6442c4c5) 7m34s (itest ingot), 6m08s (itest hilt)
cold + cache write (run 1) 11m21s — ~3 min worse
warm (run 2) 23s
build piri 4s · hilt 5s · ingot 3s · sprue 2s · delegator 2s
      piri-signing-service 2s · swarf 3s · indexing-service 2s

The whole e2e job went 13m10s → 5m29s. (13m10s is #24's e2e on the same base, same hour.)

Three things that keep this honest

  1. The warm run is a re-run of the same commit, so every layer hit. That is the ceiling, not the average. A real pull request changes Go source, which invalidates the go build layer for the services it touches — the base image, apt and go mod download layers (the bulk) still hit, so expect minutes rather than seconds. Worth re-measuring on a real source change.
  2. The cold path is ~3 minutes worse, because mode=max exports every layer. The first run on main after this merges will be slower. Don't judge it on that run.
  3. Nobody has checked the cache size against GitHub's 10 GB per-repository limit. Eight images at mode=max is not small, eviction is LRU, and an overflowing cache degrades quietly back to cold builds.

The ~6 minute image build sits on the critical path of both workflows, and neither could ever have cached it: itest.yml:143 and e2e.yml:118 shell out to plain docker build, which cannot use --cache-from type=gha at all. Only images.yml uses buildx. So the absence of a cache here was never a decision — it was the shape of the command.

The deliberate no-caching decision is images.yml's, and it stays

Its header says why, and the reason is about that workflow:

A cache hit means the build did not run, and right now the point is to provoke build failures, not to avoid them.

That argument holds where the job's product is the build. Here the build is a means to running a suite. And the load-bearing fact: images.yml runs on exactly the same events — push: [main], pull_request, workflow_dispatch — so it has already proved the cold build on the same commit, in parallel.

So images.yml stays cold and stays the canary. If a Dockerfile breaks, it goes red on the same commit and this job's cache cannot hide it. That is the whole argument; if you don't buy it, this PR should be closed rather than amended.

Why docker/build-push-action instead of docker buildx build

type=gha needs ACTIONS_RUNTIME_TOKEN and ACTIONS_CACHE_URL, and GitHub does not export those to run: steps. The usual workaround is a third-party action to export them — a new dependency in a repo that pins carefully. This repo already trusts and uses setup-buildx-action and build-push-action in images.yml, so reusing them adds nothing new.

It also leaves one way to build an image here instead of two that can drift.

Cache scoping

Per service, and shared between the two workflows on purpose: they build the same 8 images from the same commit, so whichever job runs first warms the other. Branch scoping is GitHub's own — a pull request reads its base branch's cache and writes its own.

Verified

Both files build the same 8 images, same contexts, same prod target, same Dockerfiles as the loop they replace — checked by parsing the new YAML and diffing the derived list against the old build <svc> <ctx> lines, not by reading them:

.github/workflows/itest.yml: 8 builds; identical to old shell loop: True
.github/workflows/e2e.yml:   8 builds; identical to old shell loop: True

actionlint clean on all four workflows; all four still parse to their expected jobs.

Follow-up

MONOREPO_TODO.md's "Decide how much further to go on CI wall clock" entry (landed on main via #23) says to re-measure after a warm run. That is now done and the entry should carry these numbers — separate one-file PR, since this branch predates #23.

🤖 Generated with Claude Code

https://claude.ai/code/session_01CGAGAib517Ae1kg8SCdcEt

The ~6 minute image build sits on the critical path of both workflows and
neither could ever cache it: itest.yml:143 and e2e.yml:118 shell out to plain
`docker build`, which cannot use --cache-from type=gha at all. Only images.yml
uses buildx. So the absence of a cache here was never a decision -- it was the
shape of the command.

The deliberate no-caching decision is images.yml's, and its reason is about
images.yml: "the point is to provoke build failures, not to avoid them", and a
cache hit means the build did not run. That argument holds where the job's
product *is* the build. Here the build is a means to running a suite, and
images.yml runs on exactly the same events -- push to main, pull_request,
workflow_dispatch -- so it has already proved the cold build on the same commit,
in parallel. It stays cold and stays the canary; a broken Dockerfile turns it
red where this job's cache cannot hide it.

docker/build-push-action replaces the shell loop rather than raw `docker buildx
build`, because type=gha needs ACTIONS_RUNTIME_TOKEN and ACTIONS_CACHE_URL,
which GitHub does not export to run: steps. The usual workaround is a
third-party action to export them; this repo already trusts and uses
setup-buildx-action and build-push-action in images.yml, so this adds no
dependency and leaves one way to build an image instead of two that can drift.

Cache scope is per service and shared between the two workflows deliberately:
they build the same 8 images from the same commit, so whichever runs first warms
the other. Branch scoping is GitHub's own -- a PR reads its base branch's cache
and writes its own -- so runs on main are what warm it, and the first run after
this merges will be cold.

Verified: both files build the same 8 images, same contexts, same
prod target, same Dockerfiles as the loop they replace -- checked by parsing
the new YAML and diffing the derived list against the old `build <svc> <ctx>`
lines, not by reading them. actionlint clean on all four workflows; all four
still parse to their expected jobs.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CGAGAib517Ae1kg8SCdcEt
Peeja pushed a commit that referenced this pull request Sep 17, 2026
…deadline

#23 re-homes the CI wall-clock measurement that would otherwise live only on
closed #21, plus two findings from reading main: Phase 1 has no release.yml to
use, and tags are gated on the rename because a submodule tag must sit in the
repository its module path names. #24 removes the last two per-service .github
directories. #25 layer-caches the image builds in itest and e2e while images.yml
stays cold as the canary -- the only one of the five open PRs that can fail.

The hazard is on Needs Human Work because it needs a person: forgectl's
metrics-payments and metrics-faults run against environment: mainnet every 30
minutes and 12 hours, pushing to an OTLP endpoint from repository secrets. They
run in the polyrepo today. Archiving fil-forge/forgectl stops them silently, and
reproducing them here needs an environment and secrets configured on this
repository.

Also records what was deliberately not attempted pre-rename and why: the release
workflow (unverifiable until tags can be cut, and an unrunnable workflow is the
silent-green shape this repo keeps deleting), compat.yml (needs published images
to test against), the tag scheme (long-lived, hard to reverse, a recommendation
rather than an agent's decision), and building the Dockerfile.release files
(goreleaser-shaped, they need the release flow first).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CGAGAib517Ae1kg8SCdcEt
Peeja pushed a commit that referenced this pull request Sep 17, 2026
TestUploadAndRetrieve/filesystem has failed 2 of 40 e2e runs, and both are the
same shape: compose up reports a dependency unhealthy, a different container
each time (upload-1 on run 160, plc-1 on run 182), always the filesystem
permutation and never s3. plc-1's log says connect ECONNREFUSED to its postgres,
one restart, then the healthcheck deadline. That is a startup race, not a broken
image.

Run 160 is on main at ff2f794, two and a half hours before any of today's
branches existed, which is what rules #25 out -- it changes how images are built,
so the red looked exactly like a regression until the main failure was found.
The two permutations are t.Parallel(), so ~40 containers start on one runner at
once; both observed failures being filesystem is suggestive of something
systematic, but two data points is not proof.

One re-run queued on #25, the single re-run allowed for establishing that a
failure is not the PR's. The honest fix is compose healthcheck ordering, not a
retry.

Also: main is 144b316 -- Petra merged #22, #23 and #24 while this was running.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CGAGAib517Ae1kg8SCdcEt

Peeja commented Sep 17, 2026

Copy link
Copy Markdown
Author

[From Claude:] e2e went red, and it is not this change. Establishing that before anything else, because this PR changes how images are built, so a red e2e is exactly what a regression here would look like.

It is a pre-existing flake on main

TestUploadAndRetrieve/filesystem has failed 2 of the last 40 e2e runs, and both are the same shape:

run branch / sha when container reported unhealthy result
160 main ff2f794d 14:08Z …-filesystem-upload-1 FAIL /filesystem (59.32s)
182 this PR 7fb93177 16:39Z …-filesystem-plc-1 FAIL /filesystem (62.27s)

Run 160 is on main, two and a half hours before this branch existed.

Both fail identically in compose up — dependency failed to start: container … is unhealthy — always in the filesystem permutation, never s3, and with a different container each time. That is a startup race, not a bad image. plc-1's own log is explicit:

Error: connect ECONNREFUSED 172.18.0.6:5432

It lost the race to its postgres, restarted once (restarts=1), and the healthcheck deadline hit ~200ms after the restart.

The three branches that ran in the same ten minutes

branch e2e
#23 f434a879 success
#24 e8617e6d success
main 144b3162 success
this PR 7fb93177 failure

That distribution is what made it worth ruling out properly rather than assuming a flake — but the main failure at run 160 settles it, and the failure mode is a container this PR does not build. plc is a pulled image, byte-identical across all four runs.

Also green on this PR, with the same change

itest hilt passed (16:43:08 → 16:54:51), and all 8 image jobs passed — the canary this PR's whole argument rests on. guards and all 13 unit jobs passed too.

What I have done

Re-run the failed job once — the single re-run that is for establishing a failure is not the PR's, not for wishing it away. I am not going to re-run it twice.

Recorded the flake as debt on the wiki, because it is worth more than this PR: a 5% flake in the one suite that exists to catch flakiness will keep costing exactly this diagnosis. The likely mechanism is that the two permutations are t.Parallel(), so ~40 containers start on one runner at once and healthchecks starve. That both observed failures are filesystem hints at something systematic rather than symmetric — but two data points is not proof, and I am not going to pretend otherwise.

The honest fix is compose depends_on / healthcheck ordering, not a retry, and it is out of scope here.

What I have not done

Changed anything in this PR. The diff is still the two workflow files, and the measurement it exists to produce is still pending a warm run after merge.


Generated by Claude Code

Peeja pushed a commit that referenced this pull request Sep 17, 2026
The re-run of the failed e2e job passed, which confirms the flake account, and
it was warm -- so it doubles as the measurement #25 exists to produce. All 8
images built in 23s against a 7m34s baseline; the e2e job went 13m10s to 5m29s.

Kept honest: that run is a re-run of the same commit, so every layer hit and it
is the ceiling rather than the average -- a real source change invalidates the
go build layer for the services it touches, though the base, apt and go mod
download layers still hit. The cold path is about 3 minutes worse than before
because mode=max exports every layer, so the first run on main after merging is
slower by construction. And nobody has checked the cache size against GitHub's
10 GB per-repository limit, where eviction is LRU and an overflow degrades
quietly back to cold builds.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CGAGAib517Ae1kg8SCdcEt
Peeja pushed a commit that referenced this pull request Sep 17, 2026
Petra merged #20, #17, #22, #19, #23 and #24; main is 144b316 and the open
list is #25 and #26. Dropped the merged rows from both tables rather than
letting them accumulate.

#25's e2e red turned out to be the pre-existing filesystem flake, and the one
re-run that cleared it was warm -- so it produced the measurement as well as the
green: 8 images in 23s against a 7m34s baseline, and the e2e job from 13m10s to
5m29s. #26 puts that in MONOREPO_TODO.md with the caveats attached rather than
the headline alone.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CGAGAib517Ae1kg8SCdcEt
@Peeja
Peeja merged commit 95e8366 into main Sep 17, 2026
22 of 23 checks passed
@Peeja
Peeja deleted the claude/ci-cache branch September 17, 2026 17:25
Peeja pushed a commit that referenced this pull request Sep 17, 2026
Chasing #25's e2e red to the bottom found a real bug. pg_isready without -h
probes the Unix socket; the official postgres image runs initdb against a
temporary server started with listen_addresses='', so the probe passes while TCP
is still refused. plc-postgres's log against plc's crash shows the healthcheck
going green 2.25 seconds before the port existed, and plc booting into the gap.

That explains the shape that made it look like a generic race: plc already
declared condition: service_healthy, so the ordering was never wrong -- the
readiness signal was, and whichever dependent happened to boot inside the window
was the one that failed. Hence a different container each run.

#27 fixes six sites and adds a list-free guard that also covers Go, because the
generator builds one of these strings, and that fails if it ever finds nothing
to check. Postgres was the only class member; every other healthcheck is HTTP
over localhost or redis-cli ping, TCP by construction.

Recorded with what is still unproven: the mechanism, yes; the frequency, no. A
5% flake cannot be shown fixed by one green run.

Also: main is 95e8366, #25 and #26 merged, the layer cache is live.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CGAGAib517Ae1kg8SCdcEt
Peeja pushed a commit that referenced this pull request Sep 17, 2026
…heck

The previous commit updated Current State but its companion edit failed before
writing, so this page still claimed five open PRs and #25 unmerged. Now: #27
only, with #25 and #26 merged and the cache numbers carried across.

Names the one thing on that list the agent genuinely cannot do rather than has
not done: checking the cache against GitHub's 10 GB per-repository limit needs
gh cache list or the Actions cache API, and neither is reachable from here. It
matters because eviction is LRU and an overflowing cache degrades quietly back
to cold builds, which is the silent regression shape this repo keeps deleting.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CGAGAib517Ae1kg8SCdcEt
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants