From 1e999d916da6cc1d1f9309ecddde55e6d3621db7 Mon Sep 17 00:00:00 2001 From: YueGuobin Date: Fri, 2 Oct 2026 19:44:10 +0800 Subject: [PATCH] fix(controller): keep port links across a port-list rebuild MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Node._list_ports() rebuilds every Port object, and it runs on each node update — including every compute node.updated notification (a node reporting started is one). The links are unaffected by that rebuild, but the fresh port objects started out with link=None, so after any node update the "Port is already used" guard in Link.add_node stopped matching: a second link on an occupied port reached the compute, where the port's NIO was replaced before the attach failed on the already-bound anchor, leaving the link half-torn-down (on IOU, deleting the project afterwards left the node's anchor TAPs and a per-link bridge on the host). Carry the link back-references over by (adapter_number, port_number), first match wins, mirroring get_port; a port that still exists keeps its link. Regression tests pin both the reference (test_node) and the guard (test_link: the duplicate link is refused after a node update). --- gns3server/controller/node.py | 26 ++++++++++++++++++++++++++ tests/controller/test_link.py | 29 +++++++++++++++++++++++++++++ tests/controller/test_node.py | 17 +++++++++++++++++ 3 files changed, 72 insertions(+) diff --git a/gns3server/controller/node.py b/gns3server/controller/node.py index 3500cb2db..c8d20f10a 100644 --- a/gns3server/controller/node.py +++ b/gns3server/controller/node.py @@ -1148,6 +1148,32 @@ class Node: return None def _list_ports(self): + """ + Rebuild the port list, preserving the link back-references. + + The list is rebuilt on every node update (a property change or a + compute ``node.updated`` notification), which creates fresh Port + objects — but the links themselves are unaffected by that rebuild, + so a port that still exists must keep pointing at its link. Without + this the "Port is already used" guard in ``Link.add_node`` silently + stops matching after any node update and a second link gets created + on an occupied port (the compute then fails on the already-bound + NIO). Links are carried over by (adapter, port), first match wins, + mirroring ``get_port``. + """ + preserved = {} + for port in self._ports or []: + if port.link is not None: + preserved.setdefault((port.adapter_number, port.port_number), port.link) + + self._rebuild_ports() + + for port in self._ports: + link = preserved.pop((port.adapter_number, port.port_number), None) + if link is not None and port.link is None: + port.link = link + + def _rebuild_ports(self): """ Generate the list of port display in the client if the compute has sent a list we return it (use by diff --git a/tests/controller/test_link.py b/tests/controller/test_link.py index 186e4c332..23e2d8f35 100644 --- a/tests/controller/test_link.py +++ b/tests/controller/test_link.py @@ -112,6 +112,35 @@ async def test_add_node_already_connected(project, compute): await link2.add_node(node1, 0, 4) +@pytest.mark.asyncio +async def test_add_node_already_connected_after_node_update(project, compute): + """ + A node update (any compute node.updated notification) rebuilds its port + objects; the port must still count as used afterwards, or the duplicate + link reaches the compute and fails on the already-bound NIO instead of + being refused here. + """ + + project.dump = AsyncioMagicMock() + + node1 = Node(project, compute, "node1", node_type="qemu") + node2 = Node(project, compute, "node2", node_type="qemu") + + link = Link(project) + link.create = AsyncioMagicMock() + link.node_updated = AsyncioMagicMock() + link._project.emit_notification = MagicMock() + await link.add_node(node1, 0, 0) + await link.add_node(node2, 0, 0) + + await node1.parse_node_response({"status": "started"}) + + link2 = Link(project) + link2.create = AsyncioMagicMock() + with pytest.raises(ControllerError, match="already used"): + await link2.add_node(node1, 0, 0) + + @pytest.mark.asyncio async def test_add_node_cloud(project, compute): diff --git a/tests/controller/test_node.py b/tests/controller/test_node.py index 2c634c8f9..788b560bb 100644 --- a/tests/controller/test_node.py +++ b/tests/controller/test_node.py @@ -1191,3 +1191,20 @@ async def test_parse_node_response(node): node.add_link(link) await node.parse_node_response({"status": "started"}) assert link.node_updated.called + + +@pytest.mark.asyncio +async def test_port_link_survives_port_rebuild(node): + """ + A node update rebuilds the port objects (every compute node.updated + notification does); the link back-references must survive the rebuild, + or the "Port is already used" guard silently stops matching. + """ + + port = node.get_port(0, 0) + link = MagicMock() + port.link = link + await node.parse_node_response({"status": "started"}) + rebuilt = node.get_port(0, 0) + assert rebuilt is not port # the rebuild really does replace the objects + assert rebuilt.link is link