DeepSpeed
1e90aa9f - [Workflow] run modal GPU workflows only from the merge queue (#8412)

Commit
34 days ago
[Workflow] run modal GPU workflows only from the merge queue (#8412) ## Motivation Both modal workflows spend Modal GPU quota on every PR event (`review_requested` / `ready_for_review` / `synchronize`), while the merge queue entry — the commit that actually gates the merge — never runs them, because the workflows have no `merge_group` trigger. This PR moves the GPU spend to the merge queue only. ## Behavior change | Event | Before | After | |---|---|---| | `pull_request_target` (review_requested / ready_for_review / synchronize) | collect + **Modal run** (when tests are selected) | collect only (no secrets, no Modal) — `deploy` reports *skipped*, which still satisfies Required checks | | `merge_group` (checks_requested) | **nothing runs** | collect + Modal run on the merged tree, gating the merge | | `push` to master / `workflow_dispatch` | collect + Modal run | unchanged | ## Changes - `.github/workflows/modal-torch-latest.yml` - add a `merge_group` (`checks_requested`) trigger — required checks must be reported on queued entries or the merge fails; - the candidate-tree fetch and diff-based selection now also run for `merge_group` (the merge-group commit diffed against `merge_group.base_sha`), so queue runs keep subset selection instead of falling back to the full suite; - `deploy` gains `github.event_name != 'pull_request_target'`; the explicit failure-propagation path (a failing `collect-tests` still fails the check on every event) is preserved. - `.github/workflows/modal-accelerate.yml`: same trigger and the same `deploy` gating. `dorny/paths-filter@v4` (≥ v4.0.1) supports `merge_group` natively, so path filtering is unchanged. - Docs: `TEST_SELECTION.md` (job flow, deploy gating, a `merge_group` trust-context note in the security model) and `CONTRIBUTING.md`. Net effect: one Modal run per queued (batched) merge instead of one per PR event. The repo's merge queue is active (recent merges went through it), so this takes effect immediately once merged. ## Verification - YAML parse + trigger-structure check for both workflows. - Simulated the GitHub-expression fallbacks for all four event contexts: fork head/base SHAs resolve as before; for `merge_group` the candidate resolves to the base-repo merge-group SHA and the queue's base SHA. - Deploy-gating truth table (10 cases): PR events never create a Sandbox (even for `mode=all`); selector failures still fail the check on every event; queue entries run for `all`/`subset` and skip for `none`. - `bash -n` on the modified inline script; `python ci/test_tests_fetcher.py` 16/16; `pre-commit run --files` passes on all four files. ## Notes for reviewers - `merge_group` runs use the workflow/controller from the merged commit with access to secrets — inherent to GitHub's merge queue, documented in the security-model section of `TEST_SELECTION.md`; PRs touching `ci/*` should be reviewed with that in mind. - If the modal check names are added to the required-checks list, queue entries become gated by the modal results (PR-side `skipped` counts as success). No settings change is needed for this PR itself. ## Security hardening (follow-up commit) Follow-up `586a049a4` (from Codex review): on `merge_group` runs, every GitHub-side checkout now resolves to the **trusted base revision** (`github.event.merge_group.base_sha`) instead of the merge-group commit, so queued-PR code never runs next to the Modal/HF tokens: - `modal-torch-latest`: the selector and the controller come from trusted master; the merged candidate is still only ever fetched by exact SHA inside the no-secret Modal Sandbox. - `modal-accelerate`: the `modal run` launcher comes from trusted master; the merged candidate is fetched as validated git data (`ci/torch_latest.py checkout-candidate`) into `$RUNNER_TEMP/deepspeed-candidate`, and `ci/accelerate.py` uploads the tree under test from `DS_CI_CANDIDATE_ROOT` when set. Remotely the env var is unset and the module keeps resolving inside the image at `/root` as before. The residual exposure is a queued PR rewriting the workflow YAML itself — inherent to GitHub's merge queue and documented in `TEST_SELECTION.md`. Bootstrap note: this PR's own queue entry runs the pre-change trusted launcher once; the new launcher/candidate split is live from the next merge on. --------- Signed-off-by: Ma, Guokai <guokai.ma@intel.com> Co-authored-by: Masahiro Tanaka <81312776+tohtana@users.noreply.github.com>
Author
Parents
Loading