mirror of
https://github.com/coder/coder.git
synced 2026-09-22 13:10:21 +08:00
Bobby Ho
d210b311dc
## Problem The Windows code-signing path downloads two build tools with bare `wget` and no retry, in both `ci.yaml` (`build` job) and `release.yaml` (`release` job): - `rcodesign` from GitHub releases - `jsign-6.0.jar` from GitHub releases A single transient network failure on either fetch fails the whole job. In `ci.yaml` that turns `main` red via the `required` aggregator; in `release.yaml` it fails a release. This has happened. `Install rcodesign` failed on **2026-02-25** (in the since-deleted `build-dylib` job), **2026-03-04**, and **2026-04-30**. ## Root cause Two parts, one structural and one local. **Structural:** GitHub Actions has no per-step retry. This repo already knows toolchain provisioning is network-flaky and has `.github/scripts/retry.sh` (3 attempts, 2s/4s/8s backoff), applied in roughly 20 places. But `retry.sh` is a shell wrapper, so it can only wrap `run:` steps. These four downloads are `run:` steps that were simply never wrapped. **Local:** the failing step's body, under `set -euo pipefail`, is exactly three commands: ```sh wget -O /tmp/rcodesign.tar.gz https://github.com/indygreg/apple-platform-rs/releases/download/apple-codesign%2F0.22.0/... sudo tar -xzf /tmp/rcodesign.tar.gz -C /usr/bin --strip-components=1 ... rm /tmp/rcodesign.tar.gz ``` `tar` and `rm` operate on a file that was just written, so they are deterministic. The only nondeterministic command in the step is the network fetch, and a truncated download surfaces as a `tar` failure whose cause is still the network. ### How we know Enumerated failed runs through the GitHub Actions API and extracted, per run, every failed job together with the names of its failed steps. | Scan | Scope | Runs | |---|---|---| | `ci.yaml`, `main` | 2025-08-01 to 2026-07-29 | 934 | | `ci.yaml`, all branches | most recent failures | 150 | | `release.yaml` | all recorded failures | 22 | The 934 is effectively the complete set; the API reports 923 failed `main` runs over that period and the scans overlap slightly. `Install rcodesign` appears **3 times on 3 separate dates**. Being spread across dates rather than clustered, these behave as **independent** events. That distinction is what selects the remedy, and it is why this change is retry rather than removal. For contrast, the `Setup Java` failures in the same jobs are **4 failures inside a single 90-minute window** on 2026-05-28, all from an `api.azul.com` edge failure. That is a correlated outage, where every attempt shares the same degraded dependency and retry provably cannot help. **That defect is not addressed here** and needs a different fix; see "Not addressed" below. ### Limits of the evidence Stating these plainly so a reviewer can weigh them: - **Cause is not directly confirmed.** Logs for all three `rcodesign` failures are past GitHub's 90-day retention. The inference from the step body above is strong but circumstantial. - **Step-level attribution only reaches back about five months.** GitHub prunes per-step detail from the jobs API while keeping job-level conclusions. Probed directly: runs from 2026-03-01 onward return populated `steps` arrays; runs from 2026-02-05 and earlier return empty ones. So the true count over the full period could be higher; it cannot be lower. - **Impact is small.** This whole class of failure is 10 of 800 attributed non-`required` job failures, about **1.25%** of measured `main` CI failure volume. This is not a significant reliability improvement and should not be reviewed as one. The Postgres-backed Go tests alone are over 40%. ## Solution Wrap all four downloads in the existing retry helper: ```yaml - ./.github/scripts/retry.sh -- wget -O /tmp/rcodesign.tar.gz https://... ``` Four lines changed, one per site: `ci.yaml:1287`, `ci.yaml:1321`, `release.yaml:199`, `release.yaml:225`. **How it works.** `retry.sh` runs the command, and on non-zero exit sleeps 2s, 4s, then 8s before re-attempting, up to 3 attempts, then fails with the original command in the error message. On success the first time, behavior is unchanged. **Why it works for these failures.** They are independent events, so each attempt is a fresh trial with an independent chance of success. A GitHub releases CDN blip on one run says nothing about the next 2 seconds. This is exactly the regime retry is for. **Why retry rather than deletion.** These artifacts genuinely are not present on the runner, so the network call is unavoidable. It can only be made survivable. (Where a dependency *is* avoidable, deletion is the better answer, which is the shape the `setup-java` fix will take.) **Why `wget -O` is safe to retry.** `-O` truncates its output file on each attempt, so a partial download from a failed attempt is overwritten rather than appended to. No corruption path. ## Risks Low, and worth naming precisely. | Risk | Assessment | |---|---| | Behavior change on the success path | None. `retry.sh` execs the command directly; a first-attempt success is identical to today. | | A persistently broken URL now takes longer to fail | Yes, by up to 14s of backoff, then it fails exactly as it does today. Negligible against a job that takes tens of minutes. | | `retry.sh` mangling `wget`'s own flags | `retry.sh` parses its own options with `getopt`, so this was the main correctness concern. Verified explicitly, both argument orders used in these workflows. See Verification. | | Relative path `./.github/scripts/retry.sh` resolving wrongly | These steps set no `working-directory`, so cwd is the repo root. Deliberately **excluded** the third `wget` at `release.yaml:713` (`publish-homebrew`), which runs after `cd "$temp_dir"` where a repo-relative path would break. | | Retry masking a real regression | Bounded to 3 attempts over 14s. This is not job-level auto-retry, which would hide regressions and is explicitly not proposed. | ### Verification gap a reviewer should know about **The changed steps do not run on PR CI.** `ci.yaml`'s `build` job is gated on `github.ref == 'refs/heads/main' || startsWith(github.ref, 'refs/heads/release/')`, and `release.yaml` runs only on release. So these four steps will execute for the first time on merge to `main`. Verification below is therefore local plus static analysis, not a live run of the modified steps. ## Verification `retry.sh` argument passing, using a stub that prints what it received, for both argument orders present in these workflows: ``` --- form A: -O before URL (rcodesign style) --- argc=3 arg1=[-O] arg2=[/tmp/rcodesign.tar.gz] arg3=[https://github.com/indygreg/apple-platform-rs/releases/download/apple-codesign%2F0.22.0/apple-codesign-0.22.0-x86_64-unknown-linux-musl.tar.gz] --- form B: URL before -O (jsign style) --- argc=3 arg1=[https://github.com/ebourg/jsign/releases/download/6.0/jsign-6.0.jar] arg2=[-O] arg3=[/tmp/jsign-6.0.jar] ``` Order preserved and the `%2F` encoding in the rcodesign URL intact, which was the specific failure mode to rule out. `make lint/actions` (actionlint plus zizmor security audit): ``` ✓ lint/actions/actionlint No findings to report. Good job! (29 ignored, 102 suppressed) ``` `make pre-commit-light`: ``` ✓ fmt/shfmt ✓ lint/markdown ✓ lint/actions/actionlint ✓ fmt/terraform ✓ lint/shellcheck ✓ lint/helm ✓ fmt/markdown ✓ lint/bootstrap ✓ lint/emdash ✓ lint/migrations ✓ lint/typos ✓ lint/mise-versions ✓ pre-commit-light passed (14s) ``` ## Not addressed Deliberately out of scope, listed so the remaining exposure is visible: - **`actions/setup-java` with `distribution: "zulu"`** in both files. This resolves a JDK from `api.azul.com` and downloads it from `cdn.azul.com` on **every** run, confirmed from a successful `main` build's log, because a `Java_Zulu_jdk` tool-cache lookup can never hit the runner's cache. This is the correlated-outage defect from 2026-05-28 and retry cannot fix it. The probe on this branch ([run 30492093096](https://github.com/coder/coder/actions/runs/30492093096)) has now answered what the fix should be. `depot-ubuntu-22.04-8` ships: ``` RESULT: java found at /usr/bin/java OpenJDK Runtime Environment Temurin-11.0.31+11 (build 11.0.31+11) JAVA_HOME=/usr/lib/jvm/temurin-11-jdk-amd64 JAVA_HOME_8_X64 / _11_X64 / _17_X64 / _21_X64 / _25_X64 (all present) tool cache: Java_Temurin-Hotspot_jdk ``` So the job downloads **Zulu 11.0.32+9 over two third-party hosts while Temurin 11.0.31+11 is already on the runner's `PATH`**. The follow-up PR will point `JAVA_HOME` at `$JAVA_HOME_11_X64` and drop the action, which removes both Azul hosts while keeping the Java 11 pin rather than inheriting whatever the image default becomes. - **`storybook`'s `pnpm/action-setup`**, the last direct use in the repo and the same unguarded `registry.npmjs.org` dependency originally reported on the issue. Note `cache: true` does **not** mitigate it: per the action's own `action.yml`, `cache` caches "the pnpm store directory", not the pnpm binary. ## Scope This PR is now a **single commit** (`673162b84e`) containing only the four-line retry change. A throwaway probe workflow briefly lived on this branch to answer the JDK question above. It has served its purpose and the commit was dropped, so nothing diagnostic remains here to review. Its result is quoted in "Not addressed" and will be carried into the follow-up PR. Refs coder/internal#929