这是indexloc提供的服务,不要输入任何密码
Skip to content

Conversation

@Dead2
Copy link
Member

@Dead2 Dead2 commented Jan 4, 2025

Improve image/container rebuild script to work properly under cron.

Hopefully this keeps working unattended for a while.

Summary by CodeRabbit

  • New Features

    • Enhanced script for building actions runner with improved file handling and error management.
    • Added logging for build and create processes.
    • Implemented flexible Dockerfile and file download mechanism with executable permissions for scripts.
  • Chores

    • Improved script robustness and cleanup processes.
    • Added podman image pruning capability.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 4, 2025

Walkthrough

The script actions-runner-rebuild.sh for s390 self-hosted builders has been modified to improve its handling of Docker-related files. The initial check for the existence of the /etc/actions-runner file has been removed, and a MODE variable is now set based on the presence of the actions-runner.Dockerfile. The script can download necessary files if the Dockerfile is missing. Enhanced error handling and logging to /var/log/actions-runner-build.log have been implemented, along with a cleanup process that conditionally removes temporary files and prunes unused podman images.

Changes

File Change Summary
arch/s390/self-hosted-builder/actions-runner-rebuild.sh - Removed existence check for /etc/actions-runner
- Added dynamic MODE variable based on Dockerfile presence
- Implemented file download mechanism for missing files
- Enhanced error handling for systemctl and podman commands
- Added logging for podman build and podman create
- Modified cleanup to handle temporary files conditionally
- Added podman image pruning with error handling
arch/s390/self-hosted-builder/actions-runner.Dockerfile - Added --chmod=555 option to COPY commands for entrypoint and actions-runner scripts

Sequence Diagram

sequenceDiagram
    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
Loading

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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

📥 Commits

Reviewing files that changed from the base of the PR and between 787c7f6 and ab1d728.

📒 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 issue

Add error handling and improve security for file downloads

The current implementation has several potential issues:

  1. Hardcoded references to the 'develop' branch
  2. No validation of downloaded files
  3. Unsafe directory handling
  4. 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
 fi

Consider implementing a more robust solution:

  1. Use a specific git ref or commit hash instead of a branch name
  2. Implement checksum verification for downloaded files
  3. Consider using curl with proper SSL verification instead of wget
  4. 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 suggestion

Improve 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."
 fi

Consider implementing:

  1. Service health check after start
  2. More selective image pruning strategy
  3. Logging of cleanup operations
  4. Proper error handling for all operations

Let's verify the service configuration:


1-41: Consider additional operational improvements for production reliability

Since this script runs under cron and manages critical infrastructure, consider these additional improvements:

  1. Add comprehensive logging:
+# Add to start of script
+exec 1> >(logger -s -t $(basename $0)) 2>&1
  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}"' EXIT

Consider implementing:

  1. Monitoring integration (e.g., sending status to a monitoring system)
  2. Backup mechanism for critical files and configurations
  3. Automated testing of the rebuild process in a staging environment
  4. Documentation of recovery procedures
  5. Version control for downloaded files instead of using raw GitHub URLs

Let's verify the cron configuration:

@codecov
Copy link

codecov bot commented Jan 4, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 74.84%. Comparing base (cbb6ec1) to head (7bd821e).
Report is 4 commits behind head on develop.

❗ There is a different number of reports uploaded between BASE (cbb6ec1) and HEAD (7bd821e). Click for more details.

HEAD has 5 uploads less than BASE
Flag BASE (cbb6ec1) HEAD (7bd821e)
7 2
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.
📢 Have feedback on the report? Share it here.

@Dead2 Dead2 force-pushed the fix-s390x-dockerfiles branch from ab1d728 to 7bd821e Compare January 19, 2025 14:04
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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=555 ensures 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:

  1. Add explicit version tags for the base image (e.g., almalinux:9.3) to ensure reproducible builds
  2. 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

📥 Commits

Reviewing files that changed from the base of the PR and between ab1d728 and 7bd821e.

📒 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

@Dead2 Dead2 merged commit 212563d into develop Jan 19, 2025
129 of 292 checks passed
@Dead2 Dead2 mentioned this pull request Feb 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants