RHAIENG-2853: Hermetic build for ROCm TensorFlow Jupyter image#3336
Conversation
|
@ysok — This PR is from a fork. Recommended: Push your branch to the main repo for full CI: Then open a new PR from that branch. No push access? A maintainer will cherry-pick and test your changes. See CONTRIBUTING.md for details. |
github-actions
Bot
added
the
review-requested
|
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:
📝 WalkthroughWalkthroughPipelineRun YAMLs enable hermetic prefetching, change build-platform to a larger instance, add a prefetch-dependencies task with explicit compute requests/limits, and add step-level resource overrides for the image build step. Dockerfiles add ARGs ( Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Actionable Issues
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3336 +/- ##
=====================================
Coverage 3.59% 3.59%
=====================================
Files 29 29
Lines 3310 3310
Branches 527 527
=====================================
Hits 119 119
Misses 3189 3189
Partials 2 2
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
jupyter/rocm/tensorflow/ubi9-python-3.12/Dockerfile.rocm (1)
1-35: Hermetic mongocli build stage mirrors Konflux variant.Configuration is consistent. Consider extracting shared stages to reduce duplication between Dockerfile.rocm and Dockerfile.konflux.rocm if divergence is not expected.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@jupyter/rocm/tensorflow/ubi9-python-3.12/Dockerfile.rocm` around lines 1 - 35, The mongocli-builder stage (FROM registry.access.redhat.com/ubi9/go-toolset:1.25.8-1775491036 AS mongocli-builder plus the COPY prefetch-input/mongocli, WORKDIR /tmp/mongocli-src and the hermetic RUN block) is duplicated across Dockerfile.rocm and Dockerfile.konflux.rocm; extract this shared build stage into a single reusable artifact to avoid duplication—either move the mongocli builder stage into a separate included Docker snippet or create a common base image (e.g., a shared image built from the mongocli-builder stage) and replace the inline stage in both Dockerfiles with `FROM <shared-mongocli-builder> AS mongocli-builder` (or an `INCLUDE`/`COPY` of the shared stage), ensuring the hermetic env exports, CGO/GOPROXY settings, and binary output path remain identical.scripts/lockfile-generators/helpers/pylock-to-requirements.py (1)
42-54: Regex may over-match into adjacent content.
\S+will greedily consume characters until whitespace, potentially capturing trailing punctuation or comment markers if the lockfile format varies. Consider using a more restrictive URL pattern or anchoring to expected delimiters.-_DEFAULT_INDEX_RE = re.compile(r"--default-index=(https?://\S+)") +_DEFAULT_INDEX_RE = re.compile(r"--default-index=['\"]?(https?://[^\s'\"]+)['\"]?")This pattern explicitly handles optional quotes and stops at quote/whitespace boundaries.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/lockfile-generators/helpers/pylock-to-requirements.py` around lines 42 - 54, The regex _DEFAULT_INDEX_RE in extract_default_index_from_pylock is too permissive (uses \S+); tighten it to explicitly capture an optional surrounding quote and a URL that stops at whitespace or quote characters (e.g. match optional quote, then https?:// followed by [^\s'"]+, then the matching quote) and update the extraction to return the URL capture group (not the whole match) with trailing quotes already excluded; modify _DEFAULT_INDEX_RE and the return logic in extract_default_index_from_pylock to use that URL group.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@jupyter/rocm/tensorflow/ubi9-python-3.12/Dockerfile.konflux.rocm`:
- Around line 167-172: The pip install invocation in the Dockerfile (the UV_*
environment variables line invoking "uv pip install") is using the insecure flag
"--no-verify-hashes"; remove "--no-verify-hashes" from that command so pip
enforces the --hash checks produced by pylock-to-requirements.py, and if hash
verification then fails investigate and fix the Cachi2 wheel renaming/prefetch
process (or adjust --find-links layout) rather than disabling verification.
In `@jupyter/rocm/tensorflow/ubi9-python-3.12/Dockerfile.rocm`:
- Around line 168-174: The uv pip install invocation includes the dangerous
--no-verify-hashes flag which bypasses hash verification; update the
Dockerfile.rocm UV_NO_CACHE... uv pip install command to remove
--no-verify-hashes and enable pip's hash checking instead (e.g., ensure
requirements.txt contains per-package hashes and pass --require-hashes or simply
omit the bypass flag) so package integrity is verified during the hermetic
install; locate the UV_NO_CACHE=true UV_LINK_MODE=copy
UV_PREVIEW_FEATURES=pylock uv pip install line and change the flags accordingly.
---
Nitpick comments:
In `@jupyter/rocm/tensorflow/ubi9-python-3.12/Dockerfile.rocm`:
- Around line 1-35: The mongocli-builder stage (FROM
registry.access.redhat.com/ubi9/go-toolset:1.25.8-1775491036 AS mongocli-builder
plus the COPY prefetch-input/mongocli, WORKDIR /tmp/mongocli-src and the
hermetic RUN block) is duplicated across Dockerfile.rocm and
Dockerfile.konflux.rocm; extract this shared build stage into a single reusable
artifact to avoid duplication—either move the mongocli builder stage into a
separate included Docker snippet or create a common base image (e.g., a shared
image built from the mongocli-builder stage) and replace the inline stage in
both Dockerfiles with `FROM <shared-mongocli-builder> AS mongocli-builder` (or
an `INCLUDE`/`COPY` of the shared stage), ensuring the hermetic env exports,
CGO/GOPROXY settings, and binary output path remain identical.
In `@scripts/lockfile-generators/helpers/pylock-to-requirements.py`:
- Around line 42-54: The regex _DEFAULT_INDEX_RE in
extract_default_index_from_pylock is too permissive (uses \S+); tighten it to
explicitly capture an optional surrounding quote and a URL that stops at
whitespace or quote characters (e.g. match optional quote, then https?://
followed by [^\s'"]+, then the matching quote) and update the extraction to
return the URL capture group (not the whole match) with trailing quotes already
excluded; modify _DEFAULT_INDEX_RE and the return logic in
extract_default_index_from_pylock to use that URL group.
🪄 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: Repository YAML (base), Central YAML (inherited), Repository UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 20be49e3-5c35-4ee9-9cfd-d955c0eaba0d
📒 Files selected for processing (9)
.tekton/odh-workbench-jupyter-tensorflow-rocm-py312-ubi9-odh-main-pull-request.yaml.tekton/odh-workbench-jupyter-tensorflow-rocm-py312-ubi9-odh-main-push.yamljupyter/rocm/tensorflow/ubi9-python-3.12/Dockerfile.konflux.rocmjupyter/rocm/tensorflow/ubi9-python-3.12/Dockerfile.rocmjupyter/rocm/tensorflow/ubi9-python-3.12/prefetch-inputjupyter/rocm/tensorflow/ubi9-python-3.12/pylock.tomljupyter/rocm/tensorflow/ubi9-python-3.12/pyproject.tomljupyter/rocm/tensorflow/ubi9-python-3.12/requirements.rocm.txtscripts/lockfile-generators/helpers/pylock-to-requirements.py
👮 Files not reviewed due to content moderation or server errors (1)
- jupyter/rocm/tensorflow/ubi9-python-3.12/pylock.toml
ysok
force-pushed
the
odh-RHAIENG-2853-jupyter-rocm-tensorflow
branch
from
93564ae to
aeb1041
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@jupyter/rocm/tensorflow/ubi9-python-3.12/Dockerfile.rocm`:
- Line 126: The COPY instruction currently uses an explicit filename destination
("COPY --from=mongocli-builder /tmp/mongocli /opt/app-root/bin/mongocli");
update it to use the trailing-slash directory convention to match other
Dockerfiles (e.g., replace the destination with "/opt/app-root/bin/") so the
line becomes "COPY --from=mongocli-builder /tmp/mongocli /opt/app-root/bin/"
ensuring consistency with the datascience, pytorch, and rocm variants and
predictable behavior when copying the mongocli binary.
🪄 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: Repository YAML (base), Central YAML (inherited), Repository UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 1f40afed-d8b0-4a1c-8ea5-61e5622fcf94
📒 Files selected for processing (10)
.tekton/odh-workbench-jupyter-tensorflow-rocm-py312-ubi9-odh-main-pull-request.yaml.tekton/odh-workbench-jupyter-tensorflow-rocm-py312-ubi9-odh-main-push.yamljupyter/rocm/tensorflow/ubi9-python-3.12/Dockerfile.konflux.rocmjupyter/rocm/tensorflow/ubi9-python-3.12/Dockerfile.rocmjupyter/rocm/tensorflow/ubi9-python-3.12/prefetch-inputjupyter/rocm/tensorflow/ubi9-python-3.12/pylock.tomljupyter/rocm/tensorflow/ubi9-python-3.12/pyproject.tomljupyter/rocm/tensorflow/ubi9-python-3.12/requirements.rocm.txtscripts/lockfile-generators/create-requirements-lockfile.shscripts/lockfile-generators/helpers/pylock-to-requirements.py
✅ Files skipped from review due to trivial changes (1)
- jupyter/rocm/tensorflow/ubi9-python-3.12/prefetch-input
🚧 Files skipped from review as they are similar to previous changes (3)
- jupyter/rocm/tensorflow/ubi9-python-3.12/pyproject.toml
- .tekton/odh-workbench-jupyter-tensorflow-rocm-py312-ubi9-odh-main-pull-request.yaml
- jupyter/rocm/tensorflow/ubi9-python-3.12/Dockerfile.konflux.rocm
👮 Files not reviewed due to content moderation or server errors (1)
- jupyter/rocm/tensorflow/ubi9-python-3.12/pylock.toml
| # Copy dynamically-linked mongocli built in earlier build stage | ||
| COPY --from=mongocli-builder /tmp/mongocli /opt/app-root/bin/ | ||
| ### END Copy mongocli from builder | ||
| COPY --from=mongocli-builder /tmp/mongocli /opt/app-root/bin/mongocli |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if other Dockerfiles use similar mongocli copy patterns
rg -n 'COPY.*mongocli' --type dockerfileRepository: opendatahub-io/notebooks
Length of output: 102
🏁 Script executed:
#!/bin/bash
# Find the specific Dockerfile and view context around line 126
find . -name "Dockerfile.rocm" -o -name "*Dockerfile*" | grep -E "jupyter/rocm/tensorflow" | head -5Repository: opendatahub-io/notebooks
Length of output: 193
🏁 Script executed:
#!/bin/bash
# Search for COPY patterns with mongocli in any Dockerfile-like files
find . -name "Dockerfile*" -o -name "dockerfile*" | xargs grep -l "mongocli" 2>/dev/nullRepository: opendatahub-io/notebooks
Length of output: 937
🏁 Script executed:
#!/bin/bash
# Search for similar COPY --from patterns with binary destinations
find . \( -name "Dockerfile*" -o -name "dockerfile*" \) | xargs grep -n "COPY --from.*bin/" 2>/dev/null | head -20Repository: opendatahub-io/notebooks
Length of output: 2265
🏁 Script executed:
#!/bin/bash
# View the target Dockerfile around line 126
sed -n '120,135p' ./jupyter/rocm/tensorflow/ubi9-python-3.12/Dockerfile.rocm | cat -nRepository: opendatahub-io/notebooks
Length of output: 772
🏁 Script executed:
#!/bin/bash
# Compare all mongocli COPY patterns in detail
echo "=== Patterns with explicit filename (no trailing slash) ===" && \
grep -n "COPY --from=mongocli-builder.*bin/mongocli" ./jupyter/rocm/tensorflow/ubi9-python-3.12/Dockerfile.rocm ./jupyter/pytorch+llmcompressor/ubi9-python-3.12/Dockerfile.cuda && \
echo "" && \
echo "=== Patterns with directory (trailing slash) ===" && \
grep -n "COPY --from=mongocli-builder.*bin/$" ./jupyter/datascience/ubi9-python-3.12/Dockerfile.cpu ./jupyter/datascience/ubi9-python-3.12/Dockerfile.konflux.cpuRepository: opendatahub-io/notebooks
Length of output: 697
Standardize mongocli destination syntax across Dockerfiles.
The COPY destination uses explicit filename /opt/app-root/bin/mongocli (no trailing slash), but other Dockerfiles in the codebase copy mongocli to /opt/app-root/bin/ (with trailing slash). Adopt one convention consistently—either explicit filename format or trailing slash directory convention—to avoid confusion and maintain predictability. The datascience Dockerfile uses the trailing slash approach; align rocm and pytorch+llmcompressor variants to match.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@jupyter/rocm/tensorflow/ubi9-python-3.12/Dockerfile.rocm` at line 126, The
COPY instruction currently uses an explicit filename destination ("COPY
--from=mongocli-builder /tmp/mongocli /opt/app-root/bin/mongocli"); update it to
use the trailing-slash directory convention to match other Dockerfiles (e.g.,
replace the destination with "/opt/app-root/bin/") so the line becomes "COPY
--from=mongocli-builder /tmp/mongocli /opt/app-root/bin/" ensuring consistency
with the datascience, pytorch, and rocm variants and predictable behavior when
copying the mongocli binary.
aeb1041 to
ae4b775
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
jupyter/rocm/tensorflow/ubi9-python-3.12/Dockerfile.rocm (1)
143-163:⚠️ Potential issue | 🟡 MinorAdd default value for
PYLOCK_FLAVORto prevent build failures from omitted argument.If this ARG is omitted, line 162 resolves to
requirements..txtand the build fails before dependency installation. All callers currently provide this throughbuild-args/rocm.confvia the Makefile, but a safe default protects against direct builds or non-standard invocations.Patch
-ARG PYLOCK_FLAVOR +ARG PYLOCK_FLAVOR=rocm🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@jupyter/rocm/tensorflow/ubi9-python-3.12/Dockerfile.rocm` around lines 143 - 163, The Dockerfile defines ARG PYLOCK_FLAVOR but no default, causing COPY ${TENSORFLOW_SOURCE_CODE}/requirements.${PYLOCK_FLAVOR}.txt to expand to requirements..txt when the build-arg is omitted; set a sensible default by changing the ARG declaration (ARG PYLOCK_FLAVOR=rocm) so COPY references a valid file (referencing ARG PYLOCK_FLAVOR and the COPY line) and prevent build failures for direct/non-standard builds.
🧹 Nitpick comments (1)
scripts/lockfile-generators/helpers/pylock-to-requirements.py (1)
86-99: Minor defensive gap:hashesfield type not validated.If a malformed pylock has
hashesas a non-dict (string, list, None),sdist.get("hashes", {}).items()raisesAttributeError. Unlikely with uv-generated files, but inconsistent with theisinstance(sdist, dict)pattern applied to the parent.Defensive check (optional)
sdist = pkg.get("sdist") - if isinstance(sdist, dict): - for algo, digest in sdist.get("hashes", {}).items(): + if isinstance(sdist, dict) and isinstance(sdist.get("hashes"), dict): + for algo, digest in sdist["hashes"].items(): hashes.append(f"--hash={algo}:{digest}") archive = pkg.get("archive") - if isinstance(archive, dict): - for algo, digest in archive.get("hashes", {}).items(): + if isinstance(archive, dict) and isinstance(archive.get("hashes"), dict): + for algo, digest in archive["hashes"].items(): hashes.append(f"--hash={algo}:{digest}")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/lockfile-generators/helpers/pylock-to-requirements.py` around lines 86 - 99, The loops that assume a "hashes" mapping (for wheel entries, sdist and archive) call .items() on whl.get("hashes", {}), sdist.get("hashes", {}), and archive.get("hashes", {}) without verifying the returned value is a dict; update those loops in the pylock-to-requirements logic so you first retrieve the hashes value (e.g., h = whl.get("hashes")), check isinstance(h, dict) before iterating h.items(), and do the same defensive isinstance(h, dict) check for sdist.get("hashes") and archive.get("hashes") to skip non-dict/malformed values instead of raising AttributeError.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@jupyter/rocm/tensorflow/ubi9-python-3.12/Dockerfile.rocm`:
- Around line 143-163: The Dockerfile defines ARG PYLOCK_FLAVOR but no default,
causing COPY ${TENSORFLOW_SOURCE_CODE}/requirements.${PYLOCK_FLAVOR}.txt to
expand to requirements..txt when the build-arg is omitted; set a sensible
default by changing the ARG declaration (ARG PYLOCK_FLAVOR=rocm) so COPY
references a valid file (referencing ARG PYLOCK_FLAVOR and the COPY line) and
prevent build failures for direct/non-standard builds.
---
Nitpick comments:
In `@scripts/lockfile-generators/helpers/pylock-to-requirements.py`:
- Around line 86-99: The loops that assume a "hashes" mapping (for wheel
entries, sdist and archive) call .items() on whl.get("hashes", {}),
sdist.get("hashes", {}), and archive.get("hashes", {}) without verifying the
returned value is a dict; update those loops in the pylock-to-requirements logic
so you first retrieve the hashes value (e.g., h = whl.get("hashes")), check
isinstance(h, dict) before iterating h.items(), and do the same defensive
isinstance(h, dict) check for sdist.get("hashes") and archive.get("hashes") to
skip non-dict/malformed values instead of raising AttributeError.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited), Repository UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 49f97759-e996-4fc1-94b4-b4e94b6131ea
📒 Files selected for processing (10)
.tekton/odh-workbench-jupyter-tensorflow-rocm-py312-ubi9-odh-main-pull-request.yaml.tekton/odh-workbench-jupyter-tensorflow-rocm-py312-ubi9-odh-main-push.yamljupyter/rocm/tensorflow/ubi9-python-3.12/Dockerfile.konflux.rocmjupyter/rocm/tensorflow/ubi9-python-3.12/Dockerfile.rocmjupyter/rocm/tensorflow/ubi9-python-3.12/prefetch-inputjupyter/rocm/tensorflow/ubi9-python-3.12/pylock.tomljupyter/rocm/tensorflow/ubi9-python-3.12/pyproject.tomljupyter/rocm/tensorflow/ubi9-python-3.12/requirements.rocm.txtscripts/lockfile-generators/create-requirements-lockfile.shscripts/lockfile-generators/helpers/pylock-to-requirements.py
✅ Files skipped from review due to trivial changes (2)
- jupyter/rocm/tensorflow/ubi9-python-3.12/pyproject.toml
- jupyter/rocm/tensorflow/ubi9-python-3.12/prefetch-input
🚧 Files skipped from review as they are similar to previous changes (2)
- .tekton/odh-workbench-jupyter-tensorflow-rocm-py312-ubi9-odh-main-push.yaml
- jupyter/rocm/tensorflow/ubi9-python-3.12/Dockerfile.konflux.rocm
👮 Files not reviewed due to content moderation or server errors (1)
- jupyter/rocm/tensorflow/ubi9-python-3.12/pylock.toml
ysok
force-pushed
the
odh-RHAIENG-2853-jupyter-rocm-tensorflow
branch
from
ae4b775 to
6ce1152
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (5)
jupyter/rocm/tensorflow/ubi9-python-3.12/Dockerfile.rocm (2)
1-2: UnusedARG TARGETARCHdeclaration.Same issue as Dockerfile.konflux.rocm - declared but not referenced.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@jupyter/rocm/tensorflow/ubi9-python-3.12/Dockerfile.rocm` around lines 1 - 2, Remove the unused Docker build argument declaration ARG TARGETARCH from the Dockerfile.rocm to match other files; locate the top-level ARG TARGETARCH line in Dockerfile.rocm and delete it (or reference it only if you later add logic that uses TARGETARCH), ensuring there are no remaining references to TARGETARCH in this file.
79-87: Redundant GPG key imports across build stages.Both
rocm-base(lines 47-50) androcm-jupyter-minimal(lines 79-83) import the same GPG keys. Sincerocm-jupyter-minimalinherits fromrocm-base, the keys are already imported. This adds ~3 extra layers.Proposed fix - remove redundant imports in child stage
-# [HERMETIC] Import GPG keys for prefetched RPM verification. -# CentOS key imported only if prefetched (present in Dockerfile.cpu; may be absent in Konflux). -RUN rpm --import /cachi2/output/deps/generic/RPM-GPG-KEY-CentOS-Official || true -RUN rpm --import /cachi2/output/deps/generic/RPM-GPG-KEY-EPEL-9 || true -RUN rpm --import /etc/pki/rpm-gpg/RPM-GPG-KEY-redhat-release +# GPG keys already imported in rocm-base stage +# EPEL-9 key needed only if EPEL packages are installed in this stage +RUN rpm --import /cachi2/output/deps/generic/RPM-GPG-KEY-EPEL-9 || true🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@jupyter/rocm/tensorflow/ubi9-python-3.12/Dockerfile.rocm` around lines 79 - 87, The rocm-jupyter-minimal stage duplicates GPG key imports already performed in the rocm-base stage; remove the three redundant RUN rpm --import lines in the rocm-jupyter-minimal stage (the ones importing RPM-GPG-KEY-CentOS-Official, RPM-GPG-KEY-EPEL-9 and RPM-GPG-KEY-redhat-release) so the keys remain imported only in rocm-base, leaving the subsequent COPY prefetch-input/... lines intact; ensure no other child stages re-import those same keys.jupyter/rocm/tensorflow/ubi9-python-3.12/Dockerfile.konflux.rocm (2)
1-2: UnusedARG TARGETARCHdeclaration.
TARGETARCHis declared but not referenced anywhere in the Dockerfile. If it's intended for future use or multi-arch builds, add a comment. Otherwise, remove it to avoid confusion.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@jupyter/rocm/tensorflow/ubi9-python-3.12/Dockerfile.konflux.rocm` around lines 1 - 2, The Dockerfile declares ARG TARGETARCH but never uses it; either remove the unused ARG TARGETARCH declaration or document its intended purpose with an inline comment (e.g., for future multi-arch builds) so it's not confusing—locate the ARG TARGETARCH line in the Dockerfile.konflux.rocm and either delete that ARG or replace it with a short comment explaining why TARGETARCH is kept for future use.
47-50: GPG key import failures silently ignored may mask prefetch issues.Using
|| trueon GPG key imports means a missing or corrupted key file won't fail the build. While the CentOS key may legitimately be absent, failing silently could allow RPM installations without proper signature verification.Consider logging when keys are missing:
Proposed fix
-RUN rpm --import /cachi2/output/deps/generic/RPM-GPG-KEY-CentOS-Official || true +RUN rpm --import /cachi2/output/deps/generic/RPM-GPG-KEY-CentOS-Official || echo "WARN: CentOS GPG key not found, skipping"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@jupyter/rocm/tensorflow/ubi9-python-3.12/Dockerfile.konflux.rocm` around lines 47 - 50, The CentOS GPG import currently uses a silent ignore ("RUN rpm --import /cachi2/output/deps/generic/RPM-GPG-KEY-CentOS-Official || true") which can hide missing/corrupt keys; change this to explicitly check for the file before importing and log a clear warning if it's absent, e.g. test for the presence of /cachi2/output/deps/generic/RPM-GPG-KEY-CentOS-Official and only run rpm --import if present, printing a descriptive message when it isn't, while keeping the redhat import (rpm --import /etc/pki/rpm-gpg/RPM-GPG-KEY-redhat-release) as a regular import that fails on error so unexpected issues are surfaced.scripts/lockfile-generators/helpers/pylock-to-requirements.py (1)
50-57: Edge case: regex may capture trailing characters from multiline comments.The regex
_DEFAULT_INDEX_REuses\S+which will greedily match non-whitespace. If the pylock comment contains additional flags after the URL on the same line (e.g.,--default-index=https://pypi.org/simple --some-other-flag), the URL would incorrectly include--some-other-flag.Consider tightening the pattern or splitting on whitespace:
Proposed fix
-_DEFAULT_INDEX_RE = re.compile(r"--default-index=(https?://\S+)") +_DEFAULT_INDEX_RE = re.compile(r"--default-index=(https?://[^\s'\"]+)")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/lockfile-generators/helpers/pylock-to-requirements.py` around lines 50 - 57, The regex in extract_default_index_from_pylock (using _DEFAULT_INDEX_RE with \S+) can capture trailing flags on the same line; update the extraction to only return the URL token by splitting the captured group on whitespace and taking the first token (then strip surrounding quotes), or tighten _DEFAULT_INDEX_RE to capture only URL characters (e.g., use a character class like [^\s'"]+). Apply this change in extract_default_index_from_pylock so m.group(1) is replaced with the cleaned first token before rstrip("'\"").
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@jupyter/rocm/tensorflow/ubi9-python-3.12/Dockerfile.konflux.rocm`:
- Around line 1-2: The Dockerfile declares ARG TARGETARCH but never uses it;
either remove the unused ARG TARGETARCH declaration or document its intended
purpose with an inline comment (e.g., for future multi-arch builds) so it's not
confusing—locate the ARG TARGETARCH line in the Dockerfile.konflux.rocm and
either delete that ARG or replace it with a short comment explaining why
TARGETARCH is kept for future use.
- Around line 47-50: The CentOS GPG import currently uses a silent ignore ("RUN
rpm --import /cachi2/output/deps/generic/RPM-GPG-KEY-CentOS-Official || true")
which can hide missing/corrupt keys; change this to explicitly check for the
file before importing and log a clear warning if it's absent, e.g. test for the
presence of /cachi2/output/deps/generic/RPM-GPG-KEY-CentOS-Official and only run
rpm --import if present, printing a descriptive message when it isn't, while
keeping the redhat import (rpm --import
/etc/pki/rpm-gpg/RPM-GPG-KEY-redhat-release) as a regular import that fails on
error so unexpected issues are surfaced.
In `@jupyter/rocm/tensorflow/ubi9-python-3.12/Dockerfile.rocm`:
- Around line 1-2: Remove the unused Docker build argument declaration ARG
TARGETARCH from the Dockerfile.rocm to match other files; locate the top-level
ARG TARGETARCH line in Dockerfile.rocm and delete it (or reference it only if
you later add logic that uses TARGETARCH), ensuring there are no remaining
references to TARGETARCH in this file.
- Around line 79-87: The rocm-jupyter-minimal stage duplicates GPG key imports
already performed in the rocm-base stage; remove the three redundant RUN rpm
--import lines in the rocm-jupyter-minimal stage (the ones importing
RPM-GPG-KEY-CentOS-Official, RPM-GPG-KEY-EPEL-9 and RPM-GPG-KEY-redhat-release)
so the keys remain imported only in rocm-base, leaving the subsequent COPY
prefetch-input/... lines intact; ensure no other child stages re-import those
same keys.
In `@scripts/lockfile-generators/helpers/pylock-to-requirements.py`:
- Around line 50-57: The regex in extract_default_index_from_pylock (using
_DEFAULT_INDEX_RE with \S+) can capture trailing flags on the same line; update
the extraction to only return the URL token by splitting the captured group on
whitespace and taking the first token (then strip surrounding quotes), or
tighten _DEFAULT_INDEX_RE to capture only URL characters (e.g., use a character
class like [^\s'"]+). Apply this change in extract_default_index_from_pylock so
m.group(1) is replaced with the cleaned first token before rstrip("'\"").
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited), Repository UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 540fe29f-2e0a-401e-a604-a9173f579bdb
📒 Files selected for processing (10)
.tekton/odh-workbench-jupyter-tensorflow-rocm-py312-ubi9-odh-main-pull-request.yaml.tekton/odh-workbench-jupyter-tensorflow-rocm-py312-ubi9-odh-main-push.yamljupyter/rocm/tensorflow/ubi9-python-3.12/Dockerfile.konflux.rocmjupyter/rocm/tensorflow/ubi9-python-3.12/Dockerfile.rocmjupyter/rocm/tensorflow/ubi9-python-3.12/prefetch-inputjupyter/rocm/tensorflow/ubi9-python-3.12/pylock.tomljupyter/rocm/tensorflow/ubi9-python-3.12/pyproject.tomljupyter/rocm/tensorflow/ubi9-python-3.12/requirements.rocm.txtscripts/lockfile-generators/create-requirements-lockfile.shscripts/lockfile-generators/helpers/pylock-to-requirements.py
✅ Files skipped from review due to trivial changes (2)
- jupyter/rocm/tensorflow/ubi9-python-3.12/pyproject.toml
- jupyter/rocm/tensorflow/ubi9-python-3.12/prefetch-input
🚧 Files skipped from review as they are similar to previous changes (1)
- scripts/lockfile-generators/create-requirements-lockfile.sh
👮 Files not reviewed due to content moderation or server errors (1)
- jupyter/rocm/tensorflow/ubi9-python-3.12/pylock.toml
ysok
force-pushed
the
odh-RHAIENG-2853-jupyter-rocm-tensorflow
branch
from
208b0bb to
131cf21
Compare
ysok
force-pushed
the
odh-RHAIENG-2853-jupyter-rocm-tensorflow
branch
from
131cf21 to
fd9a301
Compare
ysok
force-pushed
the
odh-RHAIENG-2853-jupyter-rocm-tensorflow
branch
from
fd9a301 to
7ea48b3
Compare
|
Konflux build is good, but need to rebase, then merge. 
|
ysok
force-pushed
the
odh-RHAIENG-2853-jupyter-rocm-tensorflow
branch
from
7ea48b3 to
6987178
Compare
|
[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 |
Hermetic ROCm TensorFlow Jupyter image: wire Tekton/Cachi2 prefetch for gomod (mongocli), RPM+generic (prefetch-input/odh), and pip (requirements.rocm.txt). Dockerfiles install Python packages from Cachi2-fetched wheels; pyproject.toml uses the public-index lock path. pylock-to-requirements gains stricter Hermeto output (hashes and direct wheel URLs). COPY prefetch-input/... paths are relative to the repo-root build context (path-context .); no per-component prefetch-input symlink, because Konflux Hermeto rejects symlink segments in tracked prefetch paths.
ysok
force-pushed
the
odh-RHAIENG-2853-jupyter-rocm-tensorflow
branch
from
6987178 to
a245e57
Compare
|
New changes are detected. LGTM label has been removed. |
3 participants
RHAIENG-2853: Hermetic build for ROCm TensorFlow Jupyter image
Description
Make the ROCm TensorFlow Jupyter notebook image build hermetically (prefetched mongocli, RPMs, and Python wheels via Cachi2), wire Tekton to run with hermetic prefetch, align requirements.rocm.txt with the lockfile index, and add the usual prefetch-input symlink so local prefetch scripts work.
How Has This Been Tested?
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:
Summary by CodeRabbit
New Features
Chores