Skip to content

Conversation

@jiridanek
Copy link
Member

@jiridanek jiridanek commented Nov 15, 2025

Description

https://issues.redhat.com/browse/RHAIENG-1965

How Has This Been Tested?

Self checklist (all need to be checked):

  • Ensure that you have run make test (gmake on macOS) before asking for review
  • Changes to everything except Dockerfile.konflux files should be done in odh/notebooks and automatically synced to rhds/notebooks. For Konflux-specific changes, modify Dockerfile.konflux files directly in rhds/notebooks as these require special attention in the downstream repository and flow to the upcoming RHOAI release.

Merge criteria:

  • The commits are squashed in a cohesive manner and have meaningful messages.
  • Testing instructions have been added in the PR body (for PRs involving changes that are not immediately obvious).
  • The developer has manually tested the changes and verified that the changes work

Summary by CodeRabbit

  • New Features

    • Added conditional Red Hat subscription refresh during image build preparation. When available, subscriptions are automatically refreshed to ensure package management systems are current and functional before package updates proceed.
  • Chores

    • Enhanced build structure organization with explicit section markers for clarity and maintainability. Build steps are now clearly delineated with structural markers across multiple container image variants.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 15, 2025

Walkthrough

Added conditional Red Hat subscription-manager refresh blocks to 23 Dockerfiles across jupyter, rstudio, and runtimes image variants. Updated Elyra bootstrapper download sections in 9 runtimes Dockerfiles with explicit BEGIN/END markers. Updated scripts/dockerfile_fragments.py to define these new fragments.

Changes

Cohort / File(s) Summary
Jupyter CPU images
codeserver/ubi9-python-3.12/Dockerfile.cpu, jupyter/datascience/ubi9-python-3.12/Dockerfile.cpu, jupyter/minimal/ubi9-python-3.12/Dockerfile.cpu, jupyter/trustyai/ubi9-python-3.12/Dockerfile.cpu
Added conditional subscription-manager refresh block before upgrade sequence
Jupyter CUDA images
jupyter/minimal/ubi9-python-3.12/Dockerfile.cuda, jupyter/pytorch/ubi9-python-3.12/Dockerfile.cuda, jupyter/pytorch+llmcompressor/ubi9-python-3.12/Dockerfile.cuda, jupyter/tensorflow/ubi9-python-3.12/Dockerfile.cuda
Added conditional subscription-manager refresh block before upgrade sequence
Jupyter ROCM images
jupyter/minimal/ubi9-python-3.12/Dockerfile.rocm, jupyter/rocm/pytorch/ubi9-python-3.12/Dockerfile.rocm, jupyter/rocm/tensorflow/ubi9-python-3.12/Dockerfile.rocm
Added conditional subscription-manager refresh block before upgrade sequence
RStudio CPU images
rstudio/c9s-python-3.12/Dockerfile.cpu, rstudio/rhel9-python-3.12/Dockerfile.cpu
Added conditional subscription-manager refresh block before upgrade sequence; removed prior upgrade-related comments in rhel9 variant
RStudio CUDA images
rstudio/c9s-python-3.12/Dockerfile.cuda, rstudio/rhel9-python-3.12/Dockerfile.cuda
Added conditional subscription-manager refresh block before upgrade sequence; removed prior duplicate heading in rhel9 variant
Runtimes CPU images
runtimes/datascience/ubi9-python-3.12/Dockerfile.cpu, runtimes/minimal/ubi9-python-3.12/Dockerfile.cpu
Added conditional subscription-manager refresh block; added explicit BEGIN/END markers around Elyra bootstrapper download section
Runtimes CUDA images
runtimes/pytorch+llmcompressor/ubi9-python-3.12/Dockerfile.cuda, runtimes/pytorch/ubi9-python-3.12/Dockerfile.cuda, runtimes/tensorflow/ubi9-python-3.12/Dockerfile.cuda
Added conditional subscription-manager refresh block; added explicit BEGIN/END markers around Elyra bootstrapper download section
Runtimes ROCM images
runtimes/rocm-pytorch/ubi9-python-3.12/Dockerfile.rocm, runtimes/rocm-tensorflow/ubi9-python-3.12/Dockerfile.rocm
Added conditional subscription-manager refresh block; added explicit BEGIN/END markers around Elyra bootstrapper download section
Dockerfile fragments script
scripts/dockerfile_fragments.py
Added new pre-upgrade subscription-manager refresh fragment and Download Elyra Bootstrapper fragment definitions

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~15 minutes

  • Highly repetitive changes applied consistently across multiple files with minimal variation
  • Subscription-manager refresh implementation uses straightforward conditional shell logic
  • Elyra bootstrapper marker additions are trivial comment/delimiter updates
  • Primary review focus should be verifying consistency of the subscription-manager block across variants

Possibly related issues

  • Addresses infrastructure maintenance for Red Hat subscription management during container builds, which aligns with the pre-upgrade/upgrade sequence modifications identified in related issue discussions around CUDA Dockerfile optimization

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description is incomplete. It lacks substantive details in the 'Description' and 'How Has This Been Tested?' sections, and all required checklist items are unchecked. Provide detailed testing description and check off all completed items in both the self-checklist and merge criteria sections to confirm the changes have been properly tested.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The PR title accurately reflects the main changes: introducing two templates (Download Elyra Bootstrapper and subscription-manager refresh) across multiple Dockerfiles as shown in the raw summary.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

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

@github-actions github-actions bot added the review-requested GitHub Bot creates notification on #pr-review-ai-ide-team slack channel label Nov 15, 2025
@openshift-ci openshift-ci bot added size/l and removed size/l labels Nov 15, 2025
jiridanek added a commit to jiridanek/notebooks that referenced this pull request Nov 15, 2025
jiridanek added a commit to jiridanek/notebooks that referenced this pull request Nov 15, 2025
@openshift-ci openshift-ci bot added size/l and removed size/l labels Nov 15, 2025
jiridanek added a commit to jiridanek/notebooks that referenced this pull request Nov 15, 2025
jiridanek added a commit to jiridanek/notebooks that referenced this pull request Nov 15, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

♻️ Duplicate comments (8)
runtimes/rocm-pytorch/ubi9-python-3.12/Dockerfile.rocm (2)

25-31: [DUPLICATE] Subscription-manager refresh logic is consistent across files.

This implementation mirrors the pattern in other files. See prior review comments on error-handling strategy.


93-99: [DUPLICATE] Elyra bootstrapper URL and error handling concerns apply here as well.

See prior review comments on v4.3.1 version verification and curl error handling.

runtimes/pytorch+llmcompressor/ubi9-python-3.12/Dockerfile.cuda (2)

27-33: [DUPLICATE] Subscription-manager refresh logic is consistent across files.

This implementation mirrors the pattern in other files. See prior review comments on error-handling strategy.


95-101: [DUPLICATE] Elyra bootstrapper URL and error handling concerns apply here as well.

See prior review comments on v4.3.1 version verification and curl error handling.

runtimes/pytorch/ubi9-python-3.12/Dockerfile.cuda (2)

27-33: [DUPLICATE] Subscription-manager refresh logic is consistent across files.

This implementation mirrors the pattern in other files. See prior review comments on error-handling strategy.


95-101: [DUPLICATE] Elyra bootstrapper URL and error handling concerns apply here as well.

See prior review comments on v4.3.1 version verification and curl error handling.

runtimes/minimal/ubi9-python-3.12/Dockerfile.cpu (2)

25-31: [DUPLICATE] Subscription-manager refresh logic is consistent across files.

This implementation mirrors the pattern in other files. See prior review comments on error-handling strategy.


100-106: [DUPLICATE] Elyra bootstrapper URL and error handling concerns apply here as well.

See prior review comments on v4.3.1 version verification and curl error handling.

🧹 Nitpick comments (6)
jupyter/rocm/pytorch/ubi9-python-3.12/Dockerfile.rocm (1)

41-47: Good placement & guard; consider clarifying error handling.

The subscription-manager refresh block is correctly positioned before the dnf upgrade and safely guards against missing subscription-manager. However, the && / || pattern combined with set -e can mask which command actually failed—if refresh fails for any reason, the message still claims "Not registered," which may mislead troubleshooting.

For clarity, consider restructuring to be more explicit:

- RUN /bin/bash <<'EOF'
- set -Eeuxo pipefail
- if command -v subscription-manager &> /dev/null; then
-   subscription-manager identity &>/dev/null && subscription-manager refresh || echo "Not registered, skipping refresh."
- fi
- EOF
+ RUN /bin/bash <<'EOF'
+ set -Eeuxo pipefail
+ if command -v subscription-manager &> /dev/null; then
+   if subscription-manager identity &>/dev/null; then
+     subscription-manager refresh
+   else
+     echo "Not registered, skipping refresh."
+   fi
+ fi
+ EOF

This makes the error path explicit and the message accurate—only "Not registered" when identity actually fails.

That said, the current implementation is functional and idempotent, so if this pattern is consistent across the PR, approval is appropriate as-is.

rstudio/c9s-python-3.12/Dockerfile.cuda (1)

18-36: Subscription-manager refresh block implementation looks good.

The conditional logic correctly checks for subscription-manager availability, verifies registration, and gracefully skips if not registered. With set -e active, the &&/|| chaining ensures the RUN command succeeds even if identity or refresh fails. The placement before the dnf upgrade is correct—subscription state should be refreshed before attempting package upgrades.

One minor readability note (optional): The &&/|| chain could be slightly more explicit with a traditional if/else for clarity:

-if command -v subscription-manager &> /dev/null; then
-  subscription-manager identity &>/dev/null && subscription-manager refresh || echo "Not registered, skipping refresh."
-fi
+if command -v subscription-manager &> /dev/null; then
+  if subscription-manager identity &>/dev/null; then
+    subscription-manager refresh || echo "Failed to refresh subscription."
+  else
+    echo "Not registered, skipping refresh."
+  fi
+fi

This makes the distinction between "not registered" and "refresh failed" clearer. However, the current approach works correctly and is valid bash idiom.

jupyter/minimal/ubi9-python-3.12/Dockerfile.rocm (1)

25-31: Error message could be more precise.

The message "Not registered, skipping refresh." is printed in two scenarios: (1) when subscription-manager identity fails (system not registered), and (2) when subscription-manager refresh fails for any other reason. The message is misleading in the latter case.

For a non-blocking best-effort operation, this is acceptable, but consider a more generic message like "Subscription refresh skipped" if you plan to standardize this across the 23 Dockerfiles mentioned in the PR.

If you'd like to improve clarity, apply this diff:

-subscription-manager identity &>/dev/null && subscription-manager refresh || echo "Not registered, skipping refresh."
+subscription-manager identity &>/dev/null && subscription-manager refresh || echo "Subscription refresh skipped."

Alternatively, if distinction matters, use a more structured approach:

-subscription-manager identity &>/dev/null && subscription-manager refresh || echo "Not registered, skipping refresh."
+{ subscription-manager identity &>/dev/null && subscription-manager refresh; } || echo "Subscription refresh skipped."
rstudio/rhel9-python-3.12/Dockerfile.cuda (1)

27-34: Subscription-manager refresh error message could be misleading.

Line 32 prints "Not registered, skipping refresh" for both not-registered and refresh-failure scenarios. If subscription-manager exists and is registered, but the refresh command fails for another reason (network, service issue), the error message will be inaccurate.

Consider distinguishing the two failure modes:

 RUN /bin/bash <<'EOF'
 set -Eeuxo pipefail
-if command -v subscription-manager &> /dev/null; then
-  subscription-manager identity &>/dev/null && subscription-manager refresh || echo "Not registered, skipping refresh."
+if command -v subscription-manager &> /dev/null; then
+  if subscription-manager identity &>/dev/null; then
+    subscription-manager refresh || echo "Warning: Failed to refresh subscription."
+  else
+    echo "Not registered, skipping refresh."
+  fi
 fi
 EOF

Alternatively, if the current message is sufficient for your logging/monitoring strategy, this can remain as-is. Please verify this pattern is applied consistently across all modified Dockerfiles in this PR.

jupyter/pytorch/ubi9-python-3.12/Dockerfile.cuda (1)

43-49: Verify subscription-manager refresh logic handles failures gracefully.

The || echo clause will suppress any refresh failure and continue the build. This is intentional for the "not registered" case, but could also mask transient failures or permission issues with subscription-manager.

If a subscription is present but the refresh fails, the build silently continues. Consider whether this is the desired resilience level, or if you'd prefer to fail fast on actual subscription problems.

The curl -fL flag in later blocks (e.g., line 80) correctly fails on HTTP errors. Ensure consistency: is silent-continue the right strategy for all external I/O failures in these images?

runtimes/datascience/ubi9-python-3.12/Dockerfile.cpu (1)

399-401: The curl command lacks explicit error exit handling in the context.

While the curl -fL flags cause failure on HTTP errors, there is no explicit || exit 1 or error trap documented. In multi-stage builds, a silent curl failure could propagate silently.

Confirm that the RUN instruction's default behavior (exit on non-zero) is sufficient, or add an explicit error check if Dockerfiles use set +e elsewhere.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8da6079 and 36f23cc.

📒 Files selected for processing (23)
  • codeserver/ubi9-python-3.12/Dockerfile.cpu (1 hunks)
  • jupyter/datascience/ubi9-python-3.12/Dockerfile.cpu (1 hunks)
  • jupyter/minimal/ubi9-python-3.12/Dockerfile.cpu (1 hunks)
  • jupyter/minimal/ubi9-python-3.12/Dockerfile.cuda (1 hunks)
  • jupyter/minimal/ubi9-python-3.12/Dockerfile.rocm (1 hunks)
  • jupyter/pytorch+llmcompressor/ubi9-python-3.12/Dockerfile.cuda (1 hunks)
  • jupyter/pytorch/ubi9-python-3.12/Dockerfile.cuda (1 hunks)
  • jupyter/rocm/pytorch/ubi9-python-3.12/Dockerfile.rocm (1 hunks)
  • jupyter/rocm/tensorflow/ubi9-python-3.12/Dockerfile.rocm (1 hunks)
  • jupyter/tensorflow/ubi9-python-3.12/Dockerfile.cuda (1 hunks)
  • jupyter/trustyai/ubi9-python-3.12/Dockerfile.cpu (1 hunks)
  • rstudio/c9s-python-3.12/Dockerfile.cpu (1 hunks)
  • rstudio/c9s-python-3.12/Dockerfile.cuda (1 hunks)
  • rstudio/rhel9-python-3.12/Dockerfile.cpu (1 hunks)
  • rstudio/rhel9-python-3.12/Dockerfile.cuda (1 hunks)
  • runtimes/datascience/ubi9-python-3.12/Dockerfile.cpu (2 hunks)
  • runtimes/minimal/ubi9-python-3.12/Dockerfile.cpu (2 hunks)
  • runtimes/pytorch+llmcompressor/ubi9-python-3.12/Dockerfile.cuda (2 hunks)
  • runtimes/pytorch/ubi9-python-3.12/Dockerfile.cuda (2 hunks)
  • runtimes/rocm-pytorch/ubi9-python-3.12/Dockerfile.rocm (2 hunks)
  • runtimes/rocm-tensorflow/ubi9-python-3.12/Dockerfile.rocm (2 hunks)
  • runtimes/tensorflow/ubi9-python-3.12/Dockerfile.cuda (2 hunks)
  • scripts/dockerfile_fragments.py (2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (53)
  • GitHub Check: build (cuda-jupyter-tensorflow-ubi9-python-3.12, 3.12, linux/amd64, false) / build
  • GitHub Check: build (rocm-runtime-tensorflow-ubi9-python-3.12, 3.12, linux/amd64, false) / build
  • GitHub Check: build (runtime-cuda-tensorflow-ubi9-python-3.12, 3.12, linux/arm64, false) / build
  • GitHub Check: build (rocm-jupyter-minimal-ubi9-python-3.12, 3.12, linux/amd64, false) / build
  • GitHub Check: build (jupyter-minimal-ubi9-python-3.12, 3.12, linux/amd64, false) / build
  • GitHub Check: build (jupyter-datascience-ubi9-python-3.12, 3.12, linux/amd64, false) / build
  • GitHub Check: build (codeserver-ubi9-python-3.12, 3.12, linux/amd64, false) / build
  • GitHub Check: build (jupyter-minimal-ubi9-python-3.12, 3.12, linux/ppc64le, false) / build
  • GitHub Check: build (cuda-jupyter-pytorch-llmcompressor-ubi9-python-3.12, 3.12, linux/amd64, false) / build
  • GitHub Check: build (jupyter-datascience-ubi9-python-3.12, 3.12, linux/ppc64le, false) / build
  • GitHub Check: build (cuda-rstudio-c9s-python-3.12, 3.12, linux/amd64, false) / build
  • GitHub Check: build (cuda-jupyter-pytorch-ubi9-python-3.12, 3.12, linux/amd64, false) / build
  • GitHub Check: build (cuda-jupyter-tensorflow-ubi9-python-3.12, 3.12, linux/arm64, false) / build
  • GitHub Check: build (runtime-cuda-tensorflow-ubi9-python-3.12, 3.12, linux/amd64, false) / build
  • GitHub Check: build (cuda-jupyter-minimal-ubi9-python-3.12, 3.12, linux/amd64, false) / build
  • GitHub Check: build (codeserver-ubi9-python-3.12, 3.12, linux/arm64, false) / build
  • GitHub Check: build (rocm-runtime-pytorch-ubi9-python-3.12, 3.12, linux/amd64, false) / build
  • GitHub Check: build (runtime-datascience-ubi9-python-3.12, 3.12, linux/amd64, false) / build
  • GitHub Check: build (runtime-minimal-ubi9-python-3.12, 3.12, linux/amd64, false) / build
  • GitHub Check: build (runtime-datascience-ubi9-python-3.12, 3.12, linux/s390x, false) / build
  • GitHub Check: build (rocm-jupyter-pytorch-ubi9-python-3.12, 3.12, linux/amd64, false) / build
  • GitHub Check: build (rstudio-c9s-python-3.12, 3.12, linux/amd64, false) / build
  • GitHub Check: build (runtime-cuda-pytorch-ubi9-python-3.12, 3.12, linux/amd64, false) / build
  • GitHub Check: build (rocm-jupyter-tensorflow-ubi9-python-3.12, 3.12, linux/amd64, false) / build
  • GitHub Check: build (runtime-cuda-pytorch-llmcompressor-ubi9-python-3.12, 3.12, linux/amd64, false) / build
  • GitHub Check: build (jupyter-trustyai-ubi9-python-3.12, 3.12, linux/amd64, false) / build
  • GitHub Check: build (runtime-minimal-ubi9-python-3.12, 3.12, linux/s390x, false) / build
  • GitHub Check: build (cuda-jupyter-minimal-ubi9-python-3.12, 3.12, linux/arm64, false) / build
  • GitHub Check: build (jupyter-minimal-ubi9-python-3.12, 3.12, linux/s390x, false) / build
  • GitHub Check: build (rstudio-rhel9-python-3.12, 3.12, linux/amd64, true) / build
  • GitHub Check: build (cuda-rstudio-rhel9-python-3.12, 3.12, linux/amd64, true) / build
  • GitHub Check: Red Hat Konflux / odh-workbench-jupyter-minimal-rocm-py312-ubi9-on-pull-request
  • GitHub Check: Red Hat Konflux / odh-workbench-jupyter-tensorflow-rocm-py312-ubi9-on-pull-request
  • GitHub Check: Red Hat Konflux / odh-workbench-jupyter-datascience-cpu-py312-ubi9-on-pull-request
  • GitHub Check: Red Hat Konflux / odh-workbench-codeserver-datascience-cpu-py312-ubi9-on-pull-request
  • GitHub Check: Red Hat Konflux / odh-workbench-rstudio-minimal-cuda-py312-rhel9-on-pull-request
  • GitHub Check: Red Hat Konflux / odh-workbench-jupyter-minimal-cuda-py312-ubi9-on-pull-request
  • GitHub Check: Red Hat Konflux / odh-workbench-jupyter-trustyai-cpu-py312-ubi9-on-pull-request
  • GitHub Check: Red Hat Konflux / odh-pipeline-runtime-pytorch-rocm-py312-ubi9-on-pull-request
  • GitHub Check: Red Hat Konflux / odh-pipeline-runtime-tensorflow-cuda-py312-ubi9-on-pull-request
  • GitHub Check: Red Hat Konflux / odh-pipeline-runtime-pytorch-llmcompressor-cuda-py312-ubi9-on-pull-request
  • GitHub Check: Red Hat Konflux / odh-workbench-jupyter-tensorflow-cuda-py312-ubi9-on-pull-request
  • GitHub Check: Red Hat Konflux / odh-workbench-jupyter-pytorch-llmcompressor-cuda-py312-ubi9-on-pull-request
  • GitHub Check: Red Hat Konflux / odh-pipeline-runtime-datascience-cpu-py312-ubi9-on-pull-request
  • GitHub Check: Red Hat Konflux / odh-pipeline-runtime-tensorflow-rocm-py312-ubi9-on-pull-request
  • GitHub Check: Red Hat Konflux / odh-workbench-rstudio-minimal-cpu-py312-c9s-on-pull-request
  • GitHub Check: Red Hat Konflux / odh-pipeline-runtime-minimal-cpu-py312-ubi9-on-pull-request
  • GitHub Check: Red Hat Konflux / odh-workbench-jupyter-pytorch-cuda-py312-ubi9-on-pull-request
  • GitHub Check: Red Hat Konflux / odh-workbench-jupyter-minimal-cpu-py312-ubi9-on-pull-request
  • GitHub Check: Red Hat Konflux / odh-workbench-rstudio-minimal-cuda-py312-c9s-on-pull-request
  • GitHub Check: Red Hat Konflux / odh-pipeline-runtime-pytorch-cuda-py312-ubi9-on-pull-request
  • GitHub Check: Red Hat Konflux / odh-workbench-rstudio-minimal-cpu-py312-rhel9-on-pull-request
  • GitHub Check: Red Hat Konflux / odh-workbench-jupyter-pytorch-rocm-py312-ubi9-on-pull-request
🔇 Additional comments (21)
jupyter/rocm/tensorflow/ubi9-python-3.12/Dockerfile.rocm (1)

41-47: Well-implemented subscription-manager refresh block.

The subscription-manager refresh logic follows shell best practices:

  • Proper error handling with set -Eeuxo pipefail
  • Safe existence check via command -v before invoking subscription-manager
  • Graceful fallback message when not registered
  • Appropriate placement before system upgrade to ensure subscriptions are current

Since the AI summary indicates this pattern is replicated across 23 Dockerfiles, please verify that the script behaves consistently across different image variants (e.g., with/without Red Hat subscriptions, different UBI versions). Also confirm alignment with related PR #2682 if there are dependencies or interactions.

runtimes/rocm-tensorflow/ubi9-python-3.12/Dockerfile.rocm (2)

25-31: Verify subscription-manager error handling semantics.

The current logic treats any failure from subscription-manager refresh as "not registered." This could mask legitimate refresh failures (e.g., network issues, subscription service errors). Please verify that this fallback behavior is intentional.

If all failures should be treated as non-blocking, consider adding a clarifying comment. Alternatively, if you need to distinguish between "not registered" and other failures, refine the conditional logic.


94-100: Elyra bootstrapper markers are properly placed and consistent.

The BEGIN/END markers follow the existing templating pattern in the file (e.g., lines 55–57, 59–68). The placement of ELYRA_INSTALL_PACKAGES within the marked section maintains logical grouping with the download step. The functional download operation remains unchanged.

jupyter/pytorch+llmcompressor/ubi9-python-3.12/Dockerfile.cuda (2)

43-49: Solid subscription-manager refresh implementation.

The conditional subscription-manager refresh block is well-structured: it safely checks for the command's existence, attempts both identity verification and refresh, and provides a graceful fallback message for unregistered systems. Placement before the dnf upgrade is logical.


1-193: Verify the Elyra bootstrapper template changes are present in this file.

The AI summary mentions "Updated Elyra bootstrapper download sections in 9 runtimes Dockerfiles with explicit BEGIN/END markers," but no Elyra bootstrapper changes are visible in the provided code. Additionally, this file is located in the jupyter directory path rather than a runtimes directory. Please verify:

  1. Whether Elyra bootstrapper changes should be present in this specific Dockerfile
  2. Whether the complete file content was provided (the code appears to end abruptly at line 193 with an incomplete RUN statement)
rstudio/rhel9-python-3.12/Dockerfile.cpu (2)

27-45: Inconsistency between summary and actual changes.

The AI summary states that the PR "removes the previous contextual comments describing the upgrade problem and workaround," but lines 36–38 (the Problem/Solution comments) remain unchanged in the provided code. Either the summary is inaccurate or this file was not fully updated.


27-34: Subscription-manager refresh logic looks sound.

The conditional refresh block correctly:

  • Uses proper bash error handling (set -Eeuxo pipefail)
  • Checks for the existence of subscription-manager before attempting to use it
  • Gates the refresh operation behind an identity check, ensuring it only runs if already registered
  • Gracefully handles the unregistered case without failing the build

The placement before the upgrade block (lines 39–43) is appropriate, as refreshing subscriptions before upgrading ensures access to the latest packages.

rstudio/c9s-python-3.12/Dockerfile.cpu (1)

23-29: Well-placed subscription-manager refresh logic.

The new subscription-manager refresh block is correctly positioned before the upgrade step and includes appropriate safeguards:

  • The command-existence check gracefully handles environments where subscription-manager is unavailable
  • Error handling with set -Eeuxo pipefail and the || fallback ensures the build won't fail if registration is unavailable or refresh encounters issues
  • The conditional logic properly chains identity verification with refresh
jupyter/minimal/ubi9-python-3.12/Dockerfile.rocm (1)

25-31: Consistency verified across all 23 Dockerfiles. All instances implement identical logic with the same error handling ("Not registered, skipping refresh."), and the pattern is consistently placed before upgrade operations. The implementations are generated from a template source (scripts/dockerfile_fragments.py), which ensures maintainability and uniform consistency across the codebase.

rstudio/rhel9-python-3.12/Dockerfile.cuda (1)

27-45: Verification complete: subscription-manager refresh blocks are consistent across all 22 Dockerfiles.

All verification points confirmed:

  1. Refresh logic identical — All 22 Dockerfiles use the exact same implementation with proper error handling
  2. No syntax or logic errors — All blocks follow identical patterns with correct bash syntax (set -Eeuxo pipefail)
  3. Placement consistent — All files place the refresh block before the dnf upgrade, marked with ### BEGIN upgrade first and ### END upgrade first

The dnf upgrade command is also consistently applied across all variants with identical flags (--refresh --nobest --skip-broken --nodocs --noplugins --setopt=install_weak_deps=0 --setopt=keepcache=0), properly addressing the systemd package conflict mentioned in the inline comments.

jupyter/datascience/ubi9-python-3.12/Dockerfile.cpu (1)

56-62: LGTM! Subscription-manager refresh block is well-structured.

The conditional subscription refresh logic correctly:

  • Guards against missing subscription-manager with command -v
  • Handles unregistered systems gracefully with the || echo fallback
  • Ensures the build continues regardless of subscription status
  • Executes before package upgrades to ensure fresh repo metadata
jupyter/trustyai/ubi9-python-3.12/Dockerfile.cpu (1)

66-72: LGTM! Consistent subscription-manager refresh implementation.

This matches the subscription refresh pattern used across the PR, correctly handling subscription-manager availability and registration status before upgrades.

jupyter/tensorflow/ubi9-python-3.12/Dockerfile.cuda (1)

43-49: LGTM! Subscription refresh correctly applied to CUDA base stage.

The subscription-manager refresh block is properly positioned in the cuda-base stage before package upgrades, maintaining consistency with CPU variants.

codeserver/ubi9-python-3.12/Dockerfile.cpu (1)

87-93: LGTM! Subscription refresh properly integrated.

The subscription-manager refresh block follows the established pattern and is correctly positioned before the upgrade phase.

jupyter/minimal/ubi9-python-3.12/Dockerfile.cuda (1)

27-33: LGTM! Consistent implementation in minimal CUDA variant.

The subscription-manager refresh block is properly integrated into the cuda-base stage with correct error handling.

jupyter/minimal/ubi9-python-3.12/Dockerfile.cpu (1)

25-31: LGTM! Consistent implementation in minimal CPU variant.

The subscription-manager refresh block is properly integrated with appropriate error handling.

scripts/dockerfile_fragments.py (2)

38-54: LGTM! Subscription-manager refresh fragment is well-defined.

The template correctly includes both the subscription refresh logic and the subsequent upgrade command, maintaining the complete upgrade flow.


71-76: Version pinning is consistent and intentional across the project.

The hardcoded Elyra version v4.3.1 in the Dockerfile matches odh-elyra==4.3.1 pinned uniformly across all pyproject.toml and pylock.toml files across seven Jupyter environments (trustyai, rocm/pytorch, rocm/tensorflow, tensorflow, pytorch, pytorch+llmcompressor, datascience). The consistency and synchronization demonstrate this is an intentional versioning strategy rather than an oversight.

runtimes/tensorflow/ubi9-python-3.12/Dockerfile.cuda (2)

29-35: LGTM! Subscription refresh correctly added to CUDA runtime.

The subscription-manager refresh block is properly positioned before upgrades in the cuda-base stage.


98-104: LGTM! Elyra Bootstrapper section properly templated.

The BEGIN/END markers correctly encapsulate the Elyra bootstrapper download and related environment variable, enabling centralized template management via scripts/dockerfile_fragments.py.

runtimes/datascience/ubi9-python-3.12/Dockerfile.cpu (1)

399-405: Verify Elyra v4.3.1 URL is still available and resolve version pinning strategy.

The bootstrapper URL is hardcoded to v4.3.1. Before merging, confirm:

  1. This GitHub tag still exists and the raw.githubusercontent.com URL is stable.
  2. Whether v4.3.1 is the intended / latest supported version for this set of notebooks.
  3. If there's a plan to manage Elyra version upgrades (e.g., via configuration, not manual Dockerfile edits).

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Nov 15, 2025

@jiridanek: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/notebook-cuda-jupyter-tf-ubi9-python-3-12-pr-image-mirror 36f23cc link true /test notebook-cuda-jupyter-tf-ubi9-python-3-12-pr-image-mirror
ci/prow/runtime-ubi9-python-3-12-pr-image-mirror 36f23cc link true /test runtime-ubi9-python-3-12-pr-image-mirror
ci/prow/runtime-cuda-pt-ubi9-python-3-12-pr-image-mirror 36f23cc link true /test runtime-cuda-pt-ubi9-python-3-12-pr-image-mirror
ci/prow/notebook-cuda-jupyter-ubi9-python-3-12-pr-image-mirror 36f23cc link true /test notebook-cuda-jupyter-ubi9-python-3-12-pr-image-mirror
ci/prow/runtime-ds-ubi9-python-3-12-pr-image-mirror 36f23cc link true /test runtime-ds-ubi9-python-3-12-pr-image-mirror
ci/prow/notebook-cuda-jupyter-pt-ubi9-python-3-12-pr-image-mirror 36f23cc link true /test notebook-cuda-jupyter-pt-ubi9-python-3-12-pr-image-mirror
ci/prow/notebook-jupyter-ds-ubi9-python-3-12-pr-image-mirror 36f23cc link true /test notebook-jupyter-ds-ubi9-python-3-12-pr-image-mirror
ci/prow/rocm-runtime-pt-ubi9-python-3-12-pr-image-mirror 36f23cc link true /test rocm-runtime-pt-ubi9-python-3-12-pr-image-mirror
ci/prow/notebook-rocm-jupyter-pt-ubi9-python-3-12-pr-image-mirror 36f23cc link true /test notebook-rocm-jupyter-pt-ubi9-python-3-12-pr-image-mirror
ci/prow/notebook-rocm-jupyter-ubi9-python-3-12-pr-image-mirror 36f23cc link true /test notebook-rocm-jupyter-ubi9-python-3-12-pr-image-mirror
ci/prow/codeserver-ubi9-python-3-12-pr-image-mirror 36f23cc link true /test codeserver-ubi9-python-3-12-pr-image-mirror
ci/prow/images 36f23cc link true /test images
ci/prow/notebook-jupyter-tai-ubi9-python-3-12-pr-image-mirror 36f23cc link true /test notebook-jupyter-tai-ubi9-python-3-12-pr-image-mirror
ci/prow/notebook-jupyter-ubi9-python-3-12-pr-image-mirror 36f23cc link true /test notebook-jupyter-ubi9-python-3-12-pr-image-mirror
ci/prow/runtime-cuda-tf-ubi9-python-3-12-pr-image-mirror 36f23cc link true /test runtime-cuda-tf-ubi9-python-3-12-pr-image-mirror
ci/prow/rocm-notebooks-py312-e2e-tests 36f23cc link true /test rocm-notebooks-py312-e2e-tests
ci/prow/notebooks-py312-ubi9-e2e-tests 36f23cc link true /test notebooks-py312-ubi9-e2e-tests

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Nov 15, 2025

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: ide-developer
Once this PR has been reviewed and has the lgtm label, please assign jstourac for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@jiridanek jiridanek merged commit cf81bf2 into opendatahub-io:main Nov 15, 2025
40 of 82 checks passed
jiridanek added a commit that referenced this pull request Nov 15, 2025
@jiridanek jiridanek deleted the jd_templatizing branch November 15, 2025 14:55
jiridanek added a commit to jiridanek/notebooks that referenced this pull request Nov 15, 2025
jiridanek added a commit to jiridanek/notebooks that referenced this pull request Nov 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtm review-requested GitHub Bot creates notification on #pr-review-ai-ide-team slack channel size/l

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants