ci: cache FRR's compiles in the CI image build with ccache

[kkroo]

Why

Every Build leg recompiles all of FRR. FRR_SOURCE_REV changes per commit, so the image's compile step (bootstrap, configure, make -j) and its make check step rerun from scratch even when a commit touches one file. On a 22.04 leg that is ~355-370 s of compile and ~265-280 s of make check, out of an 11-12 min leg, now that the dependency layers above them come from the registry cache (#134).

What

• docker/ubuntu-ci/Dockerfile installs ccache 4.14.1 (musl-static, sha256-pinned per arch, fetched like strace). Not Ubuntu's package: --enable-gcov compiles everything with -fprofile-update=atomic, which ccache only accepts from 4.10. jammy's 4.5.1 and noble's 4.9.1 log "Unknown profiling option" and run the compiler uncached (1360 of ~1490 compiles on a 22.04 build). The first attempt at this used the distro package and saved nothing; that is how this was found.
  • Only make and make check run with ccache first in PATH (/usr/local/lib/ccache masquerade links). configure runs without it, so the image's Makefiles record plain gcc and Unit-Test's make check from the image never touches ccache. configure through ccache cost 55 s on a 4-CPU builder.
  • Preprocessor mode with -DCONFDATE=* ignored. configure stamps today's date on every -dev compile line, which would make yesterday's cache useless. Ignoring it is only safe in preprocessor mode: bgpd/bgp_route.c and zebra/zebra_cli.c test #if CONFDATE > N (FRR's deprecated-code expiry), and their preprocessed output changes when the date crosses N, so they still miss. Direct mode would serve a stale object and hide the expiry.
  • The cache lives in a BuildKit cache mount. It is seeded from a ccache-seed build context (empty by default, so a plain docker build still works) and exported by a ccache-out target. After make check, every entry this build did not hit or write is evicted (--evict-older-than since a start marker; a hit refreshes the mtime, and the seed copy keeps its timestamps), so the saved cache is one build's worth: 258 MB on 22.04, unchanged from one build to the next.
• github-ci.yml: Build and Build-LTTng restore the cache from actions/cache (frr-ccache-v1-<platform>-<sha>, falling back to the newest entry for the platform) and pass it as the build context. Only master Build runs export and save it; pull requests only restore. Build-LTTng stays a step-prefix copy of Build (test_buildcache_freshness), so it never saves.
• buildcache-seed.yml: the seed's three legs restore and save the same way, which is what keeps amd64_u24_lttng fresh.

Measurements

devbox, docker-container builders pinned to 4 CPUs / 16 GB like arc-dind, 22.04, cache injected through --build-context exactly as CI does. Warm = cache from a build of master 68f9499907, then a build of master plus a one-line change in bgpd/bgp_route.c on a fresh builder. Warm and baseline ran at the same time on disjoint CPUs so both saw the same host load.

	compile step	make check	cache hits
no ccache (today)	367.9 s	281.2 s	n/a
warm ccache	277.8 s	233.6 s	1148/1149, 102/102
cold ccache (no seed)	469.2 s	301.7 s	0

~138 s per Build leg when the cache is warm. Eviction after the warm build removed exactly one entry, the edited file's old object.

Costs and limits

• A miss costs more than no ccache. ccache preprocesses before it compiles, so the cold build is ~100 s slower than today. A pull request starts from the newest master entry and pays this only for what it changes. A change to a header half the tree includes gets close to the cold number.
• This PR's own CI is a cold run. No master entry exists until it merges: pull requests can read master's entries but never write, and this PR's legs will look slower than today. The first master run after merge fills the cache, and pull requests opened after that get the warm numbers.
• actions/cache budget. About 260 MB per entry, 3 platforms, saved per master merge and per seed run (twice a day). GitHub evicts the least recently used entries above 10 GB; the MIB cache is restored by every 24.04 Build, so it stays recent.
• Restore time on arc-dind (a ~260 MB download per leg) is not measured yet; the first warm pull request run will show it.

Testing

• .github/scripts unit tests: all modules pass, including test_buildcache_freshness (85 tests). test_require_ally_review was not run to completion: it hangs on macOS on master as well, and this change does not touch it.
• check-runner-labels.py OK; both workflow files parse as YAML.
• Dockerfile cold path (no --build-context) builds; warm path verified as above.
• ccache 4.14.1 restores objects and .gcno files byte-identical on a hit (checked on FRR's own compile lines with --coverage -fprofile-update=atomic).

🤖 Generated with Claude Code

[allyblockcast]

Ally — Consolidated PR Review

Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex (applied directly; nested CLI not launched in the k8s Job runtime).
Reviewed head: 2cdb4e2

The design is sound and unusually well evidenced — the preprocessor-mode reasoning about -DCONFDATE is correct, and the eviction-by-mtime trick is the right way to keep the exported entry one build wide. All three findings are about what happens when the optimization misbehaves, which this change leaves unguarded in three places.

Critical Issues (0)

None.

Important Issues (3)

• [gstack/review] docker/ubuntu-ci/Dockerfile:363 — The safety of CCACHE_IGNOREOPTIONS=-DCONFDATE=* rests entirely on CCACHE_NODIRECT being in effect, and nothing in the build asserts that it is. The comment at Dockerfile:342-348 states the invariant precisely and correctly — direct mode plus an ignored -DCONFDATE serves a stale object across FRR's #if CONFDATE > N deprecation thresholds — but it is enforced only by an ARG default sitting three lines away. Both are overridable from the command line, and the two are coupled in a way a later editor has no way to see from either line alone. The failure mode is the worst one available here: not a build error, but a silently stale .o (and .gcno) inside an image that feeds Unit-Test, the topotest shards and coverage. It is also undetectable after the fact, because the cached object compiles and links cleanly.

  • Assert it where it is used, next to the existing ccache -z in the compile RUN (Dockerfile:375): ccache --get-config direct_mode | grep -qx false (or ccache -p | grep -q 'direct_mode = false'). One line, fails loudly at build time, and it keeps the comment and the behaviour from drifting apart.
  • Secondary, worth confirming before relying on the ARG as a switch: CCACHE_NODIRECT is a CCACHE_NO*-form boolean, for which ccache keys on the variable being set rather than on its value. If so, --build-arg CCACHE_NODIRECT=0 reads to a human as "direct mode on" and still leaves it off — harmless in itself, but it means the ARG cannot be used to flip the invariant and the assertion above is the only honest guard.

• [native-codex] .github/workflows/github-ci.yml:456, .github/workflows/buildcache-seed.yml:239 — Save ccache has no guard that Export ccache actually produced anything. du -sh at github-ci.yml:455 prints the size but does not gate on it, so an empty export is saved as a normal entry. Because every consumer reaches the cache through restore-keys: frr-ccache-v1-<platform>- (github-ci.yml:367), which returns the newest prefix match, one empty entry becomes the entry every subsequent PR restores. The Dockerfile then takes the no-seed branch (Dockerfile:371) and compiles cold — which by your own measurement is ~100 s slower per leg than before this PR (469 s vs 368 s), not a neutral fallback. It persists silently until the next master save.

  • The sha-keyed key: protects the common collision cases, but not all of them. Reachable chain: master is quiet for several days (plausible at ~2 merges/day against the 10 GB budget and ~2.6 GB/day of new ccache entries); the actions/cache entry for the current master sha is evicted by LRU; buildcache-seed runs again on that same unchanged sha with an empty restored seed, so the compile layer is a full cache-from: type=registry hit (buildcache-seed.yml:216), the frr-ccache mount on the fresh builder is never written, the export is empty — and the previously-occupied key is now free, so it saves.
  • Guard the save rather than the export: if: ... && hashFiles(format('{0}/frr-ccache/**', runner.temp)) != '', or set a step output from the export ([ -n "$(ls -A "$dir")" ]) and gate on it. Failing to save is the correct outcome when there is nothing to save.

• [pr-review-toolkit:errors] .github/workflows/github-ci.yml:435 — Export ccache is a pure optimization, but it can turn master Build red, and its blast radius is larger than the cache. It carries no continue-on-error, and it sits before Clear any previous results (github-ci.yml:461) and the Save cleared previous results (shard k) uploads. So a transient failure in the export build — a builder hiccup, a docker buildx error, no space for the 258 MB local output — fails the job after the image has already been built and pushed, and skips the shard seeds that Test depends on for its re-run semantics. This inverts the principle the restore step states at github-ci.yml:365: "A miss is a cold compile … never a failure." That should hold on the write side too.

  • continue-on-error: true on Export ccache (the same treatment buildcache-seed.yml:174 already gives its MIB restore), paired with the non-empty guard above so a half-finished export cannot then be saved. Same in buildcache-seed.yml:218.

Suggestions (3)

• [pr-review-toolkit:code] .github/workflows/github-ci.yml:446, .github/workflows/buildcache-seed.yml:229 — The run: block does not harden the shell. GitHub's default bash -e gives errexit but not nounset or pipefail; elsewhere in this workflow (e.g. the MIB populate step) the repo opts into all three explicitly.
• [pr-review-toolkit:comments] .github/scripts/test_buildcache_freshness.py:476 — The docstring still reads "a hand copy of Build's first five steps" while the workflow comment it documents is updated to "first seven" (github-ci.yml:508). The assertions themselves are prefix-based and count-agnostic, so nothing breaks — only the docstring drifts.
• [gstack/review] docker/ubuntu-ci/Dockerfile:367 — The cache mount id=frr-ccache is not keyed by Ubuntu release or LTTng flag. This is correct exactly as long as each matrix leg gets its own builder, which is what arc-dind plus a per-job setup-buildx-action gives you today. Since that assumption is load-bearing — two legs sharing a daemon would mix 22.04 and 24.04 entries into one mount, serialize on sharing=locked, and have each leg's --evict-older-than delete the other's work — it is worth one line of comment recording it next to the mount.

Strengths

• The -DCONFDATE analysis is the strongest part of this change: identifying that preprocessor mode makes the ignore safe because #if CONFDATE > N alters preprocessed output, and that direct mode would not, is exactly the distinction that matters, and it is written down where the next reader will need it.
• Pinning ccache by sha256 per arch rather than taking the distro package, with the measurement that motivated it (1360 of ~1490 compiles running uncached on jammy's 4.5.1), turns what would read as gratuitous vendoring into an obvious necessity.
• --evict-older-than against a start marker is a neat solution to a real problem — keeping the saved entry one build wide instead of letting it accrete toward CCACHE_MAXSIZE — and cp -R --preserve=timestamps for the seed is the non-obvious detail that makes it work.
• The PR description states the costs honestly, including that this PR's own CI is a cold run and that a header-wide change approaches the cold number. Reviewing against measurements rather than claims is what made the three findings above tractable.
• test_shared_steps_are_identical genuinely enforces the Build/Build-LTTng lockstep the comments assert, so the two new steps cannot drift between the jobs.

Recommended Action

1. Address Important issues this cycle.
2. Consider Suggestions opportunistically.

[kkroo]

Lease: kkroo drive session 0f0c8bcc, 2026-10-05T09:1xZ, ~45m, at 2cdb4e2b7da9512f16d9a9fff278ad2f2c2b5f9f. Taking Ally's three 08:57Z Importants:

• assert direct_mode = false in the compile RUN, next to ccache -z;
• gate Save ccache on a non-empty export;
• add continue-on-error to Export ccache, in both github-ci.yml and buildcache-seed.yml.

Fast-forward push, no force.

[kkroo]

@allyblockcast please review exact head b86224e51c118ea36df1089afda28139fd09899c and publish one consolidated review with explicit Critical Issues / Important Issues counts and a Reviewed head: attestation.

This disposes of the three 08:57Z Importants on 2cdb4e2b:

1. Dockerfile:363 direct mode. The compile RUN now asserts ccache --get-config direct_mode | grep -qx false next to ccache -z.
   • Checked in ubuntu:22.04 (ccache 4.5.1) and ubuntu:24.04 (4.9.1): it passes with CCACHE_NODIRECT=1 and fires with the variable unset.
   • On your secondary point: ccache rejects CCACHE_NODIRECT=0 outright ("invalid boolean environment variable value"), so the ARG is not an on/off switch. The assert catches that case too, and the comment now says so.
2. Empty export saved. Export ccache writes a nonempty output only after a complete export that produced files, and Save ccache requires steps.ccache-export.outputs.nonempty == 'true'. This is in both github-ci.yml and buildcache-seed.yml.
3. Export can fail the job. Export ccache gets continue-on-error: true in both workflows. Because of (2), a half-finished export can't then be saved.

actionlint reports the same 9 findings as on 2cdb4e2b, all on pre-existing lines. The 3 suggestions are not addressed in this push.

[allyblockcast]

Ally — Consolidated PR Review

Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex (applied directly; nested CLI not launched in the k8s Job runtime).
Reviewed head: b86224e

All three Importants from the previous head are fixed, and each fix is the one that was asked for rather than a restatement. The direct_mode assert in particular is fail-closed by construction: if a future ccache changed --get-config output, grep -qx false stops matching and the build fails loudly — it cannot degrade into silently permitting direct mode, which is the only way this guard could have been worth less than the comment it replaces.

Prior Findings Dispositioned (3)

• prior:2cdb4e2 important 1 — fixed — docker/ubuntu-ci/Dockerfile:379 — The compile RUN now asserts the invariant where it is used: ccache --get-config direct_mode | grep -qx false || → exit 1 with a message naming CCACHE_IGNOREOPTIONS=-DCONFDATE=* and stale objects. It sits after ccache -z (:378) and before ./bootstrap.sh, so it fires before any compile reads the cache, and the comment at :348-350 now states both that the assert exists and that CCACHE_NODIRECT is set-based rather than an on/off switch. The make check RUN (:411) carries no second copy, which is correct — CCACHE_NODIRECT is a stage-scoped ARG (:365), so both RUNs see one value and one assert covers both.
• prior:2cdb4e2 important 2 — fixed — .github/workflows/github-ci.yml:471 — Save ccache is now gated on steps.ccache-export.outputs.nonempty == 'true', and that output is written only inside the if [ -n "$(ls -A ...)" ] branch at :466, after the export build returned. Same pair in buildcache-seed.yml:254 / :249. The empty branch emits a ::warning:: instead, so the no-save is visible rather than silent. The composition with the fix below is the part that matters: under GitHub's default bash -e, a failed export exits before the GITHUB_OUTPUT write, so nonempty is unset and Save is skipped — a half-finished export cannot become the newest entry that every restore-keys prefix match returns.
• prior:2cdb4e2 important 3 — fixed — .github/workflows/github-ci.yml:454 — Export ccache carries continue-on-error: true, and buildcache-seed.yml:237 likewise. The step keeps its position before Clear any previous results, which is now harmless: a transient export failure no longer fails the job after the image is built and pushed, so the shard-seed uploads that Test depends on still run. This restores the symmetry the restore step asserts at github-ci.yml:365 — a miss is never a failure — on the write side.

Critical Issues (0)

None.

Important Issues (0)

None.

Suggestions (3)

• [pr-review-toolkit:comments] .github/workflows/github-ci.yml:522 — The comment now reads "a COPY of Build's first seven", but Build-LTTng has eight steps (Checkout, Ensure local MIB cache directory exists, Restore cached MIB files, Ensure ccache seed directory exists, Restore ccache, Set up Docker Buildx, Log in to Harbor, Build docker image (cached)), and they are a prefix of Build's fourteen. The count was already off by one before this PR ("five" against six), so incrementing it by two preserved the error rather than introducing it — but test_buildcache_freshness.py:476 still says "first five", so the two statements now disagree with each other as well as with the code. The assertions are prefix-based and count-agnostic (list(build)[: len(names)]), so nothing breaks; this is only the number a reader checks against.
• [pr-review-toolkit:code] .github/workflows/github-ci.yml:455, .github/workflows/buildcache-seed.yml:238 — The new run: blocks do not harden the shell. GitHub's default bash -e gives errexit — which the nonempty guard above correctly relies on — but not nounset or pipefail; elsewhere in this workflow the repo opts into all three explicitly. Carried from the previous head; you noted it is deliberately deferred.
• [gstack/review] docker/ubuntu-ci/Dockerfile:370 — The cache mount id=frr-ccache is still not keyed by Ubuntu release or LTTng flag. Correct exactly as long as each matrix leg gets its own builder, which arc-dind plus a per-job setup-buildx-action gives you today. Worth one line of comment recording that assumption next to the mount, since two legs sharing a daemon would mix 22.04 and 24.04 entries, serialize on sharing=locked, and have each leg's --evict-older-than delete the other's work. Also carried and deliberately deferred.

Strengths

• The three fixes compose rather than merely coexisting: continue-on-error alone would have let a half-written directory be saved, and the nonempty guard alone would have left the job red. Fixing them as a pair, and saying so in the comment at github-ci.yml:448-451, is what makes the write side genuinely optional.
• The assert is fail-closed. ccache --get-config direct_mode | grep -qx false can only stop matching, never match while direct mode is on, so no future ccache version can turn this guard into a silent no-op — which is the right property for something standing in front of a stale-object failure that compiles and links cleanly.
• Documenting that CCACHE_NODIRECT=0 is rejected as an invalid boolean (Dockerfile:349-350) closes the secondary point honestly: it records that the ARG is not a switch, instead of leaving a reader to assume it is.
• The Build-LTTng lockstep survived the change — I ran test_buildcache_freshness.py against this head's three workflow files and all 85 tests pass, with the two new steps byte-identical between Build and Build-LTTng and still a prefix of Build's.
• Deriving the eviction window from the frr-build-start marker degrades safely: if the marker is ever absent (compile layer cache-hit, make check miss), bash coerces the empty stat substitution to zero and the window becomes ~56 years, so the command evicts nothing rather than evicting everything just written. I checked this rather than assuming it; the failure direction is the harmless one.

Recommended Action

1. No blocking changes requested.
2. Merge once the remaining required CI checks finish green.