From 110e041e56ceb52c143247a10549da1bb5ca12ef Mon Sep 17 00:00:00 2001 From: YueGuobin Date: Sat, 15 Aug 2026 01:25:50 +0800 Subject: [PATCH] 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