Merge feat/install-4-backup into feat/install-5-native

docsgpt/deploy/backup.py

@@ -113,6 +113,16 @@ def validate(path: Path, manifest: Mapping[str, object]) -> list[str]:
     return list(volumes)
 
 
+def check_volume_tar(name: str, path: Path) -> None:
+    """Read a volume tar through, so a damaged one is found before any volume is replaced."""
+    try:
+        with tarfile.open(path, "r:*") as archive:
+            for _ in archive:
+                pass
+    except (tarfile.TarError, OSError) as exc:
+        raise DeployError(f"the {name} volume in this backup is damaged: {exc}") from exc
+
+
 def extract(path: Path, destination: Path) -> None:
     """Unpack the archive into ``destination`` (data filter: no paths outside it, no devices)."""
     with tarfile.open(path, "r:gz") as archive:

docsgpt/deploy/commands.py

@@ -527,9 +527,9 @@ def backup(args, context: Optional[Context] = None) -> int:
     target = out_dir / backup_format.archive_name(taken_at)
     image = _stack_image(env)
 
-    print("Pausing the backend and the worker so the database and the files match ...")
-    context.docker.compose(directory, "stop", "backend", "worker")
     try:
+        print("Pausing the backend and the worker so the database and the files match ...")
+        context.docker.compose(directory, "stop", "backend", "worker")
         _write_backup(args, context, directory, env, target, image, taken_at)
     finally:
         print("Starting the backend and the worker again ...")
@@ -580,10 +580,16 @@ def _restore_data(context: Context, directory: Path, archive: Path, volumes: lis
     with tempfile.TemporaryDirectory() as workspace:
         work = Path(workspace)
         backup_format.extract(archive, work)
+        # Read every volume tar through before replacing any of them: a damaged third tar must not
+        # be discovered with the first two already swapped in.
+        tars = {}
         for name in volumes:
             tar_path = work / backup_format.volume_member(name)
             if not tar_path.is_file():
                 raise DeployError(f"{archive} is missing the {name} volume it says it contains")
+            backup_format.check_volume_tar(name, tar_path)
+            tars[name] = tar_path
+        for name, tar_path in tars.items():
             print(f"Restoring the {name} volume ...")
             context.docker.import_volume(f"{PROJECT}_{name}", tar_path, image)
 
@@ -625,10 +631,9 @@ def restore(args, context: Optional[Context] = None) -> int:
             return 1
 
     image = _stack_image(env)
-    print("Stopping the stack ...")
-    context.docker.compose(directory, *_EVERY_PROFILE, "down")
-
     try:
+        print("Stopping the stack ...")
+        context.docker.compose(directory, *_EVERY_PROFILE, "down")
         _restore_data(context, directory, archive, volumes, image)
     except BaseException:
         # The stack is down by now. A corrupt payload inside an otherwise well-formed archive, or a

tests/deploy/test_backup.py

@@ -4,6 +4,7 @@ import io
 import json
 import stat
 import tarfile
+from pathlib import Path
 
 import pytest
 
@@ -27,13 +28,16 @@ def _manifest(path):
         return json.loads(archive.extractfile(backup_module.MANIFEST).read())
 
 
-def _rewrite(archive, dest, *, volumes=None, drop=()):
-    """The same archive with other volumes in its manifest, or with members left out."""
+def _rewrite(archive, dest, *, volumes=None, drop=(), corrupt=()):
+    """The same archive with other volumes in its manifest, or with members left out or damaged."""
     with tarfile.open(archive, "r:gz") as source, tarfile.open(dest, "w:gz") as out:
         for member in source.getmembers():
             if member.name in drop:
                 continue
             body = source.extractfile(member).read() if member.isfile() else None
+            if member.name in corrupt:
+                body = body[:60]
+                member.size = len(body)
             if member.name == backup_module.MANIFEST and volumes is not None:
                 manifest = json.loads(body.decode())
                 manifest["volumes"] = volumes
@@ -117,6 +121,22 @@ class TestBackup:
         started = next(index for index, call in enumerate(joined) if call.startswith("up -d backend worker"))
         assert stopped < dumped < started, joined
 
+    def test_the_writers_come_back_even_when_stopping_them_fails(self, tmp_path):
+        """A stop that fails partway must not leave the backend or the worker down."""
+        _installed(tmp_path)
+
+        class FailingStop(FakeDocker):
+            def compose(self, directory, *args, **kwargs):
+                if args[:1] == ("stop",):
+                    self.calls.append((Path(directory), list(args)))
+                    raise DeployError("compose stop exploded")
+                return super().compose(directory, *args, **kwargs)
+
+        docker = FailingStop()
+        with pytest.raises(DeployError, match="compose stop exploded"):
+            _run(["backup", "--dir", str(tmp_path), "--out", str(tmp_path / "b")], _context(docker))
+        assert any(" ".join(args).startswith("up -d backend worker") for _, args in docker.calls), docker.calls
+
     def test_the_writers_come_back_even_when_the_dump_fails(self, tmp_path):
         _installed(tmp_path)
 
@@ -213,6 +233,32 @@ class TestRestore:
         assert any(call.startswith("up -d --remove-orphans") for call in joined), joined
         assert "Starting DocsGPT again" in capsys.readouterr().err
 
+    def test_a_shutdown_that_fails_still_starts_the_stack_again(self, tmp_path):
+        """`compose down` can fail with containers already stopped; the stack must not stay down."""
+        archive = self._backup(tmp_path)
+
+        class FailingDown(FakeDocker):
+            def compose(self, directory, *args, **kwargs):
+                if "down" in args:
+                    self.calls.append((Path(directory), list(args)))
+                    raise DeployError("compose down exploded")
+                return super().compose(directory, *args, **kwargs)
+
+        docker = FailingDown(volumes={"docsgpt_postgres_data"})
+        with pytest.raises(DeployError, match="compose down exploded"):
+            _run(["restore", str(archive), "--dir", str(tmp_path), "--yes"], _context(docker))
+        joined = [" ".join(args) for _, args in docker.calls]
+        assert any(call.startswith("up -d --remove-orphans") for call in joined), joined
+
+    def test_a_damaged_volume_tar_is_found_before_any_volume_is_replaced(self, tmp_path):
+        """vectors sorts last, so a per-volume check would already have swapped indexes and inputs."""
+        archive = _rewrite(self._backup(tmp_path), tmp_path / "bad-volume.tar.gz",
+                           corrupt=(backup_module.volume_member("vectors"),))
+        docker = FakeDocker(volumes={"docsgpt_postgres_data"})
+        with pytest.raises(DeployError, match="damaged"):
+            _run(["restore", str(archive), "--dir", str(tmp_path), "--yes"], _context(docker))
+        assert [op for op in docker.volume_ops if op[0] == "import"] == [], docker.volume_ops
+
     def test_a_newer_backup_is_refused_without_force(self, tmp_path):
         """Restoring a 0.22 backup into 0.21 would hand an older schema newer data."""
         newer = _copy_with_version(self._backup(tmp_path), "0.22.0", tmp_path / "newer.tar.gz")