ci: use Qiniu runners for Linux jobs - #2476
Merged
Merged
Conversation
MeteorsLiu
marked this pull request as draft
September 2, 2026 05:49
![fennoai[bot]](https://avatars.githubusercontent.com/in/1924301?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Review: route Linux CI to self-hosted qiniu runners
The change is mechanically correct: every conditional runs-on expression's test value matches its matrix's Linux os entry, non-Linux (macOS/Windows) lanes fall through to matrix.os unchanged, the ubuntu-24.04-arm lane is correctly preserved on GitHub-hosted ARM, and the (a && fromJSON(...)) || matrix.os short-circuit is valid Actions syntax. The concerns below are about operational and security consequences of running on self-hosted hardware, not syntax.
Security — the main concern (fork PRs on self-hosted runners)
Most of these workflows trigger on pull_request: branches: ["**"], so fork PR code now executes on the self-hosted qiniu host. Self-hosted runners on a public repo are the classic compromise vector: unlike ephemeral GitHub-hosted VMs, the host is persistent, shared, and on your network. The release-build.yml build job is the sharpest edge (Docker socket + fork-controlled tree — flagged inline). Please confirm the qiniu runner group is not offered to fork PRs (org/repo runner-group setting), or gate the PR-triggered jobs to github.repository == 'xgo-dev/llgo' and keep fork PRs on GitHub-hosted runners. Note notify-benchmarks.yml already models the right gate on its dispatch job (github.repository == 'xgo-dev/llgo' && github.event_name != 'pull_request') — the classifier-tests job does not.
Operational (worth confirming, not blocking)
- Capacity / throughput: all Linux CI now requires the
[qiniu, ubuntu-24.04]pool. Linux was effectively elastic onubuntu-latest; on a busy multi-PR day (llgo.yml + goroot.yml shards + go.yml + benchmark + release) throughput is now bounded by the self-hosted pool size. Confirm it's sized for peak concurrent load. - Runner label assumption: the whole change rests on the
qiniurunners being labeled exactlyubuntu-24.04. If they carry a different label, every routed job fails to schedule. - Benchmark comparability:
benchmark.yml's Linux lane moves to shared self-hosted hardware. Benchmarks are CPU/neighbor-sensitive (the job even pinsGOMAXPROCS: 2to reduce variance); new numbers may not be comparable to the existing GitHub-hosted baseline. Consider a dedicated/single-tenant label for this leg.
Maintainability (minor)
- The conditional expression is duplicated ~10 times plus 2
ubuntu-24.04variants; a future group/label change must be applied in lockstep across all of them. - Mixed idiom: Linux-only matrix jobs (
targets.yml,llgo.ymlwasm) use the static list while equivalent matrix jobs use the conditional. Behavior is identical, but normalizing would read more clearly. - The matrix key stays
ubuntu-latestwhile execution is pinned toubuntu-24.04; a one-line comment explaining the indirection would help future maintainers.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Contributor
There was a problem hiding this comment.
Review: migrate Linux CI to self-hosted qiniu runners
This PR moves Linux CI lanes from GitHub-hosted runners to the self-hosted [qiniu, ubuntu-24.04] label, adds explicit Go cache/env configuration in setup-go, and adds cmake + pip --user to the setup actions.
Matrix semantics — verified correct. The runs-on: ${{ matrix.runner || matrix.os }} + include: pattern was checked in every modified workflow (including the tricky cases: goroot.yml with heavy exclude blocks, and doc.yml local_install matching on platform rather than os). In each case the runner key attaches to the intended ubuntu combinations without creating duplicate jobs or leaving jobs stranded on GitHub-hosted runners. No bug here.
Two findings below relate to running on a persistent, public-repo self-hosted fleet; both hinge on how the qiniu runners are provisioned. See inline comments.
Other notes (non-blocking):
setup-demo-deps/action.yml—pip3.12 install --user --break-system-packages numpy torch: on persistent runners--userinstalls into$HOME/.local, which sticks across jobs and can mask a missing-dep regression or cause version drift. Consider pinning versions if reproducibility matters.--useralso makes--break-system-packageslargely redundant (harmless).setup-deps/action.yml— addingcmakecorrectly satisfies thedev-lto-globaldcelane ingo.yml; good catch.setup-go/action.yml— the actiondescription("Set up and verify the requested Go toolchain") no longer reflects that it now also configures Go cache/module paths; minor.
Contributor
There was a problem hiding this comment.
Review: Migrate Ubuntu CI to self-hosted qiniu runners
This PR routes the Ubuntu CI lanes to self-hosted runners via the runs-on: ${{ matrix.runner || matrix.os }} + matrix include pattern, adds Go cache env vars to the setup-go action, and tweaks a couple of dependency installs. The migration pattern is applied consistently and the include merge semantics are correct — each include only adds a runner field to an already-existing os: ubuntu-latest combination without overwriting a base matrix value, so matrix.runner is set on exactly the intended Linux combinations and the || matrix.os fallback covers everything else.
Below are the items worth addressing before merge. See inline comments for line-specific notes.
Self-hosted runners + fork PR triggers (architectural, please confirm)
Most migrated workflows trigger on pull_request: branches: ["**"] (go.yml, llgo.yml, build-cache.yml, targets.yml, fmt.yml, stdlib-coverage.yml, doc.yml, benchmark.yml, release-build.yml's test-artifacts). This means fork PRs now execute untrusted code (go test, demo test.sh scripts, go install ./..., pip install) directly on the persistent qiniu infrastructure. On a public repo this is the scenario GitHub explicitly warns against for self-hosted runners: host compromise, cache/tooling tampering that later trusted (push/tag/release) jobs consume, and lateral movement.
Please confirm the qiniu pool is either (a) ephemeral (single-use VM/container per job), or (b) gated behind maintainer approval for outside collaborators — and/or keep fork-pull_request jobs on GitHub-hosted runners while routing only trusted events to the self-hosted pool. Note release-build.yml's GoReleaser step mounts the host Docker socket, so a host compromised by an earlier fork job could tamper with release inputs.
GOCACHE / module-cache placement (see inline on setup-go)
The two most concrete correctness/perf issues are in .github/actions/setup-go/action.yml — see the inline comment.
Minor / consistency
benchmark.ymlusesos: ubuntu-24.04for its Linux entry while every other file usesos: ubuntu-latest; aligning thematrix.osidentifier avoids confusion (both still resolveruns-onvia therunneroverride).- Job display names that interpolate
${{ matrix.os }}(e.g.go.yml,llgo.yml,goroot.yml,build-cache.yml) will showubuntu-latestin the checks list even though the job now runs on theqiniuubuntu-24.04runner — cosmetic only. setup-depsaddscmakeandsetup-demo-depsswitches topip3.12 install --user: both look reasonable for a leaner/self-hosted image; no concerns.
LLGo baseline benchmarks
Program measurements
Core language and compiler benchmarks
Timer runtime benchmarks
Compared with |
MeteorsLiu
force-pushed
the
qiniu-ci-runner
branch
from
September 5, 2026 02:21
4a2d667 to
456aa29
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.