Merge pull request #3028 from tikD1337/fix/switch-stale-bridge

fix(ethernet-switch): release a leftover bridge's ports before the crash-recovery delete

gns3server/compute/builtin/nodes/ethernet_switch.py

@@ -230,13 +230,27 @@ class EthernetSwitch(KernelDatapathMixin, BaseNode):
         this switch's UUID (kernel interface names are ≤ 15 chars).  A stale
         bridge from a previous crash is deleted first so ``brctl create`` never
         hits EEXIST.
+
+        A server killed before ``close()`` leaves that bridge with its ports
+        still enslaved: a peer's persistent anchor TAP or a cascade veth end
+        outlives uBridge, and uBridge's ``brctl delete`` refuses a bridge that
+        still has ports (EBUSY). Every leftover member is released first
+        (same-host sysfs listing); no port of this switch is wired yet, and
+        the re-wiring re-enslaves the ones that are still linked.
         """
 
         if self._bridge_created:
             return
         # deterministic short name — 10 chars, always fits the 15-char kernel cap
         self._bridge_name = "gns3" + self._id.replace("-", "")[:6]
-        # crash recovery: best-effort delete any leftover bridge
+        # crash recovery: release any leftover bridge's ports, then delete it
+        try:
+            leftover_members = os.listdir(f"/sys/class/net/{self._bridge_name}/brif")
+        except OSError:
+            leftover_members = []  # no leftover bridge
+        for member in leftover_members:
+            with contextlib.suppress(UbridgeError):
+                await self._ubridge_send(f'brctl delif "{self._bridge_name}" "{member}"')
         try:
             await self._ubridge_send(f'brctl delete "{self._bridge_name}"')
         except UbridgeError:

tests/api/routes/compute/test_ethernet_switch_nodes.py

@@ -92,6 +92,69 @@ class TestEthernetSwitchNodesRoutes:
             ]
         )
 
+    async def test_ethernet_switch_create_releases_a_leftover_bridge(
+        self, app: FastAPI, compute_client: AsyncClient, compute_project: Project
+    ) -> None:
+        """
+        A server killed before close() leaves the switch's kernel bridge
+        behind, with a peer's persistent anchor TAP still enslaved (the
+        anchor outlives uBridge). uBridge refuses to delete a bridge that
+        still has ports (EBUSY), so the crash recovery must release the
+        leftover members first: otherwise ``brctl create`` hits EEXIST and
+        the switch -- and the project holding it -- can no longer be opened.
+        """
+
+        node_id = "ada84b51-ab2c-4d3e-8f40-0123456789ab"
+        br = "gns3ada84b"  # "gns3" + the first 6 hex chars of the node id
+        brif = f"/sys/class/net/{br}/brif"
+        leftover = "gq00010203e0p0"
+        members = {leftover}
+        bridge = {"exists": True}
+
+        async def ubridge(command):
+            # uBridge's brctl: delete refuses a bridge that still has ports,
+            # create refuses an existing one.
+            if command == f'brctl delif "{br}" "{leftover}"':
+                members.discard(leftover)
+            elif command == f'brctl delete "{br}"':
+                if members:
+                    raise UbridgeError(f"Could not delete bridge {br}: Device or resource busy")
+                bridge["exists"] = False
+            elif command == f'brctl create "{br}"':
+                if bridge["exists"]:
+                    raise UbridgeError(f"Could not create bridge {br}: File exists")
+                bridge["exists"] = True
+
+        real_listdir = os.listdir
+
+        def listdir(path):
+            if str(path) == brif:
+                if not bridge["exists"]:
+                    raise FileNotFoundError(path)
+                return sorted(members)
+            return real_listdir(path)
+
+        with (
+            asyncio_patch(f"{_NODE}._ubridge_send", side_effect=ubridge) as send,
+            patch("os.listdir", new=listdir),
+        ):
+            response = await compute_client.post(
+                app.url_path_for("compute:create_ethernet_switch", project_id=compute_project.id),
+                json={"name": "Ethernet Switch 1", "node_id": node_id},
+            )
+        assert response.status_code == status.HTTP_201_CREATED
+        assert compute_project.get_node(node_id)._bridge_name == br
+        send.assert_has_calls(
+            [
+                call(f'brctl delif "{br}" "{leftover}"'),
+                call(f'brctl delete "{br}"'),
+                call(f'brctl create "{br}"'),
+                call(f'link set "{br}" up'),
+                call(f'brctl vlanfiltering "{br}" on'),
+            ]
+        )
+        assert not members
+
     async def test_ethernet_switch_get(
         self, app: FastAPI, compute_client: AsyncClient, compute_project: Project, ethernet_switch: dict
     ) -> None: