Skip to content

[TRTLLMINF-543][fix] Separate nSpect image registration from launch checks - #20913

Open
niukuo wants to merge 1 commit into
NVIDIA:mainfrom
niukuo:user/yitengn/nspect-registration-launch-checks-20261024
Open

niukuo wants to merge 1 commit into
NVIDIA:mainfrom
niukuo:user/yitengn/nspect-registration-launch-checks-20261024

Conversation

@niukuo

@niukuo niukuo commented Oct 23, 2026 •

Copy link
Copy Markdown
Collaborator

Description

The Register NGC Image For Security Check stage in BuildDockerImages runs image registration, export compliance and OSRB association, and the Launch API wait in a single nspect.py call. A slow container registration therefore fails the whole image build. Pre-merge runs that explicitly select Build-Docker-Images pass register_images=false, but the stage still runs and fails with HTTP 409 on OSRB association because the images are never registered.

This change:

  • pins nspect.py to f1a83d7 from the repository behind TRTLLM_NSPECT_REPO, which also keeps the commits pinned by release branches;
  • skips the stage when register_images is false;
  • registers the four NGC images, associates the export compliance bug, waits up to 180s for container registration, and associates the OSRB ticket, retrying three times and only marking the stage on failure;
  • runs the Launch API scan check only when wait_success_seconds is set (PostMerge), so that check alone decides the build result.

Nightly image builds now register and associate their images without failing on registration latency, and PostMerge keeps its scan gate.

Test Coverage

  • The Jenkins declarative linter and Groovy 2.x parsing pass for jenkins/BuildDockerImage.groovy.
  • The registration, OSRB, and Launch API command lines pass the argument parsing and validation of nspect.py at f1a83d7.
  • f1a83d7 and the commits pinned by release branches can be fetched by SHA with a shallow fetch, as checkoutSource does, and each contains nspect.py.
  • Not yet exercised in a Jenkins image build.

PR Checklist

  • Description explains the problem and resulting behavior.
  • Reviewed applicable coding guidelines.
  • No permanent test files added for this Jenkins-only change.
  • No API, dependency, ownership, documentation, or architecture changes required.

🤖 Generated with Claude Code

Dev Engineer Review

The registration stage now runs only when nspect_id, action == "push", and register_images are set. It checks out nSpect at a pinned commit, with an environment-variable override. Registration includes a 180-second Launch API check and retries up to three times; optional OSRB association runs in the same retry block. Registration failures are caught without failing the build, while the separate Launch API check runs when wait_success_seconds is set. Review finding counts are unavailable.

QA Engineer Review

No test changes.

Per-File QA Perspective

  • jenkins/BuildDockerImage.groovy: Verify the registration-stage conditions, pinned checkout and override, retry behavior, and optional OSRB association. Confirm that registration failures do not fail the build and that a configured wait_success_seconds Launch API check does.

…hecks

Register images and associate the export compliance bug and OSRB ticket
with retries without failing the build, so only the Launch API scan
check decides the build result. Skip the stage when register_images is
false.

Signed-off-by: Yiteng Niu <6831097+niukuo@users.noreply.github.com>
@niukuo
niukuo requested a review from a team as a code owner October 23, 2026 18:00
@niukuo
niukuo requested a review from chzblych October 23, 2026 18:03
@niukuo

niukuo commented Oct 23, 2026

Copy link
Copy Markdown
Collaborator Author

/bot run --stage-list 'Build-Docker-Images'

@coderabbitai

coderabbitai Bot commented Oct 23, 2026

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
📥 Commits

Reviewing files that changed from the base of the PR and between 4ccdca1 and 2179fa3.

📒 Files selected for processing: 1
  • jenkins/BuildDockerImage.groovy

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

Walkthrough

The nSpect registration stage now requires register_images in addition to an nSpect ID and a push action. It uses a newer pinned checkout and separates registration, optional OSRB association, and the optional launch check.

Changes

nSpect registration flow

Layer / File(s) Summary
Registration and launch checks
jenkins/BuildDockerImage.groovy
The stage adds version registration, defaults the release-type ID to 1, and includes optional compliance-bug data. Registration and optional OSRB association retry up to three times; a catchError marks the stage failed without failing the overall build. Registration includes a fixed 180-second launch check. When wait_success_seconds is set, the stage runs a separate launch check using that value. The stage condition now requires register_images, and the pinned nSpect commit changed.

Priority: ➖ Normal

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

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant Jenkins as Jenkins pipeline
  participant nSpect as nSpect registration
  participant OSRB as OSRB association
  participant LaunchAPI as Launch API
  Jenkins->>nSpect: Register image with version option and 180-second launch check
  loop Up to three attempts
    nSpect-->>Jenkins: Registration result
    opt OSRB association configured
      Jenkins->>OSRB: Associate image
    end
  end
  opt wait_success_seconds is set
    Jenkins->>LaunchAPI: Run separate launch check for configured interval
  end
Loading

Possibly related issues

  • NVIDIA/nvcf#2715 — This change separates nSpect registration and OSRB association from the optional launch check, and adds bounded retries for registration.

Possibly related PRs

Merge Risk: ⚪ Minimal · up to 2179f

The supplied evidence does not establish a merge-blocking failure. An image build should still exercise the registration and launch-check paths.

Pre-merge checks: Passed 5
Check name Status Explanation
Title check Passed The title names the main change: separating nSpect image registration from the Launch API launch check in BuildDockerImages. It follows the required format with a JIRA ticket and the [fix] type, and t…
Description check Passed The description uses the repository template's sections (Description, Test Coverage, PR Checklist) and explains the problem, the pin to f1a83d7, the register_images skip, and the separated Launch API …
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.
Full details: Title check

Explanation

The title names the main change: separating nSpect image registration from the Launch API launch check in BuildDockerImages. It follows the required format with a JIRA ticket and the [fix] type, and the raw summary confirms the registration stage now runs separately from the launch-wait check.

Full details: Description check

Explanation

The description uses the repository template's sections (Description, Test Coverage, PR Checklist) and explains the problem, the pin to f1a83d7, the register_images skip, and the separated Launch API check. Test Coverage lists linter and parsing checks and notes that a Jenkins image build has not yet run. The PR Checklist is filled in. The description is mostly complete. Minor gaps: the template's @coderabbitai summary line is absent, and the title-format guidance is not repeated. The Description does not separately state the 'why' beyond the problem statement, but that is already covered.

Full details: Docstring Coverage

Explanation

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 files. (1 skipped: 1 unsupported.)

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #80239 [ run ] triggered by Bot. Commit: 2179fa3 Link to invocation

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #80239 [ run ] completed with state FAILURE. Commit: 2179fa3
/LLM/main/L0_MergeRequest_PR pipeline #66033 (Partly Tested) completed with status: 'FAILURE'

CI Report

⚠️ 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

CI Agent Failure Analysis

Link to invocation

This branch has not been deployed

No deployments
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