mirror of
https://github.com/GNS3/gns3-server.git
synced 2026-08-27 12:30:13 +03:00
marker: drop deleted marker from port NIO cache to stop empty pcap on restart
Deleting a marker while its node was stopped, then starting the node, recreated
an empty pcap. Root cause: delete_marker_capture removed the uBridge filter and
the pcap file but not the marker spec cached on the port NIO (nio.markers) —
the data source _ubridge_apply_markers reads on node start. The stale spec
reinstalled the marker when uBridge came up.
This was a regression from switching stop_marker off update() (which re-sent
the NIO and implicitly refreshed nio.markers) to the fine-grained
node.delete(/markers/{name}) path.
Fix: make the delete port-aware so the compute can locate the NIO. The DELETE
marker route becomes /adapters/{a}/ports/{p}/markers/{name} across all six
node types; the handler resolves the NIO via get_nio and passes it to
delete_marker_capture, which now pops the marker from nio.markers. get_nio
works regardless of uBridge state, so the stopped-node case is covered. The
controller's stop_marker targets the capture side's adapter/port.
This commit is contained in:
parent
1a0ce51f38
commit
75f278228b
@ -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")
|
||||
|
||||
@ -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(
|
||||
|
||||
@ -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(
|
||||
|
||||
@ -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(
|
||||
|
||||
@ -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(
|
||||
|
||||
@ -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(
|
||||
|
||||
@ -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)
|
||||
|
||||
@ -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())
|
||||
|
||||
@ -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
|
||||
|
||||
@ -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}
|
||||
)
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user