OKD-322: Update install.sh to work on CentOS/RHEL 10#62
OKD-322: Update install.sh to work on CentOS/RHEL 10#62jatinsu wants to merge 1 commit intoopenshift:mainfrom
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@jatinsu: This pull request references OKD-322 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.22.0" version, but no target version was set. DetailsIn response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
openshift-ci
bot
added
the
do-not-merge/work-in-progress
WalkthroughAdds a conditional post-bootstrapping cleanup to Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes 🚥 Pre-merge checks | ✅ 9 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (9 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
@jatinsu: This pull request references OKD-322 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.22.0" version, but no target version was set. DetailsIn response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
|
@jatinsu: This pull request references OKD-322 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.22.0" version, but no target version was set. DetailsIn response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
openshift/install-ansible.sh (1)
15-15:⚠️ Potential issue | 🟡 Minor
pip --upgrade pipon line 15 doesn't usePIP_OPTSIf the same RHEL 10 constraint applies to pip itself (RPM-managed or PEP 668-flagged), this invocation will fail before
PIP_OPTSis ever used. If--ignore-installed(and potentially--break-system-packages) are necessary for subsequent installs, they're likely needed here too.🔧 Proposed fix
-python3 -m pip install --upgrade pip +python3 -m pip install ${PIP_OPTS} --upgrade pip🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openshift/install-ansible.sh` at line 15, Replace the plain upgrade command "python3 -m pip install --upgrade pip" with a call that honors PIP_OPTS and includes the same installer flags used later (e.g., --ignore-installed and, if required for RHEL/PEP 668 cases, --break-system-packages) so pip itself can be upgraded under the same constraints; in other words, invoke pip as "python3 -m pip install --upgrade pip $PIP_OPTS" and add the same --ignore-installed/--break-system-packages options as used for subsequent installs to ensure the upgrade won't fail in RPM/PEP 668 environments.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@openshift/install-ansible.sh`:
- Line 5: The script's PIP_OPTS only contains --ignore-installed which doesn't
address PEP 668 errors on RHEL 10; update the PIP_OPTS variable (symbol:
PIP_OPTS) to include --break-system-packages (or change to using a virtual
environment) and ensure every pip invocation uses PIP_OPTS (including the pip
upgrade call invoked with python3 -m pip install --upgrade pip) so pip won't
fail with error: externally-managed-environment.
---
Outside diff comments:
In `@openshift/install-ansible.sh`:
- Line 15: Replace the plain upgrade command "python3 -m pip install --upgrade
pip" with a call that honors PIP_OPTS and includes the same installer flags used
later (e.g., --ignore-installed and, if required for RHEL/PEP 668 cases,
--break-system-packages) so pip itself can be upgraded under the same
constraints; in other words, invoke pip as "python3 -m pip install --upgrade pip
$PIP_OPTS" and add the same --ignore-installed/--break-system-packages options
as used for subsequent installs to ensure the upgrade won't fail in RPM/PEP 668
environments.
ℹ️ Review info
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (1)
openshift/install-ansible.sh
| set -euo pipefail | ||
|
|
||
| PIP_OPTS="--no-cache-dir" | ||
| PIP_OPTS="--no-cache-dir --ignore-installed" |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
RHEL 10 CentOS 10 Python version 3.11 PEP 668 EXTERNALLY-MANAGED
💡 Result:
-
RHEL 10 / CentOS Stream 10 “Python version”: the system/default
python3is Python 3.12 (RHEL 10 docs explicitly say 3.12 is the default; CentOS Stream 10’spython3RPMs are 3.12.x). [1], [2] -
Python 3.11 availability on RHEL 10: RHEL 10 supports installing additional parallel Python versions from AppStream in minor releases, and RHEL 10 package listings include many
python3.11-*packages (indicating a Python 3.11 stack exists alongside 3.12). [1], [3] -
PEP 668 /
EXTERNALLY-MANAGEDand pip: PEP 668 standardizes anEXTERNALLY-MANAGEDmarker file in the stdlib directory. When present, pip should refuse to install into the system environment outside a venv and instead push you to venv/pipx (unless you override). [4], [5]- Practical outcome: on distros that ship this marker,
pip install ...against the system/usr/bin/python3commonly errors withexternally-managed-environment; the intended fixes are:- use a virtual environment:
python3 -m venv .venv && . .venv/bin/activate && pip install ... - or use pipx for apps
- or install distro RPMs via dnf when available
- last resort:
pip install --break-system-packages ...(explicitly “risky” per the spec). [4], [5]
- use a virtual environment:
- Practical outcome: on distros that ship this marker,
Sources:
[1] RHEL 10 docs: default Python is 3.12; additional parallel versions via AppStream. (docs.redhat.com)
[2] CentOS Stream 10 python3 RPM provides python3.12 / versioned 3.12.x packages. (rpmfind.net)
[3] RHEL 10 package-change appendix lists many python3.11-* packages. (docs.redhat.com)
[4] PEP 668 definition/behavior of EXTERNALLY-MANAGED and --break-system-packages. (peps.python.org)
[5] PyPA spec for “Externally Managed Environments” (PEP 668 marker file behavior). (packaging.python.org)
Citations:
- 1: https://docs.redhat.com/en/documentation/red_hat_enterprise_linux/10/html/installing_and_using_dynamic_programming_languages/installing-and-using-python?utm_source=openai
- 2: https://rpmfind.net/linux/RPM/centos-stream/10/baseos/x86_64/python3-3.12.5-2.el10.x86_64.html?utm_source=openai
- 3: https://docs.redhat.com/en/documentation/red_hat_enterprise_linux/10/html/considerations_in_adopting_rhel_10/changes-to-packages?utm_source=openai
- 4: https://peps.python.org/pep-0668/?utm_source=openai
- 5: https://packaging.python.org/en/latest/specifications/externally-managed-environments.html?utm_source=openai
🏁 Script executed:
# Let's also check if there's any context in the repository about the actual failure or RHEL 10 requirements
cd /root && git ls-files | grep -E "(install-ansible|Dockerfile|RHEL|CentOS)" | head -20Repository: openshift/ansible-operator-plugins
Length of output: 133
🏁 Script executed:
find . -name "install-ansible.sh" -o -name "*install*ansible*" | head -10Repository: openshift/ansible-operator-plugins
Length of output: 108
🏁 Script executed:
# Search for the file in the repository
fd "install-ansible" --type fRepository: openshift/ansible-operator-plugins
Length of output: 106
🏁 Script executed:
# Try to locate and examine the full context of install-ansible.sh
git ls-files | grep -i ansible | head -20Repository: openshift/ansible-operator-plugins
Length of output: 1246
🏁 Script executed:
cat -n ./openshift/install-ansible.shRepository: openshift/ansible-operator-plugins
Length of output: 1201
🏁 Script executed:
# Check for any Dockerfile or build context that uses this script
git log --oneline -- openshift/install-ansible.sh | head -5Repository: openshift/ansible-operator-plugins
Length of output: 129
🏁 Script executed:
# Check the actual commit to see what changed and what problem it was solving
git show c9d396d --statRepository: openshift/ansible-operator-plugins
Length of output: 50391
🏁 Script executed:
# Get more details about what changed in the commit
git show c9d396dRepository: openshift/ansible-operator-plugins
Length of output: 50393
--ignore-installed is insufficient for RHEL 10 with PEP 668-compliant Python 3.12
On RHEL 10, the default Python is 3.12 (and Python 3.11 is available via AppStream). Both versions enforce PEP 668, which marks the system environment as EXTERNALLY-MANAGED. This creates two distinct pip failure modes, requiring different solutions:
| Problem | Failure | Fix |
|---|---|---|
| pip attempts to uninstall an RPM-managed package | Package removal fails before reinstall | --ignore-installed |
| PEP 668 blocks pip installs to system environment | error: externally-managed-environment |
--break-system-packages |
Adding --ignore-installed alone addresses the first failure mode but does not resolve PEP 668 enforcement. If the original build failure on RHEL 10 was externally-managed-environment, the script will still fail even with this change. Either add --break-system-packages to PIP_OPTS or pivot to a virtual environment for a more robust solution.
Additionally, line 15 (python3 -m pip install --upgrade pip) does not use PIP_OPTS and could also fail on PEP 668-compliant systems if pip itself is system-managed.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@openshift/install-ansible.sh` at line 5, The script's PIP_OPTS only contains
--ignore-installed which doesn't address PEP 668 errors on RHEL 10; update the
PIP_OPTS variable (symbol: PIP_OPTS) to include --break-system-packages (or
change to using a virtual environment) and ensure every pip invocation uses
PIP_OPTS (including the pip upgrade call invoked with python3 -m pip install
--upgrade pip) so pip won't fail with error: externally-managed-environment.
jatinsu
changed the title
openshift-ci
bot
removed
the
do-not-merge/work-in-progress
jatinsu
changed the title
|
Ongoing Slack Thread |
chiragkyal
left a comment
chiragkyal
left a comment
There was a problem hiding this comment.
- What is the version mismatch between pip and RPM?
- Is the build and e2e going through fine with this fix? A test build is required from ART team to confirm.
- To my understanding pip gives better control over versioning than RPM. Because there is a complex dependency chain that we need to fulfil. If you see, this six version is required for certain other Python packages; otherwise, there will be issues in building this image in an isolated environment.
ansible-operator-plugins/openshift/requirements.txt
Lines 101 to 105 in d85e7f6
|
@jatinsu: This pull request references OKD-322 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.22.0" version, but no target version was set. DetailsIn response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
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 `@openshift/Dockerfile`:
- Around line 31-32: The RUN chain currently ends with "python3 -m ensurepip
--upgrade && dnf remove -y python3-six || true", where the "|| true" can mask
failures from earlier commands like "python3 -m ensurepip --upgrade"; isolate
the fallback to only the remove step by grouping the removal (for example with a
subshell or brace group) so that only "dnf remove -y python3-six" is allowed to
fail silently (e.g., replace the trailing segment with a grouped form such as
"(dnf remove -y python3-six || true)" or "{ dnf remove -y python3-six || true;
}" so failures in "python3 -m ensurepip --upgrade" still fail the build).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3319b4ae-d313-4c2a-9ea9-a9938bf6dbba
📒 Files selected for processing (1)
openshift/Dockerfile
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 `@openshift/Dockerfile`:
- Line 31: The brace group "{dnf remove -y python3-six || true; }" is missing a
space after the opening "{", causing a shell syntax error; fix it by adding a
space so the compound command reads with "{ dnf remove -y python3-six || true;
}" (ensure the existing semicolon before the closing "}" remains) to make the
shell recognize the brace group in the Dockerfile.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9056312d-1957-46a6-bc77-a1ad7f5acbaf
📒 Files selected for processing (1)
openshift/Dockerfile
|
/retest-required |
|
@jatinsu: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions 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. |
Signed-off-by: jatinsu <[email protected]>
|
@jatinsu: This pull request references OKD-322 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
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 `@openshift/Dockerfile`:
- Around line 30-31: The RUN layer that ends with "python3 -m ensurepip
--upgrade" and "{ dnf remove -y python3-six || true; }" leaves dnf cache in the
image; update that same RUN chain to perform dnf cache cleanup in the same layer
by adding the equivalent of a dnf clean step and removal of /var/cache/dnf
(e.g., run dnf clean all and rm -rf /var/cache/dnf) after the dnf remove command
so no package manager cache persists in the image.
🪄 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: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: ab1a28b1-c473-4acd-9fa2-564da8bb9578
📒 Files selected for processing (1)
openshift/Dockerfile
| && pushd /usr/local/bin && ln -sf ../../bin/python3.12 python3 && popd \ | ||
| && python3 -m ensurepip --upgrade | ||
| && python3 -m ensurepip --upgrade \ | ||
| && { dnf remove -y python3-six || true; } |
There was a problem hiding this comment.
nit: For future reference, please add a comment explaining why we're removing this. Also, please update the PR description with the correct details.
openshift-ci
bot
added
do-not-merge/hold
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: chiragkyal, jatinsu The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
the
approved
3 participants
Description of the change:
Appending
--ignore-installedto the PIP_OPTS variable in install.sh so pip will install packages without attempting to uninstall RPM-provided packagesMotivation for the change:
In CentOS/RHEL 10 the package
python3-sixis already provided. Pip attempts to uninstall it but it can't as it's a rpm package and it causes a build failure. The way I am testing this is by replacing the base image found here with a CentOS 10 image.Summary by CodeRabbit