diff --git a/README.md b/README.md index b2ddd03..3af376e 100644 --- a/README.md +++ b/README.md @@ -67,7 +67,7 @@ Without `bash -c`, the environment variables would be evaluated before `with-con | `--license` | `./rstudio-connect.lic` | Path to license file. This file must exist and be a valid Connect license. | | `--image` | | Container image to use, including tag (e.g., `posit/connect:2025.12.0`). Overrides `--version`. | | `--config` | | Path to optional rstudio-connect.gcfg configuration file | -| `--port` | `3939` | Port to map the Connect container to. Allows running multiple Connect instances simultaneously. | +| `--port` | `3939` | Port to map the Connect container to. If not given and 3939 is in use, a free port is chosen, so multiple Connect instances can run at once. `CONNECT_SERVER` has the port in use. | | `-e`, `--env` | | Environment variables to pass to the Docker container (format: KEY=VALUE). Can be specified multiple times. | | `--stop` | | Stop a running Connect container by ID, or use `CONTAINER_ID` env var if not specified. | @@ -130,7 +130,7 @@ The GitHub Action supports the following inputs: | `version` | No | `release` | Posit Connect version | | `image` | No | | Container image to use, including tag (e.g., `posit/connect:2025.12.0`). Overrides `version`. | | `config-file` | No | | Path to rstudio-connect.gcfg configuration file | -| `port` | No | `3939` | Port to map the Connect container to | +| `port` | No | `3939` | Port to map the Connect container to. If not given and 3939 is in use, a free port is chosen. The `CONNECT_SERVER` output has the port in use. | | `quiet` | No | `false` | Suppress image pull progress and the Connect logs shown while Connect starts | | `env` | No | | Environment variables to pass to Docker container (one per line, format: KEY=VALUE) | | `command` | No | | Command to run against Connect (omit for start-only mode) | diff --git a/action.yml b/action.yml index 0152a14..3d80d38 100644 --- a/action.yml +++ b/action.yml @@ -19,9 +19,8 @@ inputs: required: false default: 'false' port: - description: 'Port to map the Connect container to' + description: 'Port to map the Connect container to (default: 3939, or a free port if 3939 is in use)' required: false - default: '3939' env: description: 'Environment variables to pass to Docker container (one per line, format: KEY=VALUE)' required: false @@ -68,7 +67,10 @@ runs: fi # Build arguments - ARGS="--version ${{ inputs.version }} --port ${{ inputs.port }}" + ARGS="--version ${{ inputs.version }}" + if [ -n "${{ inputs.port }}" ]; then + ARGS="$ARGS --port ${{ inputs.port }}" + fi if [ -n "${{ inputs.config-file }}" ]; then ARGS="$ARGS --config ${{ inputs.config-file }}" fi diff --git a/main.py b/main.py index 6656cbc..9bbb31c 100644 --- a/main.py +++ b/main.py @@ -20,6 +20,7 @@ PREVIEW_IMAGE = "ghcr.io/posit-dev/connect-preview" LEGACY_IMAGE = "rstudio/rstudio-connect" VERSION = "release" +DEFAULT_PORT = 3939 # Versions at or after this registry cutover land on ghcr.io/posit-dev. # Older versions only exist on the legacy Docker Hub images. @@ -104,8 +105,7 @@ def parse_args(): parser.add_argument( "--port", type=port_number, - default=3939, - help="Port to map the Connect container to (default: 3939)", + help=f"Port to map the Connect container to (default: {DEFAULT_PORT}, or a free port if {DEFAULT_PORT} is in use)", ) parser.add_argument( "--stop", @@ -450,13 +450,17 @@ def run() -> int: ) container_env = build_container_env(bootstrap_secret, args.env_vars) + port = args.port + if port is None and is_port_free(DEFAULT_PORT): + port = DEFAULT_PORT create_kwargs = { "image": image_name, "tty": True, "stdin_open": True, "privileged": True, - "ports": {"3939/tcp": args.port}, + # None has Docker assign a free host port + "ports": {"3939/tcp": port}, "mounts": mounts, "environment": container_env, } @@ -477,18 +481,22 @@ def run() -> int: pass # Don't hide the start error behind a cleanup error raise RuntimeError(f"Failed to start container from {image_name}: {e}") - server_url = f"http://localhost:{args.port}" stop_container = True # Echo Connect's startup logs to stderr (stdout is reserved for KEY=value lines) startup_logs = ContainerLogs(container, quiet=args.quiet) try: - print(f"Waiting for port {args.port} to open...", file=sys.stderr) - if not is_port_open("localhost", args.port, timeout=60.0, container=container): + if port is None: + port = published_port(container) + print(f"Port {DEFAULT_PORT} is in use. Using port {port}.", file=sys.stderr) + server_url = f"http://localhost:{port}" + + print(f"Waiting for port {port} to open...", file=sys.stderr) + if not is_port_open("localhost", port, timeout=60.0, container=container): startup_logs.dump() if container.status in ("exited", "dead"): raise RuntimeError( - f"Posit Connect container exited before port {args.port} opened." + f"Posit Connect container exited before port {port} opened." ) raise RuntimeError("Posit Connect did not start within 60 seconds.") @@ -580,6 +588,33 @@ def is_port_open( time.sleep(min(poll_interval, remaining)) +def is_port_free(port: int) -> bool: + """ + Check if a host port is free by trying to bind to it on all interfaces, + which is how Docker publishes container ports. + + Docker reports a port taken by another container, but Docker Desktop + doesn't report one taken by a process on the host. + """ + with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as s: + try: + s.bind(("", port)) + return True + except OSError: + return False + + +def published_port(container) -> int: + """ + Return the host port Docker assigned to the container's port 3939. + """ + container.reload() + bindings = container.ports.get("3939/tcp") or [] + if not bindings: + raise RuntimeError("Docker did not assign a host port to the container") + return int(bindings[0]["HostPort"]) + + def extract_server_version(logs: str) -> str | None: """ Extract the Posit Connect version from container logs. diff --git a/test_main.py b/test_main.py index b998f83..698e638 100644 --- a/test_main.py +++ b/test_main.py @@ -485,6 +485,19 @@ def test_port_range_limits_are_accepted(): assert parse_with_argv("--port", "65535").port == 65535 +def test_port_defaults_to_none(): + assert parse_with_argv().port is None + + +def test_is_port_free(): + with main.socket.socket(main.socket.AF_INET, main.socket.SOCK_STREAM) as s: + s.bind(("", 0)) + s.listen() + port = s.getsockname()[1] + assert main.is_port_free(port) is False + assert main.is_port_free(port) is True + + def test_help_explains_how_to_stop(): code, output = parse_exit("--help") help_text = " ".join(output.split()) @@ -731,14 +744,15 @@ def test_post_bootstrap_raises_on_other_errors(): -def run_with_client(client): +def run_with_client(client, *args, port_free=True): """Run run() against a mock Docker client, expecting a RuntimeError.""" with tempfile.NamedTemporaryFile(suffix=".lic") as license_file: - argv = ["with-connect", "--license", license_file.name, "--quiet"] + argv = ["with-connect", "--license", license_file.name, "--quiet", *args] 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_free", return_value=port_free), ): try: main.run() @@ -763,7 +777,7 @@ def test_container_create_error_is_reported(): def test_container_start_error_removes_container(): - error = docker.errors.APIError("500 Server Error: port is already allocated") + error = docker.errors.APIError("500 Server Error: OCI runtime create failed") client = MagicMock() container = client.containers.create.return_value container.start.side_effect = error @@ -778,12 +792,12 @@ def test_container_start_error_removes_container(): def test_container_start_error_survives_failed_cleanup(): client = MagicMock() container = client.containers.create.return_value - container.start.side_effect = docker.errors.APIError("port is already allocated") + container.start.side_effect = docker.errors.APIError("OCI runtime create failed") container.remove.side_effect = docker.errors.APIError("removal failed") message = run_with_client(client) - assert "port is already allocated" in message + assert "OCI runtime create failed" in message def test_stop_error_is_reported(): @@ -815,6 +829,52 @@ def test_cleanup_stop_error_does_not_hide_original_error(): assert "did not start within 60 seconds" in message assert "Warning: failed to stop container abc123: daemon went away" in stderr.getvalue() +def run_until_port_wait(*args, port_free=True): + """ + Run with a mock Docker client that publishes port 50000, stopping at + the wait for Connect. Returns the client and what was printed to stderr. + """ + client = MagicMock() + container = client.containers.create.return_value + container.ports = {"3939/tcp": [{"HostIp": "0.0.0.0", "HostPort": "50000"}]} + container.logs.return_value = b"" + stderr = io.StringIO() + with patch.object(main, "is_port_open", return_value=False), redirect_stderr(stderr): + run_with_client(client, *args, port_free=port_free) + return client, stderr.getvalue() + + +def test_default_port_used_when_free(): + client, stderr = run_until_port_wait() + + assert client.containers.create.call_args.kwargs["ports"] == {"3939/tcp": 3939} + assert "Waiting for port 3939 to open" in stderr + + +def test_default_port_taken_uses_docker_assigned_port(): + client, stderr = run_until_port_wait(port_free=False) + + assert client.containers.create.call_args.kwargs["ports"] == {"3939/tcp": None} + assert "Port 3939 is in use. Using port 50000." in stderr + assert "Waiting for port 50000 to open" in stderr + + +def test_explicit_port_used_even_if_taken(): + client, stderr = run_until_port_wait("--port", "3939", port_free=False) + + assert client.containers.create.call_args.kwargs["ports"] == {"3939/tcp": 3939} + assert "Waiting for port 3939 to open" in stderr + + +def test_published_port_without_binding_is_an_error(): + container = MagicMock() + container.ports = {} + try: + main.published_port(container) + assert False, "Expected RuntimeError" + except RuntimeError as e: + assert "did not assign a host port" in str(e) + def test_docker_unreachable_is_reported(): error = docker.errors.DockerException("Error while fetching server API version") with patch.object(main.docker, "from_env", side_effect=error): @@ -960,6 +1020,12 @@ def test_installed_command_uses_main(): test_port_range_limits_are_accepted() print("✓ test_port_range_limits_are_accepted passed") + test_port_defaults_to_none() + print("✓ test_port_defaults_to_none passed") + + test_is_port_free() + print("✓ test_is_port_free passed") + test_help_explains_how_to_stop() print("✓ test_help_explains_how_to_stop passed") @@ -1032,6 +1098,18 @@ def test_installed_command_uses_main(): test_cleanup_stop_error_does_not_hide_original_error() print("✓ test_cleanup_stop_error_does_not_hide_original_error passed") + test_default_port_used_when_free() + print("✓ test_default_port_used_when_free passed") + + test_default_port_taken_uses_docker_assigned_port() + print("✓ test_default_port_taken_uses_docker_assigned_port passed") + + test_explicit_port_used_even_if_taken() + print("✓ test_explicit_port_used_even_if_taken passed") + + test_published_port_without_binding_is_an_error() + print("✓ test_published_port_without_binding_is_an_error passed") + test_docker_unreachable_is_reported() print("✓ test_docker_unreachable_is_reported passed")