Uh oh!
There was an error while loading. Please reload this page.
Compose: init containers - #25435
Conversation
✅ Deploy Preview for docsdocker ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
docker-agent
left a comment
There was a problem hiding this comment.
Assessment: 🔴 CRITICAL
This PR adds documentation for the new pre_start lifecycle hook. Three issues need attention before merging: one unresolved internal placeholder that would ship to users, and two factual inconsistencies between the how-to guide and the reference page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
d9d77ea to
6cf1272Compare
glours
left a comment
There was a problem hiding this comment.
Just 1 little nit to fix before merging, thanks @aevesdocker 🙏
Uh oh!
There was an error while loading. Please reload this page.
Co-authored-by: Guillaume Lours <705411+glours@users.noreply.github.com>
docker-agent
left a comment
There was a problem hiding this comment.
Assessment: 🟡 NEEDS ATTENTION
Two medium-severity issues found in the new pre_start reference documentation added by this PR. Both are in content/reference/compose-file/services.md.
The new how-to page (content/manuals/compose/how-tos/init-containers.md) is well-written and clear. No missing redirects, no vendored content concerns, no front matter issues.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
dvdksn
left a comment
There was a problem hiding this comment.
Three issues with the Postgres examples that need fixing before merge.
| - data:/data | ||
| db: | ||
| image: postgres:16 |
There was a problem hiding this comment.
This example is confusing because it mixes two unrelated concerns in one pre_start sequence:
./manage.py migrate— a DB migration (needs thedbservice)chown -R 1000:1000 /data— fixing ownership of a volume
The app service has volumes: data:/data but the db service has no volume. A reader trying to understand pre_start from this example will wonder why the app is chowning a path that the database doesn't share, and why the database has no persistent volume of its own.
Also, db: image: postgres:16 has no POSTGRES_PASSWORD, POSTGRES_USER, or POSTGRES_DB — the postgres container won't start.
Suggest splitting into two focused examples: one for migrations (the DB case, already done in init-containers.md) and one for volume ownership (the busybox/chown case, already done separately in init-containers.md). The combined example here adds confusion rather than showing a realistic use case.
| - command: ["./manage.py", "migrate"] | ||
| db: | ||
| image: postgres:18 |
There was a problem hiding this comment.
Two problems with this db service that would prevent it from actually working:
No
POSTGRES_PASSWORD— Postgres refuses to start without it (orPOSTGRES_HOST_AUTH_METHOD=trust), with the error:Error: Database is uninitialized and superuser password is not specified. You must specify POSTGRES_PASSWORD to a non-empty value for the superuser.Version inconsistency — this example uses
postgres:18but theservices.mdreference example usespostgres:16. Pick one version and use it consistently across both files.
At minimum the db service should look like:
db:
image: postgres:17environment:
POSTGRES_PASSWORD: examplePOSTGRES_USER: myuserPOSTGRES_DB: mydbhealthcheck:
test: ["CMD-SHELL", "pg_isready -U $${POSTGRES_USER} -d $${POSTGRES_DB}"]interval: 10sretries: 5start_period: 30stimeout: 10s| - command: ["./manage.py", "loaddata", "fixtures.json"] | ||
| db: | ||
| image: postgres:18 |
There was a problem hiding this comment.
Same issue here: postgres:18 with no POSTGRES_PASSWORD — this db container won't start. Same version inconsistency vs services.md (postgres:16).
Uh oh!
There was an error while loading. Please reload this page.
Description
As per docker/compose#13862 and compose-spec/compose-spec#647
Related issues or tickets
Reviews