diff --git a/docs/features/vendor-nos-xrd.md b/docs/features/vendor-nos-xrd.md index 68b7a21e3..41e08a7f8 100644 --- a/docs/features/vendor-nos-xrd.md +++ b/docs/features/vendor-nos-xrd.md @@ -34,7 +34,7 @@ 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-shadow"] + XV["extra_volumes: /xr-storage + /xr-storage-shadow"] end subgraph Server["gns3-server (generic mechanisms)"] CREATE["DockerVM.create() HostConfig"] @@ -47,7 +47,7 @@ graph TB subgraph Container["XRd container"] SYSTEMD["systemd (/usr/sbin/init)"] XR["XR control plane"] - XRS["/xr-storage-shadow (persisted)"] + XRS["/xr-storage (persisted, live data layer)"] end ENV --> CREATE --> SYSTEMD ENV --> MASK & HOSTCFG @@ -65,6 +65,7 @@ graph TB | 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 | automatic for vendor containers (`docker_exec`) | stop sends SIGTERM and waits up to 60 s (Docker SIGKILLs after the grace period) instead of the base class' immediate kill — systemd NOS images require a graceful shutdown | `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 @@ -93,7 +94,7 @@ Note: "journal corrupted" messages with varying machine-IDs come from the |-------|-------| | `image` | official `ios-xr/xrd-control-plane:` — no wrapper image needed | | `console_type` | `docker_exec` | -| `extra_volumes` | `["/xr-storage-shadow"]` | +| `extra_volumes` | `["/xr-storage", "/xr-storage-shadow"]` | | `extra_configs` | `{target: /firstboot.cfg, content: }` | ``` @@ -114,14 +115,22 @@ XRd-specific gotchas (image-side, not GNS3): 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. -- `/xr-storage` is a symlink layer; the real data directory is - **`/xr-storage-shadow`** (`config/` `disk1/` `scratch/` `log/` — all - `/disk0:`, `/harddisk:`, `/var/xr/*` paths converge there). Persisting - `/xr-storage` instead copies symlinks and loses data. -- `XR_FIRST_BOOT_CONFIG` only applies when `/xr-storage-shadow/config` is - empty (first boot). To re-seed, delete and recreate the node. +- **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 @@ -157,7 +166,7 @@ sequenceDiagram | 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 be `/xr-storage-shadow` | +| 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 | @@ -191,6 +200,7 @@ sequenceDiagram | Version | Date | Changes | |---------|------|---------| +| 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 + 60 s grace) 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/compute/docker/docker_vm.py b/gns3server/compute/docker/docker_vm.py index 8f3961a35..5e48a5e6c 100644 --- a/gns3server/compute/docker/docker_vm.py +++ b/gns3server/compute/docker/docker_vm.py @@ -1206,12 +1206,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() log.debug(f"Docker container '{self._name}' [{self._image}] stopped") except DockerHttp409Error: # Container is already stopped @@ -1222,6 +1218,18 @@ class DockerVM(BaseNode): return self.status = "stopped" + async def _terminate_container(self): + """ + 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 (see VendorDockerVM). + """ + + 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 e4006f683..e7b155f34 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__) @@ -138,6 +138,20 @@ class VendorDockerVM(DockerVM): pass self._console_exec_writer = None + async def _terminate_container(self): + """ + Override: vendor NOS containers run systemd and require a graceful + shutdown (e.g. Cisco XRd treats an abrupt SIGKILL as an unclean + shutdown). Send SIGTERM and wait up to 60 s for the services to stop; + Docker SIGKILLs the container itself once the grace period expires, + so no fallback kill is needed. The blocking stop call sits well + inside the manager's default 300 s query timeout. + """ + try: + await self.manager.query("POST", f"containers/{self._cid}/stop", params={"t": 60}) + except DockerHttp304Error: + pass # already stopped + async def start(self): await super().start() if self.status == "started" and not self._gns3_init: diff --git a/tests/compute/docker/test_vendor_docker_vm.py b/tests/compute/docker/test_vendor_docker_vm.py index ce81e5119..1fbbbcc1b 100644 --- a/tests/compute/docker/test_vendor_docker_vm.py +++ b/tests/compute/docker/test_vendor_docker_vm.py @@ -612,3 +612,58 @@ 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): + """Vendor containers must be 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() + + await vm._terminate_container() + + manager.query.assert_called_once_with("POST", "containers/e90e34656842/stop", params={"t": 60}) + + +@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.query = AsyncioMagicMock( + side_effect=DockerHttp304Error("Docker has returned an error: 304")) + await vm._terminate_container() # 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), not the base class' immediate kill.""" + + 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") + ): + with patch.object( + VendorDockerVM, "_fix_permissions", new=AsyncioMagicMock() + ) as mock_perms: + mock_perms.return_value = None + vm._permissions_fixed = True + with patch.object( + VendorDockerVM, "_terminate_container", new=AsyncioMagicMock() + ) as mock_term: + await vm.stop() + mock_term.assert_called_once()