Repository navigation
update(rccl-pr) update coco to 1.4 and prevent draft prs from triggering cluster run ci - #14389
Merged
Merged
Conversation
…ing cluster run ci
github-actions
Bot
added
the
github actions
✅ All Policy Checks Passed
📖 Need help? See the Policy FAQ for details on every check and how to fix failures. |
alex-mckinley
approved these changes
Oct 27, 2026
thomas-huber
approved these changes
Oct 27, 2026
1 task done
Contributor
pr-rocm-systems-14389-oci-rccl-pr-smokeBuilds
FailuresNo failing tests in this run. 15 tests skipped. |
Contributor
pr-rocm-systems-14389-ruby64-rccl-pr-buildBuilds
FailuresNo failing tests in this run. 183 tests skipped. pr-rocm-systems-14389-ruby64-rccl-pr-smokeBuilds
Failures
15 tests skipped. |
Contributor
pr-rocm-systems-14389-oci-rccl-pr-smokeBuilds
FailuresNo failing tests in this run. 15 tests skipped. |
Contributor
pr-rocm-systems-14389-ruby64-rccl-pr-buildBuilds
FailuresNo failing tests in this run. 183 tests skipped. pr-rocm-systems-14389-ruby64-rccl-pr-smokeBuilds
Failures
15 tests skipped. |
Contributor
pr-rocm-systems-14389-ruby64-rccl-pr-smokeBuilds
FailuresNo failing tests in this run. 15 tests skipped. |
alex-breslow-amd
pushed a commit
that referenced
this pull request
Oct 29, 2026
## Motivation The coco PR gate built its develop baseline at develop tip on every run, because each run uses a fresh output directory. A PR was therefore measured against code it did not contain. In 18 replayed ruby64 runs, seven unrelated PRs showed another commit's 1-node reduce_scatter change (#12928) as their own. ## Technical Details `rccl_resolve_coco_run.py` now, for PR runs: - resolves the PR's base branch (`GET /pulls/{n}`) and its merge-base with the head (`GET /compare/{base}...{head}`), using the job's `GITHUB_TOKEN`; - appends `--baseline-sha <merge-base>` to `coco_args`, which already flows into both the build-only and the test `coco pr` calls; - fails the setup job when the merge-base cannot be resolved or equals the PR head, rather than silently comparing the wrong code. API failures name the request and status. Only server errors, rate limits and transport failures (timeouts, resets, truncated responses) are retried. **When the pin applies.** It is passed only when the checked-out coco harness declares `--baseline-sha` (the probe searches `coco/cli/`). A harness without the flag gets a workflow warning and the baseline stays on its configured ref. The new `require_baseline_sha` action input (default `false`) turns that warning into a setup failure. **The PR gate requires the pin.** #14389 moved the default harness to `1.4.x`, which declares `--baseline-sha`, so the second commit passes `require_baseline_sha: 'true'` in `rccl-coco-pr.yml`. A harness that loses the flag now fails setup instead of silently measuring against develop tip. The scheduled workflow never runs in PR mode, so it is unchanged. Rolling `coco_ref` back to `1.3.x` has to turn the input off as well, and a comment in the workflow says so. The wiring test reads the action's `action.yml`, which the RCCL script-test job (`rccl-cluster-runner-allowlist.yml`) did not check out. That workflow's sparse checkout now includes `.github/actions/rccl-resolve-coco-run`, and its `paths` filter includes that directory, so an edit to the action's inputs re-runs the test that guards them. The action also passes the harness location explicitly (`HARNESS_DIR`) instead of relying on the step's working directory. Periodic runs are unchanged. ## Issue Tracking JIRA ID: AICOMRCCL-2204 ## Test Plan - `.github/scripts/tests/rccl_resolve_coco_run_test.py`: 50 tests pass when run directly, with or without an HTTP proxy in the environment. `python3 -m pytest .github/scripts/tests/*rccl*_test.py` gives 157 passed, 2 skipped. They cover: - args with and without a pin; - merge-base resolution against the PR's own base branch, a base branch with reserved characters, an uppercase head, a head that is its own merge-base, and a malformed or incomplete response; - the harness probe on a moved, single-quoted or multi-line declaration, and on a harness that only has `--baseline-stack`; - GitHub failures: a 404 and a malformed URL fail at once with the request named; 5xx, rate limits, timeouts, connection resets and truncated bodies are retried; a missing token; a non-JSON body; - `main()` end to end: missing `HARNESS_DIR`, a periodic run, a required pin on an old harness, and an API failure; - one test against a real loopback socket that resets the connection after the request, through a direct (no-proxy) opener; - the wiring: the action declares `require_baseline_sha` (default off) and the PR workflow passes it, since GitHub only warns about an undeclared input. - Formatted with the repo's black 26.5.1. - Mutation-checked: reverting retry-on-404, the literal-only probe, the lowercasing, the URL quoting, the transient-error clause, the `InvalidURL` clause and a misspelt workflow input each makes at least one test fail. - Against the real API: this PR's merge-base resolves to `c7047ffc473d` (matching `git merge-base`), an uppercase head resolves the same, and a nonexistent PR fails in 0.2 s with the request in the message. ## Test Result All pass. This PR's own gate run is the first end-to-end check: with the `1.4.x` default harness, the `coco pr` command lines should show `--baseline-sha` and the baseline stack's RCCL version line should be the PR's merge-base. Without this change the gate runs the `1.4.x` harness but still builds its baseline at develop tip. ## Submission Checklist - [x] Look over the contributing guidelines at https://github.com/ROCm/rocm-systems/blob/develop/CONTRIBUTING.md. --------- Signed-off-by: speriaswamy-amd <surya.periaswamy@amd.com>
Contributor
TheRock Submodule Bump ActivityNewest first
|
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
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.
Motivation
RCCL coco CI still checks out the harness from
1.3.x, and a draft pull request queues the same Ruby and OCI jobs as a ready one. This moves the default harness ref to1.4.xand holds the cluster queue until the pull request is marked ready for review. The RCCL PR gate stays green while the pull request is a draft, so the required check does not fail for work that has not been requested yet.Technical Details
The harness default changes from
1.3.xto1.4.xin the PR workflow, the scheduled workflow, the reusable run workflow, and the shared resolve action. A caller can still pass another ref.rccl-coco-pr.ymlnow listens forready_for_reviewandconverted_to_draftin addition to the usual pull request events.setupruns only when the pull request is in-repo and not a draft, so the cluster jobs do not start. The RCCL PR gate exits successfully on a draft after change detection has confirmed RCCL was touched. Marking the pull request ready starts the cluster jobs. Turning it back into a draft passes the gate again without another push. A manualworkflow_dispatchis unchanged.Issue Tracking
JIRA ID: AICOMRCCL-3434
Test Plan
1.4.x.coco_refand confirm it resolves1.4.x.Test Result
Not run. The branch is pushed and there is no pull request yet, so the draft, ready, and
1.4.xresolve paths have not been exercised on a runner.Submission Checklist