[rhoai-2.25] RHAIENG-287: fix(Dockerfiles): wrap remaining RUN && chains in bash heredocs - #2485
[rhoai-2.25] RHAIENG-287: fix(Dockerfiles): wrap remaining RUN && chains in bash heredocs#2485jiridanek wants to merge 3 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (7)
Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review. 📝 WalkthroughWalkthroughThe pull request converts provisioning commands in 15 Dockerfiles to strict Bash heredocs. It preserves package installation, OpenShift client setup, Python dependencies, Jupyter configuration, permissions, and ROCm library installation. ChangesDockerfile build command refactor
Estimated code review effort: 4 (Complex) | ~60 minutes Merge Risk: ⚪ Minimal · up to The PR standardizes Dockerfile shell execution across the remaining workbench images, with linting reported clean and no actionable merge-blocking risk remaining beyond normal checks. Possibly related issues
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
0ffb761 to
dd37667
Compare
CI status [antigravity]Run: Build Notebooks (push) #30261313739 — 2/2 complete · 2 skipped Workflow completed with skipped jobs. |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
jupyter/pytorch/ubi9-python-3.12/Dockerfile.cuda (1)
71-78: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAdd
--failtocurlto actually benefit from strict mode.
curl -L -oexits 0 on HTTP 4xx/5xx and writes the error page to the tarball, soset -enever trips here — the build fails later insidetarwith a misleading message. The same block is duplicated in the sibling CUDA/ROCm/CPU Dockerfiles in this PR; worth fixing consistently.🛡️ Proposed fix
-curl -L https://mirror.openshift.com/pub/openshift-v4/$(uname -m)/clients/ocp/stable/openshift-client-linux.tar.gz \ +curl --fail --location --show-error https://mirror.openshift.com/pub/openshift-v4/$(uname -m)/clients/ocp/stable/openshift-client-linux.tar.gz \ -o /tmp/openshift-client-linux.tar.gz🤖 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 `@jupyter/pytorch/ubi9-python-3.12/Dockerfile.cuda` around lines 71 - 78, Update the curl download command in the Dockerfile heredoc to include the --fail option, ensuring HTTP 4xx/5xx responses cause the command to exit nonzero before tar runs. Apply the same change to the duplicated OpenShift client download blocks in the sibling CUDA, ROCm, and CPU Dockerfiles.
🤖 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 `@jupyter/pytorch`+llmcompressor/ubi9-python-3.12/Dockerfile.cuda:
- Around line 19-26: Update the heredoc delimiter for the MongoDB CLI build
block in the Dockerfile so MONGOCLI_VERSION expands before bash executes, and
apply the same fix to the corresponding block in Dockerfile.konflux.cuda.
Preserve the existing download, extraction, and build commands.
In `@jupyter/pytorch`+llmcompressor/ubi9-python-3.12/Dockerfile.konflux.cuda:
- Around line 19-26: Update the heredoc in the Dockerfile’s mongocli build RUN
block to use an expanding delimiter instead of the quoted <<'EOF', so
${MONGOCLI_VERSION} is resolved in the curl URL, archive filename, and directory
path.
---
Nitpick comments:
In `@jupyter/pytorch/ubi9-python-3.12/Dockerfile.cuda`:
- Around line 71-78: Update the curl download command in the Dockerfile heredoc
to include the --fail option, ensuring HTTP 4xx/5xx responses cause the command
to exit nonzero before tar runs. Apply the same change to the duplicated
OpenShift client download blocks in the sibling CUDA, ROCm, and CPU Dockerfiles.
🪄 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: CHILL
Plan: Enterprise
Run ID: ce048ad0-3145-4063-98c3-ce189744e1e7
📥 Commits
Reviewing files that changed from the base of the PR and between 41f0865 and dd3766715c2b9ffb773126d36a5ac29e89334aae.
📒 Files selected for processing (30)
jupyter/minimal/ubi9-python-3.12/Dockerfile.cpujupyter/minimal/ubi9-python-3.12/Dockerfile.cudajupyter/minimal/ubi9-python-3.12/Dockerfile.konflux.cpujupyter/minimal/ubi9-python-3.12/Dockerfile.konflux.cudajupyter/minimal/ubi9-python-3.12/Dockerfile.konflux.rocmjupyter/minimal/ubi9-python-3.12/Dockerfile.rocmjupyter/pytorch+llmcompressor/ubi9-python-3.12/Dockerfile.cudajupyter/pytorch+llmcompressor/ubi9-python-3.12/Dockerfile.konflux.cudajupyter/pytorch/ubi9-python-3.12/Dockerfile.cudajupyter/pytorch/ubi9-python-3.12/Dockerfile.konflux.cudajupyter/rocm/pytorch/ubi9-python-3.12/Dockerfile.konflux.rocmjupyter/rocm/pytorch/ubi9-python-3.12/Dockerfile.rocmjupyter/rocm/tensorflow/ubi9-python-3.12/Dockerfile.konflux.rocmjupyter/rocm/tensorflow/ubi9-python-3.12/Dockerfile.rocmjupyter/tensorflow/ubi9-python-3.12/Dockerfile.cudajupyter/tensorflow/ubi9-python-3.12/Dockerfile.konflux.cudajupyter/trustyai/ubi9-python-3.12/Dockerfile.cpujupyter/trustyai/ubi9-python-3.12/Dockerfile.konflux.cpuruntimes/minimal/ubi9-python-3.12/Dockerfile.cpuruntimes/minimal/ubi9-python-3.12/Dockerfile.konflux.cpuruntimes/pytorch+llmcompressor/ubi9-python-3.12/Dockerfile.cudaruntimes/pytorch+llmcompressor/ubi9-python-3.12/Dockerfile.konflux.cudaruntimes/pytorch/ubi9-python-3.12/Dockerfile.cudaruntimes/pytorch/ubi9-python-3.12/Dockerfile.konflux.cudaruntimes/rocm-pytorch/ubi9-python-3.12/Dockerfile.konflux.rocmruntimes/rocm-pytorch/ubi9-python-3.12/Dockerfile.rocmruntimes/rocm-tensorflow/ubi9-python-3.12/Dockerfile.konflux.rocmruntimes/rocm-tensorflow/ubi9-python-3.12/Dockerfile.rocmruntimes/tensorflow/ubi9-python-3.12/Dockerfile.cudaruntimes/tensorflow/ubi9-python-3.12/Dockerfile.konflux.cuda
CI status [antigravity]Run: Build Notebooks (push) #30263039119 — 2/2 complete · 2 skipped Workflow completed with skipped jobs. |
CI status [antigravity]Run: Build Notebooks (pr) #30261316332 — 22/22 complete · 15 passed · 3 failed · 4 cancelled Failures so far
Likely root causes
Suggested next steps
|
CI status [antigravity]Run: Build Notebooks (pr) #30263042493 — 22/22 complete · 16 passed · 6 failed Failures so far
Likely root causes
Suggested next steps
|
84ca4f9 to
de943d4
Compare
CI status [antigravity]Run: Build Notebooks (push) #30267564468 — 2/2 complete · 2 skipped Workflow completed with skipped jobs. |
CI status [antigravity]Run: Build Notebooks (pr) #30267567465 — 22/22 complete · 18 passed · 4 failed Failures so far
Likely root causes
Suggested next steps
|
|
/build-konflux |
de943d4 to
8956cf2
Compare
CI status [antigravity]Run: Build Notebooks (push) #30351317338 — 2/2 complete · 2 skipped Workflow completed with skipped jobs. |
|
/build-konflux |
…eredocs Convert workbench Dockerfiles (excluding datascience and rstudio) to RUN /bin/bash <<'EOF' blocks so hadolint accepts source and multiline install steps stay readable. Co-authored-by: Cursor <cursoragent@cursor.com>
Use curl --fail --location --show-error so 4xx/5xx responses abort the heredoc before tar runs with a misleading error page tarball. Co-authored-by: Cursor <cursoragent@cursor.com>
8956cf2 to
1ec7bf9
Compare
CI status [antigravity]Run: Build Notebooks (push) #32340230139 — 2/2 complete · 2 skipped Workflow completed with skipped jobs. |
|
/build-konflux |
CI status [antigravity]Run: Build Notebooks (pr) #32340232896 — 22/22 passed All matrix jobs completed successfully. |
📋 Review SummaryThis pull request systematically refactors remaining single-line 🔍 General Feedback
|
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: ide-developer The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
ide-developer
left a comment
There was a problem hiding this comment.
Code Review (10-angle deep review + empirical verification)
Method: 10 finder angles (line-by-line scan, removed-behavior audit, cross-file tracer, bash/Dockerfile pitfall specialist, wrapping-fidelity check, plus reuse/simplification/efficiency/altitude/conventions), each run independently, followed by empirical verification against real podman build runs and a final gap-sweep.
Overall verdict: the mechanical heredoc conversion itself is faithful — verified line-for-line across all 59 hunks, no dropped/duplicated/reordered commands, no cross-file contamination. Finding 1 below is the only one worth real attention; the rest are minor/optional.
Findings
-
jupyter/pytorch+llmcompressor/ubi9-python-3.12/Dockerfile.konflux.cuda:21(and 5 sibling files: jupyter/pytorch, jupyter/tensorflow, jupyter/rocm/pytorch, jupyter/rocm/tensorflow, jupyter/trustyai) — theoc-clientcurl -Lwas hardened tocurl --fail --location --show-errorin this PR, but themongocli-builderstage'scurl -Lo mongodb-cli-mongocli-v${VERSION}.zip ...in the same diff/heredoc block was left unfixed. Same failure class: a non-2xx response silently writes an HTML error page to the .zip (verified: curl without--failexits 0), and the build fails one step later atunzipwith a confusing "End-of-central-directory signature not found" instead of a clear curl error — undercutting this PR's own stated goal of clearer failure reporting. -
ci/hadolint-config.yaml:25— no lint rule orSHELLdirective stops a future contributor from reintroducing the exact fragileRUN a && b && cpattern this PR removes (DL4006 is disabled; verified a synthetic chain passes hadolint with 0 warnings next to a converted heredoc). Only covers the 32 RUN instructions touched today, not future ones. -
Doubled blank line after
EOFin 5 of the 6 mongocli-builder files (all except jupyter/trustyai) — cosmetic only. -
runtimes/minimal/ubi9-python-3.12/Dockerfile.konflux.cpu:46— keepsdnf clean all && rm -rf /var/cache/yumas one line while ~10 sibling files split the identical pair; the if-body above (line 43) also lost its indentation. Not a functional bug (last line of aset -escript still propagates failure), but reads as an incomplete conversion next to its siblings. -
jupyter/rocm/tensorflow/ubi9-python-3.12/Dockerfile.konflux.rocm:188— wraps an already-terse single&&-pair command in a 4-line heredoc for no benefit (/tmp/link-solibs.sh && rm /tmp/link-solibs.sh). -
jupyter/trustyai/ubi9-python-3.12/Dockerfile.konflux.cpu:209— two--mountflags and the heredoc marker crammed onto one ~141-character line, less readable than the backslash-continuation style used elsewhere in this same PR.
Several plausible-looking theories were tested and refuted rather than reported: ARG values do expand correctly inside quoted <<'EOF' heredocs; RUN --mount=... <<'EOF' builds fine; a trailing A && B as the last line of a set -e script still propagates the failure; scripts/dockerfile_fragments.py's stale curl -L template is dead code for existing files (its ### BEGIN/### END markers match zero Dockerfiles in the repo).
ide-developer
left a comment
There was a problem hiding this comment.
Automated review (max effort — 10 finder angles + empirical verification + gap sweep)
Reviewed the full diff (15 Dockerfile.konflux.* files, RUN a && b && c → bash heredoc conversion + oc-client curl hardening). Several theoretical concerns were empirically tested against podman build and refuted (ARG expansion inside quoted <<'EOF' heredocs works fine; RUN --mount=... <<'EOF' works fine; a trailing A && B as the last line of a set -e script still propagates A's failure correctly). One real consistency gap and a handful of minor cleanup items survived verification.
1. mongocli-builder curl still lacks the --fail hardening (6 files) — most significant finding
This PR's own follow-up commit hardened the oc-client download from curl -L ... to curl --fail --location --show-error ... specifically so HTTP error responses fail the build cleanly instead of being silently written to the output file. The same class of problem exists one section earlier in the same files, in the same new heredoc-conversion diff, and was not given the same fix:
curl -Lo mongodb-cli-mongocli-v${MONGOCLI_VERSION}.zip https://github.com/mongodb/mongodb-cli/archive/refs/tags/mongocli/v${MONGOCLI_VERSION}.zip
e.g. jupyter/pytorch+llmcompressor/ubi9-python-3.12/Dockerfile.konflux.cuda:21, and identically in jupyter/pytorch, jupyter/tensorflow, jupyter/rocm/pytorch, jupyter/rocm/tensorflow, jupyter/trustyai.
Verified locally: if GitHub returns a non-2xx for this URL (rate-limit, moved/deleted tag, transient 5xx), curl -Lo (no --fail) exits 0 and writes the HTML error page into the .zip file. set -e does still abort the build overall — but one step later, at unzip ./mongodb-cli-mongocli-v${MONGOCLI_VERSION}.zip, with a confusing End-of-central-directory signature not found instead of the clear curl HTTP-failure message the sibling fix now provides. Worth adding --fail --location --show-error here too for consistency with the stated PR goal.
2. No guardrail against reintroducing the fixed pattern
ci/hadolint-config.yaml disables DL4006 (the one built-in rule that gestures at "use pipefail/safe RUN semantics"), and hadolint has no other rule that flags a plain RUN a && b && c chain — confirmed by running hadolint against a synthetic chain added next to a converted heredoc: 0 warnings, exit 0. Nothing stops the next contributor from adding a new multi-command RUN in one of these files without the heredoc+set -Eeuxo pipefail wrapper, silently reintroducing the exact fragile pattern this PR removes for the 32 RUN instructions it touches today. Not blocking for this PR, but worth a follow-up (lint rule, or a SHELL ["/bin/bash","-Eeuxo","pipefail","-c"] directive per stage) so the fix doesn't erode over time.
3. Minor cleanup (cosmetic, non-blocking)
- Doubled blank line after
EOF:jupyter/pytorch+llmcompressor/ubi9-python-3.12/Dockerfile.konflux.cuda:26-27(and identically injupyter/pytorch,jupyter/tensorflow,jupyter/rocm/pytorch,jupyter/rocm/tensorflow— all 6 mongocli-builder files exceptjupyter/trustyai) has two consecutive blank lines before the next comment block, where every other converted RUN block in the PR has exactly one. - Inconsistent
&&retention + lost indentation:runtimes/minimal/ubi9-python-3.12/Dockerfile.konflux.cpu:46keepsdnf clean all && rm -rf /var/cache/yumas one line while ~10 sibling files split the identical pair into two; the same file'sifbody at line 43 also lost its original indentation (plus a now-pointless trailing;). Not a functional bug (it's the last line of the script, soset -estill catches a failure there), just the one file that looks like an incomplete conversion next to its siblings. - No-op heredoc wrap:
jupyter/rocm/tensorflow/ubi9-python-3.12/Dockerfile.konflux.rocm:188-191wraps the already-terse/tmp/link-solibs.sh && rm /tmp/link-solibs.shin a 4-line heredoc — no chain was actually unwound, so it's pure ceremony. - Readability:
jupyter/trustyai/ubi9-python-3.12/Dockerfile.konflux.cpu:209combines two--mountflags and the heredoc marker onto one ~141-character line; the backslash-continuation style used elsewhere in this same PR (e.g. the oc-client curl/tar block) would be more readable here too.
Verdict
No build-breaking issues found in the heredoc conversion itself — the mechanical transformation is faithful (verified line-for-line across all 59 hunks: no dropped/duplicated/reordered commands, no cross-file package-list contamination). Item 1 is worth fixing for consistency with this PR's own stated goal; items 2-3 are optional follow-ups/nits.
Posted by an automated review (Claude, max effort: 10 finder angles, empirical podman verification, gap sweep).
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
repo="red-hat-data-services/notebooks"
pr="2485"
echo "== Pull request =="
gh pr view "$pr" --repo "$repo" --json number,title,state,reviewDecision,comments,reviews \
--jq '{number,title,state,reviewDecision,reviews: [.reviews[] | {author: .author.login, state, submittedAt, body}], comments: [.comments[] | {author: .author.login, createdAt, body}]}'
echo
echo "== Inline review comments =="
gh api --paginate "repos/$repo/pulls/$pr/comments?per_page=100" \
--jq '.[] | {id, user: .user.login, created_at, path, line, side, body, in_reply_to_id}'Length of output: 62647 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "== Current PR file list and diff statistics =="
git diff --stat origin/rhoai-2.25...HEAD
git diff --name-only origin/rhoai-2.25...HEAD
echo
echo "== MongoCLI download commands =="
rg -n -C 4 --glob 'Dockerfile*' \
'curl\s+(-Lo|--location|--fail)|MONGOCLI_VERSION|mongodb-cli-mongocli' \
jupyter
echo
echo "== OpenShift-client download commands in changed Dockerfiles =="
git diff --name-only origin/rhoai-2.25...HEAD \
| while IFS= read -r file; do
case "$file" in
*Dockerfile*)
printf '\n===== %s =====\n' "$file"
rg -n -C 2 'openshift-client-linux|curl\s+' "$file" || true
;;
esac
done
echo
echo "== Claimed cleanup locations =="
for file_and_range in \
'runtimes/minimal/ubi9-python-3.12/Dockerfile.konflux.cpu:34,52' \
'jupyter/rocm/tensorflow/ubi9-python-3.12/Dockerfile.konflux.rocm:180,195' \
'jupyter/trustyai/ubi9-python-3.12/Dockerfile.konflux.cpu:200,216'
do
file="${file_and_range%%:*}"
range="${file_and_range#*:}"
printf '\n===== %s (Lines %s) =====\n' "$file" "$range"
nl -ba "$file" | sed -n "${range}p"
done
echo
echo "== Hadolint policy configuration =="
nl -ba ci/hadolint-config.yaml | sed -n '1,120p'Length of output: 1798 🏁 Script executed: #!/bin/bash
set -euo pipefail
repo="red-hat-data-services/notebooks"
pr="2485"
echo "== Pull request base and head =="
gh pr view "$pr" --repo "$repo" --json baseRefOid,headRefOid,baseRefName,headRefName \
--jq '{baseRefName, baseRefOid, headRefName, headRefOid}'
echo
echo "== MongoCLI download commands =="
rg -n -C 4 --glob 'Dockerfile*' \
'curl\s+(-Lo|--location|--fail)|MONGOCLI_VERSION|mongodb-cli-mongocli' \
jupyter
echo
echo "== OpenShift-client download commands in current Dockerfiles =="
rg -n -C 2 --glob 'Dockerfile*' \
'openshift-client-linux|curl\s+(--fail|--location|-L)' \
jupyter runtimes
echo
echo "== Claimed cleanup locations =="
for file_and_range in \
'runtimes/minimal/ubi9-python-3.12/Dockerfile.konflux.cpu:34,52' \
'jupyter/rocm/tensorflow/ubi9-python-3.12/Dockerfile.konflux.rocm:180,195' \
'jupyter/trustyai/ubi9-python-3.12/Dockerfile.konflux.cpu:200,216'
do
file="${file_and_range%%:*}"
range="${file_and_range#*:}"
printf '\n===== %s (Lines %s) =====\n' "$file" "$range"
nl -ba "$file" | sed -n "${range}p"
done
echo
echo "== Hadolint policy configuration =="
nl -ba ci/hadolint-config.yaml | sed -n '1,120p'Length of output: 40915 🏁 Script executed: #!/bin/bash
set -euo pipefail
repo="red-hat-data-services/notebooks"
base="d4ba344eb5e7cee97f71068b6c644b9d462c09cb"
head="1ec7bf924ef74f6560b136302a3c47b352cda383"
echo "== Changed Dockerfiles that contain MongoCLI downloads =="
gh api "repos/$repo/compare/$base...$head" --paginate \
--jq '.files[] | select(.filename | test("^jupyter/.*/Dockerfile\\.konflux\\.")) | .filename' \
| while IFS= read -r file; do
if grep -q 'mongodb-cli-mongocli' "$file"; then
echo "$file"
fi
done
show_range() {
file="$1"
start="$2"
end="$3"
printf '\n===== %s (Lines %s-%s) =====\n' "$file" "$start" "$end"
sed -n "${start},${end}{=;p;}" "$file" | sed 'N;s/\n/: /'
}
show_range 'runtimes/minimal/ubi9-python-3.12/Dockerfile.konflux.cpu' 38 50
show_range 'jupyter/rocm/tensorflow/ubi9-python-3.12/Dockerfile.konflux.rocm' 184 194
show_range 'jupyter/trustyai/ubi9-python-3.12/Dockerfile.konflux.cpu' 204 214
echo
echo "== Hadolint configuration =="
sed -n '1,120{=;p;}' ci/hadolint-config.yaml | sed 'N;s/\n/: /'Length of output: 5998
The six changed I recommend this in all six affected Dockerfiles: curl --fail --location --show-error \
-o mongodb-cli-mongocli-v${MONGOCLI_VERSION}.zip \
https://github.com/mongodb/mongodb-cli/archive/refs/tags/mongocli/v${MONGOCLI_VERSION}.zipThe other items are valid but non-blocking:
The latest Konflux matrix run completed successfully with 22/22 jobs passed. You are interacting with an AI system. |
|
@coderabbitai please take a look at #2485 (review) as well. |
|
@coderabbitai please take a look at #2485 (review) as well. |
|
@coderabbitai please take a look at #2485 (review) as well. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
repo="red-hat-data-services/notebooks"
pr="2485"
review_id="4980585447"
echo "== Pull request review =="
gh api "repos/$repo/pulls/$pr/reviews/$review_id" \
--jq '{id, user: .user.login, state, submitted_at, body, commit_id}'
echo
echo "== Inline comments for this review =="
gh api --paginate "repos/$repo/pulls/$pr/comments?per_page=100" \
--jq --argjson review_id "$review_id" \
'.[] | select(.pull_request_review_id == $review_id) |
{id, path, line, original_line, side, body, diff_hunk, in_reply_to_id, commit_id}'Length of output: 1130 🏁 Script executed: #!/bin/bash
set -euo pipefail
repo="red-hat-data-services/notebooks"
pr="2485"
review_id="4980585447"
gh api --paginate "repos/$repo/pulls/$pr/comments?per_page=100" \
| jq --argjson review_id "$review_id" '
[.[] | select(.pull_request_review_id == $review_id) |
{id, path, line, original_line, side, body, in_reply_to_id, commit_id}]
'Length of output: 499
The review is approved. Its body is You are interacting with an AI system. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
repo="red-hat-data-services/notebooks"
pr="2485"
review_id="4981116166"
echo "== Review =="
gh api "repos/$repo/pulls/$pr/reviews/$review_id" \
--jq '{id, user: .user.login, state, submitted_at, commit_id, body}'
echo
echo "== Inline comments for this review =="
gh api --paginate "repos/$repo/pulls/$pr/comments?per_page=100" \
--jq --argjson review_id "$review_id" \
'.[] | select(.pull_request_review_id == $review_id) |
{id, path, line, original_line, side, original_position, commit_id, body, in_reply_to_id}'Length of output: 6088 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "== MongoCLI download commands and heredoc spacing =="
rg -n -C 5 --glob 'Dockerfile.konflux.*' \
'curl\s+.*mongodb-cli-mongocli|mongodb-cli-mongocli-v\$\{MONGOCLI_VERSION\}\.zip' \
jupyter
echo
echo "== runtimes/minimal cleanup block =="
nl -ba runtimes/minimal/ubi9-python-3.12/Dockerfile.konflux.cpu | sed -n '34,52p'
echo
echo "== ROCm TensorFlow link-solibs block =="
nl -ba jupyter/rocm/tensorflow/ubi9-python-3.12/Dockerfile.konflux.rocm | sed -n '180,198p'
echo
echo "== TrustyAI mount block =="
nl -ba jupyter/trustyai/ubi9-python-3.12/Dockerfile.konflux.cpu | sed -n '200,218p'Length of output: 9769 🏁 Script executed: #!/bin/bash
set -euo pipefail
print_range() {
file="$1"
start="$2"
end="$3"
printf '\n== %s, Lines %s-%s ==\n' "$file" "$start" "$end"
sed -n "${start},${end}p" "$file" | awk -v n="$start" '{printf "%d\t%s\n", n++, $0}'
}
print_range runtimes/minimal/ubi9-python-3.12/Dockerfile.konflux.cpu 34 52
print_range jupyter/rocm/tensorflow/ubi9-python-3.12/Dockerfile.konflux.rocm 180 198
print_range jupyter/trustyai/ubi9-python-3.12/Dockerfile.konflux.cpu 200 218Length of output: 2972
I agree with the main finding. The six I also confirm the cleanup items:
These cleanup items are non-blocking. The guardrail against future You are interacting with an AI system. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
repo="red-hat-data-services/notebooks"
pr="2485"
review_id="4981108041"
echo "== Requested review =="
gh api "repos/$repo/pulls/$pr/reviews/$review_id" \
--jq '{id, user: .user.login, state, submitted_at, body, commit_id}'
echo
echo "== Inline comments for requested review =="
gh api --paginate "repos/$repo/pulls/$pr/comments?per_page=100" \
--jq --argjson review_id "$review_id" \
'.[] | select(.pull_request_review_id == $review_id) |
{id, path, line, original_line, side, body, diff_hunk, commit_id}'
echo
echo "== Current pull request head =="
gh pr view "$pr" --repo "$repo" --json headRefOid --jq '.headRefOid'Length of output: 4539 🏁 Script executed: #!/bin/bash
set -euo pipefail
repo="red-hat-data-services/notebooks"
pr="2485"
review_id="4981108041"
echo "== Inline comments for requested review =="
gh api --paginate "repos/$repo/pulls/$pr/reviews/$review_id/comments?per_page=100" \
--jq '.[] | {id, path, line, original_line, side, body, diff_hunk, commit_id}'
echo
echo "== MongoCLI downloads in the six named Dockerfiles =="
for file in \
jupyter/pytorch+llmcompressor/ubi9-python-3.12/Dockerfile.konflux.cuda \
jupyter/pytorch/ubi9-python-3.12/Dockerfile.konflux.cuda \
jupyter/tensorflow/ubi9-python-3.12/Dockerfile.konflux.cuda \
jupyter/rocm/pytorch/ubi9-python-3.12/Dockerfile.konflux.rocm \
jupyter/rocm/tensorflow/ubi9-python-3.12/Dockerfile.konflux.rocm \
jupyter/trustyai/ubi9-python-3.12/Dockerfile.konflux.cpu
do
printf '\n===== %s =====\n' "$file"
rg -n -C 3 'mongodb-cli-mongocli|curl\s+' "$file"
done
echo
echo "== Remaining findings: current context =="
for item in \
'runtimes/minimal/ubi9-python-3.12/Dockerfile.konflux.cpu:38:50' \
'jupyter/rocm/tensorflow/ubi9-python-3.12/Dockerfile.konflux.rocm:182:196' \
'jupyter/trustyai/ubi9-python-3.12/Dockerfile.konflux.cpu:202:216'
do
file="${item%%:*}"
rest="${item#*:}"
start="${rest%%:*}"
end="${rest#*:}"
printf '\n===== %s, Lines %s-%s =====\n' "$file" "$start" "$end"
nl -ba "$file" | sed -n "${start},${end}p"
done
echo
echo "== Hadolint configuration context =="
nl -ba ci/hadolint-config.yaml | sed -n '1,80p'Length of output: 7317 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "== MongoCLI command changes in this PR =="
git diff --unified=4 origin/rhoai-2.25...HEAD -- \
jupyter/pytorch+llmcompressor/ubi9-python-3.12/Dockerfile.konflux.cuda \
jupyter/pytorch/ubi9-python-3.12/Dockerfile.konflux.cuda \
jupyter/tensorflow/ubi9-python-3.12/Dockerfile.konflux.cuda \
jupyter/rocm/pytorch/ubi9-python-3.12/Dockerfile.konflux.rocm \
jupyter/rocm/tensorflow/ubi9-python-3.12/Dockerfile.konflux.rocm \
jupyter/trustyai/ubi9-python-3.12/Dockerfile.konflux.cpu \
| rg -n -C 3 'mongodb-cli-mongocli|curl'
print_range() {
file="$1"
start="$2"
end="$3"
printf '\n===== %s, Lines %s-%s =====\n' "$file" "$start" "$end"
awk -v start="$start" -v end="$end" \
'NR >= start && NR <= end { printf "%d: %s\n", NR, $0 }' "$file"
}
echo
echo "== Remaining findings: current context =="
print_range runtimes/minimal/ubi9-python-3.12/Dockerfile.konflux.cpu 38 50
print_range jupyter/rocm/tensorflow/ubi9-python-3.12/Dockerfile.konflux.rocm 182 196
print_range jupyter/trustyai/ubi9-python-3.12/Dockerfile.konflux.cpu 202 216
echo
echo "== Hadolint configuration =="
awk '{ printf "%d: %s\n", NR, $0 }' ci/hadolint-config.yaml | sed -n '1,80p'Length of output: 1608 🏁 Script executed: #!/bin/bash
set -euo pipefail
repo="red-hat-data-services/notebooks"
base="d4ba344eb5e7cee97f71068b6c644b9d462c09cb"
head="1ec7bf924ef74f6560b136302a3c47b352cda383"
for file in \
jupyter/pytorch+llmcompressor/ubi9-python-3.12/Dockerfile.konflux.cuda \
jupyter/pytorch/ubi9-python-3.12/Dockerfile.konflux.cuda \
jupyter/tensorflow/ubi9-python-3.12/Dockerfile.konflux.cuda \
jupyter/rocm/pytorch/ubi9-python-3.12/Dockerfile.konflux.rocm \
jupyter/rocm/tensorflow/ubi9-python-3.12/Dockerfile.konflux.rocm \
jupyter/trustyai/ubi9-python-3.12/Dockerfile.konflux.cpu
do
printf '\n===== %s: base and head MongoCLI commands =====\n' "$file"
for ref in "$base" "$head"; do
printf '%s: ' "$ref"
gh api "repos/$repo/contents/$file?ref=$ref" --jq '.content' \
| base64 --decode \
| grep 'mongodb-cli-mongocli' \
| head -n 1 || true
done
done
print_range() {
file="$1"
start="$2"
end="$3"
printf '\n===== %s, Lines %s-%s =====\n' "$file" "$start" "$end"
awk -v start="$start" -v end="$end" \
'NR >= start && NR <= end { printf "%d: %s\n", NR, $0 }' "$file"
}
echo
echo "== Remaining findings: current context =="
print_range runtimes/minimal/ubi9-python-3.12/Dockerfile.konflux.cpu 38 50
print_range jupyter/rocm/tensorflow/ubi9-python-3.12/Dockerfile.konflux.rocm 182 196
print_range jupyter/trustyai/ubi9-python-3.12/Dockerfile.konflux.cpu 202 216
echo
echo "== Hadolint configuration =="
awk '{ printf "%d: %s\n", NR, $0 }' ci/hadolint-config.yaml | sed -n '1,80p'Length of output: 9378
The MongoCLI This behavior existed before this PR. The heredoc conversion did not introduce it. Therefore, I classify it as non-blocking. The proposed curl change is still worthwhile for consistent and clear HTTP failure reporting. The Hadolint finding is also correct. Hadolint does not prevent general The remaining items are optional formatting improvements:
I do not consider the You are interacting with an AI system. |
Apply the same --fail hardening used for the oc-client curl in this PR to the mongocli-builder download, so an HTTP error page isn't silently written to the archive. Also fixes leftover formatting nits from the heredoc conversion: doubled blank line, lost if-block indentation, an unsplit dnf clean line, and a cramped multi --mount RUN line. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
New changes are detected. LGTM label has been removed. |
CI status [antigravity]Run: Build Notebooks (push) #32356123803 — 2/2 complete · 2 skipped Workflow completed with skipped jobs. |
|
/build-konflux |
CI status [antigravity]Run: Build Notebooks (pr) #32356129886 — 22/22 passed All matrix jobs completed successfully. |
Description
Follow-up to #2471. Converts remaining
RUN ... && ...chains in 32 workbench Dockerfiles toRUN /bin/bash <<'EOF'blocks withset -Eeuxo pipefail, matching the pattern already applied to datascience images.Scope:
jupyter/,runtimes/, andcodeserver/workbench images (cpu/cuda/rocm + konflux pairs). Excludesrstudio/(RHAIENG-2430) and datascience (done in #2471).Conventions:
sourcepreserved inside bash heredocs (not.) for gcc-toolset / profile.d scriptsEOFbefore the next Dockerfile instruction (hadolint)bash -cmount blocks expanded to multilineif/fiwhere applicable\on standalone# Fix permissions...commentsHow Has This Been Tested?
hadolint --config ./ci/hadolint-config.yamlon all 52 workbench Dockerfiles (0 errors)Self checklist (all need to be checked):
make test(gmakeon macOS) before asking for reviewDockerfile.konfluxfiles should be done inodh/notebooksand automatically synced torhds/notebooks. For Konflux-specific changes, modifyDockerfile.konfluxfiles directly inrhds/notebooksas these require special attention in the downstream repository and flow to the upcoming RHOAI release.Merge criteria:
Made with Cursor
Summary by CodeRabbit