From 4aba897ca2ca79542ec0f9fc40910af19cf651b5 Mon Sep 17 00:00:00 2001 From: Kevin Veen-Birkenbach Date: Thu, 24 Sep 2026 09:24:24 +0200 Subject: [PATCH] fix(compose): bound the pull and build subprocess so a hang cannot stall a deploy pull.py ran docker compose build and pull through subprocess.run without a timeout. The ansible task keyword eventually kills the task, but the python in between kept waiting, and the handler name says "compose pull" while pull.py also builds, so the phase the deploy sits in is not what the log claims. A first libretranslate build downloading every Argos model outlived the task timeout, and each retry restarted the RUN layer from zero. run_cmd now passes timeout=RUN_TIMEOUT_SECONDS and turns TimeoutExpired into rc 124 plus whatever the command had already written, which every caller already treats as a failure. The matrix compose path carried no task timeout at all: its compose up has retries: 12, so a hang would never have triggered a single one of them. Co-Authored-By: Claude Opus 5 (1M context) --- roles/sys-svc-compose/files/python/pull.py | 35 ++++++++++++++----- .../flavor/compose/03_compose_pull_and_up.yml | 2 ++ .../roles/sys-svc-compose/files/test_pull.py | 18 ++++++++++ 3 files changed, 47 insertions(+), 8 deletions(-) diff --git a/roles/sys-svc-compose/files/python/pull.py b/roles/sys-svc-compose/files/python/pull.py index 83a666fbbd..4c31174012 100755 --- a/roles/sys-svc-compose/files/python/pull.py +++ b/roles/sys-svc-compose/files/python/pull.py @@ -9,6 +9,9 @@ from pathlib import Path import yaml +RUN_TIMEOUT_SECONDS = 3600 +TIMEOUT_RC = 124 + def run_cmd(cmd: list[str], *, cwd: Path, env: dict[str, str]) -> tuple[int, str, str]: """Run a command with stdout and stderr captured SEPARATELY. @@ -16,15 +19,31 @@ def run_cmd(cmd: list[str], *, cwd: Path, env: dict[str, str]) -> tuple[int, str Compose prints warnings ('variable is not set', ...) on stderr; merging them into stdout poisons every parser downstream (yaml config, the `config --images` list, the `pull --help` probe). + + Returns: + Exit code, stdout, stderr. A run that outlives + ``RUN_TIMEOUT_SECONDS`` returns ``TIMEOUT_RC`` with whatever the + command had written by then. """ - p = subprocess.run( - cmd, - cwd=str(cwd), - env=env, - text=True, - capture_output=True, - check=False, - ) + try: + p = subprocess.run( + cmd, + cwd=str(cwd), + env=env, + text=True, + capture_output=True, + check=False, + timeout=RUN_TIMEOUT_SECONDS, + ) + except subprocess.TimeoutExpired as expired: + out = expired.stdout or "" + err = expired.stderr or "" + return ( + TIMEOUT_RC, + out if isinstance(out, str) else out.decode("utf-8", "replace"), + (err if isinstance(err, str) else err.decode("utf-8", "replace")) + + f"\ntimed out after {RUN_TIMEOUT_SECONDS}s: {' '.join(cmd)}\n", + ) return p.returncode, p.stdout or "", p.stderr or "" diff --git a/roles/web-app-matrix/tasks/flavor/compose/03_compose_pull_and_up.yml b/roles/web-app-matrix/tasks/flavor/compose/03_compose_pull_and_up.yml index 2f3e0f4d3c..3cadf94b11 100644 --- a/roles/web-app-matrix/tasks/flavor/compose/03_compose_pull_and_up.yml +++ b/roles/web-app-matrix/tasks/flavor/compose/03_compose_pull_and_up.yml @@ -4,6 +4,7 @@ command: cmd: "compose pull" chdir: "{{ lookup('container', application_id, 'directories.instance') }}" + timeout: "{{ lookup('timeout', 3600) | int }}" - name: 🐳 compose up command: @@ -11,5 +12,6 @@ chdir: "{{ lookup('container', application_id, 'directories.instance') }}" register: result until: result is succeeded + timeout: "{{ lookup('timeout', 1800) | int }}" retries: 12 delay: 30 diff --git a/tests/unit/python/roles/sys-svc-compose/files/test_pull.py b/tests/unit/python/roles/sys-svc-compose/files/test_pull.py index b6822313c7..82ef8d53c5 100644 --- a/tests/unit/python/roles/sys-svc-compose/files/test_pull.py +++ b/tests/unit/python/roles/sys-svc-compose/files/test_pull.py @@ -44,6 +44,24 @@ class TestComposePull(unittest.TestCase): self.assertEqual(out, "hello\n") self.assertEqual(err, "warn: variable is not set\n") + def test_run_cmd_reports_a_timeout_instead_of_hanging(self) -> None: + def fake_run(*args, **kwargs): + self.assertEqual(kwargs["timeout"], self.m.RUN_TIMEOUT_SECONDS) + raise self.m.subprocess.TimeoutExpired( + cmd=["compose", "build"], + timeout=self.m.RUN_TIMEOUT_SECONDS, + output=b"step 3/4\n", + stderr=b"pulling\n", + ) + + with patch.object(self.m.subprocess, "run", side_effect=fake_run): + rc, out, err = self.m.run_cmd(["compose", "build"], cwd=Path("/"), env={}) + + self.assertEqual(rc, self.m.TIMEOUT_RC) + self.assertEqual(out, "step 3/4\n") + self.assertIn(f"timed out after {self.m.RUN_TIMEOUT_SECONDS}s", err) + self.assertIn("pulling", err) + def test_base_compose_cmd_delegates_to_wrapper(self) -> None: cmd = self.m.base_compose_cmd(project="p", cwd=Path("/x")) self.assertEqual(cmd[0], "/usr/bin/compose")