RHAIENG-2042: fixup(base-images): add TARGETARCH as an argument to ubi9-python-3.12 Dockerfile
#2695
Conversation
|
[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 |
github-actions
bot
added
the
review-requested
WalkthroughAdds TARGETARCH propagation to two CPU Docker base images and removes the Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related issues
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
jiridanek
changed the title
TARGETARCH as an argument to ubi9-python-3.12 DockerfileTARGETARCH as an argument to ubi9-python-3.12 Dockerfile
| --mount=type=cache,sharing=locked,id=dnf-ubi9,target=/var/cache/dnf \ | ||
| /bin/bash <<'EOF' | ||
| export TARGETARCH=$TARGETARCH | ||
| /mnt/aipcc.sh |
There was a problem hiding this comment.
@coderabbitai as expected, this fails on
+ dnf config-manager --set-enabled crb
Updating Subscription Management repositories.
Unable to read consumer identity
This system is not registered with an entitlement server. You can use subscription-manager to register.
Error: No matching repo to modify: crb.
subprocess exited with status 1
subprocess exited with status 1
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Look at the aipcc.sh script to understand what it's doing with the crb repository
cat base-images/utils/aipcc.sh
Length of output: 9069
🏁 Script executed:
#!/bin/bash
# Check if there are other Dockerfiles that might show how to handle UBI9 vs c9s differences
fd -e cpu -t f "Dockerfile" base-images/cpu
Length of output: 94
🏁 Script executed:
#!/bin/bash
# Look at the c9s equivalent to see if it handles this differently
cat base-images/cpu/c9s-python-3.12/Dockerfile.cpu 2>/dev/null || echo "File not found"
Length of output: 2395
🏁 Script executed:
#!/bin/bash
# Search for any existing handling of UBI9 vs c9s repository differences
rg -n "crb|CodeReady" base-images/
Length of output: 71
@jiridanek The issue is clear from the aipcc.sh script. The install_csb() function at line 343 tries to enable the crb (CodeReady Builder) repository, which exists in CentOS Stream 9 but not in UBI9.
The script assumes it's running on CentOS Stream 9 where the crb repository is available. UBI9 (Universal Base Image) has a different repository structure and doesn't include the CodeReady Builder repository.
Here are a few solutions:
Option 1: Detect the OS and conditionally enable crb
Modify aipcc.sh to check the OS type before enabling crb:
function install_csb() {
dnf install "${DNF_OPTS[@]}" dnf-plugins-core
# Only enable crb on CentOS Stream, not on UBI
if grep -q "CentOS Stream" /etc/os-release; then
dnf config-manager --set-enabled crb
fi
}Option 2: Check if the repository exists before enabling
function install_csb() {
dnf install "${DNF_OPTS[@]}" dnf-plugins-core
# Only enable crb if it exists
if dnf repolist --all | grep -q "^crb"; then
dnf config-manager --set-enabled crb
fi
}Option 3: Pass an environment variable from the Dockerfile
Set a variable in the ubi9 Dockerfile to indicate it's UBI9, and check it in the script.
The first option is probably the cleanest since it explicitly handles the different base images. Would you like me to prepare a patch for aipcc.sh with one of these approaches?
…`ubi9-python-3.12` Dockerfile The c9s PR did not trigger building the ubi9 version, so I missed on the fact that changes are needed there. It seems reasonable to remove the ubi9 base image as it is unused and only leave c9s. But I don't want to do it right now, first I will make it work and then make it go away.
jiridanek
force-pushed
the
jd_trgetarch_ubi9cpubase
branch
from
5bbfb95 to
33d3969
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
base-images/cpu/c9s-python-3.12/Dockerfile.cpu(1 hunks)base-images/cpu/ubi9-python-3.12/Dockerfile.cpu(1 hunks)base-images/utils/aipcc.sh(0 hunks)
💤 Files with no reviewable changes (1)
- base-images/utils/aipcc.sh
🚧 Files skipped from review as they are similar to previous changes (1)
- base-images/cpu/ubi9-python-3.12/Dockerfile.cpu
⏰ 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). (1)
- GitHub Check: Red Hat Konflux / odh-base-image-cpu-py312-ubi9-on-pull-request
🔇 Additional comments (1)
base-images/cpu/c9s-python-3.12/Dockerfile.cpu (1)
1-1: TARGETARCH propagation and crb enable addition are correct and justified.Verification confirms TARGETARCH is actively used by
/mnt/aipcc.sh(line 4:ARCH=${TARGETARCH}) and the double declaration (global line 1 and stage-level line 27) correctly follows Docker multi-stage build patterns. Thecrb enableaddition (line 69) is appropriately placed with explanatory comments for CentOS Stream 9 equivalence, replacing the previous responsibility from aipcc.sh (install_csb function no longer present). All architectural patterns align with build requirements.
| # AIPCC bases enable codeready-builder, so we need to do the CentOS equivalent | ||
| # In RHEL this is codeready-builder-for-rhel-${RELEASEVER_MAJOR}-${ARCH}-eus-rpms | ||
| # or codeready-builder-for-rhel-${RELEASEVER_MAJOR}-${ARCH}-rpms | ||
| /usr/bin/crb enable |
There was a problem hiding this comment.
🧩 Analysis chain
Clarify the shift of CRB enablement from script to Dockerfile.
The addition of /usr/bin/crb enable directly in the Dockerfile (lines 66-69) indicates that codeready-builder enablement has been moved from the aipcc.sh script (where install_csb() was presumably called). This is a functional shift that should be validated:
- Confirm that the
install_csb()function has been removed frombase-images/utils/aipcc.sh - Verify that calling
crb enabledirectly in the Dockerfile does not conflict with any invocations in the script - Ensure that this approach is consistent with the ubi9 variant (mentioned in the PR title but not shown in this review)
🏁 Script executed:
#!/bin/bash
# Description: Verify the removal of install_csb() and absence of conflicting crb enable calls
# Check if install_csb() exists in aipcc.sh
rg -n 'install_csb' base-images/utils/aipcc.sh
# Check if there are any remaining crb or codeready-builder references in aipcc.sh
rg -n -i 'crb|codeready' base-images/utils/aipcc.sh
# Verify that ubi9 Dockerfile also includes the same crb enable call
rg -n 'crb enable' base-images/cpu/ubi9-python-3.12/Dockerfile.cpuLength of output: 0
I need to gather more information about the repository structure and file contents since the initial search produced no output.
🏁 Script executed:
#!/bin/bash
# Check if files exist and get directory structure
echo "=== Checking directory structure ==="
find base-images -type f -name "aipcc.sh" 2>/dev/null || echo "aipcc.sh not found in base-images"
find base-images -type f -name "Dockerfile.cpu" 2>/dev/null | head -20
echo -e "\n=== Checking aipcc.sh exists and has content ==="
if [ -f "base-images/utils/aipcc.sh" ]; then
wc -l base-images/utils/aipcc.sh
echo "File exists"
else
echo "aipcc.sh not found at expected path"
fi
echo -e "\n=== Checking c9s Dockerfile exists ==="
if [ -f "base-images/cpu/c9s-python-3.12/Dockerfile.cpu" ]; then
echo "File exists"
else
echo "c9s Dockerfile not found"
fiLength of output: 303
🏁 Script executed:
#!/bin/bash
# View content of aipcc.sh to check for install_csb and crb references
echo "=== Searching aipcc.sh for install_ functions ==="
grep -n "^install_" base-images/utils/aipcc.sh | head -20
echo -e "\n=== Searching aipcc.sh for crb or codeready (case-insensitive) ==="
grep -n -i "crb\|codeready" base-images/utils/aipcc.sh || echo "No matches found"
echo -e "\n=== Viewing c9s Dockerfile (lines 60-75) ==="
sed -n '60,75p' base-images/cpu/c9s-python-3.12/Dockerfile.cpu
echo -e "\n=== Viewing ubi9 Dockerfile (lines 60-75) ==="
sed -n '60,75p' base-images/cpu/ubi9-python-3.12/Dockerfile.cpu
echo -e "\n=== Checking for any crb enable in aipcc.sh ==="
grep -n "crb enable" base-images/utils/aipcc.sh || echo "No crb enable found in aipcc.sh"Length of output: 796
🏁 Script executed:
#!/bin/bash
# Check if crb enable exists anywhere in ubi9 Dockerfile
echo "=== Searching ubi9 Dockerfile for crb enable ==="
grep -n "crb enable" base-images/cpu/ubi9-python-3.12/Dockerfile.cpu || echo "No crb enable found in ubi9"
echo -e "\n=== ubi9 Dockerfile total lines ==="
wc -l base-images/cpu/ubi9-python-3.12/Dockerfile.cpu
echo -e "\n=== Viewing full ubi9 Dockerfile ==="
cat base-images/cpu/ubi9-python-3.12/Dockerfile.cpuLength of output: 742
Address inconsistency: crb enable missing from ubi9 variant.
The verification confirms a significant inconsistency between the c9s and ubi9 variants:
- c9s Dockerfile (lines 66-69): Includes
/usr/bin/crb enablebefore calling/mnt/aipcc.sh - ubi9 Dockerfile (24 lines total): No
crb enablecall present
The original concerns were validated:
- ✓
install_csb()confirmed removed frombase-images/utils/aipcc.sh - ✓ No conflicting
crb enablecalls in the script - ✗ Inconsistency with ubi9: the functional shift was applied only to c9s
Either add /usr/bin/crb enable to the ubi9 Dockerfile (before calling /mnt/aipcc.sh) for consistency, or document why ubi9 differs from c9s.
🤖 Prompt for AI Agents
In base-images/cpu/ubi9-python-3.12/Dockerfile.cpu (the ubi9 variant, full file
~24 lines) the `/usr/bin/crb enable` call is missing compared to c9s (which runs
it before `/mnt/aipcc.sh` around lines 66-69); add a `/usr/bin/crb enable`
invocation immediately before the `/mnt/aipcc.sh` call in the ubi9 Dockerfile to
mirror c9s behavior, or if ubi9 must differ, add a short comment in the ubi9
Dockerfile documenting why the CRB enable step is intentionally omitted and
reference the c9s Dockerfile for context.
|
@jiridanek: The following test failed, say
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. |
1 participant
ppc64le(ands390xtodo later) architectures toodh-base-image-cpu-py312-c9s#2688s390xarchitecture to odh-base-image-cpu-py312-c9s #2690TODO
Description
The c9s PR did not trigger building the ubi9 version, so I missed on the fact that changes are needed there.
It seems reasonable to remove the ubi9 base image as it is unused and only leave c9s. But I don't want to do it right now, first I will make it work and then make it go away.
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
✏️ Tip: You can customize this high-level summary in your review settings.