ci: bake custom ARC runner image and migrate CI off depot - #321
Conversation
|
Note Gemini is unable to generate a review for this pull request due to the file types involved not being currently supported. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7f0918920a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| commit-message: | ||
| name: Commit Message | ||
| runs-on: depot-ubuntu-24.04 | ||
| runs-on: [self-hosted] |
There was a problem hiding this comment.
Keep untrusted pull requests off self-hosted runners
This workflow still runs on pull_request (lines 7-9), and after this change every job executes on self-hosted; that means PR code from contributors can run directly on your own runner infrastructure instead of isolated hosted workers. In repositories where fork PRs are enabled, this is a high-risk execution path because attacker-controlled workflow code can probe the runner host and any credentials available to it, so self-hosted should be limited to trusted events or tightly restricted runner groups.
Useful? React with 👍 / 👎.
| unit-test: | ||
| name: Run unit tests | ||
| runs-on: depot-ubuntu-24.04-4 | ||
| runs-on: [self-hosted] |
There was a problem hiding this comment.
Pin runner OS/arch labels for Linux-only jobs
Switching to runs-on: [self-hosted] removes OS/arch constraints, but these jobs invoke Linux-specific tooling (bash, sudo, make, Docker paths) and were previously pinned to Ubuntu-specific runners. If any non-Linux self-hosted runner becomes eligible for this repo/group, scheduling can become nondeterministic and fail at runtime; add explicit labels (for example Linux/x64 or a dedicated ARC label) to preserve the prior routing guarantees.
Useful? React with 👍 / 👎.
3ef9875 to
8538ae2
Compare
Switch every job in the main CI workflow off of the depot-ubuntu-24.04 runner family and onto the org-wide ARC self-hosted pool registered against the lightninglabs org. All six jobs (commit-message, static-checks, lint, cross-compile, unit-test, systest) now use the [self-hosted] label, which is applied to every runner registered by our actions-runner-controller deployment in the staging cluster. Depot exposes sized runner classes (24.04, 24.04-4, 24.04-8) but the current ARC RunnerDeployment provisions a single uniform pool with no size dimension, so every job collapses onto the same [self-hosted] label. We are intentionally not introducing custom labels per job class yet; if a job turns out to need dedicated hardware we can add a label on the RunnerDeployment side and key that job to it, rather than carrying depot's sizing taxonomy forward. Existing depot CI runs on main and recent PRs serve as the baseline for a head-to-head comparison against the new pool, so no parallel benchmarking workflow is added in this commit.
8538ae2 to
8b4d05a
Compare
The systest target hard-codes -timeout 10m and the unit-test
targets inherit Go's default 10m per-package timeout. On Depot
this was generous: a recent main run completed systest postgres
in 334s and unit-race in 599s. On ARC the same workloads land at
13-16 minutes per package (matching the slower per-vCPU profile
of the c5.2xlarge spot pool that backs the RunnerDeployment),
which clips against the 10m budget and panics the go test
process before the suite finishes:
panic: test timed out after 10m0s
FAIL github.com/lightninglabs/darepo-client/systest 600.330s
Concretely the four failing tests in the run that surfaced this
each ran 587-589s and the global 10m timer fired before the
fourth one could complete.
Parameterize the systest timeout via a SYSTEST_TIMEOUT make
variable that defaults to 10m, then override it (and the unit
test timeout flag) to 30m in main.yml. The default keeps local
dev behavior unchanged so anyone running `make systest` from a
workstation still gets a fast-fail on a real hang; CI on the
slower ARC pool gets enough headroom to actually exercise the
suite. We can revisit downward later if/when the ARC pool is
sized closer to Depot's per-job throughput.
54e0a71 to
916987f
Compare
|
@Roasbeef, remember to re-request review from reviewers when ready |
In this PR, we move darepo CI off depot and onto the new ARC self-hosted
runner pool (runbook),
and bake a darepo-specific runner image so the migration actually works.
The first head-to-head run against the stock
summerwind/actions-runnerpool surfaced two blockers we have to fix at the image layer rather than
papering over inside every workflow job:
make: command not foundandEACCESon/opt/hostedtoolcachewhenactions/setup-gotries to dropthe Go toolchain. Patching those at the workflow level is the wrong
level to fix it.
The plan is to consolidate CI on infra we operate ourselves, and to
start pulling real numbers on how the new pool compares to depot. We're
keeping the comparison cheap: rather than standing up a parallel
benchmarking workflow that double-runs every PR, we'll use the existing
depot run history on
mainand recent PRs as the baseline, and the nextbatch of runs on
[self-hosted]as the head-to-head once the new imageis live.
What changed
.github/workflows/main.yml: every job now targets[self-hosted].depot-ubuntu-24.04[self-hosted]depot-ubuntu-24.04[self-hosted]depot-ubuntu-24.04-8[self-hosted]depot-ubuntu-24.04-4[self-hosted]depot-ubuntu-24.04-4[self-hosted]depot-ubuntu-24.04-8[self-hosted].github/runner-image/Dockerfile: a new darepo-specific runner imagebased on
summerwind/actions-runner-dind(so the runner pod ships witha working Docker daemon for the test harness) layered with
build-essential,make,postgresql-client,sqlite3, and apreinstalled Go pinned to the same
GO_VERSIONas the workflow. Go isdropped into
/opt/hostedtoolcache/go/<version>/<arch>with the.completemarker soactions/setup-gotreats it as a cache hit andskips the download step entirely.
.github/workflows/runner-image.yml: build+push pipeline for therunner image. Tags
ghcr.io/lightninglabs/darepo-runner:latestonmainplus a:sha-<short>tag every time. PRs build the image tovalidate the Dockerfile but do not push. The build itself runs on
ubuntu-latest(GH-hosted) because we can't use ARC to build the ARCimage while the cluster is still on the stock summerwind image.
Sizing
Depot exposed three sized variants (
24.04,24.04-4,24.04-8) andwe were using all three. The current ARC
RunnerDeploymentprovisionsa single uniform pool with no size dimension, so every job collapses
onto the same
[self-hosted]label. We're intentionally not portingdepot's sizing taxonomy over yet. If a job turns out to need bigger
hardware (lint and systest were the 8-core consumers) we can add a
custom label on the RunnerDeployment side and key just that job to it,
rather than locking the per-job class shape in before we have data.
Lightning-infra side
This PR is half of the change. The other half lives in
lightning-infraand needs to land before the migrated workflows can go green. Once
runner-image.ymlpublishes the first:latesttag, theRunnerDeploymentincharts/actions-runner-controller/runners-staging.yamlneeds to pointat the new image and enable dockerd inside the runner pod:
If the GHCR package is private, the namespace also needs an
imagePullSecretsentry. See.github/runner-image/README.mdforthe full wiring.
First failed run (for the record)
Run 25146214086
on the unmodified pool failed every job in 15-30s, exactly as the ARC
runbook caveats predicted. The fix shipped in this PR addresses both
failure modes at the image layer.
Test plan
runner-image.ymlbuilds cleanly on this PR (no push):latestand:sha-<short>show up onghcr.io/lightninglabs/darepo-runnerlightning-infraupdated to reference the new image withdockerdWithinRunnerContainer: true[self-hosted]pool and go greenmainand decide whether we need sized labels on ARC