WIP: OKD-322: Update install.sh to work on CentOS/RHEL 10#62
WIP: OKD-322: Update install.sh to work on CentOS/RHEL 10#62jatinsu wants to merge 1 commit intoopenshift:mainfrom
Conversation
Signed-off-by: jatinsu <jsuri@redhat.com>
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
WalkthroughUpdated the PIP_OPTS configuration in the Ansible installation script to include the Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: jatinsu 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 |
|
@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: The following tests 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. |
2 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