feat(cleanup): walk every backend, reap smolmachines orphans too
`./cli.py cleanup` previously called only the env-var-selected backend's `prepare_cleanup` / `cleanup` — so a leftover smolvm machine + bundle container + bundle network from a crashed smolmachines bottle would survive a default `docker`-mode cleanup indefinitely. Smolmachines now has a real `cleanup` module (alongside `enumerate.py` from issue #77) that walks: - smolvm machines named `claude-bottle-*` (via `smolvm machine ls --json`) - bundle containers `claude-bottle-sidecars-*` - bundle networks `claude-bottle-bundle-*` Cleanup runs stop+delete on the machines, force-rm on the containers, network rm on the networks. Each step is best-effort so a failed rm doesn't block the rest. `cli.py cleanup` walks every backend in `known_backend_names()` and runs each backend's `cleanup` after a single y/N prompt that shows a combined plan. State dirs (`~/.claude-bottle/state/<slug>/`) are shared layout with the docker backend, which still owns the orphan-state-dir bucket. It now consults `enumerate_active_bottles()` for the cross-backend live identity set so a running smolmachines bottle's state dir isn't reaped during a cleanup. Tests: smolmachines cleanup (prepare + cleanup ordering + failure handling); cross-backend orphan protection on the docker state-dir check; CLI cmd_cleanup walks both backends, short- circuits on all-empty, aborts on N. 617 unit tests pass. End-to-end verified on this host: $ smolvm machine ls --json | jq '.[].name' "claude-bottle-researcher-m3hxd" $ ./cli.py cleanup --- smolmachines backend --- smolvm machine: claude-bottle-researcher-m3hxd remove all of the above? [y/N] Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit was merged in pull request #79.
This commit is contained in:
@@ -0,0 +1,105 @@
|
||||
"""Unit: `cli.py cleanup` walks every backend (issue follow-up).
|
||||
|
||||
Asserts cmd_cleanup queries each backend's `prepare_cleanup`,
|
||||
combines the y/N output, and runs each backend's `cleanup` when
|
||||
the operator confirms. Mocks the backends and stdin."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import sys
|
||||
import unittest
|
||||
from unittest.mock import patch, MagicMock
|
||||
|
||||
from claude_bottle.cli import cleanup as cmd
|
||||
|
||||
|
||||
def _make_backend(empty: bool = True):
|
||||
backend = MagicMock()
|
||||
plan = MagicMock(empty=empty)
|
||||
backend.prepare_cleanup.return_value = plan
|
||||
backend.cleanup = MagicMock()
|
||||
return backend, plan
|
||||
|
||||
|
||||
class TestCmdCleanup(unittest.TestCase):
|
||||
def test_iterates_every_backend(self):
|
||||
docker, docker_plan = _make_backend(empty=False)
|
||||
smol, smol_plan = _make_backend(empty=False)
|
||||
backends_by_name = {"docker": docker, "smolmachines": smol}
|
||||
|
||||
with patch.object(
|
||||
cmd, "known_backend_names",
|
||||
return_value=("docker", "smolmachines"),
|
||||
), patch.object(
|
||||
cmd, "get_bottle_backend",
|
||||
side_effect=lambda name: backends_by_name[name],
|
||||
), patch.object(
|
||||
cmd, "_prompt_yes", return_value=True,
|
||||
):
|
||||
self.assertEqual(0, cmd.cmd_cleanup([]))
|
||||
|
||||
docker.prepare_cleanup.assert_called_once()
|
||||
smol.prepare_cleanup.assert_called_once()
|
||||
docker.cleanup.assert_called_once_with(docker_plan)
|
||||
smol.cleanup.assert_called_once_with(smol_plan)
|
||||
|
||||
def test_short_circuits_when_all_empty(self):
|
||||
docker, _ = _make_backend(empty=True)
|
||||
smol, _ = _make_backend(empty=True)
|
||||
backends_by_name = {"docker": docker, "smolmachines": smol}
|
||||
|
||||
with patch.object(
|
||||
cmd, "known_backend_names",
|
||||
return_value=("docker", "smolmachines"),
|
||||
), patch.object(
|
||||
cmd, "get_bottle_backend",
|
||||
side_effect=lambda name: backends_by_name[name],
|
||||
), patch.object(
|
||||
cmd, "_prompt_yes",
|
||||
) as prompt:
|
||||
self.assertEqual(0, cmd.cmd_cleanup([]))
|
||||
prompt.assert_not_called()
|
||||
docker.cleanup.assert_not_called()
|
||||
smol.cleanup.assert_not_called()
|
||||
|
||||
def test_abort_at_prompt_runs_nothing(self):
|
||||
docker, _ = _make_backend(empty=False)
|
||||
smol, _ = _make_backend(empty=True)
|
||||
backends_by_name = {"docker": docker, "smolmachines": smol}
|
||||
|
||||
with patch.object(
|
||||
cmd, "known_backend_names",
|
||||
return_value=("docker", "smolmachines"),
|
||||
), patch.object(
|
||||
cmd, "get_bottle_backend",
|
||||
side_effect=lambda name: backends_by_name[name],
|
||||
), patch.object(
|
||||
cmd, "_prompt_yes", return_value=False,
|
||||
):
|
||||
self.assertEqual(0, cmd.cmd_cleanup([]))
|
||||
docker.cleanup.assert_not_called()
|
||||
smol.cleanup.assert_not_called()
|
||||
|
||||
def test_skips_empty_plans_when_others_have_work(self):
|
||||
# docker has work, smolmachines doesn't — only docker.cleanup
|
||||
# is called.
|
||||
docker, docker_plan = _make_backend(empty=False)
|
||||
smol, _ = _make_backend(empty=True)
|
||||
backends_by_name = {"docker": docker, "smolmachines": smol}
|
||||
|
||||
with patch.object(
|
||||
cmd, "known_backend_names",
|
||||
return_value=("docker", "smolmachines"),
|
||||
), patch.object(
|
||||
cmd, "get_bottle_backend",
|
||||
side_effect=lambda name: backends_by_name[name],
|
||||
), patch.object(
|
||||
cmd, "_prompt_yes", return_value=True,
|
||||
):
|
||||
cmd.cmd_cleanup([])
|
||||
docker.cleanup.assert_called_once_with(docker_plan)
|
||||
smol.cleanup.assert_not_called()
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
@@ -44,7 +44,7 @@ class TestOrphanStateDirs(_FakeHomeMixin, unittest.TestCase):
|
||||
self._teardown_fake_home()
|
||||
|
||||
def test_no_state_root_returns_empty(self):
|
||||
self.assertEqual([], _list_orphan_state_dirs(set()))
|
||||
self.assertEqual([], _list_orphan_state_dirs(set(), set()))
|
||||
|
||||
def test_state_dir_with_no_live_no_preserve_is_orphan(self):
|
||||
# Just touch the dir; no metadata, no preserve marker — the
|
||||
@@ -52,7 +52,7 @@ class TestOrphanStateDirs(_FakeHomeMixin, unittest.TestCase):
|
||||
bottle_state.write_per_bottle_dockerfile("solo-aaa", "FROM x\n")
|
||||
self.assertEqual(
|
||||
["solo-aaa"],
|
||||
_list_orphan_state_dirs(set()),
|
||||
_list_orphan_state_dirs(set(), set()),
|
||||
)
|
||||
|
||||
def test_live_project_skips_dir(self):
|
||||
@@ -61,7 +61,7 @@ class TestOrphanStateDirs(_FakeHomeMixin, unittest.TestCase):
|
||||
bottle_state.write_per_bottle_dockerfile("live-bbb", "FROM x\n")
|
||||
self.assertEqual(
|
||||
[],
|
||||
_list_orphan_state_dirs({"claude-bottle-live-bbb"}),
|
||||
_list_orphan_state_dirs({"claude-bottle-live-bbb"}, set()),
|
||||
)
|
||||
|
||||
def test_preserve_marker_skips_dir(self):
|
||||
@@ -71,14 +71,14 @@ class TestOrphanStateDirs(_FakeHomeMixin, unittest.TestCase):
|
||||
bottle_state.mark_preserved("kept-ccc")
|
||||
self.assertEqual(
|
||||
[],
|
||||
_list_orphan_state_dirs(set()),
|
||||
_list_orphan_state_dirs(set(), set()),
|
||||
)
|
||||
|
||||
def test_preserve_overrides_no_live_project(self):
|
||||
# Even without a live project, a preserve marker keeps it.
|
||||
bottle_state.write_per_bottle_dockerfile("kept-ddd", "FROM x\n")
|
||||
bottle_state.mark_preserved("kept-ddd")
|
||||
self.assertEqual([], _list_orphan_state_dirs(set()))
|
||||
self.assertEqual([], _list_orphan_state_dirs(set(), set()))
|
||||
|
||||
def test_mixed_set_categorized_correctly(self):
|
||||
bottle_state.write_per_bottle_dockerfile("orphan-eee", "FROM x\n")
|
||||
@@ -86,7 +86,7 @@ class TestOrphanStateDirs(_FakeHomeMixin, unittest.TestCase):
|
||||
bottle_state.write_per_bottle_dockerfile("kept-ggg", "FROM z\n")
|
||||
bottle_state.mark_preserved("kept-ggg")
|
||||
|
||||
result = _list_orphan_state_dirs({"claude-bottle-live-fff"})
|
||||
result = _list_orphan_state_dirs({"claude-bottle-live-fff"}, set())
|
||||
self.assertEqual(["orphan-eee"], result)
|
||||
|
||||
def test_sorted_output(self):
|
||||
@@ -94,7 +94,31 @@ class TestOrphanStateDirs(_FakeHomeMixin, unittest.TestCase):
|
||||
bottle_state.write_per_bottle_dockerfile(name, "FROM x\n")
|
||||
self.assertEqual(
|
||||
["aaa-1", "mmm-1", "zzz-1"],
|
||||
_list_orphan_state_dirs(set()),
|
||||
_list_orphan_state_dirs(set(), set()),
|
||||
)
|
||||
|
||||
def test_protected_identity_skips_dir(self):
|
||||
# `protected_identities` carries slugs that are live in
|
||||
# any backend (smolmachines included). docker's orphan
|
||||
# detection respects them so a running smolmachines
|
||||
# bottle's state dir isn't reaped while the VM is up.
|
||||
bottle_state.write_per_bottle_dockerfile("smol-hhh", "FROM x\n")
|
||||
self.assertEqual(
|
||||
[],
|
||||
_list_orphan_state_dirs(set(), {"smol-hhh"}),
|
||||
)
|
||||
|
||||
def test_protected_overrides_no_live_project(self):
|
||||
# A smolmachines bottle has no docker compose project but
|
||||
# IS in the protected set; the absence of a project
|
||||
# shouldn't cause a reap.
|
||||
bottle_state.write_per_bottle_dockerfile("smol-iii", "FROM x\n")
|
||||
self.assertEqual(
|
||||
[],
|
||||
_list_orphan_state_dirs(
|
||||
{"claude-bottle-something-else"}, # different project up
|
||||
{"smol-iii"}, # but smol-iii is live
|
||||
),
|
||||
)
|
||||
|
||||
|
||||
|
||||
@@ -0,0 +1,148 @@
|
||||
"""Unit: smolmachines backend cleanup (`cleanup.py` +
|
||||
`bottle_cleanup_plan.py`).
|
||||
|
||||
Tests mock `subprocess.run` + `has_backend` so they execute
|
||||
without docker / smolvm on PATH. Each cleanup step verifies argv
|
||||
shape; teardown verifies order (machines → bundles → networks)."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import subprocess
|
||||
import unittest
|
||||
from unittest.mock import patch
|
||||
|
||||
from claude_bottle import backend as backend_mod
|
||||
from claude_bottle.backend.smolmachines import cleanup
|
||||
from claude_bottle.backend.smolmachines.bottle_cleanup_plan import (
|
||||
SmolmachinesBottleCleanupPlan,
|
||||
)
|
||||
|
||||
|
||||
def _ok(stdout: str = "", stderr: str = "") -> subprocess.CompletedProcess:
|
||||
return subprocess.CompletedProcess(
|
||||
args=[], returncode=0, stdout=stdout, stderr=stderr,
|
||||
)
|
||||
|
||||
|
||||
class TestPrepareCleanup(unittest.TestCase):
|
||||
def test_empty_when_nothing_running(self):
|
||||
with patch.object(cleanup, "_smolvm") as smolvm, \
|
||||
patch.object(cleanup.subprocess, "run") as run, \
|
||||
patch.object(backend_mod, "has_backend", return_value=True):
|
||||
smolvm.is_available.return_value = True
|
||||
run.return_value = _ok(stdout="[]")
|
||||
plan = cleanup.prepare_cleanup()
|
||||
self.assertTrue(plan.empty)
|
||||
|
||||
def test_lists_machines_bundles_networks(self):
|
||||
def fake_run(argv, *a, **kw):
|
||||
if argv[:3] == ["smolvm", "machine", "ls"]:
|
||||
return _ok(stdout=(
|
||||
'[{"name":"claude-bottle-a-1","state":"running"},'
|
||||
' {"name":"claude-bottle-b-2","state":"created"},'
|
||||
' {"name":"unrelated","state":"running"}]'
|
||||
))
|
||||
if argv[:2] == ["docker", "ps"]:
|
||||
return _ok(stdout=(
|
||||
"claude-bottle-sidecars-a-1\n"
|
||||
"claude-bottle-sidecars-b-2\n"
|
||||
))
|
||||
if argv[:3] == ["docker", "network", "ls"]:
|
||||
return _ok(stdout=(
|
||||
"claude-bottle-bundle-a-1\n"
|
||||
"claude-bottle-bundle-b-2\n"
|
||||
))
|
||||
return _ok()
|
||||
|
||||
with patch.object(cleanup, "_smolvm") as smolvm, \
|
||||
patch.object(cleanup.subprocess, "run", side_effect=fake_run), \
|
||||
patch.object(backend_mod, "has_backend", return_value=True):
|
||||
smolvm.is_available.return_value = True
|
||||
plan = cleanup.prepare_cleanup()
|
||||
|
||||
# `unrelated` filtered out (no claude-bottle- prefix).
|
||||
self.assertEqual(
|
||||
("claude-bottle-a-1", "claude-bottle-b-2"),
|
||||
plan.machines,
|
||||
)
|
||||
self.assertEqual(
|
||||
("claude-bottle-sidecars-a-1", "claude-bottle-sidecars-b-2"),
|
||||
plan.bundles,
|
||||
)
|
||||
self.assertEqual(
|
||||
("claude-bottle-bundle-a-1", "claude-bottle-bundle-b-2"),
|
||||
plan.networks,
|
||||
)
|
||||
|
||||
def test_no_smolvm_means_no_machines(self):
|
||||
with patch.object(cleanup, "_smolvm") as smolvm, \
|
||||
patch.object(cleanup.subprocess, "run", return_value=_ok()), \
|
||||
patch.object(backend_mod, "has_backend", return_value=True):
|
||||
smolvm.is_available.return_value = False
|
||||
plan = cleanup.prepare_cleanup()
|
||||
self.assertEqual((), plan.machines)
|
||||
|
||||
|
||||
class TestCleanup(unittest.TestCase):
|
||||
def test_machines_stopped_then_deleted_then_bundles_then_networks(self):
|
||||
plan = SmolmachinesBottleCleanupPlan(
|
||||
machines=("claude-bottle-a-1",),
|
||||
bundles=("claude-bottle-sidecars-a-1",),
|
||||
networks=("claude-bottle-bundle-a-1",),
|
||||
)
|
||||
calls: list[list[str]] = []
|
||||
|
||||
def fake_run(argv, *a, **kw):
|
||||
calls.append(list(argv[:4]))
|
||||
return _ok()
|
||||
|
||||
with patch.object(cleanup.subprocess, "run", side_effect=fake_run):
|
||||
cleanup.cleanup(plan)
|
||||
|
||||
# Stop precedes delete precedes bundle rm precedes network rm.
|
||||
self.assertEqual(
|
||||
["smolvm", "machine", "stop", "--name"], calls[0],
|
||||
)
|
||||
self.assertEqual(
|
||||
["smolvm", "machine", "delete", "-f"], calls[1],
|
||||
)
|
||||
self.assertEqual(
|
||||
["docker", "rm", "-f", "claude-bottle-sidecars-a-1"], calls[2],
|
||||
)
|
||||
self.assertEqual(
|
||||
["docker", "network", "rm", "claude-bottle-bundle-a-1"], calls[3],
|
||||
)
|
||||
|
||||
def test_failures_are_warnings_not_fatal(self):
|
||||
# smolvm machine delete -f returning non-zero should warn
|
||||
# but continue with bundles + networks. The cleanup is
|
||||
# idempotent on success and tries to remove every resource.
|
||||
plan = SmolmachinesBottleCleanupPlan(
|
||||
machines=("claude-bottle-a-1",),
|
||||
bundles=("claude-bottle-sidecars-a-1",),
|
||||
networks=(),
|
||||
)
|
||||
results = iter([
|
||||
_ok(), # stop succeeds
|
||||
subprocess.CompletedProcess(args=[], returncode=1, stdout="", stderr="boom"), # delete fails
|
||||
_ok(), # bundle rm succeeds
|
||||
])
|
||||
|
||||
def fake_run(argv, *a, **kw):
|
||||
return next(results)
|
||||
|
||||
with patch.object(cleanup.subprocess, "run", side_effect=fake_run), \
|
||||
patch.object(cleanup, "warn") as warn:
|
||||
cleanup.cleanup(plan)
|
||||
# warn called once for the delete failure.
|
||||
warn.assert_called_once()
|
||||
|
||||
def test_empty_plan_is_noop(self):
|
||||
plan = SmolmachinesBottleCleanupPlan()
|
||||
with patch.object(cleanup.subprocess, "run") as run:
|
||||
cleanup.cleanup(plan)
|
||||
run.assert_not_called()
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
Reference in New Issue
Block a user