ci: migrate workflows to ARC runners on self-hosted infra - #77
Conversation
Repo hit the private-repo free-minutes limit, so ubuntu-latest jobs fail instantly on billing. Route every workflow to the org's self-hosted pool via the auto-applied [self-hosted, Linux] labels. https://claude.ai/code/session_01TjofiprA2cKgSYqzgzMScn
|
Note Gemini is unable to generate a summary for this pull request due to the file types involved not being currently supported. |
There was a problem hiding this comment.
Pull request overview
Routes GitHub Actions workloads off GitHub-hosted runners to the org’s self-hosted Linux runner pool to avoid failing CI due to the repo’s billed-minutes cap.
Changes:
- Replace
runs-on: ubuntu-latestwithruns-on: [self-hosted, Linux]across all existing workflows/jobs. - Move CI, release, triage, and automation workflows (labeling, approvals, orchestration, Claude automation) onto self-hosted runners.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| .github/workflows/triage.yml | Run issue triage/label automation on self-hosted runner. |
| .github/workflows/release.yml | Run tag-based release workflow on self-hosted runner. |
| .github/workflows/project-orchestrator.yml | Run project board orchestration on self-hosted runner. |
| .github/workflows/pr-title.yml | Run required PR title validation on self-hosted runner. |
| .github/workflows/label.yml | Run PR labeler on self-hosted runner. |
| .github/workflows/dependabot-automerge.yml | Run Dependabot automerge workflow on self-hosted runner. |
| .github/workflows/claude-review.yml | Run Claude PR review automation on self-hosted runner. |
| .github/workflows/claude-agent.yml | Run Claude agent workflow on self-hosted runner. |
| .github/workflows/ci.yml | Run all core CI jobs (check/lint/test/build/integration/sdk/e2e) on self-hosted runner. |
| .github/workflows/admin-approval.yml | Run admin approval gate workflow on self-hosted runner. |
Four runners sharing one VM's disk was thrashing `make coverage`: - `TestDLQStats_EmptyWhenNoStream` / `TestDLQStats_ReturnsCorrectCounts` hit `create stream: context deadline exceeded` inside `NewEmbedded`. The JetStream 5s default API timeout blows when `-race` overhead + `SyncAlways: true` fsyncs compete with three parallel test runs on the same disk. Keeping `-race` and `SyncAlways` — serialize the job instead with a static concurrency group so only one Test runs at a time across all PRs/pushes. Other CI jobs still parallelize. - `TestLocalCache_Overwrite` got `v1` back when it expected `v2`. The 10ms post-Set sleeps weren't enough for Ristretto's async admission goroutine under load. Swap to `c.Wait()` (already used by the sibling `TestLocalCache_ExpiredKey`) so admission is deterministic. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`make coverage` used `go test`'s default package parallelism, so even with the Test job serialized at the CI level, 8 packages were still fsyncing embedded NATS in parallel inside a single job. That shared- disk contention was enough to blow JetStream's 5s stream-creation timeout (TestDLQStats_EmptyWhenNoStream), even when no other job on the VM was active. `-p 1` serializes package execution within the coverage run. Keeps `-race` and SyncAlways untouched. Adds ~2-3 min to the Test job on either CI or local runs; use `make test` for fast local iteration. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Hypothesis was that `go test`'s default package-parallelism was contending on disk inside the Test job. Local reproduction on a dev machine (Apple Silicon, NVMe) showed 4 concurrent copies of `make coverage` all pass in ~18 s each and zero failures — so package parallelism isn't the issue. On the CI VM `TestDLQStats_*` hangs for 150-193 s before timing out even with `-p 1` and the Test job already serialized via `concurrency:`, which means the stall is inside the VM (disk/fsync/zombie-process), not inside `go test`'s scheduler. Backing out `-p 1` to restore the ~3 min of Test time it costs. VM diagnosis next. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Codify the workflow that surfaced during PR #77's CI-flake debug: validate locally with `make ci` / `make coverage` before pushing, and treat local-pass / CI-fail as an environment mismatch (not a test bug) until proven otherwise. Adds a new MANDATORY "Local-First Validation" section to AGENTS.md covering: the pre-push checklist, the diagnostic order-of-operations when local and CI disagree (reproduce → reproduce-under-load → touch the runner, in that order), and explicit instructions for delegating to subagents. Mirrors a shorter pointer into copilot-instructions.md rule #8. Reason: PR #77's DLQ-test flakes were iterated on via four CI pushes before running the same tests locally — which would have showed they pass in milliseconds, reframing the whole investigation as a VM problem rather than a test problem. The rule is written to stop that pattern recurring with future agents on this repo. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Swap `runs-on: [self-hosted, Linux]` → `[wave-rf-runners]` across all 10 workflow files. ARC's RunnerSet exposes its own label rather than the auto-applied Linux/self-hosted pair the previous self-hosted VM runners used. ci.yml Test: drop the static `ci-test-self-hosted` concurrency group. That was a workaround for github-vm's shared-zvol fsync contention; ARC's per-pod isolation eliminates the contention, and keeping the group would now globally serialize Test across all PRs. ci.yml E2E: pre-build the wavehouse image via `docker buildx build --load` before running tests. With BUILDX_BUILDER preset on the runner pod, the build routes to the shared remote BuildKit (cache shared across runners and repos) and loads the result back into the DinD sidecar's local daemon. Setting WAVEHOUSE_IMAGE then makes tests/compose.yaml use the loaded image instead of triggering its own `build:` directive — explicit per the header comment in that compose file. release.yml: label change only. Multi-arch is the BuildKit VM's concern with remote builds, and BUILDX_BUILDER is preset on the runner — no setup-qemu / setup-buildx needed at the workflow layer. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- internal/cache/local_test.go: assert NoError on both `c.Set` calls in TestLocalCache_Overwrite (was discarding the error). A failing Set would have produced a misleading downstream assertion failure rather than the actual cause. - AGENTS.md: reword the actionlint guidance per Copilot's suggestion. The repo doesn't pin or wrap `actionlint` (no make target, not in go.mod tools), so framing it as an optional local-dev tool rather than a mandated step matches reality. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Acknowledged @gemini-code-assist — this PR's diff is dominated by |
|
Acknowledged, @EricAndrechek. Thank you for clarifying the situation regarding the workflow-heavy nature of this PR and the associated limitations with AI review coverage. The documentation updates in |
Default `pull_request` event types are `[opened, synchronize, reopened]` and don't include `ready_for_review`. The e2e job is gated on `github.event.pull_request.draft == false`, so when a PR is opened as draft and later flipped ready, the existing CI run's event payload still says draft=true and e2e silently skips. Adding `ready_for_review` to the event filter ensures the full suite (including e2e) runs at the moment the PR becomes review- ready, without needing to push an empty commit to force a fresh trigger. Hit on this exact PR: e2e was missed on commit 97eb39c because the push happened while the PR was still draft, and flipping ready didn't re-fire CI. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Completes the ARC runner migration for the new workflow added in this PR. PR #77 swapped every existing workflow's runs-on from ubuntu-latest to [wave-rf-runners]; the new board-state-sync.yml introduced here was branched off pre-#77 main and missed the sweep. Without this, the mirror job would queue indefinitely on the now-empty ubuntu-latest pool.
) ## Summary Second-order fix on top of #105. The author-trust + required-check gate in `claude-review.yml` now uses the REST `check-runs` endpoint instead of GraphQL's `statusCheckRollup` field on the PR view. ## Why `gh pr view --json statusCheckRollup` asks GraphQL to navigate `checkSuite.workflowRun` on every check context on the PR's head commit. The workflow's `GITHUB_TOKEN` is not allowed to read that sub-field for checks owned by other integrations (Gemini Code Assist, Copilot, third-party statuses). When a PR has enough cross-integration checks the whole call fails with: ``` GraphQL: Resource not accessible by integration (...statusCheckRollup.contexts.nodes.5) (...nodes.0..4.checkSuite.workflowRun) ``` We don't actually use any of that traversal — the gate just needs `name`, `status`, `conclusion` for two named checks (`CI`, `PR housekeeping`). The REST endpoint returns exactly that and nothing else, so it doesn't trip the cross-integration permission issue. This failure was always there — #105 fixing `authorAssociation` just let the script reach the next bug. Visible on PR #88 (long-lived, many integrations) but not PR #7 (fewer integrations), which is why we didn't catch it in initial testing. ## What changed - `gh pr view --json` drops `statusCheckRollup`; keeps `isDraft`, `author`, `headRefOid`. - New `gh api repos/$REPO/commits/$head_sha/check-runs?per_page=100` call for the rollup, only inside the `workflow_run` branch (other event paths bypass the snapshot like before). - Status / conclusion comparisons updated from uppercase (GraphQL enum: `COMPLETED`, `SUCCESS`, ...) to lowercase (REST enum: `completed`, `success`, ...). - Gate behavior is otherwise identical: same set of pass-through conclusions, same skip notices, same fall-through to the Claude action when CI is green. ## Verified `gh api repos/Wave-RF/WaveHouse/commits/a4f5a6b1cc83ffdca5429c0d72fb593f3f232eda/check-runs` (PR #88's current head) returns both `CI` and `PR housekeeping` with `status=completed, conclusion=success`. No GraphQL errors. ## Out of scope (mentioned in investigation, not changed here) - `observability` branch and `context_aware_mq_2` branch still carry pre-#89 workflow files (`board-state-sync.yml`, `claude-agent.yml`) and a pre-#77 runner label. Rebase those branches onto current `main` to drop them. - The \"Claude\" run on `context_aware_mq_2` (event `dynamic`, runs on `ubuntu-24.04`) is GitHub Copilot Coding Agent in Claude mode, not our workflow file. - PR #107 (observability) `PR housekeeping` failures are legitimate — the PR title exceeds 72 chars. ## Test plan - [ ] Next workflow_run for PR #88 (or any internal PR with cross-integration checks) — the gate completes without GraphQL errors. - [ ] Draft / Dependabot / untrusted-author PRs still skip with their respective notices. - [ ] A green CI on a trusted-author PR still falls through to the Claude review step. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
## Why Prep for flipping the repo public: the `wave-rf-runners` ARC pool must never serve a public repo (GitHub's hard rule on self-hosted + public), and GitHub-hosted standard runners are free for public repos — with public `ubuntu-latest` being the 4-core/16 GB tier. Reverses the #77 calculus, exactly as planned in the public-launch gates. ## What 1. **`runs-on: [wave-rf-runners]` → `ubuntu-latest`** — all 11 jobs across all 10 workflows. 2. **`docker/setup-buildx-action` in `publish-dev.yml` + `release.yml`** (`d7f5e7f` = v4.1.0, SHA verified against upstream): `dockers_v2` builds the `linux/amd64+arm64` manifest via buildx, and the hosted default docker driver can't do multi-platform — the runner-pod `BUILDX_BUILDER` (remote BuildKit VM) used to provide this. No QEMU needed: `Dockerfile.goreleaser` stages are `$BUILDPLATFORM`-pinned / COPY-only by design. `goreleaser-validate.yml` correctly stays buildx-free (`build --single-target` never touches `dockers_v2`). 3. **Cache-key fix**: the pnpm/Playwright/Astro keys hashed `clients/ts/`, `tests/e2e/sdk/`, `docs/` lockfiles — paths deleted in the #190 root-workspace consolidation — so `hashFiles()` was empty and the keys degenerated to constants that never invalidated. Now keyed on the root `pnpm-lock.yaml`. Old degenerate entries stay reachable via unchanged `restore-keys`, so the first run warm-starts and re-saves under the correct key. No falcondev cache-server or registry-proxy references existed in any workflow (the redirects were runner-pod env) — `actions/cache` falls back to GitHub's backend transparently. Footprint ~2–3 GB vs the 10 GB/repo cap. ## Verification - `make ci` green locally (full pipeline incl. integration + e2e) - pre-push reviewer: ship_it, zero findings; docs review skipped on the record (workflows-only diff) - actionlint: clean for this diff (5 pre-existing info-level shellcheck nits on untouched lines) - repo-wide grep: zero remaining `wave-rf-runners` / `self-hosted` / `BUILDX_BUILDER` references ## Watch-items (post-merge) - **First multi-arch publish on main**: the buildx push path gets its first real exercise on the post-merge `publish-dev.yml` run — watch it. (A pre-merge dispatch from this branch would clobber the rolling `:dev` tag, so deliberately not done.) - **Docker Hub pulls**: test suites pull `clickhouse/clickhouse-server:latest` unauthenticated; hosted runners share egress IPs, so rate-limit flakes become *possible*. If they appear: GHCR mirror or registry login. - **Until the flip**, this repo is private → runs use the 2-core tier and bill against org included minutes; the `ci.yml` 4-core comment describes the post-flip state. Well inside the 45-min cap either way. - **Open PRs** still carry workflow files targeting the old pool — each needs a merge-from-main after this lands (`@dependabot rebase` for the bot ones), same dynamic as #77. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Summary
ubuntu-latestjob was failing instantly on billing.1ba98a5) routed all 15runs-onentries to[self-hosted, Linux]to use the org's 4-runner VM pool.7d78185): allruns-onare now[wave-rf-runners](the custom label exposed by the RunnerSet, not the auto-appliedLinux/self-hostedpair). Per-pod isolation eliminates the disk contention.docker buildx build --load) and setsWAVEHOUSE_IMAGEsotests/compose.yamluses the prebuilt image instead of re-building from source.pull_requestevents GitHub uses the workflow from the PR head, so this PR's own CI already routes to ARC and validates the change.Caveat:
pull_request_targetworkflowslabel.yml,pr-title.yml, andproject-orchestrator.ymltrigger onpull_request_target, which runs the workflow frommain. Those three will keep hitting the billing wall on this PR until merge. TheValidatecheck (pr-title) is a required merge gate per the ruleset — plan to admin-bypass the merge for this one PR only. Every subsequent PR will have all workflows on ARC.Runner assumptions
ARC pods need:
git(auto),make,bash,curl,jq,ghCLI. Go / Node install viaactions/setup-go/actions/setup-node(need writable$RUNNER_TOOL_CACHEand outbound to mirrors). Docker access via the DinD sidecar; BuildKit access via presetBUILDX_BUILDERenv var.triage.ymlalso needs outbound toapi.github.com/models.Test plan
Checkpasses on the new runner poolLintpassesTestpasses (after VM-level disk contention diagnosed and resolved by the ARC migration)BuildpassesSDK TestspassesIntegration Testspasses (Docker via DinD sidecar)E2E Tests(with prebuilt image) andReviewhttps://claude.ai/code/session_01TjofiprA2cKgSYqzgzMScn
Originally generated by Claude. Iterated by @EricAndrechek + Claude through ARC migration.