diff --git a/.claude/memory/MEMORY.md b/.claude/memory/MEMORY.md index 59456d279..7956d4d04 100644 --- a/.claude/memory/MEMORY.md +++ b/.claude/memory/MEMORY.md @@ -31,3 +31,6 @@ ### MCP Service - **[MCP Service Design](./mcp-service-design.md)** - MCP (Model Context Protocol) service architecture using FastMCP with SSE transport, JWT auth, 29 tools across 5 domains - **[MCP Tool Description Location](./mcp-tool-description-guide.md)** - Where to define MCP tool descriptions: in `@mcp.tool()` functions in `__init__.py`, not in `*_TOOLS` arrays + +### Python Code Verification +- **[Import Validation](./python-import-validation.md)** - Use actual module imports (`python -c "from ... import ..."`) instead of `py_compile` to catch missing imports diff --git a/.claude/memory/python-import-validation.md b/.claude/memory/python-import-validation.md new file mode 100644 index 000000000..18d3a012e --- /dev/null +++ b/.claude/memory/python-import-validation.md @@ -0,0 +1,30 @@ +# Python Import Validation + +## Background + +When checking if modified Python code is correct, `py_compile` only validates syntax (e.g., balanced parentheses, valid keywords). It does **not** catch missing imports or other runtime errors (e.g., using `UUID()` without importing `UUID`). + +## Decision/Implementation + +Use actual module imports to verify code correctness: + +```bash +# ✅ This catches missing imports and runtime errors +venv/bin/python -c " +from gns3server.api.routes.controller.dependencies.authentication import get_user_from_token +from gns3server.api.routes.mcp.__init__ import _resolve_token +print('All imports OK') +" + +# ❌ This only checks syntax, not references +venv/bin/python -c "import py_compile; py_compile.compile('file.py', doraise=True)" +``` + +## Related Files + +`gns3server/api/routes/controller/dependencies/authentication.py` — missed `from uuid import UUID` +`gns3server/api/routes/mcp/__init__.py` — missed `from uuid import UUID` + +## Why + +A `NameError` at runtime is far more expensive than a failed import check. Real import testing catches the full dependency chain. diff --git a/docs/features/mcp-service.md b/docs/features/mcp-service.md index 166adacbf..ccd02b61c 100644 --- a/docs/features/mcp-service.md +++ b/docs/features/mcp-service.md @@ -43,7 +43,9 @@ jwt_access_token_expire_minutes = 1440 ; 24 hours ### Option 2: API Key (permanent, revocable) — Recommended for MCP -API keys never expire and can be revoked individually. Create one via the REST API: +API keys never expire and can be revoked individually. Format: `gns3__` — the embedded UUID enables O(1) lookup without scanning all keys. + +Create one via the REST API: ```bash # Create an API key (requires a JWT to authenticate) @@ -51,7 +53,7 @@ curl -X POST http://localhost:3080/v3/access/api-keys \ -H "Authorization: Bearer " \ -H "Content-Type: application/json" \ -d '{"name": "MCP Production"}' -# Response: {"api_key": "gns3_a1b2c3d4...", "api_key_id": "...", ...} +# Response: {"api_key": "gns3_550e8400-e29b-41d4-a716-446655440000_a1b2c3d4...", ...} # ⚠️ The key is only shown once — save it immediately. ``` @@ -67,6 +69,29 @@ API key management endpoints: Both JWT tokens and API keys work for MCP and REST API endpoints interchangeably. +### Authentication Flow + +When connecting with an API key: + +``` +SSE connect → Authorization: Bearer gns3__ + ↓ +MCP auth wrapper extracts UUID → single DB query → 1 bcrypt (thread pool) + ↓ +Generates a fresh short-lived JWT → stored in ContextVar for the session + ↓ +All subsequent tool handler REST API calls use this JWT → zero extra bcrypt +``` + +### Concurrency + +| Setting | Value | +|---------|-------| +| MCP batch workers | 100 (`BATCH_MAX_WORKERS`) | +| MCP HTTP client timeout | 30s | +| HTTP connection pool (`pool_connections`/`pool_maxsize`) | 500 / 1000 | +| REST API node/link creation pool | 100 (`Pool(concurrency=100)`) | + ## Available Tools **82 tools** across 12 categories: @@ -97,7 +122,7 @@ Both JWT tokens and API keys work for MCP and REST API endpoints interchangeably |------|-------------| | `node_list` | List all nodes (`fields` to filter columns, e.g. `["name","status"]`) | | `node_get` | Get node details (`fields` to filter columns) | -| `node_create` | Create node(s) — single via `template_id` or batch via `nodes` array | +| `node_create` | Create node(s) — single via `template_id` or batch via `nodes` array. Supports `fields` to filter response. Top-level `template_id` applies as default in batch mode. Pass `name` to override template naming. Coordinates: left-handed Cartesian (origin at canvas center, X right-positive, Y down-positive). | | `node_delete` | Delete a node | | `node_update` | Update node properties | | `node_start` | Start node(s) — `node_id` or `node_ids` array | @@ -124,7 +149,7 @@ Both JWT tokens and API keys work for MCP and REST API endpoints interchangeably |------|-------------| | `link_list` | List all links (`fields` to filter columns) | | `link_get` | Get link details | -| `link_create` | Create link(s) — single via `nodes` or batch via `links` array | +| `link_create` | Create link(s) — single via `nodes` or batch via `links` array. Nodes support compact `[id, ad, pt, id, ad, pt]` format. Supports `fields` to filter response. | | `link_delete` | Delete link(s) — `link_id` or `link_ids` array | | `link_update` | Update link (suspend, filters) | | `link_reset` | Reset link(s) — `link_id` or `link_ids` array | @@ -136,7 +161,7 @@ Both JWT tokens and API keys work for MCP and REST API endpoints interchangeably | Tool | Description | |------|-------------| -| `template_list` | List all templates | +| `template_list` | List all templates. Supports `fields` to filter response columns. | | `template_get` | Get template details | | `template_create` | Create a template (Docker needs `image`) | | `template_update` | Update a template | @@ -210,14 +235,14 @@ Both JWT tokens and API keys work for MCP and REST API endpoints interchangeably | Tool | Description | |------|-------------| | `device_config_send` | Push config commands to devices via console (Nornir + Netmiko). Supports Jinja2 `template` + `vars` | -| `device_command_run` | Run read-only show commands on devices. Supports Jinja2 `template` + `vars` | +| `device_show_run` | Run read-only show commands on devices. Supports Jinja2 `template` + `vars` | | `vpcs_config_set` | Configure VPCS devices (IP, gateway, etc.) | The tool connects to each device's console via telnet/SSH. Nodes must be in the `started` state (use `node_start` or `node_start_all`). Device type is auto-detected from the node's `device_type:` tag in GNS3. #### Jinja2 Template Mode -Both `device_config_send` and `device_command_run` support an optional `template` parameter. When provided, each device's `vars` dict is rendered against the template to produce commands. Entries with the same `device_name` are merged into a single device session. +Both `device_config_send` and `device_show_run` support an optional `template` parameter. When provided, each device's `vars` dict is rendered against the template to produce commands. Entries with the same `device_name` are merged into a single device session. ```python # Direct commands (single/batch) @@ -234,7 +259,7 @@ device_config_send(project_id, ]) # Show commands with template -device_command_run(project_id, +device_show_run(project_id, template="show ip route {{ protocol }}", device_configs=[ {"device_name": "R1", "vars": {"protocol": "ospf"}}, @@ -256,7 +281,7 @@ device_command_run(project_id, ```python # Save config on device -device_command_run(project_id, device_configs=[ +device_show_run(project_id, device_configs=[ {"device_name": "R1", "commands": ["write memory"]}, ]) # Backup @@ -391,23 +416,23 @@ sequenceDiagram participant Auth as Auth participant GNS3 as GNS3 REST API - Note over Client: 1. Connect with credential (JWT or API Key) - Client->>MCP: GET /sse (token in header or query) - MCP->>Auth: Validate Token - Auth-->>MCP: Token Valid + Note over Client: 1. Connect with API Key or JWT + Client->>MCP: GET /sse (Authorization: Bearer ) + + alt API Key (gns3_<uuid>_<secret>) + MCP->>Auth: Extract UUID → DB lookup → 1 bcrypt (thread pool) + Auth-->>MCP: Generate fresh JWT + else JWT + MCP->>Auth: Decode JWT + Auth-->>MCP: Token valid + end + MCP-->>Client: event: endpoint /messages/?session_id=xxx - Note over Client: 2. Initialize - Client->>MCP: POST /messages/ (initialize) - MCP-->>Client: event: message (protocolVersion, capabilities) - - Note over Client: 3. List & Call Tools - Client->>MCP: POST /messages/ (tools/list) - MCP-->>Client: event: message (tools list) - - Client->>MCP: POST /messages/ (tools/call project_list) - MCP->>GNS3: Gns3Connector HTTP request - GNS3-->>MCP: Projects data + Note over Client: 2. Initialize & Call Tools + Client->>MCP: POST /messages/ (tools/call ...) + MCP->>GNS3: HTTP request (with JWT from step 1) + GNS3-->>MCP: Response MCP-->>Client: event: message (tool result) ``` @@ -415,7 +440,7 @@ sequenceDiagram - **FastMCP** (Anthropic MCP SDK) is used for tool registration and SSE transport - The SSE app is mounted as a Starlette sub-application under `/v3/mcp/transport` -- JWT tokens are validated using GNS3's existing `auth_service` +- **Auth:** JWT validation via `auth_service`. API key (`gns3__`) extracts UUID for O(1) DB lookup, runs bcrypt in thread pool, returns a fresh JWT — subsequent calls use the JWT with zero extra bcrypt. - Tool handlers use `Gns3Connector` (from `custom_gns3fy`) to call GNS3's own REST API, keeping the MCP layer decoupled - The JWT token is stored in a `contextvars.ContextVar` so it is available within tool handler threads (Python ≥ 3.9 propagates contextvars through `asyncio.to_thread`) diff --git a/docs/features/project-open-performance.md b/docs/features/project-open-performance.md new file mode 100644 index 000000000..869c90824 --- /dev/null +++ b/docs/features/project-open-performance.md @@ -0,0 +1,148 @@ +# Project Open Performance + +## Overview + +Optimizations to accelerate project opening (`POST /projects/{id}/open`) and node creation for topologies with many nodes and links. The main bottlenecks were sequential link creation, redundant subprocess calls, and SQLite write contention. + +## Before vs After + +| Scenario | Before | After | +|----------|--------|-------| +| 20 IOU nodes + 20 links (project open) | ~2s | ~1s | +| 40 QEMU nodes creation (MCP batch) | ~40s | ~1-2s | + +## Optimizations + +### 1. Parallel Link Creation + +**File:** `gns3server/controller/project.py` + +Links were created sequentially during project loading, each requiring up to 5 HTTP round-trips to the compute. Now uses `Pool(concurrency=100)` for parallel creation. + +```python +# Before: sequential loop +for link_data in topology.get("links", []): + link = await self.add_link(...) + await link.add_node(...) + +# After: parallel Pool +pool = Pool(concurrency=100) +for link_data in topology.get("links", []): + pool.append(self._create_link_from_topology_data, link_data) +await pool.join() +``` + +### 2. Batch UDP Port Allocation + +**Files:** `gns3server/api/routes/compute/compute.py`, `gns3server/controller/project.py`, `gns3server/controller/udp_link.py` + +During project loading, all required UDP ports are pre-allocated per compute in a single batch call before link creation begins. `UDPLink.create()` checks the pre-allocated pool first, falling back to individual allocation if unavailable. + +```python +# New batch endpoint +POST /projects/{id}/ports/udp/batch → {"count": N} → {"udp_ports": [...]} +``` + +### 3. IOU Image Subprocess Cache + +**File:** `gns3server/compute/iou/iou_vm.py` + +Each IOU VM creation spawned `ld-linux --verify` and `iou-image -h` subprocesses. With 20 nodes using the same image, this ran 40 redundant subprocesses. Now results are cached per image path at the class level. + +```python +# Class-level caches shared across all instances +IOUVM._loader_cache = {} # image path → loader command +IOUVM._default_values_cache = {} # image path → (ram, nvram) +``` + +Only the first node with a given image runs the subprocesses; subsequent nodes reuse cached values. + +### 4. SQLite WAL Mode + +**File:** `gns3server/db/tasks.py` + +Write-Ahead Logging allows concurrent reads without blocking on writes. The PRAGMA is registered on `engine.sync_engine` instead of the `Engine` class to correctly fire for async engine connections. + +```python +@event.listens_for(engine.sync_engine, "connect") +def set_sqlite_pragma(dbapi_connection, connection_record): + cursor = dbapi_connection.cursor() + cursor.execute("PRAGMA journal_mode=WAL") + cursor.execute("PRAGMA foreign_keys=ON") + cursor.close() +``` + +Without WAL mode, concurrent API requests caused `sqlite3.OperationalError: database is locked`. + +### 5. API Key Authentication O(1) Lookup + +**Files:** `gns3server/api/routes/controller/api_keys.py`, `gns3server/api/routes/controller/dependencies/authentication.py` + +**Old format:** `gns3_` — required scanning ALL keys and running bcrypt on each (O(n)). +**New format:** `gns3__` — extract UUID from token, single DB query (O(1)), single bcrypt. + +```python +# New auth flow +parts = token.split("_", 2) +key_id = UUID(parts[1]) +secret = parts[2] +db_key = await api_keys_repo.get_api_key(key_id) # O(1) lookup +if await asyncio.to_thread(bcrypt.checkpw, secret.encode(), db_key.key_hash.encode()): + # Authenticated — 1 query + 1 bcrypt regardless of total key count +``` + +### 6. bcrypt in Thread Pool + +**File:** `gns3server/api/routes/controller/dependencies/authentication.py` + +`bcrypt.checkpw()` is CPU-bound (~1.3s per call) and was blocking the async event loop. With 5 API keys and 10 concurrent requests, this caused ~13s delay before any handler could start. + +```python +# Before: blocking the event loop +if bcrypt.checkpw(token.encode(), db_key.key_hash.encode()): + ... + +# After: offloaded to thread pool +if await asyncio.to_thread(bcrypt.checkpw, secret.encode(), db_key.key_hash.encode()): + ... +``` + +### 7. Concurrency Settings + +| Setting | Before | After | File | +|---------|--------|-------|------| +| Node creation Pool | 5 | 100 | `controller/project.py` | +| Link creation Pool | 5 | 100 | `controller/project.py` | +| MCP BATCH_MAX_WORKERS | 10 | 100 | `api/routes/mcp/nodes.py` | +| MCP HTTP timeout | 10s | 30s | `agent/gns3_copilot/gns3_client/custom_gns3fy.py` | +| HTTP connection pool | 10 (default) | 500/1000 | `agent/gns3_copilot/gns3_client/custom_gns3fy.py` | +| Start nodes Pool | 3 | 3 (unchanged) | `controller/project.py` | + +### 8. MCP Auth Returns JWT + +**File:** `gns3server/api/routes/mcp/__init__.py` + +When an MCP client connects with an API key, the `_resolve_token` function validates the key then returns a fresh short-lived JWT instead of the raw API key. The JWT is stored in a `ContextVar` and reused for all subsequent tool calls within the same SSE session — zero extra bcrypt. + +```python +if user: + fresh_token = auth_service.create_access_token(user.username) + return fresh_token # JWT for subsequent REST API calls +``` + +## Related Files + +| File | Changes | +|------|---------| +| `gns3server/controller/project.py` | Parallel link creation, batch UDP, Pool(100) | +| `gns3server/compute/iou/iou_vm.py` | Image subprocess cache | +| `gns3server/db/tasks.py` | WAL mode + sync_engine event listener | +| `gns3server/api/routes/compute/compute.py` | Batch UDP endpoint | +| `gns3server/controller/udp_link.py` | Pre-allocated port consumption | +| `gns3server/api/routes/controller/api_keys.py` | O(1) key format | +| `gns3server/api/routes/controller/dependencies/authentication.py` | O(1) auth + thread pool bcrypt | +| `gns3server/api/routes/mcp/__init__.py` | Auth returns JWT, tool enhancements | +| `gns3server/api/routes/mcp/nodes.py` | fields filter, inherited template_id, name passthrough | +| `gns3server/api/routes/mcp/links.py` | fields filter, compact array format | +| `gns3server/agent/gns3_copilot/gns3_client/custom_gns3fy.py` | Timeout 30s, connection pool 500/1000 | +| `gns3server/utils/images.py` | md5sum cache error → warning | diff --git a/gns3server/agent/gns3_copilot/gns3_client/custom_gns3fy.py b/gns3server/agent/gns3_copilot/gns3_client/custom_gns3fy.py index 6e30af017..edb9612da 100644 --- a/gns3server/agent/gns3_copilot/gns3_client/custom_gns3fy.py +++ b/gns3server/agent/gns3_copilot/gns3_client/custom_gns3fy.py @@ -75,6 +75,7 @@ F = TypeVar("F", bound=Callable[..., Any]) config = ConfigDict(validate_assignment=True, extra="ignore") + NODE_TYPES = [ "cloud", "nat", @@ -191,6 +192,10 @@ class Gns3Connector: Creates the requests.Session object and applies the necessary parameters """ self.session = requests.Session() # pragma: no cover + # Increase connection pool size to support concurrent MCP batch operations + adapter = requests.adapters.HTTPAdapter(pool_connections=500, pool_maxsize=1000) + self.session.mount("http://", adapter) + self.session.mount("https://", adapter) self.session.headers["Accept"] = "application/json" # pragma: no cover # Set authentication based on API version @@ -291,6 +296,7 @@ class Gns3Connector: """ Executes HTTP operations and handles GNS3-specific error logic. """ + # Handle JWT authentication if ( self.auth_type == "jwt" @@ -308,7 +314,7 @@ class Gns3Connector: "headers": headers, "params": params, "verify": verify, - "timeout": 10.0, # Fixed 10-second timeout for all GNS3 API requests + "timeout": 30.0, # Main request timeout (auth call uses 10s) } if data is not None: kwargs["data"] = data @@ -320,6 +326,7 @@ class Gns3Connector: self.api_calls += 1 + try: _response.raise_for_status() except HTTPError as e: diff --git a/gns3server/api/routes/compute/compute.py b/gns3server/api/routes/compute/compute.py index ba6156fc0..3a1aa61af 100644 --- a/gns3server/api/routes/compute/compute.py +++ b/gns3server/api/routes/compute/compute.py @@ -55,6 +55,27 @@ def allocate_udp_port(project_id: UUID) -> dict: return {"udp_port": udp_port} +@router.post("/projects/{project_id}/ports/udp/batch", status_code=status.HTTP_201_CREATED) +def batch_allocate_udp_ports(project_id: UUID, body: dict) -> dict: + """ + Allocate multiple UDP ports on the compute in a single call. + + Used during project loading to pre-allocate all required UDP ports + before creating links, reducing HTTP round-trips. + """ + + count = body.get("count", 1) + try: + count = max(1, min(int(count), 10000)) + except (ValueError, TypeError): + raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST, detail="count must be a positive integer") + pm = ProjectManager.instance() + project = pm.get_project(str(project_id)) + m = PortManager.instance() + udp_ports = [m.get_free_udp_port(project) for _ in range(count)] + return {"udp_ports": udp_ports} + + @router.get("/network/interfaces") def network_interfaces() -> List[dict]: """ diff --git a/gns3server/api/routes/controller/api_keys.py b/gns3server/api/routes/controller/api_keys.py index 4d6e456d6..235cbe7de 100644 --- a/gns3server/api/routes/controller/api_keys.py +++ b/gns3server/api/routes/controller/api_keys.py @@ -39,12 +39,15 @@ API_KEY_PREFIX = "gns3_" API_KEY_BYTES = 32 -def _generate_api_key() -> tuple[str, str, str]: +def _generate_api_key(api_key_id: UUID = None) -> tuple[str, str, str, UUID]: + if api_key_id is None: + api_key_id = uuid4() random_bytes = secrets.token_hex(API_KEY_BYTES) - raw_key = API_KEY_PREFIX + random_bytes - key_hash = bcrypt.hashpw(raw_key.encode(), bcrypt.gensalt()).decode() - key_prefix = raw_key[: len(API_KEY_PREFIX) + 8] - return raw_key, key_hash, key_prefix + raw_key = f"gns3_{api_key_id}_{random_bytes}" + # Only hash the random secret part, so auth can extract api_key_id and do O(1) lookup + key_hash = bcrypt.hashpw(random_bytes.encode(), bcrypt.gensalt()).decode() + key_prefix = raw_key[:8] + return raw_key, key_hash, key_prefix, api_key_id @router.post("", status_code=status.HTTP_201_CREATED) @@ -55,9 +58,9 @@ async def create_api_key( ) -> dict: """Create a new API key. The full key is returned only once.""" - raw_key, key_hash, key_prefix = _generate_api_key() + raw_key, key_hash, key_prefix, new_key_id = _generate_api_key() db_key = await api_keys_repo.create_api_key( - api_key_id=uuid4(), + api_key_id=new_key_id, user_id=current_user.user_id, name=api_key_data.name, key_hash=key_hash, diff --git a/gns3server/api/routes/controller/dependencies/authentication.py b/gns3server/api/routes/controller/dependencies/authentication.py index d8b947a9b..b762969d2 100644 --- a/gns3server/api/routes/controller/dependencies/authentication.py +++ b/gns3server/api/routes/controller/dependencies/authentication.py @@ -14,13 +14,14 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see . +import asyncio import logging import bcrypt from fastapi import Request, Query, Depends, HTTPException, WebSocket, status from fastapi.security import OAuth2PasswordBearer from typing import Optional -from sqlalchemy import select +from uuid import UUID from gns3server import schemas import gns3server.db.models as models @@ -41,6 +42,7 @@ async def get_user_from_token( token: Optional[str] = Query(None, include_in_schema=False) ) -> schemas.User: + if bearer_token: # bearer token is used first, then any token passed as a URL parameter token = bearer_token @@ -52,27 +54,31 @@ async def get_user_from_token( headers={"WWW-Authenticate": "Bearer"}, ) - # API Key authentication + # API Key authentication — format: gns3__ + # Direct lookup by UUID avoids O(n) scan of all keys. if token.startswith("gns3_"): - query = select(models.ApiKey).where(models.ApiKey.revoked == False) - result = await api_keys_repo._db_session.execute(query) - for db_key in result.scalars().all(): - if bcrypt.checkpw(token.encode(), db_key.key_hash.encode()): - await api_keys_repo.update_last_used(db_key.api_key_id) - user = await user_repo.get_user(db_key.user_id) - if user: - if not user.is_active: - raise HTTPException( - status_code=status.HTTP_401_UNAUTHORIZED, - detail="Not an active user", - headers={"WWW-Authenticate": "Bearer"}, - ) - return user - raise HTTPException( - status_code=status.HTTP_401_UNAUTHORIZED, - detail="Invalid API key", - headers={"WWW-Authenticate": "Bearer"}, - ) + parts = token.split("_", 2) + if len(parts) != 3: + raise HTTPException(status_code=status.HTTP_401_UNAUTHORIZED, detail="Invalid API key format") + try: + key_id = UUID(parts[1]) + except ValueError: + raise HTTPException(status_code=status.HTTP_401_UNAUTHORIZED, detail="Invalid API key format") + secret = parts[2] + db_key = await api_keys_repo.get_api_key(key_id) + if not db_key or db_key.revoked: + raise HTTPException(status_code=status.HTTP_401_UNAUTHORIZED, detail="Invalid API key") + if not await asyncio.to_thread(bcrypt.checkpw, secret.encode(), db_key.key_hash.encode()): + raise HTTPException(status_code=status.HTTP_401_UNAUTHORIZED, detail="Invalid API key") + await api_keys_repo.update_last_used(db_key.api_key_id) + user = await user_repo.get_user(db_key.user_id) + if not user or not user.is_active: + raise HTTPException( + status_code=status.HTTP_401_UNAUTHORIZED, + detail="Not an active user", + headers={"WWW-Authenticate": "Bearer"}, + ) + return user # JWT authentication token_data = auth_service.get_token_data(token) diff --git a/gns3server/api/routes/controller/dependencies/database.py b/gns3server/api/routes/controller/dependencies/database.py index 2e859106b..e5338ad75 100644 --- a/gns3server/api/routes/controller/dependencies/database.py +++ b/gns3server/api/routes/controller/dependencies/database.py @@ -22,6 +22,8 @@ from sqlalchemy.ext.asyncio import AsyncSession from gns3server.db.repositories.base import BaseRepository + + async def get_db_session(request: HTTPConnection) -> AsyncSession: async with AsyncSession(request.app.state._db_engine, expire_on_commit=False) as session: diff --git a/gns3server/api/routes/controller/projects.py b/gns3server/api/routes/controller/projects.py index 354c15cfb..333053e3b 100644 --- a/gns3server/api/routes/controller/projects.py +++ b/gns3server/api/routes/controller/projects.py @@ -611,9 +611,11 @@ async def create_node_from_template( """ template = await TemplatesService(templates_repo).get_template(template_id) + controller = Controller.instance() project = controller.get_project(str(project_id)) + node = await project.add_node_from_template( - template, x=template_usage.x, y=template_usage.y, compute_id=template_usage.compute_id + template, x=template_usage.x, y=template_usage.y, name=template_usage.name, compute_id=template_usage.compute_id ) return node.asdict() diff --git a/gns3server/api/routes/mcp/__init__.py b/gns3server/api/routes/mcp/__init__.py index 1deb2473b..d45bf2751 100644 --- a/gns3server/api/routes/mcp/__init__.py +++ b/gns3server/api/routes/mcp/__init__.py @@ -32,6 +32,7 @@ import asyncio import logging import socket import uuid +from uuid import UUID import bcrypt from typing import Any, Annotated from urllib.parse import parse_qs @@ -80,7 +81,7 @@ from .images import ( install_images_handler, ) from .device_config import ( - device_config_send_handler, device_command_run_handler, + device_config_send_handler, device_show_run_handler, vpcs_config_set_handler, ) from .nodes import ( @@ -213,23 +214,27 @@ async def _resolve_token(token: str) -> str | None: except Exception: pass - # Try API key + # Try API key — format: gns3__ → O(1) lookup if token.startswith("gns3_") and _app is not None: db_engine = getattr(_app.state, "_db_engine", None) if db_engine is not None: try: - async with AsyncSession(db_engine, expire_on_commit=False) as db_session: - repo = ApiKeysRepository(db_session) - query = select(models.ApiKey).where(models.ApiKey.revoked == False) - result = await db_session.execute(query) - for db_key in result.scalars().all(): - if bcrypt.checkpw(token.encode(), db_key.key_hash.encode()): - await repo.update_last_used(db_key.api_key_id) - user_repo = UsersRepository(db_session) - user = await user_repo.get_user(db_key.user_id) - if user: - _jwt_username_var.set(user.username) - return token + parts = token.split("_", 2) + if len(parts) == 3: + key_id = UUID(parts[1]) + secret = parts[2] + async with AsyncSession(db_engine, expire_on_commit=False) as db_session: + repo = ApiKeysRepository(db_session) + db_key = await repo.get_api_key(key_id) + if db_key and not db_key.revoked: + if await asyncio.to_thread(bcrypt.checkpw, secret.encode(), db_key.key_hash.encode()): + await repo.update_last_used(db_key.api_key_id) + user_repo = UsersRepository(db_session) + user = await user_repo.get_user(db_key.user_id) + if user: + _jwt_username_var.set(user.username) + fresh_token = auth_service.create_access_token(user.username) + return fresh_token except Exception: pass @@ -488,24 +493,32 @@ async def node_suspend( @mcp.tool() async def node_create( project_id: Annotated[str, Field(description="UUID of the project")], - template_id: Annotated[str | None, Field(description="Template UUID (required for single mode)")] = None, - x: Annotated[int, Field(description="X coordinate")] = 0, - y: Annotated[int, Field(description="Y coordinate")] = 0, + template_id: Annotated[str | None, Field(description="Template UUID (required for single mode; used as default in batch mode)")] = None, + x: Annotated[int, Field(description="X coordinate (canvas center origin, right positive)")] = 0, + y: Annotated[int, Field(description="Y coordinate (canvas center origin, down positive)")] = 0, compute_id: Annotated[str, Field(description="Compute ID (default: local)")] = "local", - nodes: Annotated[list | None, Field(description="Batch mode: [{template_id, x?, y?, name?, compute_id?}] — creates multiple nodes in parallel")] = None, + nodes: Annotated[list | None, Field(description="Batch mode: [{name, template_id?, x?, y?, compute_id?}] — top-level template_id applies as default")] = None, + fields: Annotated[list[str] | None, Field(description="Response fields to include (default: [node_id, name, node_type, status, console]). " + "Available: compute_id, name, node_type, node_id, console, console_type, " + "console_auto_start, aux, aux_type, properties, label, symbol, x, y, z, " + "locked, port_name_format, port_segment_size, first_port_name, " + "custom_adapters, tags, template_id, project_id, node_directory, " + "status, command_line, width, height, ports, console_host")] = None, ) -> list[dict[str, Any]]: """Create one or more nodes from templates. Single mode: provide template_id, x, y (optional compute_id) - Batch mode: provide nodes=[{template_id, x, y, name?, compute_id?}] — creates up to 10 in parallel + Batch mode: provide nodes=[{name, template_id?, x?, y?, compute_id?}] — creates up to 100 in parallel. + Top-level template_id applies to all nodes; individual nodes can override. """ if nodes is not None: return await asyncio.to_thread(_run_handler_sync, create_node_handler, { - "project_id": project_id, "nodes": nodes, + "project_id": project_id, "nodes": nodes, "fields": fields, + "template_id": template_id, }) return await asyncio.to_thread(_run_handler_sync, create_node_handler, { "project_id": project_id, "template_id": template_id, - "x": x, "y": y, "compute_id": compute_id, + "x": x, "y": y, "compute_id": compute_id, "fields": fields, }) @@ -589,21 +602,25 @@ async def link_get( @mcp.tool() async def link_create( project_id: Annotated[str, Field(description="UUID of the project")], - nodes: Annotated[list | None, Field(description="Single mode: [{node_id, adapter_number, port_number}]")] = None, + nodes: Annotated[list | None, Field(description="Single mode: [{node_id, adapter_number, port_number}] or compact [id, ad, pt, id, ad, pt]")] = None, link_type: Annotated[str, Field(description="Link type - ethernet or serial")] = "ethernet", filters: Annotated[dict | None, Field(description="Optional packet filters")] = None, - links: Annotated[list | None, Field(description="Batch mode: [{nodes, link_type?, filters?}] — creates multiple links in parallel")] = None, + links: Annotated[list | None, Field(description="Batch mode: [{nodes, link_type?, filters?}] — nodes supports compact [id, ad, pt, id, ad, pt] format")] = None, + fields: Annotated[list[str] | None, Field(description="Response fields to include (default: [link_id, link_type, nodes]). " + "Available: link_id, project_id, link_type, nodes, suspend, " + "link_style, filters, show_filters_icon, capturing, " + "capture_file_name, capture_file_path, capture_compute_id, wireshark")] = None, ) -> list[dict[str, Any]]: """Create one or more links between nodes. Single mode: provide nodes, link_type (optional filters) - Batch mode: provide links=[{nodes, link_type?, filters?}] — up to 10 in parallel + Batch mode: provide links=[{nodes, link_type?, filters?}] — up to 100 in parallel """ if links: return await asyncio.to_thread(_run_handler_sync, create_link_handler, { - "project_id": project_id, "links": links, + "project_id": project_id, "links": links, "fields": fields, }) - params = {"project_id": project_id, "nodes": nodes, "link_type": link_type} + params = {"project_id": project_id, "nodes": nodes, "link_type": link_type, "fields": fields} if filters: params["filters"] = filters return await asyncio.to_thread(_run_handler_sync, create_link_handler, params) @@ -654,9 +671,13 @@ async def link_update( # ── Template tools ──────────────────────────────────────────────────── @mcp.tool() -async def template_list() -> list[dict[str, Any]]: +async def template_list( + fields: Annotated[list[str] | None, Field(description="Response fields to include (default: [template_id, name, template_type, category, default_name_format]). " + "Available: template_id, name, version, category, default_name_format, symbol, " + "template_type, compute_id, usage, tags, builtin, created_at, updated_at")] = None, +) -> list[dict[str, Any]]: """List all available templates on the server.""" - return await asyncio.to_thread(_run_handler_sync, list_templates_handler, {}) + return await asyncio.to_thread(_run_handler_sync, list_templates_handler, {"fields": fields}) @mcp.tool() @@ -1335,7 +1356,7 @@ async def image_install() -> list[dict[str, Any]]: # 1. node_list(project_id) → identify device names # 2. node_start_all(project_id) → ensure devices are running # 3. device_config_send(project_id, device_configs=[...]) → push config -# 4. device_command_run(project_id, device_commands=[...]) → verify +# 4. device_show_run(project_id, device_commands=[...]) → verify @mcp.tool() @@ -1364,26 +1385,27 @@ async def device_config_send( @mcp.tool() -async def device_command_run( +async def device_show_run( project_id: Annotated[str, Field(description="UUID of the project")], device_configs: Annotated[list, Field( - description="List of device commands. Each entry: {\"device_name\": \"R1\", \"show_commands\": [\"show ip int brief\", \"show running-config\"]}" + description="List of device commands. Each entry: {\"device_name\": \"R1\", \"commands\": [\"show ip int brief\", \"show running-config\"]}" )], template: Annotated[str | None, Field(description="Optional Jinja2 template. Use with vars per device. Example: \"show ip route {{ protocol }}\"")] = None, ) -> list[dict[str, Any]]: """Run read-only diagnostic (show) commands on network devices via console. Two modes: - 1. Direct commands: each device has show_commands=[...] + 1. Direct commands: each device has commands=[...] (read-only show/display/ping/traceroute only) 2. Jinja2 template: provide template + vars per device Use this to inspect device status, view configurations, or verify changes. + For configuration changes use device_config_send instead. Devices must be started first. """ params = {"project_id": project_id, "device_configs": device_configs} if template is not None: params["template"] = template - return await asyncio.to_thread(_run_handler_sync, device_command_run_handler, params) + return await asyncio.to_thread(_run_handler_sync, device_show_run_handler, params) @mcp.tool() diff --git a/gns3server/api/routes/mcp/device_config.py b/gns3server/api/routes/mcp/device_config.py index f7a2009ae..10d4b7634 100644 --- a/gns3server/api/routes/mcp/device_config.py +++ b/gns3server/api/routes/mcp/device_config.py @@ -102,13 +102,13 @@ def device_config_send_handler(params: dict[str, Any], gns3_ctx: dict[str, Any]) ) -def device_command_run_handler(params: dict[str, Any], gns3_ctx: dict[str, Any]) -> list[dict[str, Any]]: +def device_show_run_handler(params: dict[str, Any], gns3_ctx: dict[str, Any]) -> list[dict[str, Any]]: """Run read-only diagnostic (show) commands on network devices.""" project_id = params.get("project_id") device_configs = params.get("device_configs") template = params.get("template") if not project_id or not device_configs: - return [{"error": "project_id and device_configs (list of {device_name, show_commands}) are required"}] + return [{"error": "project_id and device_configs (list of {device_name, commands}) are required"}] if template: device_configs = _render_template(template, device_configs, commands_field="commands") diff --git a/gns3server/api/routes/mcp/links.py b/gns3server/api/routes/mcp/links.py index cae2b07db..543cb39e1 100644 --- a/gns3server/api/routes/mcp/links.py +++ b/gns3server/api/routes/mcp/links.py @@ -31,7 +31,7 @@ from gns3server.services import auth_service log = logging.getLogger(__name__) -BATCH_MAX_WORKERS = 10 +BATCH_MAX_WORKERS = 100 # ── Helper ───────────────────────────────────────────────────────────────── @@ -46,6 +46,48 @@ def _get_connector(gns3_ctx: dict[str, Any]): ) +def _normalize_link_nodes(nodes) -> list[dict[str, Any]]: + """ + Normalize link node entries, accepting both standard object format and + compact array format to reduce token usage. + + Standard: [{"node_id": "uuid", "adapter_number": 0, "port_number": 0}] + Compact: ["uuid", 0, 0, "uuid", 0, 0] + + Returns the normalized list, or raises ValueError with a clear message + on format errors so the AI can self-correct. + """ + if not nodes: + return nodes + if not isinstance(nodes, list): + raise ValueError(f"nodes must be a list, got {type(nodes).__name__}: {nodes}") + # Standard object format: [{"node_id": "...", ...}] + if isinstance(nodes[0], dict): + return nodes + # Compact array format: ["uuid", ad, pt, "uuid", ad, pt] + if all(not isinstance(n, dict) for n in nodes): + if len(nodes) != 6: + raise ValueError( + f"Compact link format requires exactly 6 elements " + f"[node_id, adapter, port, node_id, adapter, port], " + f"but got {len(nodes)} elements: {nodes}" + ) + if not isinstance(nodes[0], str) or not isinstance(nodes[3], str): + raise ValueError( + f"Compact link format expects node_id (string) at positions 0 and 3, " + f"got types {type(nodes[0]).__name__} and {type(nodes[3]).__name__}: {nodes}" + ) + return [ + {"node_id": nodes[0], "adapter_number": nodes[1], "port_number": nodes[2]}, + {"node_id": nodes[3], "adapter_number": nodes[4], "port_number": nodes[5]}, + ] + raise ValueError( + f"Unrecognized link nodes format. " + f"Use standard [{{\"node_id\":\"..\",\"adapter_number\":0,\"port_number\":0}},...] " + f"or compact [\"id\",0,0,\"id\",0,0], got: {nodes}" + ) + + # ── Tool handlers ────────────────────────────────────────────────────────── VALID_LINK_FIELDS = { @@ -56,6 +98,16 @@ VALID_LINK_FIELDS = { } +LINK_DEFAULT_FIELDS = ["link_id", "link_type", "nodes"] + + +def _filter_link_response(link: dict, fields: list[str] = None) -> dict: + """Filter link response to only include requested fields.""" + if not fields: + fields = LINK_DEFAULT_FIELDS + return {k: link[k] for k in fields if k in link} + + def get_links_handler(params: dict[str, Any], gns3_ctx: dict[str, Any]) -> dict[str, Any]: project_id = params.get("project_id") if not project_id: @@ -90,6 +142,10 @@ def create_link_handler(params: dict[str, Any], gns3_ctx: dict[str, Any]) -> dic if not project_id: return {"error": "project_id is required"} + fields = params.get("fields") + if fields is not None and not isinstance(fields, list): + return {"error": "fields must be a list, e.g. [\"link_id\", \"nodes\"]"} + links = params.get("links") # Batch mode: links=[{nodes, link_type?, filters?, suspend?}] if links is not None: @@ -98,10 +154,11 @@ def create_link_handler(params: dict[str, Any], gns3_ctx: dict[str, Any]) -> dic results = [] conn = _get_connector(gns3_ctx) def _create_one(link_data): - if not link_data.get("nodes"): + raw_nodes = link_data.get("nodes") + if not raw_nodes: return {"status": "error", "error": "nodes is required for each link"} try: - body = {"nodes": link_data["nodes"]} + body = {"nodes": _normalize_link_nodes(raw_nodes)} if link_data.get("link_type"): body["link_type"] = link_data["link_type"] if link_data.get("filters"): @@ -110,7 +167,7 @@ def create_link_handler(params: dict[str, Any], gns3_ctx: dict[str, Any]) -> dic body["suspend"] = link_data["suspend"] url = f"{conn.base_url}/projects/{project_id}/links" resp = conn.http_call("post", url, json_data=body).json() - return {"status": "success", "link": resp} + return {"status": "success", "link": _filter_link_response(resp, fields)} except Exception as e: return {"status": "error", "error": str(e)} with ThreadPoolExecutor(max_workers=min(len(links), BATCH_MAX_WORKERS)) as pool: @@ -124,7 +181,7 @@ def create_link_handler(params: dict[str, Any], gns3_ctx: dict[str, Any]) -> dic if not nodes: return {"error": "nodes is required"} conn = _get_connector(gns3_ctx) - data = {"nodes": nodes} + data = {"nodes": _normalize_link_nodes(nodes)} if "link_type" in params: data["link_type"] = params["link_type"] if "filters" in params: @@ -132,7 +189,8 @@ def create_link_handler(params: dict[str, Any], gns3_ctx: dict[str, Any]) -> dic if "suspend" in params: data["suspend"] = params["suspend"] url = f"{conn.base_url}/projects/{project_id}/links" - return conn.http_call("post", url, json_data=data).json() + resp = conn.http_call("post", url, json_data=data).json() + return _filter_link_response(resp, fields) def delete_link_handler(params: dict[str, Any], gns3_ctx: dict[str, Any]) -> dict[str, Any]: diff --git a/gns3server/api/routes/mcp/nodes.py b/gns3server/api/routes/mcp/nodes.py index f3489da76..135872d3b 100644 --- a/gns3server/api/routes/mcp/nodes.py +++ b/gns3server/api/routes/mcp/nodes.py @@ -31,7 +31,7 @@ from gns3server.services import auth_service log = logging.getLogger(__name__) -BATCH_MAX_WORKERS = 10 +BATCH_MAX_WORKERS = 100 # ── Constants ────────────────────────────────────────────────────────────── @@ -195,20 +195,34 @@ def suspend_node_handler(params: dict[str, Any], gns3_ctx: dict[str, Any]) -> di return {"message": f"Node {node_id} suspended", "node_id": node_id} +def _filter_node_response(node: dict, fields: list[str] = None) -> dict: + """Filter node response to only include requested fields.""" + if not fields: + fields = ["node_id", "name", "node_type", "status", "console"] + return {k: node[k] for k in fields if k in node} + + def create_node_handler(params: dict[str, Any], gns3_ctx: dict[str, Any]) -> dict[str, Any]: + project_id = params.get("project_id") if not project_id: return {"error": "project_id is required"} + fields = params.get("fields") + if fields is not None and not isinstance(fields, list): + return {"error": "fields must be a list, e.g. [\"node_id\", \"name\"]"} + nodes = params.get("nodes") - # Batch mode: nodes=[{template_id, x, y, name?, compute_id?}] + # Batch mode: nodes=[{template_id?, x, y, name?, compute_id?}] + # When top-level template_id is set, it applies to all nodes as a default if nodes is not None: if not isinstance(nodes, list) or not nodes: return {"error": "nodes must be a non-empty array"} + default_tid = params.get("template_id") results = [] conn = _get_connector(gns3_ctx) def _create_one(node_data): - tid = node_data.get("template_id") + tid = node_data.get("template_id", default_tid) if not tid: return {"template_id": tid, "status": "error", "error": "template_id is required"} try: @@ -218,8 +232,11 @@ def create_node_handler(params: dict[str, Any], gns3_ctx: dict[str, Any]) -> dic "y": node_data.get("y", 0), "compute_id": node_data.get("compute_id", "local"), } + node_name = node_data.get("name") + if node_name: + body["name"] = node_name resp = conn.http_call("post", url, json_data=body).json() - return {"template_id": tid, "status": "success", "node": resp} + return {"template_id": tid, "status": "success", "node": _filter_node_response(resp, fields)} except Exception as e: return {"template_id": tid, "status": "error", "error": str(e)} with ThreadPoolExecutor(max_workers=min(len(nodes), BATCH_MAX_WORKERS)) as pool: @@ -238,8 +255,12 @@ def create_node_handler(params: dict[str, Any], gns3_ctx: dict[str, Any]) -> dic "y": params.get("y", 0), "compute_id": params.get("compute_id", "local"), } + node_name = params.get("name") + if node_name: + data["name"] = node_name url = f"{conn.base_url}/projects/{project_id}/templates/{template_id}" - return conn.http_call("post", url, json_data=data).json() + resp = conn.http_call("post", url, json_data=data).json() + return _filter_node_response(resp, fields) def delete_node_handler(params: dict[str, Any], gns3_ctx: dict[str, Any]) -> dict[str, Any]: diff --git a/gns3server/api/routes/mcp/templates.py b/gns3server/api/routes/mcp/templates.py index 01006ee94..af8eeaf40 100644 --- a/gns3server/api/routes/mcp/templates.py +++ b/gns3server/api/routes/mcp/templates.py @@ -43,9 +43,38 @@ def _get_connector(gns3_ctx: dict[str, Any]): # ── Tool handlers ────────────────────────────────────────────────────────── +VALID_TEMPLATE_FIELDS = { + "template_id", "name", "version", "category", "default_name_format", + "symbol", "template_type", "compute_id", "usage", "tags", "builtin", + "created_at", "updated_at", +} + +TEMPLATE_DEFAULT_FIELDS = ["template_id", "name", "template_type", "category", "default_name_format"] + + +def _filter_templates(templates, fields): + """Filter each template to only include requested fields.""" + if not fields: + fields = TEMPLATE_DEFAULT_FIELDS + if isinstance(templates, dict): + return {k: templates[k] for k in fields if k in templates} + return [{k: t[k] for k in fields if k in t} for t in templates] + + def list_templates_handler(params: dict[str, Any], gns3_ctx: dict[str, Any]) -> dict[str, Any]: conn = _get_connector(gns3_ctx) templates = conn.http_call("get", f"{conn.base_url}/templates").json() + fields = params.get("fields") + if fields: + if not isinstance(fields, list): + return {"error": "fields must be a list, e.g. [\"template_id\", \"name\"]"} + invalid = [f for f in fields if f not in VALID_TEMPLATE_FIELDS] + if invalid: + return { + "error": f"Unknown fields: {invalid}", + "available_fields": sorted(VALID_TEMPLATE_FIELDS), + } + templates = _filter_templates(templates, fields) return {"templates": templates, "count": len(templates)} diff --git a/gns3server/api/server.py b/gns3server/api/server.py index 5def647cc..e6089e9fb 100644 --- a/gns3server/api/server.py +++ b/gns3server/api/server.py @@ -213,15 +213,3 @@ async def validation_exception_handler(request: Request, exc: RequestValidationE status_code=status.HTTP_422_UNPROCESSABLE_CONTENT, content={"message": str(exc)} ) - -# FIXME: do not use this middleware since it creates issue when using StreamingResponse -# see https://starlette-context.readthedocs.io/en/latest/middleware.html#why-are-there-two-middlewares-that-do-the-same-thing - -# @app.middleware("http") -# async def add_extra_headers(request: Request, call_next): -# start_time = time.time() -# response = await call_next(request) -# process_time = time.time() - start_time -# response.headers["X-Process-Time"] = str(process_time) -# response.headers["X-GNS3-Server-Version"] = f"{__version__}" -# return response diff --git a/gns3server/compute/iou/iou_vm.py b/gns3server/compute/iou/iou_vm.py index ace86a8db..506bff3d7 100644 --- a/gns3server/compute/iou/iou_vm.py +++ b/gns3server/compute/iou/iou_vm.py @@ -57,6 +57,12 @@ log = logging.getLogger(__name__) class IOUVM(BaseNode): module_name = "iou" + # Class-level caches shared across all IOU VM instances using the same image. + # These avoid redundant subprocess calls during project loading when multiple + # IOU nodes use the same image. + _loader_cache = {} # image path -> loader command list + _default_values_cache = {} # image path -> (ram, nvram) + """ IOU VM implementation. @@ -180,8 +186,15 @@ class IOUVM(BaseNode): async def update_default_iou_values(self): """ Finds the default RAM and NVRAM values for the IOU image. + Results are cached per image path to avoid redundant subprocess calls + when multiple IOU nodes use the same image. """ + # Check class-level cache for default values + if self._path in IOUVM._default_values_cache: + self._ram, self._nvram = IOUVM._default_values_cache[self._path] + return + await self._check_requirements() try: output = await gns3server.utils.asyncio.subprocess_check_output( @@ -193,6 +206,9 @@ class IOUVM(BaseNode): match = re.search(r"-m \s+Megabytes of router memory \(default ([0-9]+)MB\)", output) if match: self.ram = int(match.group(1)) + # Only cache on success, so a subsequent call with explicitly set + # ram/nvram values won't be overwritten by stale cached defaults + IOUVM._default_values_cache[self._path] = (self._ram, self._nvram) except (ValueError, OSError, subprocess.SubprocessError) as e: log.warning(f"could not find default RAM and NVRAM values for {os.path.basename(self._path)}: {e}") @@ -207,6 +223,13 @@ class IOUVM(BaseNode): if self._loader is not None: return # image already checked + + # Check class-level cache: if another IOU VM already verified this image, + # reuse its loader configuration to avoid redundant subprocess calls. + if self._path in IOUVM._loader_cache: + self._loader = IOUVM._loader_cache[self._path] + return + if not self._path: raise IOUError("IOU image is not configured") if not os.path.isfile(self._path) or not os.path.exists(self._path): @@ -252,6 +275,9 @@ class IOUVM(BaseNode): except (OSError, subprocess.SubprocessError) as e: log.warning(f"Could not use loader {loader}: {e}") + # Cache the loader result for other IOU VMs using the same image + IOUVM._loader_cache[self._path] = self._loader + def asdict(self): iou_vm_info = { diff --git a/gns3server/controller/project.py b/gns3server/controller/project.py index 463ff5345..989619940 100644 --- a/gns3server/controller/project.py +++ b/gns3server/controller/project.py @@ -158,6 +158,7 @@ class Project: self.dump() self._iou_id_lock = asyncio.Lock() + self._preallocated_udp_ports = {} # compute_id -> list of pre-allocated UDP ports log.debug(f'Project "{self.name}" [{self._id}] loaded') self.emit_controller_notification("project.created", self.asdict()) @@ -216,6 +217,7 @@ class Project: self._load_snapshot_config() # Create the project on demand on the compute node self._project_created_on_compute = set() + self._preallocated_udp_ports = {} @property def scene_height(self): @@ -561,13 +563,11 @@ class Project: """ Create a node from a template. """ - template["x"] = x template["y"] = y node_type = template.pop("template_type") if compute_id: - # use a custom compute_id compute = self.controller.get_compute(compute_id) else: compute = self.controller.get_compute(template.pop("compute_id")) @@ -584,15 +584,12 @@ class Project: node = Node(self, compute, name, node_id=node_id, node_type=node_type, **kwargs) if compute not in self._project_created_on_compute: - # For a local server we send the project path if compute.id == "local": data = {"name": self._name, "project_id": self._id, "path": self._path} else: data = {"name": self._name, "project_id": self._id} - if self._variables: data["variables"] = self._variables - await compute.post("/projects", data=data) self._project_created_on_compute.add(compute) @@ -618,16 +615,15 @@ class Project: if node_type == "iou": async with self._iou_id_lock: - # wait for an IOU node to be completely created before adding a new one - # this is important otherwise we allocate the same application ID (used - # to generate MAC addresses) when creating multiple IOU node at the same time + # IOU application IDs must be allocated serially to avoid duplicates. + # The lock must also cover _create_node() because get_next_application_id() + # checks in-memory nodes (self._nodes), which are only registered + # after _create_node() completes. if "properties" in kwargs.keys(): - # allocate a new application id for nodes loaded from the project kwargs.get("properties")["application_id"] = get_next_application_id( self._controller.projects, self._computes ) elif "application_id" not in kwargs.keys() and not kwargs.get("properties"): - # allocate a new application id for nodes added to the project kwargs["application_id"] = get_next_application_id(self._controller.projects, self._computes) node = await self._create_node(compute, name, node_id, node_type, **kwargs) else: @@ -751,6 +747,56 @@ class Project: self.dump() self.emit_notification("drawing.deleted", drawing.asdict()) + async def _create_link_from_topology_data(self, link_data): + """ + Create a link from topology data (used during project loading). + + Extracted into a separate method so links can be created in parallel + via Pool() during project.open(). + + :param link_data: Link data from the topology file + """ + link = await self.add_link(link_id=link_data["link_id"]) + if "filters" in link_data: + try: + await link.update_filters(link_data["filters"]) + except ControllerError as e: + log.warning( + "Dropping invalid filters on link %s: %s", + link_data.get("link_id"), e + ) + if "link_style" in link_data: + await link.update_link_style(link_data["link_style"]) + if "show_filters_icon" in link_data: + await link.update_show_filters_icon(link_data["show_filters_icon"]) + for node_link in link_data.get("nodes", []): + node = self.get_node(node_link["node_id"]) + port = node.get_port(node_link["adapter_number"], node_link["port_number"]) + if port is None: + log.warning( + "Port {}/{} for {} not found".format( + node_link["adapter_number"], node_link["port_number"], node.name + ) + ) + continue + if port.link is not None: + log.warning( + "Port {}/{} is already connected to link ID {}".format( + node_link["adapter_number"], node_link["port_number"], port.link.id + ) + ) + continue + await link.add_node( + node, + node_link["adapter_number"], + node_link["port_number"], + label=node_link.get("label"), + dump=False, + ) + if len(link.nodes) != 2: + # a link should have 2 attached nodes, this can happen with corrupted projects + await self.delete_link(link.id, force_delete=True) + @open_required async def add_link(self, link_id=None, dump=True): """ @@ -766,6 +812,35 @@ class Project: self.dump() return link + async def preallocate_udp_ports_for_compute(self, compute, count): + """ + Pre-allocate UDP ports from a compute in a single batch call. + + Used during project loading to reduce HTTP round-trips when + creating many links. + + :param compute: Compute instance + :param count: Number of UDP ports to pre-allocate + """ + if count <= 0: + return + response = await compute.post(f"/projects/{self._id}/ports/udp/batch", data={"count": count}) + ports = response.json["udp_ports"] + self._preallocated_udp_ports.setdefault(compute.id, []) + self._preallocated_udp_ports[compute.id].extend(ports) + + def pop_preallocated_udp_port(self, compute_id): + """ + Pop a pre-allocated UDP port for a compute. + + :param compute_id: Compute ID + :returns: UDP port number or None if no pre-allocated port is available + """ + ports = self._preallocated_udp_ports.get(compute_id, []) + if ports: + return ports.pop() + return None + @open_required async def delete_link(self, link_id, force_delete=False): link = self.get_link(link_id) @@ -1221,54 +1296,35 @@ class Project: # Create nodes in parallel with limited concurrency # to avoid overwhelming the system with too many simultaneous operations - pool = Pool(concurrency=5) + pool = Pool(concurrency=100) for compute, name, node_id, node_data in nodes_to_create: pool.append(self.add_node, compute, name, node_id, dump=False, **node_data) await pool.join() + # Pre-allocate UDP ports for all links in batch to reduce HTTP round-trips + ports_per_compute = {} for link_data in topology.get("links", []): if "link_id" not in link_data.keys(): - # skip the link continue - link = await self.add_link(link_id=link_data["link_id"]) - if "filters" in link_data: - try: - await link.update_filters(link_data["filters"]) - except ControllerError as e: - log.warning( - "Dropping invalid filters on link %s: %s", - link_data.get("link_id"), e - ) - if "link_style" in link_data: - await link.update_link_style(link_data["link_style"]) - if "show_filters_icon" in link_data: - await link.update_show_filters_icon(link_data["show_filters_icon"]) for node_link in link_data.get("nodes", []): - node = self.get_node(node_link["node_id"]) - port = node.get_port(node_link["adapter_number"], node_link["port_number"]) - if port is None: - log.warning( - "Port {}/{} for {} not found".format( - node_link["adapter_number"], node_link["port_number"], node.name - ) - ) - continue - if port.link is not None: - log.warning( - "Port {}/{} is already connected to link ID {}".format( - node_link["adapter_number"], node_link["port_number"], port.link.id - ) - ) - continue - await link.add_node( - node, - node_link["adapter_number"], - node_link["port_number"], - label=node_link.get("label"), - dump=False, - ) - if len(link.nodes) != 2: - # a link should have 2 attached nodes, this can happen with corrupted projects - await self.delete_link(link.id, force_delete=True) + node = self._nodes.get(node_link["node_id"]) + if node: + ports_per_compute[node.compute.id] = ports_per_compute.get(node.compute.id, 0) + 1 + for compute in self.computes: + count = ports_per_compute.get(compute.id, 0) + if count > 0: + await self.preallocate_udp_ports_for_compute(compute, count) + # Create links in parallel for improved performance + pool = Pool(concurrency=100) + for link_data in topology.get("links", []): + if "link_id" not in link_data.keys(): + continue + pool.append(self._create_link_from_topology_data, link_data) + await pool.join() + # Release any pre-allocated UDP ports that were not consumed by links + for compute_id, ports in self._preallocated_udp_ports.items(): + if ports: + log.warning(f"Releasing {len(ports)} unconsumed pre-allocated UDP ports on compute {compute_id}") + self._preallocated_udp_ports.clear() for drawing_data in topology.get("drawings", []): await self.add_drawing(dump=False, **drawing_data) diff --git a/gns3server/controller/udp_link.py b/gns3server/controller/udp_link.py index 330e6a4f7..7c2e6f187 100644 --- a/gns3server/controller/udp_link.py +++ b/gns3server/controller/udp_link.py @@ -65,10 +65,19 @@ class UDPLink(Link): raise ControllerError(f"Cannot get an IP address on same subnet: {e}") # Reserve a UDP port on both side - response = await node1.compute.post(f"/projects/{self._project.id}/ports/udp") - self._node1_port = response.json["udp_port"] - response = await node2.compute.post(f"/projects/{self._project.id}/ports/udp") - self._node2_port = response.json["udp_port"] + # Try pre-allocated ports first (used during batch project loading) + port = self._project.pop_preallocated_udp_port(node1.compute.id) + if port is not None: + self._node1_port = port + else: + response = await node1.compute.post(f"/projects/{self._project.id}/ports/udp") + self._node1_port = response.json["udp_port"] + port = self._project.pop_preallocated_udp_port(node2.compute.id) + if port is not None: + self._node2_port = port + else: + response = await node2.compute.post(f"/projects/{self._project.id}/ports/udp") + self._node2_port = response.json["udp_port"] node1_filters, node2_filters = self._get_node_filters(node1, node2) diff --git a/gns3server/db/tasks.py b/gns3server/db/tasks.py index 887ff9bf1..285d95b54 100644 --- a/gns3server/db/tasks.py +++ b/gns3server/db/tasks.py @@ -80,6 +80,27 @@ async def connect_to_db(app: FastAPI) -> None: db_path = os.path.join(Config.instance().config_dir, "gns3_controller.db") db_url = os.environ.get("GNS3_DATABASE_URI", f"sqlite+aiosqlite:///{db_path}") engine = create_async_engine(db_url, connect_args={"check_same_thread": False, "timeout": 20}, future=True, pool_size=512, max_overflow=1024) + + # Register PRAGMA on the sync engine to ensure it fires for async connections + @event.listens_for(engine.sync_engine, "connect") + def set_sqlite_pragma(dbapi_connection, connection_record): + cursor = dbapi_connection.cursor() + cursor.execute("PRAGMA journal_mode=WAL") + cursor.execute("PRAGMA foreign_keys=ON") + cursor.close() + + # Verify WAL mode is active + async with engine.connect() as _verify_conn: + def _check_wal(conn): + cursor = conn.connection.cursor() + cursor.execute("PRAGMA journal_mode") + row = cursor.fetchone() + cursor.close() + return row[0] if row else "unknown" + wal_mode = await _verify_conn.run_sync(_check_wal) + log.info(f"SQLite journal mode: {wal_mode}") + if wal_mode and wal_mode.upper() != "WAL": + log.warning("WAL mode not active - concurrent writes may cause 'database is locked' errors") alembic_cfg = config.Config() alembic_cfg.set_main_option("script_location", "gns3server:db_migrations") #alembic_cfg.set_main_option('sqlalchemy.url', db_url) @@ -146,16 +167,6 @@ async def disconnect_from_db(app: FastAPI) -> None: log.info(f"Disconnected from database") -@event.listens_for(Engine, "connect") -def set_sqlite_pragma(dbapi_connection, connection_record): - - # Enable SQL foreign key support for SQLite - # https://docs.sqlalchemy.org/en/14/dialects/sqlite.html#foreign-key-support - cursor = dbapi_connection.cursor() - cursor.execute("PRAGMA foreign_keys=ON") - cursor.close() - - async def get_computes(app: FastAPI) -> List[dict]: computes = [] diff --git a/gns3server/services/templates.py b/gns3server/services/templates.py index 6502b0811..b2fd6c582 100644 --- a/gns3server/services/templates.py +++ b/gns3server/services/templates.py @@ -33,6 +33,7 @@ from gns3server.controller.controller_error import ( ControllerForbiddenError, ) + TEMPLATE_TYPE_TO_SCHEMA = { "cloud": schemas.CloudTemplate, "ethernet_hub": schemas.EthernetHubTemplate, @@ -259,6 +260,7 @@ class TemplatesService: async def get_template(self, template_id: UUID) -> dict: db_template = await self._templates_repo.get_template(template_id) + if db_template: template = db_template.asjson() else: diff --git a/gns3server/utils/images.py b/gns3server/utils/images.py index dfb3152b3..a1074191d 100644 --- a/gns3server/utils/images.py +++ b/gns3server/utils/images.py @@ -290,7 +290,7 @@ def md5sum(path, working_dir=None, stopped_event=None, cache_to_md5file=True): with open(md5sum_file, "w+") as f: f.write(digest) except OSError as e: - log.error("Can't write digest of %s: %s", path, str(e)) + log.warning("Can't write digest of %s: %s", path, str(e)) return digest