diff --git a/gns3server/api/routes/controller/links.py b/gns3server/api/routes/controller/links.py index a6d27e044..b67fd0790 100644 --- a/gns3server/api/routes/controller/links.py +++ b/gns3server/api/routes/controller/links.py @@ -32,7 +32,7 @@ from fastapi.responses import FileResponse, StreamingResponse from gns3server import schemas from gns3server.agent.web_wireshark.manager import WebWiresharkManager from gns3server.controller import Controller -from gns3server.controller.controller_error import ControllerError +from gns3server.controller.controller_error import ControllerError, ControllerNotFoundError from gns3server.controller.link import _UNSET, Link from gns3server.db.repositories.rbac import RbacRepository from gns3server.utils.http_client import HTTPClient @@ -59,6 +59,29 @@ responses: dict[int | str, dict[str, Any]] = { router = APIRouter(responses=responses) +def _select_port(node, endpoint, other_port): + if endpoint.get("port_name") is not None: + port = node.get_port_by_name(endpoint["port_name"]) + if port is None: + raise ControllerNotFoundError(f"Port named {endpoint['port_name']} for {node.name} not found") + return port + if endpoint.get("port") == "auto": + port = node.get_free_port(other_port.link_type if other_port else None) + if port is None: + raise ControllerError(f"No free port available on {node.name}", code="no_free_port") + return port + return None + + +def _peer_port(project, endpoint): + if endpoint.get("port") == "auto": + return None + node = project.get_node(endpoint["node_id"]) + if endpoint.get("port_name") is not None: + return node.get_port_by_name(endpoint["port_name"]) + return node.get_port(endpoint["adapter_number"], endpoint["port_number"]) + + async def dep_link(project_id: UUID, link_id: UUID) -> Link: """ Dependency to retrieve a link. @@ -118,14 +141,18 @@ async def create_link(project_id: UUID, link_create: schemas.LinkCreate) -> sche if "show_filters_icon" in link_data: await link.update_show_filters_icon(link_data["show_filters_icon"]) try: - for node in link_data["nodes"]: - await link.add_node( - project.get_node(node["node_id"]), - node.get("adapter_number", 0), - node.get("port_number", 0), - label=node.get("label"), - ) + endpoints = link_data["nodes"] + attached_port = None + for index, endpoint in enumerate(endpoints): + node = project.get_node(endpoint["node_id"]) + peer_port = attached_port or _peer_port(project, endpoints[1 - index]) + port = _select_port(node, endpoint, peer_port) + adapter_number = port.adapter_number if port else endpoint["adapter_number"] + port_number = port.port_number if port else endpoint["port_number"] + await link.add_node(node, adapter_number, port_number, label=endpoint.get("label")) + attached_port = node.get_port(adapter_number, port_number) except ControllerError as e: + link.release_ports() await project.delete_link(link.id) raise e return link.asdict() diff --git a/gns3server/controller/link.py b/gns3server/controller/link.py index 3ac5c7317..50f7cbe8f 100644 --- a/gns3server/controller/link.py +++ b/gns3server/controller/link.py @@ -326,17 +326,29 @@ class Link: {"node": node, "adapter_number": adapter_number, "port_number": port_number, "port": port, "label": label} ) + if not batch: + port.link = self + if len(self._nodes) == 2 and not batch: - await self.create() + try: + await self.create() + except BaseException: + self.release_ports() + raise for n in self._nodes: n["node"].add_link(self) - n["port"].link = self self._created = True self._project.emit_notification("link.created", self.asdict()) if dump: self._project.dump() + def release_ports(self): + for n in self._nodes: + if n["port"].link == self: + n["port"].link = None + n["node"].links.discard(self) + async def update_nodes(self, nodes): for node_data in nodes: node = self._project.get_node(node_data["node_id"]) @@ -365,11 +377,7 @@ class Link: """ Delete the link """ - for n in self._nodes: - # It could be different of self if we rollback an already existing link - if n["port"].link == self: - n["port"].link = None - n["node"].remove_link(self) + self.release_ports() async def reset(self): """ diff --git a/gns3server/controller/node.py b/gns3server/controller/node.py index 3dc0bbc10..2f37d7332 100644 --- a/gns3server/controller/node.py +++ b/gns3server/controller/node.py @@ -1195,6 +1195,27 @@ class Node: return port return None + def get_port_by_name(self, name): + """ + Return the port with this name or None if the port is not found + """ + for port in self.ports: + if port.name == name: + return port + return None + + def get_free_port(self, link_type=None): + """ + Return the lowest free port, optionally restricted to a link type, + or None if there is no such port + """ + free_ports = [ + port for port in self.ports if port.link is None and (link_type is None or port.link_type == link_type) + ] + if not free_ports: + return None + return min(free_ports, key=lambda port: (port.adapter_number, port.port_number)) + def _list_ports(self): """ Generate the list of port display in the client diff --git a/gns3server/controller/ports/port.py b/gns3server/controller/ports/port.py index 9d45cbda1..4782f885c 100644 --- a/gns3server/controller/ports/port.py +++ b/gns3server/controller/ports/port.py @@ -41,6 +41,10 @@ class Port: def link(self, val): self._link = val + @property + def name(self): + return self._name + @property def adapter_number(self): return self._adapter_number diff --git a/gns3server/schemas/controller/links.py b/gns3server/schemas/controller/links.py index 88210b556..a1b741440 100644 --- a/gns3server/schemas/controller/links.py +++ b/gns3server/schemas/controller/links.py @@ -18,7 +18,7 @@ from enum import Enum from typing import Annotated, List, Literal, Optional, Tuple from uuid import UUID, uuid4 -from pydantic import BaseModel, ConfigDict, Field, field_validator +from pydantic import BaseModel, ConfigDict, Field, field_validator, model_validator from ..update import PartialUpdateModel from .labels import Label @@ -35,6 +35,32 @@ class LinkNode(BaseModel): label: Optional[Label] = None +class LinkNodeCreate(BaseModel): + """ + Link node data for link creation. + + The port is given either by adapter_number and port_number, by port_name + or by port set to "auto" (first free compatible port). + """ + + node_id: UUID + adapter_number: Optional[int] = None + port_number: Optional[int] = None + port_name: Optional[str] = Field(None, description="Name of the port on the node") + port: Optional[Literal["auto"]] = Field(None, description='Use "auto" to select the first free compatible port') + label: Optional[Label] = None + + @model_validator(mode="after") + def check_port_selector(self): + numbered = self.adapter_number is not None or self.port_number is not None + if numbered and (self.adapter_number is None or self.port_number is None): + raise ValueError("adapter_number and port_number must be given together") + forms = sum([numbered, self.port_name is not None, self.port is not None]) + if forms != 1: + raise ValueError("exactly one of adapter_number and port_number, port_name or port must be given") + return self + + class LinkType(str, Enum): """ Link type. @@ -142,7 +168,8 @@ class LinkBase(BaseModel): class LinkCreate(LinkBase): link_id: UUID = Field(default_factory=uuid4) - nodes: List[LinkNode] = Field(..., min_length=2, max_length=2) + # LinkNodeCreate narrows the port selector, so it cannot be a LinkNode subclass + nodes: List[LinkNodeCreate] = Field(..., min_length=2, max_length=2) # type: ignore[assignment] class LinkUpdate(PartialUpdateModel, LinkBase): diff --git a/tests/api/routes/controller/test_links.py b/tests/api/routes/controller/test_links.py index 1e845172c..ba1d018d0 100644 --- a/tests/api/routes/controller/test_links.py +++ b/tests/api/routes/controller/test_links.py @@ -115,6 +115,110 @@ class TestLinkRoutes: assert response.status_code == status.HTTP_409_CONFLICT assert len(project.links) == 0 + async def test_create_link_by_port_name( + self, app: FastAPI, client: AsyncClient, project: Project, nodes: Tuple[Node, Node] + ) -> None: + + node1, node2 = nodes + + with asyncio_patch("gns3server.controller.udp_link.UDPLink.create"): + response = await client.post( + app.url_path_for("create_link", project_id=project.id), + json={ + "nodes": [ + {"node_id": node1.id, "port_name": "E0"}, + {"node_id": node2.id, "adapter_number": 2, "port_number": 4}, + ] + }, + ) + + assert response.status_code == status.HTTP_201_CREATED + first, second = response.json()["nodes"] + assert (first["adapter_number"], first["port_number"]) == (0, 3) + assert (second["adapter_number"], second["port_number"]) == (2, 4) + + async def test_create_link_unknown_port_name( + self, app: FastAPI, client: AsyncClient, project: Project, nodes: Tuple[Node, Node] + ) -> None: + + node1, node2 = nodes + + response = await client.post( + app.url_path_for("create_link", project_id=project.id), + json={ + "nodes": [ + {"node_id": node1.id, "port_name": "nope"}, + {"node_id": node2.id, "adapter_number": 2, "port_number": 4}, + ] + }, + ) + + assert response.status_code == status.HTTP_404_NOT_FOUND + assert len(project.links) == 0 + + async def test_create_link_auto_port( + self, app: FastAPI, client: AsyncClient, project: Project, nodes: Tuple[Node, Node] + ) -> None: + + node1, node2 = nodes + node1._ports = [EthernetPort("E1", 0, 0, 5), EthernetPort("E0", 0, 0, 3)] + + with asyncio_patch("gns3server.controller.udp_link.UDPLink.create"): + response = await client.post( + app.url_path_for("create_link", project_id=project.id), + json={"nodes": [{"node_id": node1.id, "port": "auto"}, {"node_id": node2.id, "port": "auto"}]}, + ) + assert response.status_code == status.HTTP_201_CREATED + assert response.json()["nodes"][0]["port_number"] == 3 + + response = await client.post( + app.url_path_for("create_link", project_id=project.id), + json={"nodes": [{"node_id": node1.id, "port": "auto"}, {"node_id": node2.id, "port": "auto"}]}, + ) + + assert response.status_code == status.HTTP_409_CONFLICT + assert response.json()["code"] == "no_free_port" + assert len(project.links) == 1 + + async def test_create_link_port_in_use_code( + self, app: FastAPI, client: AsyncClient, project: Project, nodes: Tuple[Node, Node] + ) -> None: + + node1, node2 = nodes + body = { + "nodes": [ + {"node_id": node1.id, "adapter_number": 0, "port_number": 3}, + {"node_id": node2.id, "adapter_number": 2, "port_number": 4}, + ] + } + + with asyncio_patch("gns3server.controller.udp_link.UDPLink.create"): + first = await client.post(app.url_path_for("create_link", project_id=project.id), json=body) + second = await client.post(app.url_path_for("create_link", project_id=project.id), json=body) + + assert first.status_code == status.HTTP_201_CREATED + assert second.status_code == status.HTTP_409_CONFLICT + assert second.json()["code"] == "port_in_use" + assert len(project.links) == 1 + + async def test_create_link_invalid_port_selector( + self, app: FastAPI, client: AsyncClient, project: Project, nodes: Tuple[Node, Node] + ) -> None: + + node1, node2 = nodes + valid = {"node_id": node2.id, "adapter_number": 2, "port_number": 4} + + for bad in ( + {"node_id": node1.id}, + {"node_id": node1.id, "adapter_number": 0}, + {"node_id": node1.id, "port_name": "E0", "port": "auto"}, + {"node_id": node1.id, "adapter_number": 0, "port_number": 3, "port_name": "E0"}, + ): + response = await client.post( + app.url_path_for("create_link", project_id=project.id), json={"nodes": [bad, valid]} + ) + assert response.status_code == status.HTTP_422_UNPROCESSABLE_ENTITY + async def test_get_link( self, app: FastAPI, client: AsyncClient, project: Project, nodes: Tuple[Node, Node] ) -> None: diff --git a/tests/controller/test_link.py b/tests/controller/test_link.py index 12436abb5..f286bb58f 100644 --- a/tests/controller/test_link.py +++ b/tests/controller/test_link.py @@ -15,6 +15,7 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see . +import asyncio from unittest.mock import MagicMock import pytest @@ -417,3 +418,67 @@ async def test_update_link_style(project, compute): } link._project.emit_notification.assert_called_with("link.updated", link.asdict()) assert project.dump.called + + +@pytest.mark.asyncio +async def test_add_node_concurrent_same_port(project, compute): + project.dump = MagicMock() + node1 = Node(project, compute, "node1", node_type="qemu") + node1._ports = [EthernetPort("E0", 0, 0, 4)] + node2 = Node(project, compute, "node2", node_type="qemu") + node2._ports = [EthernetPort("E0", 0, 0, 4), EthernetPort("E1", 0, 0, 5)] + node3 = Node(project, compute, "node3", node_type="qemu") + node3._ports = [EthernetPort("E0", 0, 0, 4)] + + async def slow_create(): + await asyncio.sleep(0) + + async def connect(other, other_port): + link = Link(project) + link.create = slow_create + try: + await link.add_node(other, 0, other_port) + await link.add_node(node1, 0, 4) + except ControllerError as e: + return e + return link + + results = await asyncio.gather(connect(node2, 4), connect(node3, 4)) + errors = [r for r in results if isinstance(r, ControllerError)] + assert len(errors) == 1 + assert errors[0].code == "port_in_use" + assert node1._ports[0].link in results + + +@pytest.mark.asyncio +async def test_add_node_create_failure_releases_ports(project, compute): + project.dump = MagicMock() + node1 = Node(project, compute, "node1", node_type="qemu") + node1._ports = [EthernetPort("E0", 0, 0, 4)] + node2 = Node(project, compute, "node2", node_type="qemu") + node2._ports = [EthernetPort("E0", 0, 0, 4)] + + link = Link(project) + link.create = AsyncioMagicMock(side_effect=ControllerError("boom")) + await link.add_node(node1, 0, 4) + with pytest.raises(ControllerError): + await link.add_node(node2, 0, 4) + + assert node1._ports[0].link is None + assert node2._ports[0].link is None + assert not node1.links + assert not node2.links + + +def test_node_get_port_by_name_and_free_port(project, compute): + node = Node(project, compute, "node1", node_type="qemu") + node._ports = [EthernetPort("E1", 0, 0, 1), EthernetPort("E0", 0, 0, 0), SerialPort("S0", 0, 1, 0)] + + assert node.get_port_by_name("E0") is node._ports[1] + assert node.get_port_by_name("nope") is None + assert node.get_free_port() is node._ports[1] + assert node.get_free_port("serial") is node._ports[2] + node._ports[1].link = object() + assert node.get_free_port("ethernet") is node._ports[0] + node._ports[0].link = object() + assert node.get_free_port("ethernet") is None