mirror of
https://github.com/tiennm99/DocsGPT.git
synced 2026-10-13 16:14:10 +00:00
fix: make a native install take effect on systemd and stay out of Docker's way
From review of #2800: - systemd `enable --now` starts nothing when the unit is already active, so a second `up --native` kept the old ExecStart and left the API on its previous port. start enables and then restarts, as the launchd path already did by booting the job out first. - An explicit `home` now wins over XDG_CONFIG_HOME, which is what callers pass it for. - The ExecStart program must be a real executable: when `docsgpt` is not on PATH, sys.argv[0] is accepted only if it can be run, and otherwise the failure is raised before any unit is written. - `up --native` over a directory holding a Docker install now refuses and says how to proceed, instead of starting native services beside containers that down, status and uninstall would no longer see. - Docs: without a terminal only --postgres-uri is required, and the Windows fallback names `docsgpt beat`, which the worker cannot embed there. SystemdServices was the least covered part of the module and cannot be run on this machine, so it now has tests for install, start, stop, remove, is_running and a failing systemctl.
This commit is contained in:
1 parent
e2c40f7a78
commit
f9f52e99f0
4 files changed
+130
-6
No files matched your search
@@ -109,7 +109,7 @@ docsgpt up --native \
|
||||
--redis-url redis://localhost:6379
|
||||
```
|
||||
|
||||
Without a terminal both flags are required; with one, it asks for them and for the model provider. It writes the same `.env` a Docker install uses (minus the image settings), generates `INTERNAL_KEY` and `JWT_SECRET_KEY` on the first run, applies the migrations, then starts `docsgpt-api` and `docsgpt-worker` and waits for the API to answer.
|
||||
Without a terminal only `--postgres-uri` is required, since Redis defaults to `redis://localhost:6379`; with one, it asks for both and for the model provider. It writes the same `.env` a Docker install uses (minus the image settings), generates `INTERNAL_KEY` and `JWT_SECRET_KEY` on the first run, applies the migrations, then starts `docsgpt-api` and `docsgpt-worker` and waits for the API to answer.
|
||||
|
||||
One Redis URL covers all three uses: the Celery broker, its result backend and the cache go on databases 0, 1 and 2 of it. Name a database in the URL and the three start there instead, so `redis://localhost:6379/5` puts them on 5, 6 and 7 — that is how you share a Redis that already holds something else.
|
||||
|
||||
@@ -125,7 +125,7 @@ The same commands manage it:
|
||||
`uninstall` never touches the database or Redis: they were yours to begin with.
|
||||
|
||||
<Callout type="info">
|
||||
Windows has neither launchd nor systemd, so native mode is macOS and Linux only. On Windows, run DocsGPT on Docker with `docsgpt up`, or start `docsgpt api` and `docsgpt worker` yourself.
|
||||
Windows has neither launchd nor systemd, so native mode is macOS and Linux only. On Windows, run DocsGPT on Docker with `docsgpt up`, or start `docsgpt api`, `docsgpt worker` and `docsgpt beat` yourself — the worker cannot run the scheduler in-process there, so `docsgpt beat` has to run alongside it for scheduled tasks to fire.
|
||||
</Callout>
|
||||
|
||||
## Upgrade
|
||||
|
||||
@@ -218,6 +218,17 @@ def _redis_urls(base: str) -> dict[str, str]:
|
||||
}
|
||||
|
||||
|
||||
def _own_executable() -> str:
|
||||
"""The docsgpt command to put in the service files, or a DeployError naming what to install."""
|
||||
candidate = Path(sys.argv[0]).resolve()
|
||||
if candidate.is_file() and os.access(candidate, os.X_OK):
|
||||
return str(candidate)
|
||||
raise DeployError(
|
||||
"the docsgpt command is not on PATH, so the services would have nothing to run. "
|
||||
"Install it with `uv tool install docsgpt` (or `pip install docsgpt`) and run this again."
|
||||
)
|
||||
|
||||
|
||||
def _native_up(args, context: Context, directory: Path) -> int:
|
||||
"""Run the API and the worker as services on this machine, against an existing Postgres and Redis."""
|
||||
services = context.service_manager()
|
||||
@@ -263,7 +274,7 @@ def _native_up(args, context: Context, directory: Path) -> int:
|
||||
(directory / "logs").mkdir(exist_ok=True)
|
||||
envfile.update(env_path, updates)
|
||||
|
||||
executable = shutil.which("docsgpt") or sys.argv[0]
|
||||
executable = shutil.which("docsgpt") or _own_executable()
|
||||
print("Applying database migrations ...")
|
||||
# The child reads the stack's settings, not a .env in whatever directory this was run from.
|
||||
stack_env = {"DOCSGPT_HOME": str(directory), "DOCSGPT_ENV_FILE": str(env_path)}
|
||||
@@ -303,6 +314,13 @@ def up(args, context: Optional[Context] = None) -> int:
|
||||
context = context or Context.default(args)
|
||||
directory = stack.stack_dir(args.dir)
|
||||
if getattr(args, "native", False) or _mode(directory) == "native":
|
||||
if _mode(directory) != "native" and (directory / stack.COMPOSE_FILE).is_file():
|
||||
raise DeployError(
|
||||
f"{directory} holds a Docker install. Native services would run beside its containers "
|
||||
"on the same port, and down, status and uninstall would stop seeing them. Stop it "
|
||||
"first with `docsgpt down` (and `docsgpt uninstall` to remove it), or pass --dir to "
|
||||
"put the native install somewhere else."
|
||||
)
|
||||
return _native_up(args, context, directory)
|
||||
env_path = directory / ".env"
|
||||
record_path = directory / stack.RECORD_FILE
|
||||
|
||||
@@ -152,8 +152,12 @@ class SystemdServices:
|
||||
|
||||
def __init__(self, runner=subprocess.run, home: Optional[Path] = None) -> None:
|
||||
self._run = runner
|
||||
base = os.environ.get("XDG_CONFIG_HOME")
|
||||
root = Path(base) if base else (home or Path.home()) / ".config"
|
||||
# An explicit home wins: it is what a caller passes to redirect the unit directory.
|
||||
if home is not None:
|
||||
root = home / ".config"
|
||||
else:
|
||||
base = os.environ.get("XDG_CONFIG_HOME")
|
||||
root = Path(base) if base else Path.home() / ".config"
|
||||
self.directory = root / "systemd" / "user"
|
||||
|
||||
def _unit_file(self, name: str) -> Path:
|
||||
@@ -171,7 +175,10 @@ class SystemdServices:
|
||||
self._systemctl("daemon-reload")
|
||||
|
||||
def start(self, name: str) -> None:
|
||||
self._systemctl("enable", "--now", f"{name}.service", check=True)
|
||||
# restart, not `enable --now`: --now starts nothing when the unit is already active, so a
|
||||
# reinstalled unit would keep running with the ExecStart and environment it started with.
|
||||
self._systemctl("enable", f"{name}.service", check=True)
|
||||
self._systemctl("restart", f"{name}.service", check=True)
|
||||
|
||||
def stop(self, name: str) -> None:
|
||||
self._systemctl("stop", f"{name}.service")
|
||||
|
||||
@@ -2,6 +2,7 @@
|
||||
|
||||
import json
|
||||
import plistlib
|
||||
import sys
|
||||
import types
|
||||
|
||||
import pytest
|
||||
@@ -131,6 +132,21 @@ class TestNativeUp:
|
||||
assert env["API_URL"] == "http://127.0.0.1:7099"
|
||||
assert services.units["docsgpt-api"].arguments[-1] == "7099"
|
||||
|
||||
def test_it_refuses_to_run_beside_a_docker_install(self, tmp_path):
|
||||
"""Native services on the same port would orphan the containers from down/status/uninstall."""
|
||||
assert _run(["up", "--yes", "--dir", str(tmp_path)], _context()) == 0
|
||||
argv = ["up", "--native", "--dir", str(tmp_path), "--yes", "--postgres-uri", "postgresql://localhost/d"]
|
||||
with pytest.raises(DeployError, match="holds a Docker install"):
|
||||
_run(argv, _native_context())
|
||||
|
||||
def test_services_are_not_written_without_a_docsgpt_command(self, tmp_path, monkeypatch):
|
||||
"""A unit pointing at something unexecutable would fail to exec with only a health check to show it."""
|
||||
monkeypatch.setattr("docsgpt.deploy.commands.shutil.which", lambda name: None)
|
||||
monkeypatch.setattr(sys, "argv", [str(tmp_path / "not-a-program")])
|
||||
argv = ["up", "--native", "--dir", str(tmp_path), "--yes", "--postgres-uri", "postgresql://localhost/d"]
|
||||
with pytest.raises(DeployError, match="not on PATH"):
|
||||
_run(argv, _native_context())
|
||||
|
||||
def test_a_database_url_is_required(self, tmp_path):
|
||||
with pytest.raises(DeployError, match="--postgres-uri"):
|
||||
_run(["up", "--native", "--dir", str(tmp_path), "--yes"], _native_context())
|
||||
@@ -252,6 +268,89 @@ class TestLaunchd:
|
||||
assert self._services(FakeLaunchctl(state="running"), tmp_path).is_running("docsgpt-api") is True
|
||||
|
||||
|
||||
class FakeSystemctl:
|
||||
"""systemctl, recording what it was asked to do."""
|
||||
|
||||
def __init__(self, active=True):
|
||||
self.calls = []
|
||||
self.active = active
|
||||
|
||||
def __call__(self, args, capture_output=False, text=False, check=False):
|
||||
self.calls.append(args)
|
||||
code = 0 if (args[2] != "is-active" or self.active) else 3
|
||||
return types.SimpleNamespace(returncode=code, stdout="", stderr="")
|
||||
|
||||
@property
|
||||
def verbs(self):
|
||||
return [call[2] for call in self.calls]
|
||||
|
||||
|
||||
class TestSystemd:
|
||||
def _services(self, systemctl, tmp_path):
|
||||
return native.SystemdServices(runner=systemctl, home=tmp_path)
|
||||
|
||||
def test_install_writes_the_unit_and_reloads(self, tmp_path):
|
||||
systemctl = FakeSystemctl()
|
||||
services = self._services(systemctl, tmp_path)
|
||||
services.install(native.units_for(tmp_path, "/venv/bin/docsgpt", 7091, tmp_path)[0])
|
||||
unit = tmp_path / ".config" / "systemd" / "user" / "docsgpt-api.service"
|
||||
assert "ExecStart=/venv/bin/docsgpt api" in unit.read_text()
|
||||
assert systemctl.verbs == ["daemon-reload"]
|
||||
|
||||
def test_start_restarts_so_a_rewritten_unit_takes_effect(self, tmp_path):
|
||||
"""`enable --now` starts nothing when the unit is already active: it would keep the old ExecStart."""
|
||||
systemctl = FakeSystemctl()
|
||||
self._services(systemctl, tmp_path).start("docsgpt-api")
|
||||
assert "restart" in systemctl.verbs, systemctl.verbs
|
||||
assert systemctl.verbs.index("enable") < systemctl.verbs.index("restart")
|
||||
assert "--now" not in [argument for call in systemctl.calls for argument in call]
|
||||
|
||||
def test_stop_and_remove(self, tmp_path):
|
||||
systemctl = FakeSystemctl()
|
||||
services = self._services(systemctl, tmp_path)
|
||||
services.install(native.units_for(tmp_path, "/venv/bin/docsgpt", 7091, tmp_path)[0])
|
||||
unit = tmp_path / ".config" / "systemd" / "user" / "docsgpt-api.service"
|
||||
services.stop("docsgpt-api")
|
||||
assert unit.is_file(), "stopping keeps the unit"
|
||||
services.remove("docsgpt-api")
|
||||
assert not unit.exists()
|
||||
assert systemctl.verbs[-1] == "daemon-reload", "systemd is told the unit is gone"
|
||||
|
||||
def test_is_running_asks_systemd(self, tmp_path):
|
||||
assert self._services(FakeSystemctl(active=True), tmp_path).is_running("docsgpt-api") is True
|
||||
assert self._services(FakeSystemctl(active=False), tmp_path).is_running("docsgpt-api") is False
|
||||
|
||||
def test_an_explicit_home_wins_over_xdg_config_home(self, tmp_path, monkeypatch):
|
||||
"""home is how a caller redirects the unit directory; the environment must not override it."""
|
||||
monkeypatch.setenv("XDG_CONFIG_HOME", str(tmp_path / "xdg"))
|
||||
services = native.SystemdServices(runner=FakeSystemctl(), home=tmp_path)
|
||||
assert services.directory == tmp_path / ".config" / "systemd" / "user"
|
||||
|
||||
def test_xdg_config_home_is_used_when_no_home_is_given(self, tmp_path, monkeypatch):
|
||||
monkeypatch.setenv("XDG_CONFIG_HOME", str(tmp_path / "xdg"))
|
||||
services = native.SystemdServices(runner=FakeSystemctl())
|
||||
assert services.directory == tmp_path / "xdg" / "systemd" / "user"
|
||||
|
||||
def test_a_failing_systemctl_is_reported(self, tmp_path):
|
||||
class Failing(FakeSystemctl):
|
||||
def __call__(self, args, **kwargs):
|
||||
self.calls.append(args)
|
||||
return types.SimpleNamespace(returncode=1, stdout="", stderr="Failed to enable unit")
|
||||
|
||||
with pytest.raises(DeployError, match="Failed to enable unit"):
|
||||
self._services(Failing(), tmp_path).start("docsgpt-api")
|
||||
|
||||
|
||||
class TestPlatformChoice:
|
||||
def test_each_platform_gets_its_service_manager(self):
|
||||
assert isinstance(native.services_for_platform("darwin"), native.LaunchdServices)
|
||||
assert isinstance(native.services_for_platform("linux"), native.SystemdServices)
|
||||
|
||||
def test_windows_says_what_to_do_instead(self):
|
||||
with pytest.raises(DeployError, match="Windows"):
|
||||
native.services_for_platform("win32")
|
||||
|
||||
|
||||
class TestUnitFiles:
|
||||
def _unit(self, tmp_path):
|
||||
return native.Unit(
|
||||
|
||||
Reference in new issue
Block a user