docker: graceful stop for vendor NOS containers (SIGTERM + 60s grace)

DockerVM.stop() terminated containers with an immediate SIGKILL — fine
for init.sh-based containers whose state is persisted beforehand, but a
systemd NOS (Cisco XRd, SR Linux) needs a graceful shutdown and treats
the abrupt kill as an unclean shutdown (exit 137 on every stop).

Extract the final termination into _terminate_container() and override
it in VendorDockerVM: POST /containers/{id}/stop?t=60 sends SIGTERM and
waits for systemd to stop services; Docker itself SIGKILLs the
container once the grace period expires, so no fallback is needed.
Docker's 304 (already stopped) is swallowed.

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/<unit>` | `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:<ver>` — 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: <XR CLI first-boot config>}` |
 
 ```
@@ -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. |

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.

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:

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()