fix(cleanup): revalidate destructive backend plans
test / image-input-builds (pull_request) Failing after 13m11s
test / unit (pull_request) Has started running
test / coverage (pull_request) Blocked by required conditions
test / integration-docker (pull_request) Blocked by required conditions
tracker-policy-pr / check-pr (pull_request) Successful in 13s
test / image-input-builds (pull_request) Failing after 13m11s
test / unit (pull_request) Has started running
test / coverage (pull_request) Blocked by required conditions
test / integration-docker (pull_request) Blocked by required conditions
tracker-policy-pr / check-pr (pull_request) Successful in 13s
This commit is contained in:
@@ -41,8 +41,8 @@ class TestCmdCleanup(unittest.TestCase):
|
||||
):
|
||||
self.assertEqual(0, cmd.cmd_cleanup([]))
|
||||
|
||||
docker.prepare_cleanup.assert_called_once()
|
||||
fc.prepare_cleanup.assert_called_once()
|
||||
self.assertEqual(2, docker.prepare_cleanup.call_count)
|
||||
self.assertEqual(2, fc.prepare_cleanup.call_count)
|
||||
docker.cleanup.assert_called_once_with(docker_plan)
|
||||
fc.cleanup.assert_called_once_with(fc_plan)
|
||||
|
||||
@@ -68,7 +68,7 @@ class TestCmdCleanup(unittest.TestCase):
|
||||
):
|
||||
self.assertEqual(0, cmd.cmd_cleanup([]))
|
||||
|
||||
docker.prepare_cleanup.assert_called_once()
|
||||
self.assertEqual(2, docker.prepare_cleanup.call_count)
|
||||
docker.cleanup.assert_called_once_with(docker_plan)
|
||||
macos.prepare_cleanup.assert_not_called()
|
||||
|
||||
@@ -135,6 +135,25 @@ class TestCmdCleanup(unittest.TestCase):
|
||||
docker.cleanup.assert_called_once_with(docker_plan)
|
||||
fc.cleanup.assert_not_called()
|
||||
|
||||
def test_executes_refreshed_plan_after_confirmation(self):
|
||||
backend = MagicMock()
|
||||
preview = MagicMock(empty=False)
|
||||
refreshed = MagicMock(empty=False)
|
||||
backend.prepare_cleanup.side_effect = [preview, refreshed]
|
||||
|
||||
with patch.object(
|
||||
cmd, "known_backend_names", return_value=("firecracker",),
|
||||
), patch.object(
|
||||
cmd, "get_bottle_backend", return_value=backend,
|
||||
), patch.object(
|
||||
cmd, "has_backend", return_value=True,
|
||||
), patch.object(
|
||||
cmd, "_prompt_yes", return_value=True,
|
||||
):
|
||||
self.assertEqual(0, cmd.cmd_cleanup([]))
|
||||
|
||||
backend.cleanup.assert_called_once_with(refreshed)
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
|
||||
@@ -132,18 +132,46 @@ class TestCleanupRemoval(unittest.TestCase):
|
||||
vm_pids=(101,),
|
||||
run_dirs=("/run/dev-x",),
|
||||
)
|
||||
with patch.object(fc_cleanup.os, "kill") as kill, \
|
||||
with patch.object(fc_cleanup, "prepare_cleanup", return_value=plan), \
|
||||
patch.object(fc_cleanup, "_run_root", return_value=Path("/run")), \
|
||||
patch.object(fc_cleanup, "_terminate_orphan") as terminate, \
|
||||
patch.object(fc_cleanup.shutil, "rmtree") as rmtree, \
|
||||
patch.object(fc_cleanup, "info"):
|
||||
fc_cleanup.cleanup(plan)
|
||||
kill.assert_called_once()
|
||||
terminate.assert_called_once_with(101, Path("/run"))
|
||||
rmtree.assert_called_once_with("/run/dev-x", ignore_errors=True)
|
||||
|
||||
def test_cleanup_tolerates_dead_pid(self):
|
||||
plan = FirecrackerBottleCleanupPlan(vm_pids=(999,))
|
||||
with patch.object(fc_cleanup.os, "kill", side_effect=ProcessLookupError), \
|
||||
patch.object(fc_cleanup, "info"):
|
||||
fc_cleanup.cleanup(plan) # must not raise
|
||||
def test_cleanup_skips_resources_no_longer_in_refreshed_plan(self):
|
||||
preview = FirecrackerBottleCleanupPlan(
|
||||
vm_pids=(999,), run_dirs=("/run/reused",),
|
||||
)
|
||||
with patch.object(
|
||||
fc_cleanup, "prepare_cleanup",
|
||||
return_value=FirecrackerBottleCleanupPlan(),
|
||||
), patch.object(fc_cleanup, "_terminate_orphan") as terminate, \
|
||||
patch.object(fc_cleanup.shutil, "rmtree") as rmtree:
|
||||
fc_cleanup.cleanup(preview)
|
||||
terminate.assert_not_called()
|
||||
rmtree.assert_not_called()
|
||||
|
||||
def test_pidfd_prevents_pid_reuse_from_signalling_unrelated_process(self):
|
||||
with patch.object(fc_cleanup.os, "pidfd_open", return_value=7), \
|
||||
patch.object(
|
||||
fc_cleanup.Path, "read_bytes",
|
||||
return_value=b"/usr/bin/python\0worker.py\0",
|
||||
), patch.object(fc_cleanup.signal, "pidfd_send_signal") as send, \
|
||||
patch.object(fc_cleanup.os, "close"):
|
||||
fc_cleanup._terminate_orphan(101, Path("/run"))
|
||||
send.assert_not_called()
|
||||
|
||||
def test_pidfd_signals_revalidated_orphan(self):
|
||||
command = b"firecracker\0--config-file\0/run/gone/config.json\0"
|
||||
with patch.object(fc_cleanup.os, "pidfd_open", return_value=7), \
|
||||
patch.object(fc_cleanup.Path, "read_bytes", return_value=command), \
|
||||
patch.object(fc_cleanup.signal, "pidfd_send_signal") as send, \
|
||||
patch.object(fc_cleanup.os, "close"), patch.object(fc_cleanup, "info"):
|
||||
fc_cleanup._terminate_orphan(101, Path("/run"))
|
||||
send.assert_called_once_with(7, fc_cleanup.signal.SIGTERM)
|
||||
|
||||
|
||||
class TestCleanupPlan(unittest.TestCase):
|
||||
|
||||
@@ -42,6 +42,22 @@ class TestMacosContainerCleanup(unittest.TestCase):
|
||||
run.call_args_list[1].args[0],
|
||||
)
|
||||
|
||||
def test_container_enumeration_failure_aborts(self):
|
||||
completed = cleanup.subprocess.CompletedProcess(
|
||||
args=[], returncode=1, stdout="", stderr="service unavailable",
|
||||
)
|
||||
with patch.object(cleanup.subprocess, "run", return_value=completed), \
|
||||
self.assertRaisesRegex(EnumerationError, "service unavailable"):
|
||||
cleanup._list_prefixed_containers()
|
||||
|
||||
def test_network_enumeration_failure_aborts(self):
|
||||
completed = cleanup.subprocess.CompletedProcess(
|
||||
args=[], returncode=1, stdout="", stderr="service unavailable",
|
||||
)
|
||||
with patch.object(cleanup.subprocess, "run", return_value=completed), \
|
||||
self.assertRaisesRegex(EnumerationError, "service unavailable"):
|
||||
cleanup._list_prefixed_networks()
|
||||
|
||||
|
||||
class TestMacosContainerEnumerate(unittest.TestCase):
|
||||
"""The backend launches bottles again (PRD 0070), so enumeration is real
|
||||
|
||||
Reference in New Issue
Block a user