Build/Test Tools: Retry the Docker environment setup in CI - #12736
Build/Test Tools: Retry the Docker environment setup in CI#12736adimoldovan wants to merge 5 commits into
Conversation
…commands.
`start.js` runs `docker compose up` through `spawnSync` and never inspects the
result, so a failed pull does not fail the script. Execution continues into
`composer update -W` with containers that may not exist, and the error surfaces
later and in the wrong place.
`docker.js` calls `process.exit( returns.status )`, and `status` is `null` when
Docker cannot be spawned. `process.exit( null )` exits 0, so `npm run env:pull`
reports success when the Docker CLI is missing.
Reproduce with:
LOCAL_PHP=this-tag-does-not-exist npm run env:start; echo $?
See #65745.
There was a problem hiding this comment.
Pull request overview
This PR hardens the local Docker environment setup used by WordPress core CI by ensuring .env exists before any Docker Compose command runs, and by adding bounded retries for image pulls and environment startup across reusable workflows to mitigate transient Docker Hub / Packagist failures.
Changes:
- Add
ensure_env_file()intools/local-env/scripts/utils.jsand call it fromstart.js,docker.js, andinstall.jsso.envis created synchronously (when missing) before Compose/dotenv usage. - Propagate failures from
docker compose up(and improve error reporting) intools/local-env/scripts/start.js; improve error handling/exit behavior intools/local-env/scripts/docker.js. - Add 3-attempt retry loops (10s/20s backoff) for
npm run env:pullandnpm run env:startin multiple reusable CI workflows.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tools/local-env/scripts/utils.js | Adds ensure_env_file() to bootstrap .env from .env.example before any Compose usage. |
| tools/local-env/scripts/start.js | Uses the new .env bootstrap and checks docker compose up exit status to fail fast. |
| tools/local-env/scripts/install.js | Ensures .env exists before loading env vars during install flow. |
| tools/local-env/scripts/docker.js | Ensures .env exists before running Compose; adjusts error/exit handling for Compose commands. |
| .github/workflows/reusable-test-local-docker-environment-v1.yml | Adds retry loops for env:pull and env:start steps. |
| .github/workflows/reusable-phpunit-tests-v3.yml | Adds retry loop for env:start (and contextual comments for retries). |
| .github/workflows/reusable-phpunit-tests-v2.yml | Adds retry loops for env:pull and env:start. |
| .github/workflows/reusable-phpunit-tests-v1.yml | Adds retry loops for env:pull and env:start. |
| .github/workflows/reusable-performance-test-v2.yml | Adds retry loops for env:pull and env:start before building/running perf tests. |
| .github/workflows/reusable-end-to-end-tests.yml | Adds retry loops for env:pull and env:start in E2E workflow. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // `status` is null when Docker could not be spawned at all, or was killed by a signal. Ctrl+C on | ||
| // a long-running command such as `env:logs` is not a failure worth an npm error block. | ||
| process.exit( returns.signal ? 0 : ( returns.status ?? 1 ) ); |
Test using WordPress PlaygroundThe changes in this pull request can previewed and tested using a WordPress Playground instance. WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser. Some things to be aware of
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
tools/local-env/scripts/docker.js:49
docker.jsexits with code 0 for any signal-terminated Docker Compose process (returns.signal ? 0 : …). This hides abnormal termination (e.g. SIGTERM/SIGKILL) and can incorrectly report success for non-interactive commands likeenv:pullif the runner kills the process. If the intent is only to avoid an npm error block on Ctrl+C, special-case SIGINT and keep non-zero exit behavior for other signals.
// `status` is null when Docker could not be spawned at all, or was killed by a signal. Ctrl+C on
// a long-running command such as `env:logs` is not a failure worth an npm error block.
process.exit( returns.signal ? 0 : ( returns.status ?? 1 ) );
…nment command. `docker.js` exited 0 for any signal, so a command killed by SIGTERM or SIGKILL reported success. That reintroduced the false success this ticket set out to remove: the retry loop in `reusable-phpunit-tests-v3.yml` branches on the status of `npm run env:pull` and would treat a killed pull as a completed one. Restrict the exemption to SIGINT, which is how a long-running command such as `env:logs` is normally ended. Report every other signal and exit non-zero. `start.js` exempts no signal, because `env:start` runs `composer update -W` next and that must not run against containers that never came up. Say so in the comment, so the difference between the two files is deliberate. Also fold the unreachable `up.error` branch in `start.js` into the failure message. The `docker info` check above it already throws when the Docker CLI is missing or the daemon is down.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
tools/local-env/scripts/docker.js:49
returns.signal ? 0 : …exits successfully for any signal (e.g. SIGTERM), but the comment (and the linked PR context) indicate only SIGINT should be treated as a non-failure. Exiting 0 on SIGTERM can mask real failures/cancellations and cause the surrounding npm step to report success incorrectly.
// `status` is null when Docker could not be spawned at all, or was killed by a signal. Ctrl+C on
// a long-running command such as `env:logs` is not a failure worth an npm error block.
process.exit( returns.signal ? 0 : ( returns.status ?? 1 ) );
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the Core Committers: Use this line as a base for the props when committing in SVN: To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
`Start Docker environment` failed in 85 runs across 69 branches over 30 days. Of 92 sampled failures, 63 were Docker Hub and 20 were repo.packagist.org. `env:pull` was already retried in `reusable-phpunit-tests-v3.yml`. Extend that to the five remaining workflows, and retry `npm run env:start` as well, since it also runs `composer update -W`. The retry stays in the workflows rather than the npm scripts. Released branches call these reusable workflows at `@trunk` but check out their own `tools/local-env/` and `package.json`, so a retry in the scripts would only protect trunk. Keeping it in CI also means a contributor working offline gets an immediate error instead of waiting through the backoff. Add `ensure_env_file()` so that every Compose command resolves the same image tags. The pull step runs before `start.js` created the `.env` file, which made `env:pull` fall back to the Compose defaults. The synchronous copy also removes a race: the previous `copyFile` was asynchronous, so `dotenv.config()` could read a `.env` that was not written yet. See #65745.
…etries. - Retry `env:restart` in the local Docker environment workflow. It ends in `env:start`, so it has the same registry and Packagist exposure as the steps that were already retried. - Correct the retry comments. They claimed a bad tag would not hold a runner, but the loop retries every failure regardless of its cause. - Resolve `.env` and `.env.example` against the repository root instead of the current directory, so running the scripts from a subdirectory no longer creates a stray `.env` there.
e0681ac to
656fc29
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (9)
.github/workflows/reusable-test-local-docker-environment-v1.yml:161
- This retry loop uses Bash syntax (
for ...; do,$(( ... ))). Sinceruns-onis parameterized viainputs.os, this step can end up running under PowerShell on Windows runners unlessshellis set explicitly, causing a hard failure. Addshell: bashhere to ensure consistent execution.
- name: Start Docker environment (with retry)
run: |
.github/workflows/reusable-test-local-docker-environment-v1.yml:192
- Same portability issue as the other retry steps: this uses Bash-only constructs, but the workflow accepts
inputs.os(which could be Windows => PowerShell default). Addshell: bashso retries work reliably regardless of runner OS.
- name: Restart Docker environment (with retry)
run: |
.github/workflows/reusable-phpunit-tests-v3.yml:230
- The retry logic here is written for Bash, but
runs-onis controlled byinputs.os, so this can run under a different default shell (notably PowerShell on Windows). Please setshell: bashfor this step so the loop works consistently.
- name: Start Docker environment (with retry)
run: |
.github/workflows/reusable-phpunit-tests-v2.yml:177
- This step's retry loop is Bash-specific, but the workflow's
runs-onis parameterized (inputs.os), so it may execute under PowerShell on Windows. Settingshell: bashavoids a cross-OS parse failure.
- name: Start Docker environment (with retry)
run: |
.github/workflows/reusable-phpunit-tests-v1.yml:164
- This retry loop depends on Bash syntax, but
runs-onis driven byinputs.os, so the default shell may not be Bash on some runners (e.g. Windows). Addshell: bashto avoid breaking the workflow wheninputs.oschanges.
- name: Start Docker environment (with retry)
run: |
.github/workflows/reusable-test-local-docker-environment-v1.yml:143
- These retry loops rely on Bash features (POSIX
for+ arithmetic expansion$(( ... ))). Because this reusable workflow allowsinputs.os, the default shell may be PowerShell on Windows runners, which would make this step fail to parse. Consider explicitly settingshell: bashfor this step (and other retry steps) to keep the workflow portable across runner OS values.
This issue also appears in the following locations of the same file:
- line 160
- line 191
- name: Pull Docker images (with retry)
run: |
.github/workflows/reusable-phpunit-tests-v3.yml:212
- This reusable workflow allows
inputs.os, so the default shell can be non-Bash (e.g. PowerShell on Windows). The retry loop uses Bash syntax (for ...; do,$(( ... ))) and will fail to parse in that case. Setshell: bashfor this step to keep it OS-agnostic.
This issue also appears on line 229 of the same file.
- name: Pull Docker images (with retry)
run: |
.github/workflows/reusable-phpunit-tests-v2.yml:134
- Because this workflow accepts
inputs.os, the default shell isn't guaranteed to be Bash. This new retry loop uses Bash-only syntax and would break on Windows runners (PowerShell default). Addshell: bashhere to ensure the step runs consistently.
This issue also appears on line 176 of the same file.
- name: Pull Docker images (with retry)
run: |
.github/workflows/reusable-phpunit-tests-v1.yml:135
- The workflow supports an
inputs.osoverride, but this retry loop is written for Bash and will fail under shells like PowerShell (Windows default). Please setshell: bashfor this step to keep it portable.
This issue also appears on line 163 of the same file.
- name: Pull Docker images (with retry)
run: |
There was a problem hiding this comment.
Tested with head e0681ac
Two blockers:
- The branch lacks PR 12735’s latest signal fix and restores exit code 0 for SIGTERM/SIGKILL
- Reusable workflows check out the caller branch. On 6.8–7.0,
env:pulllacks the new.envbootstrap. A controlled 6.8 test confirmed .env remained missing.
Trac ticket: https://core.trac.wordpress.org/ticket/65745
Stacked on #12735 — that should land first. The retry only works once a failed
upactually fails the step.Start Docker environmentfailed in 85 runs across 69 branches over 30 days. Of 92 sampled failures, 63 were Docker Hub and 20 were repo.packagist.org.env:pullwas already retried, but only inreusable-phpunit-tests-v3.yml. This extends it to the other five workflows and also retriesnpm run env:start, which runscomposer update -Was well as starting the containers. Retrying only the pull leaves the Packagist failures uncovered.Why
ensure_env_file()is part of thisThe new pull step runs before
start.jshas created.env, soenv:pullresolved image tags from the Compose defaults whileupresolved them from.env. On trunk that pullsmysql:latest, thenmysql:9.7.Moving the bootstrap to a shared helper makes every Compose command resolve the same tags. Making it synchronous also removes a race: the previous
copyFilewas asynchronous anddotenv.config()ran on the next line, so a fresh clone could read a.envthat had not been written yet.Testing instructions
The retry is bounded and shows the real error. Three attempts, 10s and 20s apart, registry error each time, exit 1:
A cold start still works and is no slower:
env:pullandenv:startresolve the same tags. Delete.env, runnpm run env:pull, confirm.envexists afterwards and thatupdoes not pull a second database image.Local Docker Environment,PHPUnit TestsandEnd-to-end Testsexercise this across large matrices.Note for reviewers
These six workflows are consumed at
@trunkby every released branch, so this takes effect on all 30 as soon as it is committed.Use of AI Tools
AI assistance: Yes
Tool(s): Claude Code
Model(s): Claude Opus 5
Used for: Investigating the CI failures, writing the patch, and iterative code review. The cross-branch constraints above were checked against every calling branch, and the behaviour was verified locally.
This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.