ci: say which boards failed and why, not just that the nightly is red - #2297
ci: say which boards failed and why, not just that the nightly is red#2297openipc-ai wants to merge 2 commits into
Conversation
build.yml builds ~99 boards with fail-fast: false, and one failing cell turns the run red. #2271 asked whether that is the signal we want. It is, and the reasoning is now written into ci-gate rather than left as the default it started as: the nightly's product is a complete set of images, and a board that failed leaves `nightly` and `latest` holding yesterday's image for it, so green would be a lie about what got published. The transient classes are already absorbed by the seven-attempt backoff (#2036), and the same gate serves pull requests, so relaxing it to quieten the nightly would relax branch protection with it. What was actually wrong is that the colour was the only output. On 2026-08-12 (run 31649720825) a majestic-webui fixup did a bare rm on two files upstream had deleted and 94 of 99 boards failed in the same place — establishing that took opening 94 job logs, because the run said, in full, "failure". So: name the cause where the evidence is, and aggregate it once. - The retry loop now classifies each failure out of the build log it already has. Buildroot names the package and the phase in exactly one line, and the phase is the class (.stamp_downloaded is a mirror or a 404, anything later is real code). That goes into the ::error:: and ::warning:: it already emitted. - A new Build summary job renders the run: N of M failed grouped by cause, the flakes the backoff absorbed (invisible until now, and the thing that predicts the nightly going red again), headroom warnings, and a per-board strip over the last 14 scheduled runs so "red three nights running" is visible without digging through run history. Nothing is stored; it is three read-only API endpoints and ~130 requests, and the footer prints the count it used. - Reporting only. It cannot fail the run — that would be the exact mistake #2271 decided not to make — so it is not in ci-gate's needs and its step swallows a non-zero exit into a warning. What keeps it honest instead is build-summary.py --self-test in lint.yml, a merge gate that asserts the grammars still match the annotations build.yml emits: a reworded ::error:: would otherwise reduce the summariser to reporting nothing, which looks exactly like a quiet night. Verified before merge, since none of this is reachable from PR CI: the classifier extracted from build.yml gives majestic-webui-dist/target_installed on the real 2026-08-12 log, that exact annotation parses back to the same cause and 7 attempts, and the summariser was dry-run against runs 31649720825 and 32534401852 (which surfaced gk7205v300_lite at 0KB free of 5120KB). build-summary.py goes in ci-matrix.py's NO_BUILD_SCRIPTS: it cannot change a byte of an image, and unclassified it would widen every PR touching it to the full 99-board matrix. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
PR Summary by QodoCI: add per-board failure causes and a nightly build summary
AI Description
Diagram
High-Level Assessment
Files changed (4)
|
Code Review by Qodo
1.
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
…elf a nightly Review on #2297. - Api.calls was incremented from up to eight worker threads without a lock. `+=` is load-add-store, not one step, so increments can be lost. An under-report is the one wrong answer this counter must not give: its whole job is to make the API budget measured rather than assumed. - Query strings were concatenated by hand. Every value passed today is a bare word or an int, but urlencode is one line and the helper should not be a trap for the next parameter. - The failures-by-cause table rebuilt set(boards) once per element of the group. Grouping the result objects instead of the board names drops the set entirely and reads better. - preflight computes build_id as nightly-<date>-<sha> for every event, so a pull request's summary was headed with the name of a nightly release that does not exist. Only the events that publish one use it now; everything else falls back to the run id, and report_title() is covered by --self-test. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Thanks — all three findings were real. Fixed in 0fce083, plus one wart of my own that the review made me look at. 1. Racy API counter. Correct, and it matters more than a lost integer usually would: the counter exists so the API budget is measured rather than assumed, and an under-report is precisely the wrong answer — it would say the job is cheaper than it is, right up until it crowds 2. Unencoded query parameters. Every value passed today is a bare word or an int, so nothing is broken now — but the helper reads as a general client and shouldn't be a trap for the next parameter. Replaced with 3. groups[result["cause"] or "unclassified"].append(result)
...
attempts = max(result["attempts"] for result in group)
boards = [result["board"] for result in group]4. Not from the review, but adjacent and worth fixing in the same push: Re-verified: |
|
CI is fully green, and the new job ran for real rather than just being parsed.
From the 101 is the number to look at. It is exactly 2 job-list pages + 99 annotation fetches, and it matches the local dry run against a different run to the request — which is the point of putting the counter behind a lock. No traceback, no The one test-plan box still open is the post-merge one: read the first nightly's rendered summary and check the request count with the 14-run streak window attached (~130 rather than 101) against the budget |
1 participant
Closes #2271.
The question, and the answer
build.ymlbuilds ~99 matrix cells withfail-fast: false. One failing cell does not cancel theothers, but the run still reports
conclusion: failure. #2271 asked whether that is the signal wewant, and said explicitly that "current behaviour is correct" is a legitimate result — the point
was to make it a decision rather than an accident of the default.
The polarity does not change. The legibility does. The reasoning is now a comment in
ci-gaterather than an unstated default:
nightlyandlatestpointing at yesterday's image for it, so whoever flashes that board silently gets stalefirmware. Green would be a lie about what got published.
survived seven attempts across ~38 minutes of backoff (Nightly CI is flaky on master — package download 404s break ~30% of scheduled runs #2036); that is no longer a blip.
ci-gateserves pull requests too. Relaxing the matrix rule to quieten the nightly would relaxbranch protection with it.
Measured over the 30 scheduled runs from 2026-07-23 to 2026-08-21: 1 failure. Red is rare, so
the alert works. The problem was never the colour.
The actual problem
On 2026-08-12 (run 31649720825) a
majestic-webuifixup did a barermon two files upstream had deleted, and 94 of 99 boardsfailed in the same place. Establishing that took opening 94 job logs, because the run's entire
output was the word "failure". "The nightly is red" does not distinguish one broken board from
ninety-four.
What this does
1. Names the cause where the evidence is. Buildroot says which package and which phase in
exactly one line and nowhere else:
The retry loop already holds that log, so it now classifies out of it and folds the result into
the
::error::/::warning::it already emitted. The phase is the class:.stamp_downloadedis a mirror or a 404 (the #2036 family), anything after it is real code. No new steps, no new
artifacts.
2. A
Build summaryjob that aggregates it once, into the run summary:today, and the same cause exhausting its budget is what turns the nightly red. This is the
leading indicator, and nothing surfaced it.
gk7205v300_liteat 0KB free of 5120KB.hi3516ev300_neohas been red three nights running"is visible without digging through run history.
Nothing is stored: three read-only endpoints, ~130 requests, and the footer prints the count it
actually used (the token budget is 1000/hr and
publishalready spends ~400 on paced uploads).3. It cannot fail the run. That would be the exact mistake #2271 decided not to make, so the
job is deliberately not in
ci-gate'sneedsand its step swallows a non-zero exit into awarning. What keeps it honest instead is
build-summary.py --self-testinlint.yml, a mergegate that asserts the grammars still match what
build.ymlemits — a reworded::error::wouldotherwise reduce the summariser to reporting nothing, and reporting nothing looks exactly like a
quiet night.
Sample output
Dry-run against run 32534401852
(a green nightly), abridged:
Test plan
None of this is reachable from PR CI — the summary job only runs where there is a matrix, which is
the same gap that motivated
lint-workflow-shell.py— so it was proven against real data beforethis PR existed.
python3 .github/scripts/build-summary.py --self-test— 4 grammars, 7 annotations inbuild.yml, 99 fixture boards.build.yml:grammar 'exhausted' no longer matches build.yml. Added an unclassified one:build.yml's matrix job emits 8 annotations, 7 are classified. Both restored.classify_failureagainst the real 2026-08-12 log, with the function extracted from theworkflow YAML rather than retyped →
majestic-webui-dist/target_installed. Syntheticdownload failure →
opus-1.4/downloaded; repack failure →Makefile:210: repack; nothingrecognisable →
unclassified.build.ymlnow emits, parsesback to cause
majestic-webui-dist/target_installedandattempts: 7.92 of 96 boards failedin 8s / 97 requests(it predates the new annotations, so it correctly degrades to
step: Build firmwareratherthan crashing). 32534401852 →
All 99 boards built, headroom table above, 15s.python3 .github/scripts/ci-matrix.py --self-test— ok (99 boards, 132 packages, 52 cases).python3 .github/scripts/lint-workflow-shell.py [--self-test]— 48 run blocks parse clean.bash .github/scripts/test_load_hisilicon.sh,bash .github/scripts/test_sysupgrade.sh— pass.against the budget.
--no-flakesdrops the bulk of it if it ever crowdspublish.Notes for review
build-summary.pyis added toci-matrix.py'sNO_BUILD_SCRIPTS. It cannot change a byte of animage, and unclassified it would widen every PR touching it to the full 99-board matrix.
Touching
ci-matrix.pyto say so widens this PR to the full matrix —ci-matrix.pyisdeliberately unclassified, since a smoke set would build the boards that were already in
ALL_BOARDSand never a newly added one. That is one full matrix now to stop every future PRtouching the summariser from paying the same. The
build.ymledit on its own would have takenthe 13-board smoke set.
/check-runs/{job_id}/annotations, which relies on ajob id being usable as a check run id. That is true today and is verified against both runs
above, but it is not documented as a guarantee — a 404 there degrades one board to "no detail",
never to a crash, and the counts stay exact.
lint.ymlself-test is a separate job rather than a step inworkflow run blocks parse, sono existing check context is renamed and branch protection is untouched.