diff --git a/docs/general/cleanup.md b/docs/general/cleanup.md index 851c827539..a4477728c2 100644 --- a/docs/general/cleanup.md +++ b/docs/general/cleanup.md @@ -464,7 +464,11 @@ The script enforces these boundaries: daemon; both plan and apply require it to contain only one `layerfs-daemon` process, with no Exec descendants. Protected containers and binds are refused. The daemon is stopped before recovery capture. A changed identity or newly - active command blocks removal. Recovery images retain changed filesystem layers; + active command blocks removal. The identity is the container's id, creation + time, image, configuration, mounts and execution state, checked at apply and + again after recovery capture. Docker lists mounts in no fixed order, so they + are compared as a set: a reordering alone is not a change, but an added, + removed or altered mount is. Recovery images retain changed filesystem layers; removing a container alone may reclaim little space. For aggressive retirement, an exact action may set `discard_rootfs: true` and list `preserve_paths`, such as `/layerfs-local/config` and `/usr/local/bin/layerfs-daemon`. Those files are diff --git a/tools/cleanup_docker.py b/tools/cleanup_docker.py index 629e470bca..b0b3831a9f 100644 --- a/tools/cleanup_docker.py +++ b/tools/cleanup_docker.py @@ -15,10 +15,26 @@ def inspect(name): return json.loads(run(["docker", "container", "inspect", name]).stdout)[0] +def _mount_key(mount): + fields = tuple(str(mount.get(key) or "") for key in ("Type", "Name", "Source", "Destination")) + return fields + (json.dumps(mount, sort_keys=True),) + + +def normalized(recorded): + # Docker reports Mounts in no fixed order, so compare them as a set. + return recorded | {"Mounts": sorted(recorded.get("Mounts") or [], key=_mount_key)} + + def identity(info): - return {key: info[key] for key in ("Id", "Created", "Image", "Config", "Mounts")} | { - "state": {key: info["State"][key] for key in ("Status", "Running", "StartedAt", "FinishedAt")} - } + return normalized( + {key: info[key] for key in ("Id", "Created", "Image", "Config", "Mounts")} + | {"state": {key: info["State"][key] for key in ("Status", "Running", "StartedAt", "FinishedAt")}} + ) + + +def same(recorded, current): + """Compare identities; also accepts plans recorded before Mounts were sorted.""" + return normalized(recorded) == normalized(current) def guard(info, action, scope): @@ -47,7 +63,7 @@ def guard(info, action, scope): def retire(action, scope, output, pass_id, record_event, repo=None): info = inspect(action["name"]) guard(info, action, scope) - if identity(info) != action["identity"]: + if not same(action["identity"], identity(info)): raise Refused("Container identity or execution state changed since plan") dest = output / ("container-" + info["Id"][:12]) dest.mkdir(mode=0o700) @@ -116,7 +132,7 @@ def retire(action, scope, output, pass_id, record_event, repo=None): record_event(output, state="archived", action=action, recovery=recovery) current = inspect(info["Id"]) guard(current, action, scope) - if identity(current) != stopped: + if not same(stopped, identity(current)): raise Refused("Container changed during recovery capture") record_event(output, state="removing", action=action, recovery=recovery) run(["docker", "rm", info["Id"]]) diff --git a/tools/test_cleanup.py b/tools/test_cleanup.py index 6771dbf5ee..5a9a998c58 100644 --- a/tools/test_cleanup.py +++ b/tools/test_cleanup.py @@ -14,7 +14,7 @@ import cleanup from cleanup_discover import discover -from cleanup_docker import guard as guard_container +from cleanup_docker import guard as guard_container, identity as container_identity, retire, same as same_container from cleanup_retirement import retirement_open_files from cleanup_archive import archive from cleanup_state import Refused, ensure_idle, ensure_volume_idle, git, inventory, same_volume @@ -337,6 +337,66 @@ def test_container_using_pinned_volume_cannot_be_retired(self): with self.assertRaises(Refused): guard_container(info, {}, {"protected_volumes": ["pinned"]}) + def exited_container(self, mounts): + return { + "Id": "exact", + "Name": "/build", + "Created": "created", + "Image": "sha256:image", + "Config": {"Env": ["A=1"]}, + "Mounts": mounts, + "State": {"Status": "exited", "Running": False, "StartedAt": "s", "FinishedAt": "f"}, + } + + def test_container_mount_order_is_not_an_identity_change(self): + cargo = {"Type": "volume", "Name": "cargo", "Source": "/v/cargo", "Destination": "/cargo-home"} + work = {"Type": "bind", "Source": "/src", "Destination": "/work", "RW": False} + target = {"Type": "volume", "Name": "target", "Source": "/v/target", "Destination": "/target"} + planned = self.exited_container([cargo, work, target]) + recorded = container_identity(planned) + # A plan written before Mounts were sorted keeps Docker's order. + legacy = {**recorded, "Mounts": [cargo, work, target]} + current = self.exited_container([target, cargo, work]) + self.assertTrue(same_container(legacy, container_identity(current))) + self.assertEqual(recorded, container_identity(current)) + for identity in (recorded, legacy): + with ( + patch("cleanup_docker.inspect", return_value=current), + patch("cleanup_docker.run", side_effect=RuntimeError("capture reached")) as operation, + ): + # Reaching the first Docker capture command proves the identity check passed. + with self.assertRaisesRegex(RuntimeError, "capture reached"): + retire({"name": "exact", "identity": identity}, {}, self.output, "P", lambda *a, **k: None) + operation.assert_called_once_with(["docker", "diff", "exact"]) + for record in self.output.iterdir(): + for path in record.iterdir(): + path.unlink() + record.rmdir() + + def test_container_mount_set_change_refuses_before_capture(self): + cargo = {"Type": "volume", "Name": "cargo", "Source": "/v/cargo", "Destination": "/cargo-home"} + work = {"Type": "bind", "Source": "/src", "Destination": "/work", "RW": False} + recorded = container_identity(self.exited_container([cargo, work])) + changed = [ + self.exited_container([cargo, {**work, "Source": "/other"}]), + self.exited_container([cargo]), + {**self.exited_container([work, cargo]), "Image": "sha256:other"}, + { + **self.exited_container([work, cargo]), + "State": {**self.exited_container([])["State"], "Status": "created"}, + }, + ] + for current in changed: + self.assertFalse(same_container(recorded, container_identity(current))) + with ( + patch("cleanup_docker.inspect", return_value=current), + patch("cleanup_docker.run") as operation, + ): + with self.assertRaisesRegex(Refused, "identity or execution state changed"): + retire({"name": "exact", "identity": recorded}, {}, self.output, "P", lambda *a, **k: None) + operation.assert_not_called() + self.assertEqual(list(self.output.iterdir()), []) + def test_caches_discovery_keeps_helpers_and_active_profile(self): tree = self.root / "linked" target = tree / "target/linux"