Recreate a container when the content of a file it reads at creation changes (hq issue 103) #22

Merged
jschoubben merged 3 commits from fix/recreate-on-content-change into main 2026-09-23 21:44:21 +00:00
Owner

Fixes novox/hq 04-ISSUES/103. Second cut, after review (see the comment below for the point-by-point).

What happened. The store was given a new port; the host rewrote two containers' env-files with it and left both running with the old port. A container reads its env-file at CREATE, not at start, so docker restart does not help — only a recreate does. The spec hash (mesh-host.spec label) covered the env-file's path, not its content.

What the spec now covers, for a running container, by content digest:

  • every env-file
  • a file bind-mounted into it DIRECTLY (a secret, a credential file)

The digest is the wrote hash the store already records for a file the host wrote, read from the state as it stands when the container is reached (files apply before containers in declared order, so a rewrite lands in the same pass). A file the host has no record of — a predecessor's env-file, the superuser secret genesis writes before any declaration names it — is hashed on read.

Deliberately excluded: a bind-mounted DIRECTORY is not looked inside, not even for files the host wrote there — the route proxy re-reads its routes live and provisioner sidecars poll what they receive; restart-on stays the opt-in for a container that reads such a file once at start. Named volumes. A create-once seed digests as the seed, not as what grew in it. Run-once and scheduled steps read their files when they run. A held container on an adopted node is held before any of this is looked at.

Genesis fix. Genesis wrote the superuser secret as value\n; secret accept strips the line ending by design; so the module's superuser.secret was one byte shorter than what the raised store mounted, and phase three would have recreated the store it meant to adopt in place. Genesis now writes the value alone (readCredentialFile tolerated both already). Pinned by a test that uses the bytes the genesis code path writes.

Upgrade. A container carrying a label from before the host folded in what it reads is accepted when that label matches the spec as it used to be computed: what it reads is recorded then, a change is caught from that record from the next apply on, and the label is renewed at the next genuine recreate. Trade-off (stated in the code): a container already stale at upgrade time is not caught — and could not be either way.

Reporting. The store records what each container was created reading (reads, per path), looked up by declared id and by container NAME when the id has no record (bundle store → module postgres.server), so the outcome says recreated: <file> changed; the apply log line now carries the outcome's detail.

Tests. internal/apply/reads_test.go (against the machine fake, which replays the label the host gave it) and internal/bootstrap/adopt_in_place_test.go. Each of the positive ones is red with its fix reverted:

  • env-file content change recreates, names the file in report and log, unchanged re-apply is quiet — red on main
  • an env-file the host did not write is hashed on read — red on main
  • a mounted secret file's content change recreates — red on main
  • a mounted directory is not looked inside: service data no; host-written config under it no; the same config named in restart-on yes
  • a pre-upgrade label is accepted, recorded, and a later env-file change is caught and relabels — red with the seeding disabled
  • a container adopted under a new id still names the changed file — red with the by-name lookup disabled
  • the store genesis raised is adopted in place by the module's declaration, from the real genesis bytes — red with value\n restored
  • a held container on an adopted node is not recreated by a changed held file — a pin (passes on main too)

go build ./... && go vet ./... && go test -count=1 ./... green. gofmt -l reports internal/bootstrap/control_test.go, pre-existing on main and untouched.

Known and accepted: rolling the host back to a version without reads recreates each such container once (the old code sees a label with file lines it does not compute). An unrecorded :rw file mount that the service itself rewrites would flap; none in the catalogue today.

Fixes novox/hq 04-ISSUES/103. Second cut, after review (see the comment below for the point-by-point). **What happened.** The store was given a new port; the host rewrote two containers' env-files with it and left both running with the old port. A container reads its env-file at CREATE, not at start, so `docker restart` does not help — only a recreate does. The spec hash (`mesh-host.spec` label) covered the env-file's *path*, not its content. **What the spec now covers**, for a running container, by content digest: - every `env-file` - a file bind-mounted into it DIRECTLY (a secret, a credential file) The digest is the `wrote` hash the store already records for a file the host wrote, read from the state as it stands when the container is reached (files apply before containers in declared order, so a rewrite lands in the same pass). A file the host has no record of — a predecessor's env-file, the superuser secret genesis writes before any declaration names it — is hashed on read. **Deliberately excluded:** a bind-mounted DIRECTORY is not looked inside, not even for files the host wrote there — the route proxy re-reads its routes live and provisioner sidecars poll what they receive; `restart-on` stays the opt-in for a container that reads such a file once at start. Named volumes. A create-once seed digests as the seed, not as what grew in it. Run-once and scheduled steps read their files when they run. A held container on an adopted node is held before any of this is looked at. **Genesis fix.** Genesis wrote the superuser secret as `value\n`; `secret accept` strips the line ending by design; so the module's `superuser.secret` was one byte shorter than what the raised store mounted, and phase three would have recreated the store it meant to adopt in place. Genesis now writes the value alone (`readCredentialFile` tolerated both already). Pinned by a test that uses the bytes the genesis code path writes. **Upgrade.** A container carrying a label from before the host folded in what it reads is accepted when that label matches the spec as it used to be computed: what it reads is recorded then, a change is caught from that record from the next apply on, and the label is renewed at the next genuine recreate. Trade-off (stated in the code): a container already stale at upgrade time is not caught — and could not be either way. **Reporting.** The store records what each container was created reading (`reads`, per path), looked up by declared id and by container NAME when the id has no record (bundle `store` → module `postgres.server`), so the outcome says `recreated: <file> changed`; the apply log line now carries the outcome's detail. **Tests.** `internal/apply/reads_test.go` (against the `machine` fake, which replays the label the host gave it) and `internal/bootstrap/adopt_in_place_test.go`. Each of the positive ones is red with its fix reverted: - env-file content change recreates, names the file in report and log, unchanged re-apply is quiet — red on main - an env-file the host did not write is hashed on read — red on main - a mounted secret file's content change recreates — red on main - a mounted directory is not looked inside: service data no; host-written config under it no; the same config named in `restart-on` yes - a pre-upgrade label is accepted, recorded, and a later env-file change is caught and relabels — red with the seeding disabled - a container adopted under a new id still names the changed file — red with the by-name lookup disabled - the store genesis raised is adopted in place by the module's declaration, from the real genesis bytes — red with `value\n` restored - a held container on an adopted node is not recreated by a changed held file — a pin (passes on main too) `go build ./... && go vet ./... && go test -count=1 ./...` green. `gofmt -l` reports `internal/bootstrap/control_test.go`, pre-existing on main and untouched. **Known and accepted:** rolling the host back to a version without `reads` recreates each such container once (the old code sees a label with file lines it does not compute). An unrecorded `:rw` file mount that the service itself rewrites would flap; none in the catalogue today.
Author
Owner

Review response — second cut is 8ad86bf. Point by point:

1 (must) — directory-mount expansion: changed. reads() now folds only env-files and files mounted DIRECTLY; a directory mount is not looked inside, not even for host-written files. Reasoning accepted in full: the route proxy re-reads routes/mesh.json live, provisioner sidecars poll MESH_RECEIVES, and restart-on already is the opt-in. store.FilesUnder is gone (replaced by store.At(kind, target)). The test that asserted the regressing behaviour is rewritten as TestAMountedDirectoryIsNotLookedInside: service data under the mount — no recreate; host-written config under the mount — no recreate; the same config named in restart-on — recreate. The declaration.go RestartOn comment says the same.

2 (must) — genesis trailing newline: changed, and the claim was false as you said. keptOrMade now writes value with no line ending; readCredentialFile already trimmed either. New internal/bootstrap/adopt_in_place_test.go calls the real keptOrMade for the bytes, raises the store from a bundle-shaped declaration (file mounted, undeclared), then applies the module's declaration with the file as secret accept takes it (TrimRight("\r\n"), same as the controller's asSupplied) and the same container: must be unchanged, no rm/run. Verified red with value+"\n" restored.

3 (must) — upgrade restart storm: changed. A label matching the spec as it used to be computed (no file lines) is accepted when there is no reads record yet, or when the record equals what is read now; reads is recorded on acceptance; a later file change is caught from the record and the label is renewed on that recreate. Trade-off stated in the code comment: a container already stale at upgrade time is not caught, and could not be either way. The "recreated: what it reads was not on record" detail is gone with it. Verified red with the seeding disabled.

4 (should) — lookup by name: changed. When the declared id has no record, the previous reads come from the record under the container's NAME. Doing this surfaced a real bug in the by-target lookup: after adoption two file records share one path (bundle env, module postgres.env) and the first match had the stale wrote; store.At now returns the most recently applied record. Verified red with the by-name lookup disabled.

Noted items: rollback recreates once and :rw flapping — stated in the PR body as accepted. Duplicate targets in FilesUnder — moot, removed.

Disputed: nothing. Every finding held up when reproduced.

Review response — second cut is `8ad86bf`. Point by point: **1 (must) — directory-mount expansion: changed.** `reads()` now folds only env-files and files mounted DIRECTLY; a directory mount is not looked inside, not even for host-written files. Reasoning accepted in full: the route proxy re-reads `routes/mesh.json` live, provisioner sidecars poll `MESH_RECEIVES`, and `restart-on` already is the opt-in. `store.FilesUnder` is gone (replaced by `store.At(kind, target)`). The test that asserted the regressing behaviour is rewritten as `TestAMountedDirectoryIsNotLookedInside`: service data under the mount — no recreate; host-written config under the mount — no recreate; the same config named in `restart-on` — recreate. The `declaration.go` RestartOn comment says the same. **2 (must) — genesis trailing newline: changed, and the claim was false as you said.** `keptOrMade` now writes `value` with no line ending; `readCredentialFile` already trimmed either. New `internal/bootstrap/adopt_in_place_test.go` calls the real `keptOrMade` for the bytes, raises the store from a bundle-shaped declaration (file mounted, undeclared), then applies the module's declaration with the file as `secret accept` takes it (`TrimRight("\r\n")`, same as the controller's `asSupplied`) and the same container: must be `unchanged`, no `rm`/`run`. Verified red with `value+"\n"` restored. **3 (must) — upgrade restart storm: changed.** A label matching the spec as it used to be computed (no file lines) is accepted when there is no `reads` record yet, or when the record equals what is read now; `reads` is recorded on acceptance; a later file change is caught from the record and the label is renewed on that recreate. Trade-off stated in the code comment: a container already stale at upgrade time is not caught, and could not be either way. The "recreated: what it reads was not on record" detail is gone with it. Verified red with the seeding disabled. **4 (should) — lookup by name: changed.** When the declared id has no record, the previous `reads` come from the record under the container's NAME. Doing this surfaced a real bug in the by-target lookup: after adoption two file records share one path (bundle `env`, module `postgres.env`) and the first match had the stale `wrote`; `store.At` now returns the most recently applied record. Verified red with the by-name lookup disabled. **Noted items:** rollback recreates once and `:rw` flapping — stated in the PR body as accepted. Duplicate targets in `FilesUnder` — moot, removed. **Disputed:** nothing. Every finding held up when reproduced.
jschoubben added 3 commits 2026-09-23 21:42:43 +00:00
The host decided whether a container was still the one declared by a digest
of its declaration, and the declaration names an env-file's path and a
mount's path — never what is in them. So when the store was given a new
port, the host rewrote the forge's and the analytics service's environment
files, correctly, and left both containers running with the old port in
their environment: a container reads its env-file when it is CREATED, and
`docker restart` hands it the same environment again. Both looked healthy
until they answered 502.

What a running container takes in at creation is now part of its spec, by
content: every env-file, a file bind-mounted into it, and every file this
host wrote at or under a directory bind-mounted into it — the secrets,
bindings and configs under a module's state directories. The digest is the
one the store already records for a file the host wrote (`wrote`), read
from the state as it stands when the container is reached, so a file
rewritten earlier in the same apply is already the new one; a file the host
has no record of — an env-file a predecessor left, the superuser secret
genesis writes before any declaration names it — is read from disk, which
is what keeps adopting a running store in place a reconcile and not a
recreate.

Deliberately not part of it: what else is in a bind-mounted directory,
which is the service's own data and changes while it runs; a named volume;
a seed created once, which digests as the seed the host wrote and not as
what has grown in it; and a step — a run-once or scheduled container reads
its files when it runs and runs fresh each time. On an adopted node a held
container is held before any of this is looked at.

The host records what each container was created reading, per file, so
the recreate can say which file changed — "recreated: <file> changed" in
the report and, now with its detail, in the log. A container made before
this record existed is recreated once and says so.

novox/hq 04-ISSUES/103
Review of the first cut found four things.

A directory mounted into a container is no longer looked inside, not even
for the files this host wrote there. The controller records every
provider's received and contributions file as a plain file under a mounted
directory, so folding those in would have recreated the route proxy — which
re-reads its routes live, by design — on every route change, and killed
every provisioner sidecar, which polls what it receives, mid-reconcile on
every grant. Whether a service reads a file under its directory once or
watches it is the service's; restart-on is how a module says "once", and it
stays the opt-in. Env-files and files mounted directly remain by content.

Genesis wrote the superuser secret as `value\n`; `secret accept` strips the
line ending by design, so the postgres module declared `value` — and with
a mounted file's content in the spec, phase three would have recreated the
store it meant to adopt in place, with the temporary control plane
connected to it. Genesis now writes the value alone. readCredentialFile
tolerated both endings already. Pinned with the bytes the genesis code
path writes, then the module's declaration of the same container: it must
reconcile.

A container carrying a label from before the host folded in what it reads
is accepted rather than recreated, when that label matches the spec as it
used to be computed: what it reads is recorded then, a change is caught
from that record from the next apply on, and the label is renewed at the
next genuine recreate. Recreating them all would have been a restart storm
across the mesh in declaration order, the store first. The trade-off is
stated in the code: a container already stale at upgrade time is not
caught, and could not have been either way.

The record of what a container read is looked up by its name when its
declared id has none — the bundle's `store` becomes `postgres.server` for
the same container — so a change on the day it is adopted still names the
file. The by-target lookup takes the most recently applied record, since
the bundle's record for the same target is never removed by the mesh's.

novox/hq 04-ISSUES/103
The plan says what an apply would change from the declaration and the
record, before the machine is touched. The apply now recreates a container
when the content of a file it reads at creation changed, and the plan said
"check" for every recorded container — true, but a preview that hides the
one step somebody asked about.

So a recorded container whose record of what it read differs from what this
apply will hand it — a plain file declared here, by its declared content;
otherwise what this host last wrote at that path — is planned as an update
naming the file, the same comparison applyContainer makes. What the record
cannot settle stays a check: a file neither declared nor recorded is read
from the machine by the apply, not by the plan; and a container with no
record of what it read was labelled before the host kept that record and is
accepted as it is.

novox/hq 04-ISSUES/103, 104
jschoubben force-pushed fix/recreate-on-content-change from 8ad86bf50f to 4840e21405 2026-09-23 21:42:43 +00:00 Compare
Author
Owner

Rebased onto main after #23 (977df39); force-pushed as 4840e21. The rebase applied cleanly — no conflict hunks — and both sides coexist: store.At beside Mode/Genesis/Lock, the newline fix in rootsecrets.go under the new ApplyBundle, inputs threaded through every ApplyKeeping caller (cmd/mesh-host/main.go, internal/bootstrap/apply.go).

One addition so apply.Plan agrees with ApplyKeeping: a recorded container whose reads record differs from what the apply will hand it (a plain file declared here by its declared content, else the last wrote at that path) is planned as update — recreated: <file> changed since it was created. What the record cannot settle stays check: a file neither declared nor recorded (the apply reads that from the machine, the plan does not), and a container with no reads record — the pre-upgrade label the seeding rule accepts. Test TestAPlanSaysAContainerIsRecreatedWhenAFileItReadsChanged; red with the comparison disabled.

go build ./... && go vet ./... && gofmt -l . && go test -count=1 ./... green (gofmt still only lists the pre-existing internal/bootstrap/control_test.go). Not merged.

Rebased onto main after #23 (`977df39`); force-pushed as `4840e21`. The rebase applied cleanly — no conflict hunks — and both sides coexist: `store.At` beside `Mode`/`Genesis`/`Lock`, the newline fix in `rootsecrets.go` under the new `ApplyBundle`, `inputs` threaded through every `ApplyKeeping` caller (`cmd/mesh-host/main.go`, `internal/bootstrap/apply.go`). One addition so `apply.Plan` agrees with `ApplyKeeping`: a recorded container whose `reads` record differs from what the apply will hand it (a plain file declared here by its declared content, else the last `wrote` at that path) is planned as `update — recreated: <file> changed since it was created`. What the record cannot settle stays `check`: a file neither declared nor recorded (the apply reads that from the machine, the plan does not), and a container with no `reads` record — the pre-upgrade label the seeding rule accepts. Test `TestAPlanSaysAContainerIsRecreatedWhenAFileItReadsChanged`; red with the comparison disabled. `go build ./... && go vet ./... && gofmt -l . && go test -count=1 ./...` green (gofmt still only lists the pre-existing `internal/bootstrap/control_test.go`). Not merged.
jschoubben merged commit 83306b2dba into main 2026-09-23 21:44:21 +00:00
jschoubben deleted branch fix/recreate-on-content-change 2026-09-23 21:44:21 +00:00
Sign in to join this conversation.
No Reviewers
No labels
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: novox/mesh-host#22