[TRTINFRA-7698][infra] - Add SSH key authentication support for SLURM clusters#12172
[TRTINFRA-7698][infra] - Add SSH key authentication support for SLURM clusters#12172mlefeb01 merged 6 commits intoNVIDIA:mainfrom
Conversation
|
/bot run --disable-fail-fast |
📝 WalkthroughWalkthroughThe PR refactors SSH credential handling in the Jenkins test pipeline by introducing centralized credential management through CloudManager. Function signatures are updated to include cluster context and interruption tracking. A new SCP helper function is added, and result gathering is enhanced to include performance metrics and handle interruptions appropriately. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Comment Tip CodeRabbit can generate a title for your PR based on the changes.Add |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@jenkins/L0_Test.groovy`:
- Around line 848-850: The variable stageIsInterrupted is initialized but never
updated, so interrupted-stage upload logic never triggers; update the code to
set stageIsInterrupted = true when the stage is interrupted (e.g., inside the
interruption/exception handler or catch block that handles InterruptedException,
AbortException, or when you detect current build/thread is aborted) before
control proceeds to the upload section where stageIsInterrupted is read (the
variable initialized as stageIsInterrupted and later passed at the upload call
around line where it's used). Ensure the assignment occurs in the same scope as
the existing def stageIsInterrupted and before the upload invocation so the
upload branch for interrupted stages runs correctly.
- Around line 112-119: scpFromRemoteCmd currently builds scp commands without
recursion support; change its signature to def scpFromRemoteCmd(Map remote,
String remotePath, String localPath, boolean recursive=false) and, when
recursive is true, include the "-r " flag in the constructed command for both
key-auth and password-auth branches; then update the perf download caller(s)
that copy directories (the code that finds directories) to call
scpFromRemoteCmd(..., recursive:true) so directory copies succeed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d91571be-8183-4844-bac8-c05e40234f3d
📒 Files selected for processing (1)
jenkins/L0_Test.groovy
mlefeb01
force-pushed
the
slurm-ssh-auth
branch
from
15bfdf0 to
7732bc7
Compare
|
/bot run --disable-fail-fast |
|
PR_Github #38891 [ run ] triggered by Bot. Commit: |
|
PR_Github #38891 [ run ] completed with state
|
|
/bot run --disable-fail-fast |
|
PR_Github #38912 [ run ] triggered by Bot. Commit: |
|
/bot run --disable-fail-fast |
|
PR_Github #38921 [ run ] triggered by Bot. Commit: |
|
PR_Github #38921 [ run ] completed with state
|
mlefeb01
force-pushed
the
slurm-ssh-auth
branch
from
2984fb2 to
d036488
Compare
Signed-off-by: Matt Lefebvre <mlefebvre@nvidia.com>
mlefeb01
force-pushed
the
slurm-ssh-auth
branch
from
d036488 to
4e870c8
Compare
|
/bot run --disable-fail-fast |
|
PR_Github #39120 [ run ] triggered by Bot. Commit: |
|
PR_Github #39120 [ run ] completed with state
|
Signed-off-by: Matt Lefebvre <mlefebvre@nvidia.com>
|
/bot run --disable-fail-fast |
1 similar comment
|
/bot run --disable-fail-fast |
|
PR_Github #39149 [ run ] triggered by Bot. Commit: |
|
PR_Github #39149 [ run ] completed with state
|
Signed-off-by: Matt Lefebvre <mlefebvre@nvidia.com>
|
/bot run --disable-fail-fast |
|
PR_Github #39164 [ run ] triggered by Bot. Commit: |
|
PR_Github #39164 [ run ] completed with state
|
|
/bot run --disable-fail-fast |
|
PR_Github #39283 [ run ] triggered by Bot. Commit: |
|
PR_Github #39283 [ run ] completed with state
|
|
/bot run --disable-fail-fast |
|
PR_Github #39315 [ run ] triggered by Bot. Commit: |
|
PR_Github #39315 [ run ] completed with state |
|
/bot skip --comment "Sufficient testing" |
|
PR_Github #39477 [ skip ] triggered by Bot. Commit: |
|
PR_Github #39477 [ skip ] completed with state |
… clusters (NVIDIA#12172) Signed-off-by: Matt Lefebvre <mlefebvre@nvidia.com>
3 participants
@coderabbitai summary
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)
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.