From 5685f40dd69f2b70f42eb5e6e0ad682ed61d74cb Mon Sep 17 00:00:00 2001 From: YueGuobin Date: Sat, 5 Sep 2026 00:26:04 +0800 Subject: [PATCH] fix: keep payload-delivered state across container (re)creation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit create() re-runs _parse_vendor_environment on every (re)create — and a stop removes the container, so every start recreates it. Resetting the controller-allocated application id there silently flipped nodes to the hash fallback after their first stop/start (MAC change, plus collision risk with the allocation pool), and dropped any pending startup-config delivered by a PUT. The application id and startup-config state now default via class attributes instead of being re-initialized by the parser, and the hash fallback is removed outright: starting an IOL node without a coordinated allocation raises an actionable error — an uncoordinated id could collide with the pool and blackhole traffic as a MAC loop. Found in the E2E run: node booted app id 512, came back as 566 (hash fallback) after one stop/start; the PUT-ed config edit vanished the same way. --- docs/features/iol-runner-docker.md | 13 +++--- gns3server/compute/docker/iol_docker_vm.py | 48 ++++++++++++-------- tests/compute/docker/test_iol_docker_vm.py | 52 ++++++++++++++++------ 3 files changed, 75 insertions(+), 38 deletions(-) diff --git a/docs/features/iol-runner-docker.md b/docs/features/iol-runner-docker.md index 03b263282..3d2c62fbe 100644 --- a/docs/features/iol-runner-docker.md +++ b/docs/features/iol-runner-docker.md @@ -140,7 +140,10 @@ from it boots with that configuration as its personal starting point. share the nvram container format, so the server-side `nvram_import` utility produces a file IOL boots from directly. Consequences: * `write memory` survives stop/start and container recreation (a plain - restart never re-applies the startup-config over it). + restart never re-applies the startup-config over it). The first + `write memory` after a server-built NVRAM asks for a one-time + `[confirm]` (the builder stamps an IOS 15.4 version marker into the + nvram header; press return — IOS then writes its own). * Editing a node's `startup_config_content` property (PUT) re-applies it on the next start, overwriting what `write memory` had saved — the explicit edit wins, as with IOU. @@ -175,10 +178,10 @@ diagnosing wiring issues). ID (`aabb.cc{app}{iface}`), e.g. `aabb.cc03.0400`. The controller allocates the ID at node creation from the upper half of the id space (512–1022, disjoint from IOU's 1–511, limit 511 IOL Docker nodes across - opened projects sharing computes); without an allocation (raw compute API, - pre-allocation topologies) a stable node-derived fallback in the same - range is used. Nodes sharing an ID would silently drop each other's - frames as MAC loops. + opened projects sharing computes). Starting a node without an allocation + (raw compute API, pre-allocation topologies) is an error, not a fallback — + an uncoordinated ID could collide with the pool and make nodes silently + drop each other's frames as MAC loops. * **Interface names are IOL-style `Ethernet0/0`**, not `GigabitEthernet0/0` (4 ports per unit, matching the adapter-count granularity) — startup configs addressing `GigabitEthernet…` are rejected by the parser. diff --git a/gns3server/compute/docker/iol_docker_vm.py b/gns3server/compute/docker/iol_docker_vm.py index 096f3efad..38042de6d 100644 --- a/gns3server/compute/docker/iol_docker_vm.py +++ b/gns3server/compute/docker/iol_docker_vm.py @@ -31,8 +31,9 @@ wired by the generic ``GNS3_UNIX_SOCKET_NIO`` capability of VendorDockerVM (uBridge reaches them through a per-node runtime directory bound at /tmp — see ``VendorDockerVM._unix_socket_host_dir``). The controller allocates the IOL application ID (upper half of the id space, disjoint from IOU's) so that -linked nodes get distinct MACs; without an allocation a stable node-derived -fallback is used. +linked nodes get distinct MACs; starting a node without an allocation is an +error, not a fallback — an uncoordinated id could collide with the pool and +blackhole traffic as a MAC loop. This class is selected by the ``GNS3_IOL_RUNNER=1`` environment marker. """ @@ -84,6 +85,17 @@ class IOLDockerVM(VendorDockerVM): # The runner launches IOL with a fixed 256KB nvram (-n 256) _IOL_NVRAM_SIZE_KB = 256 + # Payload-delivered state. Deliberately NOT initialized in + # _parse_vendor_environment(): create() re-runs that parser on every + # (re)create (a stop removes the container, so every start recreates it) + # to pick up environment changes — resetting these there would lose the + # controller-allocated application id (MACs would flip to the fallback + # hash, colliding with the allocation pool) and any pending + # startup-config delivered by a PUT. + _application_id = None + _startup_config_content = None + _startup_config_dirty = False + def _parse_vendor_environment(self): super()._parse_vendor_environment() @@ -106,28 +118,17 @@ class IOLDockerVM(VendorDockerVM): except ValueError: pass - # Application ID allocated by the controller (upper half of the id - # space, disjoint from IOU's); None until the create payload carries it. - self._application_id = None - - # Startup-config content materialized by the controller from the file - # referenced by GNS3_IOL_STARTUP_CONFIG. Applied to the NVRAM at start - # time (not in the setter: the application id may not be final yet - # when the create payload is applied field by field). - self._startup_config_content = None - self._startup_config_dirty = False - @property def application_id(self) -> int: """ IOL application ID: drives interface MACs (aabb.cc{app}{iface}) and - the NVRAM file name. Falls back to a stable per-node value in the - IOL Docker half of the id space when no allocation is available - (raw compute API use, topologies created before allocation existed). + the NVRAM file name. Allocated by the controller from the IOL Docker + half of the id space (disjoint from IOU's) — there is deliberately + no fallback: an id derived any other way could silently collide with + an allocation and blackhole traffic as a MAC loop. Starting a node + without one raises (see _prepare_iol_runtime). """ - if self._application_id is None: - return 512 + int(self.id.replace("-", ""), 16) % 511 return self._application_id @application_id.setter @@ -197,7 +198,7 @@ class IOLDockerVM(VendorDockerVM): start pushes the new content with the already-updated name. """ - if not self._startup_config_dirty: + if not self._startup_config_dirty and self._application_id is not None: nvram_file = self._iol_nvram_file() if os.path.exists(nvram_file): try: @@ -302,6 +303,15 @@ class IOLDockerVM(VendorDockerVM): except DockerHttp404Error: state = "stopped" + if self._application_id is None: + raise DockerError( + f"IOL container '{self._name}' has no application ID: nodes must be " + "created through the controller (which allocates one from the pool " + "shared with IOU), or created with an explicit application_id " + "(512-1022) on the compute API. Without a coordinated ID two nodes " + "would share MACs and drop each other's frames as loops." + ) + os.makedirs(os.path.join(self.working_dir, "tmp", "run"), exist_ok=True) self._write_iol_config() diff --git a/tests/compute/docker/test_iol_docker_vm.py b/tests/compute/docker/test_iol_docker_vm.py index a5022eea2..2bfc4f4ec 100644 --- a/tests/compute/docker/test_iol_docker_vm.py +++ b/tests/compute/docker/test_iol_docker_vm.py @@ -81,6 +81,8 @@ def _make_vm(compute_project, manager, environment="GNS3_IOL_RUNNER=1", extra_volumes=extra_volumes or [], adapters=adapters, ) vm._cid = "e90e34656842" + # mirrors the controller flow, which always delivers an allocated id + vm.application_id = 700 return vm @@ -263,7 +265,7 @@ async def test_start_writes_iol_config(compute_project, manager): assert config["binary"] == "/binary.iol" assert config["num-eth"] == 16 # 4 adapters, each a 4-port unit assert config["num-serial"] == 0 - assert config["local-app"] == 512 + int(vm.id.replace("-", ""), 16) % 511 + assert config["local-app"] == 700 assert config["remote-app"] == 1023 assert config["memory"] == 2048 assert config["user-id"] == os.getuid() @@ -271,30 +273,33 @@ async def test_start_writes_iol_config(compute_project, manager): assert vm.status == "started" -def test_local_app_is_distinct_per_node(compute_project, manager): - # IOL derives interface MACs from the app ID: two nodes sharing one would - # drop each other's frames as MAC loops, so IDs must differ per node. - vm1 = _make_vm(compute_project, manager) - vm2 = _make_vm(compute_project, manager) - assert vm1.application_id != vm2.application_id - for vm in (vm1, vm2): - assert 512 <= vm.application_id <= 1022 +@pytest.mark.asyncio +async def test_missing_application_id_is_an_actionable_error(compute_project, manager): + # There is no fallback id on purpose: an uncoordinated one could collide + # with a pool allocation and blackhole traffic as a MAC loop. Nodes come + # through the controller, which always allocates. + vm = _make_vm(compute_project, manager) + vm._application_id = None + vm._get_container_state = AsyncioMagicMock(return_value="stopped") + + with pytest.raises(DockerError, match="no application ID"): + await vm._prepare_iol_runtime() @pytest.mark.asyncio -async def test_allocated_application_id_overrides_fallback(compute_project, manager): +async def test_allocated_application_id_used(compute_project, manager): # The controller allocates the application ID (upper half of the id - # space); the node-derived hash is only a fallback without one. + # space, disjoint from IOU's); the node uses it verbatim. vm = _make_vm(compute_project, manager) - vm.application_id = 700 - assert vm.application_id == 700 + vm.application_id = 701 + assert vm.application_id == 701 _mock_start(vm) with patch("gns3server.compute.docker.Docker.install_busybox"): with asyncio_patch("gns3server.compute.docker.Docker.query"): await vm.start() with open(os.path.join(vm.working_dir, "config", "iol-config.json")) as f: config = json.load(f) - assert config["local-app"] == 700 + assert config["local-app"] == 701 @pytest.mark.asyncio @@ -734,3 +739,22 @@ def test_asdict_exposes_startup_config_content(compute_project, manager): assert vm.asdict()["startup_config_content"] is None vm.startup_config_content = "hostname RouterOne" assert vm.asdict()["startup_config_content"] == "hostname RouterOne" + + +def test_reparse_on_recreate_keeps_payload_state(compute_project, manager): + # create() re-runs _parse_vendor_environment on every (re)create (a stop + # removes the container, so every start recreates it): payload-delivered + # state must survive the re-parse. Losing the allocated application id + # would flip MACs to the fallback hash (colliding with the allocation + # pool); losing the startup-config would drop pending PUT edits. + vm = _make_vm(compute_project, manager) + vm.application_id = 700 + vm.startup_config_content = "hostname RouterOne" + + vm._parse_vendor_environment() + + assert vm.application_id == 700 + assert vm.startup_config_content == "hostname RouterOne" + assert vm._startup_config_dirty is True + # environment-derived state still re-derives + assert vm._gns3_init is False and vm._unix_socket_nio is True