From 9f0d56b75d1d7e3be67b21dccd3b91519698ef3e Mon Sep 17 00:00:00 2001 From: codex Date: Thu, 9 Jul 2026 16:43:48 +0000 Subject: [PATCH] test(smolmachines): cover sidecar vm launch paths --- tests/unit/test_smolmachines_enumerate.py | 66 +++++ tests/unit/test_smolmachines_port_forward.py | 233 +++++++++++++++++- tests/unit/test_smolmachines_provision.py | 109 ++++++++ .../unit/test_smolmachines_sidecar_bundle.py | 31 +++ 4 files changed, 436 insertions(+), 3 deletions(-) create mode 100644 tests/unit/test_smolmachines_enumerate.py diff --git a/tests/unit/test_smolmachines_enumerate.py b/tests/unit/test_smolmachines_enumerate.py new file mode 100644 index 0000000..76487e0 --- /dev/null +++ b/tests/unit/test_smolmachines_enumerate.py @@ -0,0 +1,66 @@ +"""Unit: smolmachines active-agent enumeration.""" + +from __future__ import annotations + +import json +import subprocess +import unittest +from unittest.mock import patch + +from bot_bottle.backend.smolmachines import enumerate as _enumerate + + +def _completed( + stdout: str, + returncode: int = 0, +) -> subprocess.CompletedProcess: # type: ignore[type-arg] + return subprocess.CompletedProcess( + args=[], + returncode=returncode, + stdout=stdout, + stderr="", + ) + + +class TestEnumerateActive(unittest.TestCase): + def test_skips_sidecar_vms(self): + machines = [ + {"name": "bot-bottle-demo-abc12", "state": "running"}, + {"name": "bot-bottle-sidecars-demo-abc12", "state": "running"}, + {"name": "bot-bottle-stopped-xyz", "state": "stopped"}, + {"name": "other", "state": "running"}, + ] + + class _Metadata: + agent_name = "demo" + started_at = "2026-07-09T00:00:00Z" + label = "Demo" + color = "green" + + with patch( + "bot_bottle.backend.smolmachines.enumerate.subprocess.run", + return_value=_completed(json.dumps(machines)), + ), patch( + "bot_bottle.backend.smolmachines.enumerate._query_bundle_services", + return_value={"demo-abc12": ("egress",)}, + ), patch( + "bot_bottle.backend.smolmachines.enumerate.read_metadata", + return_value=_Metadata(), + ) as read_metadata: + active = _enumerate.enumerate_active() + + self.assertEqual(1, len(active)) + self.assertEqual("demo-abc12", active[0].slug) + self.assertEqual(("egress",), active[0].services) + read_metadata.assert_called_once_with("demo-abc12") + + def test_invalid_smolvm_output_returns_empty(self): + with patch( + "bot_bottle.backend.smolmachines.enumerate.subprocess.run", + return_value=_completed("not json"), + ): + self.assertEqual([], _enumerate.enumerate_active()) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/unit/test_smolmachines_port_forward.py b/tests/unit/test_smolmachines_port_forward.py index 8adf2e1..0c41978 100644 --- a/tests/unit/test_smolmachines_port_forward.py +++ b/tests/unit/test_smolmachines_port_forward.py @@ -1,11 +1,19 @@ from __future__ import annotations +import json import socket +import subprocess import threading import unittest +from unittest.mock import MagicMock, patch from bot_bottle.backend.smolmachines.port_forward import ( ForwardSpec, + ForwarderHandle, + _CompletedProcess, + _Forwarder, + _pipe, + main, start_forwarder, stop_forwarder, ) @@ -30,6 +38,13 @@ def _echo_server() -> tuple[socket.socket, int]: class TestForwarderValidation(unittest.TestCase): + def test_empty_specs_returns_noop_handle(self): + handle = start_forwarder(()) + + self.assertEqual((), handle.forwards) + self.assertEqual(0, handle.process.poll()) + stop_forwarder(handle) + def test_rejects_generic_localhost_listener(self): with self.assertRaises(SystemExit): start_forwarder(( @@ -66,20 +81,123 @@ class TestForwarderValidation(unittest.TestCase): ), )) + def test_rejects_invalid_ports(self): + with self.assertRaises(SystemExit): + start_forwarder(( + ForwardSpec("bad-listen", "127.0.0.2", 70000, "127.0.0.1", 9099), + )) + with self.assertRaises(SystemExit): + start_forwarder(( + ForwardSpec("bad-target", "127.0.0.2", 0, "127.0.0.1", 0), + )) + + def test_completed_process_methods_are_noops(self): + proc = _CompletedProcess() + + proc.terminate() + proc.kill() + + self.assertEqual(0, proc.poll()) + self.assertEqual(0, proc.wait(timeout=1)) + + +class _FakeStdout: + def __init__(self, line: str): + self.line = line + self.closed = False + + def readline(self) -> str: + return self.line + + def close(self) -> None: + self.closed = True + + +class TestForwarderProcess(unittest.TestCase): + def test_start_parses_ready_payload(self): + payload = { + "forwards": [{ + "label": "egress", + "listen_host": "127.0.0.2", + "listen_port": 61000, + "target_host": "127.0.0.1", + "target_port": 55000, + }], + } + proc = MagicMock() + proc.stdout = _FakeStdout(json.dumps(payload) + "\n") + + with patch( + "bot_bottle.backend.smolmachines.port_forward.subprocess.Popen", + return_value=proc, + ) as popen: + handle = start_forwarder(( + ForwardSpec("egress", "127.0.0.2", 0, "127.0.0.1", 55000), + )) + + self.assertIs(proc, handle.process) + self.assertEqual(61000, handle.forwards[0].listen_port) + self.assertTrue(proc.stdout.closed) + argv = popen.call_args.args[0] + self.assertIn("--spec-json", argv) + + def test_start_dies_when_helper_exits_before_ready(self): + proc = MagicMock() + proc.stdout = _FakeStdout("") + + with patch( + "bot_bottle.backend.smolmachines.port_forward.subprocess.Popen", + return_value=proc, + ): + with self.assertRaises(SystemExit): + start_forwarder(( + ForwardSpec("egress", "127.0.0.2", 0, "127.0.0.1", 55000), + )) + + proc.terminate.assert_called_once() + proc.wait.assert_called_once_with(timeout=5) + + def test_start_dies_on_invalid_ready_payload(self): + proc = MagicMock() + proc.stdout = _FakeStdout('{"wrong": []}\n') + + with patch( + "bot_bottle.backend.smolmachines.port_forward.subprocess.Popen", + return_value=proc, + ): + with self.assertRaises(SystemExit): + start_forwarder(( + ForwardSpec("egress", "127.0.0.2", 0, "127.0.0.1", 55000), + )) + + proc.terminate.assert_called_once() + proc.wait.assert_called_once_with(timeout=5) + + def test_stop_kills_process_after_timeout(self): + proc = MagicMock() + proc.poll.return_value = None + proc.wait.side_effect = [subprocess.TimeoutExpired("cmd", 5), 0] + + stop_forwarder(ForwarderHandle(process=proc, forwards=())) + + proc.terminate.assert_called_once() + proc.kill.assert_called_once() + self.assertEqual(2, proc.wait.call_count) + class TestForwarding(unittest.TestCase): def test_forwards_bytes_both_directions(self): try: - probe = socket.create_server(("127.0.0.16", 0)) + probe = socket.create_server(("127.0.0.2", 0)) except OSError: - self.skipTest("127.0.0.16 is not bound on this host") + self.skipTest("127.0.0.2 is not bindable on this host") else: probe.close() _listener, target_port = _echo_server() handle = start_forwarder(( ForwardSpec( label="egress", - listen_host="127.0.0.16", + listen_host="127.0.0.2", listen_port=0, target_host="127.0.0.1", target_port=target_port, @@ -94,5 +212,114 @@ class TestForwarding(unittest.TestCase): stop_forwarder(handle) +class TestForwarderInProcess(unittest.TestCase): + def test_forwarder_serves_and_stops_in_process(self): + _listener, target_port = _echo_server() + forwarder = _Forwarder(( + ForwardSpec("egress", "127.0.0.2", 0, "127.0.0.1", target_port), + )) + forwarder.start() + thread = threading.Thread(target=forwarder.serve, daemon=True) + thread.start() + try: + bound = forwarder.bound[0] + with socket.create_connection((bound.listen_host, bound.listen_port)) as sock: + sock.sendall(b"ping") + self.assertEqual(b"ping", sock.recv(4)) + finally: + forwarder.stop() + thread.join(timeout=2) + + self.assertFalse(thread.is_alive()) + + def test_forwarder_logs_target_connect_failure(self): + forwarder = _Forwarder(( + ForwardSpec("egress", "127.0.0.2", 0, "127.0.0.1", 1), + )) + forwarder.start() + thread = threading.Thread(target=forwarder.serve, daemon=True) + thread.start() + try: + bound = forwarder.bound[0] + with patch("bot_bottle.backend.smolmachines.port_forward.warn") as warn: + with socket.create_connection((bound.listen_host, bound.listen_port)): + pass + for _ in range(100): + if warn.called: + break + threading.Event().wait(0.01) + self.assertTrue(warn.called) + finally: + forwarder.stop() + thread.join(timeout=2) + + def test_pipe_stops_when_recv_fails(self): + src, dst = socket.socketpair() + src.close() + try: + _pipe(src, dst) + finally: + dst.close() + + def test_pipe_stops_when_send_fails(self): + src, src_peer = socket.socketpair() + _dst_peer, dst = socket.socketpair() + dst.close() + try: + src_peer.sendall(b"hello") + src_peer.shutdown(socket.SHUT_WR) + _pipe(src, dst) + finally: + src.close() + src_peer.close() + _dst_peer.close() + + +class TestMain(unittest.TestCase): + def test_main_starts_serves_and_stops_forwarder(self): + calls: list[str] = [] + handlers = [] + + class FakeForwarder: + def __init__(self, specs): # type: ignore[no-untyped-def] + self.specs = specs + self.bound = specs + + def start(self) -> None: + calls.append("start") + + def serve(self) -> None: + calls.append("serve") + + def stop(self) -> None: + calls.append("stop") + + def record_signal(_signum, handler): # type: ignore[no-untyped-def] + handlers.append(handler) + + spec = ForwardSpec("egress", "127.0.0.2", 0, "127.0.0.1", 9099) + with patch( + "bot_bottle.backend.smolmachines.port_forward._Forwarder", + FakeForwarder, + ), patch( + "bot_bottle.backend.smolmachines.port_forward.signal.signal", + side_effect=record_signal, + ), patch("builtins.print") as print_mock: + self.assertEqual( + 0, + main(("--spec-json", json.dumps([{ + "label": spec.label, + "listen_host": spec.listen_host, + "listen_port": spec.listen_port, + "target_host": spec.target_host, + "target_port": spec.target_port, + }]))), + ) + handlers[0](15, None) + + self.assertEqual(["start", "serve", "stop", "stop"], calls) + self.assertIn("forwards", print_mock.call_args.args[0]) + + if __name__ == "__main__": unittest.main() diff --git a/tests/unit/test_smolmachines_provision.py b/tests/unit/test_smolmachines_provision.py index bd33c77..1e848a4 100644 --- a/tests/unit/test_smolmachines_provision.py +++ b/tests/unit/test_smolmachines_provision.py @@ -9,6 +9,7 @@ from __future__ import annotations import subprocess import tempfile import unittest +from contextlib import ExitStack from dataclasses import replace from pathlib import Path from unittest.mock import MagicMock, patch @@ -27,6 +28,7 @@ from bot_bottle.backend.smolmachines.bottle_plan import ( SmolmachinesBottlePlan, ) from bot_bottle.backend.smolmachines import launch as _launch +from bot_bottle.backend.smolmachines import port_forward as _forward from bot_bottle.backend.smolmachines.launch import _bundle_launch_spec from bot_bottle.backend.util import AGENT_CA_PATH from bot_bottle.egress import EgressPlan, EgressRoute @@ -510,6 +512,113 @@ class TestProxyHost(unittest.TestCase): self.assertEqual("127.0.0.16", result) +class TestLaunchResourceWiring(unittest.TestCase): + def test_allocate_resources_uses_loopback_alias_and_bundle_name(self): + plan = _plan() + with patch( + "bot_bottle.backend.smolmachines.launch._loopback.ensure_pool", + ) as ensure_pool, patch( + "bot_bottle.backend.smolmachines.launch._loopback.allocate", + return_value="127.0.0.16", + ) as allocate: + loopback_ip, network = _launch._allocate_resources(plan, ExitStack()) + + ensure_pool.assert_called_once_with() + allocate.assert_called_once_with("demo-abc12") + self.assertEqual("127.0.0.16", loopback_ip) + self.assertEqual("bot-bottle-bundle-demo-abc12", network) + + def test_start_bundle_wraps_raw_vm_ports_with_forwarder_ports(self): + plan = _plan(supervise=True) + plan = replace( + plan, + git_gate_plan=replace( + plan.git_gate_plan, + upstreams=(GitGateUpstream( + name="bot-bottle", + upstream_url="ssh://git@host/repo.git", + upstream_host="host", + upstream_port="22", + identity_file="/tmp/key", + known_host_key="", + ),), + ), + ) + stack = MagicMock(spec=ExitStack) + raw_launch = MagicMock(raw_ports={9099: 55001, 9420: 55002, 9100: 55003}) + handle = _forward.ForwarderHandle( + process=MagicMock(), + forwards=( + _forward.ForwardSpec("egress", "127.0.0.16", 65001, "127.0.0.1", 55001), + _forward.ForwardSpec("git-http", "127.0.0.16", 65002, "127.0.0.1", 55002), + _forward.ForwardSpec("supervise", "127.0.0.16", 65003, "127.0.0.1", 55003), + ), + ) + + with patch( + "bot_bottle.backend.smolmachines.launch._provision_git_gate_keys", + return_value=plan, + ), patch( + "bot_bottle.backend.smolmachines.launch._ensure_smolmachine", + return_value=Path("/cache/sidecar.smolmachine"), + ) as ensure, patch( + "bot_bottle.backend.smolmachines.launch._bundle.start_bundle_vm", + return_value=raw_launch, + ) as start_vm, patch( + "bot_bottle.backend.smolmachines.launch._forward.start_forwarder", + return_value=handle, + ) as start_forwarder: + stamped = _launch._start_bundle(plan, "net", "127.0.0.16", stack) + + ensure.assert_called_once() + start_vm.assert_called_once() + self.assertEqual(Path("/cache/sidecar.smolmachine"), start_vm.call_args.kwargs["from_path"]) + specs = start_forwarder.call_args.args[0] + self.assertEqual( + ( + _forward.ForwardSpec("egress", "127.0.0.16", 0, "127.0.0.1", 55001), + _forward.ForwardSpec("git-http", "127.0.0.16", 0, "127.0.0.1", 55002), + _forward.ForwardSpec("supervise", "127.0.0.16", 0, "127.0.0.1", 55003), + ), + specs, + ) + self.assertEqual("http://127.0.0.16:65001", stamped.agent_proxy_url) + self.assertEqual("127.0.0.16:65002", stamped.agent_git_gate_host) + self.assertEqual("http://127.0.0.16:65003/", stamped.agent_supervise_url) + self.assertEqual(2, stack.callback.call_count) + + def test_init_vm_repairs_ownership_then_waits_for_exec(self): + plan = _plan() + with patch( + "bot_bottle.backend.smolmachines.launch._smolvm.machine_exec", + ) as machine_exec, patch( + "bot_bottle.backend.smolmachines.launch._smolvm.wait_exec_ready", + ) as wait_exec_ready: + _launch._init_vm(plan) + + machine_exec.assert_called_once() + argv = machine_exec.call_args.args[1] + self.assertEqual(["sh", "-c"], argv[:2]) + self.assertIn("chown -R node:node /home/node", argv[2]) + wait_exec_ready.assert_called_once_with(plan.machine_name) + + +class TestPortLabels(unittest.TestCase): + def test_known_and_dynamic_port_labels_round_trip(self): + for port, label in ( + (9099, "egress"), + (9420, "git-http"), + (9100, "supervise"), + (12345, "port-12345"), + ): + self.assertEqual(label, _launch._label_for_port(port)) + self.assertEqual(port, _launch._port_for_label(label)) + + def test_unknown_label_is_rejected(self): + with self.assertRaises(ValueError): + _launch._port_for_label("sidecar") + + class TestDiscoverUrls(unittest.TestCase): """_discover_urls stamps git-gate host + supervise URL into the plan.""" diff --git a/tests/unit/test_smolmachines_sidecar_bundle.py b/tests/unit/test_smolmachines_sidecar_bundle.py index 8377434..ac165dc 100644 --- a/tests/unit/test_smolmachines_sidecar_bundle.py +++ b/tests/unit/test_smolmachines_sidecar_bundle.py @@ -14,6 +14,7 @@ from unittest.mock import patch from bot_bottle.backend.smolmachines.sidecar_bundle import ( BundleLaunchSpec, + allocate_raw_host_ports, bundle_container_name, bundle_network_name, create_bundle_network, @@ -203,6 +204,15 @@ class TestEnsureBundleImage(unittest.TestCase): class TestStartBundleVm(unittest.TestCase): + def test_allocate_raw_host_ports_returns_bindable_ports(self): + raw_ports = allocate_raw_host_ports((9099, 9420)) + + self.assertEqual({9099, 9420}, set(raw_ports)) + for host_port in raw_ports.values(): + self.assertIsInstance(host_port, int) + self.assertGreater(host_port, 0) + self.assertNotEqual(raw_ports[9099], raw_ports[9420]) + def test_creates_sidecar_vm_with_ports_volumes_and_env(self): calls: list[tuple[str, tuple[object, ...], dict[str, object]]] = [] @@ -286,6 +296,27 @@ class TestStopBundleVm(unittest.TestCase): stop.assert_called_once_with("bot-bottle-sidecars-demo-abc12") delete.assert_called_once_with("bot-bottle-sidecars-demo-abc12") + def test_stop_and_delete_errors_are_warnings(self): + from bot_bottle.backend.smolmachines.smolvm import SmolvmError + result = subprocess.CompletedProcess( + args=[], + returncode=1, + stdout="", + stderr="failed", + ) + with patch( + "bot_bottle.backend.smolmachines.sidecar_bundle._smolvm.machine_stop", + side_effect=SmolvmError(["smolvm", "machine", "stop"], result), + ), patch( + "bot_bottle.backend.smolmachines.sidecar_bundle._smolvm.machine_delete", + side_effect=SmolvmError(["smolvm", "machine", "delete"], result), + ), patch( + "bot_bottle.backend.smolmachines.sidecar_bundle.warn", + ) as warn: + stop_bundle_vm("demo-abc12") + + self.assertEqual(2, warn.call_count) + if __name__ == "__main__": unittest.main()