ci: version the dispatcher pipeline and drop flaky uploadLogs - #1889
Conversation
|
👋 Hi NirWolfer! Thank you for contributing to ai-dynamo/nixl. Your PR reviewers will review your contribution then trigger the CI to test your changes. 🚀 |
7b5996d to
a0d4239
Compare
a0d4239 to
3ee94c7
Compare
|
/build |
|
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:
📝 WalkthroughWalkthroughThis PR adds a dispatcher Jenkinsfile, moves dispatcher execution to SCM-based loading, updates CI docs for the new flow, and removes a log-upload call from the main Jenkinsfile cleanup path. ChangesJenkins dispatcher pipeline changes
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Webhook as GitHub webhook
participant JJB as Jenkins Job Builder
participant Dispatcher as Jenkins dispatcher pipeline
participant GitHub as GitHub commit status API
participant LeafJobs as downstream leaf jobs
Webhook->>JJB: deliver PR payload
JJB->>Dispatcher: check out dispatcher Jenkinsfile with sha1
Dispatcher->>Dispatcher: abort stale dispatcher and leaf builds
Dispatcher->>GitHub: set NIXL CI started = PENDING
Dispatcher->>LeafJobs: run selected leaf jobs in parallel
LeafJobs-->>Dispatcher: collect results
Dispatcher->>GitHub: set NIXL CI ended = SUCCESS or FAILURE
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
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 @.ci/jenkins/pipeline/Jenkinsfile.dispatcher:
- Around line 74-110: The parallel branch definitions in Jenkinsfile.dispatcher
are duplicated and each branch repeatedly calls githubHelper.getMergedSHA(), so
hoist that value once before the parallel block and build the branch map from a
shared job list. Keep the existing branch names and job identifiers (non-gpu,
gpu, dl-gpu, dl-gpu-ep, wheel, sanitizers) but refactor the parallel closure
setup so each build step reuses the single merged SHA and the same githubData
payload.
🪄 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: ASSERTIVE
Plan: Enterprise
Run ID: 26e53c2a-320f-406f-be1a-d697107ff4a6
📒 Files selected for processing (3)
.ci/jenkins/pipeline/Jenkinsfile.ci/jenkins/pipeline/Jenkinsfile.dispatcher.ci/jenkins/pipeline/proj-jjb.yaml
💤 Files with no reviewable changes (1)
- .ci/jenkins/pipeline/Jenkinsfile
3ee94c7 to
3625817
Compare
|
/build |
|
Copy of the new dispatcher job ran at: https://nbuprod.blsm.nvidia.com/nbu-swx-nixl-main/job/Devops/job/nixl-ci-devops-dispatcher-test/6/ |
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 @.ci/jenkins/pipeline/Jenkinsfile.dispatcher:
- Around line 88-94: The dispatcher currently ignores leaf-job failures because
Jenkinsfile.dispatcher uses build with propagate: false, so unsuccessful child
jobs can still lead to a success path. Update the parallel branch handling
around the build call to capture the returned result for each leaf job and
explicitly throw or fail when it is anything other than SUCCESS, using the
existing leafJobs/branchName build flow to locate the change.
In @.ci/jenkins/pipeline/proj-jjb.yaml:
- Around line 62-70: The trusted dispatcher is currently being loaded from the
PR merge ref in the pipeline SCM checkout, which allows untrusted PR changes to
affect the sandbox:false dispatcher path. Update the SCM/refspec handling in the
Jenkins pipeline config so the privileged dispatcher is fetched from a trusted
base branch or fixed ref, while keeping refs/pull/$sha1/merge only for the leaf
jobs. Use the pipeline-scm and dispatcher-loading logic in this YAML to separate
dispatcher checkout from PR merge checkout.
🪄 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: ASSERTIVE
Plan: Enterprise
Run ID: 92b29e96-814f-4470-8a54-a19b32be328e
📒 Files selected for processing (3)
.ci/jenkins/pipeline/Jenkinsfile.ci/jenkins/pipeline/Jenkinsfile.dispatcher.ci/jenkins/pipeline/proj-jjb.yaml
💤 Files with no reviewable changes (1)
- .ci/jenkins/pipeline/Jenkinsfile
3625817 to
4a09308
Compare
|
/build |
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 @.ci/jenkins/pipeline/Jenkinsfile.dispatcher:
- Around line 70-72: The Blue Ocean link in the commit status is built with
env.JOB_BASE_NAME, which strips folder components and breaks foldered jobs.
Update the blueOceanUrl construction in Jenkinsfile.dispatcher to follow the
main Jenkinsfile pattern by using env.JOB_NAME and encoding it when building the
URL, then keep githubHelper.updateCommitStatus using that corrected link.
🪄 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: ASSERTIVE
Plan: Enterprise
Run ID: e7ef877f-ecb3-4f9e-b216-03da2ede31fa
📒 Files selected for processing (3)
.ci/jenkins/pipeline/Jenkinsfile.ci/jenkins/pipeline/Jenkinsfile.dispatcher.ci/jenkins/pipeline/proj-jjb.yaml
💤 Files with no reviewable changes (1)
- .ci/jenkins/pipeline/Jenkinsfile
4a09308 to
e0a17e5
Compare
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 @.ci/jenkins/pipeline/Jenkinsfile.dispatcher:
- Around line 49-59: The stale leaf-job cleanup in Jenkinsfile.dispatcher only
discovers children by scanning the console log, so queued or not-yet-started
jobs can be missed and keep running after the new build starts. Update the leaf
discovery around the staleBuilds loop to also look up queued items from
Jenkins.instance.queue or otherwise track child runs by upstream cause, so both
running and queued descendants are collected. Keep the existing best-effort
try/catch behavior, and ensure the collected leafBuilds includes these
additional child references before aborting them.
🪄 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: ASSERTIVE
Plan: Enterprise
Run ID: 1a8e9ba6-eabd-48e7-9915-d5fc5fe9f553
📒 Files selected for processing (4)
.ci/docs/ci-overview.md.ci/jenkins/pipeline/Jenkinsfile.ci/jenkins/pipeline/Jenkinsfile.dispatcher.ci/jenkins/pipeline/proj-jjb.yaml
💤 Files with no reviewable changes (1)
- .ci/jenkins/pipeline/Jenkinsfile
e0a17e5 to
996aa8d
Compare
996aa8d to
80cb339
Compare
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 @.ci/jenkins/pipeline/Jenkinsfile.dispatcher:
- Around line 27-33: Guard the GitHubHelper initialization in the dispatcher
flow so manual runs do not fail when VARIABLE_FROM_POST is empty. In the
Jenkinsfile.dispatcher setup around GithubHelper.getInstance and
currentPrNumber, skip the GitHub-specific credential/helper path when there is
no payload, or ensure manual dispatches always provide a payload before calling
getPRNumber.
🪄 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: ASSERTIVE
Plan: Enterprise
Run ID: 5a235188-9c3d-4af4-b9e6-20b0a725b14b
📒 Files selected for processing (4)
.ci/docs/ci-overview.md.ci/jenkins/pipeline/Jenkinsfile.ci/jenkins/pipeline/Jenkinsfile.dispatcher.ci/jenkins/pipeline/proj-jjb.yaml
💤 Files with no reviewable changes (1)
- .ci/jenkins/pipeline/Jenkinsfile
80cb339 to
79043cc
Compare
Move the dispatcher logic out of the inline dsl block in proj-jjb.yaml into .ci/jenkins/pipeline/Jenkinsfile.dispatcher, checked out via pipeline-scm. The webhook builds the PR merge ref (refs/pull/<n>/merge) from the PR number via the JSONPath concat function, so a PR into a release branch runs that branch's dispatcher and changes on main no longer disturb it; manual runs may pass any branch or commit in sha1. The dispatcher computes the merged SHA once and generates the parallel fan-out from a job map, restrictable via the LEAF_JOBS parameter to temporarily disable a leaf job without a code change. Failed leaf jobs mark their branch red (catchError) while the dispatcher itself succeeds unless dispatching fails - leaf jobs report their own GitHub statuses. Stale-build aborting now survives the pipeline checkpoint at sleep (no live Run objects or JsonSlurper held across it), fixing silently-lost kill retries. Also remove githubHelper.uploadLogs from the main Jenkinsfile: it sometimes hangs and kills the runner. Signed-off-by: NirWolfer <nwolfer@nvidia.com>
79043cc to
22d5a4c
Compare
|
👀 Investigating |
|
🤖 CI Triage Agent — Now I have enough evidence. Let me analyze the timeline carefully. Timeline analysis:
This is not a wall-clock kill of the whole job — the individual test's Note: The Summary: The Python binding test Root cause: Implicated commit: unknown — the failure is in the UCX backend File: Suggested fix:
Related: none found via issue/PR search ( |
What?
Move the dispatcher logic out of the inline dsl block in
proj-jjb.yamlinto.ci/jenkins/pipeline/Jenkinsfile.dispatcher, checked out via pipeline-scm. The build is triggered by a /build issue-comment webhook, so the dispatcher ischecked out from the PR merge ref (refs/pull//merge, derived from the PR number in the payload). This runs the PR merged into its base branch, so a PR into a release branch runs that branch's dispatcher and changes on main no
longer disturb it.
Also remove githubHelper.uploadLogs from the main Jenkinsfile: it sometimes
hangs and kills the runner.
Why?
CI improvements for release branches
CI stability for Blossom-CI issues
How?
It is optional, but for complex PRs, please provide information about the design,
architecture, approach, etc.
Summary by CodeRabbit
Summary by CodeRabbit
New Features
sha1) via SCM.Bug Fixes
Documentation
LEAF_JOBStemporarily restricts which leaf jobs run.Chores