ci: install python3-pip before awscurl/tox on minimal runners#11
Open
andybrown668 wants to merge 4 commits into
Open
ci: install python3-pip before awscurl/tox on minimal runners#11andybrown668 wants to merge 4 commits into
andybrown668 wants to merge 4 commits into
Conversation
Made-with: Cursor
There was a problem hiding this comment.
Pull request overview
This PR hardens RustFS CI and local S3 test tooling on minimal Linux runners where python3 is present but pip is missing, preventing awscurl/tox installation and breaking S3 test execution.
Changes:
- Install
python3-pipviaapt-getin GitHub Actions workflows before installingawscurlandtox. - Add a fallback in
scripts/s3-tests/run.shto installpython3-pipviaapt-getwhenpipis missing (andapt-get/sudoare available).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
scripts/s3-tests/run.sh |
Adds a runtime check to provision python3-pip on Debian/Ubuntu-like environments when pip is missing. |
.github/workflows/e2e-s3tests.yml |
Ensures python3-pip is installed on minimal runners before pip install awscurl tox in both single/multi jobs. |
.github/workflows/ci.yml |
Installs python3-pip before running implemented S3 tests to avoid No module named pip failures. |
Comment on lines
+726
to
+728
| # Debian/Ubuntu minimal images often have python3 but no pip (No module named pip). | ||
| if ! python3 -m pip --version >/dev/null 2>&1; then | ||
| if command -v apt-get >/dev/null 2>&1; then |
There was a problem hiding this comment.
The python3 -m pip --version check will also fail when python3 itself is missing, but the warning/error messages assume Python is present (“python3 has no pip”). Consider checking command -v python3 first (or distinguish exit codes) so the log guidance is accurate (e.g., install python3 vs install python3-pip).
Suggested change
| # Debian/Ubuntu minimal images often have python3 but no pip (No module named pip). | |
| if ! python3 -m pip --version >/dev/null 2>&1; then | |
| if command -v apt-get >/dev/null 2>&1; then | |
| # Distinguish between missing python3 and missing pip so remediation is accurate. | |
| if ! command -v python3 >/dev/null 2>&1; then | |
| if command -v apt-get >/dev/null 2>&1; then | |
| log_warn "python3 is missing; installing python3 and python3-pip via apt..." | |
| if [ "$(id -u)" -eq 0 ]; then | |
| apt-get update -qq | |
| DEBIAN_FRONTEND=noninteractive apt-get install -y python3 python3-pip || { | |
| log_error "Failed to install python3 and python3-pip via apt-get" | |
| return 1 | |
| } | |
| elif command -v sudo >/dev/null 2>&1; then | |
| sudo apt-get update -qq | |
| sudo DEBIAN_FRONTEND=noninteractive apt-get install -y python3 python3-pip || { | |
| log_error "Failed to install python3 and python3-pip via apt-get" | |
| return 1 | |
| } | |
| else | |
| log_error "python3 is missing. Install python3 and python3-pip, then retry." | |
| return 1 | |
| fi | |
| else | |
| log_error "python3 is missing. Install python3 and pip (e.g. python3-pip on Debian/Ubuntu), then retry." | |
| return 1 | |
| fi | |
| elif ! python3 -m pip --version >/dev/null 2>&1; then | |
| if command -v apt-get >/dev/null 2>&1; then |
Comment on lines
+731
to
+737
| apt-get update -qq | ||
| DEBIAN_FRONTEND=noninteractive apt-get install -y python3-pip || { | ||
| log_error "Failed to install python3-pip via apt-get" | ||
| return 1 | ||
| } | ||
| elif command -v sudo >/dev/null 2>&1; then | ||
| sudo apt-get update -qq |
There was a problem hiding this comment.
apt-get update -qq is executed under set -e, so if it fails the whole script will exit without emitting a log_error, which makes failures harder to diagnose. Consider wrapping apt-get update with an explicit || { log_error ...; return 1; } (or similar) so update failures produce a clear message like the install failure path does.
Suggested change
| apt-get update -qq | |
| DEBIAN_FRONTEND=noninteractive apt-get install -y python3-pip || { | |
| log_error "Failed to install python3-pip via apt-get" | |
| return 1 | |
| } | |
| elif command -v sudo >/dev/null 2>&1; then | |
| sudo apt-get update -qq | |
| apt-get update -qq || { | |
| log_error "Failed to update apt package index" | |
| return 1 | |
| } | |
| DEBIAN_FRONTEND=noninteractive apt-get install -y python3-pip || { | |
| log_error "Failed to install python3-pip via apt-get" | |
| return 1 | |
| } | |
| elif command -v sudo >/dev/null 2>&1; then | |
| sudo apt-get update -qq || { | |
| log_error "Failed to update apt package index" | |
| return 1 | |
| } |
Made-with: Cursor
Made-with: Cursor
Made-with: Cursor
| run: | | ||
| python3 -m pip install --user --upgrade pip awscurl tox | ||
| sudo apt-get update | ||
| sudo DEBIAN_FRONTEND=noninteractive apt-get install -y python3-pip |
| run: | | ||
| python3 -m pip install --user --upgrade pip awscurl tox | ||
| sudo apt-get update | ||
| sudo DEBIAN_FRONTEND=noninteractive apt-get install -y python3-pip |
Comment on lines
243
to
245
|
|
||
| - name: Make binary executable | ||
| run: chmod +x ./target/debug/rustfs |
| } | ||
| elif command -v sudo >/dev/null 2>&1; then | ||
| sudo apt-get update -qq | ||
| sudo DEBIAN_FRONTEND=noninteractive apt-get install -y python3-pip || { |
| python3 -m pip install --user --upgrade pip awscurl tox | ||
| sudo apt-get update | ||
| sudo DEBIAN_FRONTEND=noninteractive apt-get install -y python3-pip | ||
| python3 -m pip install --user --break-system-packages --upgrade pip awscurl tox |
Comment on lines
+249
to
+255
| # Ubuntu/Debian PEP 668 (externally-managed-environment) rejects plain --user installs; use | ||
| # --break-system-packages for CI-only user-site installs (same fallback as scripts/s3-tests/run.sh). | ||
| - name: Install Python tools | ||
| run: | | ||
| sudo apt-get update | ||
| sudo DEBIAN_FRONTEND=noninteractive apt-get install -y python3-pip | ||
| python3 -m pip install --user --break-system-packages --upgrade pip awscurl tox |
Comment on lines
+200
to
+203
| run: | | ||
| sudo apt-get update | ||
| sudo DEBIAN_FRONTEND=noninteractive apt-get install -y \ | ||
| build-essential \ |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Type of Change
Related Issues
N/A
Summary of Changes
Minimal CI images (e.g. Ubicloud) ship
python3withoutpip, which breakspython3 -m pip install awscurland caused S3 Implemented Tests to fail withNo module named pip.python3-pipvia apt in Install Python tools steps in.github/workflows/ci.ymland.github/workflows/e2e-s3tests.yml, then installawscurlandtoxwith pip as before.scripts/s3-tests/run.sh, if pip is missing, installpython3-pipvia apt whenapt-getandsudoare available so local runs match CI behavior.Checklist
make pre-commitVerification
Impact
Additional Notes
N/A
Thank you for your contribution! Please ensure your PR follows the community standards (CODE_OF_CONDUCT.md). If this is your first contribution, review the CLA document and sign it by commenting
I have read and agree to the CLA.on the PR.