cloud-hosting-platform/cloud-node-container
4
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Upgrade to AlmaLinux 10 and fix WebSocket proxying #2

Merged
jknapp merged 3 commits from upgrade-almalinux-10 into main 2026-08-03 16:07:23 +00:00
Conversation 2 Commits 3 Files Changed 8
+85 -15
jknapp commented 2026-08-03 14:54:57 +00:00
Owner
Copy Link
Copy Source

AlmaLinux 10 upgrade

Base image moves from almalinux/9-base to almalinux/10-base, which forces two related changes:

  • EPEL repo URL bumps to the el10 release RPM.
  • tini is replaced with dumb-init as PID 1 — tini is not packaged in EPEL 10. dumb-init provides the same signal forwarding and zombie reaping that the wedged-container fix (#1) depends on.
  • Nginx 1.26 in AlmaLinux 10 deprecates the listen ... http2 parameter, so the server block uses the separate http2 on; directive.

WebSocket fixes (socket.io, ws)

While confirming socket.io would work on this build, three real issues turned up:

Issue Before After
Connection: upgrade sent on every request, not just upgrades hardcoded 'upgrade' map $http_upgrade $connection_upgrade
Idle WebSockets cut at nginx's 60s default read timeout unset proxy_read_timeout / proxy_send_timeout 600s
Any single header line over 1KB rejected with 400 large_client_header_buffers 2 1k 4 8k (nginx default)

The header buffer issue was not socket.io-specific — session cookies and bearer tokens routinely exceed 1KB — but authenticated WebSocket handshakes are where it surfaced.

The 600s timeout is deliberately shorter than the commonly-cited 86400s: nginx runs a single worker with worker_connections 512 and each proxied client consumes two slots, so a long timeout would pin slots for dead clients.

Verification

The generated config was rendered with representative env vars and run under real nginx with a Node backend (no image build):

  • nginx -t passes on the combined nginx.conf + generated default.conf.
  • WebSocket handshake through nginx returns 101 Switching Protocols and reaches the upstream 'upgrade' event with correct headers — the exact path socket.io's WebSocket transport uses.
  • Polling requests now arrive at the backend with Connection: close instead of a bogus upgrade.
  • A 3KB cookie returns 200; reverting only large_client_header_buffers to 2 1k returns 400, confirming the fix is load-bearing.

Still needs staging verification: the image has not been built. The one change that would hard-fail container startup on a version mismatch is http2 on;, which requires nginx >= 1.25.1 — worth confirming the nginx version in the AlmaLinux 10 image. (Local validation used Ubuntu's nginx 1.24, which rejects that directive; it was stripped for the local test only.)

Also

  • README gains a WebSocket Support section covering the PM2 cluster-mode trap (a tenant-supplied ecosystem.config.js with instances > 1 breaks socket.io's polling handshake), the ~250 concurrent connection ceiling, and reconnects on PM2 memory restarts.
  • New .gitignore for .claude/.
## AlmaLinux 10 upgrade Base image moves from `almalinux/9-base` to `almalinux/10-base`, which forces two related changes: - EPEL repo URL bumps to the el10 release RPM. - **`tini` is replaced with `dumb-init`** as PID 1 — tini is not packaged in EPEL 10. dumb-init provides the same signal forwarding and zombie reaping that the wedged-container fix (#1) depends on. - Nginx 1.26 in AlmaLinux 10 deprecates the `listen ... http2` parameter, so the server block uses the separate `http2 on;` directive. ## WebSocket fixes (socket.io, ws) While confirming socket.io would work on this build, three real issues turned up: | Issue | Before | After | |---|---|---| | `Connection: upgrade` sent on every request, not just upgrades | hardcoded `'upgrade'` | `map $http_upgrade $connection_upgrade` | | Idle WebSockets cut at nginx's 60s default read timeout | unset | `proxy_read_timeout` / `proxy_send_timeout 600s` | | Any single header line over 1KB rejected with 400 | `large_client_header_buffers 2 1k` | `4 8k` (nginx default) | The header buffer issue was not socket.io-specific — session cookies and bearer tokens routinely exceed 1KB — but authenticated WebSocket handshakes are where it surfaced. The 600s timeout is deliberately shorter than the commonly-cited 86400s: nginx runs a single worker with `worker_connections 512` and each proxied client consumes two slots, so a long timeout would pin slots for dead clients. ## Verification The generated config was rendered with representative env vars and run under real nginx with a Node backend (no image build): - `nginx -t` passes on the combined `nginx.conf` + generated `default.conf`. - WebSocket handshake through nginx returns **101 Switching Protocols** and reaches the upstream `'upgrade'` event with correct headers — the exact path socket.io's WebSocket transport uses. - Polling requests now arrive at the backend with `Connection: close` instead of a bogus `upgrade`. - A 3KB cookie returns **200**; reverting only `large_client_header_buffers` to `2 1k` returns **400**, confirming the fix is load-bearing. **Still needs staging verification**: the image has not been built. The one change that would hard-fail container startup on a version mismatch is `http2 on;`, which requires nginx >= 1.25.1 — worth confirming the nginx version in the AlmaLinux 10 image. (Local validation used Ubuntu's nginx 1.24, which rejects that directive; it was stripped for the local test only.) ## Also - README gains a **WebSocket Support** section covering the PM2 cluster-mode trap (a tenant-supplied `ecosystem.config.js` with `instances > 1` breaks socket.io's polling handshake), the ~250 concurrent connection ceiling, and reconnects on PM2 memory restarts. - New `.gitignore` for `.claude/`.
jknapp added 1 commit 2026-08-03 14:54:57 +00:00
Upgrade to AlmaLinux 10 and fix WebSocket proxying e417a908f2 
Base image moves from AlmaLinux 9 to 10, which forces two related changes:
EPEL repo URL bumps to the el10 release RPM, and tini is replaced with
dumb-init as PID 1 since tini is not packaged in EPEL 10. Both provide the
signal forwarding and zombie reaping the wedged-container fix relies on.
Nginx 1.26 in AlmaLinux 10 deprecates the `listen ... http2` parameter, so
the server block now uses the separate `http2 on;` directive.

Also fixes three issues that affected WebSocket apps (socket.io, ws):

- `Connection: upgrade` was hardcoded on every proxied request, including
  ordinary HTTP. Now driven by a `map $http_upgrade $connection_upgrade`
  so only genuine upgrade requests carry it.
- No explicit proxy read/send timeout meant idle WebSockets were cut at
  nginx's 60s default. Set to 600s, which clears any sane heartbeat without
  pinning connection slots (worker_connections is 512, two per client).
- `large_client_header_buffers 2 1k` returned 400 for any single header
  line over 1KB, which session cookies and bearer tokens routinely exceed.
  Raised to the nginx default of 4 8k; buffers are allocated on demand, so
  this only costs memory for requests that need it.

Verified by rendering the generated config and running it under nginx with
a Node backend: WebSocket handshakes return 101 and reach the upstream
'upgrade' event, polling requests arrive with `Connection: close`, and a
3KB cookie returns 200 where the old buffer setting returned 400.

README gains a WebSocket Support section covering the PM2 cluster-mode
trap, the concurrency ceiling, and reconnects on memory restarts.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
jknapp added 1 commit 2026-08-03 14:56:14 +00:00
Exclude .claude/ from the Docker build context 6651872729 
Keeps local Claude Code settings out of the build context alongside the
other editor directories. No effect on the image itself -- the Dockerfile
only copies scripts/, configs/ and examples/ -- but the directory was
being sent to the daemon on every build.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
jknapp commented 2026-08-03 16:03:35 +00:00
Author
Owner
Copy Link
Copy Source

Code review

Found 2 issues:

  1. dumb-init is not a drop-in replacement for tini here: their default signal-forwarding semantics are opposites. tini without -g forwards a signal only to its direct child; dumb-init without --single-child calls setsid() and forwards to the entire process group. Verified empirically with dumb-init 1.2.5 using a parent that catches SIGTERM and deliberately does not forward it: under the default the grandchild was still signalled, under --single-child it was not. In this invocation chain (dumb-init -> su -> pm2 --no-daemon -> node), a docker stop now delivers SIGTERM to the tenant's node process directly and simultaneously with pm2, bypassing pm2's kill_timeout shutdown sequencing. PR #1 introduced tini specifically for its signal forwarding and this PR's description asserts the two are equivalent, which does not hold as invoked. Suggested fix: exec dumb-init --single-child -- su - ..., or an explicit decision that group-wide delivery is wanted.

cloud-node-container/scripts/entrypoint.sh Lines 105 to 107 in 6651872729
echo "Starting PM2 as user $user (under dumb-init as PID 1)..."
cd /home/$user/app
exec dumb-init -- su - $user -c "cd /home/$user/app && NODE_ENV=production pm2 start ecosystem.config.js --no-daemon"

  1. MEMORY-GUIDE.md still documents the old buffer sizing as an applied memory optimization after this PR raised it 16x in total (2 1k = 2KB -> 4 8k = 32KB). Line 36 says "Buffer Limits: Reduced client buffer sizes" and line 21 budgets nginx at "~8-15MB | Single worker, limited buffers". At worker_connections 512 the new worst case exceeds that stated budget. The config change is justified on its own merits (a 1KB limit rejects ordinary session cookies with a 400), but the doc claim is now wrong and this PR already edits MEMORY-GUIDE.md for the AlmaLinux 10 rename.

cloud-node-container/MEMORY-GUIDE.md Lines 34 to 38 in 6651872729
3. **Memcached Limit**: 32MB maximum cache size
4. **Nginx Workers**: Single worker process for memory efficiency
5. **Buffer Limits**: Reduced client buffer sizes
6. **Log Buffering**: 2-minute flush intervals to reduce I/O

Also worth noting, not filed as an issue: PR #1 established a staging checklist for exactly this PID-1 code path (ps -ef --forest showing the init as PID 1, pm2 stop all incrementing RestartCount, a postgres-blip restart simulation, and a tenant-supplied ecosystem.config.js starting cleanly). None of it has been re-run against dumb-init. The nginx changes in this PR were verified against real nginx with a Node backend, but no container image has been built.

Checked for bugs, CLAUDE.md compliance, git history context, prior PR discussion, and code comment consistency. The nginx WebSocket changes (heredoc escaping, map placement and visibility, http2 on against AlmaLinux 10's nginx 1.26.3) reviewed clean.

### Code review Found 2 issues: 1. `dumb-init` is not a drop-in replacement for `tini` here: their default signal-forwarding semantics are opposites. `tini` without `-g` forwards a signal only to its direct child; `dumb-init` without `--single-child` calls `setsid()` and forwards to the entire process group. Verified empirically with dumb-init 1.2.5 using a parent that catches SIGTERM and deliberately does not forward it: under the default the grandchild was still signalled, under `--single-child` it was not. In this invocation chain (`dumb-init` -> `su` -> `pm2 --no-daemon` -> node), a `docker stop` now delivers SIGTERM to the tenant's node process directly and simultaneously with pm2, bypassing pm2's `kill_timeout` shutdown sequencing. PR #1 introduced `tini` specifically for its signal forwarding and this PR's description asserts the two are equivalent, which does not hold as invoked. Suggested fix: `exec dumb-init --single-child -- su - ...`, or an explicit decision that group-wide delivery is wanted. https://repo.anhonesthost.net/cloud-hosting-platform/cloud-node-container/src/commit/6651872729e9ea51b99accd2989a4f8b35c82638/scripts/entrypoint.sh#L105-L107 2. `MEMORY-GUIDE.md` still documents the old buffer sizing as an applied memory optimization after this PR raised it 16x in total (`2 1k` = 2KB -> `4 8k` = 32KB). Line 36 says "Buffer Limits: Reduced client buffer sizes" and line 21 budgets nginx at "~8-15MB | Single worker, limited buffers". At `worker_connections 512` the new worst case exceeds that stated budget. The config change is justified on its own merits (a 1KB limit rejects ordinary session cookies with a 400), but the doc claim is now wrong and this PR already edits `MEMORY-GUIDE.md` for the AlmaLinux 10 rename. https://repo.anhonesthost.net/cloud-hosting-platform/cloud-node-container/src/commit/6651872729e9ea51b99accd2989a4f8b35c82638/MEMORY-GUIDE.md#L34-L38 Also worth noting, not filed as an issue: PR #1 established a staging checklist for exactly this PID-1 code path (`ps -ef --forest` showing the init as PID 1, `pm2 stop all` incrementing `RestartCount`, a postgres-blip restart simulation, and a tenant-supplied `ecosystem.config.js` starting cleanly). None of it has been re-run against `dumb-init`. The nginx changes in this PR were verified against real nginx with a Node backend, but no container image has been built. Checked for bugs, CLAUDE.md compliance, git history context, prior PR discussion, and code comment consistency. The nginx WebSocket changes (heredoc escaping, `map` placement and visibility, `http2 on` against AlmaLinux 10's nginx 1.26.3) reviewed clean.
jknapp added 1 commit 2026-08-03 16:06:13 +00:00
Use dumb-init --single-child for tini signal parity 80b03c884e 
Code review caught that dumb-init is not a drop-in replacement for tini as
invoked. tini without -g forwards a signal only to its direct child;
dumb-init without --single-child calls setsid() and forwards to the entire
process group. Verified against dumb-init 1.2.5 with a parent that catches
SIGTERM and does not forward it: under the default the grandchild is still
signalled, under --single-child it is not.

Without the flag, `docker stop` delivered SIGTERM to the tenant's node
process directly and simultaneously with pm2, bypassing pm2's kill_timeout
shutdown sequencing. Since the wedged-container fix in #1 introduced tini
specifically for its signal forwarding, the previous commit's claim that
the two are equivalent did not hold.

Also corrects MEMORY-GUIDE.md, which still described header buffers as
"reduced" after they were raised from 2 1k to the nginx default of 4 8k.
The nginx memory budget is unchanged: these buffers are allocated per
request only when a request needs them, not preallocated per connection.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
jknapp commented 2026-08-03 16:06:27 +00:00
Author
Owner
Copy Link
Copy Source

Review issues addressed in 80b03c8

  1. dumb-init signal parity — now exec dumb-init --single-child -- su - .... Re-verified against dumb-init 1.2.5 in the same invocation shape: the pm2 stand-in receives SIGTERM, the node stand-in does not, matching tini's single-child default. Rationale recorded in a comment above the exec line.

cloud-node-container/scripts/entrypoint.sh Lines 104 to 113 in 80b03c884e
# (dumb-init replaces tini, which is not packaged in EPEL 10.)
#
# --single-child is required for parity with tini. dumb-init defaults to
# setsid() plus forwarding signals to the whole process group, whereas tini
# without -g forwards only to its direct child. Without this flag a
# `docker stop` would signal the tenant's node process directly and at the
# same moment as pm2, bypassing pm2's kill_timeout shutdown sequencing.
echo "Starting PM2 as user $user (under dumb-init as PID 1)..."
cd /home/$user/app
exec dumb-init --single-child -- su - $user -c "cd /home/$user/app && NODE_ENV=production pm2 start ecosystem.config.js --no-daemon"

  1. MEMORY-GUIDE.md — the "Buffer Limits" entry no longer claims header buffers are reduced, and now states that these buffers are allocated per request on demand rather than preallocated per connection, so the ~8-15MB nginx budget still holds.

cloud-node-container/MEMORY-GUIDE.md Lines 34 to 42 in 80b03c884e
3. **Memcached Limit**: 32MB maximum cache size
4. **Nginx Workers**: Single worker process for memory efficiency
5. **Buffer Limits**: Reduced client body buffer (16k) and 8MB max body size.
Header buffers use the nginx default (`large_client_header_buffers 4 8k`)
rather than a reduced value -- a smaller limit rejects ordinary session
cookies with a 400. These are allocated per request only when a request
actually needs them, not preallocated per connection.
6. **Log Buffering**: 2-minute flush intervals to reduce I/O

Both modified scripts pass bash -n.

Still outstanding before merge: the staging checklist from #1 has not been run against dumb-init, and no image has been built from this branch. Specifically: PID 1 is dumb-init in ps -ef --forest; pm2 stop all increments RestartCount within ~30s; a postgres-blip restart is survived; and a tenant-supplied ecosystem.config.js still starts cleanly. The --single-child change is exactly the sort of thing that checklist exists to catch.

### Review issues addressed in 80b03c8 1. **dumb-init signal parity** — now `exec dumb-init --single-child -- su - ...`. Re-verified against dumb-init 1.2.5 in the same invocation shape: the pm2 stand-in receives SIGTERM, the node stand-in does not, matching tini's single-child default. Rationale recorded in a comment above the exec line. https://repo.anhonesthost.net/cloud-hosting-platform/cloud-node-container/src/commit/80b03c884e979f7cefd5a677437b95c169573cba/scripts/entrypoint.sh#L104-L113 2. **MEMORY-GUIDE.md** — the "Buffer Limits" entry no longer claims header buffers are reduced, and now states that these buffers are allocated per request on demand rather than preallocated per connection, so the ~8-15MB nginx budget still holds. https://repo.anhonesthost.net/cloud-hosting-platform/cloud-node-container/src/commit/80b03c884e979f7cefd5a677437b95c169573cba/MEMORY-GUIDE.md#L34-L42 Both modified scripts pass `bash -n`. **Still outstanding before merge:** the staging checklist from #1 has not been run against dumb-init, and no image has been built from this branch. Specifically: PID 1 is `dumb-init` in `ps -ef --forest`; `pm2 stop all` increments `RestartCount` within ~30s; a postgres-blip restart is survived; and a tenant-supplied `ecosystem.config.js` still starts cleanly. The `--single-child` change is exactly the sort of thing that checklist exists to catch.
jknapp merged commit d2ae99cd37 into main 2026-08-03 16:07:23 +00:00
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
No items
No labels
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
jknapp whmcs-sam whp-cicd-user whp-deploy
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: cloud-hosting-platform/cloud-node-container#2