fix(web): stop dry-runs reporting a phantom bitborg-web restart #295

Sammanfogat
supernaut sammanfogade 1 incheckning från fix/294-checkmode-phantom-web-restart in i main 2026-07-31 22:11:24 +00:00
Ägare

Closes #294. Two lines of check_mode: false, plus the comments explaining why.

How this surfaced

Gating the docs-only #293 through the apply procedure. The dry-run came back changed=1, which
under this repo's own rule — account for every changed task, because that is how a
merged-but-unapplied PR gets caught — had to be explained before the gate could be cleared.

It was not drift. It is a permanent artefact of the dry-run itself.

Mechanism

The restart decision is computed from two command probes. --check does not run commands, and
crucially it does not register "no data" in their place — it registers plausible success:

{ "rc": 0, "stdout": "", "skipped": true,
  "msg": "Command would have run if not in check mode" }

Confirmed empirically with a throwaway probe playbook against the host rather than inferred from
the docs, because the two fabricated fields break the role in opposite directions:

  • stdout: "" can never equal git.gitborg.se/gitborg/gitborg-web:latest, so
    (web_running_image.stdout … ) != (web_image ~ ':' ~ web_image_tag) is always true and the
    expression resolves to state: restarted. Hence a changed on every dry-run that a real run
    does not perform.
  • rc: 0 means every when: web_image_present.rc == 0 guard takes the success branch
    regardless. So a dry-run silently assumes the image is published even when it is not — which is
    exactly the condition "Note when the bitborg-web image is not yet published" exists to report.
    That one is a dry-run that cannot warn you, the quieter half of the same bug.

Worth recording because the obvious reading is wrong: the existing | default('') on stdout is
not what rescues or breaks this. The value is registered as "" outright, so the default never
fires. Anyone reasoning from the guard alone would conclude the task is already protected.

Why these two tasks and not the third

Both probes are strictly read-only — podman image exists and podman container inspect — and
already carry changed_when: false and failed_when: false. Running them under check mode
observes the host without touching it, which is precisely what a dry-run is for.

systemctl --user reset-failed sits in the same block and is deliberately left skipped. It
clears latched systemd state; that is a mutation, and a dry-run must not perform it. The point is
not "make check mode run more tasks", it is "let check mode read, never write".

Verification

Scoped --check --diff --limit gitborg-prod --tags web, before → after:

Task Before After
Check the bitborg-web image is present skipping ok (really running)
Inspect the running bitborg-web image skipping ok (really running)
Enable and start bitborg-web changed ok
PLAY RECAP changed=1 changed=0

Independently corroborated before changing anything: the running container was already on
git.gitborg.se/gitborg/gitborg-web:latest, matching web_image:web_image_tag — so no restart was
ever pending and changed=0 is the truthful answer, not a suppressed one. This is the distinction
that mattered: the goal was to make the dry-run honest, not to silence it.

ansible-lint roles/web/ passes on the production profile; site.yml --syntax-check clean.

Note on applying

No apply is needed for this PR. It changes only what --check reports, and the host is already
converged — the full dry-run that started this was ok=237 changed=1 failed=0, whose single
changed is the artefact being removed here.

Follow-up

Left in #294: other roles were checked and none share the drift-comparison shape, so nothing else
reports a phantom change today. The general trap is broader though — any when: or state
expression derived from a command register is reading fabricated values under --check — so a
sweep for read-only probes that want check_mode: false, plus a line in the apply procedure, would
stop the next person re-deriving this from scratch.

Closes #294. Two lines of `check_mode: false`, plus the comments explaining why. ## How this surfaced Gating the docs-only #293 through the apply procedure. The dry-run came back `changed=1`, which under this repo's own rule — account for **every** `changed` task, because that is how a merged-but-unapplied PR gets caught — had to be explained before the gate could be cleared. It was not drift. It is a permanent artefact of the dry-run itself. ## Mechanism The restart decision is computed from two `command` probes. `--check` does not run commands, and crucially it does not register "no data" in their place — it registers **plausible success**: ```json { "rc": 0, "stdout": "", "skipped": true, "msg": "Command would have run if not in check mode" } ``` Confirmed empirically with a throwaway probe playbook against the host rather than inferred from the docs, because the two fabricated fields break the role in **opposite** directions: - **`stdout: ""`** can never equal `git.gitborg.se/gitborg/gitborg-web:latest`, so `(web_running_image.stdout … ) != (web_image ~ ':' ~ web_image_tag)` is always true and the expression resolves to `state: restarted`. Hence a `changed` on every dry-run that a real run does not perform. - **`rc: 0`** means every `when: web_image_present.rc == 0` guard takes the success branch regardless. So a dry-run silently assumes the image is published even when it is not — which is exactly the condition "Note when the bitborg-web image is not yet published" exists to report. That one is a dry-run that cannot warn you, the quieter half of the same bug. Worth recording because the obvious reading is wrong: the existing `| default('')` on `stdout` is not what rescues or breaks this. The value is registered as `""` outright, so the default never fires. Anyone reasoning from the guard alone would conclude the task is already protected. ## Why these two tasks and not the third Both probes are strictly read-only — `podman image exists` and `podman container inspect` — and already carry `changed_when: false` and `failed_when: false`. Running them under check mode observes the host without touching it, which is precisely what a dry-run is for. `systemctl --user reset-failed` sits in the same block and is deliberately **left skipped**. It clears latched systemd state; that is a mutation, and a dry-run must not perform it. The point is not "make check mode run more tasks", it is "let check mode read, never write". ## Verification Scoped `--check --diff --limit gitborg-prod --tags web`, before → after: | Task | Before | After | | -------------------------------------- | ----------- | --------------------- | | Check the bitborg-web image is present | `skipping` | `ok` (really running) | | Inspect the running bitborg-web image | `skipping` | `ok` (really running) | | Enable and start bitborg-web | `changed` | `ok` | | PLAY RECAP | `changed=1` | **`changed=0`** | Independently corroborated before changing anything: the running container was already on `git.gitborg.se/gitborg/gitborg-web:latest`, matching `web_image:web_image_tag` — so no restart was ever pending and `changed=0` is the truthful answer, not a suppressed one. This is the distinction that mattered: the goal was to make the dry-run honest, not to silence it. `ansible-lint roles/web/` passes on the production profile; `site.yml --syntax-check` clean. ## Note on applying No apply is needed for this PR. It changes only what `--check` reports, and the host is already converged — the full dry-run that started this was `ok=237 changed=1 failed=0`, whose single `changed` is the artefact being removed here. ## Follow-up Left in #294: other roles were checked and none share the drift-comparison shape, so nothing else reports a phantom change today. The general trap is broader though — any `when:` or state expression derived from a `command` register is reading fabricated values under `--check` — so a sweep for read-only probes that want `check_mode: false`, plus a line in the apply procedure, would stop the next person re-deriving this from scratch.
supernaut lade till 1 incheckning 2026-07-31 22:07:34 +00:00
fix(web): stop dry-runs reporting a phantom gitborg-web restart
Alla kontroller lyckades
ci / ci (pull_request) Successful in 1m28s
75833f25dd
Closes #294.

The restart decision reads two `command` probes, and `--check` skips commands —
registering a fabricated result rather than nothing. `rc: 0` makes the
"is the image published" guards take the success branch unconditionally, and
`stdout: ""` can never equal the expected image ref, so the drift comparison
always resolved to `restarted`. Every dry-run therefore advertised a
gitborg-web restart that a real run does not perform.

Both probes are strictly read-only, so they can run under check mode. The
`systemctl reset-failed` in the same block is left skipped: it mutates state
and has no business running during a dry-run.

Scoped dry-run goes from changed=1 to changed=0, with both probes now
reporting ok instead of skipping.
supernaut sammanfogade incheckning 18e49c7457 till main 2026-07-31 22:11:24 +00:00
supernaut tog bort grenen fix/294-checkmode-phantom-web-restart 2026-07-31 22:11:24 +00:00
supernaut ändrade titeln från fix(web): stop dry-runs reporting a phantom gitborg-web restart till fix(web): stop dry-runs reporting a phantom bitborg-web restart 2026-08-03 09:58:26 +00:00
Logga in för att delta i denna konversation.
Inga granskare
Ingen milstolpe
Inget projekt
Inga tilldelade
1 deltagare
Notiser
Förfallodatum
Förfallodatumet är ogiltigt eller utanför gränserna. Använd formatet "åååå-mm-dd".

Inget förfallodatum satt.

Beroenden

Inga beroenden satta

Referens
bitborg/bitborg-infra!295
Ingen beskrivning angiven.