ci: add new GKE self-hosted runners (ARC) shadow CI - #66520
Conversation
2557a76 to
c518777
Compare
e7ed41f to
9a6b74a
Compare
b307ae9 to
6e0cf5e
Compare
cc930ae to
a0b4a10
Compare
e044ea6 to
edde4db
Compare
82009b5 to
f6bda97
Compare
kshitijk4poor
left a comment
There was a problem hiding this comment.
Solid migration — the docker-cache locality work and the parity fixes are the right calls, and the read-only-cache-for-PRs design is exactly how this should be done. A few remaining optimization opportunities surfaced while reviewing; most belong in hermes-agent-ci-infra or a follow-up rather than this PR, noting them here so they don't get lost:
1. Per-slice setup tax is still paid 8× per run (biggest remaining win)
Each of the 8 test slices independently does checkout → ripgrep download from GitHub releases → setup-uv → uv python install 3.11 → uv sync with ~8 extras — roughly the same 1–2 min of setup, × 8 pods, × every push. Now that the runner image is owned in hermes-agent-ci-infra, ripgrep, uv, Python 3.11, and a pre-warmed uv wheel cache (or a full .venv keyed to uv.lock) could be baked into the pod image. Setup collapses to checkout + cache-hit uv sync. Same insight this PR already applied to docker builds, just not yet to the Python slices.
2. actions/cache still round-trips to GitHub's cache CDN from GKE
The uv cache, npm caches, duration cache, and Playwright baselines all go through actions/cache — from a GKE pod that's a cross-network transfer to GitHub's backend. The same locality argument that motivated moving buildx cache to same-region Artifact Registry applies: pointing these at same-region GCS (or a cluster-local cache server) should shrink the cache restore/save steps materially.
3. The generate job is a full pod for seconds of work
It computes LPT slices and exits, but pays full pod scheduling + checkout + setup latency and serially gates all 8 slices. Could run on a minimal image or be folded into the first phase of the slices.
4. dind cold-start per docker job
A node-level DaemonSet pre-pulling the base image (or a pull-through registry mirror on the cluster) would shave the remaining docker-job boot time. Infra-repo territory.
5. Warm runners
If minRunners isn't set on the runner sets, cold pod scheduling adds ~30–60s to every job start. One helm value.
None of these should block this PR — #1 is the one I'd file as the first follow-up after this lands (difference between ~3–4 min and ~1–2 min wall per push).
|
You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool. What Enabling Code Scanning Means:
For more information about GitHub Code Scanning, check out the documentation. |
1ce83d0 to
eabf47d
Compare
Run a duplicate of CI on the new ARC (Actions Runner Controller) runners in GKE, beside the existing CI. The duplicate does not change production CI. Every workflow in .github/workflows/ that does not start with newci- is byte-identical to main. Watch the shadow runs for a few days, then migrate. The shadow set is 16 files: newci-ci.yml plus the 15 reusable workflows that ci.yml calls on a pull request. Only pull-request workflows are copied. js-autofix, deploy-site, and skills-index run on push or on a schedule. A copy of those would push branches and deploy the site a second time. Safety properties of the shadow: - Concurrency groups are newci-prefixed. This is the important one. The production groups use cancel-in-progress, so a shared group would let a shadow run cancel the production run. - Cache keys are newci-prefixed. The shadow cannot poison or evict a production cache entry. - Reusable-workflow calls point only at other newci-* files. No shadow job calls a production workflow. - The PR review comment runs with --dry-run. It prints the comment body to the job log. Two pollers cannot fight over the hermes-ci-review-bot comment. - The gate job is renamed to "[newci] All checks pass (informational)". The production check "All required checks pass" stays the only merge gate. - The shadow runs on pull_request only. The push trigger is removed. - docker publish and merge jobs are unreachable. Their conditions require a push to main or a release. Runner infrastructure, in the shadow copies only: - Jobs go to three scale sets: arc-runner-small for short gate jobs, arc-runner-set for general work, arc-runner-docker and arc-runner-arm64 for image builds. dind is only on the docker sets, so the other jobs stop paying for a privileged sidecar. - The runner image supplies node 26, npm 12, uv, Python, and ripgrep. The setup-node, setup-uv, and per-job install steps are gone. - Checkout uses a node-local git mirror, seeded from the runner pod env. - buildx layer cache moved to Artifact Registry in us-central1, the same region as the runners. Reads are keyless through GKE Workload Identity. Writes use GitHub OIDC and happen only on main pushes and releases, so pull-request code cannot write a layer that the publish job reads. Merge-base work, in the shadow copies only: - A new composite action, .github/actions/merge-base, deepens a shallow clone until the two histories connect. fetch-depth: 0 fetches all ~1400 refs and measured 76-81s, against 3-6s for a shallow checkout. - The action fails by default when no merge base exists. A three-dot diff over a missing merge base scans nothing and reports clean, so the supply-chain audit must stop. history-check sets fail-on-missing to false, because absence is the result it measures. - lint diffs against the base commit directly. The job checks out the PR merge ref, so base.sha is already the correct comparison point. - contributor-check uses origin/main..HEAD. The result equals the merge-base form, and the extra git call also expanded a SHA without quotes. Other changes: - .github/actionlint.yaml declares the four ARC labels. actionlint knows only GitHub-hosted labels, so every runs-on in the repo was reported as an unknown label: 40 warnings that hid real findings. - scripts/ci/resource_profile.py records CPU and memory for a job step. The timing report shows the data per step. - run_tests_parallel.py can list test files from the git index. The slice generator then needs no blobs. - Docker test files are split so boot-heavy tests run in parallel. - Container-environment parity fixes in doctor, gateway, and skill_utils, with tests. To retire the shadow: delete .github/workflows/newci-*.yml.
the idle deadline starts at popen, so the first window has to cover the helper script's interpreter startup — wall clock the test doesn't control. 3997561 shrank the stt windows to 0.1s, which is under a cold `python -u` on a loaded runner, so the child got killed before printing anything and the assertion saw 'Terminated\n' instead of its progress line. hoist the timings to named constants (idle 2.0s, 12 ticks x 0.25s) with a setup assertion that total runtime still exceeds the idle window, so a future speed pass can't silently tune the test into not exercising the deadline reset. fix the same latent race in the sibling tts test it was ported from (0.2s window vs a real spawn) before it fires too. swept tests/ for the pattern; the only other hit (win_pty_bridge) is a per-read poll inside a 5s outer loop, not a total budget.
TestPrefetchServerRetainVisibility polled the mocked op-status endpoint at the production _RETAIN_OP_POLL_INTERVAL_S of 0.5s, so each test burned ~1s of real sleep plus a cross-thread hop to the shared hindsight event loop per poll — all inside a 5.0s join on the background prefetch thread. On a busy runner that thread is exactly what gets starved, so it outlived the join and the assertion saw an empty order list: assert [] == ['recall']. collapse the interval to 0.01s in these tests (against an AsyncMock there is no server to be polite to) and route the joins through a helper that asserts the thread actually finished, so a genuine wedge reports itself instead of surfacing as a confusing empty-list mismatch. also drop the two wall-clock assertions that measured runner load rather than the contract under test: the drain-budget bound goes from 3.0s to budget+10s, and the eviction test now asserts the status endpoint saw no further calls (the actual "dropped ops aren't re-polled" contract) instead of timing the second prefetch at <0.25s. reproduced by pinning the tests to one core against N spinners on that same core: at 16x oversubscription pre-fix failed 3/3 and post-fix passed 3/3. class wall time drops 1.45s -> 0.44s for the recall test.
main added install-e2e.yml + install-e2e-run.yml; mirror them as newci-* so the migration gets the same signal it has for the other lanes. triggers match production exactly (same tag filters, same 12h cadence, cron offset :20 -> :25 so the two runs don't contend for the pool). it cannot fail the production job. it's a separate workflow that production never calls or reads, and belt-and-braces the e2e step is continue-on-error so a failing leg is reported in the summary instead of reddening a check. the tolerance lives on the STEP, not the job: job-level continue-on-error is not a legal keyword on a job that calls a reusable workflow (only name/uses/ with/secrets/strategy/needs/if/concurrency/permissions are) — my first draft had it on the matrix jobs and would not have parsed. noted in the header so nobody "fixes" it back. the legs are expected to SKIP for now. the e2e runs inside dev-sandbox.sh (bubblewrap), which needs to remount / as slave and mount a fresh /proc, and a stock ARC pod denies both. probed in-cluster on nous-gke-runner: default pod Failed to make / slave: EPERM capabilities.add: [SYS_ADMIN] Can't mount proc: EPERM SYS_ADMIN + apparmor/seccomp Unconfined Can't mount proc: EPERM privileged: true works arc-runner-docker does NOT qualify — only its dind sidecar is privileged, the runner container isn't. so a ~30s preflight job probes bwrap and skips the legs with the capability matrix in the step summary, rather than burning ~11min per leg to fail at the same mount. when infra adds a privileged set, pass its label as `runner` and the legs start running with no other change. actionlint clean; summary script exercised for both skipped-legs and failing-leg shapes (exits 0 in both).
69e17fa to
16f88e1
Compare
ops/s alone can't tell you whether a device is saturated — it reads low on a few large IOs that pin the disk at 100% busy, and high on many small cached ones. sample io_ticks (diskstats field 13) instead: its delta over the interval is device busy time, i.e. iostat's %util. take the busiest single device rather than the sum, since summing across devices exceeds 100% on a multi-disk node and means nothing as a saturation percentage. divide by the real elapsed gap rather than the nominal interval — a loaded or throttled runner drifts well past 1.0s and would otherwise report >100%. emit a downsampled series (cpu/mem/disk, 0-100 ints) alongside the existing summary, mean-bucketed to 180 points so a 40min job costs the same few KB as a 40s one. the report inlines every profile into one self-contained html file, so an unbounded 1Hz series would dominate its size. mean, not every Nth sample: a spike that survives decimation by luck is misleading. the report renders the series as svg sparklines over each gantt bar, in two states off the same markup (3px strip when collapsed, full height when expanded) via preserveAspectRatio=none. profiles predating the series field degrade to table-only, no overlay. tests/ci 62/62.
6939204 to
979b9bf
Compare
30ac062 to
52ca88f
Compare
… job the sparklines were stretched across the whole gantt bar, but the profiler wraps a SINGLE step (.github/actions/profile runs it between start/stop), so on a job dominated by checkout + uv sync + post-job cleanup the samples only describe a slice in the middle. reproduced: a 30s profile inside a 100s job whose profiled step ran t=60..90 drew at left 0% width 100% instead of left 60% width 30% — putting a cpu spike visually under a step that never ran. the profile json had no wall-clock anchor to place it with, only duration_s, so emit started_at/completed_at as iso-8601 utc in the same shape as github's job timestamps. monotonic() still drives the sampling loop (immune to clock steps); the timestamps are purely for placement. _profile_window_pct() converts that window into bar-relative percentages and both overlay states now use it — the expanded holder directly, the collapsed strip via a .res-clip wrapper so its 100%-width is relative to the window rather than the bar. the two states are asserted to agree on the x-axis. falls back to the full bar, i.e. exactly today's behaviour, when the profile predates these fields, when the timestamps don't parse, or when the window doesn't overlap the job at all (clock skew between the runner writing the profile and github's timestamps). a profiler that outran the job's completed_at is clamped to the bar, and a sub-percent window keeps a 0.5% hairline so it can't collapse to invisible. 7 new tests. verified they discriminate: forcing the old always-stretch behaviour fails 4 of them, while the 3 fallback tests keep passing since full-bar is what they want. tests/ci 145/145.
52ca88f to
9b3c423
Compare
What does this PR do?
This PR runs a duplicate of CI on the new GKE self-hosted runners (ARC),
beside the existing CI. It does not change production CI.
Every workflow in
.github/workflows/that does not start withnewci-isbyte-identical to
main. The new runner infrastructure lives in 16newci-*.ymlfiles.A runner migration changes the machine under every job. A direct migration
breaks the merge gate for everyone at once if the new runners are wrong. The
shadow gives the same signal with no shared failure mode. Watch the shadow
runs for a few days, then migrate.
How the shadow cannot break production CI:
newci-prefixed. The production groups usecancel-in-progress, so a shared group would cancel the real run.newci-prefixed.uses:points at anothernewci-*file.live_comment.pyruns--dry-run. It prints the body to the job log and posts nothing.[newci] All checks pass (informational). Branch protection still requiresAll required checks pass.dockerpublishandmergerequire a push tomainor a release. A pull request cannot reach them.js-autofix,deploy-site, andskills-indexare not copied. They run on push or on a schedule.Only
ci.ymland the 15 reusable workflows it calls on a pull request arecopied. The shadow runs on
pull_requestonly.Related Issue
No issue. This is planned infrastructure work, not a reported bug.
Type of Change
CI infrastructure. The closest match is "new feature": it adds 16 workflow
files and 3 composite actions, and changes no production behavior.
Changes Made
Shadow workflows (new, 16 files)
.github/workflows/newci-ci.yml— the shadow orchestrator. Runs onpull_requestonly, and calls the 15 shadow workflows below..github/workflows/newci-{tests,lint,js-tests,e2e-desktop,docs-site-checks,history-check,contributor-check,uv-lockfile-check,infographic-check,lockfile-diff,docker-lint,docker,supply-chain-audit,review-labels,osv-scanner}.ymlComposite actions (new)
.github/actions/merge-base/action.yml— deepens a shallow clone untiltwo histories connect.
fetch-depth: 0fetches all ~1400 refs andmeasured 76-81s, against 3-6s for a shallow checkout. The action fails
when no merge base exists, because a three-dot diff over a missing merge
base scans nothing and reports clean.
history-checkopts out withfail-on-missing: false, since absence is the result it measures..github/actions/profile/action.yml— records CPU and memory for a step..github/actions/uv-cache/action.yml— shared uv cache restore.Runner infrastructure (inside the shadow copies only)
arc-runner-smallfor short gate jobs,arc-runner-setfor general work,arc-runner-dockerandarc-runner-arm64for image builds. dind is only on the docker sets, soabout 20 workflows stop paying for a privileged sidecar.
the
setup-node,setup-uv, and per-job install steps are gone.us-central1, the sameregion as the runners. Reads are keyless through GKE Workload Identity.
Writes use GitHub OIDC and happen only on main-push and release contexts,
so pull-request code cannot write a layer the publish job later reads.
Merge-base cleanup (inside the shadow copies only)
Two jobs did not need a merge base at all:
lintchecks out the PR merge ref, sobase.shais already the correctcomparison point. Using the branch point instead pulled diagnostics that
mainintroduced after the branch point into the PR's own diff.contributor-checkusesorigin/main..HEAD, which equals the merge-baseform. The removed call also expanded a SHA without quotes.
Other
.github/actionlint.yaml— declares the four ARC labels. actionlint knowsonly GitHub-hosted labels, so every
runs-on:in the repo was reported asan unknown label: 40 warnings that hid real findings.
scripts/ci/resource_profile.py+scripts/ci/timings_report.py—per-step CPU and memory in the timing report.
scripts/run_tests_parallel.py— can list test files from the git index,so the slice generator needs no blobs.
tests/docker/— boot-heavy test files split so they run in parallel.hermes_cli/doctor.py,gateway/run.py,agent/skill_utils.py—container-environment parity fixes, with tests.
How to Test
second set with
[newci]in the name.[newci]job against its production twin on the same commit.Both must reach the same conclusion. A
[newci]job that fails while itstwin passes is a runner-environment difference, not a code problem.
[newci] CI / CI review comment (live). It prints thecomment body under
--- DRY RUN — comment body ---and posts nothing.Only one
hermes-ci-review-botcomment exists on the PR.ci-timings-reportartifact from the[newci]run for theper-step time, CPU, and memory breakdown.
What to watch during the trial
[newci]against production on the same PR.[newci]job that fails while its production twin passes.15 minutes; a warm one is about 3.
To retire the shadow. If the trial succeeds, copy the
newci-*bodiesover the production workflows, remove the
newci-prefixes and theshadow-only edits (
--dry-run, the renamed gate), and delete.github/workflows/newci-*.yml. If the trial fails, delete the 16 files.Nothing else changes.
Checklist
Code
fix(scope):,feat(scope):, etc.)pytest tests/ -qand all tests pass — see the note belowOn the test run. I ran the targeted suites through the required wrapper,
scripts/run_tests.sh, not barepytest:I did not run the full suite locally.
AGENTS.mdsays the wrapper enforcesCI parity, and the full suite is too heavy for this machine. CI runs it.
Other verification:
does not touch. Runner-label warnings went from 40 to 0.
origin/main: nodifferences.
pushed to a local bare remote, which is the PR ci(docker): split :latest (releases only) from :main #25045 failure mode and not
a simulation of it:
found=falsewithfail-on-missing=false, exit 1with
fail-on-missing=true, and 17s on a branch 150 commits behind.tests/ci/test_resource_profiles.py(21 tests, new file) andfour new
tests/docker/files.tests/test_run_tests_parallel.pygainscases for the git-index discovery path (10 tests in the file).
Documentation & Housekeeping
docs/, docstrings) — or N/AEach shadow file opens with a banner that states what it is, which
safety properties to keep, and how to delete it.
cli-config.yaml.exampleif I added/changed config keys — N/A. No config keys change.CONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows — N/A. The shadow is temporary and production CI does not change.Screenshots / Logs
The
[newci]check runs on this PR are the evidence. Compare them againstthe production runs on the same commit.
The dry-run PR comment appears in the log of
[newci] CI / CI review comment (live):Note
How do we make it live, once we're happy?
migrate
deploy-site.yml js-autofix.yml label-rerun.yml publish-e2e-evidence.yml skills-index.yml skills-index-freshness.ymlto the new runnersRestore the push: trigger on ci.yml.