adding additional packages in minimal Dockerfile.cpu for ppc64le #1175
Conversation
## Walkthrough
The conditional logic in the Dockerfile for installing development tools and dependencies was updated to include the "ppc64le" architecture alongside "s390x". Now, both architectures trigger installation of an extended set of OS packages, while other architectures install only a minimal subset.
## Changes
| Files | Change Summary |
|--------------------------------------------------|--------------------------------------------------------------------------------------------------|
| runtimes/minimal/ubi9-python-3.11/Dockerfile.cpu | Expanded architecture check for package installation to include "ppc64le" along with "s390x". |
## Possibly related PRs
- opendatahub-io/notebooks#1166: Refactors the architecture-based package installation logic in the same Dockerfile.
- opendatahub-io/notebooks#1132: Introduced the conditional logic for "s390x" that is now extended to "ppc64le".
## Suggested reviewers
- jiridanek📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
|
Hi @AaruniAggarwal. Thanks for your PR. I'm waiting for a opendatahub-io member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
runtimes/minimal/ubi9-python-3.11/Dockerfile.cpu (1)
12-18: DRY package installation logic
Themesa-libGLandskopeopackages are duplicated in both branches. Extracting common packages into a variable and appending the development toolchain conditionally can improve maintainability and readability.Example diff:
RUN ARCH=$(uname -m) && \ echo "Detected architecture: $ARCH" && \ - if [ "$ARCH" = "s390x" ] || [ "$ARCH" = "ppc64le" ]; then \ - dnf install -y mesa-libGL skopeo gcc g++ make openssl-devel autoconf automake libtool cmake; \ - else \ - dnf install -y mesa-libGL skopeo; \ - fi && \ + COMMON_PKGS="mesa-libGL skopeo" && \ + DEV_PKGS="gcc g++ make openssl-devel autoconf automake libtool cmake" && \ + if [ "$ARCH" = "s390x" ] || [ "$ARCH" = "ppc64le" ]; then \ + dnf install -y $COMMON_PKGS $DEV_PKGS; \ + else \ + dnf install -y $COMMON_PKGS; \ + fi && \
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
runtimes/minimal/ubi9-python-3.11/Dockerfile.cpu(1 hunks)
🔇 Additional comments (1)
runtimes/minimal/ubi9-python-3.11/Dockerfile.cpu (1)
14-14: Arch check extended to ppc64le
Addingppc64leensures the full build toolchain is installed on IBM Power, resolving the pyzmq wheel build failure duringdocker build.
Signed-off-by: AaruniAggarwal <aaruniagg@gmail.com>
AaruniAggarwal
force-pushed
the
nb-ppc64le
branch
from
7a1a8e8 to
e711f5b
Compare
|
@coderabbitai should the commit messages and PR title be written in imperative mood? |
|
Tip For best results, initiate chat on the files or code changes. @jiridanek Yes, absolutely! Based on your previous guidance that I have in my knowledge base, PR titles and commit messages should be written in imperative mood, completing the sentence "If applied, this commit will ___". Looking at the current PR title "adding additional packages in minimal Dockerfile.cpu for ppc64le", it should be written as "Add additional packages in minimal Dockerfile.cpu for ppc64le" to follow the imperative mood convention. This aligns with the learnings I have from your previous feedback emphasizing that PR titles should use imperative mood (e.g., "Add feature" not "Created feature") and be concise while clearly describing what the change accomplishes. thinking thoughtfully 🧠 Learnings used |
|
@coderabbitai most PRs on this project are associated with a https://issues.redhat.com Jira ticket; please explain how this is critical for traceability between requirements, release process, and product documentation. |
|
Tip For best results, initiate chat on the files or code changes. @jiridanek Associating PRs with Jira tickets from https://issues.redhat.com creates a comprehensive traceability chain that's essential for enterprise software development. Here's why this is critical: Requirements Traceability
Release Process Benefits
Product Documentation
Compliance & Audit Trail
Cross-Team Coordination
This practice ensures that every code change has a clear business justification and can be traced through the entire development lifecycle. thoughtfully explaining traceability ✏️ Learnings added
|
This PR adds additional packages in minimal runtime Dockerfile.cpu for ppc64le as pyzmq require them. Without these changes, image is failing to build on ppc64le with following error:
This change ensures notebook runtime minimal builds are successfull on IBM Power (ppc64le) and it will not impact to x86_64 builds.
Tested on: ppc64le
Summary by CodeRabbit