diff --git a/docs/README.md b/docs/README.md index bc2d414aa..c347479cc 100644 --- a/docs/README.md +++ b/docs/README.md @@ -75,6 +75,12 @@ Web-based packet capture analysis using Docker + xpra HTML5 client. Zero-install ### Marker (Traffic Insight) (`features/marker-traffic-insight.md`) Real-time traffic insight via per-link BPF markers and project-level inherited definitions. A marker taps a link in uBridge, emitting match notifications and pcap capture on BPF hit; definitions fan out to every capable link automatically. +### Docker exec Console (Vendor NOS) (`features/docker-exec-console.md`) +Console for vendor NOS containers (SR Linux, XRd, …) whose CLI is a TUI off PID 1: runs the vendor CLI via the Docker exec API, plus `GNS3_SKIP_INIT`/`GNS3_INTERFACE_NAMES` boot knobs and SKIP_INIT volume persistence. + +### Cisco XRd Control Plane (`features/vendor-nos-xrd.md`) +Cisco XRd as a GNS3 Docker router: vendor path + shm/device injection (`GNS3_SHM_SIZE`/`GNS3_DEVICES`), config-file injection (`extra_configs`), udev masking (`GNS3_MASK_UDEV`) so privileged systemd containers don't disturb the host, and the host-readiness check. + --- ## GNS3 AI Copilot (`gns3-copilot/`) @@ -103,6 +109,7 @@ Real-time traffic insight via per-link BPF markers and project-level inherited d ## Known Issues (`bugs/`) - [Telnet Server Connection Race Condition](bugs/telnet-server-connection-race-condition.md) — `getpeername()` error when client disconnects during connection setup (High severity, Open) +- [Docker Link UDP Self-Loop](bugs/link-udp-self-loop.md) — intermittent one-way link (both ends handed the same UDP port by an allocation race); **fixed** (Medium severity) --- @@ -120,4 +127,4 @@ Quick-start guide for Ubuntu 24.04: install via PPA, set up dependencies, and ru --- -_Last updated: 2026-04-20_ +_Last updated: 2026-08-14_ diff --git a/docs/bugs/link-udp-self-loop.md b/docs/bugs/link-udp-self-loop.md new file mode 100644 index 000000000..e1e05e7cf --- /dev/null +++ b/docs/bugs/link-udp-self-loop.md @@ -0,0 +1,101 @@ + + +> This documentation is organized by AI with reference to actual code. AI can make mistakes — please verify against the source code when in doubt. + +# Docker Link UDP Self-Loop Bug (One-Way Link) + +## Bug Report + +**Date**: 2026-08-14 +**Severity**: Medium (one-way connectivity, CPU burn from packet duplication; intermittent) +**Status**: **Fixed** — root cause found and unit-tested (same day) +**Component**: UDP port allocation — `gns3server/compute/port_manager.py` +(`get_free_udp_port` find-then-add race); secondary: `gns3server/controller/udp_link.py` +(`_prepare` accumulated stale `_link_data` on reset) + +## Symptoms + +Two Docker nodes (observed with Cisco XRd; node-type agnostic) linked on the +same compute cannot ping each other. Packet capture on the link shows only **one** +side sending ARP. The other side's traffic never appears on the link at all. + +## Evidence (from the live occurrence) + +Captured simultaneously on both nodes' uBridge `bridge1` (see diagnostics below): + +| Direction | Result | +|---|---| +| A → B | works — A's ARP requests arrive at B's bridge, B replies | +| B → A | dead — B's replies/ICMP appear only on **B's own bridge** (duplicated ×2–×3), nothing arrives at A | +| B's `ethN` counters | RX ≈ TX ≈ 5000+ — B receives its own transmissions back | +| A's `ethN` counters | TX > 0, RX = 0 — never receives anything | + +## Root Cause (confirmed) + +**`PortManager.get_free_udp_port` had an unguarded find-then-add sequence.** +A link allocates the UDP port for **both ends concurrently** +(`asyncio.gather` in `UDPLink._prepare` → two `POST /ports/udp`). The route +handler is a sync `def`, so FastAPI executes the two requests **in parallel +threads**. Both threads ran `find_unused_port` (socket-probing, GIL-releasing) +before either reached `_used_udp_ports.add(port)` — the set add is idempotent, +so no error was raised and **both ends were handed the same port number**: +`lport == rport` on both NIOs, a literal self-loop. + +Why it was *silent* and *asymmetric*: + +- uBridge sets `SO_REUSEADDR` on UDP NIO sockets (`ubridge/src/nio_udp.c`), so + the second bind of the same port **succeeds** instead of failing with + `EADDRINUSE` — link creation returned success. +- With two sockets bound to the same port, the kernel delivers to one of them + (last bound wins). The node that started later — in the live case B, + restarted ~77 s after A — received **everything**: A's packets *and* its own + transmissions echoed back. The 77 s restart did not cause the corruption; it + only decided which end starves. +- The same-compute condition is part of the trigger: both allocations hit the + same `PortManager` instance (a cross-compute link races two processes and + cannot self-collide). + +A second, smaller defect was found while auditing: `UDPLink._prepare()` +**appended** to `self._link_data` but the committed NIOs are always taken from +indices 0/1 — after `reset()` (delete + create on the same object) the stale, +already-released port pair was re-committed and the freshly allocated ports +were leaked. + +## Fix + +| Change | Where | +|---|---| +| `threading.RLock` making find-then-add (and reserve/release) atomic for TCP and UDP | `gns3server/compute/port_manager.py` | +| `_prepare()` rebuilds `_link_data` from scratch instead of appending | `gns3server/controller/udp_link.py` | +| Regression tests: threaded allocation never returns duplicates (red on the old code); `reset()` commits the fresh mirrored pair with `lport != rport` | `tests/compute/test_port_manager.py`, `tests/controller/test_udp_link.py` | + +## Diagnostics (uBridge console is the fast path) + +1. **uBridge console** — each node's uBridge listens on a Unix socket + `/run/user/1000/gns3/ubridge-.sock`; connect and send: + `bridge list` (NIO count per bridge), and + `bridge start_capture bridge "/tmp/ub-.pcap"` / + `bridge stop_capture bridge` to capture what the bridge actually forwards. + Comparing the two ends' pcaps localizes the break immediately. +2. **Container counters** — `docker exec ip -s link show ethN`: + TX>0/RX=0 → peer never returns; RX≈TX huge with µs-scale duplicates → self-loop. +3. **UDP sockets** — `ss -uln` (no `-p`; uBridge runs setuid-root so process names + are hidden, ports are visible): a two-node link owns exactly two "orphan" UDP + ports. **One port instead of two = this bug.** +4. **Recovery** — delete and re-create the link (or stop/start both nodes); + with the fix, the corruption no longer occurs in the first place. + +Note: a Docker node's in-container `ethN` is a **TAP device** whose file descriptor +lives inside uBridge (the interface is created host-side, then moved into the +container namespace and renamed). There is no veth host end to look for — do not +waste time hunting for one in the host namespace. + +## Related + +- `docs/features/vendor-nos-xrd.md` — troubleshooting table entry pointing here. +- Link-create batch optimization (`controller/udp_link.py`, project-open bulk path) + was exonerated: the pool path allocates sequentially in one handler and cannot + self-collide. diff --git a/docs/features/vendor-nos-xrd.md b/docs/features/vendor-nos-xrd.md new file mode 100644 index 000000000..a512b9b35 --- /dev/null +++ b/docs/features/vendor-nos-xrd.md @@ -0,0 +1,208 @@ + + +> This documentation is organized by AI with reference to actual code. AI can make mistakes — please verify against the source code when in doubt. + + +# Cisco XRd Control Plane (Vendor NOS Adaptation) + +## Overview + +Cisco XRd Control Plane runs as a first-class GNS3 Docker router node by +combining the existing vendor NOS path (`console_type: "docker_exec"` + +`GNS3_SKIP_INIT=1`, see [docker-exec-console.md](./docker-exec-console.md)) +with four generic server mechanisms added for heavy/systemd NOS containers: +`/dev/shm` and host-device injection, config-file injection (`extra_configs`), +and udev masking. XRd itself is pure appliance configuration — no image +rebuild, no source patching. + +## Why XRd must take the vendor path + +XRd boots `/usr/sbin/init` (systemd) as PID 1. GNS3's generic init.sh +wrapper chain (`/gns3/init.sh → su → run-cmd.sh → /usr/sbin/init`) crashes +XRd's glibc loader with `Fatal glibc error: dl-call-libc-early-init.c:37 +(sym != NULL)` (SIGABRT loop). With `GNS3_SKIP_INIT=1` the container runs +its native entrypoint directly and boots cleanly — same arrangement as +SR Linux. + +## Architecture + +```mermaid +graph TB + subgraph Appliance["XRd appliance (.gns3a) — pure configuration"] + ENV["environment: GNS3_SKIP_INIT / GNS3_CONSOLE_CMD / GNS3_MASK_UDEV / GNS3_SHM_SIZE / GNS3_DEVICES + XR_*"] + XC["extra_configs: /firstboot.cfg"] + XV["extra_volumes: /xr-storage + /xr-storage-shadow"] + end + subgraph Server["gns3-server (generic mechanisms)"] + CREATE["DockerVM.create() HostConfig"] + MASK["GNS3_MASK_UDEV → /dev/null binds"] + HOSTCFG["ShmSize / Devices"] + CFGINJ["extra_configs → RO single-file bind"] + VBRIDGE["VendorDockerVM volume bridge"] + HOSTCHK["host-readiness check (read-only)"] + end + subgraph Container["XRd container"] + SYSTEMD["systemd (/usr/sbin/init)"] + XR["XR control plane"] + XRS["/xr-storage (persisted, live data layer)"] + end + ENV --> CREATE --> SYSTEMD + ENV --> MASK & HOSTCFG + XC --> CFGINJ + XV --> VBRIDGE --> XRS + HOSTCHK -.->|"warn: inotify/file-max/fuse"| Server +``` + +## Mechanisms added (all generic, XRd is just the first consumer) + +| Mechanism | Interface | Effect | Where | +|-----------|-----------|--------|-------| +| shm size | `GNS3_SHM_SIZE=1024` (MB) in `environment` | native `HostConfig.ShmSize` at create time — works with or without init.sh | `docker_vm.py` `create()` | +| host devices | `GNS3_DEVICES=/dev/fuse` (`docker run --device` syntax, space-separated) | native `HostConfig.Devices` | `docker_vm.py` `_format_devices()` | +| config injection | `extra_configs: [{target, content}]` (template/node/appliance schema field) | content written under the node dir, bind-mounted **read-only** at `target` — seeds NOS startup configs without rebuilding the image | `docker_vm.py` `_mount_binds()`; persisted in `docker_templates.extra_configs` (Alembic migration) | +| udev masking | `GNS3_MASK_UDEV=1` | `/dev/null` over the 5 udev systemd units **and** `/bin|/sbin|/usr/bin/udevadm` | `docker_vm.py` `create()` | +| generic unit mask | `GNS3_MASK_SYSTEMD=u1,u2` | `/dev/null` over arbitrary `/etc/systemd/system/` | `docker_vm.py` `create()` | +| graceful stop | `GNS3_STOP_TIMEOUT=60` (seconds; default 60, max 210) | explicit user stop sends SIGTERM + a grace period (Docker SIGKILLs once it expires) instead of the base class' immediate kill — systemd NOS images require a graceful shutdown; internal paths (delete/update/close) keep the immediate kill since the container is force-deleted right after. Max 210 keeps the +30 s HTTP margin inside the controller's 240 s stop budget | `vendor_docker_vm.py` `_terminate_container()` | +| host check | automatic at Docker connect | read-only `/proc` check of inotify/file-max/FUSE; warns with exact fix commands (server is unprivileged — it can only check) | `compute/docker/__init__.py` `_check_host_readiness()` | + +`GNS3_*` variables are consumed host-side only and never forwarded into the +container (existing GNS3 behaviour); `XR_*` variables pass through normally. + +## Host-disturbance root causes (all fixed) + +A privileged systemd container can disturb the *host* desktop. Three +independent causes were isolated with plain-`docker run` A/B/C experiments +(udevd coldplug / busybox chown crash / direct `udevadm trigger`): + +| Host symptom | Root cause | Fix | +|---|---|---| +| Audio muted on every node start | container `systemd-udevd` coldplug replays **all** devices it can see (privileged → host `/sys`) | `GNS3_MASK_UDEV=1` (unit masks) | +| USB reconnects (mouse notification), journal noise | XRd's own `xr_startup.sh` calls `udevadm trigger --action=add --parent-match=` (USB license-dongle probing) — a direct binary call, unit masks don't stop it | `GNS3_MASK_UDEV=1` (udevadm null-bind) | +| Same USB/journal noise + broken persistence | static busybox `chown` dlopens container NSS modules → glibc abort → per-file coredump storm → host `systemd-coredump` rescans devices | vendor volume path prefers the container's own `chown` (`vendor_docker_vm.py`) | + +Diagnostics: `udevadm monitor --kernel --udev` (uevent stream), +`docker exec grep -n udevadm /opt/cisco/install-iosxr/base/etc/xr_startup.sh`. +Note: "journal corrupted" messages with varying machine-IDs come from the +*container's* journald (random machine-id per start), not the host journal. + +## XRd appliance recipe + +| Field | Value | +|-------|-------| +| `image` | official `ios-xr/xrd-control-plane:` — no wrapper image needed | +| `console_type` | `docker_exec` | +| `extra_volumes` | `["/xr-storage", "/xr-storage-shadow"]` | +| `extra_configs` | `{target: /firstboot.cfg, content: }` | + +``` +GNS3_SKIP_INIT=1 +GNS3_CONSOLE_CMD=/pkg/bin/xr_cli.sh +GNS3_MASK_UDEV=1 +GNS3_SHM_SIZE=1024 +GNS3_DEVICES=/dev/fuse +GNS3_STOP_TIMEOUT=40 +XR_FIRST_BOOT_CONFIG=/firstboot.cfg +XR_MGMT_INTERFACES=linux:eth0,xr_name=Mg0/RP0/CPU0/0,chksum,snoop_v4,snoop_v6 +XR_INTERFACES=linux:eth1,xr_name=Gi0/0/0/0;linux:eth2,xr_name=Gi0/0/0/1;... +``` + +XRd-specific gotchas (image-side, not GNS3): + +- Management interface xr_name is **`Mg0/RP0/CPU0/0`** (short prefix, `CPU0` + without slash). `MgmtEth0/RP0/CPU/0` is rejected: "not a valid + rack/slot/instance/port combination". +- `XR_INTERFACES` must list exactly `adapters − 1` data interfaces (eth0 is + management). Changing the adapter count requires regenerating the string. +- **Persistence layout**: in the *image*, `/xr-storage/{config,disk1,log, + scratch}` are symlinks into `/xr-storage-shadow` (a pristine spare copy of + the initial state). At boot the bootstrap replaces the symlinks with real + directories, and XR writes everything — committed config (`commitdb`, + `running`) included — into **`/xr-storage`**, never touching the shadow + again. This mirrors containerlab, which bind-mounts `/xr-storage` + (`nodes/xrd/xrd.go`: "persist data by mounting /xr-storage"). The + appliance persists **both** paths so writes land on host regardless of + whether they happen before or after the symlink→directory transition. +- `XR_FIRST_BOOT_CONFIG` only applies when XR's config storage is empty + (first boot). To re-seed, delete and recreate the node. +- The official image ships no default login; the first-boot config must + create one (e.g. `username admin / group root-lr / secret ...`). +- Docker mounts are fixed at container *create* time: after changing a + template's `extra_volumes`, existing nodes must be deleted and recreated + (a stop/start keeps the old mounts). +- Host sysctls (XRd's own requirements, same for containerlab): + `fs.inotify.max_user_instances=64000`, `max_user_watches=524288`, + `fs.file-max=1000000`, FUSE module loaded. GNS3 warns about these at + Docker connect; the admin raises them once. XRd also warns (non-fatal) + about `net.core.*` socket buffer sizes. + +## Business process + +```mermaid +sequenceDiagram + participant U as User + participant S as gns3-server + participant D as Docker daemon + participant X as XRd container + U->>S: create node from template + S->>S: parse GNS3_* env host-side + S->>D: container create (ShmSize, Devices, /dev/null binds, firstboot.cfg RO bind) + U->>S: start + S->>X: container start (native entrypoint /usr/sbin/init) + Note over X: systemd boots; udevd + udevadm masked → host untouched + S->>X: docker exec volume bridge (container's own chown) + U->>S: open console + S->>X: docker exec pty: /pkg/bin/xr_cli.sh + X-->>U: IOS XR CLI (first boot: apply /firstboot.cfg, save to /xr-storage-shadow) +``` + +## Troubleshooting + +| Symptom | Cause / fix | +|---|---| +| Node exits 139, `Fatal glibc error ... sym != NULL` in logs | init.sh wrapper path — set `GNS3_SKIP_INIT=1` **and** `console_type: docker_exec` (the flag is only honoured on the vendor class) | +| `XR_FIRST_BOOT_CONFIG ... File not found` | env path and `extra_configs` target disagree (e.g. `/firstboot.cfg` vs `/first_boot.cfg`), or entry missing | +| Console stuck at `Username:` with no credentials | image has no default user; provide a first-boot config creating one, then **recreate** the node (first-boot only runs on empty config storage) | +| `Invalid interface entries ... XR_MGMT_INTERFACES` | use `xr_name=Mg0/RP0/CPU0/0` | +| Host audio muted / USB reconnects when the node starts | set `GNS3_MASK_UDEV=1` | +| Config lost across stop/start | `extra_volumes` must include `/xr-storage` (XR's live data layer; the shadow alone is only a pristine spare). Changing `extra_volumes` requires deleting and recreating the node — Docker mounts are fixed at create time | +| Two nodes can't ping, only one side ARPs | GNS3 link wiring bug (UDP self-loop on one end), not XRd — fixed (port-allocation race); on older builds delete and re-create the link; see [link-udp-self-loop](../bugs/link-udp-self-loop.md) | +| Compute log: busybox coredump storm | fixed by the container-chown change; verify gns3-server is current | + +## Notes + +- All four mechanisms are opt-in: nodes that don't set the variables or the + field get byte-identical container configuration. +- `extra_configs` is a schema field (unlike the env knobs) because the + `environment` field is line-delimited and cannot carry multi-line file + content. +- Template fields live in three places (pydantic schema, DB column, Alembic + migration) — see the `extra_configs` DB migration when adding new ones. +- `net.core.*` socket-buffer requirements are not yet part of the + host-readiness check (XRd warns about them itself, non-fatally). + +## References + +- `gns3server/compute/docker/docker_vm.py` — HostConfig env injection, + `_UDEV_UNITS`/`_UDEVADM_PATHS`, `extra_configs` binds, `_format_devices()` +- `gns3server/compute/docker/vendor_docker_vm.py` — vendor path, volume + bridge, container-chown +- `gns3server/compute/docker/__init__.py` — `_check_host_readiness()` +- `gns3server/schemas/common.py` — `ExtraConfig` +- `gns3server/db/models/templates.py` + `db_migrations/` — persistence +- [docker-exec-console.md](./docker-exec-console.md) — the vendor NOS base + (docker_exec console, SKIP_INIT volume persistence) +- containerlab `nodes/xrd/xrd.go` — reference for XRd env defaults and + `/xr-storage` persistence + +## Version History + +| Version | Date | Changes | +|---------|------|---------| +| 1.4 | 2026-08-15 | Code-review hardening: stop-query HTTP timeout scales with `GNS3_STOP_TIMEOUT` (values >300 s no longer abort); overlapping mask/config bind targets deduplicated (Docker "Duplicate mount point"); `ExtraConfig.target` validated at save time and directory forms rejected; host-readiness check no longer aborts on one unreadable `/proc/sys` key; base env parser strips trailing commas; vendor env knobs re-parsed on create (PUT environment takes effect); graceful stop limited to explicit user stop (delete/update/close keep the immediate kill); extra_configs under a persisted volume warns. | +| 1.3 | 2026-08-14 | Persistence corrected: XR's live data layer is `/xr-storage` (the image's symlink farm is materialized into real directories at bootstrap; `/xr-storage-shadow` is a pristine spare) — the appliance persists both. Vendor containers now stop gracefully (SIGTERM + `GNS3_STOP_TIMEOUT` grace, default 60 s) instead of being SIGKILLed on the spot. | +| 1.2 | 2026-08-14 | Link UDP self-loop root-caused to a port-allocation race and fixed — see [link-udp-self-loop](../bugs/link-udp-self-loop.md) (now marked Fixed). | +| 1.1 | 2026-08-14 | Datapath validated end-to-end (XRd brings its own interfaces up; ARP/ICMP bidirectional). Add troubleshooting entry for the one-way-link symptom (GNS3 link UDP self-loop bug, see bugs/link-udp-self-loop.md). | +| 1.0 | 2026-08-14 | Initial documentation of the XRd control-plane adaptation: vendor path requirement, shm/devices/extra_configs/udev-mask mechanisms, host-disturbance root causes, appliance recipe. | diff --git a/gns3server/agent/gns3_copilot/gns3_client/custom_gns3fy.py b/gns3server/agent/gns3_copilot/gns3_client/custom_gns3fy.py index edb9612da..582e7100b 100644 --- a/gns3server/agent/gns3_copilot/gns3_client/custom_gns3fy.py +++ b/gns3server/agent/gns3_copilot/gns3_client/custom_gns3fy.py @@ -93,14 +93,19 @@ NODE_TYPES = [ "qemu", ] +# Keep in sync with gns3server.schemas.common.ConsoleType. The values are +# duplicated as literals because this module is shared with the standalone +# MCP service and cannot import the enum. "null" is gns3fy legacy. CONSOLE_TYPES = [ "vnc", "telnet", + "ssh", "http", "https", "spice", "spice+agent", "none", + "docker_exec", "null", ] diff --git a/gns3server/api/routes/compute/docker_nodes.py b/gns3server/api/routes/compute/docker_nodes.py index e64ec3d60..be55ec0f4 100644 --- a/gns3server/api/routes/compute/docker_nodes.py +++ b/gns3server/api/routes/compute/docker_nodes.py @@ -78,6 +78,7 @@ async def create_docker_node(project_id: UUID, node_data: schemas.DockerCreate) aux_type=node_data.pop("aux_type", "none"), extra_hosts=node_data.get("extra_hosts"), extra_volumes=node_data.get("extra_volumes"), + extra_configs=node_data.get("extra_configs"), memory=node_data.get("memory", 0), cpus=node_data.get("cpus", 0), ) @@ -87,7 +88,8 @@ async def create_docker_node(project_id: UUID, node_data: schemas.DockerCreate) for key in ( "console", "console_type", "console_resolution", "console_http_port", "console_http_path", "aux", "aux_type", "start_command", "environment", - "adapters", "mac_address", "extra_hosts", "extra_volumes", "memory", "cpus", + "adapters", "mac_address", "extra_hosts", "extra_volumes", "extra_configs", + "memory", "cpus", ): node_data.pop(key, None) for name, value in node_data.items(): @@ -137,6 +139,7 @@ async def update_docker_node(node_data: schemas.DockerUpdate, node: DockerVM = D "custom_adapters", "extra_hosts", "extra_volumes", + "extra_configs", "memory", "cpus", ] @@ -174,10 +177,12 @@ async def start_docker_node(node: DockerVM = Depends(dep_node)) -> None: ) async def stop_docker_node(node: DockerVM = Depends(dep_node)) -> None: """ - Stop a Docker node. + Stop a Docker node. This is the explicit user stop — the only path that + asks for a graceful SIGTERM shutdown (vendor NOS override); internal + paths (delete/update/close) keep the immediate kill. """ - await node.stop() + await node.stop(graceful=True) @router.post( diff --git a/gns3server/compute/docker/__init__.py b/gns3server/compute/docker/__init__.py index ab095084f..ed37ca626 100644 --- a/gns3server/compute/docker/__init__.py +++ b/gns3server/compute/docker/__init__.py @@ -59,6 +59,7 @@ class Docker(BaseManager): self._connector = None self._session = None self._api_version = DOCKER_MINIMUM_API_VERSION + self._host_checked = False def _select_node_class(self, **kwargs): """Select the node class based on console_type.""" @@ -160,6 +161,62 @@ class Docker(BaseManager): log.warning("Using Docker client with the minimum API version {}".format(self._api_version)) log.info("Connected to Docker daemon version {} using API version {}".format(version, self._api_version)) + self._check_host_readiness() + + def _check_host_readiness(self): + """ + Best-effort, read-only check of kernel settings that heavy NOS containers + (e.g. Cisco XRd) need. The server runs unprivileged (only the setuid + ubridge helper gets root), so we cannot raise these limits ourselves -- + we only warn, with the exact commands to fix, when they are too low or + when FUSE support is missing. Runs at most once per process. + """ + + if self._host_checked: + return + self._host_checked = True + + # Thresholds recommended for running several heavy containers (sized for + # ~15 XRd-style nodes). Raising them is harmless; the stock Linux defaults + # (e.g. max_user_instances=128) are far too low and break such images. + thresholds = { + "fs.inotify.max_user_instances": 64000, + "fs.inotify.max_user_watches": 524288, + "fs.file-max": 1000000, + } + low = [] + for key, minimum in thresholds.items(): + try: + with open(f"/proc/sys/{key.replace('.', '/')}") as f: + current = int(f.read().strip()) + except (OSError, ValueError): + # One unreadable key must not discard the warnings already + # collected nor skip the FUSE check — skip just this key. + continue + if current < minimum: + low.append((key, current, minimum)) + + fuse_supported = False + try: + with open("/proc/filesystems") as f: + filesystems = {parts[-1] for parts in (line.split() for line in f) if parts} + fuse_supported = "fuse" in filesystems or "fuseblk" in filesystems + except OSError: + pass + + if low: + details = ", ".join(f"{k}={c} (need >={m})" for k, c, m in low) + raise_cmd = " ".join(f"{k}={m}" for k, _, m in low) + log.warning( + f"Low kernel limits for heavy Docker containers ({details}). " + f"Some NOS images (e.g. Cisco XRd) may fail to start. Raise once: " + f"'sudo sysctl -w {raise_cmd}' and persist it under /etc/sysctl.d/." + ) + if not fuse_supported: + log.warning( + "FUSE filesystem support is not available in the kernel. " + "Containers that need it (e.g. Cisco XRd) will fail. Load it: 'sudo modprobe fuse'." + ) def connector(self): diff --git a/gns3server/compute/docker/docker_vm.py b/gns3server/compute/docker/docker_vm.py index 804039216..fa45c43e6 100644 --- a/gns3server/compute/docker/docker_vm.py +++ b/gns3server/compute/docker/docker_vm.py @@ -69,6 +69,30 @@ class DockerVM(BaseNode): :param extra_volumes: Additional directories to make persistent """ + # systemd units masked by GNS3_MASK_UDEV=1: the udev daemon, its activation + # sockets and the coldplug/settle triggers. Masking them stops a privileged + # systemd container from replaying device events on the host. + _UDEV_UNITS = ( + "systemd-udevd.service", + "systemd-udevd-control.socket", + "systemd-udevd-kernel.socket", + "systemd-udev-trigger.service", + "systemd-udev-settle.service", + ) + + # udevadm binary paths also null-bound by GNS3_MASK_UDEV=1. NOS startup + # scripts call udevadm directly -- Cisco XRd's xr_startup.sh runs + # `udevadm trigger --action=add --parent-match=` (USB license + # dongle probing), which synthesizes uevents into the host kernel from a + # privileged container and reconnects host USB devices. Masking the units + # alone does not stop this; the binary must be neutralized too. XRd boots + # fine without udevadm (interfaces are pre-created by GNS3). + _UDEVADM_PATHS = ( + "/bin/udevadm", + "/sbin/udevadm", + "/usr/bin/udevadm", + ) + def __init__( self, name, @@ -89,6 +113,7 @@ class DockerVM(BaseNode): console_http_path="/", extra_hosts=None, extra_volumes=[], + extra_configs=None, memory=0, cpus=0, ): @@ -118,6 +143,7 @@ class DockerVM(BaseNode): self._console_websocket = None self._extra_hosts = extra_hosts self._extra_volumes = extra_volumes or [] + self._extra_configs = extra_configs or [] self._memory = memory self._cpus = cpus self._permissions_fixed = True @@ -164,6 +190,7 @@ class DockerVM(BaseNode): "node_directory": self.working_path, "extra_hosts": self.extra_hosts, "extra_volumes": self.extra_volumes, + "extra_configs": self.extra_configs, "memory": self.memory, "cpus": self.cpus, } @@ -295,6 +322,14 @@ class DockerVM(BaseNode): def extra_volumes(self, extra_volumes): self._extra_volumes = extra_volumes + @property + def extra_configs(self): + return self._extra_configs + + @extra_configs.setter + def extra_configs(self, extra_configs): + self._extra_configs = extra_configs or [] + @property def memory(self): return self._memory @@ -391,6 +426,39 @@ class DockerVM(BaseNode): "Target": "/gns3volumes{}".format(volume) }) + # Inject extra config files: write each to the node working directory and + # bind-mount it read-only at its target path. Single-file binds are applied + # at create time, so this works for the generic init.sh path AND for vendor + # nodes that skip init.sh (the NOS reads its startup config from the mount). + for cfg in self._extra_configs: + target = cfg["target"] if isinstance(cfg, dict) else cfg.target + content = cfg["content"] if isinstance(cfg, dict) else cfg.content + if not target.startswith("/") or target.endswith("/") or ".." in target.split("/"): + raise DockerError( + f"Extra config target '{target}' must be an absolute file path and not contain '..'." + ) + for volume in self._volumes: + # A single-file bind gets covered by the volume's bind mount at + # start (init.sh or the vendor volume bridge), so the injected + # content would never be seen — or worse, frozen at whatever + # the first-start seed copied. + if target == volume or target.startswith(volume.rstrip("/") + "/"): + log.warning( + "Extra config target '%s' on container '%s' is shadowed by persisted volume '%s' " + "and will not take effect; pick a target outside persisted volumes.", + target, self._name, volume, + ) + host_path = os.path.join(self.working_dir, "configs", target.lstrip("/")) + os.makedirs(os.path.dirname(host_path), exist_ok=True) + with open(host_path, "w") as f: + f.write(content) + binds.append({ + "Type": "bind", + "Source": host_path, + "Target": target, + "ReadOnly": True, + }) + return binds def _create_network_config(self): @@ -491,6 +559,68 @@ class DockerVM(BaseNode): "Entrypoint": image_infos.get("Config", {"Entrypoint": []}).get("Entrypoint"), } + # Optional /dev/shm size and host device mappings requested through the + # environment (GNS3_SHM_SIZE in MB, GNS3_DEVICES). These are native Docker + # HostConfig keys applied at create time, so they work whether or not + # init.sh runs -- heavy NOS containers such as Cisco XRd (which skips + # init.sh via the vendor/docker_exec path) rely on them. Only injected + # when set, so ordinary nodes keep the default Docker behaviour. + if self._environment: + for line in self._environment.splitlines(): + # Strip a trailing comma like the vendor-class parser does, so + # "GNS3_MASK_UDEV=1," composed from a comma-separated list + # still activates (values are never comma-separated here). + line = line.strip().rstrip(",") + if line.startswith("GNS3_SHM_SIZE="): + try: + params["HostConfig"]["ShmSize"] = int(line.split("=", 1)[1].strip()) * (1024 * 1024) + except ValueError: + pass + elif line.startswith("GNS3_DEVICES="): + devices = self._format_devices(line.split("=", 1)[1]) + if devices: + params["HostConfig"]["Devices"] = devices + elif line.startswith("GNS3_MASK_UDEV=") and \ + line.split("=", 1)[1].strip().lower() in ("1", "true", "yes"): + # A privileged systemd-based NOS container (e.g. Cisco XRd) + # runs systemd-udevd, which coldplugs every device it can see + # -- and in privileged mode that includes the HOST's USB/input/ + # audio/disk devices, reconnecting/muting them on every start. + # XRd doesn't need udev (interfaces are pre-created by GNS3), so + # bind /dev/null over the udev units to keep it from running. + for target in [f"/etc/systemd/system/{u}" for u in self._UDEV_UNITS] + list(self._UDEVADM_PATHS): + params["HostConfig"]["Mounts"].append({ + "Type": "bind", + "Source": "/dev/null", + "Target": target, + "ReadOnly": True, + }) + elif line.startswith("GNS3_MASK_SYSTEMD="): + # Generic form: comma/semicolon-separated unit names to mask + # the same way (bind /dev/null over /etc/systemd/system/). + for unit in line.split("=", 1)[1].replace(";", ",").split(","): + unit = unit.strip() + if unit and "/" not in unit and ".." not in unit: + params["HostConfig"]["Mounts"].append({ + "Type": "bind", + "Source": "/dev/null", + "Target": f"/etc/systemd/system/{unit}", + "ReadOnly": True, + }) + + # Overlapping bind targets (GNS3_MASK_UDEV together with a + # GNS3_MASK_SYSTEMD entry for the same unit, an extra_configs target + # equal to a masked unit, a unit named twice in the list) make Docker + # reject the create outright ("Duplicate mount point") — keep only + # the first occurrence of each target. + seen_targets = set() + deduped_mounts = [] + for mount in params["HostConfig"]["Mounts"]: + if mount["Target"] not in seen_targets: + seen_targets.add(mount["Target"]) + deduped_mounts.append(mount) + params["HostConfig"]["Mounts"] = deduped_mounts + if params["Entrypoint"] is None: params["Entrypoint"] = [] if self._start_command: @@ -625,6 +755,37 @@ class DockerVM(BaseNode): raise DockerError(f"Can't apply `ExtraHosts`, wrong format: {extra_hosts}") return "\n".join([f"{h[1]}\t{h[0]}" for h in hosts]) + def _format_devices(self, devices_value): + """ + Parse a GNS3_DEVICES value into Docker HostConfig Devices entries. + + Mirrors `docker run --device`: items are whitespace/comma-separated and + each is ``host[:container[:permissions]]`` (e.g. /dev/fuse, + /dev/fuse:/dev/fuse:rwm). Docker resolves type/major/minor from the host + node itself, so the device must exist on the host -- the host-readiness + check warns when /dev/fuse is missing (load the fuse module). + """ + + formatted = [] + for raw in devices_value.replace(",", " ").split(): + parts = raw.split(":") + if len(parts) == 1: + on_host = in_container = parts[0] + permissions = "rwm" + elif len(parts) == 2: + on_host, in_container = parts + permissions = "rwm" + elif len(parts) == 3: + on_host, in_container, permissions = parts + else: + continue + formatted.append({ + "PathOnHost": on_host, + "PathInContainer": in_container, + "CgroupPermissions": permissions, + }) + return formatted + async def update(self): """ Destroy and recreate the container with the new settings @@ -1045,9 +1206,14 @@ class DockerVM(BaseNode): await telnet_server.wait_closed() self._telnet_servers = [] - async def stop(self): + async def stop(self, graceful: bool = False): """ Stops this Docker container. + + :param graceful: request a graceful SIGTERM shutdown (honoured by the + vendor NOS override). The default immediate kill is used on the + internal paths (delete, update, close, crash cleanup), where the + container is force-deleted or recreated right after anyway. """ try: @@ -1072,12 +1238,8 @@ class DockerVM(BaseNode): state = await self._get_container_state() if state != "stopped" and state != "exited": - # SIGKILL immediately. GNS3 has already persisted container state - # (permissions via _fix_permissions, /gns3volumes) before this - # point, and the business process (often an interactive shell) - # ignores SIGTERM — so a stop grace period buys nothing but latency. try: - await self.manager.query("POST", f"containers/{self._cid}/kill") + await self._terminate_container(graceful=graceful) log.debug(f"Docker container '{self._name}' [{self._image}] stopped") except DockerHttp409Error: # Container is already stopped @@ -1088,6 +1250,20 @@ class DockerVM(BaseNode): return self.status = "stopped" + async def _terminate_container(self, graceful: bool = False): + """ + Final termination of a still-running container: immediate SIGKILL. + GNS3 has already persisted container state (permissions via + _fix_permissions, /gns3volumes) before this point, and the business + process (often an interactive shell) ignores SIGTERM — a stop grace + period buys nothing but latency. Vendor NOS containers override this + with a graceful SIGTERM shutdown when asked (see VendorDockerVM); + the ``graceful`` flag is accepted here only for signature + compatibility. + """ + + await self.manager.query("POST", f"containers/{self._cid}/kill") + async def pause(self): """ Pauses this Docker container. diff --git a/gns3server/compute/docker/vendor_docker_vm.py b/gns3server/compute/docker/vendor_docker_vm.py index f0410d196..800d46603 100644 --- a/gns3server/compute/docker/vendor_docker_vm.py +++ b/gns3server/compute/docker/vendor_docker_vm.py @@ -35,7 +35,7 @@ import shutil from gns3server.utils.asyncio.telnet_server import AsyncioTelnetServer from gns3server.compute.docker.docker_vm import DockerVM -from gns3server.compute.docker.docker_error import DockerError, DockerHttp404Error +from gns3server.compute.docker.docker_error import DockerError, DockerHttp304Error, DockerHttp404Error log = logging.getLogger(__name__) @@ -55,18 +55,29 @@ class VendorDockerVM(DockerVM): (adapter order) instead of default ``eth{N}``. * ``GNS3_CONSOLE_CMD=/opt/srlinux/bin/sr_cli`` — command run inside the container by the ``docker_exec`` console (defaults to ``/bin/sh``). + * ``GNS3_STOP_TIMEOUT=60`` — SIGTERM grace period in seconds when stopping + the container (default 60; Docker SIGKILLs once it expires). """ def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) - # Prototype knobs — parsed from GNS3_-prefixed entries in create(). - # Parse eagerly so _get_container_ifname can return the right name. + self._console_exec_writer = None + # Parsed eagerly so _get_container_ifname can return the right name, + # and re-parsed on every create() so a PUT to the node's environment + # takes effect on the next (re)create instead of the next reload. + self._parse_vendor_environment() + + def _parse_vendor_environment(self): + """ + (Re)parse the GNS3_* knobs from the current ``environment`` value, + resetting to defaults first so removed entries stop applying. + """ + self._gns3_init = True self._interface_names = [] self._console_cmd = None - self._console_exec_writer = None - + self._stop_timeout = 60 if self._environment: for _line in self._environment.splitlines(): _line = _line.strip().rstrip(",") @@ -78,6 +89,24 @@ class VendorDockerVM(DockerVM): ] elif _line.startswith("GNS3_CONSOLE_CMD="): self._console_cmd = _line.split("=", 1)[1].strip() + elif _line.startswith("GNS3_STOP_TIMEOUT="): + try: + timeout = int(_line.split("=", 1)[1].strip()) + # Ceiling is derived from the call chain, not arbitrary: + # the controller's stop request times out at 240 s + # (controller/node.py) and the Docker stop query gets + # this value +30 s as its HTTP timeout — so anything + # above 210 would abort upstream first. + if 1 <= timeout <= 210: + self._stop_timeout = timeout + except ValueError: + pass + + async def create(self): + # The environment may have changed since __init__ (PUT on the node) — + # re-parse the knobs so the recreated container picks them up. + self._parse_vendor_environment() + return await super().create() # ---- hook overrides --------------------------------------------------- @@ -138,6 +167,37 @@ class VendorDockerVM(DockerVM): pass self._console_exec_writer = None + async def _terminate_container(self, graceful: bool = False): + """ + Override: vendor NOS containers run systemd and require a graceful + shutdown (e.g. Cisco XRd treats an abrupt SIGKILL as an unclean + shutdown). + + With ``graceful`` (explicit user stop), send SIGTERM and wait up to + ``GNS3_STOP_TIMEOUT`` seconds (default 60, 1-210 — the ceiling keeps + the +30 s HTTP margin inside the controller's 240 s stop budget) for + the services to stop; Docker SIGKILLs the container itself once the + grace period expires, so no fallback kill is needed. + + Without ``graceful`` (delete/update/close/crash cleanup), fall back to + the base immediate kill: those paths force-delete or recreate the + container right after anyway, so a grace period buys nothing but + latency. + """ + if not graceful: + await super()._terminate_container(graceful=False) + return + try: + response = await self.manager.http_query( + "POST", + f"containers/{self._cid}/stop", + params={"t": self._stop_timeout}, + timeout=self._stop_timeout + 30, + ) + response.close() + except DockerHttp304Error: + pass # already stopped + async def start(self): await super().start() if self.status == "started" and not self._gns3_init: @@ -183,6 +243,12 @@ class VendorDockerVM(DockerVM): target = f"/gns3volumes{volume}" log.debug("Docker container '%s' fix ownership on %s", self._name, target) try: + # chown prefers the container's own coreutils over /gns3/bin/busybox: + # busybox is static, and its chown dlopens NSS modules from the + # container, which mismatch the static glibc and abort (glibc + # "sym != NULL") on NOS images whose glibc differs from the host's + # (e.g. Cisco XRd). It falls back to busybox on minimal images that + # ship no chown. cp/chmod/find/stat don't use NSS, so stay busybox. process = await asyncio.subprocess.create_subprocess_exec( "docker", "exec", @@ -195,7 +261,7 @@ class VendorDockerVM(DockerVM): f" | /gns3/bin/busybox xargs -0 /gns3/bin/busybox stat -c '%a:%u:%g:%n' > \"{target}/.gns3_perms\"" ")" f' && /gns3/bin/busybox chmod -R u+rX "{target}"' - f' && /gns3/bin/busybox chown {uid}:{gid} -R "{target}"', + f' && ( command -v chown >/dev/null 2>&1 && chown {uid}:{gid} -R "{target}" || /gns3/bin/busybox chown {uid}:{gid} -R "{target}" )', stderr=asyncio.subprocess.PIPE, ) except OSError as e: @@ -233,7 +299,10 @@ class VendorDockerVM(DockerVM): f'/gns3/bin/busybox mount --bind "{vol_target}" "{volume}" && ' f'while IFS=: read -r PERMS OWNER GROUP FILE; do ' f' [ -L "$FILE" ] || /gns3/bin/busybox chmod "$PERMS" "$FILE" 2>/dev/null; ' - f' /gns3/bin/busybox chown -h "$OWNER:$GROUP" "$FILE" 2>/dev/null; ' + # chown: prefer the container's coreutils, fall back to busybox + # (see _fix_permissions -- static busybox chown aborts on + # mismatched-glibc NOS images like XRd). + f' ( command -v chown >/dev/null 2>&1 && chown -h "$OWNER:$GROUP" "$FILE" || /gns3/bin/busybox chown -h "$OWNER:$GROUP" "$FILE" ) 2>/dev/null; ' f'done < "{volume}/.gns3_perms"' ) # fmt: on diff --git a/gns3server/compute/port_manager.py b/gns3server/compute/port_manager.py index 6dd2549a8..dbb18fc0e 100644 --- a/gns3server/compute/port_manager.py +++ b/gns3server/compute/port_manager.py @@ -16,6 +16,7 @@ import socket import ipaddress +import threading from fastapi import HTTPException, status from gns3server.config import Config @@ -105,6 +106,13 @@ class PortManager: self._udp_host = "0.0.0.0" self._used_tcp_ports = set() self._used_udp_ports = set() + # Guards the find-then-add port allocation against concurrent threads: + # FastAPI runs sync route handlers (e.g. POST /ports/udp) in a thread + # pool, and a link allocates both of its ends concurrently — without + # the lock both threads can probe the same "free" port and hand the + # same number to both ends of a link (lport == rport self-loop). + # RLock because reserve_*_port falls back to get_free_*_port. + self._lock = threading.RLock() console_start_port_range = Config.instance().settings.Server.console_start_port_range console_end_port_range = Config.instance().settings.Server.console_end_port_range @@ -275,16 +283,17 @@ class PortManager: port_range_start = self._console_port_range[0] port_range_end = self._console_port_range[1] - port = self.find_unused_port( - port_range_start, - port_range_end, - host=self._console_host, - socket_type="TCP", - ignore_ports=self._used_tcp_ports, - ) + with self._lock: + port = self.find_unused_port( + port_range_start, + port_range_end, + host=self._console_host, + socket_type="TCP", + ignore_ports=self._used_tcp_ports, + ) - self._used_tcp_ports.add(port) - project.record_tcp_port(port) + self._used_tcp_ports.add(port) + project.record_tcp_port(port) log.debug(f"TCP port {port} has been allocated") return port @@ -305,32 +314,33 @@ class PortManager: port_range_start = self._console_port_range[0] port_range_end = self._console_port_range[1] - if port in self._used_tcp_ports: - old_port = port - port = self.get_free_tcp_port(project, port_range_start=port_range_start, port_range_end=port_range_end) - msg = f"TCP port {old_port} already in use on host {self._console_host}. Port has been replaced by {port}" - log.debug(msg) - return port - if port < port_range_start or port > port_range_end: - old_port = port - port = self.get_free_tcp_port(project, port_range_start=port_range_start, port_range_end=port_range_end) - msg = ( - f"TCP port {old_port} is outside the range {port_range_start}-{port_range_end} on host " - f"{self._console_host}. Port has been replaced by {port}" - ) - log.debug(msg) - return port - try: - PortManager._check_port(self._console_host, port, "TCP") - except OSError: - old_port = port - port = self.get_free_tcp_port(project, port_range_start=port_range_start, port_range_end=port_range_end) - msg = f"TCP port {old_port} already in use on host {self._console_host}. Port has been replaced by {port}" - log.debug(msg) - return port + with self._lock: + if port in self._used_tcp_ports: + old_port = port + port = self.get_free_tcp_port(project, port_range_start=port_range_start, port_range_end=port_range_end) + msg = f"TCP port {old_port} already in use on host {self._console_host}. Port has been replaced by {port}" + log.debug(msg) + return port + if port < port_range_start or port > port_range_end: + old_port = port + port = self.get_free_tcp_port(project, port_range_start=port_range_start, port_range_end=port_range_end) + msg = ( + f"TCP port {old_port} is outside the range {port_range_start}-{port_range_end} on host " + f"{self._console_host}. Port has been replaced by {port}" + ) + log.debug(msg) + return port + try: + PortManager._check_port(self._console_host, port, "TCP") + except OSError: + old_port = port + port = self.get_free_tcp_port(project, port_range_start=port_range_start, port_range_end=port_range_end) + msg = f"TCP port {old_port} already in use on host {self._console_host}. Port has been replaced by {port}" + log.debug(msg) + return port - self._used_tcp_ports.add(port) - project.record_tcp_port(port) + self._used_tcp_ports.add(port) + project.record_tcp_port(port) log.debug(f"TCP port {port} has been reserved") return port @@ -342,10 +352,11 @@ class PortManager: :param project: Project instance """ - if port in self._used_tcp_ports: - self._used_tcp_ports.remove(port) - project.remove_tcp_port(port) - log.debug(f"TCP port {port} has been released") + with self._lock: + if port in self._used_tcp_ports: + self._used_tcp_ports.remove(port) + project.remove_tcp_port(port) + log.debug(f"TCP port {port} has been released") def get_free_udp_port(self, project): """ @@ -353,16 +364,17 @@ class PortManager: :param project: Project instance """ - port = self.find_unused_port( - self._udp_port_range[0], - self._udp_port_range[1], - host=self._udp_host, - socket_type="UDP", - ignore_ports=self._used_udp_ports, - ) + with self._lock: + port = self.find_unused_port( + self._udp_port_range[0], + self._udp_port_range[1], + host=self._udp_host, + socket_type="UDP", + ignore_ports=self._used_udp_ports, + ) - self._used_udp_ports.add(port) - project.record_udp_port(port) + self._used_udp_ports.add(port) + project.record_udp_port(port) log.debug(f"UDP port {port} has been allocated") return port @@ -374,18 +386,20 @@ class PortManager: :param project: Project instance """ - if port in self._used_udp_ports: - raise HTTPException( - status_code=status.HTTP_409_CONFLICT, - detail=f"UDP port {port} already in use on host {self._console_host}", - ) - if port < self._udp_port_range[0] or port > self._udp_port_range[1]: - raise HTTPException( - status_code=status.HTTP_409_CONFLICT, - detail=f"UDP port {port} is outside the range " f"{self._udp_port_range[0]}-{self._udp_port_range[1]}", - ) - self._used_udp_ports.add(port) - project.record_udp_port(port) + with self._lock: + if port in self._used_udp_ports: + raise HTTPException( + status_code=status.HTTP_409_CONFLICT, + detail=f"UDP port {port} already in use on host {self._console_host}", + ) + if port < self._udp_port_range[0] or port > self._udp_port_range[1]: + raise HTTPException( + status_code=status.HTTP_409_CONFLICT, + detail=f"UDP port {port} is outside the range " + f"{self._udp_port_range[0]}-{self._udp_port_range[1]}", + ) + self._used_udp_ports.add(port) + project.record_udp_port(port) log.debug(f"UDP port {port} has been reserved") def release_udp_port(self, port, project): @@ -396,7 +410,8 @@ class PortManager: :param project: Project instance """ - if port in self._used_udp_ports: - self._used_udp_ports.remove(port) - project.remove_udp_port(port) - log.debug(f"UDP port {port} has been released") + with self._lock: + if port in self._used_udp_ports: + self._used_udp_ports.remove(port) + project.remove_udp_port(port) + log.debug(f"UDP port {port} has been released") diff --git a/gns3server/controller/udp_link.py b/gns3server/controller/udp_link.py index 86f863010..1184dad74 100644 --- a/gns3server/controller/udp_link.py +++ b/gns3server/controller/udp_link.py @@ -98,6 +98,13 @@ class UDPLink(Link): tuples, ready to be POSTed to each node's compute. """ + # Start from a clean slate: reset() re-creates the link on the same + # object (delete() + create()), and _commit_nios()/update() always + # address indices 0/1 — appending onto the previous run's entries + # would re-commit the stale (already released) port pair and orphan + # the freshly allocated ports. + self._link_data = [] + node1 = self._nodes[0]["node"] adapter_number1 = self._nodes[0]["adapter_number"] port_number1 = self._nodes[0]["port_number"] diff --git a/gns3server/db/models/templates.py b/gns3server/db/models/templates.py index f0100352c..5c271e436 100644 --- a/gns3server/db/models/templates.py +++ b/gns3server/db/models/templates.py @@ -78,6 +78,7 @@ class DockerTemplate(Template): console_resolution = Column(String) extra_hosts = Column(String) extra_volumes = Column(JSON) + extra_configs = Column(JSON) memory = Column(Integer) cpus = Column(Float) custom_adapters = Column(JSON) diff --git a/gns3server/db_migrations/versions/8f2a1c4e9d3b_add_extra_configs_to_docker_templates.py b/gns3server/db_migrations/versions/8f2a1c4e9d3b_add_extra_configs_to_docker_templates.py new file mode 100644 index 000000000..2be50f287 --- /dev/null +++ b/gns3server/db_migrations/versions/8f2a1c4e9d3b_add_extra_configs_to_docker_templates.py @@ -0,0 +1,26 @@ +"""add extra_configs to docker templates table + +Revision ID: 8f2a1c4e9d3b +Revises: f0b0de2a9 +Create Date: 2026-08-14 10:00:00.000000 + +""" +from alembic import op +import sqlalchemy as sa + + +# revision identifiers, used by Alembic. +revision = '8f2a1c4e9d3b' +down_revision = 'f0b0de2a9' +branch_labels = None +depends_on = None + + +def upgrade() -> None: + + op.add_column('docker_templates', sa.Column('extra_configs', sa.JSON())) + + +def downgrade() -> None: + + op.drop_column('docker_templates', 'extra_configs') diff --git a/gns3server/schemas/common.py b/gns3server/schemas/common.py index b15bc5201..68ca0b861 100644 --- a/gns3server/schemas/common.py +++ b/gns3server/schemas/common.py @@ -14,7 +14,7 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see . -from pydantic import BaseModel, Field +from pydantic import BaseModel, Field, field_validator from typing import Optional from enum import Enum @@ -48,6 +48,35 @@ class CustomAdapter(BaseModel): mac_address: Optional[str] = Field(None, pattern="^([0-9a-fA-F]{2}[:]){5}([0-9a-fA-F]{2})$") +class ExtraConfig(BaseModel): + """ + A configuration file injected into a Docker container. + + GNS3 writes ``content`` to a host file and bind-mounts it read-only at + ``target`` inside the container. Used to seed NOS startup configs (e.g. + XRd first-boot config, FRR frr.conf) without rebuilding the image. + """ + + target: str = Field(..., description="Absolute path inside the container where the file is mounted") + content: str = Field("", description="File content written by GNS3 and bind-mounted read-only into the container") + + @field_validator("target") + @classmethod + def target_is_an_absolute_file_path(cls, v): + """ + Reject at save time (template/appliance/node PUT) what would only + blow up at node-create time — after a potentially multi-GB image + pull: relative paths, '..' components and directory forms ('/', + '/etc/'). + """ + if not v.startswith("/") or v.endswith("/") or ".." in v.split("/"): + raise ValueError( + "target must be an absolute file path inside the container " + "(start with '/', name a file, no '..' components)" + ) + return v + + class ConsoleType(str, Enum): """ Supported console types. diff --git a/gns3server/schemas/compute/docker_nodes.py b/gns3server/schemas/compute/docker_nodes.py index 2328baabc..1461c18c1 100644 --- a/gns3server/schemas/compute/docker_nodes.py +++ b/gns3server/schemas/compute/docker_nodes.py @@ -18,7 +18,7 @@ from pydantic import BaseModel, Field from typing import Optional, List from uuid import UUID -from ..common import NodeStatus, CustomAdapter, ConsoleType, AuxType +from ..common import NodeStatus, CustomAdapter, ConsoleType, AuxType, ExtraConfig class DockerBase(BaseModel): @@ -43,6 +43,7 @@ class DockerBase(BaseModel): environment: Optional[str] = Field(None, description="Docker environment variables") extra_hosts: Optional[str] = Field(None, description="Docker extra hosts (added to /etc/hosts)") extra_volumes: Optional[List[str]] = Field(None, description="Additional directories to make persistent") + extra_configs: Optional[List[ExtraConfig]] = Field(None, description="Configuration files injected into the container (bind-mounted read-only)") memory: Optional[int] = Field(None, ge=0, description="Maximum amount of memory the container can use in MB") cpus: Optional[float] = Field(None, ge=0, description="Maximum amount of CPU resources the container can use") custom_adapters: Optional[List[CustomAdapter]] = Field(None, description="Custom adapters") diff --git a/gns3server/schemas/controller/appliances.py b/gns3server/schemas/controller/appliances.py index 23e7b32c0..44906df18 100644 --- a/gns3server/schemas/controller/appliances.py +++ b/gns3server/schemas/controller/appliances.py @@ -20,6 +20,7 @@ from enum import Enum from typing import Annotated, List, Literal, Optional, Union from uuid import UUID from pydantic import AnyUrl, BaseModel, Discriminator, EmailStr, Field, Tag +from ..common import ExtraConfig # ============================================================================ @@ -329,6 +330,7 @@ class Docker(BaseModel): console_http_path: Optional[str] = Field(None, description='Path of the web interface') extra_hosts: Optional[str] = Field(None, description='Hosts which will be written to /etc/hosts into container') extra_volumes: Optional[List[str]] = Field(None, description='Additional directories to make persistent that are not included in the images VOLUME directive') + extra_configs: Optional[List[ExtraConfig]] = Field(None, description='Configuration files injected into the container (bind-mounted read-only)') class Iou(BaseModel): @@ -479,6 +481,9 @@ class DockerPropertiesV8(BaseModel): extra_volumes: Optional[List[str]] = Field( None, title='Additional directories to make persistent' ) + extra_configs: Optional[List[ExtraConfig]] = Field( + None, title='Configuration files injected into the container (bind-mounted read-only)' + ) class IouPropertiesV8(BaseModel): diff --git a/gns3server/schemas/controller/templates/docker_templates.py b/gns3server/schemas/controller/templates/docker_templates.py index 5eb10d7f5..3d7688cbb 100644 --- a/gns3server/schemas/controller/templates/docker_templates.py +++ b/gns3server/schemas/controller/templates/docker_templates.py @@ -16,7 +16,7 @@ from . import Category, TemplateBase -from ...common import ConsoleType, AuxType, CustomAdapter +from ...common import ConsoleType, AuxType, CustomAdapter, ExtraConfig from pydantic import Field from typing import Optional, List @@ -49,6 +49,7 @@ class DockerTemplate(TemplateBase): ) extra_hosts: Optional[str] = Field("", description="Docker extra hosts (added to /etc/hosts)") extra_volumes: Optional[List] = Field([], description="Additional directories to make persistent") + extra_configs: Optional[List[ExtraConfig]] = Field(default_factory=list, description="Configuration files injected into the container (bind-mounted read-only)") memory: Optional[int] = Field(0, ge=0, description="Maximum amount of memory the container can use in MB") cpus: Optional[float] = Field(0, ge=0, description="Maximum amount of CPU resources the container can use") custom_adapters: Optional[List[CustomAdapter]] = Field(default_factory=list, description="Custom adapters") diff --git a/tests/agent/test_custom_gns3fy.py b/tests/agent/test_custom_gns3fy.py new file mode 100644 index 000000000..a07266432 --- /dev/null +++ b/tests/agent/test_custom_gns3fy.py @@ -0,0 +1,71 @@ +#!/usr/bin/env python +# +# Copyright (C) 2026 GNS3 Technologies Inc. +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +""" +The vendored gns3fy copy keeps its node/console type lists as literals +(it is shared with the standalone MCP service and cannot import server +enums). These tests fail when the server enums grow a value the vendored +lists have not picked up — exactly what happened with "docker_exec": one +vendor node failing validation made the copilot's topology reader drop +the whole project. +""" + +import pytest + + +def test_console_types_cover_server_enum(): + """ + Every server ConsoleType value must be accepted by the vendored Node model. + """ + pytest.importorskip("jwt", reason="ai-features extras not installed") + from gns3server.agent.gns3_copilot.gns3_client.custom_gns3fy import CONSOLE_TYPES + from gns3server.schemas.common import ConsoleType + + missing = {e.value for e in ConsoleType} - set(CONSOLE_TYPES) + assert not missing, f"CONSOLE_TYPES drifted from ConsoleType, missing: {missing}" + + +def test_node_types_cover_server_enum(): + """ + Every server NodeType value must be accepted by the vendored Node model. + """ + pytest.importorskip("jwt", reason="ai-features extras not installed") + from gns3server.agent.gns3_copilot.gns3_client.custom_gns3fy import NODE_TYPES + from gns3server.schemas.controller.nodes import NodeType + + missing = {e.value for e in NodeType} - set(NODE_TYPES) + assert not missing, f"NODE_TYPES drifted from NodeType, missing: {missing}" + + +def test_node_accepts_docker_exec_console(): + """ + Vendor NOS nodes use console_type "docker_exec"; the topology reader + validates the whole node list in one pass, so rejecting it poisoned + every copilot device tool for the project. + """ + pytest.importorskip("jwt", reason="ai-features extras not installed") + from gns3server.agent.gns3_copilot.gns3_client.custom_gns3fy import Node + + node = Node( + name="R1", + project_id="5f517ce3-1bc6-4245-b866-1a2fbd0ee5a7", + node_id="0d15c2e6-8f83-4b79-8875-9dbc3e5f2f1e", + node_type="docker", + console_type="docker_exec", + status="started", + ) + assert node.console_type == "docker_exec" diff --git a/tests/compute/docker/test_docker.py b/tests/compute/docker/test_docker.py index 0a28a273b..7fbb49c06 100644 --- a/tests/compute/docker/test_docker.py +++ b/tests/compute/docker/test_docker.py @@ -364,3 +364,85 @@ async def test_install_busybox_no_executables(): dst_dir = Docker.resources_path() await Docker.install_busybox(dst_dir) assert str(e.value) == "No busybox executable could be found, please install busybox (apt install busybox-static on Debian/Ubuntu) and make sure it is in your PATH" + + +@pytest.mark.asyncio +async def test_check_host_readiness_warns_when_low(caplog): + + import logging + from io import StringIO + + docker = Docker() + files = { + "/proc/sys/fs/inotify/max_user_instances": "128", + "/proc/sys/fs/inotify/max_user_watches": "8192", + "/proc/sys/fs/file-max": "100000", + "/proc/filesystems": "nodev ext4\nnodev tmpfs\n", + } + + def fake_open(path, *args, **kwargs): + return StringIO(files[path]) + + with patch("builtins.open", side_effect=fake_open): + with caplog.at_level(logging.WARNING, logger="gns3server.compute.docker"): + docker._check_host_readiness() + + message = " ".join(r.message for r in caplog.records) + assert "max_user_instances=128" in message + assert "sudo sysctl -w" in message + assert "modprobe fuse" in message # FUSE missing -> warned + + +@pytest.mark.asyncio +async def test_check_host_readiness_silent_when_ok(caplog): + + import logging + from io import StringIO + + docker = Docker() + files = { + "/proc/sys/fs/inotify/max_user_instances": "64000", + "/proc/sys/fs/inotify/max_user_watches": "524288", + "/proc/sys/fs/file-max": "1000000", + "/proc/filesystems": "nodev ext4\nnodev fuse\n", + } + + def fake_open(path, *args, **kwargs): + return StringIO(files[path]) + + with patch("builtins.open", side_effect=fake_open): + with caplog.at_level(logging.WARNING, logger="gns3server.compute.docker"): + docker._check_host_readiness() + + assert not [r for r in caplog.records if r.levelno == logging.WARNING] + + +@pytest.mark.asyncio +async def test_check_host_readiness_continues_past_unreadable_key(caplog): + """ + One unreadable /proc/sys key must not discard the warnings already + collected nor skip the FUSE check (that was a mid-loop return). + """ + + import logging + from io import StringIO + + docker = Docker() + files = { + "/proc/sys/fs/inotify/max_user_instances": "128", # low -> must warn + # max_user_watches and fs.file-max: unreadable -> skipped + "/proc/filesystems": "nodev ext4\n", # no fuse -> must warn + } + + def fake_open(path, *args, **kwargs): + if path not in files: + raise OSError("masked") + return StringIO(files[path]) + + with patch("builtins.open", side_effect=fake_open): + with caplog.at_level(logging.WARNING, logger="gns3server.compute.docker"): + docker._check_host_readiness() + + message = " ".join(r.message for r in caplog.records) + assert "max_user_instances=128" in message # collected before the gap + assert "modprobe fuse" in message # FUSE check still ran diff --git a/tests/compute/docker/test_docker_vm.py b/tests/compute/docker/test_docker_vm.py index c9e701306..70d72b3bf 100644 --- a/tests/compute/docker/test_docker_vm.py +++ b/tests/compute/docker/test_docker_vm.py @@ -69,6 +69,7 @@ def test_json(vm, compute_project): 'console_http_path': '/', 'extra_hosts': None, 'extra_volumes': [], + 'extra_configs': [], 'memory': 0, 'cpus': 0, 'aux': vm.aux, @@ -271,6 +272,105 @@ async def test_create_with_extra_hosts(compute_project, manager): assert "GNS3_EXTRA_HOSTS=199.199.199.1\ttest\n199.199.199.1\ttest2" in called_kwargs["data"]["Env"] assert vm._extra_hosts == extra_hosts + +@pytest.mark.asyncio +async def test_create_applies_env_host_config(compute_project, manager): + """ + GNS3_SHM_SIZE / GNS3_DEVICES are applied as native Docker HostConfig keys + (ShmSize, Devices) at create time -- not forwarded as container env vars -- + so they work even for vendor nodes that skip init.sh. Other GNS3_-prefixed + vars stay dropped from the container environment. + """ + + environment = ( + "GNS3_SHM_SIZE=1024\n" + "GNS3_DEVICES=/dev/fuse\n" + "GNS3_EVIL=should-be-dropped\n" # GNS3_ -> never forwarded as env + "FOO=bar" # normal var -> forwarded + ) + response = {"Id": "e90e34656806", "Warnings": []} + + with asyncio_patch("gns3server.compute.docker.Docker.list_images", return_value=[{"image": "ubuntu"}]): + with asyncio_patch("gns3server.compute.docker.Docker.query", return_value=response) as mock: + vm = DockerVM("test", str(uuid.uuid4()), compute_project, manager, "ubuntu", environment=environment) + await vm.create() + data = mock.call_args[1]["data"] + host_config = data["HostConfig"] + assert host_config["ShmSize"] == 1024 * 1024 * 1024 + assert host_config["Devices"] == [ + {"PathOnHost": "/dev/fuse", "PathInContainer": "/dev/fuse", "CgroupPermissions": "rwm"} + ] + env = data["Env"] + assert "FOO=bar" in env + assert not any( + e.startswith(("GNS3_SHM_SIZE=", "GNS3_DEVICES=", "GNS3_EVIL=")) + for e in env + ), "GNS3_ user vars must not leak into the container environment" + + +@pytest.mark.asyncio +async def test_create_masks_systemd_units(compute_project, manager): + """ + GNS3_MASK_UDEV=1 binds /dev/null over the udev units, and GNS3_MASK_SYSTEMD + does the same for arbitrary units -- stopping a privileged systemd container + from udev-coldplugging host devices. + """ + + environment = "GNS3_MASK_UDEV=1\nGNS3_MASK_SYSTEMD=foo.service,bar.socket" + response = {"Id": "e90e34656806", "Warnings": []} + + with asyncio_patch("gns3server.compute.docker.Docker.list_images", return_value=[{"image": "ubuntu"}]): + with asyncio_patch("gns3server.compute.docker.Docker.query", return_value=response) as mock: + vm = DockerVM("test", str(uuid.uuid4()), compute_project, manager, "ubuntu", environment=environment) + await vm.create() + masked = {m["Target"] for m in mock.call_args[1]["data"]["HostConfig"]["Mounts"] + if m.get("Source") == "/dev/null"} + for unit in DockerVM._UDEV_UNITS: + assert f"/etc/systemd/system/{unit}" in masked + for path in DockerVM._UDEVADM_PATHS: + assert path in masked + assert "/etc/systemd/system/foo.service" in masked + assert "/etc/systemd/system/bar.socket" in masked + + +@pytest.mark.asyncio +async def test_create_with_extra_configs(compute_project, manager): + """ + extra_configs entries are written to the node working directory and + bind-mounted read-only at their target path inside the container. + """ + + extra_configs = [{"target": "/firstboot.cfg", "content": "username clab\n!\nend"}] + response = {"Id": "e90e34656806", "Warnings": []} + + with asyncio_patch("gns3server.compute.docker.Docker.list_images", return_value=[{"image": "ubuntu"}]): + with asyncio_patch("gns3server.compute.docker.Docker.query", return_value=response) as mock: + vm = DockerVM("test", str(uuid.uuid4()), compute_project, manager, "ubuntu", extra_configs=extra_configs) + await vm.create() + mounts = mock.call_args[1]["data"]["HostConfig"]["Mounts"] + injected = [m for m in mounts if m.get("Target") == "/firstboot.cfg"] + assert len(injected) == 1 + assert injected[0]["ReadOnly"] is True + with open(injected[0]["Source"]) as f: + assert f.read() == "username clab\n!\nend" + + +@pytest.mark.asyncio +async def test_create_with_extra_configs_invalid_target(compute_project, manager): + """ + An extra_configs target that is not absolute (or contains '..') is rejected. + """ + + extra_configs = [{"target": "relative/path", "content": "x"}] + response = {"Id": "e90e34656806", "Warnings": []} + + with asyncio_patch("gns3server.compute.docker.Docker.list_images", return_value=[{"image": "ubuntu"}]): + with asyncio_patch("gns3server.compute.docker.Docker.query", return_value=response): + vm = DockerVM("test", str(uuid.uuid4()), compute_project, manager, "ubuntu", extra_configs=extra_configs) + with pytest.raises(DockerError): + await vm.create() + + @pytest.mark.asyncio async def test_create_with_colon_in_project_name(compute_project, manager): @@ -1898,3 +1998,103 @@ async def test_stop_exited_container_no_stop_query(vm): for call in mock_query.mock_calls ) assert vm.status == "stopped" + + +@pytest.mark.asyncio +async def test_create_dedups_overlapping_mount_targets(compute_project, manager): + """ + GNS3_MASK_UDEV overlapping a GNS3_MASK_SYSTEMD entry (or a unit named + twice) must not produce duplicate bind targets — Docker rejects the + create outright with "Duplicate mount point". + """ + + environment = "GNS3_MASK_UDEV=1\nGNS3_MASK_SYSTEMD=systemd-udevd.service,foo.service,foo.service" + response = {"Id": "e90e34656806", "Warnings": []} + + with asyncio_patch("gns3server.compute.docker.Docker.list_images", return_value=[{"image": "ubuntu"}]): + with asyncio_patch("gns3server.compute.docker.Docker.query", return_value=response) as mock: + vm = DockerVM("test", str(uuid.uuid4()), compute_project, manager, "ubuntu", environment=environment) + await vm.create() + mounts = mock.call_args[1]["data"]["HostConfig"]["Mounts"] + targets = [m["Target"] for m in mounts] + assert len(targets) == len(set(targets)), "duplicate bind targets in Mounts" + # the overlapping unit is present (masked) exactly once + assert targets.count("/etc/systemd/system/systemd-udevd.service") == 1 + assert targets.count("/etc/systemd/system/foo.service") == 1 + + +@pytest.mark.asyncio +async def test_create_env_trailing_comma_still_parsed(compute_project, manager): + """ + A trailing comma (environment composed from comma-separated lists) must + not silently disable the knobs — the base parser strips it like the + vendor parser does. + """ + + environment = "GNS3_MASK_UDEV=1,\nGNS3_SHM_SIZE=256," + response = {"Id": "e90e34656806", "Warnings": []} + + with asyncio_patch("gns3server.compute.docker.Docker.list_images", return_value=[{"image": "ubuntu"}]): + with asyncio_patch("gns3server.compute.docker.Docker.query", return_value=response) as mock: + vm = DockerVM("test", str(uuid.uuid4()), compute_project, manager, "ubuntu", environment=environment) + await vm.create() + host_config = mock.call_args[1]["data"]["HostConfig"] + assert host_config["ShmSize"] == 256 * 1024 * 1024 + masked = {m["Target"] for m in host_config["Mounts"] if m.get("Source") == "/dev/null"} + assert "/etc/systemd/system/systemd-udevd.service" in masked + + +@pytest.mark.asyncio +async def test_create_with_extra_configs_directory_target_rejected(compute_project, manager): + """ + Directory-form targets ('/', '/etc/', '///') would make the content write + fail with IsADirectoryError (a raw 500) — they must be rejected as + DockerError at create time. + """ + + response = {"Id": "e90e34656806", "Warnings": []} + for bad in ("/", "/etc/", "///"): + with asyncio_patch("gns3server.compute.docker.Docker.list_images", return_value=[{"image": "ubuntu"}]): + with asyncio_patch("gns3server.compute.docker.Docker.query", return_value=response): + vm = DockerVM("test", str(uuid.uuid4()), compute_project, manager, "ubuntu", + extra_configs=[{"target": bad, "content": "x"}]) + with pytest.raises(DockerError): + await vm.create() + + +def test_extra_config_schema_rejects_bad_targets(): + """ + The pydantic model rejects bad targets at template-save time (a 422) + instead of at node-create time (after a potentially multi-GB image pull). + """ + + from pydantic import ValidationError + from gns3server.schemas.common import ExtraConfig + + for bad in ("relative/path", "/has/../dots", "/", "/etc/", "no-leading-slash"): + with pytest.raises(ValidationError): + ExtraConfig(target=bad, content="x") + ok = ExtraConfig(target="/firstboot.cfg", content="x") + assert ok.target == "/firstboot.cfg" + + +@pytest.mark.asyncio +async def test_create_warns_when_extra_config_under_volume(compute_project, manager, caplog): + """ + An extra_configs target beneath a persisted volume is covered by the + volume bind at start — the injection would silently not take effect, so + warn at create time. + """ + + import logging + extra_configs = [{"target": "/xr-storage/config/foo.cfg", "content": "x"}] + response = {"Id": "e90e34656806", "Warnings": []} + + with asyncio_patch("gns3server.compute.docker.Docker.list_images", return_value=[{"image": "ubuntu"}]): + with asyncio_patch("gns3server.compute.docker.Docker.query", return_value=response): + vm = DockerVM("test", str(uuid.uuid4()), compute_project, manager, "ubuntu", + extra_configs=extra_configs, extra_volumes=["/xr-storage"]) + with caplog.at_level(logging.WARNING, logger="gns3server.compute.docker.docker_vm"): + await vm.create() + + assert any("shadowed by persisted volume" in r.message for r in caplog.records) diff --git a/tests/compute/docker/test_vendor_docker_vm.py b/tests/compute/docker/test_vendor_docker_vm.py index ce81e5119..3619d0b10 100644 --- a/tests/compute/docker/test_vendor_docker_vm.py +++ b/tests/compute/docker/test_vendor_docker_vm.py @@ -612,3 +612,127 @@ async def test_create_exec_cmd_has_no_while_true(compute_project, manager): assert captured["data"]["User"] == "root" assert captured["data"]["Tty"] is True assert "TERM=xterm" in captured["data"]["Env"] + + +# --------------------------------------------------------------------------- +# Container termination (graceful stop for vendor NOS) +# --------------------------------------------------------------------------- + +@pytest.mark.asyncio +async def test_terminate_container_graceful_stop(compute_project, manager): + """With graceful=True (explicit user stop) vendor containers are SIGTERMed + with a grace period, not SIGKILLed on the spot: systemd NOS images + (e.g. Cisco XRd) require a graceful shutdown, and Docker itself SIGKILLs + the container once the grace period expires.""" + + vm = _make_vm(compute_project, manager) + manager.query = AsyncioMagicMock() + manager.http_query = AsyncioMagicMock(return_value=MagicMock()) + + await vm._terminate_container(graceful=True) + + manager.http_query.assert_called_once_with( + "POST", "containers/e90e34656842/stop", params={"t": 60}, timeout=90) + manager.query.assert_not_called() # no kill on the graceful path + + +@pytest.mark.asyncio +async def test_terminate_container_default_is_kill(compute_project, manager): + """Without graceful (delete/update/close/crash cleanup) the vendor + container gets the base immediate kill — those paths force-delete or + recreate the container right after anyway.""" + + vm = _make_vm(compute_project, manager) + manager.query = AsyncioMagicMock() + manager.http_query = AsyncioMagicMock(return_value=MagicMock()) + + await vm._terminate_container() + + manager.query.assert_called_once_with("POST", "containers/e90e34656842/kill") + manager.http_query.assert_not_called() + + +@pytest.mark.asyncio +async def test_terminate_container_already_stopped_is_silent(compute_project, manager): + """Docker answers 304 when the container is already stopped — that is not + an error for the stop path.""" + + from gns3server.compute.docker.docker_error import DockerHttp304Error + + vm = _make_vm(compute_project, manager) + manager.http_query = AsyncioMagicMock( + side_effect=DockerHttp304Error("Docker has returned an error: 304")) + await vm._terminate_container(graceful=True) # must not raise + + +@pytest.mark.asyncio +async def test_stop_uses_graceful_termination(compute_project, manager): + """The full stop() path must route through _terminate_container (the + vendor override); the default is the fast kill — only the explicit user + stop route passes graceful=True.""" + + vm = _make_vm(compute_project, manager) + with patch.object(DockerVM, "_clean_servers", new=AsyncioMagicMock()): + with patch.object(DockerVM, "_stop_ubridge", new=AsyncioMagicMock()): + with patch.object( + DockerVM, "_get_container_state", new=AsyncioMagicMock(return_value="running") + ): + vm._permissions_fixed = True + with patch.object( + VendorDockerVM, "_terminate_container", new=AsyncioMagicMock() + ) as mock_term: + await vm.stop() + mock_term.assert_called_once_with(graceful=False) + + +def test_env_stop_timeout(compute_project, manager): + vm = _make_vm(compute_project, manager, environment="GNS3_STOP_TIMEOUT=120") + assert vm._stop_timeout == 120 + + +def test_env_stop_timeout_default_60(compute_project, manager): + vm = _make_vm(compute_project, manager, environment="GNS3_SKIP_INIT=1") + assert vm._stop_timeout == 60 + + +def test_env_stop_timeout_invalid_keeps_default(compute_project, manager): + vm = _make_vm(compute_project, manager, environment="GNS3_STOP_TIMEOUT=abc") + assert vm._stop_timeout == 60 + vm = _make_vm(compute_project, manager, environment="GNS3_STOP_TIMEOUT=9999") + assert vm._stop_timeout == 60 + # ceiling: controller stop budget (240 s) minus the +30 s HTTP margin + vm = _make_vm(compute_project, manager, environment="GNS3_STOP_TIMEOUT=211") + assert vm._stop_timeout == 60 + vm = _make_vm(compute_project, manager, environment="GNS3_STOP_TIMEOUT=210") + assert vm._stop_timeout == 210 + + +@pytest.mark.asyncio +async def test_terminate_container_uses_env_timeout(compute_project, manager): + vm = _make_vm(compute_project, manager, environment="GNS3_STOP_TIMEOUT=120") + manager.http_query = AsyncioMagicMock(return_value=MagicMock()) + + await vm._terminate_container(graceful=True) + + manager.http_query.assert_called_once_with( + "POST", "containers/e90e34656842/stop", params={"t": 120}, timeout=150) + + +@pytest.mark.asyncio +async def test_create_reparse_refreshes_env_knobs(compute_project, manager): + """A PUT to the node's environment must take effect on the next create(), + not on the next project reload: create() re-parses the vendor knobs.""" + + response = _create_response(None, entrypoint=["/init"]) + vm = _make_vm(compute_project, manager, + environment="GNS3_SKIP_INIT=1\nGNS3_STOP_TIMEOUT=120") + assert vm._gns3_init is False and vm._stop_timeout == 120 + + vm._environment = "GNS3_STOP_TIMEOUT=5" # knob removed + value changed + with asyncio_patch("gns3server.compute.docker.Docker.list_images", + return_value=[{"image": "srlinux"}]): + with asyncio_patch("gns3server.compute.docker.Docker.query", return_value=response): + await vm.create() + + assert vm._stop_timeout == 5 + assert vm._gns3_init is True # removed entry reset to default diff --git a/tests/compute/test_port_manager.py b/tests/compute/test_port_manager.py index b12af3206..ede2cea7a 100644 --- a/tests/compute/test_port_manager.py +++ b/tests/compute/test_port_manager.py @@ -16,6 +16,7 @@ # along with this program. If not, see . import pytest +import threading import uuid from fastapi import HTTPException @@ -116,6 +117,82 @@ def test_release_udp_port(): pm.reserve_udp_port(20000, project) +def test_concurrent_udp_port_allocation_no_duplicates(): + """ + Regression test for the link UDP self-loop bug (docs/bugs/link-udp-self-loop.md): + both ends of a link are allocated concurrently on the controller + (asyncio.gather -> two POST /ports/udp), and FastAPI runs the sync route + handler in a threadpool. The find-then-add allocation must be atomic, + otherwise both threads can probe and return the same "free" port — + handing lport == rport to both ends, which makes every packet loop back + to its sender (one-way link). + """ + + pm = PortManager() + pm.udp_port_range = (50000, 50100) + project = Project(project_id=str(uuid.uuid4())) + + workers = 8 + rounds = 10 + barrier = threading.Barrier(workers) + results = [] + results_lock = threading.Lock() + + def worker(): + allocated = [] + for _ in range(rounds): + # start each round together to maximize the collision window + barrier.wait() + allocated.append(pm.get_free_udp_port(project)) + with results_lock: + results.extend(allocated) + + threads = [threading.Thread(target=worker) for _ in range(workers)] + for thread in threads: + thread.start() + for thread in threads: + thread.join() + + assert len(results) == workers * rounds + assert len(set(results)) == len(results), "the same UDP port was handed to two callers" + assert pm.udp_ports == set(results) + + +def test_concurrent_tcp_port_allocation_no_duplicates(): + """ + Same race class as the UDP self-loop bug, on the console/TCP side: + concurrent get_free_tcp_port calls must never return the same port. + """ + + pm = PortManager() + pm.console_port_range = (51000, 51100) + project = Project(project_id=str(uuid.uuid4())) + + workers = 8 + rounds = 10 + barrier = threading.Barrier(workers) + results = [] + results_lock = threading.Lock() + + def worker(): + allocated = [] + for _ in range(rounds): + barrier.wait() + allocated.append(pm.get_free_tcp_port(project)) + with results_lock: + results.extend(allocated) + + threads = [threading.Thread(target=worker) for _ in range(workers)] + for thread in threads: + thread.start() + for thread in threads: + thread.join() + + assert len(results) == workers * rounds + assert len(set(results)) == len(results), "the same TCP port was handed to two callers" + assert pm.tcp_ports == set(results) + + def test_find_unused_port(): p = PortManager().find_unused_port(1000, 10000) diff --git a/tests/controller/test_udp_link.py b/tests/controller/test_udp_link.py index e322627a0..b17120757 100644 --- a/tests/controller/test_udp_link.py +++ b/tests/controller/test_udp_link.py @@ -187,6 +187,78 @@ async def test_delete(project): compute2.delete.assert_any_call("/projects/{}/vpcs/nodes/{}/adapters/3/ports/1/nio".format(project.id, node2.id), timeout=120) +@pytest.mark.asyncio +async def test_reset(project): + """ + reset() re-creates the link on the same object: the fresh port pair must + replace the stale one instead of accumulating (the committed NIOs always + come from indices 0/1) — see docs/bugs/link-udp-self-loop.md. + """ + + compute1 = MagicMock() + compute2 = MagicMock() + + node1 = Node(project, compute1, "node1", node_type="vpcs") + node1._ports = [EthernetPort("E0", 0, 0, 4)] + node2 = Node(project, compute2, "node2", node_type="vpcs") + node2._ports = [EthernetPort("E0", 0, 3, 1)] + + async def subnet_callback(compute2): + """ + Fake subnet callback + """ + return ("192.168.1.1", "192.168.1.2") + + compute1.get_ip_on_same_subnet.side_effect = subnet_callback + + # per-compute port sequences: first create -> 1024/2048, reset -> 4096/8192 + node1_ports = iter([1024, 4096]) + node2_ports = iter([2048, 8192]) + + async def compute1_callback(path, data={}, **kwargs): + if "/ports/udp" in path: + response = MagicMock() + response.json = {"udp_port": next(node1_ports)} + return response + + async def compute2_callback(path, data={}, **kwargs): + if "/ports/udp" in path: + response = MagicMock() + response.json = {"udp_port": next(node2_ports)} + return response + + compute1.post.side_effect = compute1_callback + compute1.host = "example.com" + compute2.post.side_effect = compute2_callback + compute2.host = "example.org" + + link = UDPLink(project) + await link.add_node(node1, 0, 4) + await link.add_node(node2, 3, 1) + + await link.reset() + + # exactly one (fresh) NIO spec per side — no stale entries left behind + assert len(link.debug_link_data) == 2 + assert link.debug_link_data[0]["lport"] == 4096 + assert link.debug_link_data[0]["rport"] == 8192 + assert link.debug_link_data[1]["lport"] == 8192 + assert link.debug_link_data[1]["rport"] == 4096 + # the self-loop invariant: an end's lport must never equal its rport + assert link.debug_link_data[0]["lport"] != link.debug_link_data[0]["rport"] + assert link.debug_link_data[1]["lport"] != link.debug_link_data[1]["rport"] + # the committed NIO carries the fresh pair, not the released one + compute1.post.assert_any_call("/projects/{}/vpcs/nodes/{}/adapters/0/ports/4/nio".format(project.id, node1.id), data={ + "lport": 4096, + "rhost": "192.168.1.2", + "rport": 8192, + "type": "nio_udp", + "filters": {}, + "markers": {}, + "suspend": False, + }, timeout=120) + + @pytest.mark.asyncio async def test_choose_capture_side(project): """