feat(pde2e-image): add docker and podman-compose install scripts for RHEL#581
feat(pde2e-image): add docker and podman-compose install scripts for RHEL#581amisskii wants to merge 1 commit into
Conversation
…RHEL Add CLI preinstallation scripts for RHEL E2E testing: - install_docker.sh (RHEL-specific): Docker Engine via official repo - install_podman_compose.sh (shared unix): podman-compose via dnf/pip Update Containerfile to copy shared unix scripts into the RHEL scripts directory so execute_scripts can find them. Signed-off-by: Anton Misskii <amisskii@redhat.com> Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughTwo new Bash installer scripts are added: ChangesRHEL installer scripts and image wiring
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Warning Review ran into problems🔥 ProblemsStopped waiting for pipeline failures after 30000ms. One of your pipelines takes longer than our 30000ms fetch window to run, so review may not consider pipeline-failure results for inline comments if any failures occurred after the fetch window. Increase the timeout if you want to wait longer or run a 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 |
![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
🧹 Nitpick comments (2)
pde2e-image/lib/unix/scripts/install_podman_compose.sh (1)
4-16: 💤 Low valueConsider verifying podman prerequisite.
The script installs
podman-composebut doesn't verify thatpodmanitself is installed. While this may be a reasonable assumption for the E2E pipeline, adding a prerequisite check would make the script more robust and provide clearer error messages.🛡️ Optional prerequisite check
#!/bin/bash set -e +# Verify podman is installed +if ! command -v podman &> /dev/null; then + echo "Error: podman is not installed. Please install podman first." + exit 1 +fi + echo "Installing podman-compose..."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pde2e-image/lib/unix/scripts/install_podman_compose.sh` around lines 4 - 16, The install_podman_compose.sh script installs podman-compose without first verifying that the podman prerequisite is installed, which could lead to failures with unclear error messages. Add a prerequisite check near the beginning of the script that verifies podman is installed (using a command like `command -v podman` or `which podman`), and exit with an informative error message if podman is not found. This check should occur before attempting to install podman-compose to ensure dependencies are satisfied and provide users with clear guidance if the prerequisite is missing.pde2e-image/lib/rhel/scripts/install_docker.sh (1)
1-22: ⚡ Quick winConsider making the script idempotent.
The script will fail if Docker is already installed due to
set -eand dnf errors on reinstallation attempts. For E2E environments where scripts may run multiple times, consider adding checks:if ! command -v docker &> /dev/null; then echo "Installing Docker Engine on RHEL..." # ... installation steps else echo "Docker is already installed." docker --version exit 0 fiAlternatively, use
dnf install -y --skip-brokenordnf install -y || trueif partial failures are acceptable.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pde2e-image/lib/rhel/scripts/install_docker.sh` around lines 1 - 22, The install_docker.sh script will fail if Docker is already installed due to the set -e flag causing the script to exit on dnf errors during reinstallation attempts. Add an idempotency check at the beginning of the script using command -v docker to detect if Docker is already installed. If Docker exists, display the version and exit cleanly. If Docker does not exist, proceed with the dnf and systemctl installation steps as currently written. Alternatively, modify the dnf install commands to use --skip-broken flag or append || true to handle partial failures gracefully instead of causing the script to exit.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pde2e-image/lib/unix/scripts/install_podman_compose.sh`:
- Around line 11-12: The PATH export on line 12 only affects the current script
session and does not persist after the script exits, making podman-compose
unavailable in the parent shell. To fix this, add logic after the pip3 install
command to permanently update the user's shell configuration files (such as
.bashrc, .bash_profile, or .zshrc) to include $HOME/.local/bin in their PATH.
Alternatively, if persistence is not feasible, document in comments or provide
clear user guidance that manual PATH configuration is required after
installation.
---
Nitpick comments:
In `@pde2e-image/lib/rhel/scripts/install_docker.sh`:
- Around line 1-22: The install_docker.sh script will fail if Docker is already
installed due to the set -e flag causing the script to exit on dnf errors during
reinstallation attempts. Add an idempotency check at the beginning of the script
using command -v docker to detect if Docker is already installed. If Docker
exists, display the version and exit cleanly. If Docker does not exist, proceed
with the dnf and systemctl installation steps as currently written.
Alternatively, modify the dnf install commands to use --skip-broken flag or
append || true to handle partial failures gracefully instead of causing the
script to exit.
In `@pde2e-image/lib/unix/scripts/install_podman_compose.sh`:
- Around line 4-16: The install_podman_compose.sh script installs podman-compose
without first verifying that the podman prerequisite is installed, which could
lead to failures with unclear error messages. Add a prerequisite check near the
beginning of the script that verifies podman is installed (using a command like
`command -v podman` or `which podman`), and exit with an informative error
message if podman is not found. This check should occur before attempting to
install podman-compose to ensure dependencies are satisfied and provide users
with clear guidance if the prerequisite is missing.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4f84239f-3943-4300-95c3-07336fd20031
📒 Files selected for processing (3)
pde2e-image/Containerfilepde2e-image/lib/rhel/scripts/install_docker.shpde2e-image/lib/unix/scripts/install_podman_compose.sh
| pip3 install --user podman-compose | ||
| export PATH="$HOME/.local/bin:$PATH" |
There was a problem hiding this comment.
PATH export doesn't persist after script exits.
Line 12 exports PATH to include $HOME/.local/bin, but this only affects the current script session. When the script exits, the parent shell's PATH remains unchanged, making podman-compose unavailable despite successful installation via pip.
The version check at line 15 succeeds within the script's environment, masking this issue.
🔧 Proposed fix to persist PATH changes
Add persistence logic after the pip installation:
echo "dnf installation failed, falling back to pip..."
sudo dnf install -y python3-pip
pip3 install --user podman-compose
- export PATH="$HOME/.local/bin:$PATH"
+
+ # Persist PATH for future sessions
+ if ! grep -q '$HOME/.local/bin' "$HOME/.bashrc" 2>/dev/null; then
+ echo 'export PATH="$HOME/.local/bin:$PATH"' >> "$HOME/.bashrc"
+ echo "Added $HOME/.local/bin to PATH in ~/.bashrc"
+ fi
+
+ # Update PATH for current and subsequent commands in this script
+ export PATH="$HOME/.local/bin:$PATH"
fiAlternatively, document that users must manually add $HOME/.local/bin to their PATH after installation.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| pip3 install --user podman-compose | |
| export PATH="$HOME/.local/bin:$PATH" | |
| echo "dnf installation failed, falling back to pip..." | |
| sudo dnf install -y python3-pip | |
| pip3 install --user podman-compose | |
| # Persist PATH for future sessions | |
| if ! grep -q '$HOME/.local/bin' "$HOME/.bashrc" 2>/dev/null; then | |
| echo 'export PATH="$HOME/.local/bin:$PATH"' >> "$HOME/.bashrc" | |
| echo "Added $HOME/.local/bin to PATH in ~/.bashrc" | |
| fi | |
| # Update PATH for current and subsequent commands in this script | |
| export PATH="$HOME/.local/bin:$PATH" | |
| fi |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pde2e-image/lib/unix/scripts/install_podman_compose.sh` around lines 11 - 12,
The PATH export on line 12 only affects the current script session and does not
persist after the script exits, making podman-compose unavailable in the parent
shell. To fix this, add logic after the pip3 install command to permanently
update the user's shell configuration files (such as .bashrc, .bash_profile, or
.zshrc) to include $HOME/.local/bin in their PATH. Alternatively, if persistence
is not feasible, document in comments or provide clear user guidance that manual
PATH configuration is required after installation.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pde2e-image/tkn/task.yaml`:
- Line 162: The image field at line 162 is hard-coded to the RHEL version, but
the task defines and accepts params.os and params.image-version parameters that
are intended to support multiple operating systems (windows, darwin, rhel).
Replace the hard-coded image reference with a dynamic value that properly uses
the params.os and params.image-version values to construct the correct image URL
for the requested OS, ensuring the actual image used respects the OS/version
contract established by the task parameters and prevents non-RHEL callers from
silently receiving the wrong 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 464f99bf-c45a-48f3-a676-c3ce639b2873
📒 Files selected for processing (1)
pde2e-image/tkn/task.yaml
| steps: | ||
| - name: e2e-with-podman-setup | ||
| image: quay.io/odockal/pde2e-image:v$(params.image-version)-$(params.os) | ||
| image: quay.io/rh_ee_amisskii/pde2e-image:v0.1.1-rhel |
There was a problem hiding this comment.
Hard-coded image breaks the task’s OS/version contract.
Line 162 pins the step to ...:v0.1.1-rhel, but this task still accepts params.os/params.image-version and contains windows/darwin/rhel execution branches. That makes non-RHEL callers silently run against the wrong image and breaks existing TaskRun expectations.
Suggested fix
- image: quay.io/rh_ee_amisskii/pde2e-image:v0.1.1-rhel
+ image: quay.io/rh_ee_amisskii/pde2e-image:v$(params.image-version)-$(params.os)As per coding guidelines, cross-file and configuration contract breaks should be validated against actual configuration values and caller contracts.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| image: quay.io/rh_ee_amisskii/pde2e-image:v0.1.1-rhel | |
| image: quay.io/rh_ee_amisskii/pde2e-image:v$(params.image-version)-$(params.os) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pde2e-image/tkn/task.yaml` at line 162, The image field at line 162 is
hard-coded to the RHEL version, but the task defines and accepts params.os and
params.image-version parameters that are intended to support multiple operating
systems (windows, darwin, rhel). Replace the hard-coded image reference with a
dynamic value that properly uses the params.os and params.image-version values
to construct the correct image URL for the requested OS, ensuring the actual
image used respects the OS/version contract established by the task parameters
and prevents non-RHEL callers from silently receiving the wrong image.
amisskii
force-pushed
the
add-cli-preinstallation-scripts
branch
from
1767f88 to
e52cb0a
Compare
1 participant
Summary
install_docker.shscript (RHEL-specific) to install Docker Engine from official Docker CE repoinstall_podman_compose.shscript (shared unix) to install podman-compose via dnf with pip fallbackIssue
https://github.com/containers/podman-desktop-internal/issues/963
Usage
Invoke via
--scriptPaths "install_docker.sh,install_podman_compose.sh"in the Tekton pipeline.🤖 Generated with Claude Code