diff --git a/gns3server/api/routes/compute/cloud_nodes.py b/gns3server/api/routes/compute/cloud_nodes.py index d33023053..a5f2bb6a9 100644 --- a/gns3server/api/routes/compute/cloud_nodes.py +++ b/gns3server/api/routes/compute/cloud_nodes.py @@ -297,20 +297,26 @@ async def resume_cloud_markers(node: Cloud = Depends(dep_node)) -> None: @router.delete( - "/{node_id}/markers/{marker_name}", + "/{node_id}/adapters/{adapter_number}/ports/{port_number}/markers/{marker_name}", status_code=status.HTTP_204_NO_CONTENT ) async def delete_cloud_marker_capture( + *, marker_name: str, + adapter_number: int = Path(..., ge=0, le=0), + port_number: int, link_id: str = "", node: Cloud = Depends(dep_node) ) -> None: """ Delete a marker's capture pcap (called by the controller when the marker is - removed) so the file is cleaned up even with the node stopped. + removed) so the file is cleaned up even with the node stopped. Also drops + the marker from the port NIO's cached spec so a node restart won't reinstall + it (and recreate an empty pcap). """ - await node.delete_marker_capture(marker_name, link_id) + nio = node.get_nio(port_number) + await node.delete_marker_capture(marker_name, link_id, nio) @router.put("/{node_id}/markers/{marker_name}/rebuild") diff --git a/gns3server/api/routes/compute/docker_nodes.py b/gns3server/api/routes/compute/docker_nodes.py index a5a797972..f01425565 100644 --- a/gns3server/api/routes/compute/docker_nodes.py +++ b/gns3server/api/routes/compute/docker_nodes.py @@ -453,21 +453,26 @@ async def resume_docker_markers(node: DockerVM = Depends(dep_node)) -> None: @router.delete( - "/{node_id}/markers/{marker_name}", + "/{node_id}/adapters/{adapter_number}/ports/{port_number}/markers/{marker_name}", status_code=status.HTTP_204_NO_CONTENT, dependencies=[Depends(compute_authentication)] ) async def delete_docker_marker_capture( marker_name: str, + adapter_number: int, + port_number: int, link_id: str = "", node: DockerVM = Depends(dep_node) ) -> None: """ Delete a marker's capture pcap (called by the controller when the marker is - removed) so the file is cleaned up even with the node stopped. + removed) so the file is cleaned up even with the node stopped. Also drops + the marker from the port NIO's cached spec so a node restart won't reinstall + it (and recreate an empty pcap). """ - await node.delete_marker_capture(marker_name, link_id) + nio = node.get_nio(adapter_number) + await node.delete_marker_capture(marker_name, link_id, nio) @router.put( diff --git a/gns3server/api/routes/compute/dynamips_nodes.py b/gns3server/api/routes/compute/dynamips_nodes.py index 863f192ab..c60786f1b 100644 --- a/gns3server/api/routes/compute/dynamips_nodes.py +++ b/gns3server/api/routes/compute/dynamips_nodes.py @@ -412,21 +412,26 @@ async def resume_dynamips_markers(node: Router = Depends(dep_node)) -> None: @router.delete( - "/{node_id}/markers/{marker_name}", + "/{node_id}/adapters/{adapter_number}/ports/{port_number}/markers/{marker_name}", status_code=status.HTTP_204_NO_CONTENT, dependencies=[Depends(compute_authentication)] ) async def delete_dynamips_marker_capture( marker_name: str, + adapter_number: int, + port_number: int, link_id: str = "", node: Router = Depends(dep_node) ) -> None: """ Delete a marker's capture pcap (called by the controller when the marker is - removed) so the file is cleaned up even with the node stopped. + removed) so the file is cleaned up even with the node stopped. Also drops + the marker from the port NIO's cached spec so a node restart won't reinstall + it (and recreate an empty pcap). """ - await node.delete_marker_capture(marker_name, link_id) + nio = node.get_nio(adapter_number, port_number) + await node.delete_marker_capture(marker_name, link_id, nio) @router.put( diff --git a/gns3server/api/routes/compute/iou_nodes.py b/gns3server/api/routes/compute/iou_nodes.py index 73d78a54e..76ee8c594 100644 --- a/gns3server/api/routes/compute/iou_nodes.py +++ b/gns3server/api/routes/compute/iou_nodes.py @@ -391,21 +391,26 @@ async def resume_iou_markers(node: IOUVM = Depends(dep_node)) -> None: @router.delete( - "/{node_id}/markers/{marker_name}", + "/{node_id}/adapters/{adapter_number}/ports/{port_number}/markers/{marker_name}", status_code=status.HTTP_204_NO_CONTENT, dependencies=[Depends(compute_authentication)] ) async def delete_iou_marker_capture( marker_name: str, + adapter_number: int, + port_number: int, link_id: str = "", node: IOUVM = Depends(dep_node) ) -> None: """ Delete a marker's capture pcap (called by the controller when the marker is - removed) so the file is cleaned up even with the node stopped. + removed) so the file is cleaned up even with the node stopped. Also drops + the marker from the port NIO's cached spec so a node restart won't reinstall + it (and recreate an empty pcap). """ - await node.delete_marker_capture(marker_name, link_id) + nio = node.get_nio(adapter_number, port_number) + await node.delete_marker_capture(marker_name, link_id, nio) @router.put( diff --git a/gns3server/api/routes/compute/qemu_nodes.py b/gns3server/api/routes/compute/qemu_nodes.py index aa353bc5f..9494d6044 100644 --- a/gns3server/api/routes/compute/qemu_nodes.py +++ b/gns3server/api/routes/compute/qemu_nodes.py @@ -483,21 +483,26 @@ async def resume_qemu_markers(node: QemuVM = Depends(dep_node)) -> None: @router.delete( - "/{node_id}/markers/{marker_name}", + "/{node_id}/adapters/{adapter_number}/ports/{port_number}/markers/{marker_name}", status_code=status.HTTP_204_NO_CONTENT, dependencies=[Depends(compute_authentication)] ) async def delete_qemu_marker_capture( marker_name: str, + adapter_number: int, + port_number: int = Path(..., ge=0, le=0), link_id: str = "", node: QemuVM = Depends(dep_node) ) -> None: """ Delete a marker's capture pcap (called by the controller when the marker is - removed) so the file is cleaned up even with the node stopped. + removed) so the file is cleaned up even with the node stopped. Also drops + the marker from the port NIO's cached spec so a node restart won't reinstall + it (and recreate an empty pcap). """ - await node.delete_marker_capture(marker_name, link_id) + nio = node.get_nio(adapter_number) + await node.delete_marker_capture(marker_name, link_id, nio) @router.put( diff --git a/gns3server/api/routes/compute/vpcs_nodes.py b/gns3server/api/routes/compute/vpcs_nodes.py index 251e18e16..546fabdb0 100644 --- a/gns3server/api/routes/compute/vpcs_nodes.py +++ b/gns3server/api/routes/compute/vpcs_nodes.py @@ -390,21 +390,27 @@ async def resume_vpcs_markers(node: VPCSVM = Depends(dep_node)) -> None: @router.delete( - "/{node_id}/markers/{marker_name}", + "/{node_id}/adapters/{adapter_number}/ports/{port_number}/markers/{marker_name}", status_code=status.HTTP_204_NO_CONTENT, dependencies=[Depends(compute_authentication)] ) async def delete_vpcs_marker_capture( + *, marker_name: str, + adapter_number: int = Path(..., ge=0, le=0), + port_number: int, link_id: str = "", node: VPCSVM = Depends(dep_node) ) -> None: """ Delete a marker's capture pcap (called by the controller when the marker is - removed) so the file is cleaned up even with the node stopped. + removed) so the file is cleaned up even with the node stopped. Also drops + the marker from the port NIO's cached spec so a node restart won't reinstall + it (and recreate an empty pcap). """ - await node.delete_marker_capture(marker_name, link_id) + nio = node.get_nio(port_number) + await node.delete_marker_capture(marker_name, link_id, nio) @router.put( diff --git a/gns3server/compute/base_node.py b/gns3server/compute/base_node.py index fca52ce31..76d543364 100644 --- a/gns3server/compute/base_node.py +++ b/gns3server/compute/base_node.py @@ -1134,7 +1134,7 @@ class BaseNode: # bad expression must surface instead of being silently dropped. await self._ubridge_send(cmd) - async def delete_marker_capture(self, name, link_id): + async def delete_marker_capture(self, name, link_id, nio=None): """ Remove a marker from uBridge (fine-grained ``delete_packet_filter`` — NOT reset_packet_filters, so sibling markers' pcaps aren't closed/reopened) @@ -1142,7 +1142,15 @@ class BaseNode: removed; safe with the node stopped (filter removal is skipped, the file is still unlinked). IOU overrides ``_ubridge_delete_marker_filter`` for its ``iol_bridge`` command shape. + + ``nio`` is the port NIO whose cached ``nio.markers`` carries this marker + spec; it is dropped here so a later node start / NIO reapply + (``_ubridge_apply_markers``) does not reinstall the marker. Without this, + deleting a marker while the node is stopped left the spec in + ``nio.markers``, and starting the node recreated an empty pcap. """ + if nio is not None and getattr(nio, "markers", None): + nio.markers.pop(name, None) bridge_name = self._marker_filter_bridges.pop((name, link_id), None) if bridge_name is not None: await self._ubridge_delete_marker_filter(bridge_name, name) diff --git a/gns3server/controller/udp_link.py b/gns3server/controller/udp_link.py index 0c5f2eeca..68b06b27f 100644 --- a/gns3server/controller/udp_link.py +++ b/gns3server/controller/udp_link.py @@ -444,7 +444,10 @@ class UDPLink(Link): side = next((s for s in self._nodes if str(s["node"].id) == str(capture_node_id)), None) if side is not None: try: - await side["node"].delete(f"/markers/{name}", params={"link_id": self._id}) + await side["node"].delete( + f"/adapters/{side['adapter_number']}/ports/{side['port_number']}/markers/{name}", + params={"link_id": self._id}, + ) except Exception: pass # best-effort: old compute without the route leaves the file self._project.emit_notification("link.updated", self.asdict()) diff --git a/tests/compute/test_base_node.py b/tests/compute/test_base_node.py index 2935e990b..3eaadb9f9 100644 --- a/tests/compute/test_base_node.py +++ b/tests/compute/test_base_node.py @@ -273,6 +273,20 @@ async def test_delete_marker_capture_sends_delete_filter(compute_project, manage assert ("m", "L1") not in node._marker_filter_bridges +@pytest.mark.asyncio +async def test_delete_marker_capture_drops_from_nio_markers(compute_project, manager): + # The marker spec cached on the port NIO (nio.markers) is what + # _ubridge_apply_markers reads on node start. delete_marker_capture must drop + # it, else deleting a marker while the node is stopped leaves the spec in + # nio.markers and starting the node reinstalls it (empty pcap reappears). + node = VPCSVM("test", "00010203-0405-0607-0809-0a0b0c0d0e0f", compute_project, manager) + nio = NIOUDP(1234, "127.0.0.1", 4321) + nio.markers = {"m": {"bpf": "icmp", "tag": None, "link_id": "L1", + "direction": None, "enabled": True}} + await node.delete_marker_capture("m", "L1", nio) + assert "m" not in nio.markers + + @pytest.mark.asyncio async def test_rebuild_marker_filter_delete_then_add(compute_project, manager): # rebuild re-installs a single filter (delete_packet_filter + add) with the diff --git a/tests/controller/test_marker.py b/tests/controller/test_marker.py index 9c13edbf8..d57157f2d 100644 --- a/tests/controller/test_marker.py +++ b/tests/controller/test_marker.py @@ -771,4 +771,6 @@ async def test_stop_marker_deletes_capture_pcap(project): capture.delete = AsyncioMagicMock() await link.stop_marker("icmp") - capture.delete.assert_called_once_with("/markers/icmp", params={"link_id": link.id}) + capture.delete.assert_called_once_with( + "/adapters/0/ports/0/markers/icmp", params={"link_id": link.id} + )