fix(installer-tests): bound apt's own sockets so a stalled mirror fails in seconds, not 12 minutes - #1051
fix(installer-tests): bound apt's own sockets so a stalled mirror fails in seconds, not 12 minutes#1051LukasWodka wants to merge 3 commits into
Conversation
…ls in seconds, not 12 minutes
The `Prereqs` / `PATH persist` matrix jobs fail ~33 % of the time. The
error they print blames "distro-mirror connectivity ... Re-run this job",
and that advice does not work: two consecutive attempts on 2026-09-11
failed identically.
WHAT ACTUALLY HAPPENS. `_pm_run` bounds each package-manager attempt from
OUTSIDE with `timeout 60`. apt itself has no socket bound, so on a
blackholed route -- packets dropped rather than refused -- it waits,
emitting NOTHING: no `Err:`, no `W:`, no `E:`. All three attempts are
60 s of silence, apt learns nothing between them, and the arithmetic runs
the job out:
_bootstrap_host installs sudo and curl INDEPENDENTLY
-> 2 x _pm_run(apt-get update)
-> 2 x (3 attempts x 60 s + 15 s backoff) ~ 6.5 min
-> the remaining work then hits the job's 12m bound -> exit 137
A refused connection errors instantly; only a silent stall produces that
signature, and apt's default socket timeout outlasts the external kill
every time. So the bound was in the wrong place, not missing.
THE FIX. Give apt the bounds it can act on:
-o Acquire::http::Timeout=10 -o Acquire::https::Timeout=10
-o Acquire::Retries=3 -o Acquire::ForceIPv4=true
The first three are what tracebloc-engine's test workflow already applies
to its own `apt-get update`, which does not exhibit this failure. A stall
now costs 10 s and apt retries INSIDE one attempt instead of burning a
whole _pm_run cycle; worst case the bootstrap fails honestly in well under
a minute instead of consuming the job.
ForceIPv4 is the mitigation for the LIKELY cause, and is labelled as such
in the code: a container with no working IPv6 egress resolves an AAAA,
connects and waits -- the exact silent-hang signature. A stalled run emits
no apt output, so nothing in the logs proves it. It is here because it is
cheap and harmless on an IPv4-only path, not because it was measured.
apt-only, deliberately: dnf/yum/zypper/pacman take none of these flags and
would fail on an unknown option -- turning a mirror stall on one distro
into a hard argument error on four.
Also corrected the error message. "Re-run this job" was wrong twice over:
re-running inherits the same unbounded apt, and the wording framed our own
missing flags as somebody else's outage. It now says re-running often does
not help, and tells the reader that missing `Acquire::*` bounds on the
attempts above IS the bug.
Verified by running the real script in the real container the CI job uses:
`docker run --rm -v "$PWD:/src:ro" -w /src ubuntu:24.04 bash
scripts/tests/distro-prereqs.sh` -> exit 0, bootstrap completed, zero
"stalled" warnings. Options confirmed accepted by a real apt (exit 0, not
100). shellcheck -S warning -x clean; manifest unchanged (this file is not
on the bootstrap's fetch surface).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
bugbot run |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
…both harnesses (backend#2460 follow-up)
Bugbot, and it is the right call: the previous commit fixed ONE of two
copies.
`scripts/tests/path-persist.sh` carried its own `_pm_run` plus the same
bare `apt-get update -qq` and the same "Re-run this job" message. So
`Prereqs` was fixed and `PATH persist` — two of the three jobs that were
actually failing — was not. The two copies were identical in logic and had
already drifted in prose; this change proved they drift in behaviour too,
inside the very PR meant to stop the hang.
So the runner moves to `scripts/tests/_pm.sh` and both harnesses source
it. One copy cannot half-ship.
WHAT IS SHARED: `_pm_run`, its error message, and `_APT_BOUND` — the parts
that were byte-identical apart from a comment.
WHAT IS NOT: each harness's own `_pm_install_one`. Those genuinely differ
— path-persist supports apk, distro-prereqs does not — and folding them
together would invent a package-manager matrix neither one tests.
Sourcing is safe in this context and deliberately chosen over duplication:
both harnesses run with the repo mounted at /src and `-w /src`, so
`${BASH_SOURCE[0]%/*}/_pm.sh` is a local file read needing no network and
no tools beyond the shell already running. Verified in BOTH container
families the jobs use — ubuntu:24.04 and alpine:3.20 (path-persist's alpine
leg installs bash first, then execs it) — `_pm_run` defined and
`_APT_BOUND` set in each.
Verified end to end: `docker run --rm -v "$PWD:/src:ro" -w /src
ubuntu:24.04 bash scripts/tests/distro-prereqs.sh` -> exit 0, ZERO
"stalled or failed" warnings. shellcheck -S warning -x clean on all three
files; bash -n clean. Exactly one `_pm_run()` and one `_APT_BOUND=` in the
tree, both in _pm.sh. Manifest unchanged — none of these are on the
bootstrap's fetch surface.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
bugbot run |
…ps fix cannot be copied here
tracebloc-engine#1029 landed the same class of fix on the RUNNER and
measured the actual cause, which corrects two things here.
WHAT IT MEASURED. The mirrorlist lists the archive over http first and
https second; from 06:33Z on 2026-09-11 http stopped answering while https
answered at once, so apt walked 52 index URLs at ~23 s each before falling
back — 20 minutes per step. It fixed that by rewriting the mirrorlist to
https. It also measured that Timeout/Retries ALONE still left a 29-minute
stall, i.e. "bounded but not to anything useful".
1. ForceIPv4 is REMOVED. It was here on my hypothesis that a blackholed
AAAA caused the stall. The measured cause is the SCHEME, not the address
family. The flag was harmless, but its stated reason was wrong and a flag
shipped on a contradicted hypothesis gets copied forward as fact.
2. The https rewrite is NOT adopted, and the comment says why with the
measurement. These harnesses run inside a BARE container, and
ubuntu:24.04 ships no ca-certificates, so rewriting its sources to https
makes every index fetch fail verification:
W: Failed to fetch https://…/InRelease Certificate verification failed:
The certificate is NOT trusted. The certificate issuer is unknown.
apt-get update -> exit 0 (warnings only — "succeeds" fetching nothing)
apt-get install -> exit 100, package not installed
An update that exits 0 having fetched nothing is WORSE than the hang: the
failure moves to whatever needed the package. https needs ca-certificates,
installing which needs apt, which is the circle. Measured in the image the
job actually uses, not reasoned about.
The bounds stay, and are adequate at THIS scale in a way they were not on
the runner: #1029's 29 minutes was ~52 index URLs; a bare container lists
four suites, so a total stall costs minutes and ends in an honest error
rather than a 12-minute silent kill.
Also kept from #1029: `Acquire::https::Timeout` alongside the http one,
because `Acquire::http::Timeout` does not govern https connections.
A LIMIT OF MY VERIFICATION, said plainly: the container run I used to check
this (exit 0, zero stalls) was from a network where http://archive answers.
It cannot reproduce the CI failure, so it proves the change is not broken —
not that it cures the stall. The claim is bounded-and-honest failure, and
#1029's numbers are the evidence for the bound's size.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit c2e884d. Configure here.
Measurement: the bounds reached apt, and they did not help. Saying so plainly.This PR's body claims "the claim is bounded-and-honest failure, not a cure" and What the log actually showsThe flags reach apt — 8 verbatim occurrences: But the attempt boundaries give the game away:
60.004 s is Worse, the failure is non-fatal: the script continues, the whole 3-attempt cycle runs The control that identifies the causeSame run, same runner fleet, same minute, same
So it is not apt, not containers, and not this repo's egress — Debian's The mirror fix I was about to ship, and why I am not shipping itThe obvious next move is to rewrite Ubuntu container sources to Evidence already in hand refutes it. On the same day, tracebloc-engine#1022's What this actually isAn Ubuntu-archive-wide degradation on 2026-09-11, hitting both the generic Where that leaves this PRThe honest message is worth keeping and is already correct. The bound is not, and |
| # runner to be the SCHEME, not the address family. The flag was harmless but its | ||
| # stated reason was wrong, and a flag shipped on a contradicted hypothesis is | ||
| # the kind of thing that gets copied forward as fact. | ||
| _APT_BOUND='-o Acquire::http::Timeout=10 -o Acquire::https::Timeout=10 -o Acquire::Retries=3' |
There was a problem hiding this comment.
Acquire::Retries=3 works against this fix on the documented failure mode (a silent blackhole). _pm_run already bounds each attempt with an outer timeout 60 and retries the whole call 3×. With Retries=3 apt keeps re-trying every stalled index URL inside that 60 s window (≈ Timeout×(1+Retries)=40 s per URL), so on a bare image whose apt-get update fetches several suite indices serially, apt does not return before the external timeout fires — each of the 3 _pm_run attempts is killed at 60 s exactly as before the fix, and the worst-case total (≈3×60 s + backoff, ×2 packages in distro-prereqs) can still approach the 12-min job bound. The PR title says "fails in seconds" but this file’s own comment concedes "a total stall costs minutes." For the outer-timeout + fail-fast design you want apt to give up quickly — Acquire::Retries=0 (or 1) lets a stalled URL error out at ~10 s so _pm_run fails RED well under 60 s. Retries=3 is redundant with (and here counterproductive to) the outer retry loop.
| # The bounded package-manager runner and apt's socket bounds live in ONE place; | ||
| # both container harnesses source it. See scripts/tests/_pm.sh for why. | ||
| # shellcheck source=scripts/tests/_pm.sh | ||
| . "${BASH_SOURCE[0]%/*}/_pm.sh" |
There was a problem hiding this comment.
Source path is fragile — reuse the $HERE this file already computes. ${BASH_SOURCE[0]%/*} strips the last /segment, but when the script is invoked with no directory component (e.g. cd scripts/tests && bash path-persist.sh) there is no / to strip, so %/* returns path-persist.sh unchanged and this sources path-persist.sh/_pm.sh → "No such file or directory". There is no set -e, so execution continues, _pm_run/_APT_BOUND are never defined, and under set -uo pipefail the first $_APT_BOUND reference aborts with "unbound variable". Line 62 already computes HERE="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)", which handles the no-slash case via dirname. Use . "$HERE/_pm.sh" instead. (Same issue in distro-prereqs.sh:35.)
| # The bounded package-manager runner and apt's socket bounds live in ONE place; | ||
| # both container harnesses source it. See scripts/tests/_pm.sh for why. | ||
| # shellcheck source=scripts/tests/_pm.sh | ||
| . "${BASH_SOURCE[0]%/*}/_pm.sh" |
There was a problem hiding this comment.
Same fragile source as path-persist.sh:84 — ${BASH_SOURCE[0]%/*} returns the filename unchanged when invoked with no directory component (bash distro-prereqs.sh from inside scripts/tests), sourcing distro-prereqs.sh/_pm.sh which fails; _pm_run/_APT_BOUND then stay undefined and the first $_APT_BOUND use aborts under set -u. HERE is computed on line 30 via dirname and handles this correctly — use . "$HERE/_pm.sh".
|
|
||
| # Bounded + retried package-manager invocation; "$@" = the PM argv. | ||
| # | ||
| # `command -v` (not has()) because this runs BEFORE common.sh is sourced; |
There was a problem hiding this comment.
Stale justification for one of the two callers. This says command -v is used "because this runs BEFORE common.sh is sourced" — true for distro-prereqs.sh (sources _pm.sh first), but false for path-persist.sh, which sources lib/common.sh at line ~66 and only sources this file at line 84. In that harness has() is already defined when _pm_run runs, so the stated reason does not hold. Using command -v unconditionally is still correct; just reword so the comment is accurate for both callers (or note the ordering differs).
|
|
||
| _pm_install() { # install one or more packages with whatever PM exists; best-effort | ||
| if command -v apt-get >/dev/null 2>&1; then _pm_run apt-get update -qq && _pm_run apt-get install -y -qq "$@" | ||
| # shellcheck disable=SC2086 # _APT_BOUND is a deliberate word-split argv |
There was a problem hiding this comment.
The # shellcheck disable=SC2086 sits above the whole if/elif chain, so it suppresses SC2086 for every package-manager branch (dnf/yum/zypper/apk/pacman), not just the apt line that needs the deliberate $_APT_BOUND word-split. That would mask a genuine future unquoted-variable bug introduced in any of those branches. distro-prereqs.sh:39 scopes the same directive to the apt line only — match that placement here (move the disable comment to the apt branch).
saqlainsyed007
left a comment
saqlainsyed007
left a comment
There was a problem hiding this comment.
Approving. Solid, well-evidenced remediation of the Prereqs / PATH-persist apt-stall flake class (#986). De-duplicating _pm_run into a single sourced _pm.sh is the right call — it's exactly the "half-shipped fix" (bounds on one copy only) that this consolidation prevents.
Mechanism checks out: bounding apt's own socket (Acquire::http/https::Timeout=10, Retries=3) makes a stalled mirror fail RED at bounded time instead of the old silent 12-min exit-137, and the 10s socket-inactivity bound won't red a healthy-but-slow mirror that's still making progress. apt-only scoping is correct (other PMs would error on the flags). CI is fully green, including every ubuntu:22.04/24.04 leg — the exact jobs this targets — and it's test-harness-only, no runtime paths touched.
Optional follow-ups (posted inline, none blocking — take them or leave them):
_APT_BOUNDRetries=3vs the "fails in seconds" framing: worst-case blackhole is bounded to minutes, not seconds; you note this in the description, so purely a wording point.Retries=0/1would fail faster at the cost of transient-stall recovery — your call, and matching tracebloc-engine is a reasonable tiebreaker.- Source
_pm.shvia the already-computed$HERErather than${BASH_SOURCE[0]%/*}(both harnesses) —%/*breaks on a no-slash invocation;$HEREis slash-safe. CI's invocation path works today, so low-priority. - Scope the
# shellcheck disable=SC2086to the apt line inpath-persist.shas you already do indistro-prereqs.sh, so a future unquoted var in the dnf/yum branches stays linted. - The "runs BEFORE common.sh is sourced" comment in the shared file isn't true for path-persist (it sources common.sh first) — worth a word tweak now the file is shared.
None of these block; nice fix.
2 participants
Closes #986
Why these jobs are red — it isn't the mirror, it's where the bound is
Prereqs/PATH persistfail ~33 % of the time (10 failures / 20 successes over 39 decided runs, 2026-09-10 → 09-11, across seven branches includingdevelopandmain). The error tells you to re-run. Re-running does not work — two consecutive attempts on#1043failed identically._pm_runbounds each package-manager attempt from outside withtimeout 60. apt itself has no socket bound, so on a blackholed route — packets dropped rather than refused — it waits and emits nothing: noErr:, noW:, noE:. Three attempts of pure silence, apt learning nothing between them:The arithmetic runs the job out on its own:
A refused connection errors instantly; only a silent stall produces that signature. The bound wasn't missing — it was in the wrong place.
The fix
Give apt the bounds it can act on:
The first three are what
tracebloc-engine's test workflow already applies to its ownapt-get update— which does not exhibit this failure. A stall now costs 10 s, and apt retries inside one attempt instead of burning a whole_pm_runcycle. Worst case the bootstrap fails honestly in under a minute rather than consuming the job.apt-only, deliberately.
dnf/yum/zypper/pacmantake none of these flags and would fail on an unknown option — turning a mirror stall on one distro into a hard argument error on four.ForceIPv4is labelled in the code as a mitigation for the likely cause, not a measured one. A container with no working IPv6 egress resolves an AAAA, connects, and waits — the exact silent-hang signature. But a stalled run emits no apt output, so nothing in the logs proves it. It's here because it's cheap and harmless on an IPv4-only path.The error message was wrong twice over
Re-running inherits the same unbounded apt, and the wording framed our own missing flags as somebody else's outage. It now says re-running often doesn't help, and tells the reader that missing
Acquire::*bounds on the attempts above is the bug.Verified
Ran the real script in the real container the job uses:
shellcheck -S warning -xclean,bash -ncleanscripts/gen-manifest.sh --check→ up to date, unchanged (this file is not on the bootstrap's fetch surface)What this does not claim
It does not prove the mirrors were fine — it proves that when they stall, we now find out in seconds instead of losing the job. If the 33 % persists with
Acquire::*bounds present in the log, the cause is elsewhere and the new message says exactly that.Note
Low Risk
Changes only CI test bootstrap scripts for distro containers; no production installer or runtime paths.
Overview
Fixes flaky CI container harnesses where
apt-getcould stall silently under an outertimeout 60, burning retries until jobs hit ~12 minutes and exit 137 instead of failing fast.Adds shared
scripts/tests/_pm.shwith_pm_run(deduplicated fromdistro-prereqs.shandpath-persist.sh) and_APT_BOUND(Acquire::http/https::Timeout=10,Acquire::Retries=3). Both harnesses now source that file and pass_APT_BOUNDonly onapt-get update/installso apt enforces socket limits internally; other package managers are unchanged.The shared
_pm_runfailure message is updated to stop implying “just re-run,” and to call out missingAcquire::*on apt attempts as the likely bug.Reviewed by Cursor Bugbot for commit c2e884d. Bugbot is set up for automated code reviews on this repo. Configure here.