diff --git a/Dockerfile.test b/Dockerfile.test index f4f7445..2ca9e6d 100644 --- a/Dockerfile.test +++ b/Dockerfile.test @@ -25,7 +25,7 @@ COPY tests ./tests # docker-compose.yml.j2 is here because the compose-generation test renders the # REAL template — a stub would assert nothing about what actually ships. COPY requirements-api.txt requirements-mcpunifier.txt docker-compose.yml.example docker-compose.yml.j2 run.sh ./ -COPY scripts/config_helper.py scripts/start.bat scripts/check_health.py scripts/healthcheck.sh scripts/verify_binaries.py scripts/wickworks-healthcheck.py scripts/recreate-vm.sh ./scripts/ +COPY scripts/config_helper.py scripts/start.bat scripts/check_health.py scripts/healthcheck.sh scripts/verify_binaries.py scripts/wickworks-healthcheck.py scripts/vm-watchdog.py scripts/recreate-vm.sh ./scripts/ COPY assets/binaries.lock.json ./assets/ ENV PYTHONPATH=/app diff --git a/Dockerfile.watchdog b/Dockerfile.watchdog new file mode 100644 index 0000000..2fcbd60 --- /dev/null +++ b/Dockerfile.watchdog @@ -0,0 +1,30 @@ +# VM crash watchdog. +# +# Recovery is a coordinated compose recreate (VM + the sidecars sharing its +# netns), not a Docker-API restart — see scripts/vm-watchdog.py for why a +# restart strands the sidecar. That means this image needs the docker CLI and +# the compose plugin, plus bash and PyYAML for scripts/recreate-vm.sh, which is +# the helper it shells out to. +# +# Pinned by digest, not tag: this container mounts the Docker socket, which is +# root-equivalent on the host, and a moving tag would hand that to whatever the +# registry serves tomorrow. The digest is the multi-arch OCI index for +# python:3.12-alpine, so it still resolves per-platform. +# To update: docker buildx imagetools inspect python:3.12-alpine +FROM python:3.12-alpine@sha256:d09d15e60962ca365d1cd544a48773bac9d33f2fb1b00f2aa0deec78ade7dc31 + +# docker-cli-compose provides `docker compose`; recreate-vm.sh is bash and +# parses the compose file with PyYAML. The Python dependency is pinned by +# version and hash (see requirements-watchdog.txt for why); --require-hashes +# makes pip fail closed on anything not in that file. +COPY requirements-watchdog.txt /tmp/requirements-watchdog.txt +RUN apk add --no-cache bash docker-cli docker-cli-compose \ + && pip install --no-cache-dir --require-hashes -r /tmp/requirements-watchdog.txt \ + && rm /tmp/requirements-watchdog.txt + +ENV PYTHONUNBUFFERED=1 \ + PYTHONDONTWRITEBYTECODE=1 + +# The watchdog script and recreate-vm.sh stay bind-mounted by compose rather +# than baked in, so a fix to either is a container restart and not a rebuild. +CMD ["python", "-u", "/vm-watchdog.py"] diff --git a/docker-compose.yml.example b/docker-compose.yml.example index 5503421..042882e 100644 --- a/docker-compose.yml.example +++ b/docker-compose.yml.example @@ -91,21 +91,48 @@ services: - ./scripts/rotate-logs.sh:/rotate.sh:ro command: ["sh", "/rotate.sh"] - # nginx is the single entry point for all terminal APIs. Routes - # ///... to mt5: (per-terminal Python - # API process inside the Windows VM, reachable via mt5 container's - # iptables DNAT). Auto-generated from config/config.yaml by run.sh. - # Bound to 127.0.0.1:8888 so it's loopback-only by default — LAN - # exposure is opt-in (change the host bind), tailnet exposure is via - # the optional tailscale sidecar below. - # Unified MCP endpoint. One MCP session that reaches every terminal, with - # broker/account as tool parameters, instead of one endpoint per terminal. - # The per-terminal ///mcp endpoints keep working unchanged; - # nginx routes /mcp/ here. - # - # Reads the same config/config.yaml that generates the nginx routing, so it - # cannot route somewhere nginx does not. It never waits for terminals: a - # terminal that is down fails only the calls naming it. + # VM crash watchdog. dockurr/windows keeps the container up while the + # Windows guest may have crashed internally, so restart: unless-stopped + # never fires and every terminal API in that VM stays dead. This sidecar + # polls Docker health through the socket and restarts a VM only after its + # health has stayed unhealthy for a sustained FailingStreak, with + # exponential backoff and bounded retries; state lives on a named volume. + # Runs inside the compose project (docker compose up -d), no host cron. + # See scripts/vm-watchdog.py. + vm-watchdog: + # Needs the docker CLI + compose plugin to run scripts/recreate-vm.sh, so + # it is built rather than pulled. The base image is digest-pinned inside + # the Dockerfile (this container mounts the Docker socket). + build: + context: . + dockerfile: Dockerfile.watchdog + restart: unless-stopped + command: ["python", "-u", "/vm-watchdog.py"] + volumes: + - /var/run/docker.sock:/var/run/docker.sock + - ./scripts/vm-watchdog.py:/vm-watchdog.py:ro + - vm-watchdog-state:/state + # The project itself, at the SAME absolute path the host uses. Compose + # resolves the relative bind mounts in this file client-side, so a + # different path in here would rewrite every mount to somewhere that + # does not exist on the host. run.sh exports MT5_PROJECT_DIR; the + # watchdog refuses to act (and says so at startup) if it is unset. + - ${MT5_PROJECT_DIR:?MT5_PROJECT_DIR must be the absolute host path of this project (run.sh exports it; otherwise set it in .env)}:${MT5_PROJECT_DIR}:ro + environment: + WATCHDOG_STATE_DIR: /state + WATCHDOG_PROJECT_DIR: ${MT5_PROJECT_DIR} + WATCHDOG_RECREATE_SCRIPT: ${MT5_PROJECT_DIR}/scripts/recreate-vm.sh + # The helper the watchdog runs calls `docker compose`, which interpolates + # ${MT5_PROJECT_DIR:?} in THIS file again. The watchdog sets it in the + # helper's environment itself; it is handed through here as well so a + # human running `docker compose exec vm-watchdog …/recreate-vm.sh` gets + # the same environment the watchdog uses. + MT5_PROJECT_DIR: ${MT5_PROJECT_DIR} + logging: + driver: json-file + options: + max-size: "10m" + max-file: "3" mcpunifier: build: context: . @@ -198,3 +225,8 @@ services: # - NET_RAW # depends_on: # - nginx + +volumes: + # Persistent per-container watchdog state (last restart, attempts, + # healthy-since) so backoff survives the watchdog's own restarts. + vm-watchdog-state: diff --git a/docker-compose.yml.j2 b/docker-compose.yml.j2 index 31c4095..2dbc014 100644 --- a/docker-compose.yml.j2 +++ b/docker-compose.yml.j2 @@ -120,6 +120,48 @@ services: - ./scripts/rotate-logs.sh:/rotate.sh:ro command: ["sh", "/rotate.sh"] + # VM crash watchdog. dockurr/windows keeps the container up while the + # Windows guest may have crashed internally, so restart: unless-stopped + # never fires and every terminal API in that VM stays dead. This sidecar + # polls Docker health through the socket and restarts a VM only after its + # health has stayed unhealthy for a sustained FailingStreak, with + # exponential backoff and bounded retries; state lives on a named volume. + # Runs inside the compose project (docker compose up -d), no host cron. + # See scripts/vm-watchdog.py. + vm-watchdog: + # Needs the docker CLI + compose plugin to run scripts/recreate-vm.sh, so + # it is built rather than pulled. The base image is digest-pinned inside + # the Dockerfile (this container mounts the Docker socket). + build: + context: . + dockerfile: Dockerfile.watchdog + restart: unless-stopped + command: ["python", "-u", "/vm-watchdog.py"] + volumes: + - /var/run/docker.sock:/var/run/docker.sock + - ./scripts/vm-watchdog.py:/vm-watchdog.py:ro + - vm-watchdog-state:/state + # The project itself, at the SAME absolute path the host uses. Compose + # resolves the relative bind mounts in this file client-side, so a + # different path in here would rewrite every mount to somewhere that + # does not exist on the host. run.sh exports MT5_PROJECT_DIR; the + # watchdog refuses to act (and says so at startup) if it is unset. + - ${MT5_PROJECT_DIR:?MT5_PROJECT_DIR must be the absolute host path of this project (run.sh exports it; otherwise set it in .env)}:${MT5_PROJECT_DIR}:ro + environment: + WATCHDOG_STATE_DIR: /state + WATCHDOG_PROJECT_DIR: ${MT5_PROJECT_DIR} + WATCHDOG_RECREATE_SCRIPT: ${MT5_PROJECT_DIR}/scripts/recreate-vm.sh + # The helper the watchdog runs calls `docker compose`, which interpolates + # ${MT5_PROJECT_DIR:?} in THIS file again. The watchdog sets it in the + # helper's environment itself; it is handed through here as well so a + # human running `docker compose exec vm-watchdog …/recreate-vm.sh` gets + # the same environment the watchdog uses. + MT5_PROJECT_DIR: ${MT5_PROJECT_DIR} + logging: + driver: json-file + options: + max-size: "10m" + max-file: "3" {% if enable_mcpunifier|default(true) %} # Unified MCP endpoint. One MCP session that reaches every terminal, with # broker/account as tool parameters, instead of one endpoint per terminal. @@ -229,3 +271,8 @@ services: # - NET_RAW # depends_on: # - nginx + +volumes: + # Persistent per-container watchdog state (last restart, attempts, + # healthy-since) so backoff survives the watchdog's own restarts. + vm-watchdog-state: diff --git a/docs/operations.md b/docs/operations.md index bc1c1a4..4419ad4 100644 --- a/docs/operations.md +++ b/docs/operations.md @@ -10,6 +10,7 @@ The commands, ports, tunnels, locks, queues, and logs you need after the thing b - [Cloudflare Tunnel](#cloudflare-tunnel-optional) - [Project structure](#project-structure) - [Concurrency and backpressure](#concurrency-and-backpressure) +- [Auto-recovery](#auto-recovery) - [Logs](#logs) ## Make Targets @@ -251,6 +252,215 @@ or, with the sidecar list discovered from the generated compose file: This is covered by the real Compose lifecycle regression in `tests/integration/test_wickworks_lifecycle.py`. +## Auto-recovery + +The Windows VM(s) run inside `dockurr/windows` containers with a Docker healthcheck +(`scripts/healthcheck.sh`) that probes every terminal port this VM owns. A crash +inside the guest — an unexpected shutdown (Event 6008), a wedged terminal, an +OOM — leaves the **container** up while the **API** is dead, so +`restart: unless-stopped` never fires and nothing recovers it on its own. + +### VM crash watchdog + +`vm-watchdog` is a Compose-managed sidecar for exactly that case. It is part of +the project (`docker compose up -d` brings it up with everything else) — no +host cron, no systemd unit, no machine-specific checkout path. It polls Docker +health through the mounted unix socket and uses Docker's own +`State.Health.FailingStreak` as the source of truth. + +Behavior: + +- Scopes itself to this Compose project (`com.docker.compose.project`, + discovered from its own container labels) and to the `dockurr/windows` VM + image, so the VM sweep only ever recovers the VM containers — never nginx, + the log rotator, or other containers. The netns sidecars get a second, + narrower sweep of their own (below). +- **Recovers by recreating the VM together with its netns sidecars**, by + running `scripts/recreate-vm.sh ` — the same helper documented above + and covered by `tests/integration/test_wickworks_lifecycle.py`. It does not + use `docker restart`: that keeps the container ID but gives the VM a fresh + netns on start, which strands the wickworks sidecar exactly as recreating the + VM alone does. +- Acts **only** after its Docker health has stayed `unhealthy` + for `WATCHDOG_MIN_FAILING_STREAK` consecutive healthcheck failures (default + `10`, i.e. ~5 minutes at the default 30s interval). A container that is + healthy or still starting is never touched, so running backtests on a working + VM are never interrupted — the healthcheck stays green the whole time a + terminal is serving. +- Keeps a tiny state record per VM on a named volume, keyed by compose + project + service (`/state/..json` — stable across the + recreate that recovery performs, unlike a container id): last restart, + attempt count, and when the VM + was last observed healthy. +- Enforces exponential backoff between recovery attempts + (`WATCHDOG_BACKOFF_ATTEMPTS`, default `300,900,3600` — 5m → 15m → 1h), so a + VM that crashes again immediately after recovery is not restarted into a + loop. +- Stops after `WATCHDOG_MAX_ATTEMPTS` consecutive failed recoveries (default + `3`) and logs loudly, instead of threshing forever. +- Resets the attempt budget only after the VM has stayed **continuously** + healthy for `WATCHDOG_RESET_SECONDS` (default `1800`), so a VM that recovered + then crashed later gets a fresh budget. Any non-healthy observation — a + `starting` container after a restart, or an `unhealthy` poll below the streak + threshold — restarts that clock; it does not carry over from an earlier + healthy run. +- Never selects itself, whatever `WATCHDOG_IMAGE_FILTER` is set to — it resolves + its own full container id at startup and excludes it by equality (falling + back to Docker's short-id hostname only if that inspect fails) — and matches + the image **repository exactly** (`dockurr/windows`, `dockurr/windows:5.14`, + `dockurr/windows@sha256:…` — not `dockurr/windows-something`). +- `WATCHDOG_DRY_RUN=1` (or `--dry-run`) prints what it would do without + touching any container. + +#### The stranded sidecar, which no VM ever reports + +A sidecar joins its VM with `network_mode: service:`. Docker resolves that +**once**, at the sidecar's own start, into an immutable +`NetworkMode=container:`, and it builds a **fresh namespace every +time the owner starts**. So restarting the owner strands the sidecar: the +container id is unchanged, so nothing about the binding looks wrong, but the +namespace it points at is gone. The VM comes back perfectly healthy and the VM +sweep has nothing to act on. + +That is not hypothetical here. On 2026-09-07 the `mt5` container exited cleanly +and `restart: unless-stopped` brought it back; its `wickworks` sidecar sat in +the dead namespace for **two days** — `FailingStreak` 13,700, only `lo` left, +every `/rates/ta` call 502-ing — with the VM green throughout. + +The failing streak is the point. The sidecar's own healthcheck saw the fault +the whole time. There was simply no supervisor for it. So a second sweep +follows the VM sweep and applies the **same rule** — Docker health, past the +same `WATCHDOG_MIN_FAILING_STREAK`, the same backoff and attempt cap — to the +netns sidecars: + +- It recreates **that sidecar alone** (`recreate-vm.sh ` → + `up -d --force-recreate --no-deps`). Nothing declares + `network_mode: service:`, so the helper's own discovery returns + nothing for it and exactly one container is touched. That is the documented + repair, it is 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**. An + unhealthy owner belongs to the VM sweep, which recreates owner and sidecars + together. An owner that is not running is left alone too, and that one is + load-bearing rather than a default: `/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 the sidecar + would end up stopped, invisible to both sweeps, and never retried. A + `docker compose stop mt5` for maintenance must not cost you the sidecar. +- `container:` and `container:` are legal to write by hand and + are not normalised anywhere, so the owner reference is matched by full id, by + a 12-character-or-longer prefix, and by container name. + +**This makes an orphan-aware sidecar healthcheck a requirement, not a nicety.** +A check that only probes loopback stays green inside a dead namespace, and +Docker health is this daemon's only source of truth, so nothing here will ever +fire for it. `scripts/wickworks-healthcheck.py` is the worked example: it +probes the owner's gateway services, which disappear the moment the namespace +does. + +Set `WATCHDOG_WATCH_SIDECARS=0` to restore the VM-only scope. + +Environment overrides: `WATCHDOG_INTERVAL_SECONDS`, `WATCHDOG_MIN_FAILING_STREAK`, +`WATCHDOG_IMAGE_FILTER`, `WATCHDOG_WATCH_SIDECARS`, `WATCHDOG_BACKOFF_ATTEMPTS`, +`WATCHDOG_MAX_ATTEMPTS`, `WATCHDOG_RESET_SECONDS`, `WATCHDOG_COMPOSE_PROJECT`, +`WATCHDOG_STATE_DIR`, `WATCHDOG_DOCKER_SOCKET`, `WATCHDOG_DRY_RUN`, +`WATCHDOG_PROJECT_DIR`, `WATCHDOG_RECREATE_SCRIPT`, `WATCHDOG_RECREATE_TIMEOUT`. + +`WATCHDOG_PROJECT_DIR` is the **host** path of this project, and the compose +service mounts the project through at that same absolute path. Compose resolves +the relative bind mounts in `docker-compose.yml` client-side, so a +container-local path would rewrite every mount to something that does not exist +on the host. If it is missing the watchdog reports it at startup and refuses to +act, rather than falling back to a restart that looks like recovery and is not. + +That path reaches compose as `MT5_PROJECT_DIR`, and compose interpolates it on +**every** command against `docker-compose.yml`, not only the first `up`: + +- `run.sh` exports it for its own run **and writes it to `.env`**, so `make + down`, `make logs` and a manual `docker compose …` keep working after `run.sh` + has exited. Starting the stack some other way? Put + `MT5_PROJECT_DIR=` in `.env` yourself. +- The watchdog sets it explicitly in the environment of the `recreate-vm.sh` it + runs (from its own `WATCHDOG_PROJECT_DIR`), because the container is not + handed the host's shell variables. `tests/test_vm_watchdog.py` runs the real + helper under exactly that environment, and + `tests/integration/test_vm_watchdog_lifecycle.py` drives a real recovery + through the built sidecar on a disposable Compose project. + +### Busy is not dead — and hung is not busy + +`healthcheck.sh` reports a port **healthy** when the TCP handshake completes but +no HTTP answer arrives inside the probe window: something is listening, the +guest is just saturated (a compile, a Strategy Tester run). Restarting a VM for +being busy would turn a slow batch into an outage. + +That tolerance is **bounded**. A port that accepts TCP but stays silent for +`HEALTHCHECK_SLOW_GRACE` consecutive checks (default `10`, ≈5 minutes at the 30s +interval) is reported as `hung` and the check fails — from there the watchdog's +own streak gate (`WATCHDOG_MIN_FAILING_STREAK`, another ≈5 minutes) applies, so +a wedged API is recovered in roughly ten minutes rather than never. The +per-port counters live in `HEALTHCHECK_STATE_DIR` (default `/tmp/healthcheck-slow` +inside the VM container); an HTTP answer or a refused connection resets a port's +count, and a recreate starts every count from zero. A refused connection +(nothing listening) is `DOWN` immediately, as before. If the counters cannot be +written (a full disk), the bound is off for that check and the verdict says so: +`ok (slow but listening: …) [slow-state unwritable: hung detection off]`. + +**Blast radius.** One hung terminal API is enough to mark the whole VM `DOWN`, +and the watchdog's recovery is the whole VM — every other terminal on it, and +whatever they were running, goes with it. That is the same rule the check has +always applied to a dead port; it is just now applied to a hung one after the +grace. When you catch a single wedged terminal before the watchdog does, +`POST /terminal/restart` on that terminal (see `docs/rest-api.md`) is the +cheaper first response. + +This complements the in-VM `MT5AutoReboot` scheduled task, which reboots on a +fixed timer and can interrupt long-running backtests; operators who disable that +task still get crash recovery from the watchdog. + +### The probe budget: why the ports are probed concurrently + +The check's wall clock used to be the **product** of the probe timeout and the +terminal count. `PROBE_TIMEOUT_SECONDS` is 3, so a 24-terminal VM whose ports +are all silent took 72s against the compose `timeout: 30s`. Docker killed the +check before it reached a verdict and recorded `Health check exceeded timeout +(30s)` instead of naming the ports that were down. + +That failure is worse than useless, because a supervisor cannot tell it apart +from a dead VM. It is also most likely exactly when it hurts most: every port +is slow while the VM is still booting its terminals, so a VM that was merely +starting looked identical to one that had crashed. + +The probes therefore run **concurrently**, one background job per port. The +worst case is now one port's **host walk**, not one probe: each job still tries +the leased VM IP and then the two fallback hosts in turn, so the bound is +`PROBE_TIMEOUT_SECONDS × 3` = 9s, independent of terminal count. Keep three +times `PROBE_TIMEOUT_SECONDS` comfortably under the compose `timeout:` — at 10s +it would be 30s and the original bug is back. + +**The verdicts travel in exit statuses, not in files, and that is the whole +point.** The obvious implementation gives each job a verdict file under a +`mktemp -d`. It is fine until the disk is full: the write fails, the parent +finds no verdict, and its fail-closed rule reports *every* terminal on the VM +as down. Ten of those in a row and the watchdog recreates a perfectly healthy +VM, destroying two dozen running backtests, because `/tmp` filled up. The +sequential loop this replaced needed no disk, and neither does this: each job +returns `0` up, `1` busy, `2` hung, `3` dead, `4` busy-but-the-counter-could- +not-be-written, and the parent reads them back with `wait` in port order. +Anything else — a job killed by a signal — is unknown and fails closed as down. + +The only thing here that touches the disk is the slow/hung counter, and it is +deliberately off the path that decides up or down: if the counters cannot be +written the bound is off for that check and the verdict says +`[slow-state unwritable: hung detection off]`, while every port's liveness is +still whatever its probe actually found. + +A port configured twice is probed once. Two terminals on one port is a +misconfiguration rather than a topology, and acting on it twice meant two jobs +writing one counter — and, before the fan-out, a hung bound that fired at half +the configured grace. `config_helper.py` is where a duplicated port should be +reported; a healthcheck's job is liveness. ## Logs diff --git a/requirements-watchdog.txt b/requirements-watchdog.txt new file mode 100644 index 0000000..106de9f --- /dev/null +++ b/requirements-watchdog.txt @@ -0,0 +1,16 @@ +# Dependencies baked into the vm-watchdog image (Dockerfile.watchdog). +# +# Pinned by version AND hash: this image mounts the root-equivalent Docker +# socket, so "whatever PyPI serves today" is not an acceptable input to it - +# same argument as the base-image digest pin above it. --require-hashes makes +# pip refuse anything not listed here, including a compromised or yanked +# re-upload under the same version number. +# +# The three hashes are the two musllinux cp312 wheels (python:3.12-alpine on +# x86_64 / aarch64) plus the sdist as a fallback for any other platform. +# To update: pick the new version on https://pypi.org/pypi/PyYAML/json and +# copy the sha256 digests for the matching artifacts. +pyyaml==6.0.2 \ + --hash=sha256:8b9c7197f7cb2738065c481a0461e50ad02f18c78cd75775628afb4d7137fb3b \ + --hash=sha256:0833f8694549e586547b576dcfaba4a6b55b9e96098b36cdc7ebefe667dfed48 \ + --hash=sha256:d584d9ec91ad65861cc08d42e834324ef890a082e591037abe114850ff7bbc3e diff --git a/run.sh b/run.sh index dbe4b46..1c24597 100755 --- a/run.sh +++ b/run.sh @@ -2,6 +2,27 @@ set -eo pipefail DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" + +# The vm-watchdog container recovers a crashed VM by running +# scripts/recreate-vm.sh, which shells out to `docker compose`. Compose +# resolves this file's relative bind mounts client-side, so the watchdog needs +# the project at the SAME absolute path the host uses. Exported (not just set) +# because compose interpolates it into docker-compose.yml at up time. +export MT5_PROJECT_DIR="${MT5_PROJECT_DIR:-${DIR}}" +# It must be THIS checkout. A stale export from another clone would be +# persisted to .env below and then acted on by the watchdog, which would +# recreate this project's VMs from the other clone's compose file. +if [ "$(readlink -f "${MT5_PROJECT_DIR}")" != "$(readlink -f "${DIR}")" ]; then + echo "ERROR: MT5_PROJECT_DIR is '${MT5_PROJECT_DIR}' but this checkout is '${DIR}'." + echo " Unset it (run.sh derives it) or point it at this directory." + exit 1 +fi +case "${MT5_PROJECT_DIR}" in +*"'"*) + echo "ERROR: the project path contains a single quote, which .env cannot carry: ${MT5_PROJECT_DIR}" + exit 1 + ;; +esac DEBLOAT=0 for arg in "$@"; do if [ "$arg" = "--debloat" ]; then @@ -134,6 +155,15 @@ done # Generate fresh .env each run. : >"${DIR}/.env" +# Compose interpolates ${MT5_PROJECT_DIR:?} in docker-compose.yml on EVERY +# compose command, not only the `up` below - so the value has to outlive this +# shell. The export above covers this script; this line covers `make down`, +# `make logs`, a manual `docker compose`, and the vm-watchdog's own recreate, +# all of which run after it has exited. Written first, so a later failure in +# this script cannot leave .env without it. Single-quoted: unquoted, a path +# with `$` or ` #` in it is mangled by compose's dotenv parser. +echo "MT5_PROJECT_DIR='${MT5_PROJECT_DIR}'" >>"${DIR}/.env" + API_TOKEN=$(python3 "$CFG" api_token) if [ -n "${API_TOKEN}" ]; then echo "API_TOKEN=${API_TOKEN}" >>"${DIR}/.env" diff --git a/scripts/healthcheck.sh b/scripts/healthcheck.sh index a2c48ca..26fcbbe 100755 --- a/scripts/healthcheck.sh +++ b/scripts/healthcheck.sh @@ -7,8 +7,9 @@ # for traffic originating inside the container, so localhost:PORT won't work # from here. Falls back to localhost / 127.0.0.1 if leases aren't readable yet. # -# POSIX sh (not bash) because the dockurr/windows container is alpine-based -# and has no guaranteed bash. Hence `[ ]` over `[[ ]]`. +# POSIX sh (not bash): the dockurr/windows container's /bin/sh is dash, and +# the behavioural tests also run it under busybox ash. Hence `[ ]` over `[[ ]]` +# and no arrays anywhere. # # Deliberately NO `set -e` / `set -o pipefail`: this script's whole job is # running probes that are EXPECTED to fail so it can count them. `set -e` @@ -18,15 +19,40 @@ set -u -readonly CONFIG=/shared/config/config.yaml +# Overridable only so the behavioral tests can point the script at fixtures; +# the container never sets these and gets the paths it always had. +readonly CONFIG=${HEALTHCHECK_CONFIG:-/shared/config/config.yaml} # Per-VM terminal list, bind-mounted by docker-compose from # data/vm-group-.txt. Absent on single-VM installs, which means no filter. -readonly VM_GROUP=/shared/config/vm-group.txt -readonly DNSMASQ_LEASES=/var/lib/misc/dnsmasq.leases +readonly VM_GROUP=${HEALTHCHECK_VM_GROUP:-/shared/config/vm-group.txt} +readonly DNSMASQ_LEASES=${HEALTHCHECK_LEASES:-/var/lib/misc/dnsmasq.leases} readonly PROBE_PATH=/ping readonly PROBE_TIMEOUT_SECONDS=3 # Tried after the leased VM IP so a probe still works before the lease lands. readonly FALLBACK_HOSTS='127.0.0.1 localhost' +# A port that accepts TCP but never answers HTTP is tolerated as "busy" for +# this many CONSECUTIVE checks, then counted as down - see the busy branch. +# Anything that is not a positive integer falls back to the default rather +# than to "tolerate forever" or "tolerate nothing". Leading zeros are stripped +# first: `00` is not caught by a literal `0` pattern, and `010` would read as +# octal to `$(( ))` - neither may become a one-probe trigger. +slow_grace=${HEALTHCHECK_SLOW_GRACE:-10} +case "$slow_grace" in +'' | *[!0-9]*) slow_grace=10 ;; +*) + slow_grace=${slow_grace#"${slow_grace%%[!0]*}"} + [ -n "$slow_grace" ] && [ "$slow_grace" -ge 1 ] 2>/dev/null || slow_grace=10 + ;; +esac +readonly SLOW_GRACE_CHECKS=$slow_grace +# Per-port counters behind that tolerance. /tmp is container-local, so they +# reset when the VM is recreated - the right lifetime for "consecutive". +# +# This is the ONLY thing here that touches the disk, and it is deliberately +# not on the path that decides up/down: if these cannot be written the hung +# bound is off for that check and the verdict says so, but every port's +# liveness verdict is still whatever its probe actually found. +readonly SLOW_STATE_DIR=${HEALTHCHECK_STATE_DIR:-/tmp/healthcheck-slow} [ -f "$CONFIG" ] || { echo "no config.yaml at $CONFIG" @@ -54,9 +80,18 @@ PORTS=$(awk -v groupfile="$VM_GROUP" ' } function emit() { if (port == "") return - if (!have_group) { print port; return } - if ((broker SUBSEP account SUBSEP (instance == "" ? "default" : instance)) in allowed) + # Each port is emitted ONCE. Two terminals configured on one port is a + # misconfiguration, but it used to be acted on twice: the probes are + # now one background job per port, so a duplicate put TWO jobs on the + # same verdict file and the same slow counter - and even before the + # fan-out, the sequential loop incremented that counter twice per + # check, tripping the hung bound at half the configured grace. + if (port in emitted) return + if (!have_group) { emitted[port] = 1; print port; return } + if ((broker SUBSEP account SUBSEP (instance == "" ? "default" : instance)) in allowed) { + emitted[port] = 1 print port + } } function reset() { broker = ""; account = ""; instance = ""; port = "" } BEGIN { @@ -95,9 +130,48 @@ HOSTS="" [ -n "$VM_IP" ] && HOSTS="$VM_IP" HOSTS="$HOSTS $FALLBACK_HOSTS" -dead="" -for p in $PORTS; do - found=0 +# Probes run CONCURRENTLY, one background job per port. +# +# They used to run one after another, and the worst case was the product: +# PROBE_TIMEOUT_SECONDS x number of ports. The bulk VM carries 24 terminals, so +# 24 x 3s = 72s against a Docker `timeout: 30s` - the check was killed before it +# could reach a verdict and Docker recorded the useless "Health check exceeded +# timeout (30s)" instead of naming the ports that were down. Worse, that is +# indistinguishable from a dead VM, so the watchdog restarted VMs that were +# merely still starting their terminals. +# +# Fanned out, the worst case is ONE PORT'S host walk - PROBE_TIMEOUT_SECONDS x +# the number of fallback hosts (3s x 3 = 9s today) - regardless of how many +# terminals the VM carries. It is NOT one probe: each job still tries the +# leased VM IP and then the two fallbacks in turn. Raising +# PROBE_TIMEOUT_SECONDS multiplies by three, so keep 3 x PROBE_TIMEOUT_SECONDS +# comfortably under the compose `timeout:`. +# +# Each job reports through its EXIT STATUS, not through a file. +# +# The obvious implementation - a verdict file per port under a `mktemp -d` - +# is wrong in a way that only shows up on a bad day: when the disk is full, +# `echo up >"$workdir/$p"` fails, the parent finds no verdict, and its +# fail-closed rule turns "I could not write my own scratch file" into "every +# terminal on this VM is down". Ten of those and the watchdog destroys two +# dozen running backtests because /tmp filled up. The sequential loop this +# replaced had no such dependency, and neither may this. An exit status needs +# no disk, no cleanup, and no signal handler. +# +# 0 up 1 busy 2 hung 3 dead 4 busy, counter unwritable +# +# Anything else - a job killed by a signal, a shell that died - is unknown, +# and unknown fails closed as down, matching the status whitelist below. +readonly PROBE_UP=0 +readonly PROBE_BUSY=1 +readonly PROBE_HUNG=2 +readonly PROBE_DEAD=3 +readonly PROBE_BUSY_NOSTATE=4 + +mkdir -p "$SLOW_STATE_DIR" 2>/dev/null + +probe_port() { + p=$1 for host in $HOSTS; do # NO `|| echo 000` here. curl's -w ALREADY prints 000 on a failed # connection AND exits non-zero, so that fallback appended a SECOND @@ -105,27 +179,120 @@ for p in $PORTS; do # dead port as UP. This check could therefore never fail: a total # outage sat behind a green healthcheck for hours because Docker was # told everything was fine. - code=$(curl -s --max-time "$PROBE_TIMEOUT_SECONDS" -o /dev/null \ - -w '%{http_code}' "http://$host:$p$PROBE_PATH" 2>/dev/null) + # + # time_connect comes back alongside the status because the two ways a + # probe fails need opposite verdicts - see the busy branch below. + probe=$(curl -s --max-time "$PROBE_TIMEOUT_SECONDS" -o /dev/null \ + -w '%{http_code} %{time_connect}' "http://$host:$p$PROBE_PATH" 2>/dev/null) + code=${probe%% *} + connect=${probe##* } # Whitelist the valid shape instead of blacklisting one bad string, so # any future malformed value fails CLOSED rather than open. Empty means - # curl is missing or crashed. Any real HTTP status — including 4xx/5xx, - # e.g. a 401 from the auth layer — proves the process is listening. + # curl is missing or crashed. Any real HTTP status - including 4xx/5xx, + # e.g. a 401 from the auth layer - proves the process is listening. case "$code" in [1-5][0-9][0-9]) - found=1 - break + # An answer ends any slow streak this port had. + rm -f "$SLOW_STATE_DIR/$p" + return "$PROBE_UP" + ;; + esac + # A BUSY VM IS NOT A DEAD VM. + # + # If the TCP handshake completed, something is listening on that port - + # the process is alive, it just did not answer within the probe window. + # That happens whenever the guest is CPU-saturated: a compile, a + # Strategy Tester run, or a backtest is enough. Reporting it DOWN makes + # a supervisor restart a VM that was merely working, which turns a slow + # batch into an outage and loses whatever was running. + # + # Nothing listening refuses the connection instead, leaving + # time_connect at 0.000 - that is the case this healthcheck exists to + # catch, and it still fails. + # + # BUT A HUNG API IS NOT A BUSY ONE EITHER. A process that accepts + # connections and never serves one - wedged, deadlocked, stuck on a + # dead terminal - looks exactly like "busy" on any single probe, and + # tolerating that unconditionally kept such a VM healthy forever: the + # watchdog never saw an unhealthy streak, so it never recovered it. + # The tolerance is therefore bounded: a port that is still silent after + # SLOW_GRACE_CHECKS consecutive checks is reported as hung, and DOWN. + # A real busy spell ends and the port answers, which resets its count. + case "$connect" in + 0.000000 | 0.000 | 0 | "") ;; + *) + # This port's counter file is this port's alone, so the concurrent + # jobs share no state here either. + slow_n=$(cat "$SLOW_STATE_DIR/$p" 2>/dev/null) + case "$slow_n" in + '' | *[!0-9]*) slow_n=0 ;; + esac + slow_n=$((slow_n + 1)) + echo "$slow_n" >"$SLOW_STATE_DIR/$p" 2>/dev/null || + return "$PROBE_BUSY_NOSTATE" + if [ "$slow_n" -ge "$SLOW_GRACE_CHECKS" ]; then + return "$PROBE_HUNG" + fi + return "$PROBE_BUSY" ;; esac done - [ "$found" -eq 0 ] && dead="$dead $p" + # Refused everywhere: nothing is listening. That also ends any slow + # streak - whatever was hung on this port is gone now. + rm -f "$SLOW_STATE_DIR/$p" + return "$PROBE_DEAD" +} + +# The pid list is built in the SAME ORDER as $PORTS, and read back with +# `set --` in that order, which is how a POSIX shell carries two parallel +# lists without arrays. +pids="" +for p in $PORTS; do + probe_port "$p" & + pids="$pids $!" done -if [ -n "$dead" ]; then - echo "DOWN ports:$dead (vm_ip=$VM_IP)" +dead="" +busy="" +hung="" +# Set when a counter could not be written: the bound is then off for that port, +# and the verdict says so rather than reading exactly like a healthy VM. +slow_note="" +set -- $pids +for p in $PORTS; do + pid=$1 + shift + wait "$pid" + case "$?" in + "$PROBE_UP") ;; + "$PROBE_BUSY") busy="$busy $p" ;; + "$PROBE_HUNG") hung="$hung $p" ;; + "$PROBE_BUSY_NOSTATE") + busy="$busy $p" + slow_note=" [slow-state unwritable: hung detection off]" + ;; + # PROBE_DEAD, and anything unaccounted for: a job killed by a signal, a + # shell that died before returning. An unexplained probe is a down port, + # never a silent pass. + *) dead="$dead $p" ;; + esac +done + +if [ -n "$dead" ] || [ -n "$hung" ]; then + verdict="DOWN" + [ -n "$dead" ] && verdict="$verdict ports:$dead" + [ -n "$hung" ] && verdict="$verdict hung (listening, no HTTP for $SLOW_GRACE_CHECKS+ checks):$hung" + echo "$verdict (vm_ip=$VM_IP)" exit 1 fi +# Healthy, but say so out loud: a port that only ever answers this way is worth +# looking at even though it is not a restart-worthy fault. +if [ -n "$busy" ]; then + echo "ok (slow but listening:$busy)$slow_note all ports up:" $PORTS "(vm_ip=$VM_IP)" + exit 0 +fi + # Unquoted on purpose: collapses the newline-separated list onto one line, so # the verdict `docker inspect` shows stays readable. echo "ok all ports up:" $PORTS "(vm_ip=$VM_IP)" diff --git a/scripts/vm-watchdog.py b/scripts/vm-watchdog.py new file mode 100755 index 0000000..8c581ff --- /dev/null +++ b/scripts/vm-watchdog.py @@ -0,0 +1,828 @@ +#!/usr/bin/env python3 +"""Compose-managed VM crash watchdog. + +dockurr/windows keeps the container "up" while the Windows guest may have +crashed internally (e.g. Event ID 6008 "The previous system shutdown ... was +unexpected"). Docker's healthcheck then reports ``unhealthy``, but because the +container never exits, ``restart: unless-stopped`` never fires and every +terminal API inside that VM stays dead until a human restarts it. This sidecar +is that human, running as a normal Compose service instead of a host cron job. + +It polls Docker health through the mounted unix socket and restarts a VM +container ONLY after its Docker health has stayed ``unhealthy`` for a sustained +``FailingStreak`` — the same source of truth ``docker inspect`` exposes. A +healthy or merely-starting VM is never touched, so long-running backtests on a +working VM are never interrupted; the healthcheck goes green the whole time a +terminal is serving, and only a genuinely dead VM stays red. + +Restart policy is stateful, not a fixed cooldown: + +- A tiny JSON state record per VM lives on a named volume, keyed by STABLE + COMPOSE IDENTITY (``/state/..json``): last restart time, + attempt count, and when the VM was last observed healthy. Never keyed by + container id: recovery here is a recreate, which REPLACES the container, so + an id-keyed record is orphaned by the very action that wrote it and the next + poll would start the replacement at ``attempts = 0`` - every recovery + silently refunding the attempt budget and resetting backoff. +- After the sustained-unhealthy threshold, the VM is restarted. Attempts gate + an exponential backoff (default 5m -> 15m -> 1h): a VM that crashes again + immediately after recovery is not restarted into a loop. +- After ``WATCHDOG_MAX_ATTEMPTS`` consecutive failed recoveries the watchdog + stops trying and logs loudly, so a genuinely broken VM does not thresh forever. +- Attempts reset only after the VM has stayed healthy for + ``WATCHDOG_RESET_SECONDS``, so a VM that recovered then crashed again later + gets a fresh budget instead of inheriting old failures. + +Scope is strict: for the VM sweep, only containers in this Compose project +running the ``dockurr/windows`` image are considered, so nginx, the log rotator +and the watchdog itself are never touched. + +A SECOND, narrower sweep covers the netns sidecars, because nothing watched +them and the way they fail leaves the VM sweep with nothing to act on. Docker +resolves ``network_mode: service:`` ONCE, at the sidecar's own start, into +an immutable ``NetworkMode=container:``, and it builds a fresh +namespace whenever the owner starts. So restarting the owner strands the +sidecar - the id is unchanged, the namespace is not - and the VM comes back +perfectly healthy. It happened here: mt5 exited cleanly on 2026-09-07 and +``unless-stopped`` restarted it, and its wickworks sidecar spent two days in a +dead namespace, 502-ing every ``/rates/ta`` call, with a 13,700-long failing +streak nothing was looking at. + +The streak is the point. The sidecar's OWN healthcheck saw the fault the whole +time; there was simply no supervisor for it. So this sweep applies the same +rule as the VM sweep - Docker health, past the same streak - and recreates the +sidecar ALONE, which is the documented repair and what an operator does by +hand. It acts only while the owner is a RUNNING, HEALTHY VM of this project: +an unhealthy owner belongs to the VM sweep, which recreates owner and sidecars +together, and an owner that is not running cannot be joined at all, so +recreating a sidecar under it would only leave it stopped. + +That makes an orphan-aware sidecar healthcheck a requirement, not a nicety - +see ``scripts/wickworks-healthcheck.py``. Disable the sweep entirely with +``WATCHDOG_WATCH_SIDECARS=0``. + +Recovery is a COORDINATED RECREATE, not a restart. A sidecar sharing the VM's +network namespace (``network_mode: service:``, i.e. wickworks) resolves +that binding once, at its own start, into an immutable +``NetworkMode=container:``. Restarting the owner keeps its id, but +Docker tears the netns down on stop and builds a fresh one on start, so the +sidecar is left holding a dead namespace. This module previously claimed the +opposite; ``tests/integration/test_wickworks_lifecycle.py`` disproves it, +asserting that BOTH "recreate owner alone" and "restart owner alone" strand +the sidecar, and that only recreating the owner together with its sidecars +repairs the binding. + +So the watchdog does not restart through the Docker API. It shells out to +``scripts/recreate-vm.sh`` — the same helper an operator runs by hand, and the +one that lifecycle test covers — which discovers each VM's sidecars from the +generated Compose file and recreates them together. + +That is why this container needs the compose project mounted at the SAME +absolute path the host uses: Compose resolves relative bind mounts +client-side, so ``./scripts/x`` in the compose file has to land on the host's +``/scripts/x``, not on some path inside this container. ``run.sh`` +exports ``MT5_PROJECT_DIR`` for that and persists it to ``.env``. Without it +the watchdog refuses to act and says so, rather than falling back to a +restart that looks like recovery and is not. + +The helper runs ``docker compose`` itself, and compose interpolates +``${MT5_PROJECT_DIR:?}`` in the compose file on EVERY command — so the +watchdog hands that variable to the helper explicitly (``recreate_env``). +Compose resolved it on the host when it created this container but never +passed it in, and without that line every real recovery failed at +interpolation before it could stop anything. + +Runs forever, restarting itself if it crashes: keepalive is the Compose +``restart: unless-stopped`` on this service, not a supervisor inside the loop. +""" + +from __future__ import annotations + +import copy +import http.client +import json +import os +import re +import socket +import subprocess +import sys +import time +from pathlib import Path + +# ── Configuration (env-overridable, defaults match the sidecar in compose) ── + +# Misconfiguration is collected rather than raised, so importing this module +# never explodes and validate_config() can report EVERY problem at once. An +# operator fixing one variable at a time from successive tracebacks is a worse +# afternoon than one message listing all of them. +CONFIG_ERRORS: list[str] = [] + + +def _env_int(name: str, default: str, minimum: int) -> int: + """Integer from the environment, or the default plus a recorded error. + + Falling back to the default keeps the module importable; validate_config() + is what refuses to run. Blank is rejected explicitly: an unset variable and + one set to "" mean different things to the person who wrote the compose + file, and only the second is a mistake worth naming. + """ + raw = os.environ.get(name) + if raw is None: + raw = default + text = raw.strip() + if not text: + CONFIG_ERRORS.append(f"{name} is set but empty; expected an integer >= {minimum}") + return int(default) + try: + value = int(text) + except ValueError: + CONFIG_ERRORS.append(f"{name}={raw!r} is not an integer") + return int(default) + if value < minimum: + CONFIG_ERRORS.append(f"{name}={value} is below the minimum of {minimum}") + return int(default) + return value + + +def _env_int_list(name: str, default: str, minimum: int) -> list[int]: + """Comma-separated positive integers, never empty. + + The empty case is the one that mattered: WATCHDOG_BACKOFF_ATTEMPTS= parsed + to [], which survived startup and then raised IndexError inside decide() + on the first unhealthy pass after a recorded restart - a crash loop in the + thing that exists to recover from crashes. + """ + raw = os.environ.get(name) + if raw is None: + raw = default + parts = [p.strip() for p in raw.split(",") if p.strip()] + if not parts: + CONFIG_ERRORS.append( + f"{name}={raw!r} parsed to an empty list; expected at least one integer >= {minimum}" + ) + return [int(p) for p in default.split(",")] + values: list[int] = [] + for part in parts: + try: + value = int(part) + except ValueError: + CONFIG_ERRORS.append(f"{name} entry {part!r} is not an integer") + continue + if value < minimum: + CONFIG_ERRORS.append(f"{name} entry {value} is below the minimum of {minimum}") + continue + values.append(value) + if not values: + return [int(p) for p in default.split(",")] + return values + + +def _env_bool(name: str, default: str) -> bool: + """A yes/no switch, tolerant of how people actually write them. + + Anything unrecognised is a recorded error and falls back to the default + rather than being read as false: silently disabling a recovery path + because someone wrote ``WATCHDOG_WATCH_SIDECARS=no thanks`` is the failure + mode this whole file is written against. + """ + raw = os.environ.get(name) + if raw is None: + raw = default + text = raw.strip().lower() + if text in {"1", "true", "yes", "on"}: + return True + if text in {"0", "false", "no", "off"}: + return False + CONFIG_ERRORS.append(f"{name}={raw!r} is not a boolean (1/0, true/false, yes/no, on/off)") + return default.strip().lower() in {"1", "true", "yes", "on"} + + +def _env_str(name: str, default: str, *, allow_empty: bool) -> str: + raw = os.environ.get(name) + if raw is None: + return default + if not raw.strip() and not allow_empty: + CONFIG_ERRORS.append(f"{name} is set but empty; expected a value") + return default + return raw + + +DOCKER_SOCKET = _env_str("WATCHDOG_DOCKER_SOCKET", "/var/run/docker.sock", allow_empty=False) + +# The coordinated recreate helper, and the compose project it must act on. +# PROJECT_DIR is the HOST path: compose resolves the relative bind mounts in +# docker-compose.yml against it, so a container-local path would rewrite every +# mount to somewhere that does not exist on the host. Empty means "not wired +# up" and is reported by validate_config() rather than guessed at. +RECREATE_SCRIPT = _env_str( + "WATCHDOG_RECREATE_SCRIPT", "/project/scripts/recreate-vm.sh", allow_empty=False +) +PROJECT_DIR = _env_str("WATCHDOG_PROJECT_DIR", "", allow_empty=True) +RECREATE_TIMEOUT_SECONDS = _env_int("WATCHDOG_RECREATE_TIMEOUT", "300", minimum=30) +STATE_DIR = _env_str("WATCHDOG_STATE_DIR", "/state", allow_empty=False) +INTERVAL_SECONDS = _env_int("WATCHDOG_INTERVAL_SECONDS", "30", minimum=1) + +# Restart only after this many consecutive failed healthchecks. +MIN_FAILING_STREAK = _env_int("WATCHDOG_MIN_FAILING_STREAK", "10", minimum=1) + +# Only containers running exactly this image REPOSITORY (any tag or digest) +# are considered VM containers - see _image_matches. Empty is refused rather +# than treated as "no filter": a blank value would make every container in the +# project a restart candidate - including any database sharing it. +IMAGE_FILTER = _env_str("WATCHDOG_IMAGE_FILTER", "dockurr/windows", allow_empty=False) + +# Also watch the sidecars that share a VM's network namespace +# (``network_mode: service:``). Restarting the owner strands them - same +# container id, fresh namespace - while the VM itself comes back healthy, so +# the VM sweep has nothing to act on. Their own healthchecks see it; nothing +# was watching those. On this farm that cost two days of dead TA calls on +# 2026-09-07. Set to 0 to restore the VM-only scope. +WATCH_SIDECARS = _env_bool("WATCHDOG_WATCH_SIDECARS", "1") + +# Exponential backoff per attempt: first restart after 5m, then 15m, then 1h. +BACKOFF_ATTEMPTS = _env_int_list("WATCHDOG_BACKOFF_ATTEMPTS", "300,900,3600", minimum=1) + +# Give up (and log loudly) after this many consecutive failed recoveries. +MAX_ATTEMPTS = _env_int("WATCHDOG_MAX_ATTEMPTS", "3", minimum=1) + +# A VM must stay healthy this long before its attempt budget resets. Zero is +# refused: the budget would reset on the first healthy poll after a restart, +# which silently makes MAX_ATTEMPTS unreachable and the give-up path dead code. +RESET_SECONDS = _env_int("WATCHDOG_RESET_SECONDS", "1800", minimum=1) + +# Compose project to scope to. Empty = auto-discover from this container's own +# labels (the watchdog runs as a service in the same project). +COMPOSE_PROJECT = os.environ.get("WATCHDOG_COMPOSE_PROJECT", "") + +# This container's own ID; Docker sets the container hostname to its SHORT id. +SELF_ID = os.environ.get("WATCHDOG_SELF_ID", socket.gethostname()) +# Resolved to the FULL id at startup (main -> _inspect_self) so self-exclusion +# works by equality even when the hostname is not the container id - a +# `hostname:` on the service, or WATCHDOG_SELF_ID set to a name. +SELF_FULL_ID = "" +_HEX_ID = re.compile(r"[0-9a-f]{12,64}") + + +def log(message: str, *args) -> None: + stamp = time.strftime("%Y-%m-%dT%H:%M:%SZ", time.gmtime()) + print(f"[{stamp}] [vm-watchdog] {message}" if not args else f"[{stamp}] [vm-watchdog] {message % args}") + + +# ── Minimal Docker Engine API client over the unix socket (stdlib only) ────── + + +class _UnixSocketConnection(http.client.HTTPConnection): + """HTTPConnection that talks to the Docker Engine over a unix socket.""" + + def __init__(self, socket_path: str): + super().__init__("localhost") + self._socket_path = socket_path + + def connect(self) -> None: + sock = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) + sock.connect(self._socket_path) + self.sock = sock + + +class DockerClient: + """A tiny read-only Docker API client. + + Only the endpoints the watchdog needs are implemented: list containers and + inspect one. Recovery deliberately does NOT go through this client — see + recreate_vm() and the module docstring. Anything else the daemon replies + with is surfaced as a RuntimeError carrying the status + body. + """ + + def __init__(self, socket_path: str = DOCKER_SOCKET): + self._socket_path = socket_path + + def _request(self, method: str, path: str, body: str | None = None) -> bytes: + conn = _UnixSocketConnection(self._socket_path) + try: + conn.request(method, path, body=body) + resp = conn.getresponse() + data = resp.read() + status = resp.status + finally: + conn.close() + if not (200 <= status < 300): + raise RuntimeError( + f"docker {method} {path}: HTTP {status}: {data[:300]!r}" + ) + return data + + def list_containers(self) -> list[dict]: + """Running containers (Docker's default ``/containers/json``).""" + data = self._request("GET", "/containers/json") + return json.loads(data) if data else [] + + def inspect(self, container_id: str) -> dict: + data = self._request("GET", f"/containers/{container_id}/json") + return json.loads(data) + + + +def recreate_env(project: str) -> dict[str, str]: + """The exact environment the recreate helper runs with. + + Two values are set explicitly, because the helper runs ``docker compose`` + and compose needs both while this container was handed neither: + + - ``COMPOSE_PROJECT_NAME``: compose otherwise derives the project from the + directory name, and a mismatch does not fail - it quietly creates a + SECOND set of containers alongside the running ones. + - ``MT5_PROJECT_DIR``: the compose file interpolates ``${MT5_PROJECT_DIR:?}`` + on EVERY compose command, not only the ``up`` that started the stack. + Compose resolved it on the host when it created this container, but the + container's own environment carries ``WATCHDOG_PROJECT_DIR`` instead, so + the helper's compose call failed at interpolation - "required variable + MT5_PROJECT_DIR is missing a value" - before it could stop or recreate + anything. Every real (non-dry-run) recovery was dead on arrival. + ``WATCHDOG_PROJECT_DIR`` IS that host path, so it is the source here. + + Factored out of recreate_vm() so a test can run the real helper under + precisely this environment, with nothing inherited from the host shell. + """ + env = dict(os.environ) + env["COMPOSE_PROJECT_NAME"] = project + env["MT5_PROJECT_DIR"] = PROJECT_DIR + return env + + +def recreate_vm(service: str, project: str) -> None: + """Recreate one VM together with every sidecar sharing its netns. + + Delegates to scripts/recreate-vm.sh instead of reimplementing it: that + script is what operators run, what the lifecycle regression test exercises, + and the only place that knows how to find a VM's sidecars in the generated + compose file. Reimplementing the discovery here would give the fleet two + recovery paths that could drift apart. See recreate_env() for the two + variables the helper is handed explicitly. + """ + if not PROJECT_DIR: + raise RuntimeError( + "WATCHDOG_PROJECT_DIR is unset, so the compose project cannot be " + "recreated. Start the stack through run.sh (it exports " + "MT5_PROJECT_DIR and writes it to .env), or set it to the host " + "path of the project." + ) + result = subprocess.run( + [RECREATE_SCRIPT, service], + cwd=PROJECT_DIR, + env=recreate_env(project), + capture_output=True, + text=True, + timeout=RECREATE_TIMEOUT_SECONDS, + check=False, + ) + if result.returncode != 0: + detail = (result.stderr or result.stdout or "").strip()[-500:] + raise RuntimeError( + f"{RECREATE_SCRIPT} {service} exited {result.returncode}: {detail}" + ) + + +# ── Pure policy: state + health in, action out ─────────────────────────────── + + +def state_key(project: str, service: str) -> str: + """Stable identity for one VM's state record. + + Compose project + service survives a recreate; the container id does not. + Sanitized defensively so a hostile-looking label cannot become a path - + both values come from compose labels, but this file name is the only place + they touch the filesystem. + """ + raw = f"{project}.{service}" + return "".join(c if c.isalnum() or c in "._-" else "_" for c in raw) + + +def load_state(key: str, state_dir: str | None = None) -> dict: + """The per-VM state record, or a fresh one when absent/corrupt.""" + path = Path(state_dir if state_dir is not None else STATE_DIR) / f"{key}.json" + if not path.exists(): + return {"last_restart": 0, "attempts": 0, "healthy_since": 0} + try: + return json.loads(path.read_text(encoding="utf-8")) + except (OSError, ValueError): + return {"last_restart": 0, "attempts": 0, "healthy_since": 0} + + +def save_state(key: str, state: dict, state_dir: str | None = None) -> None: + path = Path(state_dir if state_dir is not None else STATE_DIR) / f"{key}.json" + path.parent.mkdir(parents=True, exist_ok=True) + tmp = path.with_suffix(".tmp") + tmp.write_text(json.dumps(state), encoding="utf-8") + tmp.replace(path) + + +def decide(state: dict, health_status: str, failing_streak: int, now: int) -> tuple[str, str]: + """One container's next action given its state and Docker health. + + Mutates ``state`` (attempts / timestamps) and returns ``(action, reason)`` + where action is one of ``wait``, ``restart`` or ``give_up``. + """ + # healthy_since means CONTINUOUSLY healthy since. Any other observation - + # starting, unhealthy, none - breaks that continuity and the clock starts + # over on the next healthy poll. It used to survive a `starting` (and an + # unhealthy poll below the streak threshold), so a VM that was healthy at + # t0, went through a restart, and came back healthy at t0+RESET had its + # attempt budget refunded on that first healthy poll rather than after + # RESET_SECONDS of proven stability. + if health_status != "healthy": + state["healthy_since"] = 0 + + if health_status != "unhealthy": + # Healthy or starting: never touch. Track when the current healthy run + # began so a recovery that holds long enough resets the attempt budget. + if health_status == "healthy": + if not state.get("healthy_since"): + state["healthy_since"] = now + if now - state["healthy_since"] >= RESET_SECONDS: + state["attempts"] = 0 + state["last_restart"] = 0 + return "wait", "healthy long enough - attempt budget reset" + return "wait", f"health={health_status}" + + # Unhealthy from here on. + if failing_streak < MIN_FAILING_STREAK: + return "wait", f"unhealthy streak {failing_streak} < {MIN_FAILING_STREAK}" + + attempts = state.get("attempts", 0) + if attempts >= MAX_ATTEMPTS: + return "give_up", f"attempts {attempts} >= MAX_ATTEMPTS {MAX_ATTEMPTS}" + + last_restart = state.get("last_restart", 0) + # Delay before the NEXT restart, indexed by how many restarts already + # happened: after the 1st restart wait BACKOFF[0], after the 2nd wait + # BACKOFF[1], etc. First restart has no prior wait. + delay = BACKOFF_ATTEMPTS[max(0, min(attempts - 1, len(BACKOFF_ATTEMPTS) - 1))] if attempts else 0 + if last_restart and (now - last_restart) < delay: + return "wait", f"backoff {now - last_restart}s < {delay}s (attempt {attempts + 1})" + + state["attempts"] = attempts + 1 + state["last_restart"] = now + state["healthy_since"] = 0 + return "restart", f"unhealthy streak {failing_streak} >= {MIN_FAILING_STREAK} (attempt {attempts + 1})" + + +# ── Docker-facing logic ────────────────────────────────────────────────────── + + +def _inspect_self(client: DockerClient) -> dict: + """This container's own inspect record, or {} when it cannot be read.""" + try: + return client.inspect(SELF_ID) + except (RuntimeError, OSError): + return {} + + +def _compose_project(client: DockerClient) -> str | None: + """This project's name, from our own container's compose labels.""" + info = _inspect_self(client) + return (info.get("Config", {}).get("Labels", {}) or {}).get("com.docker.compose.project") + + +def _image_matches(image: str, repo: str) -> bool: + """Exact image REPOSITORY match, any tag or digest. + + ``dockurr/windows`` matches ``dockurr/windows``, ``dockurr/windows:5.14`` + and ``dockurr/windows@sha256:...`` - and not ``dockurr/windows-not-the-vm``. + A prefix test did match that last one, which for a container holding the + Docker socket meant its restart scope was "anything whose image name + happens to start the same way". + """ + return image == repo or image.startswith(repo + ":") or image.startswith(repo + "@") + + +def _is_self(container_id: str) -> bool: + """Whether this id is the watchdog's own container. + + By equality against the full id resolved at startup. Before that (or if + the self-inspect failed) fall back to the short-id prefix Docker gives the + hostname - but only when SELF_ID actually looks like one, so a service + `hostname: watchdog` never prefix-matches unrelated containers, and a real + name never quietly matches nothing while the docs promise otherwise. + """ + if SELF_FULL_ID: + return container_id == SELF_FULL_ID + if SELF_ID and _HEX_ID.fullmatch(SELF_ID): + return container_id == SELF_ID or container_id.startswith(SELF_ID) + return False + + +def _scoped_containers(client: DockerClient, project: str) -> list[dict]: + """Running containers in this project running the VM image - never itself.""" + out = [] + for c in client.list_containers(): + # Unconditional and first: a valid operator override such as + # WATCHDOG_IMAGE_FILTER=python would otherwise select the watchdog, and + # a watchdog that recreates itself mid-sweep is not a recovery path. + if _is_self(c.get("Id") or ""): + continue + labels = c.get("Labels", {}) or {} + if project and labels.get("com.docker.compose.project") != project: + continue + if not _image_matches(c.get("Image") or "", IMAGE_FILTER): + continue + out.append(c) + return out + + +def _netns_sidecars(client: DockerClient, project: str) -> list[tuple[dict, str]]: + """(container, owner-id) for every project container bound to another's netns. + + ``network_mode: service:`` shows up on the running container as + ``HostConfig.NetworkMode = "container:"`` - the resolved id, not + the service name, because Docker resolves it once at start and never + revisits it. That resolved id is exactly what makes the binding breakable, + and it is what this returns alongside the container. + + Read from the container list rather than a per-container inspect: the list + already carries HostConfig, and one API call per sweep beats one per + container on a host with two dozen of them. + """ + out: list[tuple[dict, str]] = [] + for c in client.list_containers(): + if _is_self(c.get("Id") or ""): + continue + labels = c.get("Labels", {}) or {} + if project and labels.get("com.docker.compose.project") != project: + continue + mode = ((c.get("HostConfig") or {}) or {}).get("NetworkMode") or "" + if not mode.startswith("container:"): + continue + # An empty token after `container:` needs no guard here: it matches no + # VM in _owner_container, which is where every owner reference is + # resolved, so there is one place that decides and not two. + out.append((c, mode.split(":", 1)[1])) + return out + + +def _owner_container(token: str, vms: list[dict]) -> dict | None: + """The VM container a sidecar's ``container:`` names, if any. + + ``network_mode: service:`` always resolves to a full id (verified + against the daemon), but ``container:`` and ``container:`` + are both legal to write by hand and are NOT normalised anywhere. Matching + only full ids would leave such a sidecar permanently unrecognised, and this + file's rule is that an unrecognised container is left alone - so it would + silently go unwatched while the docs promised otherwise. + """ + for vm in vms: + cid = vm.get("Id") or "" + if not cid: + continue + if cid == token or (len(token) >= 12 and cid.startswith(token)): + return vm + if any(name.lstrip("/") == token for name in (vm.get("Names") or [])): + return vm + return None + + +def _health_of(client: DockerClient, container_id: str) -> tuple[str, int]: + """(State.Health.Status, State.Health.FailingStreak) or ('none', 0).""" + info = client.inspect(container_id) + health = info.get("State", {}).get("Health", {}) or {} + status = health.get("Status", "none") + streak = health.get("FailingStreak", 0) + return status, int(streak or 0) + + +def sweep_once(client: DockerClient, project: str, dry_run: bool = False, now: int | None = None) -> int: + """One pass over the project's VMs, then over their netns sidecars. + + Returns the recovery count across both. + """ + now = now if now is not None else int(time.time()) + restarted = 0 + # What the VM pass saw, for the sidecar pass that follows: a sidecar's fate + # depends on its owner's, and re-deriving it there would be a second, + # divergent opinion about the same containers in the same tick. + vm_health: dict[str, str] = {} + vms = _scoped_containers(client, project) + for c in vms: + cid = c["Id"] + name = (c.get("Names") or ["?"])[0] + try: + status, streak = _health_of(client, cid) + except (RuntimeError, OSError) as exc: + log(f"{name}: cannot inspect health ({exc}); skipping") + continue + # Resolve the compose service BEFORE touching state: it is both what a + # recreate names and the stable half of the state key. A recreate + # replaces the container, so state keyed by container id was orphaned + # by every successful recovery - the replacement arrived with a fresh + # id, loaded a fresh record, and the attempt cap and backoff never + # carried across the one boundary they exist to police. + vm_health[cid] = status + service = (c.get("Labels") or {}).get("com.docker.compose.service") + if not service: + log(f"{name}: no compose service label; cannot recreate, skipping") + continue + state = load_state(state_key(project, service)) + # decide() MUTATES the state it is handed - it increments attempts and + # stamps last_restart. Under --dry-run that must not reach disk: a dry + # pass would consume the real backoff and attempt budget without + # restarting anything, so enough dry passes leave the VM at GIVING UP + # the moment dry-run is switched off. Evaluate against a copy instead, + # and persist nothing. + working = copy.deepcopy(state) if dry_run else state + action, reason = decide(working, status, streak, now) + if not dry_run: + save_state(state_key(project, service), working) + if action == "restart": + # Recreate by COMPOSE SERVICE, not container id: the helper has to + # name the VM and its sidecars as compose services to recreate them + # together, and a container id means nothing to compose. + if dry_run: + log(f"DRY-RUN: would recreate {service} (+ its sidecars) - {reason}") + else: + try: + recreate_vm(service, project) + log(f"recreated {service} and its sidecars - {reason}") + except (RuntimeError, OSError, subprocess.SubprocessError) as exc: + log(f"{name}: recreate failed ({exc}); state kept for backoff") + restarted += 1 + elif action == "give_up": + log(f"GIVING UP on {name} after {working.get('attempts', 0)} attempts - {reason}") + + if WATCH_SIDECARS: + restarted += sweep_sidecars_once( + client, project, vms, vm_health, dry_run=dry_run, now=now + ) + return restarted + + +def sweep_sidecars_once( + client: DockerClient, + project: str, + vms: list[dict], + vm_health: dict[str, str], + *, + dry_run: bool = False, + now: int | None = None, +) -> int: + """One pass over the sidecars sharing a VM's netns. Returns recovery count. + + Same rule as the VM sweep - Docker health, past the streak - applied to a + container class the VM sweep is not allowed to touch. What changes is only + WHOSE health decides and WHAT gets recreated: + + - Owner is a running project VM and healthy: the sidecar's own health + decides, and a recovery recreates the SIDECAR ALONE. + - Owner is a running project VM and not healthy: left alone. Recreating + owner and sidecars together is the only thing that repairs the binding, + and that is the VM sweep's job; acting here would race it and rebind to + a namespace about to be replaced. + - Owner is not a running project VM: left alone, and this is load-bearing + rather than a default. A stopped owner is INDISTINGUISHABLE from a + destroyed one over ``/containers/json``, which lists running containers + only. Recreating a sidecar under a stopped owner cannot work - the + helper stops it first, then ``up --no-deps`` cannot join a namespace + that is not there - so the sidecar would be left STOPPED, invisible to + both sweeps, and never retried. ``docker compose stop mt5`` for + maintenance must not cost the sidecar. + + This means a netns sidecar has to have a healthcheck that can SEE the + orphaning. A check that only probes loopback stays green inside a dead + namespace and nothing here will ever fire. ``scripts/wickworks-healthcheck.py`` + is the worked example: it probes the owner's gateway services, which + disappear the moment the namespace does. + """ + now = now if now is not None else int(time.time()) + recovered = 0 + for c, owner_token in _netns_sidecars(client, project): + name = (c.get("Names") or ["?"])[0] + service = (c.get("Labels") or {}).get("com.docker.compose.service") + if not service: + log(f"{name}: no compose service label; cannot recreate, skipping") + continue + + owner = _owner_container(owner_token, vms) + if owner is None: + # Not one of this project's running VMs: somebody else's netns, a + # sidecar chained to a sidecar, or an owner that is stopped. None + # of those is this sweep's to act on. + continue + owner_id = owner.get("Id") or "" + # Only a healthy owner. An unhealthy one belongs to the VM sweep, + # which recreates it together with its sidecars - the only operation + # that repairs the binding - and acting here would race that. This + # also covers the owner the VM sweep recreated moments ago in this + # same pass: a recreate only ever follows an UNHEALTHY observation, so + # such an owner can never be `healthy` here. + if vm_health.get(owner_id) != "healthy": + continue + + try: + status, streak = _health_of(client, c["Id"]) + except (RuntimeError, OSError) as exc: + log(f"{name}: cannot inspect health ({exc}); skipping") + continue + + state = load_state(state_key(project, service)) + working = copy.deepcopy(state) if dry_run else state + action, reason = decide(working, status, streak, now) + if not dry_run: + save_state(state_key(project, service), working) + if action == "restart": + # The sidecar ALONE. recreate-vm.sh discovers the netns sidecars of + # the service it is given, and nothing declares + # `network_mode: service:`, so naming one here recreates + # exactly one container - the documented repair, which does not + # disturb the VM or the terminals running inside it. + if dry_run: + log(f"DRY-RUN: would recreate sidecar {service} - {reason}") + else: + try: + recreate_vm(service, project) + log(f"recreated sidecar {service} - {reason}") + except (RuntimeError, OSError, subprocess.SubprocessError) as exc: + log(f"{name}: sidecar recreate failed ({exc}); state kept for backoff") + recovered += 1 + elif action == "give_up": + log( + f"GIVING UP on sidecar {name} after {working.get('attempts', 0)} " + f"attempts - {reason}" + ) + return recovered + + +def validate_config() -> list[str]: + """Every configuration problem found at import, as human-readable lines. + + Separate from main() so a test can assert on the messages without running + the daemon, and so an operator can see all of them in one go. + + The recovery wiring is checked here rather than at the moment a VM dies: + finding out that the watchdog cannot act only once something has already + crashed is the worst possible time to learn it. + """ + errors = list(CONFIG_ERRORS) + if not PROJECT_DIR: + errors.append( + "WATCHDOG_PROJECT_DIR is empty - recovery needs the compose project's " + "HOST path (run.sh exports MT5_PROJECT_DIR and writes it to .env). " + "Without it a crashed VM cannot be recreated." + ) + elif not os.path.isdir(PROJECT_DIR): + errors.append( + f"WATCHDOG_PROJECT_DIR={PROJECT_DIR!r} is not a directory in this " + "container - mount the project through at the same absolute path the " + "host uses, or compose will rewrite every relative bind mount." + ) + if not os.path.exists(RECREATE_SCRIPT): + errors.append( + f"WATCHDOG_RECREATE_SCRIPT={RECREATE_SCRIPT!r} not found - the " + "coordinated recreate helper must be mounted into this container." + ) + return errors + + +def main() -> int: + problems = validate_config() + if problems: + # Refuse to start rather than run on defaults. This daemon restarts + # containers; quietly substituting values an operator did not choose is + # the wrong failure mode for something holding the Docker socket. + log(f"invalid configuration ({len(problems)} problem(s)); refusing to start") + for problem in problems: + log(f" {problem}") + return 2 + + dry_run = "--dry-run" in sys.argv + if dry_run: + log("dry-run mode: will report decisions without restarting") + client = DockerClient(DOCKER_SOCKET) + + # Resolve our own FULL container id once, so self-exclusion is an equality + # test rather than a guess from the hostname. + global SELF_FULL_ID + self_info = _inspect_self(client) + SELF_FULL_ID = self_info.get("Id") or "" + if SELF_FULL_ID: + log(f"own container id resolved: {SELF_FULL_ID[:12]}") + else: + log(f"cannot inspect own container '{SELF_ID}'; self-exclusion falls back to the short-id prefix") + + project = COMPOSE_PROJECT or _compose_project(client) or "" + if not project: + log("cannot determine compose project (no WATCHDOG_COMPOSE_PROJECT and " + "self-inspect found no compose labels); refusing to run un-scoped") + return 1 + log(f"scoped to compose project '{project}' (image filter '{IMAGE_FILTER}')") + + while True: + try: + sweep_once(client, project, dry_run=dry_run) + except (RuntimeError, OSError) as exc: + log(f"sweep failed: {exc}") + time.sleep(INTERVAL_SECONDS) + + +if __name__ == "__main__": + try: + sys.exit(main()) + except KeyboardInterrupt: + sys.exit(0) diff --git a/tests/integration/test_vm_watchdog_lifecycle.py b/tests/integration/test_vm_watchdog_lifecycle.py new file mode 100644 index 0000000..50c287b --- /dev/null +++ b/tests/integration/test_vm_watchdog_lifecycle.py @@ -0,0 +1,378 @@ +"""Integration test: a REAL watchdog recovery, end to end, on a disposable +Compose project. + +psyb0t's blocker on PR #15 (2026-09-04): the watchdog container is handed +WATCHDOG_PROJECT_DIR but never MT5_PROJECT_DIR, while docker-compose.yml +requires `${MT5_PROJECT_DIR:?}`. So recreate-vm.sh's `docker compose` failed at +interpolation and no real (non-dry-run) recovery could ever complete. The unit +suite could not see it because nothing there ran the real child environment +against real compose. This does, with nothing faked in the chain: + + the sidecar image built from Dockerfile.watchdog, the real vm-watchdog.py, + the real recreate-vm.sh, the real `docker compose`, the real Docker daemon. + +It stands up a fake "VM" whose healthcheck fails once /tmp/unhealthy exists, a +`network_mode: service:vm` sidecar, and the watchdog with the Docker socket +mounted. MT5_PROJECT_DIR is supplied ONLY to this test's own `up` - as an +operator's shell would - and deliberately NOT passed into the watchdog +container, reproducing the production environment psyb0t described. Then it: + + 1. proves the blocker is real INSIDE that container: `docker compose config` + there fails with compose's own "required variable MT5_PROJECT_DIR" error; + 2. makes the VM unhealthy and waits for the watchdog to recover it; + 3. asserts a NEW VM container exists, the sidecar's NetworkMode names it, the + sidecar has an eth0 again, and the watchdog recorded exactly one attempt. + +Runs on the host (`make test-integration`), like test_wickworks_lifecycle.py. +""" + +import os +import shutil +import subprocess +import time +from pathlib import Path + +import pytest + +pytestmark = pytest.mark.integration + +REPO_ROOT = Path(__file__).resolve().parents[2] +PROJECT = "vm-watchdog-lifecycle-test" +VM_IMAGE = "python:3.12-alpine" +SIDECAR_IMAGE = "alpine:3.20" + +RECOVERY_TIMEOUT_SECONDS = 150 +STARTUP_TIMEOUT_SECONDS = 90 + + +def _require_compose(): + if shutil.which("docker") is None: + pytest.skip("docker not available") + if subprocess.run(["docker", "compose", "version"], capture_output=True).returncode != 0: + pytest.skip("docker compose not available") + + +def _write_project(project_dir: Path) -> None: + """A compose project shaped like production where it matters. + + The watchdog service copies the shipped one: socket mount, script mount, + the project mounted at the SAME absolute path through the `${MT5_PROJECT_DIR:?}` + interpolation, WATCHDOG_* set, and MT5_PROJECT_DIR conspicuously NOT in its + environment. Timings are shortened so the test finishes in seconds, and + the image filter names the fake VM's image. + """ + scripts = project_dir / "scripts" + scripts.mkdir() + helper = scripts / "recreate-vm.sh" + shutil.copy(REPO_ROOT / "scripts" / "recreate-vm.sh", helper) + helper.chmod(0o755) + + (project_dir / "docker-compose.yml").write_text( + f"""services: + vm: + image: {VM_IMAGE} + command: ["python3", "-c", "import time\\nwhile True: time.sleep(1)"] + healthcheck: + test: ["CMD", "sh", "-c", "test ! -f /tmp/unhealthy"] + interval: 1s + timeout: 1s + retries: 1 + start_period: 0s + stop_grace_period: 3s + + sidecar: + image: {SIDECAR_IMAGE} + command: ["sleep", "infinity"] + network_mode: "service:vm" + # An ORPHAN-AWARE healthcheck, which is what the watchdog's sidecar sweep + # requires and what scripts/wickworks-healthcheck.py does for real (it + # probes the owner's gateway services). A check that only looked at itself + # would stay green inside a dead namespace, which is exactly how the + # 2026-09-07 fault hid for two days. Losing eth0 is the same signal here. + healthcheck: + test: ["CMD", "sh", "-c", "test -e /sys/class/net/eth0"] + interval: 1s + timeout: 1s + retries: 1 + start_period: 0s + stop_grace_period: 3s + depends_on: + - vm + + vm-watchdog: + build: + context: {REPO_ROOT} + dockerfile: Dockerfile.watchdog + command: ["python", "-u", "/vm-watchdog.py"] + volumes: + - /var/run/docker.sock:/var/run/docker.sock + - {REPO_ROOT}/scripts/vm-watchdog.py:/vm-watchdog.py:ro + - ${{MT5_PROJECT_DIR:?MT5_PROJECT_DIR must be the absolute host path of this project}}:${{MT5_PROJECT_DIR}}:ro + tmpfs: + - /state + environment: + WATCHDOG_STATE_DIR: /state + WATCHDOG_PROJECT_DIR: {project_dir} + WATCHDOG_RECREATE_SCRIPT: {project_dir}/scripts/recreate-vm.sh + WATCHDOG_COMPOSE_PROJECT: {PROJECT} + WATCHDOG_IMAGE_FILTER: python + WATCHDOG_WATCH_SIDECARS: "1" + WATCHDOG_MIN_FAILING_STREAK: "3" + WATCHDOG_INTERVAL_SECONDS: "1" + WATCHDOG_BACKOFF_ATTEMPTS: "5" + WATCHDOG_MAX_ATTEMPTS: "3" + WATCHDOG_RESET_SECONDS: "60" + RECREATE_STOP_TIMEOUT: "3" + # Deliberately absent: MT5_PROJECT_DIR. The watchdog must hand it to the + # helper itself; the container is not given it. +""", + encoding="utf-8", + ) + + +def _compose(project_dir: Path, *args, env_extra=None, check=True): + cmd = ["docker", "compose", "-p", PROJECT, "-f", str(project_dir / "docker-compose.yml"), *args] + # A clean environment: only the test's own `up` gets MT5_PROJECT_DIR, and + # nothing here inherits one a developer happens to have exported. + env = {"PATH": os.environ.get("PATH", "/usr/bin:/bin"), "HOME": os.environ.get("HOME", "/tmp")} + if "DOCKER_HOST" in os.environ: + env["DOCKER_HOST"] = os.environ["DOCKER_HOST"] + env.update(env_extra or {}) + result = subprocess.run(cmd, capture_output=True, text=True, env=env, timeout=600) + if check and result.returncode != 0: + raise AssertionError( + f"{' '.join(cmd)} failed (rc={result.returncode})\nstdout: {result.stdout}\nstderr: {result.stderr}" + ) + return result + + +def _name(service: str) -> str: + return f"{PROJECT}-{service}-1" + + +def _inspect(service: str, tmpl: str) -> str: + result = subprocess.run(["docker", "inspect", "--format", tmpl, _name(service)], capture_output=True, text=True) + if result.returncode != 0: + raise AssertionError(f"docker inspect {_name(service)} failed: {result.stderr}") + return result.stdout.strip() + + +def _exec(service: str, *cmd: str): + return subprocess.run(["docker", "exec", _name(service), *cmd], capture_output=True, text=True) + + +def _watchdog_logs() -> str: + return subprocess.run(["docker", "logs", _name("vm-watchdog")], capture_output=True, text=True).stdout + + +def _wait(predicate, timeout, what): + deadline = time.monotonic() + timeout + while time.monotonic() < deadline: + if predicate(): + return + time.sleep(1) + raise TimeoutError(f"timed out after {timeout}s waiting for {what}\n--- watchdog log ---\n{_watchdog_logs()}") + + +@pytest.fixture(scope="module") +def stack(tmp_path_factory): + _require_compose() + project_dir = tmp_path_factory.mktemp("vm-watchdog-lifecycle") + _write_project(project_dir) + # The operator's shell: MT5_PROJECT_DIR exported for THIS command only. + _compose(project_dir, "up", "-d", "--build", env_extra={"MT5_PROJECT_DIR": str(project_dir)}) + try: + _wait(lambda: _inspect("vm", "{{.State.Health.Status}}") == "healthy", STARTUP_TIMEOUT_SECONDS, "vm healthy") + _wait(lambda: "scoped to compose project" in _watchdog_logs(), STARTUP_TIMEOUT_SECONDS, "watchdog started") + yield project_dir + finally: + # The operator's shell again: `down` interpolates the compose file too, + # so it needs MT5_PROJECT_DIR exactly as `make down` after run.sh does + # (psyb0t's finding #2, in miniature - without it this teardown failed + # at interpolation and, being check=False, silently left a + # socket-mounted watchdog running). Loud now: a leaked privileged + # container is worse than a noisy teardown. + # --rmi local: the per-project build tag would otherwise accumulate one + # dangling watchdog image per run. + # + # The watchdog is stopped FIRST, and this is not tidiness. It holds the + # Docker socket and acts once a second; `down` removes containers one + # at a time, so a sweep landing between the sidecar's removal and the + # watchdog's own recreates the sidecar behind compose's back and leaves + # it running after the project is gone. Observed, once, exactly that + # way. Best-effort: if this fails, `down` below still has to run. + _compose( + project_dir, "stop", "-t", "3", "vm-watchdog", + env_extra={"MT5_PROJECT_DIR": str(project_dir)}, check=False, + ) + _compose( + project_dir, "down", "-v", "--remove-orphans", "--rmi", "local", + env_extra={"MT5_PROJECT_DIR": str(project_dir)}, + ) + + +def test_the_watchdog_starts_clean_against_the_real_wiring(stack): + """validate_config() passed: the project is mounted at the host path, the + helper is where WATCHDOG_RECREATE_SCRIPT says, no problems were reported.""" + log = _watchdog_logs() + assert "invalid configuration" not in log + assert f"scoped to compose project '{PROJECT}'" in log + + +def test_the_blocker_is_real_inside_the_sidecar(stack): + """Reproduce psyb0t's finding where it bites: compose run INSIDE the + watchdog container, with the container's own environment, fails to even + parse the project file. This is what every pre-fix recovery hit.""" + # The INNER command's exit code is echoed so the failure is provably the + # inner compose (inside the container), not the outer `exec` wrapper. + res = _compose(stack, "exec", "-T", "vm-watchdog", "sh", "-c", + f"docker compose -f {stack}/docker-compose.yml config --quiet; echo INNER_RC=$?", check=False) + assert res.returncode == 0, f"outer exec failed, so nothing was tested: {res.stderr}" + assert "INNER_RC=" in res.stdout and "INNER_RC=0" not in res.stdout, res.stdout + assert "required variable MT5_PROJECT_DIR" in (res.stderr + res.stdout), res.stderr + + +def test_the_watchdog_gives_the_helper_what_the_container_never_got(stack): + """The fix, observed at the boundary: recreate_env() built inside the real + sidecar carries the project dir even though the container's env does not.""" + res = _compose(stack, "exec", "-T", "vm-watchdog", "python", "-c", + "import importlib.util, os\n" + "spec = importlib.util.spec_from_file_location('wd', '/vm-watchdog.py')\n" + "wd = importlib.util.module_from_spec(spec); spec.loader.exec_module(wd)\n" + "print('container-env:', os.environ.get('MT5_PROJECT_DIR', ''))\n" + f"print('helper-env:', wd.recreate_env('{PROJECT}')['MT5_PROJECT_DIR'])\n") + assert "container-env: " in res.stdout, res.stdout + assert f"helper-env: {stack}" in res.stdout, res.stdout + + +def test_a_persistently_unhealthy_vm_is_recreated_with_its_sidecar(stack): + """The recovery, for real: make the VM unhealthy, wait for the watchdog, + and check the world afterwards rather than the log alone.""" + vm_before = _inspect("vm", "{{.Id}}") + assert _inspect("sidecar", "{{.HostConfig.NetworkMode}}") == f"container:{vm_before}" + + assert _exec("vm", "touch", "/tmp/unhealthy").returncode == 0 + + def unhealthy_or_already_recreated(): + # The watchdog may win the race and replace the container between two + # polls; inspect then fails on the vanished name. That is progress, not + # an error - the recreate assertion below is what checks the outcome. + try: + return _inspect("vm", "{{.State.Health.Status}}") == "unhealthy" + except AssertionError: + return True + + _wait(unhealthy_or_already_recreated, 30, "vm to go unhealthy") + + _wait( + lambda: "recreated vm and its sidecars" in _watchdog_logs(), + RECOVERY_TIMEOUT_SECONDS, + "the watchdog to recreate the vm", + ) + log = _watchdog_logs() + assert "recreate failed" not in log, log + assert "MT5_PROJECT_DIR is missing" not in log, log + + vm_after = _inspect("vm", "{{.Id}}") + assert vm_after != vm_before, "recovery must be a recreate, not a restart" + # Fresh container filesystem: the poison file is gone, the VM comes back healthy. + _wait(lambda: _inspect("vm", "{{.State.Health.Status}}") == "healthy", STARTUP_TIMEOUT_SECONDS, "recreated vm healthy") + + # The sidecar was recreated WITH it and rejoined the new netns. + assert _inspect("sidecar", "{{.HostConfig.NetworkMode}}") == f"container:{vm_after}" + interfaces = _exec("sidecar", "ls", "/sys/class/net").stdout.split() + assert "eth0" in interfaces, f"sidecar has no eth0 after recovery: {interfaces}" + + # One attempt, recorded under the stable compose identity (not the old id). + state = _compose(stack, "exec", "-T", "vm-watchdog", "cat", f"/state/{PROJECT}.vm.json").stdout + assert '"attempts": 1' in state, state + + +def test_a_sidecar_stranded_by_an_owner_only_restart_is_rejoined_alone(stack): + """The 2026-09-07 production fault, reproduced and then recovered. + + Restarting the OWNER alone is what `restart: unless-stopped` does after a + clean guest shutdown, and it is the case no VM ever reports: the container + id does not change, so the binding still names a running, healthy VM, while + Docker has built it a brand new namespace and left the sidecar in the old + one. Only the sidecar's own healthcheck can see that. + + What is asserted afterwards is the whole point of the change: the SIDECAR + has a new container, bound to the same VM, with a working eth0, and the VM + container was never touched. + """ + vm_before = _inspect("vm", "{{.Id}}") + sidecar_before = _inspect("sidecar", "{{.Id}}") + _wait(lambda: _inspect("sidecar", "{{.State.Health.Status}}") == "healthy", + STARTUP_TIMEOUT_SECONDS, "sidecar healthy to begin with") + + # Strand it, the way production did. Note: a RESTART, not a recreate. + subprocess.run(["docker", "restart", "-t", "3", _name("vm")], capture_output=True, check=True) + + # The fault is real before the watchdog gets to it, and invisible from the + # VM: same owner id, VM healthy, sidecar with no network. + _wait(lambda: _inspect("vm", "{{.State.Health.Status}}") == "healthy", + STARTUP_TIMEOUT_SECONDS, "the restarted vm healthy") + assert _inspect("vm", "{{.Id}}") == vm_before, "a restart must not change the container id" + assert _inspect("sidecar", "{{.Id}}") == sidecar_before, "the sidecar was not recreated" + assert _inspect("sidecar", "{{.HostConfig.NetworkMode}}") == f"container:{vm_before}" + assert "eth0" not in _exec("sidecar", "ls", "/sys/class/net").stdout.split() + + _wait( + lambda: "recreated sidecar sidecar" in _watchdog_logs(), + RECOVERY_TIMEOUT_SECONDS, + "the watchdog to recreate the stranded sidecar", + ) + log = _watchdog_logs() + assert "sidecar recreate failed" not in log, log + + _wait(lambda: _inspect("sidecar", "{{.Id}}") != sidecar_before, 60, "a new sidecar container") + interfaces = _exec("sidecar", "ls", "/sys/class/net").stdout.split() + assert "eth0" in interfaces, f"sidecar still has no eth0: {interfaces}" + _wait(lambda: _inspect("sidecar", "{{.State.Health.Status}}") == "healthy", 60, "sidecar healthy") + + # Only the sidecar. The VM the terminals live in was not restarted, not + # recreated and not stopped. + assert _inspect("vm", "{{.Id}}") == vm_before, "the vm must not have been touched" + assert _inspect("vm", "{{.State.Health.Status}}") == "healthy" + + # Recorded under the sidecar's own compose identity, with its own budget. + state = _compose(stack, "exec", "-T", "vm-watchdog", "cat", f"/state/{PROJECT}.sidecar.json").stdout + assert '"attempts": 1' in state, state + + +def test_a_sidecar_under_a_stopped_owner_is_left_running(stack): + """The counter-review's sharpest finding, pinned against a real daemon. + + `/containers/json` lists running containers only, so a stopped owner looks + exactly like a destroyed one. Acting on that would run the helper, which + stops the sidecar first and then cannot start it again - nothing to join - + leaving it STOPPED and invisible to both sweeps forever. An operator + stopping a VM for maintenance must not lose the sidecar with it. + """ + sidecar_before = _inspect("sidecar", "{{.Id}}") + subprocess.run(["docker", "stop", "-t", "3", _name("vm")], capture_output=True, check=True) + try: + marker = len(_watchdog_logs()) + # Several sweeps at WATCHDOG_INTERVAL_SECONDS=1, well past the streak. + deadline = time.monotonic() + 20 + while time.monotonic() < deadline: + assert _inspect("sidecar", "{{.State.Status}}") == "running", ( + "the sidecar was stopped while its owner was merely stopped\n" + + _watchdog_logs()[marker:] + ) + time.sleep(1) + assert _inspect("sidecar", "{{.Id}}") == sidecar_before + assert "recreated sidecar" not in _watchdog_logs()[marker:] + finally: + # Starting the owner again gives it a FRESH namespace, which strands + # the sidecar exactly as the previous test did - so hand the recreate + # to compose here rather than leaving a live orphan for the watchdog to + # act on while the fixture is tearing the project down. + subprocess.run(["docker", "start", _name("vm")], capture_output=True, check=True) + _wait(lambda: _inspect("vm", "{{.State.Health.Status}}") == "healthy", + STARTUP_TIMEOUT_SECONDS, "the vm healthy again") + _compose(stack, "up", "-d", "--force-recreate", "--no-deps", "sidecar", + env_extra={"MT5_PROJECT_DIR": str(stack)}) + _wait(lambda: _inspect("sidecar", "{{.State.Health.Status}}") == "healthy", + STARTUP_TIMEOUT_SECONDS, "the sidecar healthy again") diff --git a/tests/test_healthcheck_behavior.py b/tests/test_healthcheck_behavior.py index 1f32aeb..7d26ff6 100644 --- a/tests/test_healthcheck_behavior.py +++ b/tests/test_healthcheck_behavior.py @@ -10,6 +10,7 @@ """ import shutil import subprocess +import time from pathlib import Path import pytest @@ -189,3 +190,499 @@ def test_group_file_ignores_comments_and_blank_lines(awk_prog, tmp_path): ports = _run_awk(awk_prog, str(groupfile), config) assert ports == ["6002", "6005"] + + +# ── Verdict: is the VM dead, or just busy? ─────────────────────────────────── +# +# These run the whole script with a stub `curl` on PATH, so they exercise the +# real verdict logic rather than the awk filter alone. + +def _run_healthcheck(tmp_path, config, curl_body, grace=None, state_dir=None): + """Run healthcheck.sh with a fake curl that emits `curl_body` on stdout. + + The stub mimics curl closely enough for the script: it writes what a real + `-w '%{http_code} %{time_connect}'` would print, and exits non-zero when + the status is 000, exactly as curl does on a failed request. + + The slow-port counters go under tmp_path, so consecutive calls within one + test see each other's state (as consecutive healthchecks in one container + do) while tests never see each other's. + """ + bindir = tmp_path / "bin" + bindir.mkdir(exist_ok=True) + curl = bindir / "curl" + curl.write_text( + "#!/bin/sh\n" + f"printf '%s' '{curl_body}'\n" + f"case '{curl_body}' in 000*) exit 7 ;; esac\n" + "exit 0\n", + encoding="utf-8", + ) + curl.chmod(0o755) + + env = { + "PATH": f"{bindir}:/usr/bin:/bin", + "HEALTHCHECK_CONFIG": str(config), + "HEALTHCHECK_VM_GROUP": str(tmp_path / "no-such-group.txt"), + "HEALTHCHECK_LEASES": str(tmp_path / "no-such-leases"), + "HEALTHCHECK_STATE_DIR": state_dir or str(tmp_path / "slow-state"), + } + if grace is not None: + env["HEALTHCHECK_SLOW_GRACE"] = str(grace) + return subprocess.run( + ["sh", str(HEALTHCHECK_PATH)], + capture_output=True, text=True, env=env, timeout=60, + ) + + +ONE_TERMINAL = [{"broker": "darwinex", "account": "live", "port": "6001"}] + + +def test_a_port_that_answers_is_healthy(tmp_path): + config = _write_config(tmp_path, ONE_TERMINAL) + result = _run_healthcheck(tmp_path, config, "200 0.000181") + assert result.returncode == 0, result.stdout + result.stderr + + +def test_a_port_with_nothing_listening_is_unhealthy(tmp_path): + """Connection refused leaves time_connect at zero. This is the outage the + healthcheck exists to catch, and it must still fail.""" + config = _write_config(tmp_path, ONE_TERMINAL) + result = _run_healthcheck(tmp_path, config, "000 0.000000") + assert result.returncode == 1, result.stdout + result.stderr + assert "DOWN" in result.stdout + + +def test_a_listening_but_slow_port_is_healthy(tmp_path): + """The regression this pair exists for. + + A completed TCP handshake with no HTTP response in the probe window means + the process is alive and the guest is merely CPU-saturated - a compile or a + Strategy Tester run will do it. Reporting DOWN here makes a supervisor + restart a VM that was working, turning a slow batch into an outage. + """ + config = _write_config(tmp_path, ONE_TERMINAL) + result = _run_healthcheck(tmp_path, config, "000 0.001204") + assert result.returncode == 0, result.stdout + result.stderr + assert "slow but listening" in result.stdout + + +def test_a_missing_curl_fails_closed(tmp_path): + """An empty probe result must read as DOWN, not as 'not zero, so busy'.""" + config = _write_config(tmp_path, ONE_TERMINAL) + result = _run_healthcheck(tmp_path, config, "") + assert result.returncode == 1, result.stdout + result.stderr + + +# ── Hung is not busy: the slow tolerance is bounded ────────────────────────── +# +# psyb0t (2026-09-04): "curl result 000 with a completed TCP connection" was +# healthy unconditionally, so an API that accepts TCP but never answers HTTP +# stayed healthy forever and the watchdog never recovered it. A busy spell ends +# and the port answers; a hung port does not - so tolerate the former for a +# bounded number of CONSECUTIVE checks and then call the latter what it is. + +SLOW = "000 0.001204" # handshake completed, no HTTP inside the window +ANSWERS = "200 0.000181" +REFUSED = "000 0.000000" + + +def _slow_runs(tmp_path, config, count, grace=None): + return [_run_healthcheck(tmp_path, config, SLOW, grace=grace) for _ in range(count)] + + +def test_a_slow_port_is_tolerated_below_the_grace(tmp_path): + config = _write_config(tmp_path, ONE_TERMINAL) + for result in _slow_runs(tmp_path, config, 2, grace=3): + assert result.returncode == 0, result.stdout + result.stderr + assert "slow but listening" in result.stdout + + +def test_a_port_still_silent_at_the_grace_is_hung_and_down(tmp_path): + """The regression. Third consecutive silent check with grace=3 -> DOWN, + and the verdict says hung, not merely down, so an operator can tell it + from a refused connection.""" + config = _write_config(tmp_path, ONE_TERMINAL) + first, second, third = _slow_runs(tmp_path, config, 3, grace=3) + assert first.returncode == 0 and second.returncode == 0 + assert third.returncode == 1, third.stdout + third.stderr + assert "DOWN" in third.stdout and "hung" in third.stdout and "6001" in third.stdout + # And it stays down while it stays silent. + assert _run_healthcheck(tmp_path, config, SLOW, grace=3).returncode == 1 + + +def test_an_answer_resets_the_slow_count(tmp_path): + """slow, slow, ANSWER, slow, slow with grace=3 is healthy throughout: the + answer proves the process serves, so the count starts over.""" + config = _write_config(tmp_path, ONE_TERMINAL) + sequence = [SLOW, SLOW, ANSWERS, SLOW, SLOW] + for body in sequence: + result = _run_healthcheck(tmp_path, config, body, grace=3) + assert result.returncode == 0, (body, result.stdout + result.stderr) + # ...and the third silent check after the answer is the one that trips. + assert _run_healthcheck(tmp_path, config, SLOW, grace=3).returncode == 1 + + +def test_a_refused_connection_resets_the_slow_count(tmp_path): + """Refused is DOWN on its own terms (nothing listening), and it means the + hung process is gone - the next listener starts with a clean count.""" + config = _write_config(tmp_path, ONE_TERMINAL) + _slow_runs(tmp_path, config, 2, grace=3) + refused = _run_healthcheck(tmp_path, config, REFUSED, grace=3) + assert refused.returncode == 1 and "hung" not in refused.stdout + first, second = _slow_runs(tmp_path, config, 2, grace=3) + assert first.returncode == 0 and second.returncode == 0 + + +def test_the_default_grace_is_ten_checks(tmp_path): + """Ten consecutive silent checks is ~5 minutes at the 30s interval - longer + than any compile burst seen on the farm, far shorter than forever.""" + config = _write_config(tmp_path, ONE_TERMINAL) + results = _slow_runs(tmp_path, config, 10) + assert all(r.returncode == 0 for r in results[:9]) + assert results[9].returncode == 1 and "hung" in results[9].stdout + + +@pytest.mark.parametrize("bad", ["0", "00", "010", "banana", "", "-3", "2.5", " 5"]) +def test_a_bad_grace_falls_back_to_the_default_not_to_forever(tmp_path, bad): + """A typo must not silently restore the unbounded tolerance (or zero it). + + `00` slipped past a literal-`0` pattern and made the FIRST silent probe + hung (`[ 1 -ge 00 ]` is true); `010` would read as octal. Both now strip + to their decimal value first - `010` is simply 10, the default.""" + config = _write_config(tmp_path, ONE_TERMINAL) + results = _slow_runs(tmp_path, config, 10, grace=bad) + assert all(r.returncode == 0 for r in results[:9]), bad + assert results[9].returncode == 1, bad + + +def test_an_unwritable_state_dir_degrades_to_tolerance_not_to_restarts(tmp_path): + """If the counters cannot be kept, the script cannot know a port is hung - + so it falls back to the busy verdict rather than inventing a streak. The + wrong failure mode here would be restarting busy VMs whenever /tmp fills + up. Documented degradation, pinned so nobody 'fixes' it into fail-closed.""" + config = _write_config(tmp_path, ONE_TERMINAL) + bindir = tmp_path / "bin" + bindir.mkdir(exist_ok=True) + (bindir / "curl").write_text( + "#!/bin/sh\nprintf '%s' '000 0.001204'\nexit 7\n", encoding="utf-8" + ) + (bindir / "curl").chmod(0o755) + env = { + "PATH": f"{bindir}:/usr/bin:/bin", + "HEALTHCHECK_CONFIG": str(config), + "HEALTHCHECK_VM_GROUP": str(tmp_path / "no-such-group.txt"), + "HEALTHCHECK_LEASES": str(tmp_path / "no-such-leases"), + # A file where the directory should be: mkdir -p and every write fail. + "HEALTHCHECK_STATE_DIR": str(tmp_path / "not-a-dir"), + "HEALTHCHECK_SLOW_GRACE": "2", + } + (tmp_path / "not-a-dir").write_text("", encoding="utf-8") + for _ in range(4): + result = subprocess.run(["sh", str(HEALTHCHECK_PATH)], capture_output=True, text=True, env=env, timeout=60) + assert result.returncode == 0, result.stdout + result.stderr + assert "slow but listening" in result.stdout + # ...but never silently: the verdict says the bound is off. + assert "slow-state unwritable" in result.stdout + + +def test_a_leading_zero_grace_does_not_trip_on_the_first_probe(tmp_path): + """The exact hole the counter-review found: with grace `00` the first + silent probe used to be reported hung.""" + config = _write_config(tmp_path, ONE_TERMINAL) + first = _run_healthcheck(tmp_path, config, SLOW, grace="00") + assert first.returncode == 0, first.stdout + first.stderr + assert "hung" not in first.stdout + + +# ── Probe budget: many terminals must not blow the Docker healthcheck timeout ─ + + +MANY_TERMINALS = [ + {"broker": "darwinex", "account": "live", "instance": chr(c), "port": str(6600 + i)} + for i, c in enumerate(range(ord("a"), ord("a") + 24)) +] + + +def _run_with_slow_curl(tmp_path, config, delay, timeout): + """Run the script with a curl stub that sleeps, mimicking an unanswered probe. + + Every probe here hangs for `delay` seconds and then reports a refused + connection, which is what a terminal that has not finished starting looks + like. + """ + bindir = tmp_path / "bin" + bindir.mkdir(exist_ok=True) + curl = bindir / "curl" + curl.write_text( + "#!/bin/sh\n" + f"sleep {delay}\n" + "printf '000 0.000000'\n" + "exit 7\n", + encoding="utf-8", + ) + curl.chmod(0o755) + env = { + "PATH": f"{bindir}:/usr/bin:/bin", + "HEALTHCHECK_CONFIG": str(config), + "HEALTHCHECK_VM_GROUP": str(tmp_path / "no-such-group.txt"), + "HEALTHCHECK_LEASES": str(tmp_path / "no-such-leases"), + # Without this the run reads and writes the REAL /tmp/healthcheck-slow, + # so it is neither hermetic nor safe to run in parallel with itself. + "HEALTHCHECK_STATE_DIR": str(tmp_path / "slow-state"), + } + started = time.monotonic() + result = subprocess.run( + ["sh", str(HEALTHCHECK_PATH)], + capture_output=True, text=True, env=env, timeout=timeout, + ) + return result, time.monotonic() - started + + +def test_24_slow_terminals_still_finish_inside_the_docker_timeout(tmp_path): + """The bug this parallelisation exists for. + + 24 terminals x a 3s probe ran sequentially is 72s, well past the compose + `timeout: 30s`. Docker killed the check and recorded "Health check exceeded + timeout" — which a supervisor cannot tell apart from a dead VM, so it + restarted VMs that were merely still starting. + + Fanned out, the wall clock is one probe, not twenty-four of them. + """ + config = _write_config(tmp_path, MANY_TERMINALS) + result, elapsed = _run_with_slow_curl(tmp_path, config, delay=2, timeout=60) + + assert elapsed < 20, f"probes did not run concurrently: {elapsed:.1f}s for 24 ports" + # And the verdict must still be the useful one, naming the ports. + assert result.returncode == 1 + assert "DOWN ports:" in result.stdout + + +def test_concurrent_probes_still_report_every_dead_port(tmp_path): + """Fanning out must not lose results: all 24 ports belong in the verdict.""" + config = _write_config(tmp_path, MANY_TERMINALS) + result = _run_healthcheck(tmp_path, config, "000 0.000000") + + assert result.returncode == 1 + reported = {tok for tok in result.stdout.split() if tok.isdigit()} + # The SET, not the count: a duplicated port plus a dropped one would pass a + # length check while losing a real result. + assert reported == {t["port"] for t in MANY_TERMINALS}, result.stdout + + +def test_the_hung_bound_survives_the_fan_out(tmp_path): + """The two changes meet here. + + The bound was written against the sequential probe loop, where the counter + lived in the parent shell's own flow. Fanned out, each port's counter is + incremented inside its own background job, so this pins that all 24 still + count independently and all 24 reach the bound together - a hung API is a + hung API whether the VM carries one terminal or two dozen. + """ + config = _write_config(tmp_path, MANY_TERMINALS) + first, second, third = _slow_runs(tmp_path, config, 3, grace=3) + + assert first.returncode == 0 and second.returncode == 0 + assert third.returncode == 1, third.stdout + third.stderr + assert "hung" in third.stdout + for terminal in MANY_TERMINALS: + assert terminal["port"] in third.stdout + + +def test_a_slow_state_write_failure_is_reported_once_not_per_port(tmp_path): + """With the counters unwritable every one of the 24 jobs fails to write. + The parent learns it from a marker rather than from a variable it cannot + see across the fork, and says so once.""" + config = _write_config(tmp_path, MANY_TERMINALS) + (tmp_path / "not-a-dir").write_text("", encoding="utf-8") + result = _run_healthcheck( + tmp_path, config, SLOW, grace=3, state_dir=str(tmp_path / "not-a-dir") + ) + + assert result.returncode == 0, result.stdout + result.stderr + assert result.stdout.count("slow-state unwritable") == 1 + + +def test_a_port_configured_twice_is_probed_once(tmp_path): + """A duplicated port is a misconfiguration, and it used to be acted on + twice. + + Fanned out that is two background jobs writing one verdict file and one + slow counter, which is a real race; sequentially it was worse in a quieter + way, because the counter was incremented twice per check and the hung bound + fired at half the configured grace. Emitting each port once removes both. + """ + duplicated = [ + {"broker": "darwinex", "account": "live", "instance": "a", "port": "6001"}, + {"broker": "darwinex", "account": "live", "instance": "b", "port": "6001"}, + ] + config = _write_config(tmp_path, duplicated) + + first, second = _slow_runs(tmp_path, config, 2, grace=3) + assert first.returncode == 0 and second.returncode == 0 + # Named once in the port list... + listed = first.stdout.split("all ports up:")[1] + assert listed.split().count("6001") == 1, first.stdout + # ...and counted once, so the grace still means what it says: the third + # consecutive silent check is the one that trips it, not the second. + assert (tmp_path / "slow-state" / "6001").read_text().strip() == "2" + third = _run_healthcheck(tmp_path, config, SLOW, grace=3) + assert third.returncode == 1 and "hung" in third.stdout + + +# ── What the fan-out must not break ────────────────────────────────────────── + + +def _run_with_scripted_curl(tmp_path, config, body, extra_env=None): + """Run the script with a curl stub that can answer differently per port.""" + bindir = tmp_path / "bin" + bindir.mkdir(exist_ok=True) + curl = bindir / "curl" + curl.write_text("#!/bin/sh\n" + body, encoding="utf-8") + curl.chmod(0o755) + env = { + "PATH": f"{bindir}:/usr/bin:/bin", + "HEALTHCHECK_CONFIG": str(config), + "HEALTHCHECK_VM_GROUP": str(tmp_path / "no-such-group.txt"), + "HEALTHCHECK_LEASES": str(tmp_path / "no-such-leases"), + "HEALTHCHECK_STATE_DIR": str(tmp_path / "slow-state"), + } + env.update(extra_env or {}) + return subprocess.run( + ["sh", str(HEALTHCHECK_PATH)], capture_output=True, text=True, env=env, timeout=120 + ) + + +def test_each_port_gets_its_own_verdict(tmp_path): + """Routing. Every other test feeds one uniform reply to all 24 ports, so a + job that reported another port's result would sail through them. + + Here the reply depends on the port: one third answer, one third accept but + stay silent, one third refuse - and each third must land in its own bucket. + """ + config = _write_config(tmp_path, MANY_TERMINALS) + result = _run_with_scripted_curl( + tmp_path, config, + # $@ ends with the URL; take the port out of it. + 'url=$(eval echo \\$$#)\n' + 'port=${url##*:}; port=${port%%/*}\n' + 'case $((port % 3)) in\n' + ' 0) printf "%s" "200 0.000181"; exit 0 ;;\n' + ' 1) printf "%s" "000 0.001204"; exit 7 ;;\n' + ' 2) printf "%s" "000 0.000000"; exit 7 ;;\n' + 'esac\n', + ) + ports = [int(t["port"]) for t in MANY_TERMINALS] + reported_dead = {int(t) for t in result.stdout.split("(")[0].split() if t.isdigit()} + + assert result.returncode == 1, result.stdout + result.stderr + assert reported_dead == {p for p in ports if p % 3 == 2}, result.stdout + # And the slow third is counted as slow, not lost and not called dead. + slow_counted = sorted(int(f.name) for f in (tmp_path / "slow-state").iterdir()) + assert slow_counted == sorted(p for p in ports if p % 3 == 1) + + +def test_a_job_that_dies_before_reporting_is_a_down_port(tmp_path): + """Fail closed, which is the documented rule and had no test. + + The parent reads each job's exit status; a job killed outright returns + 128+signal, which is not one of the verdicts. That must read as down, and + must not touch the ports beside it. + """ + config = _write_config(tmp_path, MANY_TERMINALS[:3]) + doomed = MANY_TERMINALS[1]["port"] + result = _run_with_scripted_curl( + tmp_path, config, + 'url=$(eval echo \\$$#)\n' + 'port=${url##*:}; port=${port%%/*}\n' + f'if [ "$port" = "{doomed}" ]; then kill -9 $PPID; sleep 5; fi\n' + 'printf "%s" "200 0.000181"\n', + ) + assert result.returncode == 1, result.stdout + result.stderr + assert doomed in result.stdout + for survivor in (MANY_TERMINALS[0]["port"], MANY_TERMINALS[2]["port"]): + assert survivor not in result.stdout.split("(")[0], result.stdout + + +def test_a_disk_that_cannot_be_written_never_becomes_a_port_outage(tmp_path): + """The reason the verdicts travel in exit statuses rather than in files. + + A per-port verdict file under a `mktemp -d` looks obviously fine until the + disk is full: the write fails, the parent finds no verdict, and its + fail-closed rule reports every terminal on the VM as down. Ten of those and + the watchdog recreates a perfectly healthy VM, killing two dozen running + backtests, because /tmp filled up. The sequential loop this replaced had no + disk dependency and neither may this. + """ + config = _write_config(tmp_path, MANY_TERMINALS) + blocked = tmp_path / "not-a-dir" + blocked.write_text("", encoding="utf-8") + + result = _run_with_scripted_curl( + tmp_path, config, 'printf "%s" "200 0.000181"\n', + # Every place a scratch-file implementation could put its verdicts: + # a `mktemp -d` honours TMPDIR, a fixed root honours HEALTHCHECK_RUN_DIR, + # and the slow counters use HEALTHCHECK_STATE_DIR. All three point at a + # FILE, so any attempt to create a directory there fails - as root too, + # which a permission bit would not achieve. + extra_env={ + "HEALTHCHECK_STATE_DIR": str(blocked), + "HEALTHCHECK_RUN_DIR": str(blocked), + "TMPDIR": str(blocked), + }, + ) + assert result.returncode == 0, result.stdout + result.stderr + assert "ok all ports up" in result.stdout + for terminal in MANY_TERMINALS: + assert terminal["port"] in result.stdout + + +def test_the_check_leaves_nothing_behind_to_clean_up(tmp_path): + """No scratch directory means nothing can leak when Docker SIGKILLs a check + that overran its timeout - and no trap is needed to promise otherwise.""" + config = _write_config(tmp_path, MANY_TERMINALS) + before = set(Path("/tmp").glob("tmp.*")) + result = _run_healthcheck(tmp_path, config, "200 0.000181") + assert result.returncode == 0, result.stdout + result.stderr + assert set(Path("/tmp").glob("tmp.*")) == before + assert sorted(p.name for p in tmp_path.iterdir()) == ["bin", "config.yaml", "slow-state"] + + +def test_curl_is_invoked_the_way_its_output_is_parsed(tmp_path): + """The stub curl ignores its arguments, so every other test here passes + against a script that calls curl wrongly. + + It bit for real: a `-w` format that reached curl with literal quotes around + it produced output whose first field was not a status code, and the script + reported every port on both live VMs as `slow but listening` while all 24 + terminals were answering 401. Nothing in this file could see it. + + So: capture the real argument vector and assert the two things the parser + depends on - the exact `-w` format, and a bounded `--max-time`. + """ + config = _write_config(tmp_path, ONE_TERMINAL) + argv = tmp_path / "argv.txt" + result = _run_with_scripted_curl( + tmp_path, config, + f'for a in "$@"; do printf "%s\\n" "$a" >>"{argv}"; done\n' + 'printf "%s" "200 0.000181"\n', + ) + assert result.returncode == 0, result.stdout + result.stderr + + args = argv.read_text(encoding="utf-8").splitlines() + assert "%{http_code} %{time_connect}" in args, args + assert "--max-time" in args, args + assert args[args.index("--max-time") + 1] == "3", args + assert any(a.endswith(":6001/ping") for a in args), args + + +def test_a_real_http_status_from_the_stub_reads_as_up_not_slow(tmp_path): + """The symptom the mangled format produced, pinned from the other side: an + answering port must land in `all ports up` and NOT in `slow but listening`, + and a 401 from the auth layer counts as answering.""" + config = _write_config(tmp_path, ONE_TERMINAL) + for body in ("200 0.000181", "401 0.000379", "503 0.000112"): + result = _run_healthcheck(tmp_path, config, body) + assert result.returncode == 0, result.stdout + result.stderr + assert "slow but listening" not in result.stdout, (body, result.stdout) + assert result.stdout.startswith("ok all ports up:"), (body, result.stdout) diff --git a/tests/test_run_env_persistence.py b/tests/test_run_env_persistence.py new file mode 100644 index 0000000..e3e946b --- /dev/null +++ b/tests/test_run_env_persistence.py @@ -0,0 +1,204 @@ +"""run.sh must persist MT5_PROJECT_DIR to .env, not only export it. + +psyb0t (2026-09-04): run.sh exported MT5_PROJECT_DIR to its own shell and +truncated/rebuilt .env without writing it, so `make down`, `make logs`, a +manual `docker compose` and - through recreate-vm.sh - the watchdog's own +recovery all failed at `${MT5_PROJECT_DIR:?}` interpolation once run.sh had +exited. + +These run the ACTUAL .env-generation block lifted out of run.sh (bounded by +two comment lines that already exist there) under bash with a stub +config_helper, and assert on the .env it writes. A source-substring assertion +would pass with the line in the wrong place or behind a failing command; this +does not. +""" + +import os +import subprocess +from pathlib import Path + +import pytest + +_REPO = Path(__file__).resolve().parents[1] +RUN_SH = _REPO / "run.sh" + +# The block starts at the truncation and ends where nginx generation begins. +_START = "# Generate fresh .env each run." +_END = "# Generate nginx.conf from config.yaml terminals." + + +def _env_block(): + src = RUN_SH.read_text(encoding="utf-8") + _before, sep, rest = src.partition(_START) + assert sep, "start marker missing from run.sh - the .env block moved" + block, sep2, _after = rest.partition(_END) + assert sep2, "end marker missing from run.sh - the .env block moved" + return block + + +def _run_env_block(tmp_path, *, token="tok-123", ts_key="", ts_server="", fail_on=None): + """Execute the block with DIR=tmp_path and a config_helper stub on PATH. + + `fail_on` names a config_helper subcommand the stub should fail on, to show + what .env looks like when the script dies partway through. + """ + bindir = tmp_path / "bin" + bindir.mkdir(exist_ok=True) + stub = bindir / "python3" + stub.write_text( + "#!/bin/sh\n" + f"[ \"$2\" = \"{fail_on or ''}\" ] && [ -n \"{fail_on or ''}\" ] && exit 1\n" + 'case "$2" in\n' + f" api_token) printf '%s' '{token}' ;;\n" + f" ts_auth_key) printf '%s' '{ts_key}' ;;\n" + f" ts_login_server) printf '%s' '{ts_server}' ;;\n" + ' *) echo "stub config_helper: unexpected $*" >&2; exit 1 ;;\n' + "esac\n", + encoding="utf-8", + ) + stub.chmod(0o755) + + script = ( + "set -eo pipefail\n" + f'DIR="{tmp_path}"\n' + 'CFG="${DIR}/scripts/config_helper.py"\n' + + _env_block() + ) + env = { + "PATH": f"{bindir}{os.pathsep}{os.environ.get('PATH', '/usr/bin:/bin')}", + "MT5_PROJECT_DIR": str(tmp_path), + } + return subprocess.run( + ["bash", "-c", script], capture_output=True, text=True, env=env, timeout=60, check=False, + ) + + +def _env_lines(tmp_path): + return (tmp_path / ".env").read_text(encoding="utf-8").splitlines() + + +def _project_dir_line(path): + # Single-quoted on purpose: unquoted, `$` or ` #` in a path is mangled by + # compose's dotenv parser. + return f"MT5_PROJECT_DIR='{path}'" + + +def test_env_carries_the_project_dir_first(tmp_path): + res = _run_env_block(tmp_path) + assert res.returncode == 0, res.stderr + lines = _env_lines(tmp_path) + assert lines[0] == _project_dir_line(tmp_path) + assert "API_TOKEN=tok-123" in lines + + +def test_env_is_regenerated_not_appended_to(tmp_path): + """The truncation is load-bearing: a stale value must not survive a re-run.""" + (tmp_path / ".env").write_text("MT5_PROJECT_DIR=/old/path\nSTALE=1\n", encoding="utf-8") + res = _run_env_block(tmp_path) + assert res.returncode == 0, res.stderr + lines = _env_lines(tmp_path) + assert lines.count(_project_dir_line(tmp_path)) == 1 + assert "MT5_PROJECT_DIR=/old/path" not in lines + assert "STALE=1" not in lines + + +def test_the_project_dir_survives_a_failure_later_in_the_block(tmp_path): + """Written before anything that can fail. If reading the API token blows + up, .env must still make compose usable rather than be left half-built + without the one variable every compose command needs.""" + res = _run_env_block(tmp_path, fail_on="api_token") + assert res.returncode != 0 + assert _env_lines(tmp_path) == [_project_dir_line(tmp_path)] + + +def test_optional_tailscale_values_are_still_written_when_present(tmp_path): + """The block's existing behaviour is unchanged around the new line.""" + res = _run_env_block(tmp_path, ts_key="tskey-abc", ts_server="https://hs.example") + assert res.returncode == 0, res.stderr + lines = _env_lines(tmp_path) + assert lines[0] == _project_dir_line(tmp_path) + assert "TS_AUTHKEY=tskey-abc" in lines + assert any(line.startswith("TS_EXTRA_ARGS=") and "hs.example" in line for line in lines) + + +# ── run.sh refuses a MT5_PROJECT_DIR that is not this checkout ─────────────── +# +# Counter-review finding: `${MT5_PROJECT_DIR:-$DIR}` accepted any pre-exported +# value. With two clones and a stale export from the other one, .env would +# record the wrong path and the watchdog would recreate THIS project's VMs from +# the OTHER clone's compose file. These run a copy of the real run.sh from a +# temp dir; the check sits before anything the script creates or downloads. + + +def _run_copy_of_run_sh(tmp_path, project_dir_value): + import shutil + + script = tmp_path / "run.sh" + shutil.copy(RUN_SH, script) + script.chmod(0o755) + env = {"PATH": os.environ.get("PATH", "/usr/bin:/bin"), "SKIP_KVM_CHECK": "1"} + if project_dir_value is not None: + env["MT5_PROJECT_DIR"] = project_dir_value + return subprocess.run([str(script)], capture_output=True, text=True, env=env, timeout=60, check=False) + + +def test_run_sh_refuses_a_project_dir_that_is_another_checkout(tmp_path): + other = tmp_path / "other-clone" + other.mkdir() + res = _run_copy_of_run_sh(tmp_path, str(other)) + assert res.returncode == 1 + assert "MT5_PROJECT_DIR is" in res.stdout + res.stderr + # It stopped before doing any of run.sh's work in the directory. + assert not (tmp_path / "data").exists() + assert not (tmp_path / ".env").exists() + + +def test_run_sh_accepts_the_same_checkout_through_a_symlink(tmp_path): + """Equality is by resolved path, so a symlinked checkout is not rejected.""" + link = tmp_path.parent / (tmp_path.name + "-link") + link.symlink_to(tmp_path, target_is_directory=True) + try: + res = _run_copy_of_run_sh(tmp_path, str(link)) + finally: + link.unlink() + # Past the guard it fails later, on the missing compose sources - the + # point is only that the guard did not fire. + assert "MT5_PROJECT_DIR is" not in res.stdout + res.stderr + assert "neither vms.yaml" in res.stdout + res.stderr + + +def test_run_sh_derives_the_project_dir_when_unset(tmp_path): + res = _run_copy_of_run_sh(tmp_path, None) + assert "MT5_PROJECT_DIR is" not in res.stdout + res.stderr + assert "neither vms.yaml" in res.stdout + res.stderr + + +@pytest.mark.skipif( + subprocess.run(["docker", "compose", "version"], capture_output=True).returncode != 0 + if __import__("shutil").which("docker") else True, + reason="needs the docker compose CLI (the offline test image has none; runs on the host)", +) +def test_the_written_env_serves_compose_outside_run_sh(tmp_path): + """The property psyb0t named: a compose command in a FRESH shell, with no + MT5_PROJECT_DIR exported, must interpolate the shipped compose file from + .env alone - and fail without that line, so this is testing the line.""" + import shutil + + shutil.copy(_REPO / "docker-compose.yml.example", tmp_path / "docker-compose.yml") + assert _run_env_block(tmp_path).returncode == 0 + + def compose_config(env_extra=None): + env = {"PATH": os.environ.get("PATH", "/usr/bin:/bin"), "HOME": os.environ.get("HOME", "/tmp")} + env.update(env_extra or {}) + return subprocess.run( + ["docker", "compose", "-f", str(tmp_path / "docker-compose.yml"), "config", "--quiet"], + capture_output=True, text=True, env=env, cwd=str(tmp_path), timeout=120, check=False, + ) + + ok = compose_config() + assert ok.returncode == 0, ok.stderr + + (tmp_path / ".env").write_text("API_TOKEN=tok-123\n", encoding="utf-8") + broken = compose_config() + assert broken.returncode != 0 + assert "MT5_PROJECT_DIR" in broken.stderr diff --git a/tests/test_vm_watchdog.py b/tests/test_vm_watchdog.py new file mode 100644 index 0000000..3420817 --- /dev/null +++ b/tests/test_vm_watchdog.py @@ -0,0 +1,1328 @@ +"""Behavioural coverage for the compose-managed vm-watchdog sidecar. + +These tests exercise the decision policy against a fake Docker transport, so +they run in the offline suite (Dockerfile.test) with no docker daemon. They +cover the behaviours psyb0t asked for on PR #15: healthy/starting exclusion, +sustained unhealthy restart, image/label scoping, cooldown/backoff, bounded +retries, and reset after stable health. +""" + +import importlib.util +import json +import os +import shutil +import subprocess +from pathlib import Path + +import pytest + +SCRIPT = Path(__file__).resolve().parents[1] / "scripts" / "vm-watchdog.py" + + +def _load(): + spec = importlib.util.spec_from_file_location("vm_watchdog_test", SCRIPT) + module = importlib.util.module_from_spec(spec) + assert spec.loader is not None + spec.loader.exec_module(module) + return module + + +@pytest.fixture +def wd(): + module = _load() + module.STATE_DIR = "unused-in-policy" + return module + + +class _Recorder: + """Captures recreate_vm() calls in place of the old restart log.""" + + def __init__(self): + self.calls = [] + self.raises = None + + @property + def services(self): + return [service for service, _project in self.calls] + + def __call__(self, service, project): + if self.raises: + raise self.raises + self.calls.append((service, project)) + + +@pytest.fixture +def recorder(wd, monkeypatch): + rec = _Recorder() + monkeypatch.setattr(wd, "recreate_vm", rec) + return rec + + +# ── Fake Docker transport ──────────────────────────────────────────────────── + + +class _FakeClient: + """Scripted docker client: containers list and per-id health. + + Recovery is deliberately NOT on this client any more: a restart through the + Docker API strands the wickworks sidecar (fresh netns on start), so the + watchdog shells out to scripts/recreate-vm.sh instead. The recreate calls + it makes are captured by the `recorder` fixture below. + """ + + def __init__(self, containers, health=None, project="mt5-httpapi"): + self.containers = containers + self.health = health or {} + self.project = project + + def list_containers(self): + return self.containers + + def inspect(self, cid): + if cid == "self-watchdog-id": + return { + "Config": { + "Labels": {"com.docker.compose.project": self.project} + } + } + if cid in self.health: + return {"State": {"Health": self.health[cid]}} + return {"State": {}} + + + +def _container(cid, name, image="dockurr/windows:5.14", labels=None): + return { + "Id": cid, + "Names": [f"/{name}"], + "Image": image, + # The service label is what a recreate names; default it to the id so + # the assertions below can keep talking about one identifier. + "Labels": labels or { + "com.docker.compose.project": "mt5-httpapi", + "com.docker.compose.service": cid, + }, + } + + +def _health(status, streak): + return {"Status": status, "FailingStreak": streak} + + +# ── Healthy / starting exclusion ───────────────────────────────────────────── + +def test_healthy_vm_is_never_restarted(wd, tmp_path, recorder): + wd.STATE_DIR = str(tmp_path) + cid = "aaa" + client = _FakeClient( + [_container(cid, "mt5")], + health={cid: _health("healthy", 0)}, + ) + with pytest.MonkeyPatch.context() as mp: + mp.setattr(wd, "log", lambda *a, **k: None) + assert wd.sweep_once(client, "mt5-httpapi") == 0 + assert recorder.services == [] + # healthy clock starts, but nothing is reset yet (below reset window). + state = json.loads((tmp_path / f"{wd.state_key('mt5-httpapi', cid)}.json").read_text()) + assert state["healthy_since"] > 0 + assert state["attempts"] == 0 + + +def test_starting_vm_is_never_restarted(wd, tmp_path, recorder): + wd.STATE_DIR = str(tmp_path) + cid = "aaa" + client = _FakeClient( + [_container(cid, "mt5")], + health={cid: _health("starting", 0)}, + ) + with pytest.MonkeyPatch.context() as mp: + mp.setattr(wd, "log", lambda *a, **k: None) + assert wd.sweep_once(client, "mt5-httpapi") == 0 + assert recorder.services == [] + # starting must not start the healthy clock either. + state = json.loads((tmp_path / f"{wd.state_key('mt5-httpapi', cid)}.json").read_text()) + assert state["healthy_since"] == 0 + + +# ── Sustained unhealthy threshold ──────────────────────────────────────────── + +def test_unhealthy_below_streak_threshold_waits(wd, tmp_path, recorder): + wd.STATE_DIR = str(tmp_path) + wd.MIN_FAILING_STREAK = 10 + cid = "aaa" + client = _FakeClient( + [_container(cid, "mt5")], + health={cid: _health("unhealthy", 4)}, + ) + with pytest.MonkeyPatch.context() as mp: + mp.setattr(wd, "log", lambda *a, **k: None) + assert wd.sweep_once(client, "mt5-httpapi") == 0 + assert recorder.services == [] + + +def test_unhealthy_at_streak_threshold_restarts(wd, tmp_path, recorder): + wd.STATE_DIR = str(tmp_path) + wd.MIN_FAILING_STREAK = 10 + cid = "aaa" + client = _FakeClient( + [_container(cid, "mt5")], + health={cid: _health("unhealthy", 10)}, + ) + with pytest.MonkeyPatch.context() as mp: + mp.setattr(wd, "log", lambda *a, **k: None) + assert wd.sweep_once(client, "mt5-httpapi") == 1 + assert recorder.services == [cid] + + +# ── Image / label scoping ──────────────────────────────────────────────────── + +def test_only_this_projects_vm_image_containers_are_restarted(wd, tmp_path, recorder): + wd.STATE_DIR = str(tmp_path) + wd.MIN_FAILING_STREAK = 1 + vm = _container("vm1", "mt5") + other_project = _container("other1", "some-other-vm", labels={"com.docker.compose.project": "other"}) + sidecar = _container("side1", "wickworks", image="psyb0t/wickworks:v0.3.1") + logrotator = _container("rot1", "log-rotator", image="alpine:3.20") + client = _FakeClient( + [vm, other_project, sidecar, logrotator], + health={ + "vm1": _health("unhealthy", 99), + "other1": _health("unhealthy", 99), + "side1": _health("unhealthy", 99), + "rot1": _health("unhealthy", 99), + }, + ) + with pytest.MonkeyPatch.context() as mp: + mp.setattr(wd, "log", lambda *a, **k: None) + assert wd.sweep_once(client, "mt5-httpapi") == 1 + assert recorder.services == ["vm1"] + + +# ── Backoff ────────────────────────────────────────────────────────────────── + +def test_exponential_backoff_between_restarts(wd, tmp_path, recorder): + wd.STATE_DIR = str(tmp_path) + wd.MIN_FAILING_STREAK = 1 + wd.BACKOFF_ATTEMPTS = [300, 900, 3600] + wd.MAX_ATTEMPTS = 99 + cid = "aaa" + client = _FakeClient([_container(cid, "mt5")], health={cid: _health("unhealthy", 99)}) + + now = 1_000_000 + with pytest.MonkeyPatch.context() as mp: + mp.setattr(wd, "log", lambda *a, **k: None) + # First restart is immediate at threshold. + assert wd.sweep_once(client, "mt5-httpapi", now=now) == 1 + # After 1st restart, backoff tier 0 = 300s: too soon at +299, ok at +301. + assert wd.sweep_once(client, "mt5-httpapi", now=now + 299) == 0 + assert wd.sweep_once(client, "mt5-httpapi", now=now + 301) == 1 + # After 2nd restart (at +301), backoff tier 1 = 900s. + assert wd.sweep_once(client, "mt5-httpapi", now=now + 302) == 0 + assert wd.sweep_once(client, "mt5-httpapi", now=now + 301 + 901) == 1 + # After 3rd restart (at +1202), backoff tier 2 = 3600s. + assert wd.sweep_once(client, "mt5-httpapi", now=now + 1203) == 0 + assert wd.sweep_once(client, "mt5-httpapi", now=now + 1202 + 3601) == 1 + + assert recorder.services == [cid, cid, cid, cid] + state = json.loads((tmp_path / f"{wd.state_key('mt5-httpapi', cid)}.json").read_text()) + assert state["attempts"] == 4 + + +# ── Bounded retries ────────────────────────────────────────────────────────── + +def test_gives_up_after_max_attempts(wd, tmp_path, capsys, recorder): + wd.STATE_DIR = str(tmp_path) + wd.MIN_FAILING_STREAK = 1 + wd.MAX_ATTEMPTS = 3 + wd.BACKOFF_ATTEMPTS = [0, 0, 0] # no backoff: three rapid attempts + cid = "aaa" + client = _FakeClient([_container(cid, "mt5")], health={cid: _health("unhealthy", 99)}) + + now = 1_000_000 + assert wd.sweep_once(client, "mt5-httpapi", now=now) == 1 + assert wd.sweep_once(client, "mt5-httpapi", now=now + 1) == 1 + assert wd.sweep_once(client, "mt5-httpapi", now=now + 2) == 1 + # Attempt budget exhausted: no more restarts, and it logs loudly. + assert wd.sweep_once(client, "mt5-httpapi", now=now + 3) == 0 + assert wd.sweep_once(client, "mt5-httpapi", now=now + 999_999) == 0 + + assert recorder.services == [cid, cid, cid] + assert "GIVING UP" in capsys.readouterr().out + + +# ── Reset after stable health ──────────────────────────────────────────────── + +def test_attempts_reset_after_sustained_healthy_period(wd, tmp_path, recorder): + wd.STATE_DIR = str(tmp_path) + wd.MIN_FAILING_STREAK = 1 + wd.MAX_ATTEMPTS = 1 + wd.BACKOFF_ATTEMPTS = [0] + wd.RESET_SECONDS = 1800 + cid = "aaa" + state_file = tmp_path / f"{wd.state_key('mt5-httpapi', cid)}.json" + + def make_client(status, streak): + return _FakeClient([_container(cid, "mt5")], health={cid: _health(status, streak)}) + + now = 1_000_000 + with pytest.MonkeyPatch.context() as mp: + mp.setattr(wd, "log", lambda *a, **k: None) + # Two unhealthy sweeps: first restarts, second exceeds MAX_ATTEMPTS. + assert wd.sweep_once(make_client("unhealthy", 99), "mt5-httpapi", now=now) == 1 + assert wd.sweep_once(make_client("unhealthy", 99), "mt5-httpapi", now=now + 1) == 0 + # The budget really is spent - one attempt recorded, one recreate made. + # (This line used to read `assert ... or True`, which asserted nothing.) + assert json.loads(state_file.read_text())["attempts"] == 1 + assert recorder.services == [cid] + + # VM recovers and stays healthy long enough to reset the budget. + assert wd.sweep_once(make_client("healthy", 0), "mt5-httpapi", now=now + 100) == 0 + assert wd.sweep_once(make_client("healthy", 0), "mt5-httpapi", now=now + 100 + 1800) == 0 + state = json.loads(state_file.read_text()) + assert state["attempts"] == 0 + assert state["last_restart"] == 0 + + # A later crash gets a fresh budget again. + assert wd.sweep_once(make_client("unhealthy", 99), "mt5-httpapi", now=now + 100 + 1801) == 1 + assert recorder.services == [cid, cid] + + +# ── healthy_since means CONTINUOUSLY healthy ───────────────────────────────── +# +# psyb0t (2026-09-04): healthy_since survived a `starting` state, so a VM that +# was healthy before a restart had its attempt budget refunded on the first +# healthy poll AFTER it - not after RESET_SECONDS of proven stability. The same +# leak existed for an unhealthy poll below the streak threshold. + + +def _vm(cid, status, streak=0): + return _FakeClient([_container(cid, "mt5")], health={cid: _health(status, streak)}) + + +def _budget_spent_then_healthy_at(wd, cid, now): + """Spend the single-attempt budget, then observe the VM healthy at `now`. + Returns nothing; the caller continues the timeline.""" + assert wd.sweep_once(_vm(cid, "unhealthy", 99), "mt5-httpapi", now=now - 10) == 1 + assert wd.sweep_once(_vm(cid, "healthy"), "mt5-httpapi", now=now) == 0 + + +@pytest.mark.parametrize( + "interruption", + [("starting", 0), ("unhealthy", 1), ("none", 0)], + ids=["starting", "unhealthy-below-streak", "no-healthcheck"], +) +def test_any_non_healthy_observation_restarts_the_reset_clock(wd, tmp_path, recorder, interruption): + wd.STATE_DIR = str(tmp_path) + wd.MIN_FAILING_STREAK = 10 + wd.MAX_ATTEMPTS = 1 + wd.BACKOFF_ATTEMPTS = [0] + wd.RESET_SECONDS = 1800 + cid = "aaa" + status, streak = interruption + key = wd.state_key("mt5-httpapi", cid) + + t0 = 1_000_000 + with pytest.MonkeyPatch.context() as mp: + mp.setattr(wd, "log", lambda *a, **k: None) + # Streak 99 so the spend is not blocked by the threshold above. + assert wd.sweep_once(_vm(cid, "unhealthy", 99), "mt5-httpapi", now=t0 - 10) == 1 + assert wd.sweep_once(_vm(cid, "healthy"), "mt5-httpapi", now=t0) == 0 + # Interrupted 1000s in, back to healthy 1900s in: 1900s since the + # ORIGINAL healthy poll (> RESET_SECONDS), but only 900s continuous. + assert wd.sweep_once(_vm(cid, status, streak), "mt5-httpapi", now=t0 + 1000) == 0 + assert wd.sweep_once(_vm(cid, "healthy"), "mt5-httpapi", now=t0 + 1900) == 0 + assert wd.load_state(key)["attempts"] == 1, "budget refunded across a non-healthy state" + + # Continuity restored at t0+1900: the reset happens 1800s after THAT. + assert wd.sweep_once(_vm(cid, "healthy"), "mt5-httpapi", now=t0 + 1900 + 1799) == 0 + assert wd.load_state(key)["attempts"] == 1 + assert wd.sweep_once(_vm(cid, "healthy"), "mt5-httpapi", now=t0 + 1900 + 1800) == 0 + assert wd.load_state(key)["attempts"] == 0 + + assert recorder.services == [cid] + + +def test_the_healthy_clock_starts_at_the_first_healthy_poll_after_recovery(wd, tmp_path, recorder): + """The recovery itself zeroes the clock; the next healthy poll starts it.""" + wd.STATE_DIR = str(tmp_path) + wd.MIN_FAILING_STREAK = 1 + wd.BACKOFF_ATTEMPTS = [0] + cid = "aaa" + key = wd.state_key("mt5-httpapi", cid) + with pytest.MonkeyPatch.context() as mp: + mp.setattr(wd, "log", lambda *a, **k: None) + assert wd.sweep_once(_vm(cid, "unhealthy", 99), "mt5-httpapi", now=500) == 1 + assert wd.load_state(key)["healthy_since"] == 0 + assert wd.sweep_once(_vm(cid, "starting"), "mt5-httpapi", now=600) == 0 + assert wd.load_state(key)["healthy_since"] == 0 + assert wd.sweep_once(_vm(cid, "healthy"), "mt5-httpapi", now=700) == 0 + assert wd.load_state(key)["healthy_since"] == 700 + + +# ── Dry-run ────────────────────────────────────────────────────────────────── + +def test_dry_run_reports_without_restarting(wd, tmp_path, recorder): + wd.STATE_DIR = str(tmp_path) + wd.MIN_FAILING_STREAK = 1 + cid = "aaa" + client = _FakeClient([_container(cid, "mt5")], health={cid: _health("unhealthy", 99)}) + with pytest.MonkeyPatch.context() as mp: + mp.setattr(wd, "log", lambda *a, **k: None) + assert wd.sweep_once(client, "mt5-httpapi", dry_run=True) == 1 + assert recorder.services == [] + + +def test_dry_run_writes_no_state(wd, tmp_path, recorder): + """A dry pass must not consume the real backoff or attempt budget. + + decide() mutates the state it is handed, and sweep_once used to persist it + before branching on dry_run. So --dry-run spent attempts and stamped + last_restart without restarting anything, and enough dry passes left the VM + at GIVING UP the moment dry-run was switched off - the supervisor refusing + to act precisely when it was finally allowed to. + """ + wd.STATE_DIR = str(tmp_path) + wd.MIN_FAILING_STREAK = 1 + cid = "aaa" + client = _FakeClient([_container(cid, "mt5")], health={cid: _health("unhealthy", 10)}) + + with pytest.MonkeyPatch.context() as mp: + mp.setattr(wd, "log", lambda *a, **k: None) + assert wd.sweep_once(client, "mt5-httpapi", dry_run=True, now=1000000) == 1 + + assert recorder.services == [] + written = list(tmp_path.iterdir()) + assert written == [], f"dry-run persisted state: {[p.name for p in written]}" + + +def test_repeated_dry_runs_do_not_exhaust_the_attempt_budget(wd, tmp_path, recorder): + """The consequence, end to end: dry passes then a real one. + + Whatever the attempt cap is, running dry that many times and then switching + dry-run off must still restart. Asserting on the restart rather than on a + number keeps this honest if the cap ever moves. + """ + wd.STATE_DIR = str(tmp_path) + wd.MIN_FAILING_STREAK = 1 + cid = "aaa" + + def fresh(): + return _FakeClient([_container(cid, "mt5")], health={cid: _health("unhealthy", 10)}) + + with pytest.MonkeyPatch.context() as mp: + mp.setattr(wd, "log", lambda *a, **k: None) + for i in range(wd.MAX_ATTEMPTS + 2): + wd.sweep_once(fresh(), "mt5-httpapi", dry_run=True, now=1000000 + i * 100000) + wd.sweep_once(fresh(), "mt5-httpapi", dry_run=False, now=2000000) + + assert recorder.services == [cid], "dry runs consumed the budget for the real one" + + +def test_a_real_run_still_persists_state(wd, tmp_path, recorder): + """The other direction: skipping persistence must be dry-run only.""" + wd.STATE_DIR = str(tmp_path) + wd.MIN_FAILING_STREAK = 1 + cid = "aaa" + client = _FakeClient([_container(cid, "mt5")], health={cid: _health("unhealthy", 10)}) + + with pytest.MonkeyPatch.context() as mp: + mp.setattr(wd, "log", lambda *a, **k: None) + wd.sweep_once(client, "mt5-httpapi", dry_run=False, now=1000000) + + assert recorder.services == [cid] + state = wd.load_state(wd.state_key("mt5-httpapi", cid)) + assert state["attempts"] == 1 + assert state["last_restart"] == 1000000 + + +# ── Project self-discovery ─────────────────────────────────────────────────── + +def test_compose_project_discovered_from_own_labels(wd): + client = _FakeClient([], project="mt5-httpapi") + wd.SELF_ID = "self-watchdog-id" + assert wd._compose_project(client) == "mt5-httpapi" + + +def test_recreate_failure_keeps_state_for_backoff(wd, tmp_path, recorder): + """A failed recreate must still record the attempt + timestamp so the + backoff logic prevents an immediate retry loop.""" + wd.STATE_DIR = str(tmp_path) + wd.MIN_FAILING_STREAK = 1 + wd.BACKOFF_ATTEMPTS = [300] + cid = "aaa" + client = _FakeClient( + [_container(cid, "mt5")], + health={cid: _health("unhealthy", 99)}, + ) + recorder.raises = RuntimeError("boom") + now = 1_000_000 + with pytest.MonkeyPatch.context() as mp: + mp.setattr(wd, "log", lambda *a, **k: None) + assert wd.sweep_once(client, "mt5-httpapi", now=now) == 1 + # Recreate failed, but the state now enforces the backoff window. + assert wd.sweep_once(client, "mt5-httpapi", now=now + 10) == 0 + assert wd.sweep_once(client, "mt5-httpapi", now=now + 301) == 1 + assert recorder.services == [] + + +# ── Configuration parsing ──────────────────────────────────────────────────── +# +# The daemon holds the Docker socket, so bad configuration has to stop it at +# startup rather than surface as a crash mid-sweep. Reported by psyb0t: an +# empty WATCHDOG_BACKOFF_ATTEMPTS survived startup as [] and then raised +# IndexError inside decide() on the first unhealthy pass after a restart. + +_REPO = SCRIPT.resolve().parents[1] + + +def _load_with(monkeypatch, **env): + for key in list(os.environ): + if key.startswith("WATCHDOG_"): + monkeypatch.delenv(key, raising=False) + # The recovery wiring is supplied by the compose service, not defaulted in + # the script, so provide it here the way the shipped compose does. Tests + # that care about it missing pass it explicitly. + env.setdefault("WATCHDOG_PROJECT_DIR", str(_REPO)) + env.setdefault("WATCHDOG_RECREATE_SCRIPT", str(_REPO / "scripts" / "recreate-vm.sh")) + for key, value in env.items(): + monkeypatch.setenv(key, value) + return _load() + + +def test_empty_backoff_list_is_refused_at_startup(monkeypatch): + """The reported crash. [] survived startup, then IndexError'd in decide().""" + wd = _load_with(monkeypatch, WATCHDOG_BACKOFF_ATTEMPTS="") + problems = wd.validate_config() + assert any("WATCHDOG_BACKOFF_ATTEMPTS" in p for p in problems), problems + assert wd.main() == 2 + + +def test_a_refused_config_never_reaches_the_indexing_that_crashed(monkeypatch): + """Defence in depth: even if validation were bypassed, the fallback value + must be usable rather than empty, so decide() cannot raise IndexError.""" + wd = _load_with(monkeypatch, WATCHDOG_BACKOFF_ATTEMPTS="") + assert wd.BACKOFF_ATTEMPTS, "fell back to an empty list" + state = {"attempts": 1, "last_restart": 1000, "healthy_since": 0} + wd.decide(state, "unhealthy", 99, 1000) # must not raise + + +@pytest.mark.parametrize("value", ["", " ", "abc", "0", "-1"]) +def test_bad_interval_values_are_reported(monkeypatch, value): + wd = _load_with(monkeypatch, WATCHDOG_INTERVAL_SECONDS=value) + assert any("WATCHDOG_INTERVAL_SECONDS" in p for p in wd.validate_config()), value + + +@pytest.mark.parametrize( + "name,value", + [ + ("WATCHDOG_MIN_FAILING_STREAK", "0"), + ("WATCHDOG_MAX_ATTEMPTS", "0"), + ("WATCHDOG_RESET_SECONDS", "0"), + ("WATCHDOG_BACKOFF_ATTEMPTS", "300,0"), + ("WATCHDOG_BACKOFF_ATTEMPTS", "300,-5"), + ("WATCHDOG_BACKOFF_ATTEMPTS", "300,abc"), + ], +) +def test_zero_and_negative_are_rejected_where_they_make_no_sense(monkeypatch, name, value): + wd = _load_with(monkeypatch, **{name: value}) + assert any(name in p for p in wd.validate_config()), (name, value) + + +def test_an_empty_image_filter_is_refused(monkeypatch): + """Not in the report, same class of bug and worse consequences. + + "".startswith() matches every image, so a blank filter would make every + container in the project a restart candidate - the watchdog itself, and any + database sharing the project. + """ + wd = _load_with(monkeypatch, WATCHDOG_IMAGE_FILTER="") + assert any("WATCHDOG_IMAGE_FILTER" in p for p in wd.validate_config()) + assert wd.main() == 2 + + +def test_every_problem_is_reported_at_once(monkeypatch): + """One message listing all of them beats fixing them one traceback at a time.""" + wd = _load_with( + monkeypatch, + WATCHDOG_BACKOFF_ATTEMPTS="", + WATCHDOG_INTERVAL_SECONDS="0", + WATCHDOG_MAX_ATTEMPTS="nope", + ) + problems = wd.validate_config() + assert len(problems) >= 3, problems + + +def test_a_valid_configuration_reports_nothing(monkeypatch): + wd = _load_with( + monkeypatch, + WATCHDOG_BACKOFF_ATTEMPTS="60, 120 ,240", + WATCHDOG_INTERVAL_SECONDS="15", + WATCHDOG_MAX_ATTEMPTS="5", + ) + assert wd.validate_config() == [] + assert wd.BACKOFF_ATTEMPTS == [60, 120, 240] + assert wd.INTERVAL_SECONDS == 15 + + +def test_defaults_alone_are_valid(monkeypatch): + """The shipped compose sets almost nothing, so the defaults must pass.""" + wd = _load_with(monkeypatch) + assert wd.validate_config() == [] + + +# ── Coordinated recreate ───────────────────────────────────────────────────── +# +# Why recovery is a recreate and not a restart: a sidecar sharing the VM's netns +# (`network_mode: service:`) resolves that binding once, at its own start. +# Restarting the owner keeps its container id but gives it a FRESH netns, so the +# sidecar is left on a dead one — tests/integration/test_wickworks_lifecycle.py +# asserts exactly that. Only recreating the owner together with its sidecars +# repairs it, which is what scripts/recreate-vm.sh does. + + +def test_recovery_names_the_compose_service_not_the_container_id(wd, tmp_path, recorder): + """recreate-vm.sh takes compose service names; a container id means nothing + to compose, and passing one would recreate nothing at all.""" + wd.STATE_DIR = str(tmp_path) + wd.MIN_FAILING_STREAK = 1 + cid = "9f3c1a2b4d5e" + client = _FakeClient( + [_container(cid, "mt5", labels={ + "com.docker.compose.project": "mt5-httpapi", + "com.docker.compose.service": "mt5-fast", + })], + health={cid: _health("unhealthy", 99)}, + ) + with pytest.MonkeyPatch.context() as mp: + mp.setattr(wd, "log", lambda *a, **k: None) + assert wd.sweep_once(client, "mt5-httpapi", now=1_000_000) == 1 + assert recorder.calls == [("mt5-fast", "mt5-httpapi")] + + +def test_a_vm_without_a_service_label_is_skipped_not_guessed(wd, tmp_path, recorder): + """Compose always sets the label; something hand-run may not. Skipping is + the honest outcome — inventing a service name would recreate the wrong + thing, or silently nothing.""" + wd.STATE_DIR = str(tmp_path) + wd.MIN_FAILING_STREAK = 1 + cid = "aaa" + client = _FakeClient( + [_container(cid, "mt5", labels={"com.docker.compose.project": "mt5-httpapi"})], + health={cid: _health("unhealthy", 99)}, + ) + with pytest.MonkeyPatch.context() as mp: + mp.setattr(wd, "log", lambda *a, **k: None) + assert wd.sweep_once(client, "mt5-httpapi", now=1_000_000) == 0 + assert recorder.calls == [] + + +# ── State survives the recreate it triggered ───────────────────────────────── + +def _same_service_as(cid, service="mt5"): + """One VM container for the compose service, under whatever id Docker + assigned this incarnation.""" + return _container(cid, "mt5", labels={ + "com.docker.compose.project": "mt5-httpapi", + "com.docker.compose.service": service, + }) + + +def test_the_attempt_cap_survives_the_recreate_it_triggered(wd, tmp_path, recorder, capsys): + """Recovery REPLACES the container, so the next poll sees a new id. + + State used to be keyed by container id: the recreate orphaned the record + that had just spent an attempt, the replacement loaded a fresh one, and a + persistently-broken VM was recovered forever at "attempt 1" - the cap and + backoff reset themselves on every recovery they were meant to bound. + """ + wd.STATE_DIR = str(tmp_path) + wd.MIN_FAILING_STREAK = 1 + wd.MAX_ATTEMPTS = 1 + wd.BACKOFF_ATTEMPTS = [999_999] + + with pytest.MonkeyPatch.context() as mp: + mp.setattr(wd, "log", lambda *a, **k: None) + old = _FakeClient([_same_service_as("old-container-id")], + health={"old-container-id": _health("unhealthy", 10)}) + assert wd.sweep_once(old, "mt5-httpapi", now=1_000_000) == 1 + + # The recreate happened: same compose service, replacement id, still + # unhealthy. The single-attempt budget is already spent. + new = _FakeClient([_same_service_as("new-container-id")], + health={"new-container-id": _health("unhealthy", 10)}) + assert wd.sweep_once(new, "mt5-httpapi", now=1_000_010) == 0 + + assert recorder.services == ["mt5"], "the replacement id refunded the attempt budget" + + +def test_backoff_survives_the_recreate_it_triggered(wd, tmp_path, recorder): + """Same replacement-id scenario, asserted on backoff rather than the cap: + inside the backoff window the replacement must wait, and once the window + passes it is recovered again - carrying the attempt count forward.""" + wd.STATE_DIR = str(tmp_path) + wd.MIN_FAILING_STREAK = 1 + wd.MAX_ATTEMPTS = 99 + wd.BACKOFF_ATTEMPTS = [300] + + def incarnation(cid): + return _FakeClient([_same_service_as(cid)], health={cid: _health("unhealthy", 10)}) + + now = 1_000_000 + with pytest.MonkeyPatch.context() as mp: + mp.setattr(wd, "log", lambda *a, **k: None) + assert wd.sweep_once(incarnation("id-1"), "mt5-httpapi", now=now) == 1 + # Replacement, inside the 300s window: waits. + assert wd.sweep_once(incarnation("id-2"), "mt5-httpapi", now=now + 200) == 0 + # Window passed: second attempt, on yet another id. + assert wd.sweep_once(incarnation("id-3"), "mt5-httpapi", now=now + 301) == 1 + + state = wd.load_state(wd.state_key("mt5-httpapi", "mt5")) + assert state["attempts"] == 2, "attempts must accumulate across container ids" + + +def test_state_key_is_stable_identity_not_container_id(wd): + assert wd.state_key("proj", "mt5") == wd.state_key("proj", "mt5") + assert wd.state_key("proj", "mt5") != wd.state_key("proj", "mt5-b") + # Labels are the only outside text that becomes a file name. With every + # separator replaced the key is a single path component, so it cannot + # traverse out of the state directory no matter what a label carries. + hostile = wd.state_key("pro/ject", "../../etc/passwd") + assert "/" not in hostile and "\\" not in hostile + + +def test_recreate_refuses_without_the_project_path(wd, monkeypatch): + """Compose resolves this project's relative bind mounts client-side, so + without the host path the recreate would rewrite every mount. Refuse loudly + rather than run a compose that quietly mounts the wrong things.""" + monkeypatch.setattr(wd, "PROJECT_DIR", "") + with pytest.raises(RuntimeError, match="WATCHDOG_PROJECT_DIR"): + wd.recreate_vm("mt5", "mt5-httpapi") + + +def test_recreate_hands_the_helper_the_project_name_and_the_project_dir(wd, monkeypatch, tmp_path): + """Two variables compose needs that the container was never handed. + + COMPOSE_PROJECT_NAME: compose derives the project from the directory name + otherwise, and a mismatch does not fail — it creates a SECOND set of + containers beside the running ones. + + MT5_PROJECT_DIR: the compose file interpolates `${MT5_PROJECT_DIR:?}` on + every compose command. This test used to assert only the project name, so + the helper failing at interpolation - psyb0t's 2026-09-04 blocker - was + invisible to it. The host variable is scrubbed first: the helper must get + it from the watchdog, not by inheritance from whoever ran the tests. + """ + seen = {} + + def fake_run(cmd, **kwargs): + seen["cmd"] = cmd + seen["cwd"] = kwargs.get("cwd") + seen["env"] = dict(kwargs.get("env") or {}) + + class R: + returncode = 0 + stdout = "" + stderr = "" + + return R() + + monkeypatch.delenv("MT5_PROJECT_DIR", raising=False) + monkeypatch.setattr(wd, "PROJECT_DIR", str(tmp_path)) + monkeypatch.setattr(wd, "RECREATE_SCRIPT", "/project/scripts/recreate-vm.sh") + monkeypatch.setattr(wd.subprocess, "run", fake_run) + + wd.recreate_vm("mt5", "mt5-httpapi") + + assert seen["cmd"] == ["/project/scripts/recreate-vm.sh", "mt5"] + assert seen["cwd"] == str(tmp_path) + assert seen["env"]["COMPOSE_PROJECT_NAME"] == "mt5-httpapi" + assert seen["env"]["MT5_PROJECT_DIR"] == str(tmp_path) + + +def test_recreate_env_is_the_watchdogs_own_project_dir_not_the_hosts(wd, monkeypatch, tmp_path): + """Even when the host shell HAS the variable, the helper gets the + watchdog's WATCHDOG_PROJECT_DIR: that is the path the project is mounted at + inside this container, which is what compose must resolve mounts against.""" + monkeypatch.setenv("MT5_PROJECT_DIR", "/somewhere/else/entirely") + monkeypatch.setattr(wd, "PROJECT_DIR", str(tmp_path)) + env = wd.recreate_env("mt5-httpapi") + assert env["MT5_PROJECT_DIR"] == str(tmp_path) + assert env["COMPOSE_PROJECT_NAME"] == "mt5-httpapi" + # Everything else is inherited: the helper needs PATH, HOME, DOCKER_HOST... + assert env["PATH"] == os.environ["PATH"] + + +def test_recreate_surfaces_the_scripts_failure(wd, monkeypatch, tmp_path): + def fake_run(cmd, **kwargs): + class R: + returncode = 1 + stdout = "" + stderr = "no such service: mt5" + + return R() + + monkeypatch.setattr(wd, "PROJECT_DIR", str(tmp_path)) + monkeypatch.setattr(wd.subprocess, "run", fake_run) + with pytest.raises(RuntimeError, match="no such service"): + wd.recreate_vm("mt5", "mt5-httpapi") + + +def test_missing_project_dir_is_reported_at_startup(monkeypatch): + """Finding out recovery cannot work only once a VM has crashed is the worst + possible time to learn it.""" + wd = _load_with(monkeypatch, WATCHDOG_PROJECT_DIR="") + assert any("WATCHDOG_PROJECT_DIR" in p for p in wd.validate_config()) + + +def test_missing_recreate_script_is_reported_at_startup(monkeypatch): + wd = _load_with(monkeypatch, WATCHDOG_RECREATE_SCRIPT="/nope/recreate-vm.sh") + assert any("WATCHDOG_RECREATE_SCRIPT" in p for p in wd.validate_config()) + + +# ── The REAL helper, under the watchdog's EXACT child environment ──────────── +# +# psyb0t (2026-09-04): the unit suite passed while every real recovery failed, +# because nothing here ever ran scripts/recreate-vm.sh with the environment the +# watchdog actually gives it. These do. The docker CLI is a stub - the offline +# image has no daemon - but the stub emulates the one compose behaviour that +# matters: `${VAR:?msg}` interpolation of the compose file fails BEFORE any +# container is touched when VAR is unset or empty. Everything else in the chain +# is real: the watchdog's env construction, the bash script, its PyYAML sidecar +# discovery, the compose file. + +_COMPOSE_REQUIRING_PROJECT_DIR = """\ +services: + mt5: + image: dockurr/windows:5.14 + wickworks: + image: psyb0t/wickworks + network_mode: "service:mt5" + vm-watchdog: + image: python:3.12-alpine + volumes: + - ${MT5_PROJECT_DIR:?MT5_PROJECT_DIR must be the absolute host path of this project}:${MT5_PROJECT_DIR}:ro +""" + +_STUB_DOCKER = r"""#!/bin/sh +# Stand-in for the docker CLI. Emulates compose's `${VAR:?msg}` interpolation: +# when the compose file names MT5_PROJECT_DIR as required and it is unset or +# empty in THIS process's environment, fail exactly as compose does, before +# doing anything. Otherwise record the call and succeed. +printf 'argv: %s\n' "$*" >>"$STUB_LOG" +env | grep -E '^(MT5_PROJECT_DIR|COMPOSE_PROJECT_NAME)=' >>"$STUB_LOG" || true +file="" +prev="" +for a in "$@"; do + [ "$prev" = "-f" ] && file="$a" + prev="$a" +done +if [ -n "$file" ] && grep -q 'MT5_PROJECT_DIR:?' "$file" && [ -z "${MT5_PROJECT_DIR:-}" ]; then + echo 'error while interpolating services.vm-watchdog.volumes.[]: required variable MT5_PROJECT_DIR is missing a value' >&2 + exit 1 +fi +exit 0 +""" + + +def _project_with_stub_docker(tmp_path): + """A compose project holding the REAL helper, plus a stub docker on PATH.""" + project = tmp_path / "project" + (project / "scripts").mkdir(parents=True) + helper = project / "scripts" / "recreate-vm.sh" + shutil.copy(_REPO / "scripts" / "recreate-vm.sh", helper) + helper.chmod(0o755) + (project / "docker-compose.yml").write_text(_COMPOSE_REQUIRING_PROJECT_DIR, encoding="utf-8") + + bindir = tmp_path / "bin" + bindir.mkdir() + (bindir / "docker").write_text(_STUB_DOCKER, encoding="utf-8") + (bindir / "docker").chmod(0o755) + return project, helper, bindir, tmp_path / "stub.log" + + +def test_the_real_helper_recreates_under_the_watchdogs_exact_child_environment(wd, monkeypatch, tmp_path): + """The blocker, end to end: recreate_vm() -> real recreate-vm.sh -> docker + compose, with MT5_PROJECT_DIR scrubbed from the host so the ONLY way the + helper can have it is the watchdog putting it there.""" + project, helper, bindir, log = _project_with_stub_docker(tmp_path) + monkeypatch.delenv("MT5_PROJECT_DIR", raising=False) + monkeypatch.setenv("PATH", f"{bindir}{os.pathsep}{os.environ['PATH']}") + monkeypatch.setenv("STUB_LOG", str(log)) + monkeypatch.setattr(wd, "PROJECT_DIR", str(project)) + monkeypatch.setattr(wd, "RECREATE_SCRIPT", str(helper)) + + wd.recreate_vm("mt5", "mt5-httpapi") # raises RuntimeError on any failure + + recorded = log.read_text(encoding="utf-8") + assert f"MT5_PROJECT_DIR={project}" in recorded + assert "COMPOSE_PROJECT_NAME=mt5-httpapi" in recorded + # The real script planned the real operation, with the sidecar it found + # in the compose file - not some path the stub short-circuited. + assert "compose" in recorded and "stop" in recorded + assert "--force-recreate" in recorded + assert "wickworks" in recorded + + +def test_without_the_injected_variable_the_same_helper_fails_at_interpolation(tmp_path, monkeypatch): + """Guards the test above. Run the identical helper WITHOUT MT5_PROJECT_DIR + and it fails with compose's own error, before stopping anything - so a + regression to the old child environment turns the suite red rather than + quietly green.""" + project, helper, bindir, log = _project_with_stub_docker(tmp_path) + env = { + "PATH": f"{bindir}{os.pathsep}{os.environ['PATH']}", + "STUB_LOG": str(log), + "COMPOSE_PROJECT_NAME": "mt5-httpapi", + # No MT5_PROJECT_DIR - the pre-fix child environment. + } + res = subprocess.run( + [str(helper), "mt5"], cwd=str(project), env=env, + capture_output=True, text=True, timeout=60, check=False, + ) + assert res.returncode != 0 + assert "required variable MT5_PROJECT_DIR is missing a value" in res.stderr + assert "--force-recreate" not in log.read_text(encoding="utf-8"), "recreate ran despite the failed stop" + + +# ── Scope: exact image repository, and never itself ────────────────────────── +# +# psyb0t (2026-08-21), still open at c29b289: `startswith(IMAGE_FILTER)` also +# matched `dockurr/windows-not-the-vm`, and nothing excluded SELF_ID, so a valid +# operator override like WATCHDOG_IMAGE_FILTER=python selected the watchdog. + + +@pytest.mark.parametrize( + "image,selected", + [ + ("dockurr/windows", True), + ("dockurr/windows:5.14", True), + ("dockurr/windows@sha256:" + "ab" * 32, True), + ("dockurr/windows-not-the-vm:latest", False), + ("dockurr/windows2:1", False), + ("psyb0t/wickworks:v0.3.1", False), + ("notdockurr/windows:5.14", False), + ], +) +def test_image_filter_matches_the_repository_exactly(wd, tmp_path, recorder, image, selected): + wd.STATE_DIR = str(tmp_path) + wd.MIN_FAILING_STREAK = 1 + wd.IMAGE_FILTER = "dockurr/windows" + client = _FakeClient([_container("c1", "vm", image=image)], health={"c1": _health("unhealthy", 99)}) + with pytest.MonkeyPatch.context() as mp: + mp.setattr(wd, "log", lambda *a, **k: None) + wd.sweep_once(client, "mt5-httpapi", now=1_000_000) + assert recorder.services == (["c1"] if selected else []), image + + +def test_the_watchdog_never_selects_itself_whatever_the_filter_says(wd, tmp_path, recorder): + """Docker sets the hostname to the SHORT container id; the list endpoint + reports the full one. Self-exclusion is unconditional and first, before + the image filter gets a say.""" + wd.STATE_DIR = str(tmp_path) + wd.MIN_FAILING_STREAK = 1 + wd.IMAGE_FILTER = "python" + wd.SELF_ID = "abcdef123456" + me = _container("abcdef123456" + "0" * 52, "vm-watchdog", image="python:3.12-alpine") + vm = _container("vm1", "fake-vm", image="python:3.12-alpine") + client = _FakeClient( + [me, vm], + health={me["Id"]: _health("unhealthy", 99), "vm1": _health("unhealthy", 99)}, + ) + with pytest.MonkeyPatch.context() as mp: + mp.setattr(wd, "log", lambda *a, **k: None) + assert wd.sweep_once(client, "mt5-httpapi", now=1_000_000) == 1 + assert recorder.services == ["vm1"] + + +def test_an_empty_self_id_excludes_nothing_by_accident(wd): + """"".startswith("") is True for every id: an unset SELF_ID must not make + every container look like the watchdog and silently disable it.""" + wd.SELF_ID = "" + wd.SELF_FULL_ID = "" + assert wd._is_self("anything") is False + + +def test_self_exclusion_uses_the_resolved_full_id_when_the_hostname_is_a_name(wd, tmp_path, recorder): + """`hostname: watchdog` on the service (or WATCHDOG_SELF_ID set to a name) + used to defeat self-exclusion entirely, because the prefix test compared + container ids against a word. With the full id resolved at startup the + test is equality, and the hostname no longer matters.""" + wd.STATE_DIR = str(tmp_path) + wd.MIN_FAILING_STREAK = 1 + wd.IMAGE_FILTER = "python" + wd.SELF_ID = "watchdog" + full = "f" * 64 + wd.SELF_FULL_ID = full + me = _container(full, "vm-watchdog", image="python:3.12-alpine") + vm = _container("vm1", "fake-vm", image="python:3.12-alpine") + client = _FakeClient([me, vm], health={full: _health("unhealthy", 99), "vm1": _health("unhealthy", 99)}) + with pytest.MonkeyPatch.context() as mp: + mp.setattr(wd, "log", lambda *a, **k: None) + assert wd.sweep_once(client, "mt5-httpapi", now=1_000_000) == 1 + assert recorder.services == ["vm1"] + + +def test_a_name_hostname_without_a_resolved_id_never_prefix_matches_other_containers(wd): + """The fallback is only the hex short-id prefix. A word must not exclude a + container whose id merely starts with the same letters.""" + wd.SELF_FULL_ID = "" + wd.SELF_ID = "abc" # too short / not a container id + assert wd._is_self("abc" + "0" * 61) is False + wd.SELF_ID = "watchdog" + assert wd._is_self("watchdog-lookalike") is False + wd.SELF_ID = "abcdef123456" # a real short id still works by prefix + assert wd._is_self("abcdef123456" + "0" * 52) is True + + +def test_main_resolves_its_own_full_id_before_the_first_sweep(monkeypatch): + """The resolution is what makes the equality test possible in production.""" + wd = _load_with(monkeypatch, WATCHDOG_COMPOSE_PROJECT="mt5-httpapi") + wd.SELF_ID = "self-watchdog-id" + + class Client(_FakeClient): + def inspect(self, cid): + if cid == "self-watchdog-id": + return {"Id": "e" * 64, "Config": {"Labels": {"com.docker.compose.project": "mt5-httpapi"}}} + return super().inspect(cid) + + monkeypatch.setattr(wd, "DockerClient", lambda *_a, **_k: Client([])) + # Stop main() at its first sleep, after startup work is done. + monkeypatch.setattr(wd.time, "sleep", lambda *_a: (_ for _ in ()).throw(KeyboardInterrupt())) + monkeypatch.setattr(wd, "log", lambda *a, **k: None) + with pytest.raises(KeyboardInterrupt): + wd.main() + assert wd.SELF_FULL_ID == "e" * 64 + + +# ── Netns sidecars ─────────────────────────────────────────────────────────── +# +# 2026-09-07, in production: mt5's container exited cleanly, `unless-stopped` +# restarted it, and its wickworks sidecar spent two days in the namespace that +# restart destroyed - 13,700 failed healthchecks, every /rates/ta call 502-ing, +# while the VM reported healthy the whole time. A restart KEEPS the container +# id, so nothing about the binding looked wrong from the outside; only the +# sidecar's own healthcheck saw it, and nothing was watching that. + + +def _sidecar(cid, name, owner, service=None): + """A container bound to another container's network namespace. + + `HostConfig.NetworkMode` carries the OWNER as compose resolved it, which + for `network_mode: service:` is always a full id. + """ + return { + "Id": cid, + "Names": [f"/{name}"], + "Image": "wickworks:latest", + "HostConfig": {"NetworkMode": f"container:{owner}"}, + "Labels": { + "com.docker.compose.project": "mt5-httpapi", + "com.docker.compose.service": service or name, + }, + } + + +def _vm_container(cid="vm-id", service="mt5"): + return _container(cid, service, labels={ + "com.docker.compose.project": "mt5-httpapi", + "com.docker.compose.service": service, + }) + + +def _pair(vm_status, vm_streak, side_status, side_streak, owner=None, extra=None): + vm = _vm_container() + containers = [vm, _sidecar("side-id", "wickworks", owner or vm["Id"])] + (extra or []) + return _FakeClient(containers, health={ + "vm-id": _health(vm_status, vm_streak), + "side-id": _health(side_status, side_streak), + }) + + +def _sweep(wd, client, now=1_000_000, **kw): + with pytest.MonkeyPatch.context() as mp: + mp.setattr(wd, "log", kw.pop("log", lambda *a, **k: None)) + return wd.sweep_once(client, "mt5-httpapi", now=now, **kw) + + +def test_an_unhealthy_sidecar_under_a_healthy_owner_is_recreated_alone(wd, tmp_path, recorder): + """The 2026-09-07 recovery. Only the sidecar is named, so the VM and the + twelve terminals inside it are not disturbed.""" + wd.STATE_DIR = str(tmp_path) + assert _sweep(wd, _pair("healthy", 0, "unhealthy", 13700)) == 1 + assert recorder.services == ["wickworks"] + + +def test_the_streak_gate_applies_to_a_sidecar_too(wd, tmp_path, recorder): + """One bad poll is not an outage, here as anywhere else.""" + wd.STATE_DIR = str(tmp_path) + assert _sweep(wd, _pair("healthy", 0, "unhealthy", 3)) == 0 + assert recorder.services == [] + assert _sweep(wd, _pair("healthy", 0, "unhealthy", 10), now=1_000_001) == 1 + assert recorder.services == ["wickworks"] + + +def test_a_healthy_sidecar_is_never_touched(wd, tmp_path, recorder): + wd.STATE_DIR = str(tmp_path) + assert _sweep(wd, _pair("healthy", 0, "healthy", 0)) == 0 + assert recorder.services == [] + + +def test_a_sidecar_whose_healthcheck_cannot_see_the_orphaning_is_not_recovered(wd, tmp_path, recorder): + """Documented limitation, pinned so it cannot be mistaken for a bug later. + + A netns sidecar whose check only probes loopback stays green inside a dead + namespace, and health is this daemon's only source of truth. The fix is an + orphan-aware check on the sidecar (see scripts/wickworks-healthcheck.py), + not a structural guess here: over `/containers/json`, which lists running + containers only, a stopped owner and a destroyed one are the same thing. + """ + wd.STATE_DIR = str(tmp_path) + assert _sweep(wd, _pair("healthy", 0, "healthy", 0)) == 0 + assert recorder.services == [] + + +def test_an_unhealthy_owner_below_the_streak_still_keeps_the_sweep_out(wd, tmp_path, recorder): + """The guard that matters, isolated. + + Counter-review finding: the earlier test set the owner to a streak past the + threshold, so the VM sweep recreated it in the same tick and the sidecar + was skipped by the ALREADY-RECREATED check - deleting the owner-health + guard changed nothing. Here the owner is unhealthy BELOW the threshold, so + the VM sweep decides `wait` and this guard is the only thing standing + between a sick VM and a sidecar recreate that would race its recovery. + """ + wd.STATE_DIR = str(tmp_path) + assert _sweep(wd, _pair("unhealthy", 2, "unhealthy", 99)) == 0 + assert recorder.services == [] + + +def test_a_starting_owner_keeps_the_sweep_out(wd, tmp_path, recorder): + """A VM that is still booting has sidecars that cannot be healthy yet.""" + wd.STATE_DIR = str(tmp_path) + assert _sweep(wd, _pair("starting", 0, "unhealthy", 99)) == 0 + assert recorder.services == [] + + +def test_an_owner_recreated_this_tick_is_not_followed_by_a_sidecar_recreate(wd, tmp_path, recorder): + """The VM sweep recreates owner AND sidecars together, so a second recreate + would stop a container that was just rebuilt - and charge it an attempt.""" + wd.STATE_DIR = str(tmp_path) + assert _sweep(wd, _pair("unhealthy", 99, "unhealthy", 99)) == 1 + assert recorder.services == ["mt5"] + + +def test_a_sidecar_under_a_stopped_owner_is_left_alone(wd, tmp_path, recorder): + """`docker compose stop mt5` for maintenance must not cost the sidecar. + + Counter-review finding, and the sharpest one: `/containers/json` lists + RUNNING containers only, so a stopped owner and a destroyed one are + indistinguishable. Acting on "not running" would have run the helper, which + stops the sidecar first and then cannot start it again (nothing to join), + leaving it stopped - and a stopped container is invisible to both sweeps, + so it would never be retried. + """ + wd.STATE_DIR = str(tmp_path) + stopped_owner_gone = _FakeClient( + [_sidecar("side-id", "wickworks", "an-owner-that-is-not-running")], + health={"side-id": _health("unhealthy", 99)}, + ) + assert _sweep(wd, stopped_owner_gone) == 0 + assert recorder.services == [] + assert not list(tmp_path.glob("*.json")), "no budget may be spent either" + + +def test_a_sidecar_of_something_that_is_not_our_vm_is_not_touched(wd, tmp_path, recorder): + """Its owner is up; it is simply not a VM this watchdog manages.""" + wd.STATE_DIR = str(tmp_path) + client = _FakeClient( + [ + _container("db-id", "postgres", image="postgres:16", labels={ + "com.docker.compose.project": "mt5-httpapi", + "com.docker.compose.service": "postgres", + }), + _sidecar("side-id", "exporter", "db-id"), + ], + health={"db-id": _health("healthy", 0), "side-id": _health("unhealthy", 99)}, + ) + assert _sweep(wd, client) == 0 + assert recorder.services == [] + + +def test_a_sidecar_chained_to_another_sidecar_is_not_touched(wd, tmp_path, recorder): + """`network_mode: service:` is legal, and its owner is not a VM.""" + wd.STATE_DIR = str(tmp_path) + client = _pair("healthy", 0, "healthy", 0, extra=[_sidecar("chain-id", "sniffer", "side-id")]) + client.health["chain-id"] = _health("unhealthy", 99) + assert _sweep(wd, client) == 0 + assert recorder.services == [] + + +def test_a_sidecar_in_another_project_is_not_touched(wd, tmp_path, recorder): + wd.STATE_DIR = str(tmp_path) + foreign = _sidecar("side-id", "wickworks", "vm-id") + foreign["Labels"]["com.docker.compose.project"] = "someone-else" + client = _FakeClient([_vm_container(), foreign], health={ + "vm-id": _health("healthy", 0), "side-id": _health("unhealthy", 99)}) + assert _sweep(wd, client) == 0 + assert recorder.services == [] + + +def test_the_watchdog_never_selects_itself_in_the_sidecar_sweep(wd, tmp_path, recorder): + """The VM sweep excludes self by id; so must this one. A watchdog with + `network_mode` set that recreates itself mid-sweep is not a recovery path. + + The id here is deliberately NOT the fake client's magic self-inspect id: + with that one the health lookup is short-circuited, the sweep sees + `health=none`, and the test passes whether the exclusion exists or not. + """ + wd.STATE_DIR = str(tmp_path) + wd.SELF_FULL_ID = "the-watchdogs-own-id" + me = _sidecar("the-watchdogs-own-id", "vm-watchdog", "vm-id", service="vm-watchdog") + client = _FakeClient([_vm_container(), me], health={ + "vm-id": _health("healthy", 0), "the-watchdogs-own-id": _health("unhealthy", 99)}) + assert _sweep(wd, client) == 0 + assert recorder.services == [] + + +def test_a_sidecar_with_no_compose_service_label_is_reported_not_recreated(wd, tmp_path, recorder): + """A recreate names a compose service. Without one there is nothing to + name, and guessing would be worse than saying so.""" + wd.STATE_DIR = str(tmp_path) + unlabelled = _sidecar("side-id", "wickworks", "vm-id") + del unlabelled["Labels"]["com.docker.compose.service"] + client = _FakeClient([_vm_container(), unlabelled], health={ + "vm-id": _health("healthy", 0), "side-id": _health("unhealthy", 99)}) + logs = [] + assert _sweep(wd, client, log=logs.append) == 0 + assert recorder.services == [] + assert any("no compose service label" in line for line in logs), logs + + +def test_a_network_mode_with_no_owner_is_ignored(wd, tmp_path, recorder): + """`container:` with nothing after it names nobody.""" + wd.STATE_DIR = str(tmp_path) + client = _FakeClient([_vm_container(), _sidecar("side-id", "wickworks", "")], health={ + "vm-id": _health("healthy", 0), "side-id": _health("unhealthy", 99)}) + assert _sweep(wd, client) == 0 + assert recorder.services == [] + + +def test_a_container_with_no_host_config_is_ignored(wd, tmp_path, recorder): + """Docker's container list is not a contract this daemon controls, and a + KeyError here would kill the sweep and with it every VM's recovery.""" + wd.STATE_DIR = str(tmp_path) + bare = {"Id": "bare-id", "Names": ["/odd"], "Image": "whatever:1", + "Labels": {"com.docker.compose.project": "mt5-httpapi", + "com.docker.compose.service": "odd"}} + client = _FakeClient([_vm_container(), bare], health={ + "vm-id": _health("healthy", 0), "bare-id": _health("unhealthy", 99)}) + assert _sweep(wd, client) == 0 + assert recorder.services == [] + + +# A real container id, and the 12-character short form Docker prints for it. +VM_LONG_ID = "8e796ef18a62c4f8650fc3f5c9407a3f198f5f2e3b771a5b5a3ab3fbe04e7bdb" +VM_SHORT_ID = VM_LONG_ID[:12] + + +@pytest.mark.parametrize("token", [VM_LONG_ID, VM_SHORT_ID, "mt5"]) +def test_a_hand_written_owner_reference_is_still_recognised(wd, tmp_path, recorder, token): + """`container:` and `container:` are legal to write by hand + and are not normalised anywhere. Matching full ids only would leave such a + sidecar unwatched while the docs promised otherwise.""" + wd.STATE_DIR = str(tmp_path) + vm = _vm_container(cid=VM_LONG_ID) + client = _FakeClient([vm, _sidecar("side-id", "wickworks", token)], health={ + VM_LONG_ID: _health("healthy", 0), "side-id": _health("unhealthy", 99)}) + assert _sweep(wd, client) == 1 + assert recorder.services == ["wickworks"] + + +def test_a_prefix_shorter_than_a_short_id_is_not_a_match(wd, tmp_path, recorder): + """Prefix matching is bounded at Docker's own 12-character short id. `v` + must not select a VM, or a stray token would rebind an unrelated sidecar.""" + wd.STATE_DIR = str(tmp_path) + client = _FakeClient( + [_vm_container(cid=VM_LONG_ID), _sidecar("side-id", "wickworks", VM_LONG_ID[:4])], + health={VM_LONG_ID: _health("healthy", 0), "side-id": _health("unhealthy", 99)}) + assert _sweep(wd, client) == 0 + assert recorder.services == [] + + +def test_the_sidecar_sweep_can_be_switched_off(wd, tmp_path, recorder): + wd.STATE_DIR = str(tmp_path) + wd.WATCH_SIDECARS = False + assert _sweep(wd, _pair("healthy", 0, "unhealthy", 99)) == 0 + assert recorder.services == [] + + +def test_a_dry_run_recreates_no_sidecar_and_spends_no_budget(wd, tmp_path, recorder): + wd.STATE_DIR = str(tmp_path) + _sweep(wd, _pair("healthy", 0, "unhealthy", 99), dry_run=True) + assert recorder.services == [] + assert not list(tmp_path.glob("*.json")) + + +def test_the_sidecar_attempt_cap_is_the_same_one_the_vms_get(wd, tmp_path, recorder): + """A sidecar that cannot be repaired must stop being retried, like any + other container - the shared decide() is what guarantees it.""" + wd.STATE_DIR = str(tmp_path) + logs = [] + for step in range(6): + # Far enough apart that backoff never masks the cap. + _sweep(wd, _pair("healthy", 0, "unhealthy", 99), + now=1_000_000 + step * 100_000, log=logs.append) + assert len(recorder.services) == wd.MAX_ATTEMPTS + assert any("GIVING UP on sidecar" in line for line in logs), logs + + +def test_a_sidecar_and_its_vm_keep_separate_budgets(wd, tmp_path, recorder): + """State is keyed by compose service, so a sidecar's attempts can never + exhaust the VM's budget or the other way round.""" + wd.STATE_DIR = str(tmp_path) + _sweep(wd, _pair("healthy", 0, "unhealthy", 99)) + written = sorted(f.name for f in tmp_path.glob("*.json")) + assert written == ["mt5-httpapi.mt5.json", "mt5-httpapi.wickworks.json"], written + import json as _json + vm_state = _json.loads((tmp_path / "mt5-httpapi.mt5.json").read_text()) + side_state = _json.loads((tmp_path / "mt5-httpapi.wickworks.json").read_text()) + assert vm_state["attempts"] == 0, "the healthy VM must not be charged" + assert side_state["attempts"] == 1 + + +def test_every_sidecar_of_one_owner_is_considered(wd, tmp_path, recorder): + """A VM may carry more than one netns sidecar, and they fail independently.""" + wd.STATE_DIR = str(tmp_path) + client = _pair("healthy", 0, "unhealthy", 99, + extra=[_sidecar("side-b", "tailscale", "vm-id")]) + client.health["side-b"] = _health("unhealthy", 99) + assert _sweep(wd, client) == 2 + assert sorted(recorder.services) == ["tailscale", "wickworks"] + + +# ── _env_bool ──────────────────────────────────────────────────────────────── + + +@pytest.mark.parametrize("raw,expected", [ + ("1", True), ("true", True), ("TRUE", True), ("yes", True), ("on", True), (" 1 ", True), + ("0", False), ("false", False), ("FALSE", False), ("no", False), ("off", False), +]) +def test_env_bool_accepts_the_spellings_people_actually_write(monkeypatch, raw, expected): + monkeypatch.setenv("WATCHDOG_WATCH_SIDECARS", raw) + module = _load() + assert module.WATCH_SIDECARS is expected + assert not [e for e in module.CONFIG_ERRORS if "WATCH_SIDECARS" in e] + + +@pytest.mark.parametrize("raw", ["banana", "", "2", "no thanks"]) +def test_env_bool_refuses_to_guess(monkeypatch, raw): + """Unrecognised is a recorded error, so validate_config() refuses to start. + + NOT silently false: quietly disabling a recovery path because someone typed + it wrong is the failure mode this whole file is written against. Same call + _env_int already makes for its own junk. + """ + monkeypatch.setenv("WATCHDOG_WATCH_SIDECARS", raw) + module = _load() + assert [e for e in module.CONFIG_ERRORS if "WATCHDOG_WATCH_SIDECARS" in e] + assert any("WATCHDOG_WATCH_SIDECARS" in p for p in module.validate_config())