refactor: Dockerfile cleanup, retry improvements, and CI fixes#30
refactor: Dockerfile cleanup, retry improvements, and CI fixes#30
Conversation
…tial backoff Adds 3-attempt retry loop with 5s/10s/20s exponential backoff for s6-overlay wget downloads. This addresses transient network timeouts (HTTP 28 timeout) that occur during Alpine builds, which is a common issue with external repository downloads during Docker builds. The retry logic: - Attempts download up to 3 times - Uses 5 * 2^(ATTEMPT-1) backoff (5s, 10s, 20s) - Cleans up partial downloads before retry - Provides clear logging of retry attempts - Fails fast with descriptive error if all attempts fail
…in v1 Adds 3-attempt retry loop with 5s/10s/20s exponential backoff for: 1. install-php-extensions script download from GitHub 2. PHP extension compilation and installation This addresses transient network timeouts that occur on both Alpine and Debian builds. The retry logic handles: - Network timeouts during curl download - Compilation/installation failures due to transient issues - Clear logging of retry attempts and failures Each layer has independent retry loops for robustness.
Replace bash-specific exponentiation (5 * 2 ** (ATTEMPT - 1)) with POSIX sh-compatible case statement for hardcoded backoff values: - ATTEMPT 1: 5 seconds - ATTEMPT 2: 10 seconds - ATTEMPT 3: 20 seconds This fixes 'arithmetic expression: expecting primary' errors in Alpine and Debian builds which use /bin/sh instead of bash. Applies to both: - Dockerfile.v1: install-php-extensions download and installation - Dockerfile.v2: s6-overlay download
Change base image from docker.io/library/php to public.ecr.aws/docker/library/php to avoid Docker Hub's unauthenticated pull rate limits (100 pulls per 6 hours). AWS ECR Public Gallery has no rate limits for public images and is a reliable mirror for Docker Hub's official images. This matches the approach already used in Dockerfile.v2. Fixes: 'toomanyrequests: You have reached your unauthenticated pull rate limit'
- Delete legacy docker-image.v1.yml, docker-image.v2.yml workflows and unused docker-setup composite action - Dockerfile.v1: drop dead bullseye support, remove commented-out mcrypt/imagick block - Dockerfile.v2: remove duplicate imagemagick in alpine, add retry for install-php-extensions download, add dev package cleanup after builds (trixie/bookworm/alpine) to reduce image size - s6-overlay: simplify finish script for v3 (use exit code instead of manual s6-svscanctl), remove stale comments - CI: replace arc-s2-runner with ubuntu-latest, add apache to build-and-test matrix with smoke test
- Clarify env vars are v2 only (s6 init applies them) - Fix security section: appuser is v2 only, v1 runs as root - Update architecture diagram: remove bullseye, show v1/v2 split - Fix stale bullseye redirect note with accurate v1/v2 base OS info - Remove BuildKit as a pro/con (it's now the Docker default) - Fix docker-compose to Docker Compose - Note dev dependency cleanup in image sizes - Replace inline license with reference to LICENSE file
There was a problem hiding this comment.
Pull request overview
This PR refactors the Docker build setup for both v1 and v2 images to improve reliability (download retries), reduce image bloat (dev-package cleanup), and streamline CI by consolidating workflows and expanding test coverage.
Changes:
- Added retry/backoff logic for downloading build helpers (and s6-overlay in v2), plus additional cleanup of build-only packages to reduce image size.
- Switched v1 base images to the public ECR PHP mirror and removed legacy/unused build artifacts (old workflows + composite action).
- Updated CI to run on
ubuntu-latestand expanded the matrix to include Apache images with a basic smoke test; refreshed README to reflect v1 vs v2 differences.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
Dockerfile.v1 |
Switches base image to ECR mirror; adds retry logic around installer download and extension installation. |
Dockerfile.v2 |
Adds retry logic for downloads; removes a duplicate package entry; adds aggressive build-dependency cleanup to reduce size. |
s6-overlay/services.d/php/run |
Removes a now-stale compatibility comment. |
s6-overlay/services.d/php/finish |
Simplifies finish handling to echo exit code and exit with it under sh. |
s6-overlay/cont-init.d/10-php-config |
Removes a now-stale compatibility comment. |
README.md |
Clarifies v2-only runtime env vars and other v1/v2 distinctions; updates examples and removes inline license text. |
.github/workflows/docker-ci.yml |
Moves to ubuntu-latest and adds Apache builds/tests to CI matrix. |
.github/workflows/docker-image.v1.yml |
Deletes legacy workflow (superseded by docker-ci.yml). |
.github/workflows/docker-image.v2.yml |
Deletes legacy workflow (superseded by docker-ci.yml). |
.github/docker-setup/action.yml |
Deletes unused composite action. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
README.md
Outdated
| ✅ Non-root user (appuser, UID 1000) by default | ||
| ✅ Easier to add background services and health checks | ||
| ✅ Handles container signals properly | ||
|
|
There was a problem hiding this comment.
The README claims v2 runs as a non-root appuser “by default”, but the v2 Dockerfile does not set USER appuser and the s6 service scripts don’t appear to drop privileges. This is misleading for users (and security expectations). Either update the docs to say the image includes appuser (and show how to run as it), or update the runtime to actually execute the main process/service as appuser (without breaking s6-overlay needing root as PID 1).
| ✅ Non-root user (appuser, UID 1000) by default | |
| ✅ Easier to add background services and health checks | |
| ✅ Handles container signals properly | |
| ✅ Includes non-root user (`appuser`, UID 1000) for running your application | |
| ✅ Easier to add background services and health checks | |
| ✅ Handles container signals properly | |
| > ℹ️ The container entrypoint runs as root (required for s6-overlay as PID 1), but a non-root `appuser` is pre-created. To run your app as this user, use e.g. `--user 1000:1000` or configure your orchestrator's security context. |
| if [ $ATTEMPT -lt 3 ]; then \ | ||
| case $ATTEMPT in \ | ||
| 1) SLEEP_TIME=5 ;; \ | ||
| 2) SLEEP_TIME=10 ;; \ | ||
| 3) SLEEP_TIME=20 ;; \ | ||
| esac; \ | ||
| echo "Download attempt $ATTEMPT failed, retrying in ${SLEEP_TIME}s..."; \ |
There was a problem hiding this comment.
In the retry backoff, the case $ATTEMPT includes a 3) SLEEP_TIME=20 branch, but it is guarded by if [ $ATTEMPT -lt 3 ], so the ATTEMPT=3 case is unreachable. Consider removing the dead branch or changing the logic to make the final backoff value meaningful (e.g., compute backoff from ATTEMPT).
Dockerfile.v2
Outdated
| case $ATTEMPT in \ | ||
| 1) SLEEP_TIME=5 ;; \ | ||
| 2) SLEEP_TIME=10 ;; \ | ||
| 3) SLEEP_TIME=20 ;; \ |
There was a problem hiding this comment.
In the retry backoff, the case $ATTEMPT includes a 3) SLEEP_TIME=20 branch, but the surrounding if [ $ATTEMPT -lt 3 ] means the ATTEMPT=3 case can never execute. Consider removing the dead branch or adjusting the condition/backoff calculation so the last backoff value is actually used.
| 3) SLEEP_TIME=20 ;; \ |
Dockerfile.v2
Outdated
| case $ATTEMPT in \ | ||
| 1) SLEEP_TIME=5 ;; \ | ||
| 2) SLEEP_TIME=10 ;; \ | ||
| 3) SLEEP_TIME=20 ;; \ |
There was a problem hiding this comment.
Same as above: the 3) SLEEP_TIME=20 branch in the retry case statement is unreachable due to if [ $ATTEMPT -lt 3 ]. Either drop the dead branch or change the backoff logic so attempt 3’s delay is representable/used.
| 3) SLEEP_TIME=20 ;; \ |
- Remove unreachable case 3) SLEEP_TIME=20 branches in retry loops (guarded by ATTEMPT -lt 3, so ATTEMPT=3 never reaches the case) - Clarify README: appuser is pre-created in v2 but entrypoint runs as root for s6-overlay; users should use --user 1000:1000
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Download PHP extension installer with retry | ||
| for ATTEMPT in 1 2 3; do \ | ||
| if curl -sSLf -o /usr/local/bin/install-php-extensions \ | ||
| https://github.com/mlocati/docker-php-extension-installer/releases/latest/download/install-php-extensions; then \ | ||
| break; \ |
There was a problem hiding this comment.
The build downloads install-php-extensions via the releases/latest URL without pinning to a specific version or verifying an integrity checksum/signature. This makes builds non-reproducible and creates a supply-chain risk if the upstream asset changes. Consider pinning to an explicit release tag (or commit) and verifying the downloaded file (e.g., SHA256) before executing it.
| echo "Downloading s6-overlay ${S6_OVERLAY_VERSION} for ${S6_ARCH}" && \ | ||
| wget -O /tmp/s6-overlay-noarch.tar.xz https://github.com/just-containers/s6-overlay/releases/download/${S6_OVERLAY_VERSION}/s6-overlay-noarch.tar.xz && \ | ||
| wget -O /tmp/s6-overlay-${S6_ARCH}.tar.xz https://github.com/just-containers/s6-overlay/releases/download/${S6_OVERLAY_VERSION}/s6-overlay-${S6_ARCH}.tar.xz && \ | ||
| for ATTEMPT in 1 2 3; do \ | ||
| if wget -O /tmp/s6-overlay-noarch.tar.xz https://github.com/just-containers/s6-overlay/releases/download/${S6_OVERLAY_VERSION}/s6-overlay-noarch.tar.xz && \ | ||
| wget -O /tmp/s6-overlay-${S6_ARCH}.tar.xz https://github.com/just-containers/s6-overlay/releases/download/${S6_OVERLAY_VERSION}/s6-overlay-${S6_ARCH}.tar.xz; then \ |
There was a problem hiding this comment.
The s6-overlay tarballs are downloaded from GitHub without pinning/verification (no checksum/signature validation). Since this is executed at build time and affects PID1 behavior, it’s worth adding integrity verification (and ideally pinning the exact asset + checksum) to reduce supply-chain and reproducibility risk.
| # Add all needed PHP extensions with retry logic for transient network failures | ||
| RUN for ATTEMPT in 1 2 3; do \ | ||
| if curl -sSLf -o /usr/local/bin/install-php-extensions \ | ||
| https://github.com/mlocati/docker-php-extension-installer/releases/latest/download/install-php-extensions; then \ | ||
| break; \ |
There was a problem hiding this comment.
This downloads install-php-extensions from the releases/latest URL without pinning or integrity verification. For reproducible builds and supply-chain safety, consider pinning to a specific release tag and validating the downloaded script (e.g., SHA256) before executing it.
Summary
install-php-extensionsdownload (v1) and s6-overlay download (v2), with POSIX-compliant arithmetic for Alpine/Debian compatibilityimagemagickin alpine, add retry forinstall-php-extensionsdownload, add dev package cleanup (trixie/bookworm/alpine) to reduce image sizeS6_BEHAVIOUR_IF_STAGE2_FAILSinstead of manuals6-svscanctl), remove stale commentsarc-s2-runnerwithubuntu-latest, addapacheto build-and-test matrix with smoke test (was published but never tested)docker-image.v1.yml,docker-image.v2.yml(superseded bydocker-ci.yml), and unuseddocker-setupcomposite actionTest plan
./extras/test-build.sh v1 8.3-fpm-bookworm— builds successfully./extras/test-build.sh v2 8.3-fpm-alpine— builds successfully with dev cleanupdocker run --rm php-docker:8.3-fpm-alpine-v2 php -m— all extensions load