Ci fleet hardening v2 - #1682
Closed
DawidWesierski4 wants to merge 12 commits into
Closed
DawidWesierski4 wants to merge 12 commits into
DawidWesierski4 wants to merge 12 commits into
Conversation
DawidWesierski4
requested review from
Sakoram,
awilczyns,
moleksy and
soopel
as code owners
August 20, 2026 14:44
DawidWesierski4
force-pushed
the
ci-fleet-hardening-v2
branch
2 times, most recently
from
August 20, 2026 17:58
0c0e014 to
a6a354b
Compare
Five failures of the same kind: the build asserting something about its
environment instead of asking it.
- The rocky9 image reported "libxdp is absent after install" from a successful
install. xdp-tools and libbpf default LIBDIR to ${PREFIX}/lib64, which Debian's
pkg-config does not search and which does not exist on RHEL, so neither default
is portable and the multiarch directory libbpf was pinned to by hand only moved
the problem. Asking `pkg-config --variable pc_path` does not answer it either:
on Rocky 9 that is a string from a .pc file the distribution ships, and it omits
the compiled-in /usr/local/lib64/pkgconfig. So plant a probe .pc in each
candidate under the prefix and install into one pkg-config actually reads, and
register it with the dynamic linker, whose default path is just as
distribution-specific and which fails at load time rather than at link time.
- With libxdp visible, meson enables the manager's XDP target and clang stops on
gnu/stubs-32.h, which no package in the rocky9 image provides. The Ubuntu images
install gcc-multilib for this; glibc-devel.i686 is the RHEL counterpart.
- -flarge-source-files is gcc's, and clang rejects unknown -f arguments outright,
so every RxTxApp translation unit failed once the fuzz leg built with clang. It
is now offered to the compiler and used only if taken, like -msse4.2 above it.
- The fuzz targets link against GPU direct, which their sources call: the wrappers
#include the library sources they exercise, so those callees have to be on the
fuzz link line and not only on libmtl's. tests/unit carries the same conditional.
The assertion that started this was worth keeping: without it the image built
green with pkg-config reporting libxdp absent, which does not fail a build -- it
configures MTL without AF_XDP.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
gcc builds these; clang's -Werror does not, so the fuzz leg -- which has to use clang for libFuzzer -- stopped in the test build: - st40p_handler.cpp carried a constant nothing reads; - st40i_tests.cpp captured a compile-time constant into a lambda that does not need it captured; - St30pRedundantLatency stored a latency and a starting time it never read. The constructor body looked like it used one of them, but it assigned to its own parameter, not to the inherited startingTime -- and initializeTiming(), which every caller runs straight after construction, sets that member anyway. So nothing observable changes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Every access to these counters already goes through C11 atomics -- the tasklet adds to them, st*p_*_get_session_stats() reads them from another thread -- but the fields themselves were plain uint64_t. gcc's atomic builtins accept that; clang does not, so the library did not compile with clang at all, which is how it stayed unnoticed. Same codegen under gcc, because gcc was already emitting atomic operations. The USDT probes are the other half: their argument macros do arithmetic on what they are handed, and clang refuses that on an _Atomic operand, so the three sites that passed framebuff->stat now read it out first. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Auto pacing selection picks rl on any driver whose rl_type is TM, and then treats the two ways that choice can fail differently: a queue whose rate limit will not set falls back to tsc with a warning, while a tm hierarchy that will not build fails mtl_init outright. Both mean the same thing -- the driver in front of us has no rate limiter to offer -- and in auto mode neither is the user's choice to defend, so both should degrade the same way. Found while looking at an E830 whose PF grants its VF no QoS capability. That host crashes inside the iavf PMD before returning, so this does not rescue it; what it fixes is the case where a driver reports the missing capability properly, which today is a refusal to start rather than a session paced by tsc. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
DawidWesierski4
force-pushed
the
ci-fleet-hardening-v2
branch
8 times, most recently
from
August 21, 2026 14:58
dd168df to
e21cfcd
Compare
The workflows implemented themselves. Host setup, cache decisions, driver builds, test invocation and report collection were multi-line `run:` programs inside YAML, which meant no operation could be run, read or tested anywhere except on a runner, by pushing. Debugging a bare-metal job took a push and a forty-minute wait for a forty-megabyte log. Every operation is now a focused script under `.github/scripts/ci/`, verb-style and runnable by hand, with `Taskfile.yml` as the single entry point both the workflows and a developer call. The YAML orchestrates; it does not compute. `check-yaml-policy.sh` keeps it that way, and also rejects a third-party action that is not pinned to an immutable SHA. Three contracts the scripts encode, each one a failure this fleet actually had: **Jobs verify host state; they never install it.** apt packages, kernel modules, DMA bindings, the media share, the analyser -- a job that repairs what it finds hides drift in the host image and races every other job on the machine. So each check fails in seconds with the one command that fixes it, on the host that needs it. `configure-host.sh`, `media-assets.sh`, `ebu-list.sh` and `ice-required.sh` are that contract; a missing analyser is degraded mode rather than failure, because absence is not misconfiguration, and `MTL_CI_REQUIRE_COMPLIANCE=1` makes it fatal again on a host that has one. Lab facts come from `/etc/mtl-ci/runner.env` on the host that owns the hardware, not from GitHub secrets, which are a second copy of lab configuration kept in sync by hand. **A cache hit has to be usable, not merely present.** `actions/cache` saves in a post step that runs whether the job passed or not, so a run that died half-way through installing MTL stores the half-written tree under an unchanged key, and every later run restores it, skips the build and fails in the first consumer that resolves `mtl.pc`. The keys are content-addressed with an explicit schema (`cache-keys.sh`, `cache-schema.env`) so a fixed layout bug can be rotated past, entries are immutable, and `validate-cache.sh`, `validate-ice.sh`, `validate-jpegxs.sh` and `validate-dependencies.sh` reject a hit that cannot be used -- including an ICE module whose vermagic, kernel ABI fingerprint, compiler identity or Kahawai QoS capability does not match the host it is about to load on. **The acceptance virtualenv is a cache, not host state.** It is built from `requirements.txt` in the checkout, lives in the runner user's cache outside anything `git clean` touches, and is the same for every job on the host. So it is created once, rebuilt when the requirements change or when the host's python moves under it, built with `python3 -m venv`, `virtualenv` or `uv` -- whichever the host has, since none of them installs anything -- and names `python3-venv` when the host has none. `watch-run.sh` is the developer's side of the same layer: it resolves a commit from `--run/--pr/--sha/--branch`, defaulting to the pushed tip so it cannot report on a commit that only exists locally, distinguishes "queued, no runner yet" from a failure, and ends with the job, the failed step and the error lines. Twenty lines instead of the log. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The YAML now only orchestrates: each step names a `task ci:*` entry point, so the same command runs in a job and on a developer's machine. What is left in the workflows is the part that is genuinely about GitHub -- what triggers, what runs where, what may run at once, and what a queue is allowed to cost. - **A shared fleet is the scheduling problem.** Every NIC label is served by one host, so a second job at a label is a queue and not throughput. Superseded Build, smoke, base and docker runs are cancelled before they queue, and `pr-gate.yml` states its wait as a queue budget rather than a flat twenty minutes -- a build that has not started yet is not a build that is failing. When the budget does expire, the gate says it gave up because no runner came, which is a different thing from a failed build and used to look identical. - **A bare-metal job has to be bounded at every level.** Job timeout, run timeout and per-suite caps, so a hung test releases the host instead of holding a card for hours; `gtest-bare-metal.yml` carries the bounds the local harness asserts. - **The smoke matrix says what each leg is for.** The `i225` leg runs the low-bandwidth subset with no capture device and its own timeouts, because that card has no SR-IOV and two PFs rather than VFs; it is `optional` while the label is new. A leg with `no_capture` skips the analyser check it cannot use. - **`provision-runner.yml` is dispatch-only**, the single deliberate exception to "jobs install nothing": a human asks for a host to be prepared, by name. - Privileged bare-metal steps no longer open with a trace-fd error, the acceptance report the non-smoke suites write in place is left alone, and a host running an ICE driver that is not the one the suite needs is told so before it tests. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A change to a bare-metal job could only be tested by pushing it and waiting for a runner that might be busy for hours. `run-job.sh` runs the same job here -- `--runner docker` for anything that does not need a card, `--runner host` on a machine that has one -- through the same Taskfile entry points the workflow calls, with a local cache store that reproduces `actions/cache` semantics including its immutability. That is what proved the `i225` leg before CI ever dispatched it: two runs on a host with an I225-LM, `11 passed, 3 skipped` in about twenty-two minutes, while the leg itself was still queued behind a label nothing advertised. `tests/` holds the assertions about all of this that do not need hardware: cache schema rotation and poisoning, the gtest bounds, the wait-for-workflow script, the YAML migration, the virtualenv builders, and that a missing compliance analyser is not a gate. `task ci:test-dependencies` runs them, and so does the build workflow. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The loop an agent runs on CI is: push, find the run, find the failing job, find the failing step, find the error in a forty-megabyte log. The first four steps are mechanical and the last is a needle in ANSI-coded output, so all five became tools: `ci_pr_checks` and `ci_pr_failures` for what a pull request's checks say, `ci_last_log` for the interesting lines of one, and `ci_watch_run(pr=…)`, which blocks until the runs of a commit finish and then names the job, the step and the error line. The other half drives the local harness rather than reading GitHub -- `ci_list_jobs`, `ci_run_job`, `ci_test_pr`, `ci_list_tasks`, `ci_run_task`, `ci_cache_status`, `ci_check_ebpf`, `ci_diagnostics` -- so the same agent can reproduce a leg here instead of queueing behind the fleet. Failure counts and captured stderr are bounded, so a broken job cannot flood a context window; the repository argument is validated before anything is spawned; and the watcher's subprocess is given two minutes beyond its own poll deadline, so it reports the timeout itself instead of being killed while writing it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The four agents each own a phase and nothing owned the sequence: which of them takes the next task, and which tasks may run at the same time. That decision was made ad hoc, once per session, and it is where this repository's own facts matter more than general judgement. So the orchestrator is the only agent permitted to invoke the others, and its parallel-safety test is five mutexes that are real here: a file, the build tree and acceptance virtualenv, a physical host with its hugepages and VF layout, a NIC label -- one host per label, so a second job at one is a queue and not throughput -- and the gate chain of a single change. It dispatches, records evidence and never implements: its edit tool is for the task board and its shell is for observation. Evidence is named per dispatch, including a non-empty `runner_name` for a CI job, which is the distinction the i225 analysis turned on. The `mtl-cicd` skill is the CI half of the routing matrix, which had no agent: the design contract that workflows orchestrate and scripts implement, and the inventory to read before editing either. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Resolve the datapath from the ports under test instead of hardcoding it, register the JPEG XS plugin tree, capture ST 2110 pcaps under sudo so compliance checks get complete captures, make room before recording raw video and delete the RX recording once it has been checked. Generated configs and the low-bandwidth cases are adjusted for the i225 smoke leg.
… leg Three things a person needs and could previously only get by reading the workflows: what the pipeline is (`cicd_setup_proposition.md`, with the prebuilt dependency problem it solves in `.github/github_actions_issue.md`), what a runner has to have before it can serve a label (`ci_runner_setup.md` -- the packages the jobs check but cannot fix, the media share, the EBU LIST analyser and when its absence is degraded mode rather than failure, and which host carries which card), and why the `i225` leg was red for two days without ever running (`i225_leg_analysis.md`). That last one is here because the evidence is not visible in the Actions UI. A job queued because the fleet is busy and a job queued because no host advertises its label are the same grey dot; they differ only in `runner_name`, and every i225 job record had an empty one until mtl-runner-12 came online. The one real failure after that took twelve seconds and named its own fix. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ners The local harness, its cache store and the reports the jobs collect all land in the checkout, and `git status` has to stay readable for the suites that assert on it. CODEOWNERS gains the CI directories, which had no owner. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Collaborator
Author
|
Superseded by #1683, which now contains the fleet-hardening stack and the unified Checkpath linter work. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.