From b415cabf67dcc1ceffcdea0d5d4f88961e285c6c Mon Sep 17 00:00:00 2001 From: Neal Richardson Date: Fri, 23 Oct 2026 09:51:23 -0400 Subject: [PATCH 1/2] fix: Stop Connect containers promptly by signaling Connect directly Newer images run startup.sh as PID 1, which ignores SIGTERM, so stopping the container waited out Docker's 10 second grace period before killing it. with-connect sends SIGTERM to the Connect process instead, which shuts down cleanly in under a second, and falls back to a 2 second stop timeout. Fixes #56. Co-Authored-By: Claude Opus 5.5 --- main.py | 30 +++++++++++++++++++++++++++--- test_main.py | 51 ++++++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 77 insertions(+), 4 deletions(-) diff --git a/main.py b/main.py index 22ffbd2..3bf42b8 100644 --- a/main.py +++ b/main.py @@ -25,6 +25,9 @@ REGISTRY_CUTOVER_YEAR = 2026 REGISTRY_CUTOVER_MONTH = 4 +# Matches the Connect server's command line, for pkill -f +CONNECT_PROCESS_PATTERN = "^/opt/rstudio-connect/bin/connect( |$)" + DESCRIPTION = """\ Starts Posit Connect in Docker and waits until it is ready. @@ -274,6 +277,28 @@ def get_docker_tag(version: str) -> tuple[str, str]: return (LEGACY_IMAGE, version) +def stop_container(container) -> None: + """ + Stop a Connect container promptly. + + Newer images run startup.sh as PID 1, and a shell running as PID 1 + ignores SIGTERM, so container.stop() waits out its 10 second grace + period and then kills the container. Signaling the Connect process + directly lets it shut down cleanly in under a second, and the container + exits with it. If that doesn't work, fall back to a short stop timeout. + """ + try: + exit_code, _ = container.exec_run( + ["pkill", "-TERM", "-f", CONNECT_PROCESS_PATTERN] + ) + if exit_code == 0: + container.wait(timeout=10) + return + except Exception: + pass # e.g. no pkill in the image, or Connect didn't exit in time + container.stop(timeout=2) + + def main() -> int: """ Main entry point for the with-connect CLI tool. @@ -297,7 +322,7 @@ def main() -> int: client = docker.from_env() try: container = client.containers.get(container_id) - container.stop() + stop_container(container) print(f"Stopped container {container_id}", file=sys.stderr) return 0 except docker.errors.NotFound: @@ -417,7 +442,7 @@ def main() -> int: return exit_code finally: if stop_container: - container.stop() + stop_container(container) def is_port_open(host: str, port: int, timeout: float = 30.0) -> bool: @@ -476,7 +501,6 @@ def wait_for_http_server( if "Unable to obtain a valid license" in logs: print("\nContainer logs:", file=sys.stderr) print(logs, file=sys.stderr) - container.stop() raise RuntimeError( "Unable to obtain a valid license. Your Posit Connect license may be expired or invalid. Please check your license file." ) diff --git a/test_main.py b/test_main.py index 41525dc..6e589d6 100644 --- a/test_main.py +++ b/test_main.py @@ -13,6 +13,7 @@ import docker import main +import requests def test_license_file_not_exists(): @@ -75,7 +76,8 @@ def test_invalid_license_detection(): except RuntimeError as e: assert "Unable to obtain a valid license" in str(e) assert "expired or invalid" in str(e) - mock_container.stop.assert_called_once() + # main() stops the container when this raises + mock_container.stop.assert_not_called() def test_valid_license_http_server_starts(): @@ -346,6 +348,44 @@ def test_stop_nonexistent_container(): assert "Container not found" in result.stderr +def test_stop_container_signals_connect(): + container = MagicMock() + container.exec_run.return_value = (0, b"") + + main.stop_container(container) + + container.exec_run.assert_called_once_with( + ["pkill", "-TERM", "-f", main.CONNECT_PROCESS_PATTERN] + ) + container.wait.assert_called_once_with(timeout=10) + container.stop.assert_not_called() + + +def test_stop_container_falls_back_when_connect_not_signaled(): + # pkill exits 1 when nothing matches and 127 when it isn't installed + for exit_code in (1, 127): + container = MagicMock() + container.exec_run.return_value = (exit_code, b"") + + main.stop_container(container) + + container.wait.assert_not_called() + container.stop.assert_called_once_with(timeout=2) + + +def test_stop_container_falls_back_on_errors(): + exec_failure = MagicMock() + exec_failure.exec_run.side_effect = docker.errors.APIError("container is not running") + wait_timeout = MagicMock() + wait_timeout.exec_run.return_value = (0, b"") + wait_timeout.wait.side_effect = requests.exceptions.ReadTimeout("Read timed out.") + + for container in (exec_failure, wait_timeout): + main.stop_container(container) + + container.stop.assert_called_once_with(timeout=2) + + def _decode_segment(segment): return json.loads(base64.urlsafe_b64decode(segment + "=" * (-len(segment) % 4))) @@ -510,6 +550,15 @@ def test_post_bootstrap_raises_on_other_errors(): test_stop_nonexistent_container() print("✓ test_stop_nonexistent_container passed") + test_stop_container_signals_connect() + print("✓ test_stop_container_signals_connect passed") + + test_stop_container_falls_back_when_connect_not_signaled() + print("✓ test_stop_container_falls_back_when_connect_not_signaled passed") + + test_stop_container_falls_back_on_errors() + print("✓ test_stop_container_falls_back_on_errors passed") + test_make_bootstrap_jwt() print("✓ test_make_bootstrap_jwt passed") From 7e590ffedc3ab53490894d8814eca9f74f635f85 Mon Sep 17 00:00:00 2001 From: Neal Richardson Date: Fri, 23 Oct 2026 09:53:25 -0400 Subject: [PATCH 2/2] fix: Rename stop helper so main()'s stop_container flag doesn't shadow it Adds tests that stop Connect through main() in --stop and command mode. Addresses roborev job 683. Co-Authored-By: Claude Opus 5.5 --- main.py | 6 ++--- test_main.py | 69 +++++++++++++++++++++++++++++++++++++++++++--------- 2 files changed, 60 insertions(+), 15 deletions(-) diff --git a/main.py b/main.py index 3bf42b8..a0d2f25 100644 --- a/main.py +++ b/main.py @@ -277,7 +277,7 @@ def get_docker_tag(version: str) -> tuple[str, str]: return (LEGACY_IMAGE, version) -def stop_container(container) -> None: +def stop_connect_container(container) -> None: """ Stop a Connect container promptly. @@ -322,7 +322,7 @@ def main() -> int: client = docker.from_env() try: container = client.containers.get(container_id) - stop_container(container) + stop_connect_container(container) print(f"Stopped container {container_id}", file=sys.stderr) return 0 except docker.errors.NotFound: @@ -442,7 +442,7 @@ def main() -> int: return exit_code finally: if stop_container: - stop_container(container) + stop_connect_container(container) def is_port_open(host: str, port: int, timeout: float = 30.0) -> bool: diff --git a/test_main.py b/test_main.py index 6e589d6..6da2fc1 100644 --- a/test_main.py +++ b/test_main.py @@ -348,11 +348,11 @@ def test_stop_nonexistent_container(): assert "Container not found" in result.stderr -def test_stop_container_signals_connect(): +def test_stop_connect_container_signals_connect(): container = MagicMock() container.exec_run.return_value = (0, b"") - main.stop_container(container) + main.stop_connect_container(container) container.exec_run.assert_called_once_with( ["pkill", "-TERM", "-f", main.CONNECT_PROCESS_PATTERN] @@ -361,19 +361,58 @@ def test_stop_container_signals_connect(): container.stop.assert_not_called() -def test_stop_container_falls_back_when_connect_not_signaled(): +def test_stop_mode_signals_connect(): + client = MagicMock() + container = client.containers.get.return_value + container.exec_run.return_value = (0, b"") + with ( + patch.object(sys, "argv", ["with-connect", "--stop", "abc123"]), + patch.object(main.docker, "from_env", return_value=client), + redirect_stderr(io.StringIO()), + ): + assert main.main() == 0 + + container.exec_run.assert_called_once_with( + ["pkill", "-TERM", "-f", main.CONNECT_PROCESS_PATTERN] + ) + + +def test_command_mode_signals_connect_after_command(): + client = MagicMock() + container = client.containers.run.return_value + container.exec_run.return_value = (0, b"") + with tempfile.NamedTemporaryFile(suffix=".lic") as license_file: + argv = ["with-connect", "--license", license_file.name, "--", "true"] + with ( + patch.object(sys, "argv", argv), + patch.object(main.docker, "from_env", return_value=client), + patch.object(main, "ensure_image"), + patch.object(main, "is_port_open", return_value=True), + patch.object(main, "wait_for_http_server", return_value=True), + patch.object(main, "get_api_key", return_value="key"), + patch.object(main.subprocess, "run", return_value=Mock(returncode=0)), + redirect_stderr(io.StringIO()), + ): + assert main.main() == 0 + + container.exec_run.assert_called_once_with( + ["pkill", "-TERM", "-f", main.CONNECT_PROCESS_PATTERN] + ) + + +def test_stop_connect_container_falls_back_when_connect_not_signaled(): # pkill exits 1 when nothing matches and 127 when it isn't installed for exit_code in (1, 127): container = MagicMock() container.exec_run.return_value = (exit_code, b"") - main.stop_container(container) + main.stop_connect_container(container) container.wait.assert_not_called() container.stop.assert_called_once_with(timeout=2) -def test_stop_container_falls_back_on_errors(): +def test_stop_connect_container_falls_back_on_errors(): exec_failure = MagicMock() exec_failure.exec_run.side_effect = docker.errors.APIError("container is not running") wait_timeout = MagicMock() @@ -381,7 +420,7 @@ def test_stop_container_falls_back_on_errors(): wait_timeout.wait.side_effect = requests.exceptions.ReadTimeout("Read timed out.") for container in (exec_failure, wait_timeout): - main.stop_container(container) + main.stop_connect_container(container) container.stop.assert_called_once_with(timeout=2) @@ -550,14 +589,20 @@ def test_post_bootstrap_raises_on_other_errors(): test_stop_nonexistent_container() print("✓ test_stop_nonexistent_container passed") - test_stop_container_signals_connect() - print("✓ test_stop_container_signals_connect passed") + test_stop_connect_container_signals_connect() + print("✓ test_stop_connect_container_signals_connect passed") + + test_stop_mode_signals_connect() + print("✓ test_stop_mode_signals_connect passed") + + test_command_mode_signals_connect_after_command() + print("✓ test_command_mode_signals_connect_after_command passed") - test_stop_container_falls_back_when_connect_not_signaled() - print("✓ test_stop_container_falls_back_when_connect_not_signaled passed") + test_stop_connect_container_falls_back_when_connect_not_signaled() + print("✓ test_stop_connect_container_falls_back_when_connect_not_signaled passed") - test_stop_container_falls_back_on_errors() - print("✓ test_stop_container_falls_back_on_errors passed") + test_stop_connect_container_falls_back_on_errors() + print("✓ test_stop_connect_container_falls_back_on_errors passed") test_make_bootstrap_jwt() print("✓ test_make_bootstrap_jwt passed")