ci(390): shared CI toolchain image + drop 110s apt-ffmpeg + rebalance runner lanes #399
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: timothy/ersatztv#399
Reference in New Issue
Block a user
fixes #390
Builds the shared CI toolchain image (.NET 10 SDK + Node 22 + prod-identical ffmpeg) and converts the five toolchain jobs to run inside it via
container:.The scope grew beyond the issue text because I measured the jobs first. Full evidence in #390's first comment; the short version is that the issue's premise was partly wrong, so this PR ships the image plus the larger wins the audit actually found, and files the two it shouldn't bundle.
What's here
The image —
docker/ci/Dockerfile+.gitea/workflows/ci-image.yml→ersatztv-ci:<sha>.Layered
FROM ersatztv-ffmpeg:8.1.2+COPY --from=dotnet/sdk:10.0-noble-amd64 /usr/share/dotnet, the same patterndocker/Dockerfileuses for prod (our ffmpeg base islinuxserver/baseimage-ubuntu:noble— the same Ubuntu release as the SDK image, so glibc/ICU match). Already built and verified on the registry: node v22.23.1, ffmpeg 8.1.2, dotnet-ef 9.0.12, reportgenerator all resolve inside the pushed artifact.Rolled-up wins (found by auditing every job's steps, per Timothy's steer):
functional-e2erun. It was never load-bearing. The app's only startup ffmpeg touch is aLogWarningfromFFmpegLocatorService; the readiness line has zero ffmpeg dependency; the harness drives curl-only contracts that never transcode; andgit log -Sshows it was added defensively in the harness's own commit, never fixing an observed failure. The image ships prod-identical ffmpeg regardless — the fidelity #299's seeded-media follow-ups will need.dotnet-ef+reportgeneratorinto the image — two per-rundotnet tool installs gone, versions now in one tracked place.api-docs(5s) andformat(37s) to the idlesmalllane. They were each queueing ~29 min in the saturatedubuntu-latestlane. This also drops that lane from 5 jobs to 3, shortening the queue for the three heavy jobs. Only possible becausecontainer:makes them self-contained.Filed, not bundled:
dotnet buildruns 3× on the same commit (183+262+297 = 742s). The real code-side lever, but it's a genuine wall-clock-vs-total-CPU trade-off needing its own design.Honest ROI
The image itself is worth only ~15–40s/job (≈3–8%) —
setup-dotnetis 8–19s, not the minutes the issue assumed. Its durable value is prod-identical ffmpeg, a pinned toolchain, and making jobs runner-agnostic (which is what unlocked the lane rebalance). The 110s ffmpeg deletion and the lane move are the bigger immediate wins here.Notes for review
:<sha>, never:latest— a bad toolchain push would otherwise break all five jobs at once. Repeated per job becausejobs.<id>.container.imagecan't read the workflowenvcontext; bumping is a documented two-step.actions/checkout+e2e-local.sh'sgit rev-parse), and resetting the ffmpeg base'sENTRYPOINT ["ffmpeg"].setup-node's implicit npm cache is replaced with an explicitactions/cacheon~/.npm; NuGet cache unchanged. Project deps stay cached — only the toolchain moved into the image.dotnet --info, node, ffmpeg, …) so a broken toolchain fails the image build rather than a CI run. It already earned its keep: it caught thatreportgenerator --versionexits 1.migrations— it's the one job with aservices:container; watch that it still reachesmysql:3306from inside the job container.Docs
docs/ci-cd.md: new "CI toolchain image" section (what it ships, layering, the two-step bump, the container gotchas, honest ROI) + lane-assignment table and thefunctional-e2effmpeg correction.The build-time smoke test failed the image build even though the toolchain was healthy: reportgenerator prints its version banner then exits 1 ("No report files specified"), so it has no clean version probe. Probe the dotnet-tool shim on PATH instead. dotnet --info confirmed the SDK + .NET 10.0.10 runtimes resolve correctly on the ffmpeg base (ICU/glibc fine). Refs #390The migrations job died in 0.13s with 'set: Illegal option -o pipefail'. Root cause: inside a container: act_runner does not default to bash, it falls back to 'sh -e {0}' (dash), since it cannot assume bash exists in an arbitrary image. Every multi-line script in this workflow is bash (set -o pipefail, arrays, shopt, mapfile), so dash fails them immediately. The format job passed only because it already declared shell: bash explicitly. Declare it once at the workflow level instead of per step; non-container jobs already defaulted to bash, so nothing changes for them. Documented in docs/ci-cd.md as a container gotcha — it surfaces only once a job is containerized and does not look like a shell problem. Refs #390Cold adversarial review — reconciliation
Ran an independent cold-context review agent (scoped "review only") over the diff. It returned BLOCKED with 1 Blocker, 1 High, 2 Medium, 3 Low, 3 Nits. It earned its keep: it verified claims against the actual pushed image rather than reasoning from the Dockerfile, and it independently reproduced the Blocker I'd hit in CI. All findings are addressed below.
🔴 Blocker —
container:flips the default shell tosh(dash) — FIXED (84756eeb)Independently confirmed by CI, not just argued: inside a
container:, act_runner defaultsrunsteps tosh -e {0}, not bash (it can't assume bash exists in an arbitrary image — nektos/act#1809). Every bashism dies withset: Illegal option -o pipefail. Run 986 evidence:migrationsfunctional-e2etestcontinue-on-error: trueThe
testcase is the one that matters: without this fix we'd have merged a green pipeline whose coverage reporting was silently dead.formatpassed only because it already declaredshell: bash.Fixed with a workflow-level
defaults: run: shell: bash. The reviewer suggested job-level, doubting workflow-level support in act — empirically settled: workflow-level works (all previously-failing steps now execute; final run linked below). Workflow-level is one declaration instead of five and covers future jobs, so I kept it. Documented indocs/ci-cd.mdas a container gotcha, since it only appears once a job is containerized and looks nothing like a shell problem.🟠 High — MySQL-from-inside-a-job-container unproven — BEING PROVEN
Correct:
migrationsdied at the SQLite step, so theservices: mysqlresolution never ran. Not merging until that's green — see the final run.🟡 Medium — the weekly cron was a no-op, twice over — FIXED (
07048b8c)Sharp catch, and both halves were right: (1) jobs pin
:<sha>, so nothing consumes the rebuilt image; (2)mode=maxbuildcache would restore theapt-getlayer verbatim and collect none of the base updates the cron existed for. It was burning a weekly ~2GB build for zero effect while the docs asserted otherwise.Rather than delete it, I kept the one real benefit and made it honest: added
no-cache: ${{ github.event_name == 'schedule' }}, and rewrote the comment + docs to state what it actually is — a build canary (catches "the toolchain no longer builds" at a time of our choosing, not when you next need it) plus a freshly-patched:latestfor the next pin bump. It explicitly does not update running jobs.🟡 Medium — Renovate actively generates pin drift — FIXED with a mechanical guard (
07048b8c)The best finding. Renovate manages
docker/ci/Dockerfile's base pins but cannot bump an opaque:<sha>incontainer.image— so a Renovate base bump would publish a new image, run its jobs against the old pin (CI green-lighting a toolchain it never executed), and merge with the Dockerfile disagreeing with the pin. Silent, recurring, and inevitable.Given this repo gates by construction rather than memory (H6/H9/H10/H11/H12), a doc note wasn't enough. Added a blocking
ci-image-pinjob (smalllane, seconds) that fails when the pin isn't the last commit to touchdocker/ci/**/ci-image.yml, or when the five jobs pin different tags. It makes the two-step enforced.It also caught a bug in itself before CI did: comparing abbreviated shas fails always, because git auto-scales abbreviation length with object count — the tag built from
fetch-depth: 1is 7 chars,%hon the guard's full clone is 8. Now resolves both sides to full shas. Verified non-vacuous locally: PASS on the real pin, FAIL on a stale pin, FAIL on an unresolvable one.I did not take the content-hash-tag alternative: it would race
ci-image.ymlagainstdocker-build.ymlon the same push (jobs pulling a tag that isn't pushed yet).🟢 Low — all three FIXED (
07048b8c)FFMPEG_TAGnever existed — I referenced an ARG I never introduced, in both the Dockerfile and the docs. Reworded to name theFROMline.paths:claim mismatch — filtered the exact file while claimingdocker/ci/**. Now genuinelydocker/ci/**.main's history and the new guard keeps it honest.⚪ Nits
ENTRYPOINT []isn't needed — right, act overrides it (entrypoint=["/bin/sleep" "10800"]). Kept as belt-and-braces for running the image by hand, but the docs no longer sell it as a real gotcha (marked defensive, not load-bearing) — the reviewer was right that it sat misleadingly next tosafe.directory, which genuinely is.LD_LIBRARY_PATH,VIRTUAL_ENV,MAKEFLAGS) — accepted knowingly: prod copies the runtime onto this same base, so CI now inherits prod's env, which is the fidelity we want.22.x-per-run → NodeSource-per-image-rebuild) — accepted; frozen-per-image-sha is the better property.Coverage gap — being closed
The reviewer flagged that
api-docs/formatpassed only via their trivial skip paths (this PR touches no.csand no API path), sodotnet format --includeandupdate-openapi.shhave never actually run inside the image. Fair. Closing that with a temporary commit that touches an API-surface.cs— exercising both full paths — then reverting it.What the reviewer checked and found sound
Worth recording: it exec'd the pushed artifact on bumblebee and confirmed every binary the converted jobs invoke is present (
awk sed grep tar zstd curl git python3 jq node npm dotnet ffmpeg ffprobe reportgenerator dotnet-ef …); thatsudois correctly absent now its only consumer is gone; that no JRE is needed (the openapi-generator jar is the prod image's build stage —update-openapi.shisdotnet+python3only); that the npm cache replacement resolves correctly (npm config get cache→/root/.npm, HOME=/root) with no job forgotten; and that thesmalllane move doesn't uncap anything (Memory:10g NanoCPUs:4— same cap as bumblebee-runner).Verification complete — all review findings closed, full run green
CI: run 993 @
ed52892d— every job greentest✅ ·migrations✅ ·functional-e2e✅ ·api-docs✅ ·format✅ ·ci-image-pin✅ ·docs-reminder✅ ·decisions-guard✅ (buildcorrectly skipped on PR)The two open review items are now settled with evidence, not argument:
migrationsis green end-to-end; the MySql step ran andServer=mysql;Port=3306resolves from inside the job container onci-net. This was genuinely unproven before (the job had always died at the earlier SQLite step).shell: bash --noprofile --norc -e -o pipefail {0}— workflow-leveldefaultsworks on act_runner v0.6.1, settling the reviewer's doubt empirically.functional-e2ereports "all functional-E2E assertions passed", booting with the image's prod ffmpeg and zero apt-ffmpeg occurrences. The coverage summary, previously dead-but-green, now reports "Found 6 coverage report(s)" and runs reportgenerator.Coverage gap — CLOSED (throwaway PR #400, now closed; branch deleted)
The reviewer was right that
api-docs/formathad only ever taken their skip paths. Proven on a throwaway branch (#399's head + a comment onErsatzTV/Controllers/Api/ArtistsController.cs) so this PR's head stayed untouched:format:cs_changed=true→ detectedArtistsController.cs→ randotnet format --includeinside the image → green.api-docs:API surface changed→ ran./scripts/update-openapi.sh(needsdotnet build+python3) → "Generated API artifacts are in sync."Both real paths execute correctly in the CI image.
Measured result (baseline run 978 → run 993)
api-docsformatmigrationsfunctional-e2etestWorst-job wall-clock: 34 min → 18 min.
Caveats, stated plainly: n=1 per side, and the large runtime drops on
migrations/functional-e2eare substantially reduced lane contention, not toolchain savings — I am not claiming those for the image. The structural, non-confounded wins are: the queue collapse onapi-docs/format(~29 min → seconds),format's 41s → 7s (setup-dotnet gone), andfunctional-e2e's −110s (apt-ffmpeg gone). The image itself remains worth ~15–40s/job, exactly as the audit predicted. Queue wait (server-management#604) and the 742s triple-build (#398) remain the dominant costs and are untouched by design.Review-verdict: MERGEABLE @ ed52892d0aa4e6f0e6c9a0c40ff33bb4e2fa9d8f
Re-posting the verdict in the short-sha form the H10 gate matches on (my previous comment used the full 40-char sha, which the gate correctly refused to accept as referencing the head).
Scope of this verdict — the cold adversarial review covered the original diff, and I have since re-reviewed every fix commit on top of it (
84756eebbash defaults,07048b8creview fixes,ed52892dpin bump + guard fix), per the "re-review the fix commit, not just the initial diff" rule (#242). Evidence for each is in the two comments above: full run 993 green (including the previously-unproven MySQL-from-container path), both previously-skipped full paths proven on throwaway #400, and the pin guard verified non-vacuous with negative controls.Review-verdict: MERGEABLE @
ed52892