diff --git a/docs/features/docker-exec-console.md b/docs/features/docker-exec-console.md index 332e22af4..41a3a641b 100644 --- a/docs/features/docker-exec-console.md +++ b/docs/features/docker-exec-console.md @@ -322,41 +322,46 @@ This is the one place where skipping init.sh changes behaviour beyond boot: **nothing writes through to the host** — the container writes to its overlay filesystem and the data is lost on stop. -The bridge (see init.sh lines 35–52) has two parts: +init.sh (as the entrypoint) is safe because it runs **before** the +application: for each volume it seeds the host directory with the image's +original files on first start, then `mount --bind /gns3volumes ` +bridges persistent storage into place. + +`VendorDockerVM` cannot use that position (the NOS must own its entrypoint), +so the same persistence is established entirely **outside the container and +before it exists**: ``` -host ──Docker bind mount──▶ /gns3volumes/etc/opt/srlinux (always mounted) - │ init.sh: mount --bind - ▼ - /etc/opt/srlinux (where the NOS writes) +create() 之前: host dir seeded from the image (docker create + docker cp, first time only) +create() 时: host ──Docker bind mount──▶ /etc/opt/srlinux (direct, at the real path) +启动: NOS native entrypoint — the persisted config is visible from the first process ``` -`VendorDockerVM` replicates this for SKIP_INIT containers: +1. **`_prepare_volumes()`** — host-side, at `create()` time (after the image + is present, before the container is created). For each persistent volume + whose host directory lacks the `.gns3_perms` marker, a throwaway + `docker create` container (nothing executes) is used as a `docker cp -a` + source to seed the host directory with the image's original content. The + marker is written after the copy attempt — a volume that has it (every + node that ever started, on any GNS3 version) is **never re-seeded**, so + saved configuration is never overwritten with factory content. -1. **`_setup_skip_init_volumes()`** — runs once per start, right after the - container is up (`VendorDockerVM.start()`). For each persistent volume it - `docker exec`s a busybox script that: - - seeds the host directory with the container's original files on first - start (`cp -a` + `.gns3_perms` marker), exactly like init.sh; - - `mount --bind /gns3volumes ` to bridge persistent storage - back to the in-container path — on subsequent starts the persisted data - replaces the fresh overlay content; - - restores the permissions recorded in `.gns3_perms` at the previous stop - (best-effort). +2. **`_mount_binds()` override** — the volume binds target the **real + in-container paths** (`/etc/opt/srlinux`) instead of `/gns3volumes`. + With the content seeded first, the image's files are never shadowed by an + empty mount, and the NOS sees its persisted configuration from the very + first process — no post-start mount pass that could race the NOS reading + its startup config (see "History: the exec-bridge race" below). -2. **Container-side `_fix_permissions()` override targeting `/gns3volumes`** - — `DockerVM._fix_permissions` operates on the in-container paths - (`/etc/opt/srlinux`, …), which only resolve to persistent storage while - the `mount --bind` bridge is up; after a container restart the bridge is - gone and it would chown the overlay copy instead of the host files. It - also restarts an exited container just to chown. The override instead - runs the same busybox record/chmod/chown script **inside the container - (as root) on the `/gns3volumes` paths** — the Docker bind-mount - targets, which exist for the whole container lifetime and need no bridge. - A stopped/exited container is **not** restarted: the pass is skipped and - the next start fixes ownership. It runs at start (so the controller can - read project files while the node runs) and at stop (for files written - during runtime). +3. **Container-side `_fix_permissions()` override** — runs the same busybox + record/chmod/chown script **inside the container (as root) on the volume + paths**. Because the volumes are Docker bind mounts created with the + container, the in-container paths resolve to the host files for the whole + container lifetime. A stopped/exited container is **not** restarted (the + base class would, just to chown; vendor NOS images are heavy to boot): + the pass is skipped and the next start fixes ownership. It runs at start + (so the controller can read project files while the node runs) and at + stop (for files written during runtime). > The fix must run container-side: files written by the container are > host-side root-owned, and an unprivileged GNS3 process cannot chown them @@ -375,9 +380,10 @@ the base class just created. Without `GNS3_SKIP_INIT` the mount is kept | Phase | Normal Docker node | `VendorDockerVM` + `GNS3_SKIP_INIT` | |-------|--------------------|--------------------------------------| -| start | init.sh seeds + bind-mounts + restores perms (in-container, before the app starts) | `docker exec` after start: seed + bind-mount + restore perms; then container-side chown on `/gns3volumes` | -| stop | container-side `_fix_permissions` on in-container paths (restarts an exited container) | container-side `_fix_permissions` on `/gns3volumes` paths (skips dead containers, no restart) | -| volume config | identical `_mount_binds` (host → `/gns3volumes`) | identical | +| create | — | `_prepare_volumes()` seeds host dirs from the image (first create only); volumes bound **directly** at their real paths | +| start | init.sh seeds + bind-mounts + restores perms (in-container, before the app starts) | container-side `_fix_permissions` on the volume paths (skips dead containers, no restart) | +| stop | container-side `_fix_permissions` on in-container paths (restarts an exited container) | container-side `_fix_permissions` on the volume paths (skips dead containers, no restart) | +| volume config | `_mount_binds`: host → `/gns3volumes` | `_mount_binds` override: host → `` directly | ### Runtime ownership safety @@ -403,17 +409,28 @@ ever matters, drop the start-time pass and keep only the stop-time one (standard behaviour — the trade-off is mid-run `Permission denied` in the file browser, identical to regular Docker nodes). -### Boot-ordering caveat +### History: the exec-bridge race (fixed) -The volume bridge (`mount --bind`) is established **after** the vendor -entrypoint has started (there is no init.sh to do it before), so the NOS's -early boot reads the overlay copy of the volume paths — default image -content, not the persisted data. Whether the persisted config takes effect -depends on the NOS re-reading those files after the bridge is up (SR Linux's -daemons do re-read/write their managed files during boot, as observed). -Always verify the closed loop when adopting a new image: `save` a config → -stop the node → start it → confirm the config is actually applied, not just -present on the host. +The first SKIP_INIT implementation replicated init.sh's script **via +`docker exec` after the container started** instead of binding directly at +create time. That copied the mechanism but not the invariant that makes +init.sh safe — the entrypoint position, which guarantees the volume is in +place *before* the application runs. An exec-based bind runs **concurrently** +with the NOS boot, so whether the NOS reads its persisted config or the +overlay's factory copy was a timing race: + +- a single node stop/start on an idle system won it (the exec landed ~1 s + in, SR Linux reads its startup config at ~2–4 s) — which is why the + round-trip "save → stop → start → config still there" passed; +- a server restart + project reload lost it (all nodes start concurrently, + the Docker API queue delays the execs by several seconds) — SR Linux + booted factory while the persisted `config.json` sat intact on the host; +- XRd was immune either way (systemd boots for tens of seconds before any + XR process touches `/xr-storage`), which is why the race was never seen + on it. + +The direct-bind-at-create design removes the window entirely; there is no +ordering requirement left to verify when adopting a new NOS image. ## Troubleshooting @@ -472,17 +489,17 @@ present on the host. root-owned files at runtime. **9. Persistent volume empty on the host after `save` + stop** -- Ensure `GNS3_SKIP_INIT=1` is set (so the host-side bridge path is taken) and - the volume path is in `extra_volumes`; check the compute log for - `Volume '' bound to persistent storage`. +- Ensure `GNS3_SKIP_INIT=1` is set (so the direct-bind path is taken) and + the volume path is in `extra_volumes`; check that the host directory + carries the `.gns3_perms` marker (written at create-time seeding) and the + compute log for `Seeded persistent volume`. **10. Persisted config present on the host but not applied after restart** -- The volume bridge is established after the NOS has booted (see - *Boot-ordering caveat*); the NOS may have already loaded the overlay's - default config into memory. Verify with a visible change (hostname, - interface description): `save` → stop → start → check the change took - effect. If it does not, the image needs the bridge earlier (a - vendor-specific entrypoint wrapper, not covered by this prototype). +- On builds since the direct-bind rework this should not happen: the volume + is in place before the first process. If you see it, confirm the server + build includes the rework (older builds established the bind via a + post-start `docker exec` that could lose the race against the NOS reading + its startup config — see *History: the exec-bridge race*). **11. Web console flickers (full-screen clear/redraw) on every command** - The PTY is stuck at the tall 511×10000 default while a CPR-answering client @@ -516,17 +533,16 @@ present on the host. 4. **Rootful-Docker assumption** (`UsernsMode: host`, set for all GNS3 Docker nodes) so the container-side chown acts on the host files' real uid/gid (see the volume-persistence section). -5. **Post-boot volume bridge.** The bind-mount bridge is established after the - vendor entrypoint has started (init.sh would do it before). A NOS that - strictly requires its persisted files at its very first read may need a - different boot arrangement (see *Boot-ordering caveat*). +5. **Docker CLI dependency.** Volume seeding shells out to the `docker` + binary (`docker create` + `docker cp` + `docker rm`) at create time — + the same dependency the permission passes already have. ## References - `gns3server/compute/docker/vendor_docker_vm.py` — `VendorDockerVM`: `_start_docker_exec_console`, `_LazyExecTelnetServer`, - `_setup_skip_init_volumes`, container-side `_fix_permissions` on - `/gns3volumes`, `start()`. + `_prepare_volumes` (host-side seeding), direct volume binds in + `_mount_binds`, container-side `_fix_permissions`, `start()`. - `gns3server/compute/docker/docker_vm.py` — `DockerVM` extension hooks (`_prepare_init_and_interface_env`, `_start_console_server`, `_get_container_ifname`, `_cleanup_console_resources`). @@ -545,6 +561,7 @@ present on the host. | Version | Date | Changes | |---------|------|---------| +| 1.7 | 2026-08-22 | SKIP_INIT volume persistence rebuilt: host-side seeding at create time (`docker create` + `docker cp -a`, marker-gated so saved config is never overwritten) and direct bind mounts at the real in-container paths replace the post-start `docker exec` bridge. Root cause: the exec bridge raced the NOS reading its startup config — SR Linux read `config.json` at ~2–4 s and booted factory whenever concurrent node starts (server restart + project reload) delayed the exec past that point, while single-node stop/start and XRd (systemd touches `/xr-storage` tens of seconds in) never lost the race. New `_prepare_volumes` hook on `DockerVM`; `_fix_permissions` now targets the volume paths directly. | | 1.6 | 2026-08-20 | Terminal geometry and size forwarding: WS binary control frames `{"cols","rows"}` → NAWS / asyncssh resize (controller now forwards binary frames; compute intercepts them); tall 511×10000 default kept for non-NAWS clients, applied post-creation and restored on last disconnect (client size racing exec creation wins over the default); new `GNS3_CONSOLE_RESIZE=0` knob for paging CLIs (XRd) where a browser resize would break concurrent netmiko sessions on the shared exec; documented the SR Linux flicker root cause (tall rows × CPR-answering client → ~2.4× re-emitted output; rows-driven, width-independent). | | 1.5 | 2026-08-13 | Add appliance (`gns3a`) packaging section: 35-adapter full-chassis design, the three server-side schema fixes (DockerConsoleType, ApplianceV1_6.custom_adapters, extra_volumes passthrough), and the symbol-theme caveat (any `:/symbols/` symbol is rewritten to the category default at load). | | 1.4 | 2026-08-13 | Reconnect fix: drop the while-true wrapper (it restarted the CLI with no client to answer CPR → blank screen on reconnect); the exec is now recreated on connect when the upstream has died. `_LazyExecTelnetServer` extracted to module level and unit-tested. | diff --git a/docs/features/vendor-nos-xrd.md b/docs/features/vendor-nos-xrd.md index 6875cfb70..13ed3960f 100644 --- a/docs/features/vendor-nos-xrd.md +++ b/docs/features/vendor-nos-xrd.md @@ -41,7 +41,7 @@ graph TB MASK["GNS3_MASK_UDEV → /dev/null binds"] HOSTCFG["ShmSize / Devices"] CFGINJ["extra_configs → RO single-file bind"] - VBRIDGE["VendorDockerVM volume bridge"] + VBRIDGE["VendorDockerVM volume seeding + direct binds"] HOSTCHK["host-readiness check (read-only)"] end subgraph Container["XRd container"] @@ -158,11 +158,12 @@ sequenceDiagram 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) + S->>D: seed volume host dirs from image (docker create + cp, first time only) + S->>D: container create (ShmSize, Devices, /dev/null binds, firstboot.cfg RO bind, volumes bound directly at /xr-storage*) 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) + S->>X: docker exec permission fix (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) @@ -198,7 +199,7 @@ sequenceDiagram - `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 + seeding + direct binds, container-chown - `gns3server/compute/docker/__init__.py` — `_check_host_readiness()` - `gns3server/schemas/common.py` — `ExtraConfig` - `gns3server/db/models/templates.py` + `db_migrations/` — persistence @@ -211,6 +212,7 @@ sequenceDiagram | Version | Date | Changes | |---------|------|---------| +| 1.6 | 2026-08-21 | SKIP_INIT volume persistence rebuilt: host-side seeding at create time (`docker create` + `docker cp`) and direct bind mounts at the real in-container paths replace the post-start `docker exec` bridge, which raced the NOS reading its startup config (visible on SR Linux: factory boot after a server restart + project reload; XRd was immune only because systemd touches `/xr-storage` tens of seconds in). No behaviour change for XRd beyond the race removal. | | 1.5 | 2026-08-20 | Appliance env gains `GNS3_CONSOLE_RESIZE=0`: client-driven console resizes are ignored so the shared exec PTY stays at the tall no-paging geometry for concurrent netmiko/copilot sessions (browsers included). | | 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. | diff --git a/gns3server/compute/docker/docker_vm.py b/gns3server/compute/docker/docker_vm.py index 1fade0fd1..6a04f64d5 100644 --- a/gns3server/compute/docker/docker_vm.py +++ b/gns3server/compute/docker/docker_vm.py @@ -379,6 +379,51 @@ class DockerVM(BaseNode): result = await self.manager.query("GET", f"images/{self._image}/json") return result + def _persistent_volume_list(self, image_info, include_network_config=True): + """ + The in-container paths that get a persistent volume mount: GNS3's + /etc/network, every VOLUME declared by the image and the node's + extra_volumes. Overlapping paths are de-duplicated so that a path + covered by a more general volume is not mounted twice. + + :param include_network_config: include GNS3's hardcoded /etc/network + volume (consumed by init.sh; subclasses that skip init.sh pass + False so the list matches the mounts they actually create). + """ + + for volume in self._extra_volumes: + if not volume.strip() or volume[0] != "/" or volume.find("..") >= 0: + raise DockerError( + f"Persistent volume '{volume}' has invalid format. It must start with a '/' and not contain '..'." + ) + volumes = [] + if include_network_config: + volumes.append("/etc/network") + volumes.extend((image_info.get("Config", {}).get("Volumes") or {}).keys()) + volumes.extend(self._extra_volumes) + + deduped = [] + # define lambdas for validation checks + nf = lambda x: re.sub(r"//+", "/", (x if x.endswith("/") else x + "/")) + generalises = lambda v1, v2: nf(v2).startswith(nf(v1)) + for volume in volumes: + # remove any mount that is equal or more specific, then append this one + deduped = list(filter(lambda v: not generalises(volume, v), deduped)) + # if there is nothing more general, append this mount + if not [v for v in deduped if generalises(v, volume)]: + deduped.append(volume) + return deduped + + async def _prepare_volumes(self, image_info): + """ + Hook: prepare persistent volumes before the container (and its + mounts) are created. The default implementation does nothing — + init.sh performs the first-copy seeding inside the container at + boot. Subclasses that skip init.sh override this to seed the host + directories from the image instead, so their mounts can be bound + directly at the real in-container paths from the very first process. + """ + def _mount_binds(self, image_info): """ :returns: Return the path that we need to map to local folders @@ -402,26 +447,7 @@ class DockerVM(BaseNode): self._create_network_config() except OSError as e: raise DockerError(f"Could not create network config in the container: {e}") - volumes = ["/etc/network"] - - volumes.extend((image_info.get("Config", {}).get("Volumes") or {}).keys()) - for volume in self._extra_volumes: - if not volume.strip() or volume[0] != "/" or volume.find("..") >= 0: - raise DockerError( - f"Persistent volume '{volume}' has invalid format. It must start with a '/' and not contain '..'." - ) - volumes.extend(self._extra_volumes) - - self._volumes = [] - # define lambdas for validation checks - nf = lambda x: re.sub(r"//+", "/", (x if x.endswith("/") else x + "/")) - generalises = lambda v1, v2: nf(v2).startswith(nf(v1)) - for volume in volumes: - # remove any mount that is equal or more specific, then append this one - self._volumes = list(filter(lambda v: not generalises(volume, v), self._volumes)) - # if there is nothing more general, append this mount - if not [v for v in self._volumes if generalises(v, volume)]: - self._volumes.append(volume) + self._volumes = self._persistent_volume_list(image_info) for volume in self._volumes: source = os.path.join(self.working_dir, os.path.relpath(volume, "/")) @@ -544,6 +570,10 @@ class DockerVM(BaseNode): f"(max available is {available_cpus} CPUs)" ) + # Prepare persistent volume content before the container and its + # mounts are created (no-op for the init.sh path). + await self._prepare_volumes(image_infos) + params = { "Hostname": self._name, "Image": self._image, diff --git a/gns3server/compute/docker/vendor_docker_vm.py b/gns3server/compute/docker/vendor_docker_vm.py index 13e6e0566..6089663f2 100644 --- a/gns3server/compute/docker/vendor_docker_vm.py +++ b/gns3server/compute/docker/vendor_docker_vm.py @@ -48,9 +48,11 @@ class VendorDockerVM(DockerVM): (host-side only — GNS3_ entries are never forwarded into the container): * ``GNS3_SKIP_INIT=1`` — do not prepend /gns3/init.sh; the container runs - its own entrypoint (e.g. SR Linux's ``sr_linux``). Init.sh's volume - persistence (bind-mount /gns3volumes → target) is replicated via - ``docker exec`` after the container starts. + its own entrypoint (e.g. SR Linux's ``sr_linux``). Persistent volumes + are seeded host-side and bound directly at their real in-container + paths at create time (see ``_prepare_volumes`` / ``_mount_binds``), so + the NOS sees its saved configuration from the very first process — + no post-start mount pass that could race the NOS reading its config. * ``GNS3_INTERFACE_NAMES=mgmt0,e1-1,e1-2`` — rename injected interfaces (adapter order) instead of default ``eth{N}``. * ``GNS3_CONSOLE_CMD=/opt/srlinux/bin/sr_cli`` — command run inside the @@ -127,6 +129,17 @@ class VendorDockerVM(DockerVM): Removes the bind, drops the volume from self._volumes (so GNS3_VOLUMES and the vendor passes stay consistent) and deletes the host-side skeleton directory the base class just created. + + Additionally, the persistent volumes are bound directly at their + real in-container paths instead of /gns3volumes. With + init.sh skipped there is no in-container mount pass, so a volume + bound at /gns3volumes would only be moved into place by a post-start + ``docker exec`` — racing the NOS reading its startup configuration + (an SR Linux node booted factory whenever the exec lost that race, + e.g. on the concurrent node starts of a project reload). Binding at + the real path is safe because the content is seeded host-side before + the container is created (see _prepare_volumes): the image's files + are never shadowed by an empty mount. """ binds = super()._mount_binds(image_info) if self._gns3_init: @@ -136,7 +149,115 @@ class VendorDockerVM(DockerVM): shutil.rmtree(os.path.join(self.working_dir, "etc", "network"), ignore_errors=True) with contextlib.suppress(OSError): os.rmdir(os.path.join(self.working_dir, "etc")) - return binds + + # Re-target the volume binds from /gns3volumes to . + retargeted = [] + for bind in binds: + target = bind.get("Target", "") + if target.startswith("/gns3volumes"): + volume = target[len("/gns3volumes"):] + if volume in self._volumes: + bind = {**bind, "Target": volume} + retargeted.append(bind) + return retargeted + + async def _prepare_volumes(self, image_info): + """ + Override: for SKIP_INIT containers, seed every persistent volume's + host directory with the image's original content *before* the + container is created. This is the host-side replacement of init.sh's + first-copy: because the volume is then bound directly at its real + in-container path (see _mount_binds), the seed must exist first or + the NOS would boot with an empty config directory. + + ``.gns3_perms`` doubles as the seeded marker: a volume that has it + (every node that ever started, on any GNS3 version) is never + re-seeded — a re-seed would overwrite the node's saved + configuration with the factory image content. + """ + if self._gns3_init: + return + volumes = self._persistent_volume_list(image_info, include_network_config=False) + to_seed = [] + for volume in volumes: + host_dir = os.path.join(self.working_dir, os.path.relpath(volume, "/")) + os.makedirs(host_dir, exist_ok=True) + if not os.path.exists(os.path.join(host_dir, ".gns3_perms")): + to_seed.append((volume, host_dir)) + if not to_seed: + return + seed_cid = await self._create_seed_container() + try: + for volume, host_dir in to_seed: + await self._seed_volume_from_container(seed_cid, volume, host_dir) + # Write the marker only after the copy attempt, mirroring + # init.sh: a volume without it is (re)seeded on the next + # create(), so a partial seed self-heals. + open(os.path.join(host_dir, ".gns3_perms"), "a").close() + finally: + await self._remove_seed_container(seed_cid) + + async def _create_seed_container(self): + """ + A throwaway ``docker create`` container (nothing executes) used as + the copy source for seeding persistent volumes with the image's + original content. + """ + + try: + process = await asyncio.subprocess.create_subprocess_exec( + "docker", "create", self._image, + stdout=asyncio.subprocess.PIPE, stderr=asyncio.subprocess.PIPE, + ) + except OSError as e: + raise DockerError(f"Could not seed persistent volumes for '{self._name}': {e}") + stdout, stderr = await process.communicate() + if process.returncode != 0: + raise DockerError( + f"Could not create a seeding container for image '{self._image}': " + f"{stderr.decode(errors='replace').strip()}" + ) + return stdout.decode().strip() + + async def _seed_volume_from_container(self, seed_cid, volume, host_dir): + """ + Copy one volume's original content from the seeding container to its + host directory with ``docker cp -a`` (preserves modes/ownership; no + dependency on tools inside the image). + """ + + try: + process = await asyncio.subprocess.create_subprocess_exec( + "docker", "cp", "-a", f"{seed_cid}:{volume}/.", host_dir + "/", + stdout=asyncio.subprocess.PIPE, stderr=asyncio.subprocess.PIPE, + ) + except OSError as e: + raise DockerError(f"Could not seed persistent volume '{volume}' for '{self._name}': {e}") + _, stderr = await process.communicate() + if process.returncode != 0: + # A path the image does not contain (e.g. XRd's /xr-storage-shadow) + # is not an error: the volume starts empty. Same tolerance as + # init.sh's first copy (cp -a ... 2>/dev/null). + log.info( + "Persistent volume '%s' on '%s' not seedable from image '%s' (%s); starting empty", + volume, self._name, self._image, stderr.decode(errors="replace").strip(), + ) + return + log.info("Seeded persistent volume '%s' for '%s' from image '%s'", volume, self._name, self._image) + + async def _remove_seed_container(self, seed_cid): + """ + Best-effort removal of the seeding container. + """ + + try: + process = await asyncio.subprocess.create_subprocess_exec( + "docker", "rm", "-f", seed_cid, + stdout=asyncio.subprocess.PIPE, stderr=asyncio.subprocess.PIPE, + ) + except OSError: + return + await process.communicate() def _prepare_init_and_interface_env(self, params): """ @@ -209,23 +330,22 @@ class VendorDockerVM(DockerVM): async def start(self): await super().start() if self.status == "started" and not self._gns3_init: - await self._setup_skip_init_volumes() - # Fix host-side ownership of the seeded volume right away so the - # controller can read project files while the node runs. Reset the - # "fixed" flag afterwards: files written by the container during - # runtime still need the stop-time pass. + # Persistent volumes are seeded and bound directly at create time + # (see _prepare_volumes / _mount_binds), so there is no post-start + # bridge to run. Fix host-side ownership right away so the + # controller can read project files while the node runs, and reset + # the "fixed" flag: files written by the container during runtime + # still need the stop-time pass. await self._fix_permissions() self._permissions_fixed = False async def _fix_permissions(self): """ Container-side override of DockerVM._fix_permissions for vendor NOS - containers. It targets the Docker bind-mount paths - (`/gns3volumes`) directly instead of the in-container paths: - the in-container paths only resolve to persistent storage while the - `mount --bind` bridge from _setup_skip_init_volumes is up, and after a - container restart the bridge is gone — the base implementation would - then chown the overlay copy instead of the host files. + containers. The persistent volumes are Docker bind mounts created + with the container (see _mount_binds), so the in-container paths + resolve to the host-side files for the container's whole lifetime — + no /gns3volumes aliasing is needed. The busybox script runs inside the container as root (a host-side GNS3 process may be unprivileged and cannot chown root-owned files). @@ -248,7 +368,7 @@ class VendorDockerVM(DockerVM): uid, gid = os.getuid(), os.getgid() for volume in self._volumes: - target = f"/gns3volumes{volume}" + target = 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: @@ -284,61 +404,6 @@ class VendorDockerVM(DockerVM): else: self._permissions_fixed = True - async def _setup_skip_init_volumes(self): - """ - Replicate the volume-persistence portion of init.sh (lines 35–52) for - containers that skip init.sh (GNS3_SKIP_INIT=1). - - On first start the container's original files are seeded into the - persistent host directory; on subsequent starts the persisted data - is bind-mounted over the in-container path so writes land on the host. - Permission-changes recorded by _fix_permissions at the previous - stop are restored (best-effort). - """ - for volume in self._volumes: - vol_target = f"/gns3volumes{volume}" - # fmt: off - script = ( - f'mkdir -p "{volume}" && ' - f'if [ ! -f "{vol_target}/.gns3_perms" ]; then ' - f' /gns3/bin/busybox cp -a "{volume}/." "{vol_target}/" 2>/dev/null; ' - f' /gns3/bin/busybox touch "{vol_target}/.gns3_perms"; ' - f'fi && ' - 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; ' - # 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 - try: - process = await asyncio.subprocess.create_subprocess_exec( - "docker", - "exec", - self._cid, - "sh", - "-c", - script, - stdout=asyncio.subprocess.PIPE, - stderr=asyncio.subprocess.PIPE, - ) - stdout, stderr = await process.communicate() - if process.returncode != 0: - err = stderr.decode(errors="replace").strip() - log.warning( - "Volume setup for '%s' on container '%s' returned %d: %s", - volume, self._name, process.returncode, err, - ) - else: - log.info("Volume '%s' bound to persistent storage for '%s'", volume, self._name) - except OSError as e: - log.warning( - "Could not setup volume '%s' for container '%s': %s", volume, self._name, e - ) - async def _start_console_server(self): """ Override: add the ``docker_exec`` console type alongside the diff --git a/tests/compute/docker/test_vendor_docker_vm.py b/tests/compute/docker/test_vendor_docker_vm.py index e70bc6dd0..9f578c797 100644 --- a/tests/compute/docker/test_vendor_docker_vm.py +++ b/tests/compute/docker/test_vendor_docker_vm.py @@ -24,8 +24,10 @@ These tests cover: * init.sh prepend being skipped with GNS3_SKIP_INIT; * GNS3_INTERFACE_NAMES renaming injected interfaces (move_to_ns target); * the hardcoded /etc/network mount being dropped for SKIP_INIT containers; + * persistent volumes being seeded host-side and bound directly at their + real in-container paths (no post-start bridge racing the NOS boot); * the docker_exec console dispatch in start(); - * the SKIP_INIT volume bridge and container-side _fix_permissions passes. + * the container-side _fix_permissions passes. """ import uuid @@ -235,28 +237,36 @@ async def test_create_interface_names_sets_max_ethernet(compute_project, manager async def test_create_drops_etc_network_for_skip_init(compute_project, manager): response = _create_response(None, volumes={"/opt/srlinux/appmgr": None}) + seed_proc = MagicMock() + seed_proc.communicate = AsyncioMagicMock(return_value=(b"seedcid", b"")) + seed_proc.returncode = 0 with asyncio_patch("gns3server.compute.docker.Docker.list_images", return_value=[{"image": "srlinux"}]): with asyncio_patch("gns3server.compute.docker.Docker.query", return_value=response) as mock: - vm = VendorDockerVM("srlinux-1", str(uuid.uuid4()), compute_project, - manager, "srlinux:latest", - console_type="docker_exec", - environment="GNS3_SKIP_INIT=1", - extra_volumes=["/etc/opt/srlinux"]) - await vm.create() - sent = mock.call_args.kwargs["data"] - targets = [m["Target"] for m in sent["HostConfig"]["Mounts"]] - # /etc/network must NOT be mounted - assert "/gns3volumes/etc/network" not in targets - # but the declared volumes ARE mounted - assert "/gns3volumes/opt/srlinux/appmgr" in targets - assert "/gns3volumes/etc/opt/srlinux" in targets - # GNS3_VOLUMES env must also exclude /etc/network - vol_env = [v for v in sent["Env"] if v.startswith("GNS3_VOLUMES=")][0] - assert "/etc/network" not in vol_env - # host skeleton dir removed - assert not os.path.exists(os.path.join(vm.working_dir, "etc", "network")) + with patch("asyncio.subprocess.create_subprocess_exec", + return_value=seed_proc): + vm = VendorDockerVM("srlinux-1", str(uuid.uuid4()), compute_project, + manager, "srlinux:latest", + console_type="docker_exec", + environment="GNS3_SKIP_INIT=1", + extra_volumes=["/etc/opt/srlinux"]) + await vm.create() + sent = mock.call_args.kwargs["data"] + targets = [m["Target"] for m in sent["HostConfig"]["Mounts"]] + # /etc/network must NOT be mounted + assert "/gns3volumes/etc/network" not in targets + assert "/etc/network" not in targets + # the declared volumes are bound DIRECTLY at their real paths — + # no /gns3volumes aliasing and no post-start bridge + assert "/opt/srlinux/appmgr" in targets + assert "/etc/opt/srlinux" in targets + assert not any(t.startswith("/gns3volumes/") for t in targets) + # GNS3_VOLUMES env must also exclude /etc/network + vol_env = [v for v in sent["Env"] if v.startswith("GNS3_VOLUMES=")][0] + assert "/etc/network" not in vol_env + # host skeleton dir removed + assert not os.path.exists(os.path.join(vm.working_dir, "etc", "network")) @pytest.mark.asyncio @@ -321,7 +331,6 @@ async def test_start_docker_exec_dispatches_console(compute_project, manager): vm._get_namespace = AsyncioMagicMock(return_value=42) vm._add_ubridge_connection = AsyncioMagicMock() vm._start_docker_exec_console = AsyncioMagicMock() - vm._setup_skip_init_volumes = AsyncioMagicMock() vm._fix_permissions = AsyncioMagicMock() with patch("gns3server.compute.docker.Docker.install_busybox"): @@ -330,8 +339,8 @@ async def test_start_docker_exec_dispatches_console(compute_project, manager): vm._start_docker_exec_console.assert_called_once() assert vm.status == "started" - # SKIP_INIT path runs the volume bridge + permission fix - vm._setup_skip_init_volumes.assert_called_once() + # SKIP_INIT path still runs the permission fix (volumes are already + # seeded and bound at create time — no post-start bridge anymore) vm._fix_permissions.assert_called_once() @@ -346,19 +355,18 @@ async def test_start_without_skip_init_skips_vendor_passes(compute_project, mana vm._get_namespace = AsyncioMagicMock(return_value=42) vm._add_ubridge_connection = AsyncioMagicMock() vm._start_docker_exec_console = AsyncioMagicMock() - vm._setup_skip_init_volumes = AsyncioMagicMock() vm._fix_permissions = AsyncioMagicMock() with patch("gns3server.compute.docker.Docker.install_busybox"): with asyncio_patch("gns3server.compute.docker.Docker.query"): await vm.start() - # init.sh runs (no SKIP_INIT) → no vendor bridge/fix passes - vm._setup_skip_init_volumes.assert_not_called() + # init.sh runs (no SKIP_INIT) → no vendor permission pass + vm._fix_permissions.assert_not_called() # --------------------------------------------------------------------------- -# _fix_permissions — container-side, skips dead containers, targets /gns3volumes +# _fix_permissions — container-side, skips dead containers, targets volume paths # --------------------------------------------------------------------------- @pytest.mark.asyncio @@ -387,7 +395,7 @@ async def test_fix_permissions_skips_missing_container(compute_project, manager) @pytest.mark.asyncio -async def test_fix_permissions_targets_gns3volumes(compute_project, manager): +async def test_fix_permissions_targets_volume_paths(compute_project, manager): vm = _make_vm(compute_project, manager, environment="GNS3_SKIP_INIT=1") vm._volumes = ["/etc/opt/srlinux", "/var/log/srlinux"] @@ -404,37 +412,120 @@ async def test_fix_permissions_targets_gns3volumes(compute_project, manager): await vm._fix_permissions() # one exec per volume assert mock_exec.call_count == 2 - # each script must target /gns3volumes, not the raw path + # each script must target the real in-container path (the direct + # bind mount), never the old /gns3volumes alias for call_obj in mock_exec.call_args_list: script = call_obj.args[-1] # last positional arg is the sh -c script - assert "/gns3volumes" in script - # must NOT chown the in-container path directly - assert 'chown' in script and '"/gns3volumes' in script + assert "/gns3volumes" not in script + assert '"/etc/opt/srlinux"' in script or '"/var/log/srlinux"' in script + assert 'chown' in script # --------------------------------------------------------------------------- -# _setup_skip_init_volumes — bridge via docker exec +# _prepare_volumes — host-side seeding (docker create + cp + rm) # --------------------------------------------------------------------------- +def _seed_proc(stdout=b"seedcid\n", returncode=0): + proc = MagicMock() + proc.communicate = AsyncioMagicMock(return_value=(stdout, b"")) + proc.returncode = returncode + return proc + + @pytest.mark.asyncio -async def test_setup_skip_init_volumes_runs_exec(compute_project, manager): +async def test_prepare_volumes_seeds_unmarked_volume(compute_project, manager): vm = _make_vm(compute_project, manager, environment="GNS3_SKIP_INIT=1", extra_volumes=["/etc/opt/srlinux"]) - vm._volumes = ["/etc/opt/srlinux"] - - proc = MagicMock() - proc.communicate = AsyncioMagicMock(return_value=(b"", b"")) - proc.returncode = 0 + image_info = {"Config": {"Volumes": {}}} with patch("asyncio.subprocess.create_subprocess_exec", - return_value=proc) as mock_exec: - await vm._setup_skip_init_volumes() - assert mock_exec.call_count == 1 - script = mock_exec.call_args.args[-1] - # must do the bind mount - assert "mount --bind" in script - assert "/gns3volumes/etc/opt/srlinux" in script + return_value=_seed_proc()) as mock_exec: + await vm._prepare_volumes(image_info) + # docker create + docker cp + docker rm + assert mock_exec.call_count == 3 + argvs = [c.args for c in mock_exec.call_args_list] + assert argvs[0][1:3] == ("create", "srlinux:latest") + assert argvs[1][1:4] == ("cp", "-a", "seedcid:/etc/opt/srlinux/.") + assert argvs[2][1:3] == ("rm", "-f") + host_dir = os.path.join(vm.working_dir, "etc", "opt", "srlinux") + assert os.path.exists(os.path.join(host_dir, ".gns3_perms")) + + # a second create() must not re-seed (marker present): no docker CLI call + await vm._prepare_volumes(image_info) + assert mock_exec.call_count == 3 + + +@pytest.mark.asyncio +async def test_prepare_volumes_never_overwrites_marked_volume(compute_project, manager): + """Regression guard: a volume that ever started (marker present) holds the + node's saved configuration — re-seeding would reset it to factory.""" + + vm = _make_vm(compute_project, manager, environment="GNS3_SKIP_INIT=1", + extra_volumes=["/etc/opt/srlinux"]) + host_dir = os.path.join(vm.working_dir, "etc", "opt", "srlinux") + os.makedirs(host_dir, exist_ok=True) + marker = os.path.join(host_dir, ".gns3_perms") + open(marker, "w").close() + saved = os.path.join(host_dir, "config.json") + with open(saved, "w") as f: + f.write('{"user": "config"}') + + with patch("asyncio.subprocess.create_subprocess_exec", + return_value=_seed_proc()) as mock_exec: + await vm._prepare_volumes({"Config": {"Volumes": {}}}) + mock_exec.assert_not_called() + with open(saved) as f: + assert f.read() == '{"user": "config"}' + + +@pytest.mark.asyncio +async def test_prepare_volumes_tolerates_missing_image_path(compute_project, manager): + """A volume path the image does not contain (e.g. XRd's /xr-storage-shadow) + starts empty — cp fails, the marker is still written, no raise.""" + + vm = _make_vm(compute_project, manager, environment="GNS3_SKIP_INIT=1", + extra_volumes=["/xr-storage-shadow"]) + calls = {"n": 0} + + def proc_factory(*args, **kwargs): + # first call (docker create) succeeds, second (docker cp) fails, + # third (docker rm) succeeds + codes = [0, 1, 0] + proc = _seed_proc(returncode=codes[calls["n"]]) + calls["n"] += 1 + return proc + + with patch("asyncio.subprocess.create_subprocess_exec", + side_effect=proc_factory): + await vm._prepare_volumes({"Config": {"Volumes": {}}}) + assert calls["n"] == 3 # rm still ran (finally path) + host_dir = os.path.join(vm.working_dir, "xr-storage-shadow") + assert os.path.exists(os.path.join(host_dir, ".gns3_perms")) + + +@pytest.mark.asyncio +async def test_prepare_volumes_skips_without_skip_init(compute_project, manager): + + vm = _make_vm(compute_project, manager) # no SKIP_INIT + with patch("asyncio.subprocess.create_subprocess_exec", + return_value=_seed_proc()) as mock_exec: + await vm._prepare_volumes({"Config": {"Volumes": {"/etc/opt/srlinux": None}}}) + mock_exec.assert_not_called() + + +@pytest.mark.asyncio +async def test_prepare_volumes_raises_when_seed_container_fails(compute_project, manager): + """If `docker create` itself fails, creation must abort loudly instead of + binding an empty directory over the NOS's config path.""" + + vm = _make_vm(compute_project, manager, environment="GNS3_SKIP_INIT=1", + extra_volumes=["/etc/opt/srlinux"]) + proc = _seed_proc(stdout=b"", returncode=1) + + with patch("asyncio.subprocess.create_subprocess_exec", return_value=proc): + with pytest.raises(DockerError): + await vm._prepare_volumes({"Config": {"Volumes": {}}}) # ---------------------------------------------------------------------------