mirror of
https://github.com/tiennm99/DocsGPT.git
synced 2026-10-05 12:13:55 +00:00
fix: docsgpt up removes Caddy when leaving a domain; clearer Docker permission error
Caddy sits behind the https profile, so once COMPOSE_PROFILES no longer enables it, `up --remove-orphans` left it running on ports 80 and 443 and `down -v` left its volumes. `up` now removes Caddy when an install moves off its domain, and down and uninstall name the profile explicitly. A user outside the docker group was told Docker is not running; the error now says how to get access to the socket.
This commit is contained in:
1 parent
68d9772f2e
commit
0bd0afc1f8
4 files changed
+52
-14
No files matched your search
@@ -23,6 +23,9 @@ PROJECT = "docsgpt"
|
||||
DATABASE_VOLUME = f"{PROJECT}_postgres_data"
|
||||
_MOVING_TAGS = ("latest", "develop")
|
||||
_KEY = re.compile(r"^[A-Za-z_][A-Za-z0-9_]*$")
|
||||
# Commands that stop or remove services name every profile, so Caddy goes too
|
||||
# even when COMPOSE_PROFILES no longer enables it.
|
||||
_EVERY_PROFILE = ("--profile", "https")
|
||||
|
||||
EXPOSURE_CHOICES = [
|
||||
("local", "Only this computer"),
|
||||
@@ -208,6 +211,9 @@ def up(args, context: Optional[Context] = None) -> int:
|
||||
envfile.update(env_path, updates)
|
||||
env = envfile.read(env_path)
|
||||
|
||||
if stack.exposure(existing) == "domain" and stack.exposure(env) != "domain":
|
||||
# With the https profile off, `up --remove-orphans` would leave Caddy running on ports 80 and 443.
|
||||
context.docker.compose(directory, *_EVERY_PROFILE, "rm", "--stop", "--force", "caddy", check=False)
|
||||
up_args = ["up", "-d", "--remove-orphans"]
|
||||
if image_tag in _MOVING_TAGS:
|
||||
up_args += ["--pull", "always"]
|
||||
@@ -252,7 +258,7 @@ def down(args, context: Optional[Context] = None) -> int:
|
||||
directory = stack.stack_dir(args.dir)
|
||||
if _installed(directory) is None:
|
||||
return 1
|
||||
context.docker.compose(directory, "down")
|
||||
context.docker.compose(directory, *_EVERY_PROFILE, "down")
|
||||
return 0
|
||||
|
||||
|
||||
@@ -369,7 +375,7 @@ def uninstall(args, context: Optional[Context] = None) -> int:
|
||||
if not context.prompter.confirm(f"Remove the DocsGPT {what} in {directory}?", default=False):
|
||||
print("Nothing removed.")
|
||||
return 1
|
||||
context.docker.compose(directory, "down", "--remove-orphans", *(["-v"] if args.purge else []))
|
||||
context.docker.compose(directory, *_EVERY_PROFILE, "down", "--remove-orphans", *(["-v"] if args.purge else []))
|
||||
if args.purge:
|
||||
shutil.rmtree(directory)
|
||||
print(f"Removed DocsGPT and its data from {directory}.")
|
||||
|
||||
@@ -59,7 +59,13 @@ class Docker:
|
||||
"""Make sure Docker is installed and running and Compose is new enough, starting Docker Desktop on macOS."""
|
||||
if not self._which("docker"):
|
||||
raise DeployError("Docker is not installed. Get it from https://docs.docker.com/get-docker/ and run this again.")
|
||||
if not self.daemon_running():
|
||||
info = self._run(["docker", "info"], capture=True, check=False)
|
||||
if info.returncode != 0:
|
||||
if "permission denied" in (info.stderr or "").lower():
|
||||
raise DeployError(
|
||||
"Your user cannot use Docker (permission denied on its socket). Add it to the docker group "
|
||||
"with `sudo usermod -aG docker $USER`, log out and back in, and run this again."
|
||||
)
|
||||
self._start_daemon()
|
||||
result = self._run(["docker", "compose", "version", "--short"], capture=True, check=False)
|
||||
version = _parse_version(result.stdout) if result.returncode == 0 else None
|
||||
|
||||
@@ -11,6 +11,8 @@ from docsgpt import cli
|
||||
from docsgpt.deploy import commands, envfile, stack
|
||||
from docsgpt.deploy.docker import DeployError
|
||||
|
||||
EVERY_PROFILE = ["--profile", "https"]
|
||||
|
||||
|
||||
class FakeDocker:
|
||||
def __init__(self, volumes=(), project_dirs=()):
|
||||
@@ -24,7 +26,7 @@ class FakeDocker:
|
||||
|
||||
def compose(self, directory, *args, capture=False, check=True):
|
||||
self.calls.append((Path(directory), list(args)))
|
||||
if args and args[0] == "down" and "-v" in args:
|
||||
if "down" in args and "-v" in args:
|
||||
self.volumes.clear()
|
||||
return subprocess.CompletedProcess(["docker", "compose", *args], 0, stdout="", stderr="")
|
||||
|
||||
@@ -88,7 +90,7 @@ class TestUpFirstInstall:
|
||||
assert env["LLM_PROVIDER"] == "docsgpt"
|
||||
assert env["INTERNAL_KEY"] and env["JWT_SECRET_KEY"] and env["POSTGRES_PASSWORD"]
|
||||
assert docker.preflights == 1
|
||||
assert (tmp_path, ["up", "-d", "--remove-orphans"]) in docker.calls
|
||||
assert docker.calls == [(tmp_path, ["up", "-d", "--remove-orphans"])]
|
||||
record = json.loads((tmp_path / "install.json").read_text())
|
||||
assert record["version"] == "0.21.0"
|
||||
assert "http://localhost:7091" in capsys.readouterr().out
|
||||
@@ -113,7 +115,8 @@ class TestUpFirstInstall:
|
||||
assert env["DOCSGPT_DOMAIN"] == "docs.example.com"
|
||||
assert env["LLM_PROVIDER"] == "openai"
|
||||
|
||||
def test_a_missing_api_key_is_an_error_without_a_terminal(self, tmp_path):
|
||||
def test_a_missing_api_key_is_an_error_without_a_terminal(self, tmp_path, monkeypatch):
|
||||
monkeypatch.delenv("DOCSGPT_API_KEY", raising=False)
|
||||
with pytest.raises(DeployError, match="API key"):
|
||||
_run(["up", "--yes", "--dir", str(tmp_path), "--provider", "openai"], _context())
|
||||
|
||||
@@ -159,6 +162,22 @@ class TestUpAgain:
|
||||
assert after[key] == before[key]
|
||||
assert context.prompter.questions == [], "a configured install is not asked again"
|
||||
|
||||
def test_leaving_the_domain_removes_caddy_before_starting(self, tmp_path):
|
||||
"""With the https profile off, `up --remove-orphans` alone would leave Caddy on ports 80 and 443."""
|
||||
assert _run(["up", "--yes", "--dir", str(tmp_path), "--domain", "docs.example.com"], _context()) == 0
|
||||
docker = FakeDocker(volumes={"docsgpt_postgres_data"})
|
||||
assert _run(["up", "--yes", "--dir", str(tmp_path), "--expose", "local"], _context(docker)) == 0
|
||||
assert docker.calls == [
|
||||
(tmp_path, [*EVERY_PROFILE, "rm", "--stop", "--force", "caddy"]),
|
||||
(tmp_path, ["up", "-d", "--remove-orphans"]),
|
||||
]
|
||||
|
||||
def test_staying_on_the_domain_leaves_caddy_alone(self, tmp_path):
|
||||
assert _run(["up", "--yes", "--dir", str(tmp_path), "--domain", "docs.example.com"], _context()) == 0
|
||||
docker = FakeDocker(volumes={"docsgpt_postgres_data"})
|
||||
assert _run(["up", "--yes", "--dir", str(tmp_path)], _context(docker)) == 0
|
||||
assert docker.calls == [(tmp_path, ["up", "-d", "--remove-orphans"])]
|
||||
|
||||
def test_another_projects_containers_need_consent(self, tmp_path):
|
||||
docker = FakeDocker(project_dirs={"/srv/old-docsgpt"})
|
||||
with pytest.raises(DeployError, match="--adopt"):
|
||||
@@ -177,11 +196,11 @@ class TestManage:
|
||||
def _installed(tmp_path, *extra):
|
||||
assert _run(["up", "--yes", "--dir", str(tmp_path), *extra], _context()) == 0
|
||||
|
||||
def test_down(self, tmp_path):
|
||||
def test_down_includes_caddy(self, tmp_path):
|
||||
self._installed(tmp_path)
|
||||
docker = FakeDocker()
|
||||
assert _run(["down", "--dir", str(tmp_path)], _context(docker)) == 0
|
||||
assert docker.calls == [(tmp_path, ["down"])]
|
||||
assert docker.calls == [(tmp_path, [*EVERY_PROFILE, "down"])]
|
||||
|
||||
def test_commands_on_a_missing_install(self, tmp_path, capsys):
|
||||
assert _run(["status", "--dir", str(tmp_path)], _context()) == 1
|
||||
@@ -228,7 +247,7 @@ class TestManage:
|
||||
self._installed(tmp_path)
|
||||
docker = FakeDocker()
|
||||
assert _run(["uninstall", "--yes", "--dir", str(tmp_path)], _context(docker, installer=lambda: "uv")) == 0
|
||||
assert docker.calls == [(tmp_path, ["down", "--remove-orphans"])]
|
||||
assert docker.calls == [(tmp_path, [*EVERY_PROFILE, "down", "--remove-orphans"])]
|
||||
assert (tmp_path / ".env").is_file(), "the database password lives there"
|
||||
assert not (tmp_path / "docker-compose.yaml").exists()
|
||||
assert not (tmp_path / "install.json").exists()
|
||||
@@ -239,7 +258,7 @@ class TestManage:
|
||||
self._installed(directory)
|
||||
docker = FakeDocker()
|
||||
assert _run(["uninstall", "--yes", "--purge", "--dir", str(directory)], _context(docker)) == 0
|
||||
assert docker.calls == [(directory, ["down", "--remove-orphans", "-v"])]
|
||||
assert docker.calls == [(directory, [*EVERY_PROFILE, "down", "--remove-orphans", "-v"])]
|
||||
assert not directory.exists()
|
||||
|
||||
def test_uninstall_asks_first(self, tmp_path):
|
||||
|
||||
@@ -10,7 +10,7 @@ from docsgpt.deploy.docker import DeployError, Docker
|
||||
|
||||
|
||||
class FakeRunner:
|
||||
"""Answers docker commands from a table of (argv prefix -> result) and records every call."""
|
||||
"""Answers docker commands from a table of argv prefix -> (code, stdout[, stderr]) and records every call."""
|
||||
|
||||
def __init__(self, answers=None):
|
||||
self.answers = list((answers or {}).items())
|
||||
@@ -22,10 +22,10 @@ class FakeRunner:
|
||||
if list(args[: len(prefix)]) == list(prefix):
|
||||
if callable(answer):
|
||||
answer = answer()
|
||||
code, out = answer
|
||||
code, out, err = (*answer, "")[:3]
|
||||
if check and code != 0:
|
||||
raise DeployError(f"{' '.join(args)} failed")
|
||||
return subprocess.CompletedProcess(args, code, stdout=out, stderr="")
|
||||
return subprocess.CompletedProcess(args, code, stdout=out, stderr=err)
|
||||
return subprocess.CompletedProcess(args, 0, stdout="", stderr="")
|
||||
|
||||
|
||||
@@ -43,6 +43,13 @@ class TestPreflight:
|
||||
with pytest.raises(DeployError, match="systemctl start docker"):
|
||||
_docker(runner).preflight()
|
||||
|
||||
def test_no_permission_on_the_socket_says_how_to_get_it(self):
|
||||
"""A user outside the docker group is not told that Docker is down."""
|
||||
denied = "permission denied while trying to connect to the Docker daemon socket at unix:///var/run/docker.sock"
|
||||
runner = FakeRunner({("docker", "info"): (1, "", denied)})
|
||||
with pytest.raises(DeployError, match="docker group"):
|
||||
_docker(runner).preflight()
|
||||
|
||||
def test_daemon_down_on_macos_starts_docker_desktop(self):
|
||||
state = {"started": False}
|
||||
|
||||
@@ -99,7 +106,7 @@ class TestQueries:
|
||||
|
||||
|
||||
class TestWaitHealthy:
|
||||
def test_succeeds_once_the_api_answers(self, monkeypatch):
|
||||
def test_succeeds_once_the_api_answers(self):
|
||||
attempts = {"n": 0}
|
||||
|
||||
def opener(url, timeout):
|
||||
|
||||
Reference in new issue
Block a user