feat(watchdog): supervise the netns sidecars, on their own health - #21
Closed
Marinski wants to merge 5 commits into
Closed
feat(watchdog): supervise the netns sidecars, on their own health#21Marinski wants to merge 5 commits into
Marinski wants to merge 5 commits into
Conversation
dockurr/windows keeps its container up while the Windows guest inside may have crashed, so `restart: unless-stopped` never fires and every terminal API in that VM stays dead until a human intervenes. This adds a compose-managed sidecar that watches Docker health and recovers a VM on its own. Recovery is a COORDINATED RECREATE, not a restart ------------------------------------------------- An earlier revision of this branch used `docker restart` through the Docker API, on the reasoning that keeping the owner's container ID keeps a wickworks sidecar's netns attachment intact. That reasoning is wrong, and tests/integration/test_wickworks_lifecycle.py already proves it: Docker tears the netns down on stop and builds a fresh one on start, so restarting the owner alone strands the sidecar exactly as recreating the owner alone does. Only recreating the owner together with its sidecars repairs the binding. So the watchdog shells out to scripts/recreate-vm.sh -- the helper an operator runs by hand, and the one that lifecycle test covers -- rather than reimplementing sidecar discovery. Two recovery paths that could drift apart is precisely what this avoids. Consequences of using compose from inside a container: - The sidecar image now carries the docker CLI, the compose plugin, bash and PyYAML (Dockerfile.watchdog, base still digest-pinned because this container mounts the root-equivalent Docker socket). - Compose resolves this project's relative bind mounts client-side, so the project has to be mounted through at the SAME absolute path the host uses. run.sh exports MT5_PROJECT_DIR; validate_config() reports it at startup when it is missing and the watchdog refuses to act, rather than falling back to a restart that looks like recovery and is not. - COMPOSE_PROJECT_NAME is passed explicitly. Compose otherwise derives the project from the directory name, and a mismatch would not fail -- it would quietly create a second set of containers beside the running ones. - Recovery names the compose SERVICE, taken from the container's com.docker.compose.service label; a container id means nothing to compose. A VM without that label is skipped rather than guessed at. Watchdog behaviour ------------------ - Scoped to this compose project and the dockurr/windows image, so nginx, wickworks, the log rotator and the watchdog itself are never touched. - Acts only after health has stayed unhealthy for a sustained FailingStreak, so a busy VM mid-backtest is never interrupted. - Per-container state on a named volume, exponential backoff between attempts, a bounded attempt budget, and a reset only after sustained health -- so a VM that crashes again immediately is not thrashed. - --dry-run evaluates against a copy of the state, so dry passes cannot consume the real backoff and attempt budget. Full suite passes in the container test image: 458 passed, 2 skipped.
…d; pin pyyaml by hash Recovery is a recreate, which replaces the container - so state keyed by container id was orphaned by the very recovery that wrote it. The next poll saw a fresh id, loaded a fresh record at attempts=0, and the attempt cap and backoff reset themselves on every recovery they were meant to bound: a persistently broken VM was recovered forever, always at 'attempt 1'. State is now keyed by stable compose identity (project + service label), which survives the recreate. The service label is resolved before state is touched; a container without one is skipped up front, since it can neither be recreated nor tracked. Labels are sanitized before becoming a file name. Two regression tests drive the exact replacement-id scenario from review: the attempt cap and the backoff window must both survive the recreate they triggered, with the same service returning under a new container id each pass. Both fail against the previous script. Also from review: Dockerfile.watchdog installed unpinned pyyaml at build time in an image that mounts the root-equivalent Docker socket. The dependency is now pinned by version and hash (requirements-watchdog.txt, pip --require-hashes: musllinux cp312 wheels for x86_64/aarch64 plus the sdist), same trust argument as the digest-pinned base image.
…sy tolerance; continuous healthy clock
The container was given WATCHDOG_PROJECT_DIR but never MT5_PROJECT_DIR, while
docker-compose.yml requires ${MT5_PROJECT_DIR:?} on every compose command. So
recreate-vm.sh's `docker compose` failed at interpolation before it could stop
anything, and no real (non-dry-run) recovery could complete. recreate_env()
now builds the helper's environment explicitly - COMPOSE_PROJECT_NAME and
MT5_PROJECT_DIR from the watchdog's own host path - and run.sh persists the
value to .env (first line, single-quoted) so make down/logs and manual compose
keep working after it exits; run.sh also refuses a stale export from another
checkout, which would otherwise be persisted and then acted on. The compose
files pass the variable through as well.
healthcheck.sh: a port that accepts TCP but never answers HTTP is no longer
healthy forever. The busy tolerance is bounded at HEALTHCHECK_SLOW_GRACE
consecutive checks (default 10), after which the port is reported hung and
DOWN; an answer or a refused connection resets it. An unwritable state dir
degrades to the old tolerance and says so in the verdict.
vm-watchdog.py: healthy_since means continuously healthy - starting, a
sub-threshold unhealthy poll, or no healthcheck all restart the reset clock.
Image filter is an exact repository match, and the watchdog resolves its own
full container id at startup and never selects itself.
Tests: the real recreate-vm.sh runs under the watchdog's exact child
environment with the host variable scrubbed (plus a control proving the
pre-fix environment fails at interpolation); run.sh's actual .env block is
executed; a host integration test drives a real recovery through the built
sidecar on a disposable Compose project. The `assert ... or True` no-op and
the COMPOSE_PROJECT_NAME-only assertion are replaced.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…R too, and must not be silent `docker compose down` interpolates the compose file like every other compose command, so without MT5_PROJECT_DIR it failed at the required-variable check - the very finding this PR round fixes, reproduced by its own harness - and check=False hid that, leaving the disposable project (a socket-mounted watchdog included) running after the suite. The teardown now passes the variable the way an operator's shell does and raises if it fails. The in-container reproduction test also echoes the inner exit code, so its failure is provably the compose run inside the sidecar and not the outer exec. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
On 2026-09-07 the mt5 container on our farm exited cleanly and
`restart: unless-stopped` brought it back. Its wickworks sidecar spent the next
two days in the namespace that restart destroyed: FailingStreak 13,700, only
`lo` left, every /rates/ta call 502-ing. The VM reported healthy throughout.
Docker resolves `network_mode: service:<vm>` ONCE, at the sidecar's start, into
an immutable NetworkMode=container:<owner-id>, and builds a fresh namespace
every time the owner starts. A restart therefore strands the sidecar while
changing nothing an outside observer can see: the container id is the same, the
owner is healthy. Nothing about the binding looks wrong.
The failing streak is the point. The sidecar's own healthcheck saw the fault
the whole time; there was no supervisor for it. So a second sweep applies the
SAME rule as the VM sweep - Docker health, past the same streak, the same
backoff and attempt cap - to the netns sidecars, and recreates the offender
alone. `recreate-vm.sh <sidecar>` finds no sidecars of a sidecar, so that is
exactly one container: the documented repair, what an operator does by hand,
and it leaves the VM and its terminals alone.
It acts only while the owner is a running, HEALTHY VM of this project:
- unhealthy owner -> the VM sweep's job; it recreates owner and sidecars
together, which is the only thing that repairs the
binding, and acting here would race it.
- owner not running -> left alone, and this one is load-bearing.
`/containers/json` lists running containers only, so a
STOPPED owner is indistinguishable from a destroyed one.
Recreating a sidecar under a stopped owner cannot work -
the helper stops it first, then `up --no-deps` has no
namespace to join - so it would end up stopped, invisible
to both sweeps and never retried. `docker compose stop
mt5` for maintenance must not cost you the sidecar.
An earlier draft also recreated a sidecar whose owner id was simply absent from
the running set, on the theory that a dead binding is structural evidence
needing no streak. Counter-review killed it twice over: that is the stopped-VM
hazard above, and the incident it was justified by is not caught by it at all,
because a restart keeps the container id. Health-gating throughout is both
safer and the honest description of what recovers the fault. What is left is
that a netns sidecar needs a healthcheck able to SEE the orphaning -
scripts/wickworks-healthcheck.py is the worked example - and the docs now say
so plainly instead of implying this covers a sidecar that cannot.
Owner references are matched by full id, by a 12-character-or-longer prefix and
by name, because `container:<name>` is legal to write by hand and normalised
nowhere; failing to resolve one means leave alone, never act.
WATCHDOG_WATCH_SIDECARS=0 restores the VM-only scope.
The integration suite gains the whole fault on a disposable Compose project
with nothing faked - built sidecar image, real vm-watchdog.py, real
recreate-vm.sh, real compose, real daemon. It restarts the owner, asserts the
fault first (id unchanged, VM healthy, sidecar without eth0), then asserts the
recovery, including that the VM container id did not change. A second test
holds the owner stopped and asserts the sidecar is left running. 6 passed,
twice, no leaks.
548 passed, lint clean.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
Author
|
Folding this into #15 rather than asking you to review it separately, and closing it here. Same reason as #20: this changes The commit is now on If you would rather review it on its own after all, say so and I will reopen it — the branch is untouched. |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
The fault nothing was watching
On 2026-09-07 the
mt5container on our farm exited cleanly andrestart: unless-stoppedbrought it back. Itswickworkssidecar spent thenext two days in the network namespace that restart destroyed:
FailingStreak13,700, onlyloleft, every/rates/tacall returning 502through the API. The VM reported
healthythe entire time.Docker resolves
network_mode: service:<vm>once, at the sidecar's ownstart, into an immutable
NetworkMode=container:<owner-id>, and it builds afresh namespace every time the owner starts. So a restart strands the
sidecar while changing nothing an outside observer can see: the container id is
unchanged, the owner is healthy, the binding still names a running container.
#13 documented that lifecycle. This is the supervision for it.
The failing streak is the point. The sidecar's own healthcheck saw the fault
the whole time. There was simply no supervisor for it.
The change
A second sweep after the VM sweep, applying the same rule — Docker health,
past the same
WATCHDOG_MIN_FAILING_STREAK, the same backoff and attempt cap —to containers whose
NetworkModenames another container.Recreating the sidecar alone goes through the same
recreate-vm.sh.Nothing declares
network_mode: service:<sidecar>, so the helper's own sidecardiscovery returns nothing for it and the command is exactly
up -d --force-recreate --no-deps <sidecar>: one container. That is thedocumented repair, it is what an operator does by hand, and it does not disturb
the VM or the terminals inside it.
"Not a running project VM" is load-bearing, not a default.
/containers/jsonlists running containers only, so a stopped owner isindistinguishable from a destroyed one. Acting on that would run the helper,
which stops the sidecar first and then cannot start it again —
up --no-depshas no namespace to join — leaving the sidecar stopped, invisible to both
sweeps, and never retried. A
docker compose stop mt5for maintenance must notcost you the sidecar.
Owner references are matched by full id, by a 12-character-or-longer prefix,
and by name, because
container:<name>is legal to write by hand and isnormalised nowhere. Failing to resolve one means leave alone, never act.
WATCHDOG_WATCH_SIDECARS=0restores the VM-only scope.What I got wrong first, and why it is worth saying
My first version also recreated a sidecar whose owner id was simply absent from
the running set, on the theory that a dead binding is structural evidence that
needs no streak. Counter-review killed it twice over.
docker compose stop mt5would have costyou the sidecar, permanently.
restart: unless-stoppedrestart keeps the container id — I verified thisagainst the daemon rather than assuming it — so on 2026-09-07 the binding
still named a running, healthy VM and that branch would never have fired.
What saw the fault was the sidecar's own healthcheck, which is where the
13,700 came from. My code comments, the docs and one test all claimed
otherwise.
So the branch is gone. Health-gating throughout is both safer and the honest
description of what recovers this. The real consequence is a requirement, and
the docs now state it plainly instead of implying it is covered: a netns
sidecar needs a healthcheck that can see the orphaning. One that only probes
loopback stays green inside a dead namespace, and Docker health is this
daemon's only source of truth.
scripts/wickworks-healthcheck.pyis the workedexample — it probes the owner's gateway services, which vanish with the
namespace.
Proof it recovers, not just decides
tests/integration/test_vm_watchdog_lifecycle.pygains the whole fault on adisposable Compose project, with nothing faked in the chain: the sidecar image
built from
Dockerfile.watchdog, the realvm-watchdog.py, the realrecreate-vm.sh, realdocker compose, real daemon.It restarts the owner, which is what
unless-stoppeddid in production,and asserts the fault before asserting the fix: the owner's container id is
unchanged, the VM is healthy, and the sidecar has lost its
eth0. Then itwaits for the watchdog and checks the world — a new sidecar container,
eth0back, healthy again, one attempt recorded under the sidecar's own compose
identity, and the VM container id unchanged, which is what proves only the
sidecar was touched.
A second test holds the owner stopped and asserts the sidecar is left
running and untouched across many sweeps. That is the counter-review's finding,
pinned against a real daemon.
6 passed, twice, ~68s, zero leftover containers or images — after fixing a
teardown race this work introduced, where restarting the owner in a test's
cleanup handed the watchdog a fresh orphan to act on while compose was removing
the project. It recreated the sidecar behind compose's back and left it running.
The fixture now stops the watchdog before
down.Twenty-three unit tests beside it. Deleting any of the guards now fails
something: I re-ran the mutants the counter-review found surviving, and the
two that still survived turned out to be genuinely dead code and were removed
rather than tested.
What was checked
Five phases, receipt kept locally under a gitignored directory, run against
exactly the files this PR touches.
reading: can
NetworkModecarry a container name rather than an id? Ifit could,
owner in running-idswould be false for a healthy sidecar and thesweep would recreate it forever. Started a throwaway pair with
--network container:<NAME>; Docker stores the resolved full id.sidecar never share a record and the healthy VM is never charged.
findings, two of which changed the design.
make test-unit(548) andmake lintare green.Merge order
Stacked on #15, which is still changing
vm-watchdog.py. It shows #15'scommits until that lands, after which it collapses to
scripts/vm-watchdog.py,its tests, the integration test and one docs subsection.
#15 → #20 → this → #16 → #18 → #19 → #10, and I will rebasepromptly as each predecessor merges. I rehearsed it: after #15, this and #20 both merge with zero conflicts, in either order, and the
combined tree is green. Happy to hold this until #15 is merged if you would
rather review it as a plain diff.
No CHANGELOG entry, for the same reason as #20: the watchdog is
unreleased and #15 carries none, so one entry covering all of it belongs on
whichever lands last.
🤖 Generated with Claude Code