Skip to content

Commit 54642e3

Browse files
Merge pull request #1 from controlaltdelete-nl/feature/nginx-support
Add memory-bounded nginx web server
2 parents 4c38354 + c6786b4 commit 54642e3

24 files changed

Lines changed: 1067 additions & 4 deletions

.claude/architecture.md

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
# Architecture
2+
3+
All-in-one Docker base images for Magento 2 CI/CD. One image bundles PHP-FPM, MySQL,
4+
Elasticsearch, Redis, and Varnish so a pipeline needs only a single container.
5+
Supervisord is the process manager (PID 1 via `CMD ["/usr/bin/supervisord", "-n"]`).
6+
7+
## Directory Structure
8+
9+
- `Dockerfile` — single multi-stage-free build, parameterized by `PHP_VERSION` and
10+
`NODE_VERSION` build args. Installs the full stack, configures MySQL/Elasticsearch,
11+
installs Composer, Node (via nvm), and n98-magerun2.
12+
- `scripts/` — runtime helpers copied into the image WORKDIR (`/data`):
13+
- `start-services` — start MySQL, Elasticsearch, Redis, PHP-FPM (and Varnish when `ENABLE_VARNISH=true`)
14+
- `stop-services` — stop all services
15+
- `retry` — retry wrapper for flaky startup steps
16+
- `templates/` — service configuration baked into the image:
17+
- `supervisord.conf` and `supervisord/*.conf` — one program block per service
18+
- `elasticsearch/``elasticsearch.yml` and JVM GC options
19+
- `varnish/default.vcl`, `memory-limit-php.ini`
20+
- `tests/run-tests.sh` — Bash assertion suite run inside the built container
21+
- `.github/workflows/build-php-images.yml` — matrix build → test → push pipeline
22+
- `build`, `test.sh` — local debugging helpers (NOT used by CI)
23+
24+
## Build and Release Flow
25+
26+
1. CI matrix builds one image per PHP version (7.1–8.5), `fail-fast: false`.
27+
2. Each image is loaded locally and tested twice: default, then `ENABLE_VARNISH=true`.
28+
3. On `main`, images are pushed to Docker Hub (`michielgerritsen/magento2-base-image`)
29+
and ghcr.io (`ghcr.io/controlaltdelete-nl/magento2-docker-base-images/magento2-base-image`),
30+
tagged `<php-version>` and `php<NN>-fpm`.
31+
32+
## Conventions
33+
34+
- Service config lives in `templates/`, never inlined in the `Dockerfile`.
35+
- Version-specific behavior (opcache packaging, Magerun phar URL) branches explicitly
36+
inside the `Dockerfile`.
37+
- Pre-configured databases: `magento` / `magento-test`, user/pass `magento` / `password`
38+
and `magento-test` / `password`.
39+
- Exposed ports: 9000 (PHP-FPM), 3306 (MySQL), 9200 (Elasticsearch), 6379 (Redis), 80 (Varnish/HTTP).
40+
- Node.js is installed via nvm; runtime version switching is supported.
41+
42+
## Current Focus
43+
44+
Branch `feature/reduce-startup-time` — work aimed at reducing container/service
45+
startup time. Keep `start-services` and Supervisord/service tuning changes measured
46+
against the `tests/run-tests.sh` suite to avoid regressing service readiness.

.claude/code-standards.md

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
# Code Standards
2+
3+
## Style
4+
5+
- Shell: POSIX-friendly Bash. `#!/bin/bash` shebang, `set -e` where a failure should
6+
abort. Quote variable expansions (`"$VAR"`). No em dashes anywhere.
7+
- Dockerfile: group related `apt-get`/setup work into single `RUN` layers, clean apt
8+
caches in the same layer (`apt-get clean && rm -rf /var/lib/apt/lists/*`), pin
9+
third-party repos by signed keyring.
10+
11+
## Linting
12+
13+
```bash
14+
# Lint the Dockerfile
15+
hadolint Dockerfile
16+
17+
# Lint shell scripts
18+
shellcheck scripts/* tests/run-tests.sh build test.sh
19+
```
20+
21+
## Pre-commit Checks
22+
23+
- `hadolint Dockerfile` is clean (or findings are deliberate and justified)
24+
- `shellcheck` is clean on changed scripts
25+
- The image builds for the affected PHP version(s)
26+
- `tests/run-tests.sh` passes inside the built container
27+
28+
## Conventions
29+
30+
- Keep service config in `templates/`, not inlined in the `Dockerfile`.
31+
- Helper scripts live in `scripts/` and are copied to the image WORKDIR (`/data`).
32+
- Version-conditional logic in the `Dockerfile` uses explicit `if`/`sort -V` checks;
33+
document the reason inline only when the branch is non-obvious.
34+
- No comments in shell logic that a descriptive function or variable name can replace.
35+
- Copyright years, when added, use the current year (2026).

.claude/pipeline.md

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
# Pipeline
2+
3+
Configure which agents run at each phase of the development workflow. Each entry is an agent name resolved from the project's `.claude/agents/` directory (local override) or the plugin's `agents/` directory (default).
4+
5+
## post-plan
6+
- devils-advocate
7+
8+
## post-implementation
9+
<!-- - standards-enforcer -->
Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
# Task 001: Install nginx and ship minimal front-controller config
2+
3+
**Status**: completed
4+
**Depends on**: none
5+
**Retry count**: 0
6+
7+
## Description
8+
Install `nginx-light` in the image and ship a minimal, valid generic PHP
9+
front-controller configuration. The config must work out of the box (`listen 80`,
10+
`root /data`, fastcgi to php-fpm on `127.0.0.1:9000`) so the image serves PHP without
11+
any runtime rendering, and so `nginx -t` passes at build time. Keep the footprint low:
12+
`worker_processes 1` and conservative buffers.
13+
14+
## Context
15+
- Related files:
16+
- `Dockerfile` (add `nginx-light` to the existing stack `apt-get install` RUN layer; clean apt cache in the same layer; remove the distro default site; `COPY` both config files to explicit paths)
17+
- `templates/nginx/nginx.conf` (new: main config; COPY to `/etc/nginx/nginx.conf`)
18+
- `templates/nginx/default.conf` (new: server block; COPY to `/etc/nginx/conf.d/default.conf`)
19+
- Patterns to follow:
20+
- Service config lives under `templates/` and is `COPY`-ed in (see `templates/varnish/`, `templates/elasticsearch/`).
21+
- Layer hygiene: install + `apt-get clean && rm -rf /var/lib/apt/lists/*` in one RUN.
22+
- Notes:
23+
- php-fpm listens on TCP `127.0.0.1:9000` (keep TCP; do not switch to a socket).
24+
- The `nginx.conf` we ship REPLACES the distro `/etc/nginx/nginx.conf`. It must remain
25+
a complete, valid main config: keep `events {}`, an `http {}` block with
26+
`include /etc/nginx/mime.types;`, `default_type application/octet-stream;`, `sendfile on;`,
27+
`gzip on;`, and crucially `include /etc/nginx/conf.d/*.conf;`. Set `worker_processes 1`,
28+
`daemon off;`, and log `access_log /dev/stdout;` / `error_log /dev/stderr;`. Do NOT
29+
re-add `include /etc/nginx/sites-enabled/*;` (that is how the distro default `:80`
30+
server block sneaks back in).
31+
- Remove `/etc/nginx/sites-enabled/default` (and the symlink target if present) so no
32+
second `:80` server block exists. Because our shipped `nginx.conf` does not include
33+
`sites-enabled`, this is belt-and-suspenders, but do it anyway for clarity.
34+
- Front-controller routing in `default.conf`: `listen 80;`, `root /data;`,
35+
`index index.php index.html;`, `try_files $uri $uri/ /index.php$is_args$args;`, then a
36+
`location ~ \.php$` block passing to `fastcgi_pass 127.0.0.1:9000;`. Set
37+
`fastcgi_param SCRIPT_FILENAME $document_root$fastcgi_script_name;` (use
38+
`$document_root`, NOT a hardcoded path, so the task 003 docroot override only needs to
39+
rewrite the single `root` directive). Include `/etc/nginx/fastcgi_params`.
40+
41+
## Requirements (Test Descriptions)
42+
These become assertions in `tests/run-tests.sh` (in-container) plus a build-time check.
43+
44+
- [x] `it has the nginx binary installed and on the path`
45+
- [x] `it ships an nginx config that passes nginx -t syntax validation`
46+
- [x] `it sets nginx worker_processes to 1`
47+
- [x] `it removes the distro default nginx site so only the front-controller server block is active` (assert `/etc/nginx/sites-enabled/default` is absent)
48+
- [x] `it includes only conf.d in the main nginx.conf so no other directory is auto-loaded` (assert `nginx.conf` has `include /etc/nginx/conf.d/*.conf;` and has NO `include` of `sites-enabled` or `available`; this is what keeps task 004's `/etc/nginx/available/magento.conf` wrapper inert)
49+
- [x] `it configures fastcgi_pass to 127.0.0.1:9000`
50+
- [x] `it sets SCRIPT_FILENAME from $document_root so a docroot override needs only the root directive`
51+
52+
## Acceptance Criteria
53+
- All requirements have passing assertions in `tests/run-tests.sh`
54+
- `nginx -t` succeeds inside the built image
55+
- nginx-light is installed in the same RUN layer as the rest of the stack with apt cache cleaned
56+
- Code follows project standards (quoted shell, layer hygiene, no em dashes)
57+
58+
## Implementation Notes
59+
(Left blank - filled in by programmer during implementation)
Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
# Task 002: Bound php-fpm pool memory
2+
3+
**Status**: completed
4+
**Depends on**: none
5+
**Retry count**: 0
6+
7+
## Description
8+
Cap php-fpm worker memory to prevent the GitHub Actions OOM events. Ship a pool
9+
override that switches process management to on-demand with a low, conservative
10+
`max_children` default, an idle timeout, and a request recycle limit. Bake it at build
11+
time into the version-specific pool directory using the `PHP_VERSION` build arg.
12+
13+
## Context
14+
- Related files:
15+
- `templates/php-fpm/zz-magento.conf` (new: pool override)
16+
- `Dockerfile` (COPY/render the pool override into `/etc/php/${PHP_VERSION}/fpm/pool.d/zz-magento.conf`)
17+
- Patterns to follow:
18+
- `PHP_VERSION` is a build arg already used throughout the `Dockerfile`
19+
(see the `php-fpm.conf` sed and the opcache/Magerun version branches).
20+
- Config templates live under `templates/`.
21+
- Notes / rationale:
22+
- Current risk: `memory_limit = 2048M` (`templates/memory-limit-php.ini`) with the
23+
default pool `pm = dynamic, pm.max_children = 5` allows ~5 fat workers.
24+
- Target settings: `pm = ondemand`, `pm.max_children = 4` (default),
25+
`pm.process_idle_timeout = 10s`, `pm.max_requests = 500`, `catch_workers_output = yes`.
26+
- The pool file must end up in the version-specific path
27+
`/etc/php/${PHP_VERSION}/fpm/pool.d/zz-magento.conf` (Ondřej Surý PPA layout, confirmed
28+
by the existing `/usr/sbin/php-fpm$PHP_VERSION` sed in the Dockerfile). The file name
29+
`zz-magento.conf` sorts after the distro `www.conf` so its directives win.
30+
- The override must reuse the SAME pool name as the distro pool. The distro pool is named
31+
`[www]` in `www.conf`. Start `zz-magento.conf` with the `[www]` header and override the
32+
directives under it; a new pool name would create a SECOND pool listening on the same
33+
`127.0.0.1:9000` socket and `php-fpm -t` / startup would fail with an "address already
34+
in use" / duplicate-pool error. (Alternatively, override `pm` etc. inside `[www]`.)
35+
- `pm = ondemand` ignores the `dynamic`-only directives (`pm.start_servers`,
36+
`pm.min_spare_servers`, `pm.max_spare_servers`) that `www.conf` still sets; this is
37+
accepted by php-fpm without error, no need to unset them.
38+
- The build-time validation binary is version-specific: `php-fpm${PHP_VERSION} -t`
39+
(there is no unversioned `php-fpm` binary). Use the `PHP_VERSION` build arg.
40+
- Runtime override of `max_children` (`PHP_FPM_MAX_CHILDREN`) is handled in task 003,
41+
not here. This task ships the static, bounded defaults.
42+
43+
## Requirements (Test Descriptions)
44+
These become assertions in `tests/run-tests.sh` (reading the effective pool config) plus a build-time check.
45+
46+
- [x] `it configures the php-fpm pool with pm set to ondemand`
47+
- [x] `it caps php-fpm pm.max_children at 4 by default`
48+
- [x] `it sets a php-fpm process idle timeout`
49+
- [x] `it recycles php-fpm workers via pm.max_requests`
50+
- [x] `it passes php-fpm configuration validation` (assertion invokes `php-fpm${PHP_VERSION} -t`; `PHP_VERSION` is exported into the test container via `-e`)
51+
52+
## Acceptance Criteria
53+
- All requirements have passing assertions in `tests/run-tests.sh`
54+
- `php-fpm${PHP_VERSION} -t` succeeds inside the built image
55+
- Pool override lands in the correct version-specific `pool.d` directory and reuses the
56+
`[www]` pool name (no duplicate pool / duplicate `:9000` listener)
57+
- Code follows project standards (no em dashes)
58+
59+
## Implementation Notes
60+
(Left blank - filled in by programmer during implementation)
Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
# Task 003: Run nginx under Supervisord; wire startup port/docroot/pool overrides
2+
3+
**Status**: completed
4+
**Depends on**: 001, 002
5+
**Retry count**: 0
6+
7+
## Description
8+
Make nginx a managed Supervisord program and wire the runtime knobs into
9+
`scripts/start-services`: switch the nginx `listen` port to `8080` when Varnish is
10+
enabled, swap the docroot when `NGINX_DOCROOT` is set, and override the php-fpm pool
11+
`max_children` when `PHP_FPM_MAX_CHILDREN` is set. All rendering happens BEFORE
12+
supervisord launches (start-services line ~19) so daemons boot with the final config.
13+
Also fix the stale Varnish port echo.
14+
15+
## Context
16+
- Related files:
17+
- `templates/supervisord/nginx.conf` (new: `program:nginx`, `command=nginx -g 'daemon off;'`, `autostart=true`, logs to stdout/stderr). The explicit `-g 'daemon off;'` here is the source of truth for foregrounding even if task 001 also set `daemon off;` in nginx.conf; passing it twice is harmless, but keep it on the supervisord command so the program never daemonizes out from under supervisord.
18+
- `Dockerfile` (add ONE explicit `COPY templates/supervisord/nginx.conf /etc/supervisor/conf.d/nginx.conf` line next to the existing per-file COPYs at lines ~48-52; the Dockerfile uses one explicit COPY per supervisord conf, NOT a glob, so add a matching explicit line)
19+
- `scripts/start-services` (render port/docroot/pool before launching supervisord; fix port echo)
20+
- Patterns to follow:
21+
- Existing supervisord program blocks under `templates/supervisord/` (e.g. `varnish.conf`, `php-fpm.conf`).
22+
- `start-services` already does pre-supervisord setup (MySQL init) before line 19; add the rendering there.
23+
- Varnish is `autostart=false` and started later; nginx on `8080` and Varnish on `80` do not overlap.
24+
- Notes:
25+
- All rendering must live in `scripts/start-services` BEFORE line 19 (the
26+
`supervisord -n` launch). This only takes effect when the container is started via
27+
`./start-services` (the CI flow and the README usage). The default `CMD
28+
["/usr/bin/supervisord","-n"]` path does NOT run start-services, so nginx boots with
29+
its shipped defaults (`listen 80`, `root /data`) and `ENABLE_VARNISH`/overrides are
30+
only honored through start-services. This is acceptable: the defaults are the correct
31+
no-override behavior. Do NOT move rendering into a supervisord pre-hook.
32+
- Port switch: default config has `listen 80`. When `ENABLE_VARNISH=true`, `sed` it to
33+
`listen 8080` in `/etc/nginx/conf.d/default.conf` so Varnish (`-a :80`) fronts nginx.
34+
Anchor the sed (e.g. `s/listen 80;/listen 8080;/`) so it cannot also rewrite a `:80`
35+
inside a comment or fastcgi value.
36+
- Docroot override: when `NGINX_DOCROOT` is set, `sed` ONLY the `root` directive in
37+
`/etc/nginx/conf.d/default.conf` (task 001 sets `SCRIPT_FILENAME` from `$document_root`,
38+
so the single `root` rewrite is sufficient; do not touch the fastcgi block).
39+
Also `mkdir -p "$NGINX_DOCROOT"` so nginx does not 404/500 on a missing root.
40+
- Pool override: when `PHP_FPM_MAX_CHILDREN` is set, `sed` `pm.max_children` in the pool
41+
file via the glob `/etc/php/*/fpm/pool.d/zz-magento.conf` (avoids needing PHP_VERSION at
42+
runtime). The glob must run before supervisord starts php-fpm so the new value is read
43+
at boot (php-fpm reads the pool config once at start, not per request).
44+
- Fix: `scripts/start-services` currently echoes "Varnish is available on port 6081";
45+
Varnish binds `:80`. Correct the message to reference port 80.
46+
- Override assertions are GUARDED by their env vars so they pass/skip in the default run
47+
and only execute in task 005's dedicated CI override step (mirrors the `ENABLE_VARNISH` guard
48+
already in `run-tests.sh`).
49+
- The `/data` mount is empty in CI. The HTTP-serving tests MUST first write a throwaway
50+
`index.php` (e.g. `<?php echo "nginx-ok";`) into the docroot being served
51+
(`/data` by default, `$NGINX_DOCROOT` when set) before curling, and assert the response
52+
body contains the sentinel string. For the custom-docroot test, `mkdir -p "$NGINX_DOCROOT"`
53+
first (the override CI step in task 005 may point it at a non-existent path).
54+
- nginx (`fastcgi_pass 127.0.0.1:9000`) connects to php-fpm per request, so nginx can boot
55+
before php-fpm is ready; but php-fpm itself is started by supervisord and start-services
56+
returns after the Elasticsearch loop without waiting for php-fpm/nginx. The serving
57+
assertions MUST retry the curl with a short bounded loop (a few attempts, 1s apart) to
58+
avoid a flaky failure against a php-fpm/nginx that is still coming up. Same applies to the
59+
Varnish-fronted curl on `:80` (start-services starts Varnish asynchronously via
60+
`supervisorctl start varnish`).
61+
62+
## Requirements (Test Descriptions)
63+
These become assertions in `tests/run-tests.sh`.
64+
65+
- [x] `it runs nginx as a supervisord-managed program`
66+
- [x] `it serves a php file from the document root over http on port 80 when varnish is disabled`
67+
- [x] `it serves php through varnish on port 80 with nginx listening on 8080 when varnish is enabled`
68+
- [x] `it serves from a custom document root when NGINX_DOCROOT is set`
69+
- [x] `it overrides php-fpm pm.max_children when PHP_FPM_MAX_CHILDREN is set`
70+
- [x] `it reports the correct varnish port in start-services output`
71+
72+
## Acceptance Criteria
73+
- All requirements have passing assertions in `tests/run-tests.sh`
74+
- Default run (no Varnish): nginx serves php on `:80`
75+
- `ENABLE_VARNISH=true` run: nginx on `:8080`, Varnish serves php on `:80`
76+
- Override assertions are env-guarded and do not break the default and Varnish runs
77+
- Stale 6081 echo corrected
78+
- Code follows project standards (quoted shell, early returns, no em dashes)
79+
80+
## Implementation Notes
81+
(Left blank - filled in by programmer during implementation)

0 commit comments

Comments
 (0)