Skip to content

ci: route merge-queue Linux jobs to the ECS runner pool - #5853

Closed
wenshao wants to merge 1 commit into
QwenLM:mainfrom
wenshao:ci/route-merge-group-to-ecs
Closed

ci: route merge-queue Linux jobs to the ECS runner pool#5853
wenshao wants to merge 1 commit into
QwenLM:mainfrom
wenshao:ci/route-merge-group-to-ecs

Conversation

@wenshao

@wenshao wenshao commented Jun 25, 2026

Copy link
Copy Markdown
Collaborator

What this PR does

The merge queue (merge_group) skipped the classify_pr job — it was gated to pull_request — so the required Test (ubuntu-latest) gate and the Integration Tests (CLI, No Sandbox) job both fell back to hosted ubuntu-latest. Once the queue went live, every queue entry piled those Linux jobs onto the small hosted pool while the self-hosted ECS runners sat idle. The required gate then starved in queued for 30+ minutes and the queue stalled (no merges for hours).

This extends the existing ECS routing to merge_group:

  • classify_pr now also runs for merge_group and emits the ECS runner label set (["self-hosted", "linux", "x64", "ecs-qwen"]). The merge queue runs in the trusted base-repo context, so this is safe. The required Test (ubuntu-latest) gate picks the runner up automatically through classify_pr's ubuntu_runner output — no change needed in the gate job itself.
  • integration_cli is routed through classify_pr the same way, with hosted/self-hosted Node handling mirroring the Ubuntu gate: ECS has Node 22 pre-installed and can't reach nodejs.org reliably, so on self-hosted it reuses the machine's Node instead of actions/setup-node.

macOS/Windows jobs stay on hosted runners (the ECS pool is Linux-only). The MAINTAINER_ECS_RUNNER_DISABLED repository-variable kill-switch still reverts everything back to hosted.

Why it's needed

After #5842 the queue no longer runs CodeQL/E2E, but the required Linux gate still ran on hosted because merge_group never hit the ECS routing. Under load the hosted Linux pool saturates and the queue's required check sits unscheduled while ECS capacity is idle — exactly the stall this fixes. Routing the queue's Linux jobs to ECS keeps the gate unblocked and leaves the scarce hosted Linux runners free for fork PRs and the macOS/Windows jobs.

Testing

  • actionlint and yamllint pass (node scripts/lint.js --actionlint --yamllint).
  • Reuses the exact ECS routing pattern already proven by the per-PR Test job and the ECS-based PR-review workflow (which calls the model API from ECS), so runner availability, Node provisioning (Node 22 / .nvmrc), and model egress are all known-good on the ECS pool.

Out of scope

The runner label cleanup (16 ecs-qwen-runner-N carry only ecs, not ecs-qwen) and a maximumEntriesToBuild bump are runner-side / merge-queue-settings changes, not workflow code, so they are handled separately.

中文说明

这个 PR 做了什么

合并队列(merge_group)会跳过 classify_pr(它的 if 只认 pull_request),导致必需门禁 Test (ubuntu-latest)Integration Tests (CLI, No Sandbox) 都回落到 hosted ubuntu-latest。队列上线后,每个队列条目都把这些 Linux job 压到很小的 hosted 池,而自托管的 ECS runner 却闲置。必需门禁因此在 queued 里饿了 30+ 分钟,队列停摆(几个小时没有合并)。

本 PR 把已有的 ECS 路由扩展到 merge_group:

  • classify_pr 现在也在 merge_group 下运行,并输出 ECS 标签集(["self-hosted", "linux", "x64", "ecs-qwen"])。合并队列运行在受信任的 base-repo 上下文,所以这是安全的。必需门禁 Test (ubuntu-latest) 通过 classify_prubuntu_runner 输出自动拿到 ECS runner——门禁 job 本身无需改动。
  • integration_cli 以同样方式经由 classify_pr 路由,并加上与 Ubuntu 门禁一致的 hosted/self-hosted Node 处理:ECS 上预装了 Node 22 且无法稳定访问 nodejs.org,因此在自托管上复用机器自带的 Node,而不是 actions/setup-node

macOS/Windows 仍跑在 hosted(ECS 池只有 Linux)。MAINTAINER_ECS_RUNNER_DISABLED 这个仓库变量的总开关依然能把一切切回 hosted。

为什么需要

#5842 之后队列已经不再跑 CodeQL/E2E,但必需的 Linux 门禁仍然跑在 hosted,因为 merge_group 从未命中 ECS 路由。负载一上来,hosted Linux 池就被占满,队列的必需检查排不上号,而 ECS 容量却闲着——正是本 PR 修复的停摆。把队列的 Linux job 路由到 ECS,门禁就不会再被堵,同时把稀缺的 hosted Linux runner 留给 fork PR 和 macOS/Windows job。

测试

  • actionlintyamllint 通过(node scripts/lint.js --actionlint --yamllint)。
  • 复用了 per-PR Test job 以及基于 ECS 的 PR-review 工作流(它在 ECS 上调用模型 API)已经验证过的同一套 ECS 路由模式,因此 runner 可用性、Node 供给(Node 22 / .nvmrc)、模型出网在 ECS 池上都是已知可用的。

不在本 PR 范围内

runner 标签清理(16 台 ecs-qwen-runner-N 只带 ecs、缺 ecs-qwen)与调大 maximumEntriesToBuild 属于 runner 侧 / 合并队列设置的改动,不是工作流代码,单独处理。

The merge queue (`merge_group`) skipped `classify_pr` (it was gated to
`pull_request`), so the required `Test (ubuntu-latest)` gate and the
`Integration Tests` job both fell back to hosted `ubuntu-latest`. With the
queue live, every entry piled those onto the small hosted Linux pool while
the self-hosted ECS runners sat idle — the required gate starved in
`queued` for 30+ minutes and the queue stalled.

Extend the existing ECS routing to `merge_group`:

- `classify_pr` now also runs for `merge_group` and emits the ECS runner
  label set (the queue runs in the base-repo context, so it is trusted).
  The required `Test` gate picks this up automatically via its output.
- `integration_cli` routes through `classify_pr` like the gate, with the
  same hosted/self-hosted Node handling (ECS has Node 22 pre-installed and
  cannot reach nodejs.org reliably).

macOS/Windows stay hosted (ECS is Linux-only). The
`MAINTAINER_ECS_RUNNER_DISABLED` kill-switch still reverts everything to
hosted.

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for the PR, @wenshao! The fix itself is well-described and the problem it solves is clear — merge-queue stalling because classify_pr only ran for pull_request events is a real pain point.

However, the PR body is missing several required sections from the PR template. Please add:

  • ## Reviewer Test Plan — including ### How to verify, ### Evidence (Before & After) (N/A is fine for CI-only changes), and the ### Tested on table.
  • ## Risk & Scope — main risk/tradeoff, out-of-scope items, breaking changes.
  • ## Linked Issues — even if just "N/A" or a reference to the queue-stall incident.

The testing notes you already included are helpful — just move them into the Reviewer Test Plan section and fill in the remaining headings so reviewers have a consistent structure to work from.

中文说明

感谢 PR,@wenshao!修复的问题描述得很清楚 —— 合并队列因为 classify_pr 只在 pull_request 事件下运行而停摆,这确实是个实际痛点。

但 PR 正文缺少 PR 模板 中几个必需的章节,请补充:

  • ## Reviewer Test Plan — 包括 ### How to verify### Evidence (Before & After)(纯 CI 改动可以写 N/A)和 ### Tested on 表格。
  • ## Risk & Scope — 主要风险/取舍、不在范围内的事项、破坏性变更。
  • ## Linked Issues — 哪怕只写 "N/A" 或引用队列停摆的事件。

已有的测试说明很有帮助 —— 只需把它们移到 Reviewer Test Plan 章节里,再补齐其余模板标题,这样审查者就能在统一的结构下工作。

Qwen Code · qwen3.7-max

@wenshao

wenshao commented Jun 25, 2026

Copy link
Copy Markdown
Collaborator Author

Closing in favor of #5854, which covers the same merge_group → ECS routing plus broader merge-queue/Linux CI hardening (checkout verification on the merge queue, etc.). I left the integration_cli routing — the one piece #5854 doesn't yet cover — as a suggestion over there so it isn't lost.

@wenshao wenshao closed this Jun 25, 2026
@wenshao
wenshao deleted the ci/route-merge-group-to-ecs branch June 25, 2026 08:23

@yiliang114 yiliang114 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM — routing the queue's ubuntu + integration onto ECS frees the hosted Linux pool, and the kill-switch + classify_pr fallback are intact. Non-blocking follow-up: this also moves the queue's ubuntu checkout to ECS/squid, so worth extending the verify-checkout guard to merge_group.

yiliang114 added a commit that referenced this pull request Jun 25, 2026
The merge queue runs in the base-repo context but classify_pr was PR-only, so its ubuntu_runner output was empty in the queue and the Ubuntu Test + Integration jobs fell back to the shared hosted pool — piling onto the scarce hosted Linux runners exactly when the queue is busiest. Run classify_pr on merge_group too and route both the Ubuntu gate and Integration onto the same in-repo ECS pool as PRs; the MAINTAINER_ECS_RUNNER_DISABLED kill-switch and the hosted fallback are intact, and fork PRs are unaffected.

Also extend the checkout-verification guard to the merge queue: now that the queue's Ubuntu checkout runs on ECS behind the squid egress proxy, a stale-ref checkout could silently test the wrong tree, and a wrong-tree pass in the queue would merge bad code. One sub-second merge-base check.

Routing approach carried forward from #5853 by @wenshao (closed); this adds the merge-queue checkout guard on top.

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Well-structured PR. The ECS routing logic is correct for all event types (merge_group, same-repo PR, fork PR, kill-switch) and the classify_pr downstream dependency pattern is consistent with the existing jobs. actionlint and yamllint pass.

— qwen3.7-max via Qwen Code /review

Comment thread .github/workflows/ci.yml
echo "::error::Node.js is not on PATH for this self-hosted runner. Provision Node 22.x or set the MAINTAINER_ECS_RUNNER_DISABLED repository variable to 'true' to route the merge queue back to hosted runners."
exit 1
fi
echo "Using pre-installed Node $(node -v) / npm $(npm -v)"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Suggestion] The test job's self-hosted Node step (lines 173-175) includes a major-version guard that this block omits:

if [[ "$(node -p 'process.versions.node.split(".")[0]')" != "22" ]]; then
  echo "::warning::Expected Node 22.x but found $(node -v); integration tests will run against the runner's Node."
fi

Without it, if an ECS runner is reprovisioned with a different Node major version, integration tests silently run against it. The test job would emit a warning but this job would not, making it harder to correlate version-specific integration test failures.

— qwen3.7-max via Qwen Code /review

qqqys pushed a commit to qqqys/qwen-code that referenced this pull request Jun 25, 2026
* ci: route the merge queue's Linux jobs onto ECS

The merge queue runs in the base-repo context but classify_pr was PR-only, so its ubuntu_runner output was empty in the queue and the Ubuntu Test + Integration jobs fell back to the shared hosted pool — piling onto the scarce hosted Linux runners exactly when the queue is busiest. Run classify_pr on merge_group too and route both the Ubuntu gate and Integration onto the same in-repo ECS pool as PRs; the MAINTAINER_ECS_RUNNER_DISABLED kill-switch and the hosted fallback are intact, and fork PRs are unaffected.

Also extend the checkout-verification guard to the merge queue: now that the queue's Ubuntu checkout runs on ECS behind the squid egress proxy, a stale-ref checkout could silently test the wrong tree, and a wrong-tree pass in the queue would merge bad code. One sub-second merge-base check.

Routing approach carried forward from QwenLM#5853 by @wenshao (closed); this adds the merge-queue checkout guard on top.

* ci: fix verify-step wiring test and guard integration_cli on ECS

The merge-queue ECS routing renamed the test job's checkout guard to "Verify
checkout includes expected head commit", but no-ak-integration-ci.test.js still
asserted the old "Verify PR checkout includes head commit" name, failing the
wiring test. Update the assertion.

integration_cli now also routes to ECS (via classify_pr on merge_group) yet
lacked the protections the Ubuntu gate has. Mirror them: fetch-depth 1 (nothing
walks history; a full clone is the heaviest transfer on the ECS runner) and the
same stale-checkout guard, keyed on merge_group.head_sha.

* ci: mirror the Node 22 version probe into integration_cli

The self-hosted Node step claimed to mirror the Ubuntu gate but omitted the
major-version probe, so a non-22 Node on the ECS runner would run integration
tests with no log signal. Add the same warning-only check.
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