mirror of
https://github.com/GNS3/gns3-server.git
synced 2026-08-27 20:40:13 +03:00
fix: scope marker reconcile to the current bridge/NIO
The reconcile pass in _ubridge_apply_markers walked the node-wide _marker_filter_bridges map but compared against `desired`, which only carries the markers of the NIO being updated. Updating any one link therefore deleted every other link's markers (and their pcaps) on that node — a regression from the add-only→reconcile switch. IOU's override had the same flaw across its ports. Guard the delete pass with the current bridge (base_node) / IOL location (IOU) so only markers on the NIO being reconciled can be removed. Added a regression test that fails without the guard.
This commit is contained in:
parent
5de902c8b1
commit
958aa59d7e
@ -1256,7 +1256,13 @@ class BaseNode:
|
||||
desired = {(name, spec.get("link_id", "")): spec for name, spec in markers.items()}
|
||||
|
||||
# 1. Remove installed markers that are no longer desired (marker/def delete).
|
||||
# Scope to THIS bridge: the map is node-wide and also holds markers
|
||||
# installed on this node's other links/NIOs. Without this guard,
|
||||
# reconciling one NIO would delete every other link's markers + pcaps
|
||||
# (desired only carries the current NIO's markers) — a regression.
|
||||
for key in list(self._marker_filter_bridges):
|
||||
if self._marker_filter_bridges[key] != bridge_name:
|
||||
continue
|
||||
if key not in desired:
|
||||
mname, link_id = key
|
||||
installed_bridge = self._marker_filter_bridges.pop(key)
|
||||
|
||||
@ -1291,7 +1291,12 @@ class IOUVM(BaseNode):
|
||||
desired = {(name, spec.get("link_id", "")): spec for name, spec in markers.items()}
|
||||
|
||||
# 1. Remove installed markers that are no longer desired.
|
||||
# Scope to THIS port's IOL location — the map is node-wide and also
|
||||
# holds markers on this IOU's other ports, which must not be deleted
|
||||
# when reconciling a single NIO (see base_node for the same guard).
|
||||
for key in list(self._marker_filter_bridges):
|
||||
if self._marker_filter_bridges[key] != location:
|
||||
continue
|
||||
if key not in desired:
|
||||
mname, link_id = key
|
||||
installed_location = self._marker_filter_bridges.pop(key)
|
||||
|
||||
@ -410,6 +410,36 @@ async def test_apply_markers_rebuilds_changed_bpf(compute_project, manager):
|
||||
assert any("add_packet_filter VPCS-10 m mark" in c and "tcp" in c for c in cmds) # new added
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_apply_markers_preserves_markers_on_other_bridges(compute_project, manager):
|
||||
# Regression: reconciling one NIO must not delete markers installed on this
|
||||
# node's OTHER bridges/NIOs. _marker_filter_bridges is node-wide, but
|
||||
# `desired` only carries the current NIO's markers — the delete pass must be
|
||||
# scoped to the current bridge, or updating one link wipes every other link's
|
||||
# markers + pcaps.
|
||||
node = VPCSVM("test", "00010203-0405-0607-0809-0a0b0c0d0e0f", compute_project, manager)
|
||||
node._ubridge_send = AsyncioMagicMock()
|
||||
node._ubridge_hypervisor = MagicMock()
|
||||
node._ubridge_hypervisor.is_running.return_value = True
|
||||
# Two links, each with a marker on its own bridge:
|
||||
node._marker_filter_bridges["m", "L1"] = "VPCS-10"
|
||||
node._marker_specs["m", "L1"] = {"bpf": "icmp", "enabled": True}
|
||||
node._marker_filter_bridges["m", "L2"] = "VPCS-20"
|
||||
node._marker_specs["m", "L2"] = {"bpf": "tcp", "enabled": True}
|
||||
# Update only the VPCS-10 NIO; "m" is gone from this link — but the marker
|
||||
# on VPCS-20 (L2) must survive untouched.
|
||||
nio = NIOUDP(1234, "127.0.0.1", 4321)
|
||||
nio.markers = {}
|
||||
with patch("gns3server.compute.marker.marker_manager.MarkerManager") as mm:
|
||||
mm.instance.return_value.unregister = MagicMock()
|
||||
await node._ubridge_apply_markers("VPCS-10", nio)
|
||||
cmds = [c.args[0] for c in node._ubridge_send.call_args_list]
|
||||
assert any("delete_packet_filter VPCS-10 m" in c for c in cmds) # L1 removed on its bridge
|
||||
assert not any("VPCS-20" in c for c in cmds) # other bridge untouched
|
||||
assert ("m", "L1") not in node._marker_filter_bridges
|
||||
assert ("m", "L2") in node._marker_filter_bridges # L2 preserved
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_stop_ubridge_clears_marker_bridges(compute_project, manager):
|
||||
# uBridge stopping drops every marker filter — the map must clear so the next
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user