-
-
Notifications
You must be signed in to change notification settings - Fork 308
s390x docker rebuild script improvements #1846
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughThe script Changes
Sequence DiagramsequenceDiagram
participant Script as actions-runner-rebuild.sh
participant System as System Services
participant Podman as Podman Container Engine
alt Dockerfile Exists
Script->>Script: Set MODE to 1
Script->>System: Stop actions-runner service
Script->>Podman: Remove existing container
Script->>Podman: Build new image
Script->>Podman: Create new container
else Dockerfile Missing
Script->>Script: Set MODE to 2
Script->>Script: Create temporary directory
Script->>Script: Download Dockerfile and files
Script->>System: Stop actions-runner service
Script->>Podman: Remove existing container
Script->>Podman: Build new image
Script->>Podman: Create new container
Script->>Script: Clean up temporary files
end
Script->>Podman: Prune unused images
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
arch/s390/self-hosted-builder/actions-runner-rebuild.sh(1 hunks)
🔇 Additional comments (3)
arch/s390/self-hosted-builder/actions-runner-rebuild.sh (3)
4-12:⚠️ Potential issueAdd error handling and improve security for file downloads
The current implementation has several potential issues:
- Hardcoded references to the 'develop' branch
- No validation of downloaded files
- Unsafe directory handling
- No error handling for wget commands
Consider applying these improvements:
if [ -f actions-runner.Dockerfile ]; then MODE=1 else MODE=2 - TMPDIR="$(mktemp -d)" - cd $TMPDIR - wget https://raw.githubusercontent.com/zlib-ng/zlib-ng/refs/heads/develop/arch/s390/self-hosted-builder/actions-runner.Dockerfile - wget https://raw.githubusercontent.com/zlib-ng/zlib-ng/refs/heads/develop/arch/s390/self-hosted-builder/actions-runner - wget https://raw.githubusercontent.com/zlib-ng/zlib-ng/refs/heads/develop/arch/s390/self-hosted-builder/entrypoint + ORIGINAL_DIR="$(pwd)" + TMPDIR="$(mktemp -d)" || exit 1 + cd "${TMPDIR}" || exit 1 + + # Define base URL and files to download + BASE_URL="https://raw.githubusercontent.com/zlib-ng/zlib-ng/refs/heads/develop/arch/s390/self-hosted-builder" + FILES=("actions-runner.Dockerfile" "actions-runner" "entrypoint") + + # Download and verify each file + for file in "${FILES[@]}"; do + if ! wget -q --show-progress "${BASE_URL}/${file}"; then + echo "Failed to download ${file}" >&2 + cd "${ORIGINAL_DIR}" + rm -rf "${TMPDIR}" + exit 1 + fi + + # Basic file validation + if [ ! -s "${file}" ]; then + echo "Downloaded file ${file} is empty" >&2 + cd "${ORIGINAL_DIR}" + rm -rf "${TMPDIR}" + exit 1 + fi + done fiConsider implementing a more robust solution:
- Use a specific git ref or commit hash instead of a branch name
- Implement checksum verification for downloaded files
- Consider using
curlwith proper SSL verification instead ofwget- Store the files locally and update them through a controlled process instead of downloading them during runtime
Let's verify the branch and file existence:
28-41: 🛠️ Refactor suggestionImprove service restart and cleanup reliability
The current implementation lacks proper service verification and has potentially unsafe cleanup operations.
Consider applying these improvements:
-systemctl start actions-runner || true +# Start service with verification +if ! systemctl start actions-runner; then + echo "Error: Failed to start actions-runner service" >&2 + exit 1 +fi + +# Verify service is running +if ! timeout 30s bash -c 'until systemctl is-active --quiet actions-runner; do sleep 1; done'; then + echo "Error: Service failed to start within 30 seconds" >&2 + exit 1 +fi -podman image prune -af || true +# Only prune images older than 24h to avoid removing recently used images +podman image prune -f --filter "until=24h" || echo "Warning: Image pruning failed" >&2 # Clean up tempfile if [ "$MODE" == "2" ] ; then - cd $TMPDIR - rm actions-runner.Dockerfile - rm actions-runner - rm entrypoint - cd .. - rmdir $TMPDIR + if [ -d "${TMPDIR}" ]; then + cd "${ORIGINAL_DIR}" || exit 1 + rm -rf "${TMPDIR}" + fi echo "Deleted tempfiles." fiConsider implementing:
- Service health check after start
- More selective image pruning strategy
- Logging of cleanup operations
- Proper error handling for all operations
Let's verify the service configuration:
1-41: Consider additional operational improvements for production reliabilitySince this script runs under cron and manages critical infrastructure, consider these additional improvements:
- Add comprehensive logging:
+# Add to start of script +exec 1> >(logger -s -t $(basename $0)) 2>&1
- Add script locking to prevent concurrent runs:
+# Add after shebang +LOCK_FILE="/var/run/actions-runner-rebuild.lock" +if ! mkdir "${LOCK_FILE}" 2>/dev/null; then + echo "Script is already running" >&2 + exit 1 +fi +trap 'rm -rf "${LOCK_FILE}"' EXITConsider implementing:
- Monitoring integration (e.g., sending status to a monitoring system)
- Backup mechanism for critical files and configurations
- Automated testing of the rebuild process in a staging environment
- Documentation of recovery procedures
- Version control for downloaded files instead of using raw GitHub URLs
Let's verify the cron configuration:
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #1846 +/- ##
===========================================
- Coverage 82.44% 74.84% -7.60%
===========================================
Files 138 127 -11
Lines 10994 8011 -2983
Branches 2778 2141 -637
===========================================
- Hits 9064 5996 -3068
+ Misses 1239 993 -246
- Partials 691 1022 +331 ☔ View full report in Codecov by Sentry. |
ab1d728 to
7bd821e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
arch/s390/self-hosted-builder/actions-runner.Dockerfile (1)
41-42: LGTM! The permission changes enhance security and reliability.The addition of
--chmod=555ensures consistent and secure permissions for the scripts, preventing modification while allowing execution by any user. This is particularly important for unattended operation.Consider these additional security improvements:
- Add explicit version tags for the base image (e.g.,
almalinux:9.3) to ensure reproducible builds- Add health checks to monitor the runner's status:
+HEALTHCHECK --interval=5m --timeout=3s \ + CMD pgrep -f actions-runner || exit 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
arch/s390/self-hosted-builder/actions-runner-rebuild.sh(1 hunks)arch/s390/self-hosted-builder/actions-runner.Dockerfile(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- arch/s390/self-hosted-builder/actions-runner-rebuild.sh
🧰 Additional context used
📓 Learnings (1)
📓 Common learnings
Learnt from: Dead2
PR: zlib-ng/zlib-ng#1846
File: arch/s390/self-hosted-builder/actions-runner-rebuild.sh:16-19
Timestamp: 2025-01-05T11:57:38.357Z
Learning: It is acceptable to remove the podman container without checking for its existence in actions-runner-rebuild.sh.
Learnt from: Dead2
PR: zlib-ng/zlib-ng#1846
File: arch/s390/self-hosted-builder/actions-runner-rebuild.sh:22-25
Timestamp: 2025-01-05T12:02:00.645Z
Learning: User prefers overwriting logs in the self-hosted builder script rather than rotating them, due to an automated environment. They also do not want the script to exit on errors because no human actively monitors the container build process.
⏰ Context from checks skipped due to timeout of 90000ms (167)
- GitHub Check: macOS Clang Native Instructions (ARM64)
- GitHub Check: macOS GCC UBSAN
- GitHub Check: macOS Clang ASAN
- GitHub Check: Windows GCC Compat No Opt
- GitHub Check: Windows GCC Native Instructions (AVX)
- GitHub Check: Windows GCC
- GitHub Check: Windows ClangCl Win64 Native Instructions (AVX)
- GitHub Check: Windows ClangCl Win64
- GitHub Check: Windows ClangCl Win32
- GitHub Check: Windows MSVC 2019 v140 Win64
- GitHub Check: Windows MSVC 2019 v140 Win32
- GitHub Check: Windows MSVC 2019 v141 Win32
- GitHub Check: Windows MSVC 2022 v142 Win64
- GitHub Check: Windows MSVC 2022 v142 Win32
- GitHub Check: Ubuntu MinGW i686
- GitHub Check: Ubuntu 20.04 Clang
- GitHub Check: Ubuntu MinGW i686
- GitHub Check: macOS Clang Native Instructions (ARM64)
- GitHub Check: macOS GCC UBSAN
- GitHub Check: macOS Clang ASAN
- GitHub Check: Windows GCC Compat No Opt
- GitHub Check: Windows GCC Native Instructions (AVX)
- GitHub Check: Windows GCC
- GitHub Check: Windows ClangCl Win64 Native Instructions (AVX)
- GitHub Check: Windows ClangCl Win64
- GitHub Check: Windows ClangCl Win32
- GitHub Check: Windows MSVC 2019 v140 Win64
- GitHub Check: Windows MSVC 2019 v140 Win32
- GitHub Check: Windows MSVC 2019 v141 Win32
- GitHub Check: Windows MSVC 2022 v142 Win64
- GitHub Check: Windows MSVC 2022 v142 Win32
- GitHub Check: Ubuntu MinGW i686
- GitHub Check: Ubuntu 20.04 Clang
- GitHub Check: Ubuntu MinGW i686
- GitHub Check: macOS Clang Native Instructions (ARM64)
- GitHub Check: macOS GCC UBSAN
- GitHub Check: macOS Clang ASAN
- GitHub Check: Windows GCC Compat No Opt
- GitHub Check: Windows GCC Native Instructions (AVX)
- GitHub Check: Windows GCC
- GitHub Check: Windows ClangCl Win64 Native Instructions (AVX)
- GitHub Check: Windows ClangCl Win64
- GitHub Check: Windows ClangCl Win32
- GitHub Check: Windows MSVC 2019 v140 Win64
- GitHub Check: Windows MSVC 2019 v140 Win32
- GitHub Check: Windows MSVC 2019 v141 Win32
- GitHub Check: Windows MSVC 2022 v142 Win64
- GitHub Check: Windows MSVC 2022 v142 Win32
- GitHub Check: Ubuntu MinGW i686
- GitHub Check: Ubuntu 20.04 Clang
- GitHub Check: Ubuntu MinGW i686
- GitHub Check: macOS Clang Native Instructions (ARM64)
- GitHub Check: macOS GCC UBSAN
- GitHub Check: macOS Clang ASAN
- GitHub Check: Windows GCC Compat No Opt
- GitHub Check: Windows GCC Native Instructions (AVX)
- GitHub Check: Windows GCC
- GitHub Check: Windows ClangCl Win64 Native Instructions (AVX)
- GitHub Check: Windows ClangCl Win64
- GitHub Check: Windows ClangCl Win32
- GitHub Check: Windows MSVC 2019 v140 Win64
- GitHub Check: Windows MSVC 2019 v140 Win32
- GitHub Check: Windows MSVC 2019 v141 Win32
- GitHub Check: Windows MSVC 2022 v142 Win64
- GitHub Check: Windows MSVC 2022 v142 Win32
- GitHub Check: Ubuntu MinGW i686
- GitHub Check: Ubuntu 20.04 Clang
- GitHub Check: Ubuntu MinGW i686
- GitHub Check: macOS Clang Native Instructions (ARM64)
- GitHub Check: macOS GCC UBSAN
- GitHub Check: macOS Clang ASAN
- GitHub Check: Windows GCC Compat No Opt
- GitHub Check: Windows GCC Native Instructions (AVX)
- GitHub Check: Windows GCC
- GitHub Check: Windows ClangCl Win64 Native Instructions (AVX)
- GitHub Check: Windows ClangCl Win64
- GitHub Check: Windows ClangCl Win32
- GitHub Check: Windows MSVC 2019 v140 Win64
- GitHub Check: Windows MSVC 2019 v140 Win32
- GitHub Check: Windows MSVC 2019 v141 Win32
- GitHub Check: Windows MSVC 2022 v142 Win64
- GitHub Check: Windows MSVC 2022 v142 Win32
- GitHub Check: Ubuntu MinGW i686
- GitHub Check: Ubuntu 20.04 Clang
- GitHub Check: Ubuntu MinGW i686
- GitHub Check: macOS Clang Native Instructions (ARM64)
- GitHub Check: macOS GCC UBSAN
- GitHub Check: macOS Clang ASAN
- GitHub Check: Windows GCC Compat No Opt
- GitHub Check: Windows GCC Native Instructions (AVX)
- GitHub Check: Windows GCC
- GitHub Check: Windows ClangCl Win64 Native Instructions (AVX)
- GitHub Check: Windows ClangCl Win64
- GitHub Check: Windows ClangCl Win32
- GitHub Check: Windows MSVC 2019 v140 Win64
- GitHub Check: Windows MSVC 2019 v140 Win32
- GitHub Check: Windows MSVC 2019 v141 Win32
- GitHub Check: Windows MSVC 2022 v142 Win64
- GitHub Check: Windows MSVC 2022 v142 Win32
- GitHub Check: Ubuntu MinGW i686
- GitHub Check: Ubuntu 20.04 Clang
- GitHub Check: Ubuntu MinGW i686
- GitHub Check: macOS Clang Native Instructions (ARM64)
- GitHub Check: macOS GCC UBSAN
- GitHub Check: macOS Clang ASAN
- GitHub Check: Windows GCC Compat No Opt
- GitHub Check: Windows GCC Native Instructions (AVX)
- GitHub Check: Windows GCC
- GitHub Check: Windows ClangCl Win64 Native Instructions (AVX)
- GitHub Check: Windows ClangCl Win64
- GitHub Check: Windows ClangCl Win32
- GitHub Check: Windows MSVC 2019 v140 Win64
- GitHub Check: Windows MSVC 2019 v140 Win32
- GitHub Check: Windows MSVC 2019 v141 Win32
- GitHub Check: Windows MSVC 2022 v142 Win64
- GitHub Check: Windows MSVC 2022 v142 Win32
- GitHub Check: Ubuntu MinGW i686
- GitHub Check: Ubuntu 20.04 Clang
- GitHub Check: Ubuntu MinGW i686
- GitHub Check: macOS GCC UBSAN
- GitHub Check: macOS Clang ASAN
- GitHub Check: Windows GCC Compat No Opt
- GitHub Check: Windows GCC Native Instructions (AVX)
- GitHub Check: Windows GCC
- GitHub Check: Windows ClangCl Win64 Native Instructions (AVX)
- GitHub Check: Windows ClangCl Win64
- GitHub Check: Windows ClangCl Win32
- GitHub Check: Windows MSVC 2019 v140 Win64
- GitHub Check: Windows MSVC 2019 v140 Win32
- GitHub Check: Windows MSVC 2019 v141 Win32
- GitHub Check: Windows MSVC 2022 v142 Win64
- GitHub Check: Windows MSVC 2022 v142 Win32
- GitHub Check: Ubuntu MinGW i686
- GitHub Check: Ubuntu 20.04 Clang
- GitHub Check: Ubuntu MinGW i686
- GitHub Check: macOS GCC UBSAN
- GitHub Check: macOS Clang ASAN
- GitHub Check: Windows GCC Compat No Opt
- GitHub Check: Windows GCC Native Instructions (AVX)
- GitHub Check: Windows GCC
- GitHub Check: Windows ClangCl Win64 Native Instructions (AVX)
- GitHub Check: Windows ClangCl Win64
- GitHub Check: Windows ClangCl Win32
- GitHub Check: Windows MSVC 2019 v140 Win64
- GitHub Check: Windows MSVC 2019 v140 Win32
- GitHub Check: Windows MSVC 2019 v141 Win32
- GitHub Check: Windows MSVC 2022 v142 Win64
- GitHub Check: Windows MSVC 2022 v142 Win32
- GitHub Check: Ubuntu MinGW i686
- GitHub Check: Ubuntu 20.04 Clang
- GitHub Check: Ubuntu MinGW i686
- GitHub Check: macOS GCC UBSAN
- GitHub Check: macOS Clang ASAN
- GitHub Check: Windows GCC Compat No Opt
- GitHub Check: Windows GCC Native Instructions (AVX)
- GitHub Check: Windows GCC
- GitHub Check: Windows ClangCl Win64 Native Instructions (AVX)
- GitHub Check: Windows ClangCl Win64
- GitHub Check: Windows ClangCl Win32
- GitHub Check: Windows MSVC 2019 v140 Win64
- GitHub Check: Windows MSVC 2019 v140 Win32
- GitHub Check: Windows MSVC 2019 v141 Win32
- GitHub Check: Windows MSVC 2022 v142 Win64
- GitHub Check: Windows MSVC 2022 v142 Win32
- GitHub Check: Ubuntu MinGW i686
- GitHub Check: Ubuntu 20.04 Clang
- GitHub Check: Ubuntu MinGW i686
Improve image/container rebuild script to work properly under cron.
Hopefully this keeps working unattended for a while.
Summary by CodeRabbit
New Features
Chores