Skip to content

[TRTLLMINF-420][fix] Add Jenkins instance name to SLURM job - #19327

Open
lyxxn0414 wants to merge 2 commits into
NVIDIA:mainfrom
lyxxn0414:dev-slurm-job-name-instance
Open

lyxxn0414 wants to merge 2 commits into
NVIDIA:mainfrom
lyxxn0414:dev-slurm-job-name-instance

Conversation

@lyxxn0414

@lyxxn0414 lyxxn0414 commented Sep 17, 2026

Copy link
Copy Markdown

Dev Engineer Review

getJenkinsInstanceName() derives the instance name from JENKINS_URL or Jenkins.instance.rootUrl, then removes trailing slashes. Both SLURM execution paths include this name in their lower-case suffix with BUILD_TAG and a six-character random identifier. No material source risks were identified.

QA Engineer Review

No test changes.

Per-File QA Perspective

  • jenkins/L0_Test.groovy: Verify job, node, workspace, log, and generated script names across all Jenkins instances. Verify behavior when the instance name comes from either supported URL source.

Description

Sub test jobs now run on four downstream Jenkins instances (github-1..4) instead of a single one. This adds the Jenkins instance name to the unique suffix on both SLURM submission paths:

  • runLLMTestlistWithSbatch
  • runLLMTestlistWithAgent

Test Coverage

SLURM job name:
image

SLURM output log file name:
image

The UNSTABLE is just the multi-GPU ci: full pre-merge approved label gate, not a test failure.

PR Checklist

  • Please check this after reviewing the above items as appropriate for this PR.

Sub test jobs now run on four downstream Jenkins instances (github-1..4)
instead of a single one. BUILD_TAG carries only the job name and build
number, and the instances run the same job definitions with independent
build counters, so a SLURM job name no longer identifies the instance it
came from -- while all four share one svc_tensorrt account and scripts
directory on the cluster.

Key the unique suffix on the Jenkins instance name as well, on both
submission paths:

- runLLMTestlistWithSbatch: the suffix flows into jobUID, the job
  workspace and every generated script name. No --job-name is set, so
  sbatch reports the basename of <jobUID>-slurm_launch.sh as the job
  name.
- runLLMTestlistWithAgent: the suffix flows into nodeName, which reaches
  the cluster in the SLURM log path and the bloom/scripts and
  enroot-config directory names.

Signed-off-by: Yue Li (TensorRT) <yuel@nvidia.com>
@lyxxn0414

Copy link
Copy Markdown
Author

/bot run --stage-list "DGX_B200-CPP-1,DGX_H100-2_GPUs-PyTorch-GptOss-1"

@lyxxn0414 lyxxn0414 changed the title [None][infra] Add Jenkins instance name to SLURM job and node names [None][fix] Add Jenkins instance name to SLURM job Sep 17, 2026
@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #74054 [ run ] triggered by Bot. Commit: ff674d3 Link to invocation

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #74054 [ run ] completed with state SUCCESS. Commit: ff674d3
/LLM/main/L0_MergeRequest_PR pipeline #60896 (Partly Tested) completed with status: 'UNSTABLE'

CI Report

⚠️ Multi-GPU Label Required:
Multi-GPU tests require the ci: full pre-merge approved label on this PR. Either:

  • Ask a member of NVIDIA/trt-llm-ci-approvers to add the label, or
  • Wait for the PR to be fully approved — the label is added automatically once approval is complete.
    Then re-trigger CI with the same bot command (no rebase needed).

⚠️ Action Required:

  • Please check the failed tests and fix your PR
  • If you cannot view the failures, ask the CI triggerer to share details
  • Once fixed, request an NVIDIA team member to trigger CI again

Link to invocation

@lyxxn0414
lyxxn0414 marked this pull request as ready for review September 17, 2026 09:46
@lyxxn0414
lyxxn0414 requested a review from a team as a code owner September 17, 2026 09:46
@lyxxn0414 lyxxn0414 changed the title [None][fix] Add Jenkins instance name to SLURM job [TRTLLMINF-420][fix] Add Jenkins instance name to SLURM job Sep 17, 2026
@coderabbitai

coderabbitai Bot commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 6b4124db-aee7-46cc-b595-2885b1fef6d5

📥 Commits

Reviewing files that changed from the base of the PR and between 9d0a788 and 4f287b8.

📒 Files selected for processing (1)
  • jenkins/L0_Test.groovy

Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review.


Walkthrough

The Jenkins pipeline adds a helper that derives the Jenkins instance name from configured URLs. SLURM agent and multi-node job suffixes now include this name before the build tag and random identifier.

Changes

Jenkins-aware SLURM naming

Layer / File(s) Summary
Instance-aware SLURM suffixes
jenkins/L0_Test.groovy
The pipeline derives the Jenkins instance name and includes it in SLURM agent and multi-node job suffixes.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: brnguyen2

Merge Risk: ⚪ Minimal · up to 4f287

The change adds the Jenkins instance token to both relevant SLURM suffixes, and no established failure remains that should block merging.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly identifies the fix and matches the main change: adding the Jenkins instance name to SLURM job identifiers. It also follows the required ticket and type format.
Description check ✅ Passed The description explains the problem, identifies both affected submission paths, and provides test coverage evidence. It includes the required Description, Test Coverage, and PR Checklist sections. So…
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

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.

2 participants