From 86d30f34b451a03dc5acd4923e4c6c56b844cf2b Mon Sep 17 00:00:00 2001 From: YueGuobin Date: Thu, 13 Aug 2026 22:39:48 +0800 Subject: [PATCH 01/16] docker: inject /dev/shm size and host devices via HostConfig from env Heavy NOS containers (e.g. Cisco XRd) need /dev/shm larger than Docker's 64 MB default and host device nodes such as /dev/fuse. Add two opt-in environment variables, consumed host-side and applied as native Docker HostConfig keys at create time: GNS3_SHM_SIZE (MB) -> HostConfig.ShmSize (bytes) GNS3_DEVICES -> HostConfig.Devices in `docker run --device` syntax (host[:container[:perm]]; Docker resolves major/minor from the host node itself) Native HostConfig (rather than remount/mknod inside init.sh) is used so this works for vendor NOS nodes that skip init.sh (console_type=docker_exec) -- the path XRd must take, since GNS3's init.sh wrapper crashes XRd's glibc loader. It applies whether or not init.sh runs, needs no schema/API/UI change (reuses the `environment` field), and only takes effect when the vars are set, so ordinary nodes keep default Docker behaviour. GNS3_-prefixed user env vars stay dropped from the container environment (only consumed here host-side), keeping GNS3-injected vars safe. --- gns3server/compute/docker/docker_vm.py | 50 ++++++++++++++++++++++++++ tests/compute/docker/test_docker_vm.py | 36 +++++++++++++++++++ 2 files changed, 86 insertions(+) diff --git a/gns3server/compute/docker/docker_vm.py b/gns3server/compute/docker/docker_vm.py index 804039216..0f98363ad 100644 --- a/gns3server/compute/docker/docker_vm.py +++ b/gns3server/compute/docker/docker_vm.py @@ -491,6 +491,25 @@ 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(): + line = line.strip() + 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 + if params["Entrypoint"] is None: params["Entrypoint"] = [] if self._start_command: @@ -625,6 +644,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 diff --git a/tests/compute/docker/test_docker_vm.py b/tests/compute/docker/test_docker_vm.py index c9e701306..8938ceaa9 100644 --- a/tests/compute/docker/test_docker_vm.py +++ b/tests/compute/docker/test_docker_vm.py @@ -271,6 +271,42 @@ 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_with_colon_in_project_name(compute_project, manager): From 2bee34031c5ef66d6331b1e39a1f4054a4facb51 Mon Sep 17 00:00:00 2001 From: YueGuobin Date: Thu, 13 Aug 2026 22:39:48 +0800 Subject: [PATCH 02/16] docker: warn about low inotify/file-max and missing FUSE at connect Add a read-only _check_host_readiness() that runs once after the Docker daemon connection is established. It reads /proc/sys inotify/file-max limits and /proc/filesystems (for FUSE), and logs a warning with the exact commands to fix when they are too low for heavy containers -- XRd wants ~4000 inotify instances per node against a stock default of 128. The server runs unprivileged (only the setuid ubridge helper has root), so it can only check, not set; the warning tells the admin exactly what to raise once. Stays silent when the limits are already sufficient. --- gns3server/compute/docker/__init__.py | 55 +++++++++++++++++++++++++++ tests/compute/docker/test_docker.py | 51 +++++++++++++++++++++++++ 2 files changed, 106 insertions(+) diff --git a/gns3server/compute/docker/__init__.py b/gns3server/compute/docker/__init__.py index ab095084f..28c76e09b 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,60 @@ 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): + return # Not Linux or unreadable -- nothing to check. + 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/tests/compute/docker/test_docker.py b/tests/compute/docker/test_docker.py index 0a28a273b..11b610bb7 100644 --- a/tests/compute/docker/test_docker.py +++ b/tests/compute/docker/test_docker.py @@ -364,3 +364,54 @@ 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] From 08e37a4509dd9afc716c9d3bc15b60a6c046932e Mon Sep 17 00:00:00 2001 From: YueGuobin Date: Thu, 13 Aug 2026 23:33:34 +0800 Subject: [PATCH 03/16] docker: inject config files into containers via extra_configs Add an `extra_configs` field (list of {target, content}) to the docker node/template/appliance schemas. For each entry GNS3 writes `content` to a file in the node working directory and bind-mounts it read-only at `target` inside the container. This lets a NOS appliance seed its startup config without rebuilding the image: XRd points XR_FIRST_BOOT_CONFIG at an injected /firstboot.cfg, FRR at /etc/frr/frr.conf, etc. The bind is a single-file mount applied at create time, so it works for both the generic init.sh path and vendor nodes that skip init.sh (console_type=docker_exec). Entries are only injected when present, so ordinary nodes are unaffected. The content can't go through `environment` (it is line-delimited, one var per line), hence a dedicated field -- the same plumbing shape as extra_volumes. --- gns3server/api/routes/compute/docker_nodes.py | 5 ++- gns3server/compute/docker/docker_vm.py | 33 ++++++++++++++++ gns3server/schemas/common.py | 13 +++++++ gns3server/schemas/compute/docker_nodes.py | 3 +- gns3server/schemas/controller/appliances.py | 5 +++ .../controller/templates/docker_templates.py | 3 +- tests/compute/docker/test_docker_vm.py | 39 +++++++++++++++++++ 7 files changed, 98 insertions(+), 3 deletions(-) diff --git a/gns3server/api/routes/compute/docker_nodes.py b/gns3server/api/routes/compute/docker_nodes.py index e64ec3d60..528925ff9 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", ] diff --git a/gns3server/compute/docker/docker_vm.py b/gns3server/compute/docker/docker_vm.py index 0f98363ad..a2980f359 100644 --- a/gns3server/compute/docker/docker_vm.py +++ b/gns3server/compute/docker/docker_vm.py @@ -89,6 +89,7 @@ class DockerVM(BaseNode): console_http_path="/", extra_hosts=None, extra_volumes=[], + extra_configs=None, memory=0, cpus=0, ): @@ -118,6 +119,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 +166,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 +298,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 +402,28 @@ 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 ".." in target.split("/"): + raise DockerError( + f"Extra config target '{target}' must be an absolute path and not contain '..'." + ) + 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): diff --git a/gns3server/schemas/common.py b/gns3server/schemas/common.py index b15bc5201..fbf8ddf11 100644 --- a/gns3server/schemas/common.py +++ b/gns3server/schemas/common.py @@ -48,6 +48,19 @@ 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") + + 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/compute/docker/test_docker_vm.py b/tests/compute/docker/test_docker_vm.py index 8938ceaa9..8041b623a 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, @@ -307,6 +308,44 @@ async def test_create_applies_env_host_config(compute_project, manager): ), "GNS3_ user vars must not leak into the container environment" +@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): From 088f1da77a072caa5778c6d4b2e96861d835552f Mon Sep 17 00:00:00 2001 From: YueGuobin Date: Fri, 14 Aug 2026 00:20:08 +0800 Subject: [PATCH 04/16] docker: persist extra_configs in the template DB table The extra_configs schema field needs a column on the docker_templates table to actually round-trip through the controller DB (the schema alone is accepted but dropped by the SQLAlchemy model mapping). Add the JSON column and an Alembic migration so existing databases get it on upgrade. --- gns3server/db/models/templates.py | 1 + ...b_add_extra_configs_to_docker_templates.py | 26 +++++++++++++++++++ 2 files changed, 27 insertions(+) create mode 100644 gns3server/db_migrations/versions/8f2a1c4e9d3b_add_extra_configs_to_docker_templates.py 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') From 347537f1b36884707a565a20ad734c19dfdfa9f1 Mon Sep 17 00:00:00 2001 From: YueGuobin Date: Fri, 14 Aug 2026 01:12:21 +0800 Subject: [PATCH 05/16] docker: mask systemd-udevd in privileged containers (GNS3_MASK_UDEV) A privileged systemd-based NOS container (Cisco XRd boots /usr/sbin/init) runs systemd-udevd, which on startup coldplugs every device it can reach. In privileged mode that includes the HOST's USB/input/audio/disk devices, so every XRd start reconnects USB, mutes audio, and disrupts the host journal -- highly disruptive on Linux desktops (caught in the act: the container's udevd was even rescanning the host BTRFS root device). XRd doesn't need udev (its interfaces are pre-created by GNS3 veth and mapped via XR_INTERFACES). Add two opt-in env vars, consumed host-side at container create time in the inherited DockerVM.create (so VendorDockerVM nodes get it too): GNS3_MASK_UDEV=1 -> bind /dev/null over the udevd unit, its two activation sockets, and the coldplug/settle trigger services GNS3_MASK_SYSTEMD=u1,u2 -> bind /dev/null over arbitrary units in /etc/systemd/system/ (comma/semicolon list) Only injected when set, so ordinary nodes are unaffected. --- gns3server/compute/docker/docker_vm.py | 38 ++++++++++++++++++++++++++ tests/compute/docker/test_docker_vm.py | 23 ++++++++++++++++ 2 files changed, 61 insertions(+) diff --git a/gns3server/compute/docker/docker_vm.py b/gns3server/compute/docker/docker_vm.py index a2980f359..382db3690 100644 --- a/gns3server/compute/docker/docker_vm.py +++ b/gns3server/compute/docker/docker_vm.py @@ -69,6 +69,17 @@ 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", + ) + def __init__( self, name, @@ -542,6 +553,33 @@ class DockerVM(BaseNode): 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 unit in self._UDEV_UNITS: + params["HostConfig"]["Mounts"].append({ + "Type": "bind", + "Source": "/dev/null", + "Target": f"/etc/systemd/system/{unit}", + "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, + }) if params["Entrypoint"] is None: params["Entrypoint"] = [] diff --git a/tests/compute/docker/test_docker_vm.py b/tests/compute/docker/test_docker_vm.py index 8041b623a..b71c9e3ef 100644 --- a/tests/compute/docker/test_docker_vm.py +++ b/tests/compute/docker/test_docker_vm.py @@ -308,6 +308,29 @@ async def test_create_applies_env_host_config(compute_project, manager): ), "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 + 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): """ From b928a2f48a3c64c63b7d932b776891422fbcb98a Mon Sep 17 00:00:00 2001 From: YueGuobin Date: Fri, 14 Aug 2026 01:27:18 +0800 Subject: [PATCH 06/16] docker: use the container's chown (not busybox) in vendor volume/perms path The vendor skip-init path (_setup_skip_init_volumes, _fix_permissions) runs `/gns3/bin/busybox chown` inside the container via docker exec. busybox is statically linked, and its chown dlopens NSS modules (libnss_*) from the container; on NOS images whose glibc differs from the host's (e.g. Cisco XRd) that mismatches and aborts with the glibc assertion `_dl_call_libc_early_init: sym != NULL` (SIGABRT). The per-file chown loop then crash-loops, and the resulting core-dump storm -- processed by the host's systemd-coredump -- cascades into host device rescans, reconnecting USB / resetting audio / corrupting the journal on every XRd start. cp/chmod/find/stat don't touch NSS and work fine on busybox, so only chown is affected. Prefer the container's own coreutils chown (`command -v chown && chown ...`), falling back to busybox chown only when the container ships no chown (minimal images, where the glibc matches and busybox is safe). --- gns3server/compute/docker/vendor_docker_vm.py | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/gns3server/compute/docker/vendor_docker_vm.py b/gns3server/compute/docker/vendor_docker_vm.py index f0410d196..e4006f683 100644 --- a/gns3server/compute/docker/vendor_docker_vm.py +++ b/gns3server/compute/docker/vendor_docker_vm.py @@ -183,6 +183,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 +201,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 +239,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 From e8b51aa12c3a9468ad59053f0e280a48892131f3 Mon Sep 17 00:00:00 2001 From: YueGuobin Date: Fri, 14 Aug 2026 19:09:48 +0800 Subject: [PATCH 07/16] docker: also null-bind udevadm in GNS3_MASK_UDEV Masking the udev systemd units stopped the daemon's coldplug (host audio resets), but host USB devices still reconnected on every XRd start. A/B testing with plain `docker run` isolated the trigger: XRd's own xr_startup.sh calls udevadm directly (USB license-dongle probing, e.g. `udevadm trigger --action=add --parent-match=`), which synthesizes uevents into the host kernel from the privileged container -- no udevd required. GNS3_MASK_UDEV=1 now also binds /dev/null over the udevadm binary (/bin, /sbin, /usr/bin). Verified with a plain-run experiment: with the bind, host udev monitor shows zero usb/input/hid/sound events during XRd boot (only normal docker veth traffic), and XRd itself boots to running state -- it does not need udevadm under GNS3 (interfaces are pre-created veths). --- gns3server/compute/docker/docker_vm.py | 17 +++++++++++++++-- tests/compute/docker/test_docker_vm.py | 2 ++ 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/gns3server/compute/docker/docker_vm.py b/gns3server/compute/docker/docker_vm.py index 382db3690..8f3961a35 100644 --- a/gns3server/compute/docker/docker_vm.py +++ b/gns3server/compute/docker/docker_vm.py @@ -80,6 +80,19 @@ class DockerVM(BaseNode): "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, @@ -561,11 +574,11 @@ class DockerVM(BaseNode): # 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 unit in self._UDEV_UNITS: + 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": f"/etc/systemd/system/{unit}", + "Target": target, "ReadOnly": True, }) elif line.startswith("GNS3_MASK_SYSTEMD="): diff --git a/tests/compute/docker/test_docker_vm.py b/tests/compute/docker/test_docker_vm.py index b71c9e3ef..53bfb2445 100644 --- a/tests/compute/docker/test_docker_vm.py +++ b/tests/compute/docker/test_docker_vm.py @@ -327,6 +327,8 @@ async def test_create_masks_systemd_units(compute_project, manager): 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 From 0f12786885880139066a7c47de76d26056d973d5 Mon Sep 17 00:00:00 2001 From: YueGuobin Date: Fri, 14 Aug 2026 19:14:59 +0800 Subject: [PATCH 08/16] docs: document the XRd control-plane adaptation New feature doc covering why XRd takes the vendor docker_exec/SKIP_INIT path (init.sh wrapper crashes its glibc loader), the four generic mechanisms added for it (GNS3_SHM_SIZE/GNS3_DEVICES HostConfig injection, extra_configs file injection, GNS3_MASK_UDEV + udevadm null-binding, host-readiness check), the three host-disturbance root causes isolated by plain docker-run A/B/C testing, the appliance recipe with XRd-specific gotchas (Mg0/RP0/CPU0/0, /xr-storage-shadow persistence, first-boot semantics), and troubleshooting. Indexed in docs/README.md alongside the docker-exec-console base doc. --- docs/README.md | 8 +- docs/features/vendor-nos-xrd.md | 193 ++++++++++++++++++++++++++++++++ 2 files changed, 200 insertions(+), 1 deletion(-) create mode 100644 docs/features/vendor-nos-xrd.md diff --git a/docs/README.md b/docs/README.md index bc2d414aa..6c5498c50 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/`) @@ -120,4 +126,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/features/vendor-nos-xrd.md b/docs/features/vendor-nos-xrd.md new file mode 100644 index 000000000..e586e3f3d --- /dev/null +++ b/docs/features/vendor-nos-xrd.md @@ -0,0 +1,193 @@ + + +> 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-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-shadow (persisted)"] + 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()` | +| 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-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 +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. +- `/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. +- The official image ships no default login; the first-boot config must + create one (e.g. `username admin / group root-lr / secret ...`). +- 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 be `/xr-storage-shadow` | +| 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.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. | From 1cddeb8c3fe05151f56a123346f8f038053ee775 Mon Sep 17 00:00:00 2001 From: YueGuobin Date: Fri, 14 Aug 2026 19:50:58 +0800 Subject: [PATCH 09/16] docs: record the link UDP self-loop bug; mark XRd datapath validated Add bugs/link-udp-self-loop.md: the intermittent one-way docker link observed during XRd validation (one end's nio_udp rport pointing at its own lport after a uBridge restart), with the evidence table, the uBridge console capture diagnostics, the delete/recreate workaround, and the narrowed suspects (batch port preallocation / link re-creation race). Also note that a docker node's in-container ethN is a TAP device held by uBridge (no veth host end exists). Update the XRd feature doc: datapath validated end-to-end (XRd brings its own interfaces up, ARP/ICMP bidirectional), add a troubleshooting row pointing at the bug doc. Index the bug in docs/README.md. --- docs/README.md | 1 + docs/bugs/link-udp-self-loop.md | 77 +++++++++++++++++++++++++++++++++ docs/features/vendor-nos-xrd.md | 2 + 3 files changed, 80 insertions(+) create mode 100644 docs/bugs/link-udp-self-loop.md diff --git a/docs/README.md b/docs/README.md index 6c5498c50..79908e27c 100644 --- a/docs/README.md +++ b/docs/README.md @@ -109,6 +109,7 @@ Cisco XRd as a GNS3 Docker router: vendor path + shm/device injection (`GNS3_SHM ## 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 (one end's NIO points at itself); delete/recreate the link as workaround (Medium severity, Open) --- diff --git a/docs/bugs/link-udp-self-loop.md b/docs/bugs/link-udp-self-loop.md new file mode 100644 index 000000000..98c4ba1cc --- /dev/null +++ b/docs/bugs/link-udp-self-loop.md @@ -0,0 +1,77 @@ + + +> 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**: Open — root cause not yet pinpointed; workaround reliable +**Component**: Link wiring — `gns3server/controller/udp_link.py` (`_prepare` / +`pop_preallocated_udp_port` in `gns3server/controller/project.py`) interacting with +node/uBridge restarts + +## Symptoms + +Two Docker nodes (observed with Cisco XRd; likely 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 a 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 | + +Conclusion: **B's `nio_udp` rport pointed at B's own lport** — a UDP self-loop. B's +uBridge had restarted ~77 s after A's (node stop/start during the session), +i.e. the link was re-wired at least once. + +Deleting and re-creating the link fixed it immediately (fresh port allocation). + +## Root Cause Analysis (narrowed, not final) + +- `UDPLink._prepare()` builds mirrored NIO data correctly + (`node1: lport=P1/rport=P2`, `node2: lport=P2/rport=P1`) — the logic itself is sound. +- Suspects for the corrupted runtime state: + 1. `Project.pop_preallocated_udp_port()` — the batch project-open preallocation + (link-create performance work) racing with link re-creation; + 2. link re-creation racing a node/uBridge restart (commit NIOs to a uBridge that is + being torn down/rebuilt), leaving a stale/self-pointing NIO on one side. +- A deterministic reproduction is still needed: create two Docker nodes + link, + restart one node, then verify the UDP wiring (see diagnostics). + +## 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. +4. **Workaround** — delete and re-create the link (or stop/start both nodes). + +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). diff --git a/docs/features/vendor-nos-xrd.md b/docs/features/vendor-nos-xrd.md index e586e3f3d..633a4b580 100644 --- a/docs/features/vendor-nos-xrd.md +++ b/docs/features/vendor-nos-xrd.md @@ -158,6 +158,7 @@ sequenceDiagram | `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` | +| Two nodes can't ping, only one side ARPs | GNS3 link wiring bug (UDP self-loop on one end), not XRd — 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 @@ -190,4 +191,5 @@ sequenceDiagram | Version | Date | Changes | |---------|------|---------| +| 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. | From 350f2b24e79f963e6b0a1097c89fb960b0c26ba2 Mon Sep 17 00:00:00 2001 From: YueGuobin Date: Fri, 14 Aug 2026 21:31:58 +0800 Subject: [PATCH 10/16] fix: UDP port allocation race causing link self-loop (one-way links) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PortManager.get_free_udp_port had an unguarded find-then-add sequence. A link allocates both ends concurrently (asyncio.gather in UDPLink._prepare -> two POST /ports/udp) and FastAPI runs the sync route handler in a threadpool, so both threads could probe the same 'free' port before either recorded it — handing lport == rport to both ends. uBridge sets SO_REUSEADDR on UDP NIO sockets, so the double bind succeeds silently and the kernel delivers everything to the last-bound socket: one node starves, the other echoes to itself. Make every TCP/UDP allocate/reserve/release path atomic with an RLock, and rebuild _link_data in UDPLink._prepare so reset() commits the fresh port pair instead of re-sending the stale, already-released one. Regression tests: threaded barrier allocation never returns duplicates (red on the old code, UDP and TCP); reset() leaves exactly one mirrored NIO pair per side with lport != rport (red on the old code). --- docs/README.md | 2 +- docs/bugs/link-udp-self-loop.md | 72 ++++++++++----- docs/features/vendor-nos-xrd.md | 3 +- gns3server/compute/port_manager.py | 141 ++++++++++++++++------------- gns3server/controller/udp_link.py | 7 ++ tests/compute/test_port_manager.py | 77 ++++++++++++++++ tests/controller/test_udp_link.py | 72 +++++++++++++++ 7 files changed, 285 insertions(+), 89 deletions(-) diff --git a/docs/README.md b/docs/README.md index 79908e27c..c347479cc 100644 --- a/docs/README.md +++ b/docs/README.md @@ -109,7 +109,7 @@ Cisco XRd as a GNS3 Docker router: vendor path + shm/device injection (`GNS3_SHM ## 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 (one end's NIO points at itself); delete/recreate the link as workaround (Medium 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) --- diff --git a/docs/bugs/link-udp-self-loop.md b/docs/bugs/link-udp-self-loop.md index 98c4ba1cc..e1e05e7cf 100644 --- a/docs/bugs/link-udp-self-loop.md +++ b/docs/bugs/link-udp-self-loop.md @@ -5,25 +5,24 @@ See LICENSE file for licensing information. > 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**: Open — root cause not yet pinpointed; workaround reliable -**Component**: Link wiring — `gns3server/controller/udp_link.py` (`_prepare` / -`pop_preallocated_udp_port` in `gns3server/controller/project.py`) interacting with -node/uBridge restarts +**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; likely node-type agnostic) linked on the +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 a live occurrence) +## Evidence (from the live occurrence) Captured simultaneously on both nodes' uBridge `bridge1` (see diagnostics below): @@ -34,23 +33,44 @@ Captured simultaneously on both nodes' uBridge `bridge1` (see diagnostics below) | B's `ethN` counters | RX ≈ TX ≈ 5000+ — B receives its own transmissions back | | A's `ethN` counters | TX > 0, RX = 0 — never receives anything | -Conclusion: **B's `nio_udp` rport pointed at B's own lport** — a UDP self-loop. B's -uBridge had restarted ~77 s after A's (node stop/start during the session), -i.e. the link was re-wired at least once. +## Root Cause (confirmed) -Deleting and re-creating the link fixed it immediately (fresh port allocation). +**`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. -## Root Cause Analysis (narrowed, not final) +Why it was *silent* and *asymmetric*: -- `UDPLink._prepare()` builds mirrored NIO data correctly - (`node1: lport=P1/rport=P2`, `node2: lport=P2/rport=P1`) — the logic itself is sound. -- Suspects for the corrupted runtime state: - 1. `Project.pop_preallocated_udp_port()` — the batch project-open preallocation - (link-create performance work) racing with link re-creation; - 2. link re-creation racing a node/uBridge restart (commit NIOs to a uBridge that is - being torn down/rebuilt), leaving a stale/self-pointing NIO on one side. -- A deterministic reproduction is still needed: create two Docker nodes + link, - restart one node, then verify the UDP wiring (see diagnostics). +- 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) @@ -63,8 +83,10 @@ Deleting and re-creating the link fixed it immediately (fresh port allocation). 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. -4. **Workaround** — delete and re-create the link (or stop/start both nodes). + 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 @@ -74,4 +96,6 @@ 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). +- 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 index 633a4b580..68b7a21e3 100644 --- a/docs/features/vendor-nos-xrd.md +++ b/docs/features/vendor-nos-xrd.md @@ -158,7 +158,7 @@ sequenceDiagram | `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` | -| Two nodes can't ping, only one side ARPs | GNS3 link wiring bug (UDP self-loop on one end), not XRd — delete and re-create the link; see [link-udp-self-loop](../bugs/link-udp-self-loop.md) | +| 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 @@ -191,5 +191,6 @@ sequenceDiagram | Version | Date | Changes | |---------|------|---------| +| 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/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/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): """ From e9339faaa7b04bfb55d6a5113736cba466228620 Mon Sep 17 00:00:00 2001 From: YueGuobin Date: Fri, 14 Aug 2026 22:02:58 +0800 Subject: [PATCH 11/16] docker: graceful stop for vendor NOS containers (SIGTERM + 60s grace) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 | 30 ++++++---- gns3server/compute/docker/docker_vm.py | 18 ++++-- gns3server/compute/docker/vendor_docker_vm.py | 16 +++++- tests/compute/docker/test_vendor_docker_vm.py | 55 +++++++++++++++++++ 4 files changed, 103 insertions(+), 16 deletions(-) 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() From 62076c1727d22169eda2ef8e21fc4c3ce5701bd2 Mon Sep 17 00:00:00 2001 From: YueGuobin Date: Fri, 14 Aug 2026 22:11:34 +0800 Subject: [PATCH 12/16] docker: make the vendor graceful-stop grace period configurable (GNS3_STOP_TIMEOUT) The 60 s SIGTERM grace was hardcoded, unlike every other vendor knob (GNS3_SHM_SIZE, GNS3_DEVICES, GNS3_MASK_UDEV, ...) which rides the environment line. Parse GNS3_STOP_TIMEOUT= (default 60, clamped to 1-600, invalid values keep the default) and use it in VendorDockerVM._terminate_container(). --- docs/features/vendor-nos-xrd.md | 4 +-- gns3server/compute/docker/vendor_docker_vm.py | 21 +++++++++++---- tests/compute/docker/test_vendor_docker_vm.py | 27 +++++++++++++++++++ 3 files changed, 45 insertions(+), 7 deletions(-) diff --git a/docs/features/vendor-nos-xrd.md b/docs/features/vendor-nos-xrd.md index 41e08a7f8..8a9083005 100644 --- a/docs/features/vendor-nos-xrd.md +++ b/docs/features/vendor-nos-xrd.md @@ -65,7 +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()` | +| graceful stop | `GNS3_STOP_TIMEOUT=60` (seconds; default 60) | vendor containers are stopped with SIGTERM + a grace period (Docker SIGKILLs once it expires) 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 @@ -200,7 +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.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/compute/docker/vendor_docker_vm.py b/gns3server/compute/docker/vendor_docker_vm.py index e7b155f34..ca85c1813 100644 --- a/gns3server/compute/docker/vendor_docker_vm.py +++ b/gns3server/compute/docker/vendor_docker_vm.py @@ -55,6 +55,8 @@ 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): @@ -66,6 +68,7 @@ class VendorDockerVM(DockerVM): 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(): @@ -78,6 +81,13 @@ 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()) + if 1 <= timeout <= 600: + self._stop_timeout = timeout + except ValueError: + pass # ---- hook overrides --------------------------------------------------- @@ -142,13 +152,14 @@ class VendorDockerVM(DockerVM): """ 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. + shutdown). Send SIGTERM and wait up to ``GNS3_STOP_TIMEOUT`` seconds + (default 60) 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}) + await self.manager.query("POST", f"containers/{self._cid}/stop", params={"t": self._stop_timeout}) except DockerHttp304Error: pass # already stopped diff --git a/tests/compute/docker/test_vendor_docker_vm.py b/tests/compute/docker/test_vendor_docker_vm.py index 1fbbbcc1b..33c5ce726 100644 --- a/tests/compute/docker/test_vendor_docker_vm.py +++ b/tests/compute/docker/test_vendor_docker_vm.py @@ -667,3 +667,30 @@ async def test_stop_uses_graceful_termination(compute_project, manager): ) as mock_term: await vm.stop() mock_term.assert_called_once() + + +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 + + +@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.query = AsyncioMagicMock() + + await vm._terminate_container() + + manager.query.assert_called_once_with("POST", "containers/e90e34656842/stop", params={"t": 120}) From 1124d7a5390eb570b33d75e3ca13ce282397d035 Mon Sep 17 00:00:00 2001 From: YueGuobin Date: Sat, 15 Aug 2026 00:09:53 +0800 Subject: [PATCH 13/16] docs: XRd appliance tunes GNS3_STOP_TIMEOUT to 40 s; note version-agnostic template image --- docs/features/vendor-nos-xrd.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/features/vendor-nos-xrd.md b/docs/features/vendor-nos-xrd.md index 8a9083005..e0610d4be 100644 --- a/docs/features/vendor-nos-xrd.md +++ b/docs/features/vendor-nos-xrd.md @@ -103,6 +103,7 @@ 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;... From 9604c85fdafc8347036cd1212b6cd648db939d3e Mon Sep 17 00:00:00 2001 From: YueGuobin Date: Sat, 15 Aug 2026 00:52:55 +0800 Subject: [PATCH 14/16] docker: harden the shm/devices/extra_configs/masking work (code review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Nine fixes from a review of the docker-shm-devices diff: * GNS3_STOP_TIMEOUT >300 s aborted at the manager's default HTTP timeout before Docker finished the stop — the stop query now gets a timeout with a margin over the grace period. * Overlapping bind targets (GNS3_MASK_UDEV + GNS3_MASK_SYSTEMD on the same unit, a unit named twice, an extra_configs target equal to a masked unit) made Docker reject the create with 'Duplicate mount point' — Mounts are deduplicated by target. * ExtraConfig.target now carries a pydantic validator (absolute file path, no '..'), so bad targets 422 at template-save time instead of failing at node-create time after a multi-GB image pull; directory forms ('/', '/etc/') are also rejected by the runtime guard instead of raising IsADirectoryError (raw 500). * _check_host_readiness skipped every remaining check when one /proc/sys key was unreadable (mid-loop return) — now continues. * The base-class GNS3_* env parser strips trailing commas like the vendor parser, so 'GNS3_MASK_UDEV=1,' composed from a list still activates. * Vendor env knobs are re-parsed on every create(), so a PUT to the node's environment takes effect on the next (re)create. * The graceful SIGTERM stop is now limited to the explicit user stop route; delete/update/close/crash-cleanup keep the immediate kill (those paths force-delete or recreate the container right after). * An extra_configs target beneath a persisted volume is shadowed by the volume bind — warn at create time. --- docs/features/vendor-nos-xrd.md | 3 +- gns3server/api/routes/compute/docker_nodes.py | 6 +- gns3server/compute/docker/__init__.py | 4 +- gns3server/compute/docker/docker_vm.py | 48 +++++++-- gns3server/compute/docker/vendor_docker_vm.py | 53 ++++++++-- gns3server/schemas/common.py | 18 +++- tests/compute/docker/test_docker.py | 31 ++++++ tests/compute/docker/test_docker_vm.py | 100 ++++++++++++++++++ tests/compute/docker/test_vendor_docker_vm.py | 77 ++++++++++---- 9 files changed, 297 insertions(+), 43 deletions(-) diff --git a/docs/features/vendor-nos-xrd.md b/docs/features/vendor-nos-xrd.md index e0610d4be..cc18c5019 100644 --- a/docs/features/vendor-nos-xrd.md +++ b/docs/features/vendor-nos-xrd.md @@ -65,7 +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 | `GNS3_STOP_TIMEOUT=60` (seconds; default 60) | vendor containers are stopped with SIGTERM + a grace period (Docker SIGKILLs once it expires) instead of the base class' immediate kill — systemd NOS images require a graceful shutdown | `vendor_docker_vm.py` `_terminate_container()` | +| graceful stop | `GNS3_STOP_TIMEOUT=60` (seconds; default 60, max 600) | 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 | `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 @@ -201,6 +201,7 @@ sequenceDiagram | 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). | diff --git a/gns3server/api/routes/compute/docker_nodes.py b/gns3server/api/routes/compute/docker_nodes.py index 528925ff9..be55ec0f4 100644 --- a/gns3server/api/routes/compute/docker_nodes.py +++ b/gns3server/api/routes/compute/docker_nodes.py @@ -177,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 28c76e09b..ed37ca626 100644 --- a/gns3server/compute/docker/__init__.py +++ b/gns3server/compute/docker/__init__.py @@ -190,7 +190,9 @@ class Docker(BaseManager): with open(f"/proc/sys/{key.replace('.', '/')}") as f: current = int(f.read().strip()) except (OSError, ValueError): - return # Not Linux or unreadable -- nothing to check. + # 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)) diff --git a/gns3server/compute/docker/docker_vm.py b/gns3server/compute/docker/docker_vm.py index 5e48a5e6c..fa45c43e6 100644 --- a/gns3server/compute/docker/docker_vm.py +++ b/gns3server/compute/docker/docker_vm.py @@ -433,10 +433,21 @@ class DockerVM(BaseNode): 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 ".." in target.split("/"): + if not target.startswith("/") or target.endswith("/") or ".." in target.split("/"): raise DockerError( - f"Extra config target '{target}' must be an absolute path and not contain '..'." + 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: @@ -556,7 +567,10 @@ class DockerVM(BaseNode): # when set, so ordinary nodes keep the default Docker behaviour. if self._environment: for line in self._environment.splitlines(): - line = line.strip() + # 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) @@ -594,6 +608,19 @@ class DockerVM(BaseNode): "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: @@ -1179,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: @@ -1207,7 +1239,7 @@ class DockerVM(BaseNode): state = await self._get_container_state() if state != "stopped" and state != "exited": try: - await self._terminate_container() + await self._terminate_container(graceful=graceful) log.debug(f"Docker container '{self._name}' [{self._image}] stopped") except DockerHttp409Error: # Container is already stopped @@ -1218,14 +1250,16 @@ class DockerVM(BaseNode): return self.status = "stopped" - async def _terminate_container(self): + 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 (see VendorDockerVM). + 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") diff --git a/gns3server/compute/docker/vendor_docker_vm.py b/gns3server/compute/docker/vendor_docker_vm.py index ca85c1813..7d4729b67 100644 --- a/gns3server/compute/docker/vendor_docker_vm.py +++ b/gns3server/compute/docker/vendor_docker_vm.py @@ -62,14 +62,22 @@ class VendorDockerVM(DockerVM): 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(",") @@ -89,6 +97,12 @@ class VendorDockerVM(DockerVM): 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 --------------------------------------------------- def _mount_binds(self, image_info): @@ -148,18 +162,35 @@ class VendorDockerVM(DockerVM): pass self._console_exec_writer = None - async def _terminate_container(self): + 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). Send SIGTERM and wait up to ``GNS3_STOP_TIMEOUT`` seconds - (default 60) 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. + shutdown). + + With ``graceful`` (explicit user stop), send SIGTERM and wait up to + ``GNS3_STOP_TIMEOUT`` seconds (default 60, 1-600) for the services to + stop; Docker SIGKILLs the container itself once the grace period + expires, so no fallback kill is needed. The stop query gets an HTTP + timeout with a margin over the grace period — the manager's default + 300 s would abort first for values above it. + + 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: - await self.manager.query("POST", f"containers/{self._cid}/stop", params={"t": self._stop_timeout}) + 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 diff --git a/gns3server/schemas/common.py b/gns3server/schemas/common.py index fbf8ddf11..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 @@ -60,6 +60,22 @@ class ExtraConfig(BaseModel): 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): """ diff --git a/tests/compute/docker/test_docker.py b/tests/compute/docker/test_docker.py index 11b610bb7..7fbb49c06 100644 --- a/tests/compute/docker/test_docker.py +++ b/tests/compute/docker/test_docker.py @@ -415,3 +415,34 @@ async def test_check_host_readiness_silent_when_ok(caplog): 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 53bfb2445..70d72b3bf 100644 --- a/tests/compute/docker/test_docker_vm.py +++ b/tests/compute/docker/test_docker_vm.py @@ -1998,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 33c5ce726..a4e8e92e3 100644 --- a/tests/compute/docker/test_vendor_docker_vm.py +++ b/tests/compute/docker/test_vendor_docker_vm.py @@ -620,17 +620,36 @@ async def test_create_exec_cmd_has_no_while_true(compute_project, manager): @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.""" + """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/stop", params={"t": 60}) + manager.query.assert_called_once_with("POST", "containers/e90e34656842/kill") + manager.http_query.assert_not_called() @pytest.mark.asyncio @@ -641,15 +660,16 @@ async def test_terminate_container_already_stopped_is_silent(compute_project, ma from gns3server.compute.docker.docker_error import DockerHttp304Error vm = _make_vm(compute_project, manager) - manager.query = AsyncioMagicMock( + manager.http_query = AsyncioMagicMock( side_effect=DockerHttp304Error("Docker has returned an error: 304")) - await vm._terminate_container() # must not raise + 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), not the base class' immediate kill.""" + 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()): @@ -657,16 +677,12 @@ async def test_stop_uses_graceful_termination(compute_project, manager): with patch.object( DockerVM, "_get_container_state", new=AsyncioMagicMock(return_value="running") ): + vm._permissions_fixed = True 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() + 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): @@ -689,8 +705,29 @@ def test_env_stop_timeout_invalid_keeps_default(compute_project, manager): @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.query = AsyncioMagicMock() + manager.http_query = AsyncioMagicMock(return_value=MagicMock()) - await vm._terminate_container() + await vm._terminate_container(graceful=True) - manager.query.assert_called_once_with("POST", "containers/e90e34656842/stop", params={"t": 120}) + 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 From 110e041e56ceb52c143247a10549da1bb5ca12ef Mon Sep 17 00:00:00 2001 From: YueGuobin Date: Sat, 15 Aug 2026 01:25:50 +0800 Subject: [PATCH 15/16] docker: cap GNS3_STOP_TIMEOUT at 210 s (controller stop budget) The 600 s clamp was unreachable in practice: the controller's stop request times out at 240 s (controller/node.py) and the Docker stop query gets the value +30 s as its HTTP timeout, so anything above 210 would abort upstream first and surface an error while the stop keeps running server-side. Cap at the derived ceiling and document the chain in the clamp and the docstring. --- docs/features/vendor-nos-xrd.md | 2 +- gns3server/compute/docker/vendor_docker_vm.py | 16 ++++++++++------ tests/compute/docker/test_vendor_docker_vm.py | 5 +++++ 3 files changed, 16 insertions(+), 7 deletions(-) diff --git a/docs/features/vendor-nos-xrd.md b/docs/features/vendor-nos-xrd.md index cc18c5019..a512b9b35 100644 --- a/docs/features/vendor-nos-xrd.md +++ b/docs/features/vendor-nos-xrd.md @@ -65,7 +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 | `GNS3_STOP_TIMEOUT=60` (seconds; default 60, max 600) | 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 | `vendor_docker_vm.py` `_terminate_container()` | +| 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 diff --git a/gns3server/compute/docker/vendor_docker_vm.py b/gns3server/compute/docker/vendor_docker_vm.py index 7d4729b67..800d46603 100644 --- a/gns3server/compute/docker/vendor_docker_vm.py +++ b/gns3server/compute/docker/vendor_docker_vm.py @@ -92,7 +92,12 @@ class VendorDockerVM(DockerVM): elif _line.startswith("GNS3_STOP_TIMEOUT="): try: timeout = int(_line.split("=", 1)[1].strip()) - if 1 <= timeout <= 600: + # 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 @@ -169,11 +174,10 @@ class VendorDockerVM(DockerVM): shutdown). With ``graceful`` (explicit user stop), send SIGTERM and wait up to - ``GNS3_STOP_TIMEOUT`` seconds (default 60, 1-600) for the services to - stop; Docker SIGKILLs the container itself once the grace period - expires, so no fallback kill is needed. The stop query gets an HTTP - timeout with a margin over the grace period — the manager's default - 300 s would abort first for values above it. + ``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 diff --git a/tests/compute/docker/test_vendor_docker_vm.py b/tests/compute/docker/test_vendor_docker_vm.py index a4e8e92e3..3619d0b10 100644 --- a/tests/compute/docker/test_vendor_docker_vm.py +++ b/tests/compute/docker/test_vendor_docker_vm.py @@ -700,6 +700,11 @@ def test_env_stop_timeout_invalid_keeps_default(compute_project, manager): 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 From f43a717b20629073557988dd606d353411fdca5d Mon Sep 17 00:00:00 2001 From: YueGuobin Date: Sat, 15 Aug 2026 23:32:24 +0800 Subject: [PATCH 16/16] copilot: sync CONSOLE_TYPES with the server ConsoleType enum The vendored gns3fy copy keeps its type lists as literals (the module is shared with the standalone MCP service and cannot import server enums), and CONSOLE_TYPES had drifted: 'ssh' and 'docker_exec' were missing while both are valid server-side. Impact: the copilot topology reader validates the whole node list in one pydantic pass, so a single vendor NOS node (console_type 'docker_exec') made it drop the entire project and return zero devices to every copilot device tool. Add the missing values plus drift tests asserting the vendored lists cover the server enums (skipped when ai-features extras are absent). --- .../gns3_copilot/gns3_client/custom_gns3fy.py | 5 ++ tests/agent/test_custom_gns3fy.py | 71 +++++++++++++++++++ 2 files changed, 76 insertions(+) create mode 100644 tests/agent/test_custom_gns3fy.py 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/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"