Plan 75 §G1, second migration. docker_service was the heaviest file in the
census: 49 raw subprocess call sites -> 2. Census 258 -> 211.
DockerService.run now SITS ON run_checked instead of re-implementing it, so
G3's runner and G1's door are one thing rather than two. Adds
DockerService.run_compose() for the compose subcommands, whose argv head is
whatever _get_compose_cmd() resolved (`docker compose` v2 vs `docker-compose`
v1) and therefore cannot go through run().
The 2 survivors are the genuine streaming Popen sites — the live container
log stream and the compose-up transcript. A captured call cannot do those.
run_checked gained two things this migration needed:
- `stderr` (the raw stream) as a separate key from `error` (the verdict, None
on success). git and `docker exec` write to stderr and still succeed, so
folding the stream into the verdict would invent failures. exec_command
needs the stream; every other caller wants the verdict.
- merge_stderr=True, for `docker logs` and `compose logs`. A container writes
its log to both streams; the `result.stdout + result.stderr` concatenation
this replaces loses the interleaving, so a crash tail landed above the lines
that led to it.
TIMEOUTS ARE UNCHANGED, deliberately. run_checked defaults to 60s and every
docker call in this file is unbounded today; inheriting the default would cap
`compose up --build`, `docker pull` and `docker build` at a minute and turn
working deploys into timeouts. Every migrated site keeps exactly what it has,
with timeout=None passed explicitly at the long ones and six tests pinning it
(mutation-checked: removing it from pull_image fails the test). Bounding
docker's long operations is a real improvement and a deliberate behaviour
change — its own commit, not a side effect of this one.
Two regressions caught by the suite and fixed here:
- Dropping the try/except around the compose-version probe let an exception
escape into the caller, surfacing as `compose up` exit_code -1 instead of a
v1 fallback. run_checked converts the exec failures it knows about, but this
probe runs before everything else in the compose path and must not be able
to raise at all. Guard restored, with a test.
- test_docker_protection hand-rolled a CompletedProcess stand-in with only
returncode and stdout. Once list_containers went through run_checked,
reading .stderr off it raised AttributeError, the caller's blanket except
swallowed it, and the test saw an EMPTY CONTAINER LIST rather than an error.
Retargeted onto the §G7 kit. A private stub is only correct until something
reads the attribute it forgot.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Plan 75 §G3. Three services kept private `_docker()` wrappers around the
same `subprocess.run(['docker', ...])` call, and the three had drifted
into three different return contracts:
- db_admin_sso_service returned a dict and mapped FileNotFoundError to
"Docker not found";
- db_config_tuner_service returned a dict that collapsed every failure
into str(e), so a caller could not tell "install docker" from "the
daemon is wedged";
- image_update_service returned a raw CompletedProcess, so a caller
reading result['success'] against it got a TypeError, not an answer.
DockerService.run(args, timeout=60) is now that call, with one shape —
{'success', 'output', 'error'} — that keeps the three distinctions §A
cares about apart: docker said no, docker is absent, docker did not answer
in time. DockerService.available() asks the daemon rather than the binary,
because "CLI installed, daemon down" is the state the callers check for.
The three named seams stay (each service's tests stub them); only the
bodies go. image_update_service's two digest probes move to the dict
shape, and drop the blanket try/except that was there to absorb the
CompletedProcess path's exceptions.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
