Emil SimeonovandDevin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
a61075ebde
Post-merge review of GRM-168 found three defects: the docker ps feeding the probe loop lacked a failure guard, so a slow daemon would abort the healthcheck under pipefail exactly when diagnostics matter; CreatedAt contains spaces so whitespace-splitting read only captured the date and broke the 15-minute age gate (GNU date also rejects the +0000 UTC suffix); and head -200 could truncate the inspect State block the spec requires. Pipe-separated fields, guarded enumeration, stripped UTC suffix, and full inspect output fix all three. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
2.8 KiB
GRM-170: Fix stall-detection robustness bugs in runner healthcheck
Problem
Post-merge review of the GRM-168 stall-detection block in
runner-healthcheck.sh.j2 found three defects:
-
Missing
|| trueon the container enumeration.timeout 15 docker ps … | while …runs underset -euo pipefail. If the daemon is unresponsive — precisely the condition the section exists to diagnose —docker psexits nonzero, pipefail propagates it, and the healthcheck dies mid-run before reaching the runner-service check. Every other docker call in the script is guarded; this one is not. -
CreatedAt split bug.
docker ps --format '{{.ID}} {{.Names}} {{.CreatedAt}}'emits a timestamp containing spaces (2026-09-18 10:30:00 +0000 UTC), butread -r cid cname ccreated _restonly captures2026-09-18— the date part.date -dthen computes age from midnight: containers created today always appear ≥N hours old, so the 15-minute gate effectively never filters. -
head -200truncatesdocker inspect. Inspect output is ~300+ lines and theStateblock (OOMKilled, Pid, times) the spec requires can be cut off.
Approach
REQ-1: Wrap the enumeration so a failed docker ps yields empty input
instead of aborting the script: { timeout 15 docker ps … || true; } | while ….
REQ-2: Emit fields separated by | ({{.ID}}|{{.Names}}|{{.CreatedAt}})
and parse with IFS='|' read -r cid cname ccreated _rest so the full
timestamp reaches date -d; also strip the redundant UTC suffix
because GNU date rejects +0000 UTC together. The age gate then
compares real minutes.
REQ-3: Remove the head -200 truncation on docker inspect output so
the full State block is captured in the diagnostics bundle.
REQ-4: Diagnostics-only constraint unchanged — no kills, no restarts.
Test Plan
- Render the template and run
bash -non the output. - Shell-simulate: feed a fake
docker psline with spaced CreatedAt and verifydate -dcomputes minutes correctly (manual check). make lint-all(ansible-lint, actionlint, ruff) passes.- Molecule gitea_runner scenario converges with the template change.
Deploy Plan
Merge via auto-merge → release (fix: commit bumps patch) → infra dependency-bump PR picks up the new role version → runner role applied on next infra run. This PR also carries the merged-but-unreleased GRM-168 healthcheck into the release.
Rollback Plan
Revert the three-line change set; the section degrades to the GRM-168 behavior (still diagnostics-only, just less robust).
Acceptance Criteria
docker psenumeration guarded against nonzero exit.- Full CreatedAt timestamp parsed via
|separator. docker inspectcaptured without truncation.- Rendered script passes
bash -n. make lint-allpasses.