Emil SimeonovandDevin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
8cd0d4dd57
CI / validate (pull_request) Successful in 1m9s
CI / molecule-tests (1) (pull_request) Successful in 5m23s
CI / molecule-tests (3) (pull_request) Successful in 7m5s
CI / molecule-tests (2) (pull_request) Successful in 7m27s
CI / molecule-tests (4) (pull_request) Successful in 7m31s
CI / auto-merge (pull_request) Successful in 7m50s
docker inspect captures the container Env block, which for CI job containers carries CI_GITEA_API_TOKEN and similar secrets — the diag bundle must not become a secret artifact (OBL-INFRA-548 S02). Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
74 lines
3.0 KiB
Markdown
74 lines
3.0 KiB
Markdown
# 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:
|
|
|
|
1. **Missing `|| true` on the container enumeration.** `timeout 15
|
|
docker ps … | while …` runs under `set -euo pipefail`. If the daemon
|
|
is unresponsive — precisely the condition the section exists to
|
|
diagnose — `docker ps` exits 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.
|
|
|
|
2. **CreatedAt split bug.** `docker ps --format '{{.ID}} {{.Names}}
|
|
{{.CreatedAt}}'` emits a timestamp containing spaces
|
|
(`2026-09-18 10:30:00 +0000 UTC`), but `read -r cid cname ccreated
|
|
_rest` only captures `2026-09-18` — the date part. `date -d` then
|
|
computes age from midnight: containers created today always appear
|
|
≥N hours old, so the 15-minute gate effectively never filters.
|
|
|
|
3. **`head -200` truncates `docker inspect`.** Inspect output is ~300+
|
|
lines and the `State` block (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 — but pipe through a `sed` filter that
|
|
redacts the value of any env entry whose name contains TOKEN, PASSWORD,
|
|
SECRET, or KEY. Job containers carry CI tokens in their Env block; the
|
|
diagnostics bundle must not become a secret-material artifact
|
|
(OBL-INFRA-548 S02).
|
|
|
|
REQ-4: Diagnostics-only constraint unchanged — no kills, no restarts.
|
|
|
|
## Test Plan
|
|
|
|
- Render the template and run `bash -n` on the output.
|
|
- Shell-simulate: feed a fake `docker ps` line with spaced CreatedAt and
|
|
verify `date -d` computes 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
|
|
|
|
- [x] `docker ps` enumeration guarded against nonzero exit.
|
|
- [x] Full CreatedAt timestamp parsed via `|` separator.
|
|
- [x] `docker inspect` captured without truncation.
|
|
- [x] Rendered script passes `bash -n`.
|
|
- [x] `make lint-all` passes.
|