This commit is contained in:
@@ -0,0 +1,131 @@
|
||||
"""A load-bearing deploy step must not report success when it failed.
|
||||
|
||||
`deploy.sh` runs its real work over ssh. The exit status ssh hands back
|
||||
is the status of the *remote* command — and when that command ends in a pipe,
|
||||
the status belongs to the last stage of the pipe, not to the work:
|
||||
|
||||
ssh "$SERVER" "docker compose pull … 2>&1 | grep -E 'Pulled|Already|Error'"
|
||||
|
||||
A failed pull prints a line containing `Error`, `grep` matches it and exits 0, and
|
||||
the deploy walks on to swap the containers. `set -o pipefail` cannot see this: the
|
||||
remote pipeline ran inside the remote shell and from here the ssh succeeded.
|
||||
|
||||
A sibling service hit the same shape on 2026-09-15 with
|
||||
`alembic upgrade head 2>&1 | tail -5`. The migration raised a TypeError, the
|
||||
script printed the traceback and swapped the containers anyway, and the service
|
||||
answered 500 on every request touching the changed table for eleven minutes.
|
||||
|
||||
So: tail and grep locally, on captured output, where `pipefail` and `set -e` can
|
||||
still act. The rule below is deliberately narrow — it covers only the steps whose
|
||||
failure must stop a deploy. `docker image prune | tail -1` is cosmetic and stays
|
||||
exempt, because a rule that forbids every pipe would be worked around rather than
|
||||
followed.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import re
|
||||
from pathlib import Path
|
||||
|
||||
DEPLOY = Path(__file__).resolve().parent.parent / "deploy.sh"
|
||||
|
||||
#: The steps a deploy must not walk past. Anything else may pipe as it likes.
|
||||
LOAD_BEARING = re.compile(
|
||||
r"docker pull|compose[^\n]*\bpull\b|compose[^\n]*\bup -d\b|alembic upgrade|/app/deploy/"
|
||||
)
|
||||
|
||||
#: Filters that replace the exit status of everything before them.
|
||||
SWALLOWS_STATUS = re.compile(r"\|\s*(tail|head|grep)\b")
|
||||
|
||||
#: Every ssh line ends `| sed "s/^/[${SERVER}] /"`, which runs *here*. Local
|
||||
#: pipes are covered by `set -o pipefail`; only what runs on the far side is
|
||||
#: invisible, so the local suffix is cut off before the check.
|
||||
LOCAL_SUFFIX = '| sed "s/^/'
|
||||
|
||||
|
||||
def _logical_lines() -> list[str]:
|
||||
"""The script with backslash-continuations joined, so one command is one line.
|
||||
|
||||
Comments are dropped. The first version of this check flagged the comment
|
||||
inside `run_remote` that quotes the offending line in order to explain it —
|
||||
prose about a statement is not the statement, and a checker that cannot tell
|
||||
them apart makes the explanation unwritable.
|
||||
"""
|
||||
joined = DEPLOY.read_text().replace("\\\n", " ")
|
||||
return [line for line in joined.splitlines() if not line.lstrip().startswith("#")]
|
||||
|
||||
|
||||
def _remote_part(line: str) -> str:
|
||||
head, _, _ = line.partition(LOCAL_SUFFIX)
|
||||
return head
|
||||
|
||||
|
||||
def _offenders() -> list[str]:
|
||||
return [
|
||||
" ".join(line.split())[:120]
|
||||
for line in _logical_lines()
|
||||
if "ssh " in line
|
||||
and LOAD_BEARING.search(line)
|
||||
and SWALLOWS_STATUS.search(_remote_part(line))
|
||||
]
|
||||
|
||||
|
||||
def test_the_detector_sees_the_shape_it_exists_for() -> None:
|
||||
"""Guards the guard, against the exact line that caused the outage."""
|
||||
broken = 'ssh -n "$SERVER" "cd ~/x && docker compose pull api 2>&1 | grep -E \'Pulled\'"'
|
||||
|
||||
assert LOAD_BEARING.search(broken)
|
||||
assert SWALLOWS_STATUS.search(_remote_part(broken))
|
||||
|
||||
|
||||
def test_a_local_tail_is_not_an_offence() -> None:
|
||||
"""`pipefail` already covers this side of the ssh; the rule is about the far
|
||||
side. A check that flagged both would push the fix in the wrong direction."""
|
||||
fine = 'ssh -n "$SERVER" "docker compose pull api" | sed "s/^/[x] /" | tail -3'
|
||||
|
||||
assert not SWALLOWS_STATUS.search(_remote_part(fine))
|
||||
|
||||
|
||||
def test_the_script_is_there_and_has_ssh_steps() -> None:
|
||||
"""An unreadable or renamed script would make the test below vacuously green."""
|
||||
assert [line for line in _logical_lines() if "ssh " in line]
|
||||
|
||||
|
||||
def test_no_load_bearing_step_hides_its_exit_status() -> None:
|
||||
offenders = _offenders()
|
||||
|
||||
assert not offenders, "remote pipes swallow the status of:\n " + "\n ".join(offenders)
|
||||
|
||||
|
||||
class TestTheHostRecordsOnlyWhatItRuns:
|
||||
"""`~/netork/.env` on the server is a record, not an intention.
|
||||
|
||||
It used to be written before the pull. A deploy that then failed — a tag the
|
||||
registry does not have, say — left the host recording a version it had never
|
||||
run, and the next `docker compose up` a human typed there reached for an
|
||||
image that does not exist. Observed on a test host while verifying the
|
||||
failure path above: the deploy stopped exactly where it should, and still
|
||||
left `NETORK_VERSION=main-0000000` behind.
|
||||
|
||||
So it is written after the swap and after the image-id check agreed, and this
|
||||
holds that order. Positional rather than behavioural, which is the honest
|
||||
limit of a static check — but the order is the whole property.
|
||||
"""
|
||||
|
||||
@staticmethod
|
||||
def _position(needle: str) -> int:
|
||||
text = DEPLOY.read_text()
|
||||
assert text.count(needle) == 1, f"{needle!r} appears {text.count(needle)} times"
|
||||
return text.index(needle)
|
||||
|
||||
def test_env_is_recorded_after_the_image_check(self) -> None:
|
||||
verified = self._position("Verified: containers run")
|
||||
recorded = self._position("Recording registry settings")
|
||||
|
||||
assert recorded > verified, (
|
||||
"the host records NETORK_VERSION before the deploy is verified; "
|
||||
"a failed deploy then leaves it pointing at a version never run"
|
||||
)
|
||||
|
||||
def test_env_is_recorded_after_the_pull(self) -> None:
|
||||
assert self._position("Recording registry settings") > self._position("Pulling images...")
|
||||
Reference in New Issue
Block a user