fix(webserver): tolerate a stale non-FIFO /var/tmp/logpipe on container start (#8635) [skip ci] - #8635
Conversation
|
Download the artifacts for this pull request:
See Testing a PR. |
History of the
|
| Date | Commit | PR | Summary |
|---|---|---|---|
| 2023-05-13 | d957ea71f |
#4895 (fixes #4889) | Introduced the mechanism. Replaced command: sleep infinity with pre-start.sh as PID 1, adding the /var/tmp/logpipe FIFO + SIGTERM trap, because writing directly to /proc/1/fd/1 from a docker exec'd process didn't work on Gitpod (gitpod-io/gitpod#17551). |
| 2023-07-10 | f39c884c6 |
#5107 | Fixed a typo in the trap (kill -- --1 -> kill -- -1) so the "kill everything" handler actually worked. |
| 2024-11-11 | bc338507e |
#6706 | Unrelated portability fix: #!/bin/bash -> #!/usr/bin/env bash. |
| 2025-10-21 | 889558ea7 |
#7716 | Removed all Gitpod support from DDEV. Left pre-start.sh's Gitpod-referencing comment in place as acknowledged historical color; did not touch the FIFO mechanism itself. |
| 2026-05-08 | 3d29d0ae0 |
#8396 (fixes #8295) | Fixed a different bug in the same mechanism: cat running in the foreground deferred bash's SIGTERM trap handling, so ddev stop took the full 10s grace period before Docker SIGKILLed the container. Backgrounding cat (cat < pipe & ; wait) dropped stop time to ~1.4s. |
| 2026-07-30 | 2a2c1ce0b |
#8635 (this PR) | Fixed the mkfifo race this PR addresses: a non-atomic check-then-create ([[ ! -p ]] then mkfifo) failed with "File exists" when a stale non-FIFO file was left at /var/tmp/logpipe, apparently from a Docker/overlay2 storage-layer race on the CI runner recreating a same-named container back-to-back. |
Is the mechanism still appropriate?
Given Gitpod is gone, it's fair to ask whether this whole apparatus is now legacy complexity that should be reverted to the pre-2023 sleep infinity design. It isn't purely that: /var/tmp/logpipe is also the stdout_logfile target for every supervisord-managed process (apache.conf, php-fpm.conf, supervisord-nginx-fpm.conf), so it's the general log fan-in mechanism for the container, not just a wrapper around the one docker exec /start.sh call. #8396 landed after Gitpod removal and shows real, current value (10s -> 1.4s stop time) still being extracted from it. So the design itself still looks right; it's just been a fragile ~25 lines that has needed three narrow fixes over its life, of which this PR is the third.
…er start
The web container's logpipe setup (`pre-start.sh` and both `start.sh`
variants) checked `[[ ! -p ${logpipe} ]]` before calling `mkfifo`, but did
not remove a pre-existing non-pipe file at that path first. On a freshly
created container this should never happen, since `/var/tmp` is only the
container's own writable layer, but a Docker storage-layer race on CI
runners (containers stopped/recreated back-to-back with the same name)
occasionally left stale content there, causing `mkfifo` to fail with
"File exists" and the container to exit immediately.
`rm -f` the path before `mkfifo` so a stale non-pipe file no longer
crashes container startup.
Bumps WebTag since the ddev-webserver container scripts changed.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2a2c1ce to
a653ee1
Compare
a653ee1 to
67ec69b
Compare
|
New image pushed, rebased after mysql 9.7 PR |
…er start (ddev#8635) [skip ci] Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Any change under containers/ddev-webserver/ needs a matching WebTag bump in versionconstants.go so ddev actually pulls an image built from this branch instead of the one WebTag previously pointed at, matching the convention used by prior webserver-only fixes (e.g. ddev#8635). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The Issue
The
all-project-typesjob failed inTestDbServerwith:which made the
ddev-webservercontainer exit immediately, soddev starttimed out waiting for it to become healthy. A restart of the job made it
pass.
How This PR Solves The Issue
The
ddev-webservercontainer's actual startup command is/pre-start.sh(set via
command: /pre-start.shinapp_compose_template.yaml). Its onlyjob is to create a FIFO at
/var/tmp/logpipeandcatit forever as PID 1;DDEV's Go code later
docker execs/start.shinto the running container,redirecting its output into that same pipe so it shows up in
docker logs.Both
start.shvariants use the identical pattern for the same reason(so
/var/tmp/logpipealso works for non-DDEV/standalone container usage).All three copies used a non-atomic check-then-create:
In the failing run, the test had just stopped/removed the
ddev-TestPkgWordpress-webcontainer (finishing a mariadb 11.8 iteration)and immediately recreated a brand-new container of the same name a couple
of seconds later for a mariadb 12.3 iteration.
/var/tmpis only thecontainer's own writable layer (not a bind mount or named volume), so a
genuinely fresh container should never have anything at that path already.
The fact that something was there — and wasn't recognized as a valid FIFO —
points to a Docker/overlay2 storage-layer race on the GitHub-hosted runner,
where the previous container's writable layer wasn't fully torn down before
the new one (same name) was created, so it inherited stale leftover state.
This doesn't fix that underlying container-runtime race (which is outside
DDEV's control), but it makes the three scripts tolerant of it by removing
any stale non-pipe file before calling
mkfifo:WebTaginpkg/versionconstants/versionconstants.gois bumped to match,since the
ddev-webservercontainer scripts changed.Manual Testing Instructions
make linux_amd64(or your platform) and confirm the build succeeds.ddev starta project and confirm the web container comes up healthy asusual -- this is a defensive fix for a rare race, so normal startup is
unaffected.
simulate the stale-file condition before a restart:
(
mkfifo: File exists). After this change, it starts normally.Automated Testing Overview
No new automated test is added -- this addresses a rare Docker
storage-layer race on CI runners that isn't reliably reproducible in a
unit/integration test. Existing
ddevappintegration tests (e.g.TestDbServer) exercise the affected startup path on every run.Release/Deployment Notes
Bumps the
ddev-webserverimage tag (WebTag). No user-facing behaviorchange under normal conditions; only affects the rare case where a stale
/var/tmp/logpipefile exists at container start.