Skip to content

ci: run workflows on Blacksmith runners - #397

Merged
deanq merged 2 commits into
mainfrom
ci/blacksmith-runners
Oct 29, 2026
Merged

deanq merged 2 commits into
mainfrom
ci/blacksmith-runners

Conversation

@justinwlin

Copy link
Copy Markdown
Contributor

The runpod org now has an IP allow list, and GitHub-hosted runners are outside it. Any job that calls the GitHub API from them fails with the runpod organization has an IP allow list enabled, even with the default GITHUB_TOKEN. This moves every job to blacksmith-2vcpu-ubuntu-2404, the label runpod/main-ui and runpod/runpod-assistant already use.

The 1.21.1 Release run failed this way (release-please's app token request got a 403), so 1.21.1 is on main but has no tag and isn't on PyPI. Once this merges, rerun the Release workflow on main. Merge runpod-workers/flash#112 first: 1.21.1 includes #390, which expects the 1.6.1 worker image.

@OneAsterisk OneAsterisk left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude review, round 1

Verdict: Ready after fixes. The runner move is correct and complete, but 2 vCPU roughly doubles Quality Gates time and puts 3.14 within a minute of its timeout.
Approval gate: 0 Critical, 1 High, 0 Medium+Low → not approved: 1 High open.

PR description vs code

Claim Status Where
GitHub-hosted runners are blocked by the org IP allow list ✅ Release run 40932225724: create-github-app-token gets a 403 "IP allow list enabled"
Moves every job to blacksmith-2vcpu-ubuntu-2404 ✅ No ubuntu-latest left under .github/ (11 runs-on lines, all Blacksmith)
"Any job that calls the GitHub API ... fails, even with the default GITHUB_TOKEN" ⚠️ Not reproduced. CI on main passed on ubuntu-latest today (40932225676). The confirmed failure is the App token request.
Same label runpod/main-ui and runpod/runpod-assistant use ⚠️ runpod-assistant uses 2 vCPU. main-ui uses blacksmith-4vcpu-ubuntu-2404 for 8 of 13 jobs and 2 vCPU for 4.
1.21.1 is on main, untagged, not on PyPI ✅ Latest tag v1.21.0; PyPI latest 1.21.0
Merge runpod-workers/flash#112 first ✅ Still open: "chore(main): release 1.6.1"

Findings

High

  • Quality Gates now runs at 94% of its timeout (.github/workflows/ci.yml:225). On this PR the matrix took 9m5s to 14m7s, against timeout-minutes: 15. The last four main runs on ubuntu-latest took 3m54s to 5m35s; this repo is public, so those were 4 vCPU runners. Run-to-run spread on main is about 40%, so a slower 3.14 leg will time out and fail Validation, which blocks merges. Fix: use blacksmith-4vcpu-ubuntu-2404 for quality-gates (main-ui's common label), or raise the timeout.

Nits

  • .github/workflows/ci.yml:315 the Nix job comment still says it runs on "x86_64-linux (ubuntu-latest)".

Verified clean

  • Guard, pre-check, build, validation, and e2e need no runner-specific setup; all steps are standard actions on Ubuntu 24.04, same OS as ubuntu-latest.
  • Nix flake took 9m4s here vs 5m17s to 8m10s on main, well under its 30-minute timeout.
  • Pre-check (11s vs 3m timeout) and Guard (9s vs 2m) have plenty of margin.
  • release-please.yml keeps id-token: write and the pypi-production environment, so Trusted Publishing claims are unchanged.
  • PR title follows the repo's Conventional Commits template; ci: is a hidden type, so no changelog entry.

Questions for the author

  1. pypi-publish does not call the org API. The pypa/gh-action-pypi-publish README calls non-GitHub-provided runners "best effort" and recommends publishing from GitHub-hosted runners. Was keeping that one job on ubuntu-latest considered, or has Trusted Publishing been confirmed on Blacksmith in another runpod repo?
  2. This repo is public. Fork PRs will now run on Blacksmith (billed) instead of free hosted runners. Is that intended, and does the org's Blacksmith setup require approval for fork runs?

Reviewed by Claude (claude-opus-5-5) via the pr-review skill. Tokens: 22 in / 9,885 out / 929,814 cache read / 86,068 cache write = 1,025,789. Estimated cost: $2.45 at the skill's bundled Opus 5.5 rate table (1-hour cache writes at 2× input). Round 1 of this PR. This is automated assistance; a human owns the final call.

Comment thread .github/workflows/ci.yml Resolved
Comment thread .github/workflows/ci.yml Resolved
Comment thread .github/workflows/e2e.yml Resolved
Quality Gates legs ran 9-14 min on 2 vCPU against a 15 min timeout, and
E2E would hit its 45 min timeout. Also update the Nix job comment to
name the Blacksmith runner.

@OneAsterisk OneAsterisk left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude review, round 2

Verdict: Approved — both round 1 findings and @deanq's E2E thread are fixed at 65baf56, and CI on this head is green with Quality Gates back at main's timings.
Approval gate: 0 Critical, 0 High, 1 Medium+Low → approved.

Status of prior findings

# Finding Severity Status Evidence
R1-1 Quality Gates ran at 94% of its 15-min timeout on 2 vCPU High Fixed ci.yml:225 is blacksmith-4vcpu-ubuntu-2404 (65baf56). Run 40967445910: five legs 4m27s to 5m18s. Round 1 run: 9m05s to 14m07s. Main on ubuntu-latest: 3m54s to 5m35s.
R1-2 Nix job comment still said "ubuntu-latest" Nit Fixed ci.yml:314-315 (65baf56)
deanq-1 E2E on 2 vCPU could run past timeout-minutes: 45 Medium (human thread) Fixed in code, runtime unverified e2e.yml:25 is blacksmith-4vcpu-ubuntu-2404 (65baf56). No E2E dispatch run on this branch; last run anywhere was 2026-06-05 on main.

Existing feedback

  • @deanq on e2e.yml:25: Already addressed, replied with the runtime caveat. Thread resolved by deanq. #397 (comment)
  • Round 1 threads on ci.yml:225 and ci.yml:322: verified fixed, replies added. Both were resolved by @deanq after the fix commit.

PR description vs code

Claim Status Where
Moves every job to blacksmith-2vcpu-ubuntu-2404 ⚠️ Stale 9 of 11 jobs. ci.yml:225 (Quality Gates) and e2e.yml:25 (E2E) now use blacksmith-4vcpu-ubuntu-2404.
Same label main-ui and runpod-assistant use ⚠️ (as in round 1) main-ui uses 4 vCPU for most jobs. This PR now mirrors that split.
1.21.1 is on main, untagged, not on PyPI ✅ Latest tag v1.21.0, PyPI latest 1.21.0 (re-checked)
Merge runpod-workers/flash#112 first ✅ Still open
IP allow list blocks GitHub-hosted runners ✅ Unchanged from round 1

Findings

Low

  • PR body is stale on the runner label. The body says every job moves to blacksmith-2vcpu-ubuntu-2404; Quality Gates and E2E now use the 4 vCPU label. One sentence fixes it: every job moves to Blacksmith, 4 vCPU for Quality Gates and E2E, 2 vCPU for the rest.

Nits

  • ci.yml:315 the comment now repeats the runs-on value seven lines below it and will go stale again on the next runner change. "x86_64-linux" alone says what matters. Optional.

Verified clean

  • CI run 40967445910 on 65baf56 is green and every job scheduled on a Blacksmith runner (runner names confirm both labels resolve). Quality Gates 4m27s to 5m18s on 4 vCPU (15-min timeout). Nix flake 7m43s on 2 vCPU (30-min). Pre-check 13s (3-min), Guard 7s (2-min), Validation 6s (1-min).
  • Build Package did not run here (should_build=false, no packaging changes). On ubuntu-latest it takes 46s to 50s against a 5-min timeout, so 2 vCPU has margin even at 2.5× slower.
  • No ubuntu-latest left under .github/. The diff since round 1 is the two label changes and the comment fix, nothing else.
  • Round 1 checks still hold: release-please.yml keeps id-token: write and the pypi-production environment, and no job has runner-specific setup.

Questions for the author (carried from round 1, still unanswered)

  1. pypi-publish runs on Blacksmith. pypa/gh-action-pypi-publish calls non-GitHub runners "best effort" for Trusted Publishing. Was keeping that one job on ubuntu-latest considered, or has publishing from Blacksmith been confirmed in another runpod repo?
  2. This repo is public, so fork PRs now run on billed Blacksmith runners instead of free hosted ones. Intended? Does the org's Blacksmith setup require approval for fork runs?

Reviewed by Claude (claude-fable-5-1) via the pr-review skill. Tokens: 482 in / 23,571 out / 1,202,888 cache read / 89,216 cache write = 1,316,157. Estimated cost: $3.27 at the published Fable 5.1 rates ($10 input, $50 output, $0.25 cache hit, $20 1-hour cache write per MTok; 1-hour cache TTL assumed). Round 2 of this PR. This is automated assistance; a human owns the final call.

@deanq
deanq merged commit 9fb06c1 into main Oct 29, 2026
10 checks passed
@deanq
deanq deleted the ci/blacksmith-runners branch October 29, 2026 23:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants