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")