-
Notifications
You must be signed in to change notification settings - Fork 2
fix(agent-server): make DB sidecars reachable by name, isolate DEV/PROD data #24
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| --- | ||
| "@appx-org/agent-server": minor | ||
| --- | ||
|
|
||
| Make DB/cache sidecars actually work, with DEV/PROD data isolation. | ||
|
|
||
| The outer image installed podman with `--no-install-recommends`, which drops the | ||
| CNI `dnsname` plugin — so sibling containers on a `podman network create` | ||
| network could never be reached by container name, breaking the deploy-app | ||
| skill's entire multi-container section. The plugin is now installed explicitly. | ||
|
|
||
| The skill's multi-container section now mandates per-environment sidecars | ||
| (`<project>-db-dev` / `<project>-db-prod`) with per-environment data volumes | ||
| (`<project>-db-dev-data` / `<project>-db-prod-data`), states that redeploys | ||
| replace containers but never remove data volumes, and requires migrations | ||
| instead of drop-and-recreate once data persists. Drift-guard tests pin the new | ||
| conventions, and `container-smoke.sh` gains a deterministic sidecar step | ||
| (labelled network + DNS by name + volume durability) plus delete-reap checks | ||
| for the labelled network and volume. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -119,19 +119,41 @@ Then report the relevant **public URL** (`dev.url` after a DEV deploy, | |
|
|
||
| ## Multi-container apps (db, cache, etc.) | ||
|
|
||
| If the app needs a database or other service, run them as sibling containers | ||
| named `<project>-db` etc. on a shared `<project>` network. **Only the app | ||
| container publishes a host port**, in the same two-number form as above; siblings | ||
| are reached by container name over the shared network. Secrets for those services | ||
| are app config, never LLM keys. | ||
|
|
||
| Label the network and any named volume as well, so they are reaped with the | ||
| If the app needs a database or other service, run it as sibling containers on a | ||
| shared `<project>` network — **one sidecar per environment**, mirroring the | ||
| app's DEV/PROD split: `<project>-db-dev` and `<project>-db-prod`. **Only the | ||
| app containers publish host ports**, in the same two-number form as above; | ||
| siblings are reached by container name over the shared network (the DEV app | ||
| connects to `<project>-db-dev`, the PROD app to `<project>-db-prod`). Secrets | ||
| for those services are app config, never LLM keys. | ||
|
|
||
| Label the network and every named volume as well, so they are reaped with the | ||
| project: | ||
|
|
||
| ```bash | ||
| $APP_CONTAINER_RUNTIME network create --label appx.project="$PROJECT" <project> | ||
| $APP_CONTAINER_RUNTIME volume create --label appx.project="$PROJECT" <project>-db-data | ||
| $APP_CONTAINER_RUNTIME run -d --name <project>-db \ | ||
| $APP_CONTAINER_RUNTIME volume create --label appx.project="$PROJECT" <project>-db-dev-data | ||
| $APP_CONTAINER_RUNTIME run -d --name <project>-db-dev \ | ||
| --label appx.project="$PROJECT" \ | ||
| --network <project> -v <project>-db-data:/var/lib/postgresql/data <image> | ||
| --network <project> -v <project>-db-dev-data:/var/lib/postgresql/data <image> | ||
| ``` | ||
|
|
||
| and the same for PROD with `<project>-db-prod` + `<project>-db-prod-data`. Run | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. medium — This sentence is the only place the app is told to join the project network. The canonical command blocks don't do it: deploy/redeploy (lines 72–77) and promote (lines 88–92) still run the app with just An agent that follows §2 literally on the next iterate loop — Put |
||
| the app containers with `--network <project>` too, so they can reach their | ||
| sidecars. | ||
|
|
||
| ### Persistent data rules | ||
|
|
||
| - **DEV and PROD never share a database or a volume.** The user iterates | ||
| against DEV; a bad DEV migration or a data reset must not be able to touch | ||
| PROD's real data. That isolation is exactly why the volumes are | ||
| `<project>-db-dev-data` / `<project>-db-prod-data`, never one shared volume. | ||
| - **Redeploy replaces containers, never data volumes.** A redeploy or promote | ||
| may `rm -f` and re-run the `-db-dev`/`-db-prod` *containers*, but must reuse | ||
| the existing named volume. Never run `volume rm` on a data volume when | ||
| redeploying or "cleaning up" — that is user data loss. Volumes are removed | ||
| only when the project itself is deleted (the reaper finds them by label). | ||
| - **Schema changes are migrations, not resets.** Once data persists across | ||
| redeploys, never "drop and recreate" the database to apply a schema change — | ||
| apply migrations against the existing data. Prove a migration on DEV first, | ||
| then promote and run the same migration against PROD's volume. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -183,6 +183,31 @@ check "DEV bundle now contains the marker" bundle_contains "$DEV_PORT" "CSMOKE_M | |
| check "PROD bundle does NOT contain the marker (untouched)" \ | ||
| bundle_lacks "$PROD_PORT" "CSMOKE_MARKER_V2" | ||
|
|
||
| # ── 7b. db-sidecar mechanics: network DNS + durable per-env volume ─────────── | ||
| # The deploy-app skill's multi-container section: sidecars live on a labelled | ||
| # `<project>` network and are reached BY NAME (needs the CNI dnsname plugin — | ||
| # a podman Recommends that --no-install-recommends drops; the Dockerfile | ||
| # installs it explicitly, and this step guards that from regressing). Data | ||
| # lives in per-env named volumes that survive container replacement. Reuses | ||
| # the app image as the "sidecar" so the step pulls nothing new. | ||
|
|
||
| echo "[7b] db sidecar: labelled network, DNS by name, durable volume" | ||
| check "podman network create (labelled)" \ | ||
| outer_exec podman network create --label "appx.project=${PROJECT}" "${PROJECT}" | ||
| check "podman volume create (labelled, per-env name)" \ | ||
| outer_exec podman volume create --label "appx.project=${PROJECT}" "${PROJECT}-db-dev-data" | ||
| check "run sidecar ${PROJECT}-db-dev on the network with the volume" \ | ||
| outer_exec podman run -d --name "${PROJECT}-db-dev" --label "appx.project=${PROJECT}" \ | ||
| --network "${PROJECT}" -v "${PROJECT}-db-dev-data:/data" "${PROJECT}-app:dev" | ||
| check "sibling on the network reaches the sidecar BY NAME (dnsname)" \ | ||
| outer_exec podman run --rm --network "${PROJECT}" "${PROJECT}-app:dev" \ | ||
| sh -c "for i in 1 2 3 4 5; do wget -q -O /dev/null http://${PROJECT}-db-dev:${APP_PORT}/ && exit 0; sleep 1; done; exit 1" | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. low — busybox There's no So if the CNI bridge is misconfigured such that packets are blackholed rather than refused — precisely the failure class this step exists to catch — each of the 5 attempts blocks for 900 s. The job hits the workflow's
|
||
| # Redeploy rule: replacing the container must not touch the data volume. | ||
| check "volume data survives sidecar replacement" \ | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. low/medium — this leaves the network empty and the volume unused, so the new step-9 checks pass trivially. The durability check
Re-running the sidecar after the durability check (a one-liner, image already built) makes DELETE reap a network with a live endpoint and a volume that's in use, which is what step 9 is meant to prove. |
||
| bash -c "docker exec $NAME podman run --rm --user 0 -v ${PROJECT}-db-dev-data:/data ${PROJECT}-app:dev sh -c 'echo CSMOKE_DB_V1 > /data/marker' \ | ||
| && docker exec $NAME podman rm -f ${PROJECT}-db-dev > /dev/null \ | ||
| && docker exec $NAME podman run --rm --user 0 -v ${PROJECT}-db-dev-data:/data ${PROJECT}-app:dev sh -c 'grep -q CSMOKE_DB_V1 /data/marker'" | ||
|
|
||
| # ── 8. restart survival + recovery ─────────────────────────────────────────── | ||
|
|
||
| echo "[8] outer restart: registry + workspace survive, podman start --all recovers" | ||
|
|
@@ -227,6 +252,13 @@ check "DEV port ${DEV_PORT} refuses connections" \ | |
| check "PROD port ${PROD_PORT} refuses connections" \ | ||
| bash -c "! curl -fsS --max-time 5 http://127.0.0.1:${PROD_PORT}/ > /dev/null 2>&1" | ||
|
|
||
| # The 7b network + volume carry the appx.project label, so the reaper must | ||
| # remove them too (dependency order: containers → networks → volumes). | ||
| check "labelled network is reaped" \ | ||
| bash -c "! docker exec $NAME podman network ls --format '{{.Name}}' | grep -qx '${PROJECT}'" | ||
| check "labelled data volume is reaped" \ | ||
| bash -c "! docker exec $NAME podman volume ls --format '{{.Name}}' | grep -qx '${PROJECT}-db-dev-data'" | ||
|
|
||
| check "project metadata + working dir are gone" \ | ||
| bash -c "! curl -fsS -H 'Authorization: Bearer ${TOKEN}' http://127.0.0.1:4001/v1/projects | grep -q ${PROJECT} \ | ||
| && ! docker exec $NAME test -d /workspace/${PROJECT}" | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
medium — no per-env DB host mechanism, so the promoted PROD container talks to the DEV database.
Per-environment sidecars require a per-environment DB host at run time. But the contract mandates "dev = prod, one Dockerfile, one build target" and "DEV and PROD differ only by image tag, container name, and host port" — and the skill shows no
-eor env mechanism anywhere. The only mention of-eis the prohibition two bullets down ("Never pass secrets into app containers... intorunwith-e"), which reads as "don't use-e" if you're an LLM skimming for the rule.The likely agent behaviour is baking
<project>-db-devinto the image or app config. Promote then runs that same source as<project>-app-prod, which connects to the DEV database — exactly the cross-environment corruption this PR exists to prevent, and now with PROD writing to DEV's volume.Add an explicit per-environment example (
-e DATABASE_URL=postgres://...@<project>-db-dev:5432/appfor DEV,-db-prodfor PROD) and state plainly that app config does go in via-e— it's LLM keys specifically that must not.