sandbox: retry deadsnakes apt install on transient PPA failures

[jwbron]

Summary

• The deadsnakes PPA occasionally returns HTTP 5xx during apt-get update, which silently leaves the deadsnakes package index unpopulated. The subsequent apt-get install python3.14-venv python3.14-dev then exits 100 with "Unable to locate package", killing make build in CI (see Fix #2685: skip PR-phase auto-PR in slice-DAG mode; include contract on context PR #2694 Integration Tests failure, comment #4435427457).
• Both repo-deps (Stage 1) and base (Stage 2) of sandbox/Dockerfile independently install python3.14 from deadsnakes, so both stages can fail this way.
• Wrap the post-PPA apt-get update + apt-get install in each stage with a 5-attempt retry loop using 15s/30s/45s/60s backoff, and pass -o Acquire::Retries=3 for in-call resilience to short blips. Outer loop survives multi-minute outages, inner option survives sub-second flakes.
• No other apt invocations are touched — the install of base Ubuntu packages doesn't go through deadsnakes and hasn't shown flakiness.

Test plan

• CI Integration Tests / Integration Tests passes on this branch — confirms the hardened build still produces a working sandbox image.
• Visually inspect that the retry loop appears in both Stage 1 and Stage 2 RUN blocks of sandbox/Dockerfile.
• Local smoke: make build on a clean machine succeeds (recovery path can't be exercised without a flake injector, but the always-fail path was verified offline to exit non-zero after 5 attempts).

[egg-reviewer]

Approve — targeted CI flake fix, logic is sound

The retry wrapper around the post-PPA apt-get update + install is the right shape for this failure mode: the deadsnakes mirror returns 5xx during update, apt-get update silently succeeds with an unpopulated package index, then install fails non-zero with "Unable to locate package". A retry that re-runs both update and install is exactly what catches that pattern.

I verified the shell semantics in dash (Ubuntu's /bin/sh, which is what Dockerfile RUN uses):

• set -e is disabled inside the if <cmd1> && <cmd2>; then …; fi condition, so a failed apt-get update && apt-get install does not terminate the RUN — it just makes the if false and the loop continues. ✓
• for i in 1 2 3 4 5; do … done and $((i*15)) arithmetic are POSIX-clean. ✓
• Backoff sequence is 15s/30s/45s/60s (four sleeps across five attempts), exiting 1 on the fifth failure before the unreached sleep 75. Total wall time before giving up: 150s + apt time. ✓
• On success, break exits the loop with status 0 so the chained && update-alternatives … proceeds. ✓
• Acquire::Retries=3 provides inner-loop resilience to single-fetch blips without the cost of a full apt re-run. ✓

Scope is appropriate: only sandbox/Dockerfile uses deadsnakes; gateway/Dockerfile and orchestrator/Dockerfile have no PPAs, so they don't need this treatment.

Non-blocking observations

1. 150s of total backoff may not cover longer PPA outages. 15+30+45+60 = 150s. The PR description calls this "multi-minute outages," which is technically true (2.5 minutes), but for a sustained 5–10 minute outage CI will still fail. Worth keeping in mind if this keeps re-occurring after the merge — at that point either bump the backoff (i*30 gives 30/60/90/120 = 300s) or add more attempts. Not worth changing pre-merge; the current shape is a reasonable default.

2. apt-get update can exit 0 even when a source fails. That's exactly the bug being worked around: the silent 5xx leaves the index unpopulated but update returns 0, and only install surfaces the problem. A future-proofing option is -o APT::Update::Error-Mode=any, which makes apt's index errors fatal — that would make the outer loop trigger on update failure directly instead of waiting for install to discover the gap. Optional defense-in-depth; the current install-failure-as-signal works.

3. The 17-line retry block is duplicated between Stage 1 and Stage 2. The two copies are byte-identical except for the surrounding update-alternatives lines. Not worth extracting to a script for two call sites, but if a third deadsnakes-using stage gets added later, factor it out.

4. Other PPA-style sources (gh, glow, charm.sh) are not wrapped. Acknowledged in the PR description ("hasn't shown flakiness"). Reasonable scoping decision — adding retry to non-flaky sources would just slow the failure mode without changing real-world success rates. Revisit if any of those start flaking.

Test plan

The PR's manual test plan correctly notes the recovery path can't be exercised without a flake injector. The unchanged success path is what CI will validate; the failure path is verified by inspection of the shell logic (which I've re-confirmed above).

[james-in-a-box]

Thanks for the careful read. Per-item disposition for the four non-blocking observations:

1. Bump backoff beyond 150s — disagree (current shape is the right default; reviewer also noted "Not worth changing pre-merge"). The PR targets the short-blip-to-multi-minute window deadsnakes has actually exhibited. Bumping pre-emptively trades CI wall-time on the always-fail path for coverage of an outage class we haven't observed. If a sustained 5–10 min outage actually shows up, that's the right moment to revisit, with real data on the duration.

2. -o APT::Update::Error-Mode=any as defense-in-depth — fixed-in-PR (commit c0e3442). Added to both retry loops. This directly improves the failure-detection mechanism the PR exists for: a deadsnakes 5xx now trips update non-zero immediately, so the outer retry triggers without waiting for the doomed install to discover the empty index. Saves the wasted install attempt and gives a cleaner signal in the failure log.

3. Extract the 17-line retry block to a shared script — disagree (reviewer's own assessment: "Not worth extracting to a script for two call sites"). Agreed — at two sites the duplication cost is lower than the indirection cost. If a third deadsnakes-using stage gets added, factor it out then.

4. Wrap other PPA sources (gh, glow, charm.sh) — disagree (intentional scope; none have shown flakiness). The PR is responding to an observed failure on deadsnakes specifically. Wrapping non-flaky sources adds CI failure latency without changing real-world success rates. Revisit if any of those start flaking.

— Authored by egg

[egg-reviewer]

Approve — feedback addressed cleanly

Re-reviewing the delta (e2cd2d4 → c0e3442): -o APT::Update::Error-Mode=any was added to both apt-get update calls inside the retry loops, with comment updates explaining the rationale. This directly addresses the non-blocking observation #2 from my prior review and tightens the failure-detection mechanism the PR exists for.

Verification

• Option is real and does what the commit message claims. APT::Update::Error-Mode accepts any | pointless | no; default is pointless, which is exactly the silent-success behavior the PR is working around (per-source failure leaves an empty index but apt-get update exits 0). any promotes any per-source error — 5xx from deadsnakes, GPG verification failure, malformed Release file — to a non-zero exit. That makes the outer retry trip on update directly instead of waiting for the doomed install to discover the gap. (Ansible PR #86061 discussion describes the same default behavior issue.)
• Applied symmetrically. Both Stage 1 (sandbox/Dockerfile:45) and Stage 2 (sandbox/Dockerfile:151) got the flag inside their retry loops. No other apt-get update calls in the file were touched — correctly leaving the pre-PPA base-Ubuntu update at lines 32 and 141 alone, since those are outside the retry scope and not the source of observed flakiness.
• Shell semantics still hold. set -e remains suspended inside the if cmd1 && cmd2; then …; fi condition, so a now-fatal apt-get update exit just makes the if false and the loop body continues to the sleep/exit-1 path. The behavior change is "trip retry sooner with a cleaner signal," not "alter the success/failure contract of the loop." ✓
• Comment updates accurately describe the new behavior. The two block comments at lines 41–43 and 147–149 explain the why, which is exactly what someone debugging a CI failure will need.

Side effect worth noting (non-blocking)

With Error-Mode=any, transient errors from the base-Ubuntu sources (not just deadsnakes) that occur inside the retry loop will now also trip the outer retry. That's a feature, not a regression — those sources hadn't shown flakiness, so the new behavior is mostly "free defense-in-depth." The pre-PPA apt-get update at lines 32 and 141 still has the old soft-fail behavior, but that's outside the PR's scope and pre-existing.

Other prior items

Items 1 (backoff bump), 3 (script extraction), and 4 (wrap other PPAs) were dispositioned as disagree with reasonable framing — agreed on all three. Two-site duplication is fine; pre-emptive scope expansion on non-flaky sources would just add CI failure latency without changing real-world success rates.

— Authored by egg

[james-in-a-box]

egg review completed. View run logs

3 previous review(s) hidden.