ci: auto-derive CI_IMAGE_TAG in cidemo-init.sh#1933
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. 🚀 |
|
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:
📝 WalkthroughWalkthrough
ChangesCI image tag management
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Jenkins
participant cidemo-init.sh
participant Git
participant MatrixYAML
Jenkins->>cidemo-init.sh: start CI initialization
cidemo-init.sh->>Git: find latest commit affecting CI files
Git-->>cidemo-init.sh: return commit hash or content-hash fallback
cidemo-init.sh->>MatrixYAML: replace CI_MANAGED with derived tag
MatrixYAML-->>Jenkins: provide patched image configuration
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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/cidemo-init.sh:
- Around line 37-40: Update the YAML patching loop in .ci/cidemo-init.sh to
verify that each sed replacement actually matched the exact CI_IMAGE_TAG
placeholder. Track whether the placeholder exists before or after replacement,
and exit nonzero with a clear error identifying the affected yaml file when it
is missing; retain the existing patched message only after successful
replacement.
🪄 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: 81c79f78-1a31-4656-b8ca-1daddb94e30f
📒 Files selected for processing (7)
.ci/cidemo-init.sh.ci/docs/ci-overview.md.ci/jenkins/lib/build-matrix.yaml.ci/jenkins/lib/test-dl-ep-matrix.yaml.ci/jenkins/lib/test-dl-matrix.yaml.ci/jenkins/lib/test-matrix.yaml.ci/jenkins/lib/test-sanitizer-matrix.yaml
NirWolfer
force-pushed
the
ci-auto-ci-image-tag
branch
from
b387c6e to
0d3623a
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/cidemo-init.sh:
- Around line 25-34: Update the fallback in the CI tag derivation to fail closed
when any file in CI_FILES cannot be read: validate the complete input before
calculating NEW_TAG, and exit nonzero with a clear error instead of hashing
partial or empty content. Preserve the existing sha256sum truncation behavior
when all files are readable.
🪄 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: ee924a77-aa47-43d6-96af-1bdc19487280
📒 Files selected for processing (7)
.ci/cidemo-init.sh.ci/docs/ci-overview.md.ci/jenkins/lib/build-matrix.yaml.ci/jenkins/lib/test-dl-ep-matrix.yaml.ci/jenkins/lib/test-dl-matrix.yaml.ci/jenkins/lib/test-matrix.yaml.ci/jenkins/lib/test-sanitizer-matrix.yaml
NirWolfer
force-pushed
the
ci-auto-ci-image-tag
branch
from
0d3623a to
49f14b4
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/cidemo-init.sh:
- Around line 26-27: Update the NEW_TAG derivation in the CI initialization
script to include the patched matrix YAML files alongside the existing
Dockerfiles and common scripts when determining the latest commit hash. Ensure
changes to YAML-defined BASE_IMAGE or build_args produce a new CI_IMAGE_TAG
rather than reusing an older image tag.
🪄 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: 80043307-8543-4cc9-bdd1-ac85c14e6cc7
📒 Files selected for processing (7)
.ci/cidemo-init.sh.ci/docs/ci-overview.md.ci/jenkins/lib/build-matrix.yaml.ci/jenkins/lib/test-dl-ep-matrix.yaml.ci/jenkins/lib/test-dl-matrix.yaml.ci/jenkins/lib/test-matrix.yaml.ci/jenkins/lib/test-sanitizer-matrix.yaml
|
/build |
|
🤖 CI Triage Agent — TL;DR: The "Run Nixlbench tests" stage was killed (exit 143) after a ~48-minute hang, not a slowness — the UCCL WRITE VRAM→DRAM benchmark printed its results and "Engine destroyed" at 17:42:49 but one nixlbench process never exited, so Full analysisSummary: Jenkins stage Root cause: Wall-clock kill is a symptom. Log timestamps show continuous activity until 17:42:49.585Z ( Implicated commit: Not the PR under test. PR #1933 ("ci: auto-derive CI_IMAGE_TAG in cidemo-init.sh") is CI-config-only and unrelated. The hanging component's most recent functional changes are in the UCCL backend: File: Suggested fix:
Related: UCCL backend PRs #1428, #1271, #1151, #895; test-harness sleep/timeout history #1560, #1081. No existing issue specifically tracking the UCCL teardown hang was found — consider filing one.
|
|
🤖 CI Triage Agent — TL;DR: The "Allocate DL EP Environment" stage failed because a Slurm Full analysisSummary: Stage 226 ( Root cause: GB200 ( Implicated commit: unknown (not code-related; infrastructure/cluster capacity) File: N/A — failure occurs in the CI Slurm allocation step ( Suggested fix: Re-run the build; this is a transient cluster-capacity failure. If it recurs, the CI infra owners should (a) check GB200 partition availability/quota for the Related: none |
|
Amazing, thanks for implementing this! |
NirWolfer
force-pushed
the
ci-auto-ci-image-tag
branch
from
49f14b4 to
db4d199
Compare
|
/build |
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/docs/ci-overview.md:
- Line 169: Add blank lines after the `nixl-ci-build-wheel` and corresponding
heading near the second referenced location, ensuring each heading is separated
from following content to satisfy MD022.
- Around line 30-33: Reconcile the Jenkins job counts in the Jenkins jobs
section and the note around proj-jjb.yaml: change the total to 11, identify 6 PR
CI jobs and 5 standalone jobs, and add cleanup-artifacts to the standalone list.
Ensure both sections use the same totals and job names.
🪄 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: 63901e93-9138-4942-a132-aee067988c54
📒 Files selected for processing (8)
.ci/cidemo-init.sh.ci/docs/ci-overview.md.ci/jenkins/lib/build-matrix.yaml.ci/jenkins/lib/build-wheel-matrix.yaml.ci/jenkins/lib/test-dl-ep-matrix.yaml.ci/jenkins/lib/test-dl-matrix.yaml.ci/jenkins/lib/test-matrix.yaml.ci/jenkins/lib/test-sanitizer-matrix.yaml
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
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/docs/ci-overview.md:
- Line 169: Add blank lines after the `nixl-ci-build-wheel` and corresponding
heading near the second referenced location, ensuring each heading is separated
from following content to satisfy MD022.
- Around line 30-33: Reconcile the Jenkins job counts in the Jenkins jobs
section and the note around proj-jjb.yaml: change the total to 11, identify 6 PR
CI jobs and 5 standalone jobs, and add cleanup-artifacts to the standalone list.
Ensure both sections use the same totals and job names.
🪄 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: 63901e93-9138-4942-a132-aee067988c54
📒 Files selected for processing (8)
.ci/cidemo-init.sh.ci/docs/ci-overview.md.ci/jenkins/lib/build-matrix.yaml.ci/jenkins/lib/build-wheel-matrix.yaml.ci/jenkins/lib/test-dl-ep-matrix.yaml.ci/jenkins/lib/test-dl-matrix.yaml.ci/jenkins/lib/test-matrix.yaml.ci/jenkins/lib/test-sanitizer-matrix.yaml
🛑 Comments failed to post (2)
.ci/docs/ci-overview.md (2)
30-33: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail file=".ci/docs/ci-overview.md" echo "== line count ==" wc -l "$file" echo echo "== relevant excerpts ==" sed -n '1,240p' "$file" | nl -ba | sed -n '1,240p' echo echo "== job-count mentions ==" grep -nE '11 Jenkins jobs|10 jobs|cleanup-artifacts|nixl-ci-dispatcher|build-container|build-wheel-nightly|build-llm-container|test-llm-container' "$file" || true echo echo "== markdown heading spacing candidates ==" python3 - <<'PY' from pathlib import Path p = Path(".ci/docs/ci-overview.md") lines = p.read_text().splitlines() for i, line in enumerate(lines[:-1]): if line.startswith('#') and lines[i+1].strip() and not lines[i+1].startswith(' '): # heading followed by non-blank line print(f"heading@{i+1}: {line!r} -> next line: {lines[i+2]!r}" if i+2 < len(lines) else f"heading@{i+1}: {line!r}") PYRepository: ai-dynamo/nixl
Length of output: 263
🏁 Script executed:
#!/bin/bash set -euo pipefail file=".ci/docs/ci-overview.md" echo "== line count ==" wc -l "$file" echo echo "== relevant excerpts ==" nl -ba "$file" | sed -n '1,240p' echo echo "== job-count mentions ==" grep -nE '11 Jenkins jobs|10 jobs|cleanup-artifacts|nixl-ci-dispatcher|build-container|build-wheel-nightly|build-llm-container|test-llm-container' "$file" || true echo echo "== markdown heading spacing candidates ==" python3 - <<'PY' from pathlib import Path p = Path(".ci/docs/ci-overview.md") lines = p.read_text().splitlines() for i, line in enumerate(lines[:-1]): if line.startswith('#') and lines[i+1].strip() and not lines[i+1].startswith(' '): print(f"heading@{i+1}: {line!r} -> next line@{i+2}: {lines[i+1]!r}") PYRepository: ai-dynamo/nixl
Length of output: 263
🏁 Script executed:
pwdRepository: ai-dynamo/nixl
Length of output: 171
🏁 Script executed:
#!/bin/bash set -euo pipefail file=".ci/docs/ci-overview.md" echo "== line count ==" wc -l "$file" echo echo "== relevant excerpts ==" nl -ba "$file" | sed -n '1,240p' echo echo "== job-count mentions ==" grep -nE '11 Jenkins jobs|10 jobs|cleanup-artifacts|nixl-ci-dispatcher|build-container|build-wheel-nightly|build-llm-container|test-llm-container' "$file" || trueRepository: ai-dynamo/nixl
Length of output: 263
🏁 Script executed:
#!/bin/bash set -euo pipefail file=".ci/docs/ci-overview.md" echo "== 25-40 ==" awk 'NR>=25 && NR<=40 { printf "%4d %s\n", NR, $0 }' "$file" echo echo "== 135-160 ==" awk 'NR>=135 && NR<=160 { printf "%4d %s\n", NR, $0 }' "$file" echo echo "== 165-210 ==" awk 'NR>=165 && NR<=210 { printf "%4d %s\n", NR, $0 }' "$file" echo echo "== job-count mentions ==" grep -nE '11 Jenkins jobs|10 jobs|cleanup-artifacts|nixl-ci-dispatcher|build-container|build-wheel-nightly|build-llm-container|test-llm-container' "$file" || trueRepository: ai-dynamo/nixl
Length of output: 11209
Reconcile the Jenkins job counts and standalone list. The Jenkins jobs section still says
proj-jjb.yamldefines 10 jobs and lists only four standalone jobs, while the note above says 11 and five. Update both sections to use the same totals and includecleanup-artifacts.🤖 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 @.ci/docs/ci-overview.md around lines 30 - 33, Reconcile the Jenkins job counts in the Jenkins jobs section and the note around proj-jjb.yaml: change the total to 11, identify 6 PR CI jobs and 5 standalone jobs, and add cleanup-artifacts to the standalone list. Ensure both sections use the same totals and job names.
169-169: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add blank lines below these headings.
Both headings violate MD022 and may fail the documentation lint gate.
Proposed fix
### `nixl-ci-build-wheel` (dispatcher-triggered) + - **Config:** `.ci/jenkins/lib/build-wheel-matrix.yaml` ... ### `nixl-ci-cleanup-artifacts` (standalone) + - **Trigger:** Daily cron at 6 AM, or manual run with optional `DRY_RUN=true` parameter.Also applies to: 189-189
🧰 Tools
🪛 markdownlint-cli2 (0.23.0)
[warning] 169-169: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below(MD022, blanks-around-headings)
🤖 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 @.ci/docs/ci-overview.md at line 169, Add blank lines after the `nixl-ci-build-wheel` and corresponding heading near the second referenced location, ensuring each heading is separated from following content to satisfy MD022.Source: Linters/SAST tools
NirWolfer
force-pushed
the
ci-auto-ci-image-tag
branch
from
db4d199 to
54b0d4f
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/lib/build-matrix.yaml:
- Line 48: Add the existing “auto-derived/patched by cidemo-init.sh; do not
hand-edit” comment immediately above CI_IMAGE_TAG: "CI_MANAGED" in
.ci/jenkins/lib/build-matrix.yaml:48, .ci/jenkins/lib/test-matrix.yaml:57,
.ci/jenkins/lib/test-dl-matrix.yaml:57, and
.ci/jenkins/lib/test-dl-ep-matrix.yaml:63, matching the wording and formatting
used by the already documented matrix files.
🪄 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: 1ce5d7d5-55db-459c-bff7-f3469bf3e5ef
📒 Files selected for processing (8)
.ci/cidemo-init.sh.ci/docs/ci-overview.md.ci/jenkins/lib/build-matrix.yaml.ci/jenkins/lib/build-wheel-matrix.yaml.ci/jenkins/lib/test-dl-ep-matrix.yaml.ci/jenkins/lib/test-dl-matrix.yaml.ci/jenkins/lib/test-matrix.yaml.ci/jenkins/lib/test-sanitizer-matrix.yaml
Replace the manual CI_IMAGE_TAG bump requirement with automatic
workspace-level derivation. When CI source files (Dockerfiles, build.sh,
common.sh) change, the tag now updates automatically with no developer
action required.
How it works:
- cidemo-init.sh derives CI_IMAGE_TAG as the short git hash of the most
recent commit touching any CI source file, using:
git log -1 --format=%h -- "${CI_FILES[@]}"
- It then patches all five matrix YAML files in the Jenkins workspace via
sed before the matrix library reads them. No commit or push is made.
- The YAML files now carry CI_IMAGE_TAG: "CI_MANAGED" as a placeholder
that signals the value is managed automatically and must not be
hand-edited.
Caching is preserved: identical CI file history produces the same tag
across PRs, so Artifactory base images are reused when nothing changed.
A PR that modifies a Dockerfile or build script gets a new tag and
triggers a rebuild automatically.
Removes the old validation gate that failed CI when developers forgot to
bump the tag manually.
Signed-off-by: NirWolfer <nwolfer@nvidia.com>
NirWolfer
force-pushed
the
ci-auto-ci-image-tag
branch
from
54b0d4f to
34ab903
Compare
|
/build |
4 participants
What?
Replace the manual CI_IMAGE_TAG bump requirement with automatic workspace-level derivation. When CI source files (Dockerfiles, build.sh, common.sh) change, the tag now updates automatically with no developer action required.
How it works:
Caching is preserved: identical CI file history produces the same tag across PRs, so Artifactory base images are reused when nothing changed. A PR that modifies a Dockerfile or build script gets a new tag and triggers a rebuild automatically.
Removes the old validation gate that failed CI when developers forgot to bump the tag manually.
Why?
HPCINFRA-4578
How?
It is optional, but for complex PRs, please provide information about the design,
architecture, approach, etc.
Summary by CodeRabbit
CI_MANAGEDplaceholder that’s replaced at the start of each CI run with a derived tag.