diff --git a/tests/agent/mcp/test_device_tools.py b/tests/agent/mcp/test_device_tools.py index 3a4308053..e4ff08fc1 100644 --- a/tests/agent/mcp/test_device_tools.py +++ b/tests/agent/mcp/test_device_tools.py @@ -10,6 +10,8 @@ overwriting each other's commands and outputs. import json from unittest.mock import MagicMock, patch +import pytest + from gns3server.agent.gns3_copilot.utils.device_configs import ( merge_duplicate_device_configs, ) @@ -334,6 +336,27 @@ class TestDuplicateDeviceMerging: assert result[0]["status"] == "partial_success" assert set(result[0]["blocked_commands"]) == {"debug ip routing", "debug ospf events"} + def test_entry_without_device_name_fails_the_whole_batch(self): + """An unaddressable entry must abort with an error, not vanish.""" + from gns3server.agent.gns3_copilot.tools_v2.display_tools_nornir import ( + ExecuteMultipleDeviceCommands, + ) + + result = ExecuteMultipleDeviceCommands()._run( + json.dumps( + { + "project_id": PROJECT_ID, + "device_configs": [ + {"device_name": "R1", "commands": ["show version"]}, + {"commands": ["show clock"]}, + ], + } + ) + ) + + assert result[0]["status"] == "failed" + assert "device_configs[1]" in result[0]["error"] + class TestMergeDuplicateDeviceConfigs: """Unit tests for the shared normalization helper.""" @@ -361,12 +384,21 @@ class TestMergeDuplicateDeviceConfigs: assert merged == [{"device_name": "R1", "commands": ["a", "b"], "extra": 1}] assert original[0]["commands"] == ["a"] - def test_entry_without_device_name_is_dropped(self): - merged = merge_duplicate_device_configs( - [{"commands": ["a"]}, {"device_name": "R1", "commands": ["b"]}], - commands_field="commands", - ) - assert merged == [{"device_name": "R1", "commands": ["b"]}] + def test_entry_without_device_name_is_rejected(self): + # Silently dropping it would let the batch execute partially while + # every returned row reports success. + with pytest.raises(ValueError, match=r"device_configs\[0\]"): + merge_duplicate_device_configs( + [{"commands": ["a"]}, {"device_name": "R1", "commands": ["b"]}], + commands_field="commands", + ) + + def test_entry_that_is_not_an_object_is_rejected(self): + with pytest.raises(ValueError, match=r"device_configs\[1\]"): + merge_duplicate_device_configs( + [{"device_name": "R1", "commands": ["a"]}, "R2"], + commands_field="commands", + ) def test_missing_commands_field_defaults_to_empty(self): merged = merge_duplicate_device_configs( diff --git a/tests/api/routes/compute/test_ethernet_switch_nodes.py b/tests/api/routes/compute/test_ethernet_switch_nodes.py index b685b6a0d..3069c45de 100644 --- a/tests/api/routes/compute/test_ethernet_switch_nodes.py +++ b/tests/api/routes/compute/test_ethernet_switch_nodes.py @@ -22,7 +22,9 @@ import pytest_asyncio from fastapi import FastAPI, status from httpx import AsyncClient +from gns3server.compute.error import NodeError from gns3server.compute.project import Project +from gns3server.compute.ubridge.ubridge_error import UbridgeError from tests.utils import asyncio_patch # The builtin Ethernet switch talks to uBridge (brctl/bridge modules) instead of @@ -927,8 +929,6 @@ class TestEthernetSwitchNodesRoutes: proceeds — a lost race is not an error. """ - from gns3server.compute.ubridge.ubridge_error import UbridgeError - anchor, peer = "gs1a2b3c4d5e0", "gs1a2b3c4d5e1" node = compute_project.get_node(ethernet_switch["node_id"]) exists = {"value": False} @@ -1217,9 +1217,94 @@ class TestEthernetSwitchNodesRoutes: with self._sysfs(anchor_exists=True): await compute_client.post(url, json=self._anchor_params(anchor)) + node = compute_project.get_node(ethernet_switch["node_id"]) + node._ubridge_send.reset_mock() + + ports = [{"name": f"Ethernet{i}", "port_number": i, "type": "access", "vlan": 1} for i in range(8)] + ports[0]["vlan"] = 10 + response = await compute_client.put( + app.url_path_for( + "compute:update_ethernet_switch", + project_id=ethernet_switch["project_id"], + node_id=ethernet_switch["node_id"], + ), + json={"ports_mapping": ports}, + ) + assert response.status_code == status.HTTP_200_OK + + br = node._bridge_name + commands = [c.args[0] for c in node._ubridge_send.call_args_list] + assert commands == [ + f'brctl vlan_del "{br}" "{anchor}" 1', + f'brctl vlan_add "{br}" "{anchor}" 10 pvid untagged', + ], commands + assert not any("delif" in c or "addif" in c for c in commands) + + async def test_ethernet_switch_update_ports_skips_a_deferred_anchor( + self, app: FastAPI, compute_client: AsyncClient, compute_project: Project, ethernet_switch: dict + ) -> None: + """ + A kernel port whose peer is stopped has no interface yet (the join is + deferred by design): a VLAN change must not send commands that would + fail ENODEV and abort the whole reconcile — the port keeps the new + settings in the mapping and they apply when the peer starts. + """ + + anchor = "gv00010203e0p0" + url = app.url_path_for( + "compute:create_ethernet_switch_nio", + project_id=ethernet_switch["project_id"], + node_id=ethernet_switch["node_id"], + adapter_number="0", + port_number="0", + ) + with self._sysfs(anchor_exists=False): + await compute_client.post(url, json=self._anchor_params(anchor)) + node = compute_project.get_node(ethernet_switch["node_id"]) + assert node._kernel_ports.get(0) == anchor # deferred but registered node._ubridge_send.reset_mock() + ports = [{"name": f"Ethernet{i}", "port_number": i, "type": "access", "vlan": 1} for i in range(8)] + ports[0]["vlan"] = 10 + with self._sysfs(anchor_exists=False): + response = await compute_client.put( + app.url_path_for( + "compute:update_ethernet_switch", + project_id=ethernet_switch["project_id"], + node_id=ethernet_switch["node_id"], + ), + json={"ports_mapping": ports}, + ) + + assert response.status_code == status.HTTP_200_OK + commands = [c.args[0] for c in node._ubridge_send.call_args_list] + assert not any("vlan_" in c for c in commands), commands + + async def test_ethernet_switch_update_ports_rolls_the_mapping_back_on_failure( + self, app: FastAPI, compute_client: AsyncClient, compute_project: Project, ethernet_switch: dict + ) -> None: + """ + A failed VLAN reconcile must not keep the new mapping: the retry would + then diff it against itself and report success with the VLANs never + applied. + """ + + await compute_client.post( + app.url_path_for( + "compute:create_ethernet_switch_nio", + project_id=ethernet_switch["project_id"], + node_id=ethernet_switch["node_id"], + adapter_number="0", + port_number="0", + ), + json=self._udp_params(), + ) + + node = compute_project.get_node(ethernet_switch["node_id"]) + before = [dict(port) for port in node.ports_mapping] + node._ubridge_send.side_effect = NodeError("207-reconcile failed") + ports = [{"name": f"Ethernet{i}", "port_number": i, "type": "access", "vlan": 1} for i in range(8)] ports[0]["vlan"] = 10 response = await compute_client.put( @@ -1230,12 +1315,7 @@ class TestEthernetSwitchNodesRoutes: ), json={"ports_mapping": ports}, ) - assert response.status_code == status.HTTP_200_OK - br = node._bridge_name - commands = [c.args[0] for c in node._ubridge_send.call_args_list] - assert commands == [ - f'brctl vlan_del "{br}" "{anchor}" 1', - f'brctl vlan_add "{br}" "{anchor}" 10 pvid untagged', - ], commands - assert not any("delif" in c or "addif" in c for c in commands) + assert response.status_code == status.HTTP_409_CONFLICT + assert response.json()["message"] == "207-reconcile failed" + assert [dict(port) for port in node.ports_mapping] == before diff --git a/tests/compute/docker/test_docker_kernel_datapath.py b/tests/compute/docker/test_docker_kernel_datapath.py index c50c35c6a..853c1999a 100644 --- a/tests/compute/docker/test_docker_kernel_datapath.py +++ b/tests/compute/docker/test_docker_kernel_datapath.py @@ -899,7 +899,6 @@ async def test_update_nio_kernel_clears_netem(vm): async def test_apply_netem_reset_tolerates_missing_qdisc(vm): vm._ubridge_hypervisor = MagicMock() - from gns3server.compute.ubridge.ubridge_error import UbridgeError async def refuse(command): raise UbridgeError("207-Could not reset qdisc on gv0: No such file or directory") @@ -914,7 +913,6 @@ async def test_apply_netem_reset_tolerates_missing_qdisc(vm): async def test_apply_netem_reset_reraises_real_errors(vm): vm._ubridge_hypervisor = MagicMock() - from gns3server.compute.ubridge.ubridge_error import UbridgeError async def refuse(command): raise UbridgeError("206-Could not reset qdisc on gv0: Operation not permitted") @@ -997,6 +995,79 @@ async def test_start_capture_second_concurrent_fails(vm): with pytest.raises(UbridgeError, match="already in progress"): await vm.start_capture(0, "/tmp/capture.pcap") + # the failed start must not leave the port marked as capturing: a stuck + # flag refuses the retry and lets this port's stop kill the winner + assert nio.capturing is False + assert vm._kernel_capture_ifc is None + + +@pytest.mark.asyncio +async def test_start_capture_second_port_on_the_same_node_is_refused(vm): + """ + uBridge serves one kernel capture per process while the server models + captures per port: a second port's capture must be refused with a clear + error and its flag rolled back — otherwise its stop would issue the + process-wide, argument-less ``capture stop_kernel`` and kill the first + port's capture while its own flag still says capturing. + """ + + vm._ubridge_hypervisor = MagicMock() + vm._ubridge_send = AsyncioMagicMock() + vm.status = "started" + vm._ethernet_adapters.append(type(vm._ethernet_adapters[0])()) + host_ifc1, _ = vm._veth_names(0, 0) + host_ifc2, _ = vm._veth_names(1, 0) + vm._kernel_veths[(0, 0)] = host_ifc1 + vm._kernel_veths[(1, 0)] = host_ifc2 + nio1 = vm.manager.create_nio({"type": "nio_bridge", "bridge": BRIDGE}) + nio2 = vm.manager.create_nio({"type": "nio_bridge", "bridge": BRIDGE}) + vm._ethernet_adapters[0].add_nio(0, nio1) + vm._ethernet_adapters[1].add_nio(0, nio2) + + await vm.start_capture(0, "/tmp/capture.pcap") + + with pytest.raises(DockerError, match="one kernel capture per node"): + await vm.start_capture(1, "/tmp/capture2.pcap") + + assert nio1.capturing is True + assert nio2.capturing is False + vm._ubridge_send.assert_any_call(f'capture start_kernel {host_ifc1} "/tmp/capture.pcap"') + assert not any("stop_kernel" in str(c) for c in vm._ubridge_send.call_args_list) + + +@pytest.mark.asyncio +async def test_stop_capture_on_a_port_that_does_not_own_the_slot_skips_stop_kernel(vm): + """ + ``capture stop_kernel`` is process-wide and takes no interface: a stop on + a port whose flag says capturing while another port owns the slot must + not reach uBridge — it would stop the owner's capture. + """ + + vm._ubridge_hypervisor = MagicMock() + vm._ubridge_send = AsyncioMagicMock() + vm.status = "started" + vm._ethernet_adapters.append(type(vm._ethernet_adapters[0])()) + host_ifc1, _ = vm._veth_names(0, 0) + host_ifc2, _ = vm._veth_names(1, 0) + vm._kernel_veths[(0, 0)] = host_ifc1 + vm._kernel_veths[(1, 0)] = host_ifc2 + nio1 = vm.manager.create_nio({"type": "nio_bridge", "bridge": BRIDGE}) + nio2 = vm.manager.create_nio({"type": "nio_bridge", "bridge": BRIDGE}) + vm._ethernet_adapters[0].add_nio(0, nio1) + vm._ethernet_adapters[1].add_nio(0, nio2) + + await vm.start_capture(0, "/tmp/capture.pcap") # port 0 owns the slot + nio2.start_packet_capture("/tmp/capture2.pcap") # a stale second flag + + await vm.stop_capture(1) + + assert nio2.capturing is False + assert not any("stop_kernel" in str(c) for c in vm._ubridge_send.call_args_list) + + # the owner's capture is untouched and still stoppable + await vm.stop_capture(0) + vm._ubridge_send.assert_any_call("capture stop_kernel") + @pytest.mark.asyncio async def test_connect_nio_kernel_restarts_capture(vm): diff --git a/tests/compute/docker/test_docker_vm.py b/tests/compute/docker/test_docker_vm.py index 51a1e4176..dbddda0ae 100644 --- a/tests/compute/docker/test_docker_vm.py +++ b/tests/compute/docker/test_docker_vm.py @@ -1642,6 +1642,24 @@ async def test_adapter_add_nio_binding_1(vm): assert vm._ethernet_adapters[0].get_nio(0) == nio +@pytest.mark.asyncio +async def test_adapter_add_nio_binding_refuses_an_occupied_port(vm): + """ + The compute-side backstop of the controller's duplicate-port guard: a + second NIO for a port that already carries one is refused instead of + silently overwriting the binding the first link's teardown still needs. + """ + + first = vm.manager.create_nio({"type": "nio_udp", "lport": 4242, "rport": 4343, "rhost": "127.0.0.1"}) + await vm.adapter_add_nio_binding(0, first) + + second = vm.manager.create_nio({"type": "nio_udp", "lport": 4244, "rport": 4345, "rhost": "127.0.0.1"}) + with pytest.raises(DockerError, match="already has a link"): + await vm.adapter_add_nio_binding(0, second) + + assert vm._ethernet_adapters[0].get_nio(0) is first + + @pytest.mark.asyncio async def test_adapter_add_nio_binding_sets_carrier(vm): diff --git a/tests/compute/docker/test_iol_docker_kernel_datapath.py b/tests/compute/docker/test_iol_docker_kernel_datapath.py index b3f692706..3491c128e 100644 --- a/tests/compute/docker/test_iol_docker_kernel_datapath.py +++ b/tests/compute/docker/test_iol_docker_kernel_datapath.py @@ -22,6 +22,7 @@ import pytest_asyncio from gns3server.compute.docker import Docker from gns3server.compute.docker.docker_error import DockerError from gns3server.compute.docker.iol_docker_vm import IOLDockerVM +from gns3server.utils.kernel_anchor import kernel_anchor_name, kernel_anchor_type from tests.utils import AsyncioMagicMock NODE_ID = "00010203-0405-0607-0809-0a0b0c0d0e0f" @@ -385,8 +386,6 @@ def test_anchor_names_follow_the_iol_docker_key(): never the docker veth namespace. """ - from gns3server.utils.kernel_anchor import kernel_anchor_name, kernel_anchor_type - environment = "GNS3_IOL_RUNNER=1\nGNS3_IOL_STARTUP_CONFIG=cfg.txt" assert kernel_anchor_type("docker", environment) == "iol_docker" assert kernel_anchor_type("docker", "GNS3_UNIX_SOCKET_NIO=1") == "docker" diff --git a/tests/compute/dynamips/test_dynamips_kernel_datapath.py b/tests/compute/dynamips/test_dynamips_kernel_datapath.py index d09932eea..d1af1c808 100644 --- a/tests/compute/dynamips/test_dynamips_kernel_datapath.py +++ b/tests/compute/dynamips/test_dynamips_kernel_datapath.py @@ -309,6 +309,35 @@ async def test_slot_add_nio_binding_attach_failure_leaves_the_port_unbound(route assert router._slots[0].get_nio(0) is None +@pytest.mark.asyncio +async def test_attach_kernel_nio_failure_undoes_the_binding_so_a_retry_works(router): + """ + A failure after the hypervisor binding (the enslave step here) must undo + the half-attach: the _tap_nios entry would otherwise short-circuit every + retry, and a link delete reads the still-unbound adapter as "nothing to + do" — a silent black hole until the router restarts. + """ + + router.status = "started" + router._kernel_taps[(0, 0)] = TAP00 + nio = await _nio(router) + + real_attach = router._kernel_attach + router._kernel_attach = AsyncioMagicMock(side_effect=UbridgeError("208-Device or resource busy")) + with pytest.raises(UbridgeError): + await router._attach_kernel_nio(0, 0, nio) + router._kernel_attach = real_attach + + # the half-attach went away with the failure... + assert (0, 0) not in router._tap_nios + assert any("slot_remove_nio_binding" in c for c in _hypervisor_commands(router)) + + # ...so the retry wires the port for real + await router._attach_kernel_nio(0, 0, nio) + assert (0, 0) in router._tap_nios + router._ubridge_send.assert_any_call(f'brctl addif "{BRIDGE}" "{TAP00}"') + + @pytest.mark.asyncio async def test_attach_kernel_nio_without_anchor_on_a_running_node_raises(router): """ diff --git a/tests/compute/iou/test_iou_vm.py b/tests/compute/iou/test_iou_vm.py index 67a0fc5f1..509240c00 100644 --- a/tests/compute/iou/test_iou_vm.py +++ b/tests/compute/iou/test_iou_vm.py @@ -26,9 +26,11 @@ from unittest.mock import MagicMock, call import pytest import pytest_asyncio +from gns3server.compute.error import NodeError from gns3server.compute.iou import IOU from gns3server.compute.iou.iou_error import IOUError from gns3server.compute.iou.iou_vm import IOUVM, IOUL1KeepaliveProtocol +from gns3server.compute.ubridge.ubridge_error import UbridgeError from tests.utils import AsyncioMagicMock, asyncio_patch @@ -151,6 +153,26 @@ async def test_start_failure_stops_l1_responder(vm): vm._stop_l1_keepalive_responder.assert_called_once_with() +@pytest.mark.asyncio +async def test_start_failure_in_the_tap_setup_stops_ubridge(vm): + """ + A raise before the IOU launch (the tap datapath setup here) must take + the uBridge datapath down again: a node reporting a failed start must + not keep a uBridge process and its anchor TAPs alive. + """ + + vm._check_requirements = AsyncioMagicMock(return_value=True) + vm._check_iou_license = AsyncioMagicMock(return_value=True) + vm._start_ubridge = AsyncioMagicMock() + vm._prepare_tap_datapath = AsyncioMagicMock(side_effect=IOUError("tap create refused")) + vm._stop_ubridge = AsyncioMagicMock() + + with pytest.raises(IOUError, match="tap create refused"): + await vm.start() + + assert vm._stop_ubridge.call_count == 1 + + @pytest.mark.asyncio async def test_start_with_iourc(vm, tmpdir, config): @@ -580,6 +602,36 @@ def test_get_legacy_vm_workdir(): assert IOU.get_legacy_vm_workdir(42, "bla") == "iou/device-42" +@pytest.mark.asyncio +async def test_ubridge_apply_filters_rejects_kernel_only(vm): + """ + The IOL relay path carries the base guard: a kernel-only filter reaching + a relay port is a clear NodeError, not a late uBridge failure. + """ + + vm._ubridge_send = AsyncioMagicMock() + with pytest.raises(NodeError, match="kernel-datapath"): + await vm._ubridge_apply_filters("bridge0 0 0", {"rate": ["512kbit"]}) + vm._ubridge_send.assert_not_called() + + +@pytest.mark.asyncio +async def test_ubridge_apply_filters_degrades_a_bad_bpf_line(vm): + """ + A BPF line that no longer compiles on this uBridge's libpcap is a + warning, not a hard failure — the same degradation as the base relay + path (the controller validated the syntax with tcpdump at update time). + """ + + async def refuse(command): + if "add_packet_filter" in command: + raise UbridgeError("Cannot compile filter 'icmp': syntax error") + return ["OK"] + + vm._ubridge_send = refuse + await vm._ubridge_apply_filters("bridge0 0 0", {"bpf": ["icmp"]}) + + @pytest.mark.asyncio async def test_invalid_iou_file(vm, iourc_file): diff --git a/tests/compute/qemu/test_qemu_kernel_datapath.py b/tests/compute/qemu/test_qemu_kernel_datapath.py index ba6ea96ec..9c5c211c0 100644 --- a/tests/compute/qemu/test_qemu_kernel_datapath.py +++ b/tests/compute/qemu/test_qemu_kernel_datapath.py @@ -376,6 +376,73 @@ async def test_stop_stops_the_process_before_deleting_the_taps(vm): assert order == ["process", "taps"] +@pytest.mark.asyncio +async def test_stop_releases_the_datapath_even_when_the_stop_body_raises(vm): + """ + The finally keeps a mid-stop raise (an export error here) from skipping + the uBridge/TAP release — the datapath would otherwise outlive a failed + stop with nothing left to reclaim it. + """ + + _running(vm) + stopped = AsyncioMagicMock() + + async def failing_export(): + raise QemuError("qemu-img is missing") + + with ( + patch("gns3server.utils.asyncio.wait_for_process_termination", new=AsyncioMagicMock()), + patch.object(vm, "_export_config", new=failing_export), + patch.object(vm, "_stop_ubridge", new=stopped), + asyncio_patch("gns3server.compute.qemu.qemu_vm.QemuVM._clear_save_vm_stated"), + ): + with pytest.raises(QemuError, match="qemu-img is missing"): + await vm.stop() + + assert stopped.call_count == 1 + + +@pytest.mark.asyncio +async def test_start_failure_before_the_launch_tears_down_the_datapath(vm): + """ + A raise before the QEMU launch (a broken qemu_path in _build_command) + must take the uBridge/TAP datapath down again: a node reporting a failed + start must not keep a uBridge process and the anchor TAPs alive. + """ + + stopped = AsyncioMagicMock() + with ( + patch.object(vm, "check_available_ram"), + patch.object(vm, "_prepare_tap_datapath", new=AsyncioMagicMock()), + patch.object(vm, "_build_command", new=AsyncioMagicMock(side_effect=QemuError("qemu-img is missing"))), + patch.object(vm, "_stop_ubridge", new=stopped), + ): + with pytest.raises(QemuError, match="qemu-img is missing"): + await vm.start() + + assert stopped.call_count == 1 + + +@pytest.mark.asyncio +async def test_add_nio_binding_refuses_an_occupied_adapter(vm): + """ + The compute-side backstop of the controller's duplicate-port guard: a + second NIO for an adapter that already carries one is refused instead of + silently overwriting the binding the first link's teardown still needs. + """ + + vm._tap_datapath = True + vm._kernel_taps[(0, 0)] = TAP0 + first = vm.manager.create_nio({"type": "nio_bridge", "bridge": BRIDGE}) + await vm.adapter_add_nio_binding(0, first) + + second = vm.manager.create_nio({"type": "nio_bridge", "bridge": "gns3otherbridge"}) + with pytest.raises(QemuError, match="already has a link"): + await vm.adapter_add_nio_binding(0, second) + + assert vm._ethernet_adapters[0].get_nio(0) is first + + @pytest.mark.asyncio async def test_concurrent_stops_sweep_every_tap_exactly_once(vm): """ diff --git a/tests/compute/ubridge/test_tc_probe.py b/tests/compute/ubridge/test_tc_probe.py index 1cdeba245..45275368c 100644 --- a/tests/compute/ubridge/test_tc_probe.py +++ b/tests/compute/ubridge/test_tc_probe.py @@ -19,6 +19,8 @@ The standalone `tc capabilities` probe behind the compute /capabilities payload: spawn a throwaway uBridge, ask once, cache by binary identity. """ +import os + import pytest from gns3server.compute.ubridge import tc_probe @@ -91,11 +93,13 @@ def probe_env(monkeypatch, tmp_path): tc_probe._iol_tap_cache.clear() tc_probe._bridge_tap_cache.clear() FakeHypervisor.last = None - # a real file on disk: the cache is keyed on its (path, mtime, size) + # a real file on disk: the cache is keyed on its (path, mtime, size, + # ctime) binary = tmp_path / "ubridge" binary.write_bytes(b"") monkeypatch.setattr(tc_probe.shutil, "which", lambda name: str(binary)) monkeypatch.setattr(tc_probe, "Hypervisor", FakeHypervisor) + return str(binary) async def test_probe_returns_parsed_report(): @@ -134,6 +138,28 @@ async def test_probe_missing_binary_means_unknown(monkeypatch): assert FakeHypervisor.spawned == 0 +async def test_probe_reprobes_when_the_binary_attributes_change(probe_env): + + assert await probe_tc_capabilities() is not None + # chmod touches ctime but neither mtime nor size: a setcap/chmod remedy + # after a permission-related probe failure must be able to flip the + # cached verdict without a server restart. + os.chmod(probe_env, 0o600) + assert await probe_tc_capabilities() is not None + assert FakeHypervisor.spawned == 2 + + +async def test_probe_bridge_tap_unknown_when_ubridge_will_not_start(monkeypatch): + """start() can raise UbridgeError (unusable binary): unknown, never a + failed /capabilities response.""" + + async def failing_start(self): + raise UbridgeError("uBridge executable version must be >= 2.4.0") + + monkeypatch.setattr(FakeHypervisor, "start", failing_start) + assert await probe_bridge_tap_support() is None + + async def test_probe_empty_reply_means_unknown(): FakeHypervisor.reply = [] diff --git a/tests/controller/test_kernel_datapath_link.py b/tests/controller/test_kernel_datapath_link.py index 7d326f896..0f34d6ed6 100644 --- a/tests/controller/test_kernel_datapath_link.py +++ b/tests/controller/test_kernel_datapath_link.py @@ -26,9 +26,13 @@ import pytest from gns3server.config import Config from gns3server.controller.controller_error import ControllerError +from gns3server.controller.link import FILTERS from gns3server.controller.node import Node from gns3server.controller.ports.ethernet_port import EthernetPort +from gns3server.controller.ports.serial_port import SerialPort from gns3server.controller.udp_link import UDPLink +from gns3server.utils.kernel_anchor import kernel_anchor_name, kernel_cascade_names +from tests.utils import AsyncioMagicMock def _node(project, compute, name, node_type="docker", status="stopped", environment=None): @@ -231,8 +235,6 @@ async def test_kernel_datapath_not_eligible_for_dynamips_serial_ports(project): decided by the controller's port matrix. """ - from gns3server.controller.ports.serial_port import SerialPort - compute = MagicMock() compute.id = "compute-1" compute.capabilities = {"ubridge_tap": True} @@ -253,8 +255,6 @@ async def test_kernel_datapath_not_eligible_for_serial_ports(project): kernel path would fail at the compute (no anchor on a serial bay). """ - from gns3server.controller.ports.serial_port import SerialPort - compute = MagicMock() compute.id = "compute-1" compute.capabilities = {"ubridge_tap": True, "ubridge_iol_tap": True} @@ -431,6 +431,33 @@ async def test_kernel_datapath_not_eligible_for_unix_socket_containers(project): assert link._kernel_datapath_eligible(node1, node2) is False +@pytest.mark.asyncio +async def test_kernel_datapath_eligible_when_unix_socket_marker_is_falsy(project): + """ + GNS3_UNIX_SOCKET_NIO=0 (like false/no) leaves the container on the + standard veth wiring — the compute parses the value, so the controller + must not exclude it with a substring match, or the link silently stays + on the relay forever. + """ + + link, node1, node2 = await _kernel_link(project) + node2._properties = {"environment": "GNS3_UNIX_SOCKET_NIO=0"} + assert link._kernel_datapath_eligible(node1, node2) is True + + +@pytest.mark.asyncio +async def test_kernel_datapath_eligible_when_the_marker_is_mid_line(project): + """ + A mid-line mention (NOTE=GNS3_UNIX_SOCKET_NIO=1) is not the marker: + the compute's line-based parsing does not select VendorDockerVM for it, + so the controller must not exclude the node either. + """ + + link, node1, node2 = await _kernel_link(project) + node2._properties = {"environment": "NOTE=GNS3_UNIX_SOCKET_NIO=1"} + assert link._kernel_datapath_eligible(node1, node2) is True + + @pytest.mark.asyncio async def test_iol_runner_containers_gate_on_the_bridge_tap_capabilities(project): """ @@ -476,8 +503,6 @@ async def test_iol_runner_container_on_a_switch_gates_and_names_the_iol_docker_a a late error. """ - from gns3server.utils.kernel_anchor import kernel_anchor_name - compute = MagicMock() compute.id = "compute-1" compute.capabilities = {"ubridge_bridge_tap": True, "ubridge_tap": True} @@ -880,8 +905,6 @@ async def test_available_filters_unconstrained_without_report(project): validation is the guard. """ - from gns3server.controller.link import FILTERS - link, _node1, _node2 = await _kernel_link(project) link._link_data = [{"type": "nio_bridge"}] assert [f["type"] for f in link.available_filters()] == [f["type"] for f in FILTERS] @@ -946,8 +969,6 @@ async def test_switch_link_prepare_emits_anchor_and_external_bridge(project): interface. """ - from gns3server.utils.kernel_anchor import kernel_anchor_name - compute = MagicMock() compute.id = "compute-1" switch = _node(project, compute, "sw1", node_type="ethernet_switch") @@ -987,8 +1008,6 @@ async def test_switch_link_repushes_the_switch_nio_on_node_start(project): replaced). Relay links do nothing. """ - from tests.utils import AsyncioMagicMock - compute = MagicMock() compute.id = "compute-1" switch = _node(project, compute, "sw1", node_type="ethernet_switch") @@ -1031,8 +1050,6 @@ async def test_prepare_cascade_link_emits_paired_anchor_nios(project): once like any other kernel link. """ - from gns3server.utils.kernel_anchor import kernel_cascade_names - compute = MagicMock() compute.id = "compute-1" sw1 = _node(project, compute, "sw1", node_type="ethernet_switch") @@ -1071,8 +1088,6 @@ async def test_node_started_repushes_both_cascade_ends(project): re-checks and re-joins its own half. """ - from tests.utils import AsyncioMagicMock - compute = MagicMock() compute.id = "compute-1" sw1 = _node(project, compute, "sw1", node_type="ethernet_switch") diff --git a/tests/e2e/test_docker_link_local_frames.py b/tests/e2e/test_docker_link_local_frames.py index 8ac4a5404..42743d83a 100644 --- a/tests/e2e/test_docker_link_local_frames.py +++ b/tests/e2e/test_docker_link_local_frames.py @@ -251,7 +251,8 @@ def _llprobe(): if cc is None: pytest.skip("cannot build the raw-frame probe (no C compiler found)") try: - subprocess.run([cc, "-static", "-O2", "-o", binary, source], check=True, capture_output=True) + # S603: resolved compiler path, fixed argv, no shell + subprocess.run([cc, "-static", "-O2", "-o", binary, source], check=True, capture_output=True) # noqa: S603 except subprocess.CalledProcessError as e: pytest.skip(f"cannot build the raw-frame probe (cc -static): {e}") return binary diff --git a/tests/utils/test_kernel_anchor.py b/tests/utils/test_kernel_anchor.py index 7f5558d1a..247237644 100644 --- a/tests/utils/test_kernel_anchor.py +++ b/tests/utils/test_kernel_anchor.py @@ -45,8 +45,11 @@ def test_anchor_type_keys_iol_runner_containers_apart(): assert kernel_anchor_type("docker", "GNS3_IOL_RUNNER=1") == "iol_docker" assert kernel_anchor_type("docker", "GNS3_IOL_STARTUP_CONFIG=cfg.txt\nGNS3_IOL_RUNNER=1") == "iol_docker" + assert kernel_anchor_type("docker", "GNS3_IOL_RUNNER=1,") == "iol_docker" assert kernel_anchor_type("docker") == "docker" assert kernel_anchor_type("docker", "GNS3_UNIX_SOCKET_NIO=1") == "docker" + # a mid-line mention is not the marker (mirrors _select_node_class) + assert kernel_anchor_type("docker", "NOTE=GNS3_IOL_RUNNER=1") == "docker" assert kernel_anchor_type("qemu") == "qemu" diff --git a/tests/utils/test_packet_filter_validation.py b/tests/utils/test_packet_filter_validation.py index 2bada6f13..0cbf970b3 100644 --- a/tests/utils/test_packet_filter_validation.py +++ b/tests/utils/test_packet_filter_validation.py @@ -360,8 +360,13 @@ class TestNetemExtensionFilters: {"delay": [100, 20, ""], "packet_loss": [10, 0], "corrupt": [2], "bpf": ["icmp"]} ) assert dropped == set() - # zero correlation and empty distribution are kept as-is (no-ops) - assert clean == {"delay": [100, 20, ""], "packet_loss": [10, 0], "corrupt": [2], "bpf": ["icmp"]} + # Zero correlation and an empty distribution carry no feature, so + # neither is reported as dropped. The packet_loss correlation slot + # is stripped all the same: the relay filter takes exactly one + # argument (ubridge's packet_loss_setup rejects argc != 1), while + # the delay's empty 3rd value is an unquoted empty token that + # tokenizes away — harmless kept as-is. + assert clean == {"delay": [100, 20, ""], "packet_loss": [10], "corrupt": [2], "bpf": ["icmp"]} def test_kernel_only_features_empty(self): assert split_kernel_only_features({}) == ({}, set())