[TRTLLMINF-218][infra] Gate multi-GPU CI stages behind 'ci: full pre-merge approved' label#16578
Open
ZhanruiSunCh wants to merge 4 commits into
Open
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThe PR adds approval-label validation, gates pre-merge x86_64 and SBSA multi-GPU Jenkins stages, supports configurable single-GPU routing for selected jobs, and updates bot help and CI documentation. ChangesMulti-GPU approval label enforcement
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant GitHub
participant ApprovalWorkflow
participant TeamMembershipAPI
participant Jenkins
participant MultiGPUStages
GitHub->>ApprovalWorkflow: Apply ci: full pre-merge approved
ApprovalWorkflow->>GitHub: Fetch latest label event
ApprovalWorkflow->>TeamMembershipAPI: Verify labeling actor
TeamMembershipAPI-->>ApprovalWorkflow: Return membership status
ApprovalWorkflow->>GitHub: Remove label and comment if unverified
Jenkins->>GitHub: Validate pull-request approval label
GitHub-->>Jenkins: Return label authorization status
Jenkins->>MultiGPUStages: Continue or fail pre-merge multi-GPU stage
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
jenkins/L0_MergeRequest.groovy (1)
1574-1589: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the duplicated multi-GPU label gate into a helper. Both stages carry identical gate logic (label check,
currentBuild.descriptionmarker, anderrormessage) differing only by thex86_64/SBSAarch string. Fold it into one helper (e.g.requireMultiGpuApprovalLabel(pipeline, globalVars, arch)) so the exemption rules, marker HTML, and error text stay in sync.
jenkins/L0_MergeRequest.groovy#L1574-L1589: replace the inline x86_64 gate with a call to the shared helper, passing"x86_64".jenkins/L0_MergeRequest.groovy#L1701-L1716: replace the inline SBSA gate with the same helper, passing"SBSA".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@jenkins/L0_MergeRequest.groovy` around lines 1574 - 1589, Extract the duplicated multi-GPU approval gate into a shared requireMultiGpuApprovalLabel helper that accepts pipeline, globalVars, and arch, preserving the GitHub PR and PostMerge exemptions, label check, description marker, and arch-specific error text. Replace the inline gate at jenkins/L0_MergeRequest.groovy lines 1574-1589 with a helper call using "x86_64", and the gate at lines 1701-1716 with a call using "SBSA".
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/source/developer-guide/ci-overview.md`:
- Line 108: Add the repository-required NVIDIA copyright header at the beginning
of ci-overview.md and set its year to 2026, leaving the existing “Multi-GPU
Tests” documentation unchanged.
- Line 108: Add the Multi-GPU Tests heading to the table of contents using the
`#multi-gpu-tests` anchor, then renumber all subsequent entries to preserve the
document’s section order.
- Around line 119-123: Update the rerun instructions near the multi-GPU command
to preserve the originally selected mode: tell readers to rerun the same command
they used, and include distinct examples for --only-multi-gpu-test and
--add-multi-gpu-test so the broader pipeline is not selected unintentionally.
In `@jenkins/L0_MergeRequest.groovy`:
- Around line 660-668: Update the curl invocation in the pipeline.sh call to
include the --fail option alongside --silent, ensuring HTTP error responses
produce a non-zero exit status and reach the existing fail-open catch handler.
Leave the JSON parsing and normal successful-response behavior unchanged.
---
Nitpick comments:
In `@jenkins/L0_MergeRequest.groovy`:
- Around line 1574-1589: Extract the duplicated multi-GPU approval gate into a
shared requireMultiGpuApprovalLabel helper that accepts pipeline, globalVars,
and arch, preserving the GitHub PR and PostMerge exemptions, label check,
description marker, and arch-specific error text. Replace the inline gate at
jenkins/L0_MergeRequest.groovy lines 1574-1589 with a helper call using
"x86_64", and the gate at lines 1701-1716 with a call using "SBSA".
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 1987f20b-6a09-48aa-a110-cdf448988613
📒 Files selected for processing (4)
.github/workflows/bot-command.yml.github/workflows/full-premerge-approval.ymldocs/source/developer-guide/ci-overview.mdjenkins/L0_MergeRequest.groovy
ZhanruiSunCh
force-pushed
the
user/zhanruis/0716_multi_gpu_label_gate
branch
from
July 22, 2026 09:28
9f82671 to
eb669b5
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/full-premerge-approval.yml:
- Around line 157-222: The cleanup flow around the timeline re-check and
issues.removeLabel must reconcile the latest approval event after deletion to
detect an intervening authorized label addition, restore that verified newer
approval label, and stop without posting a denial comment. Track whether this
run actually removed the unauthorized state, and only create the denial comment
when removal succeeded without a valid newer approval; handle concurrent 404
removals accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 50965d17-a352-4c2b-ab3d-fc2ceeadfadd
📒 Files selected for processing (4)
.github/workflows/bot-command.yml.github/workflows/full-premerge-approval.ymldocs/source/developer-guide/ci-overview.mdjenkins/L0_MergeRequest.groovy
🚧 Files skipped from review as they are similar to previous changes (3)
- jenkins/L0_MergeRequest.groovy
- .github/workflows/bot-command.yml
- docs/source/developer-guide/ci-overview.md
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@jenkins/L0_Test.groovy`:
- Around line 2082-2085: Populate MULTI_GPU_RUN_WITH_SINGLE with the intended
multi-GPU stage name patterns so matching stages can be exempted from
approval-label gating. Preserve the existing relocation logic at
jenkins/L0_Test.groovy lines 5893-5901; no direct change is required there.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: e8189e40-30e3-41f8-8ab4-6092445a46ba
📒 Files selected for processing (2)
jenkins/L0_MergeRequest.groovyjenkins/L0_Test.groovy
…merge approved' label Gate x86_64 and SBSA multi-GPU pre-merge CI stages behind the GitHub label 'ci: full pre-merge approved' so resource-intensive multi-GPU tests only run when explicitly approved by a CI approver. Changes: - L0_MergeRequest.groovy: add hasGithubPRLabel() (fail-open, reuses github-cred-trtllm-ci); insert label check at the top of [Test-x86_64-Multi-GPU] and [Test-SBSA-Multi-GPU] Remote Run stages; write description marker so wrapper can surface call-to-action - full-premerge-approval.yml: new GitHub Actions workflow (modeled after post-merge-approval.yml) to remove the label and post a deny comment when an unauthorized user applies the label - bot-command.yml: document label requirement for --add-multi-gpu-test and --only-multi-gpu-test - ci-overview.md: add Multi-GPU Tests section explaining label gating PostMerge pipelines and GitLab MR builds are exempt from the check. Wrapper (trtllm-github-ci) change tracked in GitLab branch trtllminf-218-full-premerge-label (based on MR NVIDIA#86). Signed-off-by: ZhanruiSunCh <184402041+ZhanruiSunCh@users.noreply.github.com>
- hasGithubPRLabel: add --fail --show-error to curl so HTTP errors (401/403/5xx) exit non-zero and reach the catch/fail-open handler instead of returning empty HTML that blocks CI - Extract duplicated multi-GPU label gate logic into shared requireMultiGpuApprovalLabel() helper - ci-overview.md: add NVIDIA copyright header, add Multi-GPU Tests entry to TOC, clarify rerun instructions for both --add-multi-gpu-test and --only-multi-gpu-test Signed-off-by: ZhanruiSunCh <184402041+ZhanruiSunCh@users.noreply.github.com>
… add exempt stage list - L0_MergeRequest.groovy: replace hasGithubPRLabel() with trtllm_utils.validatePRLabelApproval() from the shared lib, which verifies both label existence and labeler team membership; error message now distinguishes "label not present" from "labeler not authorized"; fail-open on API errors; echo reason on early return - L0_Test.groovy: add MULTI_GPU_RUN_WITH_SINGLE exempt list; matching multi-GPU stages run inside the single-GPU job instead of waiting for the separate multi-GPU dispatch (which requires the label) Signed-off-by: ZhanruiSunCh <184402041+ZhanruiSunCh@users.noreply.github.com>
ZhanruiSunCh
force-pushed
the
user/zhanruis/0716_multi_gpu_label_gate
branch
from
July 24, 2026 08:50
8878d27 to
408f423
Compare
Collaborator
Author
|
/bot run --add-multi-gpu-test |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@jenkins/L0_MergeRequest.groovy`:
- Line 1620: Move the requireMultiGpuApprovalLabel(...) call outside and before
the surrounding try blocks at both jenkins/L0_MergeRequest.groovy lines
1620-1620 and 1737-1737, leaving each approval failure uncaught by the ignore
handlers so denial remains blocking.
In `@jenkins/L0_Test.groovy`:
- Around line 5940-5950: Remove the unconditional test-only bypass block
surrounding the if (true) statement, including its forced multi-GPU marker,
logging, and early return. Preserve the normal singleGpuJobs execution flow in
the surrounding test logic.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 350e4bd0-4cef-42c1-9276-1f4d6bac30e2
📒 Files selected for processing (5)
.github/workflows/bot-command.yml.github/workflows/full-premerge-approval.ymldocs/source/developer-guide/ci-overview.mdjenkins/L0_MergeRequest.groovyjenkins/L0_Test.groovy
🚧 Files skipped from review as they are similar to previous changes (2)
- .github/workflows/bot-command.yml
- docs/source/developer-guide/ci-overview.md
| echo "x86_64 test job is skipped due to Jenkins configuration" | ||
| return | ||
| } | ||
| requireMultiGpuApprovalLabel(pipeline, globalVars, "x86_64") |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Keep approval-gate failures outside the ignore handlers.
requireMultiGpuApprovalLabel() throws on denial, but both surrounding catch (Exception) blocks can downgrade that failure when *_TEST_CHOICE == ignore. This contradicts the intended blocking gate.
jenkins/L0_MergeRequest.groovy#L1620-L1620: callrequireMultiGpuApprovalLabel(...)before thetryblock.jenkins/L0_MergeRequest.groovy#L1737-L1737: callrequireMultiGpuApprovalLabel(...)before thetryblock.
📍 Affects 1 file
jenkins/L0_MergeRequest.groovy#L1620-L1620(this comment)jenkins/L0_MergeRequest.groovy#L1737-L1737
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@jenkins/L0_MergeRequest.groovy` at line 1620, Move the
requireMultiGpuApprovalLabel(...) call outside and before the surrounding try
blocks at both jenkins/L0_MergeRequest.groovy lines 1620-1620 and 1737-1737,
leaving each approval failure uncaught by the ignore handlers so denial remains
blocking.
Comment on lines
+5940
to
+5950
| // [TEST-ONLY] Force multi-GPU marker to test label gate. | ||
| // Remove this block after testing. | ||
| if (true) { | ||
| if (globalVars[ACTION_INFO]['parents'].size() > 0) { | ||
| def parentJob = globalVars[ACTION_INFO]['parents'][-2] | ||
| def archStr = (env.targetArch == X86_64_TRIPLE) ? "x86_64" : (env.targetArch == AARCH64_TRIPLE ? "SBSA" : "Unknown") | ||
| trtllm_utils.appendBuildDescription(this, parentJob['name'], parentJob['build_number'], "====Require ${archStr} Multi-GPU Testing====<br/>") | ||
| echo "[TEST-ONLY] Forced multi-GPU marker for label gate testing. Skipping actual single-GPU tests." | ||
| } | ||
| return | ||
| } |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Remove the unconditional test-only bypass.
if (true) always returns before singleGpuJobs run, so every single-GPU downstream job reports success without executing tests and always requests multi-GPU dispatch. Delete this block before merge.
Proposed fix
- // [TEST-ONLY] Force multi-GPU marker to test label gate.
- // Remove this block after testing.
- if (true) {
- if (globalVars[ACTION_INFO]['parents'].size() > 0) {
- def parentJob = globalVars[ACTION_INFO]['parents'][-2]
- def archStr = (env.targetArch == X86_64_TRIPLE) ? "x86_64" : (env.targetArch == AARCH64_TRIPLE ? "SBSA" : "Unknown")
- trtllm_utils.appendBuildDescription(this, parentJob['name'], parentJob['build_number'], "====Require ${archStr} Multi-GPU Testing====<br/>")
- echo "[TEST-ONLY] Forced multi-GPU marker for label gate testing. Skipping actual single-GPU tests."
- }
- return
- }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // [TEST-ONLY] Force multi-GPU marker to test label gate. | |
| // Remove this block after testing. | |
| if (true) { | |
| if (globalVars[ACTION_INFO]['parents'].size() > 0) { | |
| def parentJob = globalVars[ACTION_INFO]['parents'][-2] | |
| def archStr = (env.targetArch == X86_64_TRIPLE) ? "x86_64" : (env.targetArch == AARCH64_TRIPLE ? "SBSA" : "Unknown") | |
| trtllm_utils.appendBuildDescription(this, parentJob['name'], parentJob['build_number'], "====Require ${archStr} Multi-GPU Testing====<br/>") | |
| echo "[TEST-ONLY] Forced multi-GPU marker for label gate testing. Skipping actual single-GPU tests." | |
| } | |
| return | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@jenkins/L0_Test.groovy` around lines 5940 - 5950, Remove the unconditional
test-only bypass block surrounding the if (true) statement, including its forced
multi-GPU marker, logging, and early return. Preserve the normal singleGpuJobs
execution flow in the surrounding test logic.
Collaborator
|
PR_Github #61546 [ run ] triggered by Bot. Commit: |
Collaborator
|
PR_Github #61546 [ run ] completed with state
|
ZhanruiSunCh
force-pushed
the
user/zhanruis/0716_multi_gpu_label_gate
branch
from
July 24, 2026 09:38
408f423 to
e14a237
Compare
Collaborator
Author
|
/bot run --add-multi-gpu-test |
Collaborator
|
PR_Github #61559 [ run ] triggered by Bot. Commit: |
…abel gate Skip all single-GPU tests and unconditionally write the multi-GPU marker to trigger the label check in L0_MergeRequest. This makes the test cycle fast (no actual tests run) while exercising the full label validation path. Remove this commit after testing. Signed-off-by: ZhanruiSunCh <184402041+ZhanruiSunCh@users.noreply.github.com>
Collaborator
|
PR_Github #61559 [ run ] completed with state
|
ZhanruiSunCh
force-pushed
the
user/zhanruis/0716_multi_gpu_label_gate
branch
from
July 24, 2026 10:26
e14a237 to
e157bcf
Compare
Collaborator
Author
|
/bot run --add-multi-gpu-test |
Collaborator
|
PR_Github #61565 [ run ] triggered by Bot. Commit: |
Collaborator
|
PR_Github #61565 [ run ] completed with state
|
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.
NEED MORE TEST BEFORE MERGE.
Gate x86_64 and SBSA multi-GPU pre-merge CI stages behind the GitHub label 'ci: full pre-merge approved' so resource-intensive multi-GPU tests only run when explicitly approved by a CI approver.
Changes:
PostMerge pipelines and GitLab MR builds are exempt from the check. Wrapper (trtllm-github-ci) change tracked in GitLab branch trtllminf-218-full-premerge-label (based on MR #86).
Summary
This PR gates the x86_64 and SBSA multi-GPU pre-merge CI stages behind the GitHub label
ci: full pre-merge approved. It does this with:NVIDIA/trt-llm-ci-approvers).Key Changes
jenkins/L0_MergeRequest.groovy
trtllm-jenkins-shared-lib) to a PR-specific branch/version.requireMultiGpuApprovalLabel(pipeline, globalVars, String arch)to enforceci: full pre-merge approvedfor relevant multi-GPU pre-merge stages.GITHUB_PR_API_URL),*PostMerge*context, orvalidatePRLabelApproval()reports incomplete validation (checkCompletedis false).currentBuild.descriptionand stops the pipeline with instructions to add the label and re-trigger CI.L0_Test-x86_64-Multi-GPUL0_Test-SBSA-Multi-GPU.github/workflows/full-premerge-approval.yml
pull_request_targetlabel events forci: full pre-merge approved(scoped toNVIDIA/TensorRT-LLM).NVIDIA/trt-llm-ci-approversvia the GitHub Org Team Memberships API..github/workflows/bot-command.yml
/bot runhelp text (helpMessage) for:--only-multi-gpu-test--add-multi-gpu-testci: full pre-merge approved(including the “ask a member of NVIDIA/trt-llm-ci-approvers” guidance).docs/source/developer-guide/ci-overview.md
NVIDIA/trt-llm-ci-approvers,/bot runexamples to re-trigger CI after labeling,jenkins/L0_Test.groovy
MULTI_GPU_RUN_WITH_SINGLE: stage-name patterns used to route certain multi-GPU stages through the single-GPU execution path (by moving matches fromdgxJobstosingleGpuJobsduring the Test phase).env.JOB_NAME ==~ /.*Single-GPU.*/, added a[TEST-ONLY]description marker onto the parent job and an early return path (the multi-GPU marker behavior is forced while that block remains enabled).Dev Engineer Review
Correctness & Consistency
checkCompleted == falsebehavior), reducing the risk of CI lockouts.NVIDIA/trt-llm-ci-approversmember; removal is guarded to avoid unintended deletions.L0_Test.groovychanges introduce a configurable pattern list to ensure some multi-GPU stages can intentionally run via the single-GPU path instead of being blocked by the multi-GPU label gate.Config/Repo Hygiene
if (true)block inL0_Test.groovyto confirm it’s intentional and won’t cause unexpected early returns beyond the intended scenario, and around correct behavior of the “latest approval-label event/actor” selection logic in the GitHub Actions workflow.QA Engineer Review
No test changes.
Description
Test Coverage
PR Checklist
Please review the following before submitting your PR:
PR description clearly explains what and why. If using CodeRabbit's summary, please make sure it makes sense.
PR Follows TRT-LLM CODING GUIDELINES to the best of your knowledge.
Test cases are provided for new code paths (see test instructions)
If PR introduces API changes, an appropriate PR label is added - either
api-compatibleorapi-breaking. Forapi-breaking, includeBREAKINGin the PR title.Any new dependencies have been scanned for license and vulnerabilities
CODEOWNERS updated if ownership changes
Documentation updated as needed
Update tava architecture diagram if there is a significant design change in PR.
The reviewers assigned automatically/manually are appropriate for the PR.
Please check this after reviewing the above items as appropriate for this PR.
GitHub Bot Help
To see a list of available CI bot commands, please comment
/bot help.