diff --git a/bin/base-demo-services b/bin/base-demo-services index 70e56cb..209adce 100755 --- a/bin/base-demo-services +++ b/bin/base-demo-services @@ -359,10 +359,19 @@ def terminate_process_group( process_group_id: int, process: subprocess.Popen[Any] | None = None, ) -> bool: + def group_is_running() -> bool: + try: + os.killpg(process_group_id, 0) + except ProcessLookupError: + return False + except PermissionError: + return True + return True + def stopped() -> bool: - if process is not None: - return process.poll() is not None - return not process_is_running(pid) + if process is not None and process.poll() is None: + return False + return not group_is_running() try: os.killpg(process_group_id, signal.SIGTERM) @@ -385,6 +394,36 @@ def terminate_process_group( return stopped() +def run_command_check(command: list[str], root: Path, label: str) -> CheckResult: + process: subprocess.Popen[bytes] | None = None + try: + process = subprocess.Popen( + command, + cwd=root, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + start_new_session=True, + ) + try: + process.communicate(timeout=3.0) + except subprocess.TimeoutExpired as exc: + stopped = terminate_process_group(process.pid, process.pid, process) + try: + process.communicate(timeout=2.0) + except subprocess.TimeoutExpired: + stopped = False + detail = f"{label} ({exc})" + if not stopped: + detail = f"{detail}; process group cleanup failed" + return CheckResult(False, label, detail) + except OSError as exc: + return CheckResult(False, label, f"{label} ({exc})") + + if process is None: + return CheckResult(False, label, f"{label} (process did not start)") + return CheckResult(process.returncode == 0, label, label) + + def readiness_timeout(service: dict[str, Any]) -> float: lifecycle = service.get("lifecycle") or {} configured = os.environ.get("BASE_DEMO_SERVICES_START_TIMEOUT") @@ -743,18 +782,7 @@ def check_service( if not isinstance(command, list) or not all(isinstance(item, str) for item in command): return CheckResult(False, "command:", "command:") label = f"command:{shlex.join(command)}" - try: - result = subprocess.run( - command, - cwd=root, - check=False, - capture_output=True, - text=False, - timeout=3.0, - ) - except (OSError, subprocess.TimeoutExpired) as exc: - return CheckResult(False, label, f"{label} ({exc})") - return CheckResult(result.returncode == 0, label, label) + return run_command_check(command, root, label) detail = f"unsupported:{check_type}" return CheckResult(False, detail, detail) diff --git a/tests/services_test.bats b/tests/services_test.bats index f59b24e..f9b8f75 100644 --- a/tests/services_test.bats +++ b/tests/services_test.bats @@ -1020,3 +1020,80 @@ EOF wait "$UNRELATED_PID" 2>/dev/null || true UNRELATED_PID="" } + +@test "services command timeouts terminate owned descendants" { + local catalog="$TEST_TMPDIR/catalog.json" + local command="$TEST_TMPDIR/timeout-command.py" + local detached_marker="$TEST_TMPDIR/detached-marker" + local inherited_marker="$TEST_TMPDIR/inherited-marker" + + cat > "$command" <<'PY' +#!/usr/bin/env python3 +import pathlib +import subprocess +import sys +import time + +marker, mode = sys.argv[1:] +child = [ + sys.executable, + "-c", + "import pathlib,sys,time; time.sleep(4); pathlib.Path(sys.argv[1]).touch()", + marker, +] +kwargs = {} +if mode == "detached": + kwargs = {"stdout": subprocess.DEVNULL, "stderr": subprocess.DEVNULL} +subprocess.Popen(child, **kwargs) +time.sleep(10) +PY + chmod +x "$command" + + python3 - "$catalog" "$command" "$detached_marker" "$inherited_marker" <<'PY' +import json +import sys + +catalog, command, detached_marker, inherited_marker = sys.argv[1:] +with open(catalog, "w", encoding="utf-8") as handle: + json.dump( + { + "services": [ + { + "name": "timeout-detached", + "kind": "service", + "runtime": "test", + "required": True, + "check": {"type": "command", "command": [command, detached_marker, "detached"]}, + }, + { + "name": "timeout-inherited", + "kind": "service", + "runtime": "test", + "required": False, + "check": {"type": "command", "command": [command, inherited_marker, "inherited"]}, + }, + { + "name": "later-service", + "kind": "service", + "runtime": "test", + "required": False, + "check": {"type": "none"}, + }, + ] + }, + handle, + ) +PY + + run "$TEST_ROOT/bin/base-demo-services" --catalog "$catalog" check + + [ "$status" -eq 1 ] + [[ "$output" == *"timeout-detached fail command:"* ]] + [[ "$output" == *"timeout-inherited skip optional command:"* ]] + [[ "$output" == *"later-service ok"* ]] + [[ "$output" != *"process group cleanup failed"* ]] + + sleep 5 + [ ! -e "$detached_marker" ] + [ ! -e "$inherited_marker" ] +}