diff --git a/docsgpt/deploy/commands.py b/docsgpt/deploy/commands.py index 5ba2c4e3..c03ccecb 100644 --- a/docsgpt/deploy/commands.py +++ b/docsgpt/deploy/commands.py @@ -298,17 +298,26 @@ def _port_is_free(port: int) -> bool: def _refuse_busy_port(directory: Path, port: int, services, names: tuple[str, str]) -> None: """Whatever holds the port would answer the health check while these services failed to bind. - An install re-running on its own port is the exception: its services are holding it, and - starting them again replaces them. + The one exception is this install's own API holding the port it is recorded on. Ownership is not + inferred from the install having some service running: asking for a different port that something + else holds is how a move to a new port would look, and the API would fail to bind it. """ if _port_is_free(port): return - if _record(directory).get("mode") == "native" and any(services.is_running(name) for name in names): + record = _record(directory) + api_service = names[0] + owns_the_port = ( + record.get("mode") == "native" + and str(record.get("port") or "") == str(port) + and services.is_running(api_service) + ) + if owns_the_port: return raise DeployError( - f"port {port} is already in use by something other than this install. The API would fail to " - f"bind it while the health check answered from whatever holds it, so the install would look " - f"healthy and be dead. Free the port, or give this install another one with --port." + f"port {port} is already in use by something other than this install's API. It would fail to " + f"bind while the health check answered from whatever holds the port, so the install would " + f"look healthy and be dead. Free the port, stop this install first with `docsgpt down` if it " + f"is the one holding it on another port, or choose another port with --port." ) @@ -406,7 +415,7 @@ def _native_up(args, context: Context, directory: Path) -> int: # that status, down and uninstall can see and clean up, rather than orphaned units. now = datetime.now(timezone.utc).isoformat(timespec="seconds") record = record or {"installed_at": now} - record.update(version=context.version, mode="native", updated_at=now) + record.update(version=context.version, mode="native", port=port, updated_at=now) (directory / stack.RECORD_FILE).write_text(json.dumps(record, indent=2) + "\n", encoding="utf-8") for unit in native.units_for(directory, launcher, port, directory, names): diff --git a/tests/deploy/test_native.py b/tests/deploy/test_native.py index ce66744f..d5a41696 100644 --- a/tests/deploy/test_native.py +++ b/tests/deploy/test_native.py @@ -295,16 +295,33 @@ class TestNativeUp: _run(argv, _native_context()) assert not (tmp_path / ".env").exists(), "it refuses before writing anything" - def test_an_install_may_keep_the_port_its_own_services_hold(self, tmp_path): - """Re-running an install must not trip over the services it is about to replace.""" + def test_an_install_may_keep_the_port_its_own_api_is_recorded_on(self, tmp_path): + """Re-running an install must not trip over the API it is about to replace.""" + services = FakeServices() + with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as probe: + probe.bind(("127.0.0.1", 0)) + port = probe.getsockname()[1] + argv = ["up", "--native", "--dir", str(tmp_path), "--yes", "--port", str(port), + "--postgres-uri", "postgresql://localhost/d"] + assert _run(argv, _native_context(services)) == 0 + assert json.loads((tmp_path / "install.json").read_text())["port"] == port + + with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as held: + held.bind(("127.0.0.1", port)) + held.listen(1) + assert _run(["up", "--dir", str(tmp_path), "--yes"], _native_context(services)) == 0 + + def test_moving_an_install_onto_a_busy_port_is_refused(self, tmp_path): + """Ownership is of one port, not of any port while some service of the install runs.""" services = FakeServices() argv = ["up", "--native", "--dir", str(tmp_path), "--yes", "--postgres-uri", "postgresql://localhost/d"] assert _run(argv, _native_context(services)) == 0 with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as held: held.bind(("127.0.0.1", 0)) held.listen(1) - envfile.update(tmp_path / ".env", {"DOCSGPT_PORT": str(held.getsockname()[1])}) - assert _run(["up", "--dir", str(tmp_path), "--yes"], _native_context(services)) == 0 + elsewhere = held.getsockname()[1] + with pytest.raises(DeployError, match="already in use"): + _run(["up", "--dir", str(tmp_path), "--yes", "--port", str(elsewhere)], _native_context(services)) def test_a_database_url_is_required(self, tmp_path): with pytest.raises(DeployError, match="--postgres-uri"):