Repository navigation
docs(adr): Docker surface decisions (compose, base image, Dockerfile split) #853
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
0ec8fbc
docs(adr): research-and-decide outputs for Docker surface (3 ADRs)
LucasSantana-Dev 8be65e0
Merge branch 'release/v2.11.0' into docs/docker-decision-adrs
LucasSantana-Dev 296d7b3
docs(adr): mark frontend-dockerfile-keep-separate as superseded by PR…
LucasSantana-Dev 5e23095
Merge branch 'release/v2.11.0' into docs/docker-decision-adrs
LucasSantana-Dev File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,71 @@ | ||
| # ADR — Stay on `node:22-alpine` (not bookworm-slim, not distroless) | ||
|
|
||
| - **Status:** Accepted (flips Phase-1 recommendation) | ||
| - **Date:** 2026-05-13 | ||
| - **Decided by:** `/research-and-decide` composite | ||
|
|
||
| ## Context | ||
|
|
||
| Phase-1 research recommended migrating from `node:22-alpine` to `node:22-bookworm-slim` for better debuggability and to "fix" PR #846's class of native-module breaks. Critic Phase-2 review flipped this recommendation by surfacing the actual root cause of PR #846. | ||
|
|
||
| **Re-examined root cause of PR #846 (verified by reading commit `0eb13d0f` + PR body):** | ||
| - Dependabot PR #831 bumped `Dockerfile.frontend` from `node:22-alpine` to `node:26-alpine`. | ||
| - `Dockerfile.frontend`'s builder runs `npm ci` against the workspace ROOT, which installs `@discordjs/opus` (a bot-only native dep) into the frontend build. | ||
| - `@discordjs/opus@0.10.0` uses `@discordjs/node-pre-gyp@0.4.5`, which ships no Node-26 ABI prebuilt. Falls through to source compile. | ||
| - `node:26-alpine` has no C toolchain → build fails. `docker-publish` was red for 4 days. | ||
|
|
||
| **This is a prebuilt-binary availability problem, not a musl-vs-glibc problem.** Migrating to bookworm-slim would have produced the same failure (Debian-slim also lacks a C toolchain by default). The actual fixes are: | ||
| - Don't install `@discordjs/opus` in the frontend image (separate Dockerfile concerns — covered by ADR `2026-05-13-frontend-dockerfile-keep-separate.md`'s revisit trigger #1). | ||
| - Add build tools when needed (`apk add build-base` or `apt-get install build-essential`). | ||
| - Pin Node major until ecosystem prebuilts catch up. | ||
|
|
||
| ## Decision | ||
|
|
||
| **Stay on `node:22-alpine` for bot + backend production images. Stay on `nginxinc/nginx-unprivileged:1.27-alpine` for frontend + reverse-proxy nginx (already shipped in PR #848).** | ||
|
|
||
| This is a reversal of Phase-1's recommendation. The migration to bookworm-slim was justified by a wrong root cause; the genuine wins (debuggability, glibc) are real but small relative to a solo-operator-with-Claude-Code workflow where observability (Langfuse + OTel + Sentry) reduces the value of `gdb` / `strace` on the image itself. | ||
|
|
||
| ## Alternatives considered | ||
|
|
||
| - **`node:22-bookworm-slim`** — glibc, native `apt`, ships shell + utilities. Real upsides: prebuilt-binary availability is broader, `strace`/`gdb` available. Real downsides: ~200MB larger image (180MB → ~380MB for bot), longer pulls, larger surface for the webhook container that holds docker.sock (per critic). Rejected: no current break Alpine causes, observability stack reduces the human-debuggability case. | ||
| - **`cgr.dev/chainguard/node` (Wolfi)** — Daily CVE cadence is attractive but Wolfi lacks first-party python3 + ffmpeg + yt-dlp packaging. Migration cost is high and creates an unfamiliar base image to maintain solo. Rejected unless a CVE storm forces it. | ||
| - **`gcr.io/distroless/nodejs22-debian12`** — Smallest, glibc, but no shell at all. Incompatible with the current `CMD ["sh", "-c", "npx prisma migrate deploy ..."]` pattern. Would require a build-time entrypoint script. Rejected on operator-legibility grounds. | ||
| - **`FROM scratch`** — Listed for completeness in research. Not viable with `@discordjs/opus` native modules. | ||
|
|
||
| ## Consequences | ||
|
|
||
| **Positive** | ||
| - Zero migration churn on the heels of PR #848. | ||
| - Smallest production images (~180MB bot, ~95MB backend). | ||
| - Consistent base across bot, backend, frontend builder, dev frontend — no per-image quirks. | ||
|
|
||
| **Negative** | ||
| - Native-module ecosystem support on musl remains thinner than glibc. New native deps (e.g., `sharp`, `canvas`) may require `apk add build-base` in the build stage. | ||
| - Debuggability on the image itself is minimal. Observability tooling (Langfuse, OTel, Sentry) must remain the primary diagnostic path — this is now a load-bearing assumption, not a tie-breaker. | ||
| - Node-major bumps from Dependabot must be reviewed for prebuilt availability on Alpine before merging. Lock `node:22-alpine` until `@discordjs/opus` or its successor publishes Node-24/26 prebuilts. | ||
|
|
||
| ## Pilot / adoption plan | ||
|
|
||
| None required (no change). The follow-up work this ADR implies belongs in: | ||
| - ADR `2026-05-13-frontend-dockerfile-keep-separate.md` — trigger #1 covers stopping the frontend from installing bot's native deps. | ||
| - Dependabot config — ensure `Dockerfile.frontend` and `Dockerfile` Node bumps are reviewed by the operator, not auto-merged. | ||
|
|
||
| ## Revisit when | ||
|
|
||
| 1. **`@discordjs/opus` (or its replacement) ships Node-24 prebuilts on `linux-musl-x64`** — verify with `npm view @discordjs/opus@latest dist-tags` + `npm install` in an `alpine:latest` test container. If clean, the case for bumping past Node 22 reopens but does not force a base-image change. | ||
| 2. **Alpine 0-day CVE escalation event** — flip to `node:22-bookworm-slim` or chainguard immediately. Track Alpine security advisories (alpine-security@ ML or `apk audit`). | ||
| 3. **A new bot dependency requires glibc** (e.g., a closed-source vendor SDK with only glibc binaries) → bookworm-slim migration is forced. | ||
| 4. **A bot/backend image exceeds 500MB** — re-evaluate; alpine is no longer paying for itself. | ||
| 5. **Annual review on 2027-05-13** — confirm the assumption holds. | ||
|
|
||
| ## Cross-decision interactions | ||
|
|
||
| - This ADR pairs with `2026-05-13-frontend-dockerfile-keep-separate.md`: keeping Alpine assumes the frontend stops needing the bot's native deps eventually (separate Dockerfile's trigger #1). | ||
| - This ADR is independent of `2026-05-13-orchestration-stay-on-compose.md`: base image and orchestrator are orthogonal at single-node scale. | ||
|
|
||
| ## References | ||
|
|
||
| - PR #846 — `fix(docker): revert frontend image to node:22-alpine` | ||
| - PR #831 — Dependabot Node 26 bump (the trigger) | ||
| - PR #848 — `chore/docker-overhaul` | ||
| - Critic Phase-2 root-cause re-analysis (this composite session) |
64 changes: 64 additions & 0 deletions
64
docs/decisions/2026-05-13-frontend-dockerfile-keep-separate.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,64 @@ | ||
| # ADR — Keep `Dockerfile.frontend` separate (deferred consolidation) | ||
|
|
||
| - **Status:** Superseded by [PR #851 — `refactor/dockerfile-frontend-consolidation`](https://github.com/LucasSantana-Dev/Lucky/pull/851) | ||
| - **Date:** 2026-05-13 | ||
| - **Superseded:** 2026-05-14 | ||
| - **Decided by:** `/research-and-decide` composite | ||
| - **Related:** PR #848 (`chore/docker-overhaul`), PR #846 (Node 26 revert), PR #851 (consolidation) | ||
|
|
||
| ## Context | ||
|
|
||
| The Lucky monorepo builds frontend images via a standalone `Dockerfile.frontend` that runs its own `npm ci` against the workspace root. The main `Dockerfile` builds bot + backend. The two Dockerfiles duplicate dependency installation, `prisma generate`, and shared package build steps. | ||
|
|
||
| PR #848 added a non-trivial `Dockerfile.frontend` (nginx-unprivileged migration, healthcheck, build cache) that diverged further from the main file. PR #846 surfaced a sharp downside of this separation: because the frontend Dockerfile runs `npm ci` at the workspace root, it pulls in the bot's native `@discordjs/opus` dependency, which lacked Node-26 prebuilt binaries on Alpine and broke `docker-publish` for two days. | ||
|
|
||
| The Phase-1 research evaluated five options: | ||
| 1. Status quo — separate Dockerfiles, accept duplication | ||
| 2. Consolidated multi-target Dockerfile | ||
| 3. Shared `deps` stage across Dockerfiles via cross-image COPY | ||
| 4. Pre-build dist in CI and ship a slim nginx | ||
| 5. Turborepo prune to ship the frontend subgraph only | ||
|
|
||
| ## Decision | ||
|
|
||
| **Keep `Dockerfile.frontend` separate for now (option 1). Mark consolidation as 12-month tech debt with a hard re-evaluation trigger.** | ||
|
|
||
| The status quo is defensible *now* because PR #848 just stabilized it (non-root, healthcheck, pinned base) and adding refactor risk inside a security-hardening PR was not worth the blast radius. | ||
|
|
||
| **This decision was superseded on 2026-05-14.** PR #851 (`refactor/dockerfile-frontend-consolidation`) consolidates `Dockerfile.frontend` into the main `Dockerfile` as a `production-frontend` multi-stage target, addressing the root cause identified in the Revisit trigger #1 below (the frontend Dockerfile installing bot native deps). See PR #851 for the full rationale. | ||
|
|
||
| ## Alternatives considered | ||
|
|
||
| - **Consolidated multi-target (option 2)** — Real cache wins on shared dep install, but couples the frontend image's evolution to the bot/backend Dockerfile. Lock-in to docker-compose `target:` syntax. Rejected on operability grounds. | ||
| - **Shared deps stage cross-Dockerfile (option 3)** — Stronger candidate after the critic review surfaced PR #846's actual root cause: the frontend installing the bot's native deps was the *real* problem. Pruning or sharing-then-deduplicating would fix it. Rejected only because the simpler immediate fix — adding `apk add build-base` to the frontend builder or pruning the workspace — has lower blast radius and PR #846's revert already mitigated the bleeding. | ||
| - **Pre-built dist (option 4)** — Loses reproducibility (`docker build .` no longer matches CI output). Rejected. | ||
| - **Turborepo prune (option 5)** — Best long-term answer but pulls in Turborepo as a load-bearing CI dependency for a 4-package monorepo. Premature. | ||
|
|
||
| ## Consequences | ||
|
|
||
| **Positive** | ||
| - Zero churn on PR #848's hardening work. | ||
| - Frontend image evolution stays independent of bot's native-module troubles. | ||
| - Junior-readable Dockerfiles, no cross-file references. | ||
|
|
||
| **Negative** | ||
| - Cache duplication (~10-12s per build, not the 40s originally estimated — confirmed by critic review). | ||
| - Frontend Dockerfile keeps installing the bot's `@discordjs/opus` for no functional reason. If `@discordjs/opus` or another native bot dep breaks again with a new Node bump, the frontend image break recurs. | ||
| - Two `prisma generate` invocations per release. Benign as long as `prisma/schema.prisma` is identical to both builds (it is, single source). | ||
|
|
||
| ## Revisit when | ||
|
|
||
| Hard triggers (any one flips this to option 3 or option 5): | ||
|
|
||
| 1. **PR #846-class break recurs**: another native bot dep ships without prebuilts for the chosen Node version → frontend Dockerfile must stop installing bot deps. *(This trigger was hit — see PR #851.)* | ||
| 2. **Frontend ship cadence ≥5×/week** and bot ship cadence ≤2/month for 4 consecutive weeks → invalidation isolation matters less than cache reuse. | ||
| 3. **CI build time > 8 min** for the frontend image → option 5 (Turborepo prune) becomes justified. | ||
| 4. **12 months elapsed (2027-05-13)** with no other trigger → re-evaluate anyway. Defer-forever is a failure mode. | ||
|
|
||
| ## References | ||
|
|
||
| - PR #848 — `chore/docker-overhaul` | ||
| - PR #846 — `fix(docker): revert frontend image to node:22-alpine` | ||
| - PR #851 — `refactor/dockerfile-frontend-consolidation` (supersedes this ADR) | ||
| - Critic review captured in this composite's Phase 2 | ||
| - `feedback_tbd_release_branches` memory | ||
56 changes: 56 additions & 0 deletions
56
docs/decisions/2026-05-13-orchestration-stay-on-compose.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,56 @@ | ||
| # ADR — Stay on docker-compose v2 (single-node homelab) | ||
|
|
||
| - **Status:** Accepted | ||
| - **Date:** 2026-05-13 | ||
| - **Decided by:** `/research-and-decide` composite | ||
| - **Related:** ADR `2026-05-13-deploy-target-keep-homelab.md`, PR #848 | ||
|
|
||
| ## Context | ||
|
|
||
| Lucky runs on a single-node homelab box behind Cloudflare Tunnel. Current orchestration is docker-compose v2. PR #848 added resource limits, healthchecks, and security hardening but did not change orchestration. The question: is compose still the right runtime, or should we migrate to nomad / k3s / podman-quadlet / docker swarm? | ||
|
|
||
| Phase-1 research evaluated five candidates across 8 dimensions (effort, weekly tax, observability fit, systemd recovery, zero-downtime, volume safety, asleep-failure mode, 12-month ops cost). Critic Phase 2 stress-tested the leading "stay on compose" choice. | ||
|
|
||
| ## Decision | ||
|
|
||
| **Stay on docker-compose v2.** No migration. Re-evaluate only when concrete escalation criteria fire (see Revisit triggers). | ||
|
|
||
| ## Alternatives considered | ||
|
|
||
| - **Nomad single-node** — Strongest runner-up. ~130h migration. Native Prometheus/OTel, declarative rolling reschedule. Rejected because the deploy-target ADR locks homelab single-node and there is no team to onboard. | ||
| - **k3s** — ~156h migration. kubectl skill transferable, but operational overhead high for solo-operator + single-node. | ||
| - **Podman + quadlet** — ~52h migration. Systemd-native, rootless. Critic flagged this was undervalued in research — operator already uses systemd-style hooks (claude-env, launchd). Acknowledged but not enough to flip the decision: compose's existing momentum (PR #848 just shipped 8 commits of hardening) outweighs the migration cost. | ||
| - **Docker Swarm** — ~62h migration. Compose-compatible, multi-node-ready. Rejected: no multi-node need on the roadmap. | ||
|
|
||
| ## Consequences | ||
|
|
||
| **Positive** | ||
| - Zero migration cost. PR #848's hardening immediately pays off. | ||
| - Observability stack (Langfuse + OTel + Grafana, also docker-compose) stays compatible. | ||
| - Operator legibility — `docker compose up -d` is universally understood. | ||
|
|
||
| **Negative — must be acknowledged in this record per critic review** | ||
| 1. **`/var/run/docker.sock` is mounted into the webhook container** (`deploy/Dockerfile` + `docker-compose.yml`). This grants the webhook process effective root on the host. The blast radius is bounded only by "the homelab is a private box" — not by any container-level mitigation. Acceptable under homelab-only deploy posture (ADR `2026-05-13-deploy-target-keep-homelab.md`); would NOT be acceptable on shared infrastructure. Mitigated by pinning `almir/webhook:2.8.3` in PR #848 instead of `:latest`. | ||
| 2. **Voice-drop on bot restart is NOT inherent to single-node** — Discord's gateway supports session resumption. The actual risk is *ungraceful* termination. Solving that needs graceful-shutdown hooks in the bot, not orchestration. Tracked separately as a backend improvement, not as a reason to switch orchestrators. | ||
| 3. **No horizontal escape hatch** — if a service exceeds its `mem_limit` consistently, compose's only response is restart. Nomad/k3s would allow burst-to-secondary. Mitigated by the observability stack now being able to surface OOM patterns before they become incidents. | ||
|
|
||
| ## Pilot / adoption plan | ||
|
|
||
| None required (no change). | ||
|
|
||
| ## Revisit when | ||
|
|
||
| Any one of these triggers a re-evaluation (with `nomad single-node` as the default candidate): | ||
|
|
||
| 1. **OOM kills on any service > 1/month** for two consecutive months → compose's lack of headroom is biting. | ||
| 2. **Single reboot causes > 5 min of public unavailability** (measured via Cloudflare Tunnel uptime) → declarative scheduler needed. | ||
| 3. **Operator needs to scale frontend, backend, or bot to > 1 replica** (e.g., to test rolling deploys without voice drops) → compose can't coordinate. | ||
| 4. **A second machine joins the homelab** for any reason (more storage, hardware redundancy) → swarm or nomad's multi-node value materialises. | ||
| 5. **Observability data over 90 days from 2026-05-13** shows steady-state resource use within 60% of the configured limits *and* no incidents → confirms compose was the right call; reset the clock. | ||
|
|
||
| ## References | ||
|
|
||
| - ADR `docs/decisions/2026-05-13-deploy-target-keep-homelab.md` | ||
| - PR #848 — `chore/docker-overhaul` | ||
| - Memory: `project_homelab_observability_deployed` | ||
| - Critic Phase-2 findings on docker.sock + escalation criteria |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.