diff --git a/docs/features/docker-exec-console.md b/docs/features/docker-exec-console.md index dd741ff87..b55fe68d0 100644 --- a/docs/features/docker-exec-console.md +++ b/docs/features/docker-exec-console.md @@ -124,11 +124,31 @@ console. Key points: CLI"* otherwise), and `Env: ["TERM=xterm"]` (the TUI library needs a recognised terminal). -3. **while-true wrapper.** The command is wrapped in - `sh -c "while true; do ; done"` so that when the CLI exits (user types - `quit`, or the NOS's own idle timeout logs the session out), a fresh CLI - instance starts in the same pty instead of killing the shared console - session. +3. **No while-true wrapper.** The command runs as `sh -c ""` (no + restart loop). When the CLI exits (`quit`, the NOS's own idle timeout, or a + crash) the exec pty closes, the broadcast task ends, and the next client + connection **recreates** the exec (see *Reconnection*). A `while true` + wrapper would restart the CLI mid-session with no client attached to + answer its startup CPR probe, producing a blank/degraded screen on + reconnect. + +### Reconnection + +The exec is created lazily and **recreated on reconnect if it has died**. +`client_connected_hook` checks `_upstream_alive()` (exec id set, writer open, +broadcast task not done) before each connect: + +- **First connect / dead upstream** → (re)create the exec. Because a client is + now attached, the CLI's startup CPR probe is answered by xterm.js → full + TUI. A half-dead writer is closed first to avoid a socket leak. +- **Live upstream** → reuse the existing exec, just send `Ctrl-L` to redraw + for the new client. + +This is what makes the console survive `quit`, idle timeout, and CLI +crashes: the death is detected (pty EOF ends the broadcast task) and the +next connection spins up a fresh exec with a terminal present. The +`_LazyExecTelnetServer` is extracted to module level specifically so this +reconnect logic is unit-tested. 4. **Hijacked raw-HTTP start.** The exec is started with `POST exec/{eid}/start` sent as a raw HTTP upgrade over the Docker unix @@ -324,9 +344,11 @@ present on the host. issue. **7. "Session has been idle, will logout in 300 seconds" → Connection closed** -- SR Linux's own CLI idle timeout. The while-true wrapper restarts the CLI - automatically, but to keep a permanent session disable the timeout in the - CLI: `enter candidate` → `/system cli idle-timeout disable` → `commit now`. +- SR Linux's own CLI idle timeout logs the CLI out, the exec pty closes, and + the console disconnects. Reopening the console recreates the exec (see + *Reconnection*) and gives a fresh login. To keep a permanent session, + disable the timeout in the CLI: `enter candidate` → + `/system cli idle-timeout disable` → `commit now`. **8. Controller logs `Permission denied` reading files under the node's project directory while the node runs** @@ -400,6 +422,7 @@ present on the host. | Version | Date | Changes | |---------|------|---------| +| 1.4 | 2026-08-13 | Reconnect fix: drop the while-true wrapper (it restarted the CLI with no client to answer CPR → blank screen on reconnect); the exec is now recreated on connect when the upstream has died. `_LazyExecTelnetServer` extracted to module level and unit-tested. | | 1.3 | 2026-08-12 | Document runtime ownership safety (root processes, self-healing daemons, ACL evidence for SR Linux), the boot-ordering caveat (bridge after boot → verify save/stop/start closed loop), and troubleshooting #10. | | 1.2 | 2026-08-12 | `_fix_permissions` rewritten: container-side (as root) on the `/gns3volumes` bind-mount targets instead of host-side — host-side chown cannot touch root-owned files when GNS3 is unprivileged. Dead containers are skipped instead of restarted. | | 1.1 | 2026-08-12 | Refactor: vendor logic extracted from `DockerVM` into `VendorDockerVM` subclass with 4 hook points + class-selection factory. Add SKIP_INIT volume persistence (`_setup_skip_init_volumes` + `_fix_permissions`) and lifecycle comparison. Add troubleshooting entries for idle timeout and permission-denied files. | diff --git a/gns3server/compute/docker/vendor_docker_vm.py b/gns3server/compute/docker/vendor_docker_vm.py index 9d0c683f6..f0410d196 100644 --- a/gns3server/compute/docker/vendor_docker_vm.py +++ b/gns3server/compute/docker/vendor_docker_vm.py @@ -288,127 +288,7 @@ class VendorDockerVM(DockerVM): Command from GNS3_CONSOLE_CMD. """ - command = self._console_cmd or "/bin/sh" - vm = self - manager = self.manager - cid = self._cid - - class _LazyExecTelnetServer(AsyncioTelnetServer): - """Telnet console whose docker exec (pty + command) is created on the - first client connection and then broadcast to all clients.""" - - def __init__(srv): - super().__init__( - reader=None, - writer=None, - binary=True, - echo=False, - naws=True, - window_size_changed_callback=srv._on_naws, - ) - srv._exec_id = None - srv._started = False - srv._lock = asyncio.Lock() - srv._log_name = f"docker_exec console '{vm.name}'" - - async def _on_naws(srv, columns, rows): - if srv._exec_id: - try: - await manager.query( - "POST", - f"exec/{srv._exec_id}/resize", - params={"h": str(rows), "w": str(columns)}, - ) - except DockerError: - pass - - async def run(srv, network_reader, network_writer): - """Catch and log any exception that kills the client session.""" - try: - await super().run(network_reader, network_writer) - except Exception as exc: - log.warning(f"{srv._log_name}: client session terminated: {exc}", exc_info=True) - - async def _create_exec(srv): - # create exec with a pty; run as root (vendor CLIs reject the - # image's default unprivileged user) and export TERM=xterm. - result = await manager.query( - "POST", - f"containers/{cid}/exec", - data={ - "AttachStdin": True, - "AttachStdout": True, - "AttachStderr": True, - "Tty": True, - "User": "root", - "Env": ["TERM=xterm"], - "Cmd": ["sh", "-c", f"while true; do {command}; done"], - }, - ) - srv._exec_id = result["Id"] - log.info(f"{srv._log_name}: exec created ({srv._exec_id})") - - # start the exec via a hijacked raw HTTP request on the Docker - # unix socket; with Tty:true the response body is a raw - # bidirectional pty byte stream (no multiplexing). - reader, writer = await asyncio.open_unix_connection(manager._server_url) - body = json.dumps({"Detach": False, "Tty": True}) - request = ( - f"POST /v{manager._api_version}/exec/{srv._exec_id}/start HTTP/1.1\r\n" - "Host: docker\r\n" - "Connection: Upgrade\r\n" - "Upgrade: tcp\r\n" - "Content-Type: application/json\r\n" - f"Content-Length: {len(body)}\r\n\r\n{body}" - ).encode() - writer.write(request) - await writer.drain() - try: - headers = await asyncio.wait_for(reader.readuntil(b"\r\n\r\n"), timeout=5) - except (asyncio.IncompleteReadError, asyncio.TimeoutError) as e: - writer.close() - raise DockerError(f"Docker exec start failed: {e}") - status_line = headers.split(b"\r\n", 1)[0] - log.info(f"{srv._log_name}: hijacked start -> {status_line.decode(errors='ignore')}") - if b" 101 " not in status_line and b" 200 " not in status_line: - writer.close() - raise DockerError(f"Docker exec start rejected: {status_line.decode(errors='ignore')}") - - # wire the exec stream as this server's upstream and start the - # broadcast task. AsyncioTelnetServer.start() only starts the - # broadcast when a reader is set at construction time, so with a - # lazy upstream we start it manually here. - srv._reader = reader - srv._writer = writer - vm._console_exec_writer = writer # for stop() cleanup - srv._broadcast_task = asyncio.create_task(srv._broadcast_from_upstream()) - log.info(f"{srv._log_name}: broadcast task started, upstream wired, ready") - - async def client_connected_hook(srv): - await super().client_connected_hook() - log.info(f"{srv._log_name}: client connected, lazy_started={srv._started}") - async with srv._lock: - if not srv._started: - try: - await srv._create_exec() - except Exception as exc: - log.warning(f"{srv._log_name}: failed to create exec: {exc}", exc_info=True) - raise - srv._started = True - try: - await srv._on_naws(80, 24) # initial size before NAWS - except Exception: - pass - # ask the TUI to (re)draw for the client that just connected. - if srv._writer: - try: - srv._writer.write(b"\x0c") # Ctrl-L -> TUI redraws - await srv._writer.drain() - except Exception as exc: - log.warning(f"{srv._log_name}: Ctrl-L write failed: {exc}") - log.info(f"{srv._log_name}: client_connected_hook done") - - telnet = _LazyExecTelnetServer() + telnet = _LazyExecTelnetServer(self, self.manager, self._cid, self._console_cmd or "/bin/sh") try: self._telnet_servers.append( await telnet.start(self._manager.port_manager.console_host, self.console) @@ -418,3 +298,153 @@ class VendorDockerVM(DockerVM): f"Could not start console server on socket {self._manager.port_manager.console_host}:{self.console}: {e}" ) log.debug(f"Docker container '{self.name}' started docker_exec console (lazy) on {self.console}") + + +class _LazyExecTelnetServer(AsyncioTelnetServer): + """Telnet console whose docker exec (pty + command) is created lazily on + the first client connection and recreated if the upstream dies. + + Extracted to module level (rather than a closure inside + _start_docker_exec_console) so the reconnect/recreate logic is unit-testable. + + Lifecycle: the exec is created on the first connect. When the CLI exits + (quit / idle timeout / crash) the exec pty closes, the broadcast task ends, + and the *next* client connection recreates the exec — with a terminal + attached, so the CLI's startup CPR probe is answered. No ``while true`` + wrapper: that would restart the CLI mid-session with no client to answer + CPR, producing a blank/degraded screen on reconnect. + """ + + def __init__(self, vm, manager, cid, command): + super().__init__( + reader=None, + writer=None, + binary=True, + echo=False, + naws=True, + window_size_changed_callback=self._on_naws, + ) + self._vm = vm + self._manager = manager + self._cid = cid + self._command = command + self._exec_id = None + self._broadcast_task = None + self._lock = asyncio.Lock() + self._log_name = f"docker_exec console '{vm.name}'" + + def _upstream_alive(self): + """True if the exec pty + broadcast task are still pumping.""" + if self._exec_id is None or self._writer is None: + return False + if self._writer.is_closing(): + return False + if self._broadcast_task is not None and self._broadcast_task.done(): + return False + return True + + async def _on_naws(self, columns, rows): + if self._exec_id: + try: + await self._manager.query( + "POST", + f"exec/{self._exec_id}/resize", + params={"h": str(rows), "w": str(columns)}, + ) + except DockerError: + pass + + async def run(self, network_reader, network_writer): + """Catch and log any exception that kills the client session.""" + try: + await super().run(network_reader, network_writer) + except Exception as exc: + log.warning(f"{self._log_name}: client session terminated: {exc}", exc_info=True) + + async def _create_exec(self): + # create exec with a pty; run as root (vendor CLIs reject the image's + # default unprivileged user) and export TERM=xterm. + result = await self._manager.query( + "POST", + f"containers/{self._cid}/exec", + data={ + "AttachStdin": True, + "AttachStdout": True, + "AttachStderr": True, + "Tty": True, + "User": "root", + "Env": ["TERM=xterm"], + "Cmd": ["sh", "-c", self._command], + }, + ) + self._exec_id = result["Id"] + log.info(f"{self._log_name}: exec created ({self._exec_id})") + + # start the exec via a hijacked raw HTTP request on the Docker unix + # socket; with Tty:true the response body is a raw bidirectional pty + # byte stream (no multiplexing). + reader, writer = await asyncio.open_unix_connection(self._manager._server_url) + body = json.dumps({"Detach": False, "Tty": True}) + request = ( + f"POST /v{self._manager._api_version}/exec/{self._exec_id}/start HTTP/1.1\r\n" + "Host: docker\r\n" + "Connection: Upgrade\r\n" + "Upgrade: tcp\r\n" + "Content-Type: application/json\r\n" + f"Content-Length: {len(body)}\r\n\r\n{body}" + ).encode() + writer.write(request) + await writer.drain() + try: + headers = await asyncio.wait_for(reader.readuntil(b"\r\n\r\n"), timeout=5) + except (asyncio.IncompleteReadError, asyncio.TimeoutError) as e: + writer.close() + raise DockerError(f"Docker exec start failed: {e}") + status_line = headers.split(b"\r\n", 1)[0] + log.info(f"{self._log_name}: hijacked start -> {status_line.decode(errors='ignore')}") + if b" 101 " not in status_line and b" 200 " not in status_line: + writer.close() + raise DockerError(f"Docker exec start rejected: {status_line.decode(errors='ignore')}") + + # wire the exec stream as this server's upstream and start the broadcast + # task. AsyncioTelnetServer.start() only starts the broadcast when a + # reader is set at construction time, so with a lazy upstream we start + # it manually here. + self._reader = reader + self._writer = writer + self._vm._console_exec_writer = writer # for stop() cleanup + self._broadcast_task = asyncio.create_task(self._broadcast_from_upstream()) + log.info(f"{self._log_name}: broadcast task started, upstream wired, ready") + + async def client_connected_hook(self): + await super().client_connected_hook() + async with self._lock: + # (Re)create the exec if it was never created or has died (CLI + # exited → pty EOF → broadcast task ended). Doing this with a + # client attached means the CLI's startup CPR probe is answered by + # a real terminal. + if not self._upstream_alive(): + log.info(f"{self._log_name}: client connected, (re)creating exec") + # close a half-dead writer before replacing it + if self._writer is not None and not self._writer.is_closing(): + with contextlib.suppress(Exception): + self._writer.close() + try: + await self._create_exec() + except Exception as exc: + log.warning(f"{self._log_name}: failed to create exec: {exc}", exc_info=True) + raise + try: + await self._on_naws(80, 24) # initial size before NAWS + except Exception: + pass + else: + log.info(f"{self._log_name}: client connected, reusing live exec") + # ask the TUI to (re)draw for the client that just connected. + if self._writer: + try: + self._writer.write(b"\x0c") # Ctrl-L -> TUI redraws + await self._writer.drain() + except Exception as exc: + log.warning(f"{self._log_name}: Ctrl-L write failed: {exc}") + log.info(f"{self._log_name}: client_connected_hook done") diff --git a/tests/compute/docker/test_vendor_docker_vm.py b/tests/compute/docker/test_vendor_docker_vm.py index ba444bb79..ce81e5119 100644 --- a/tests/compute/docker/test_vendor_docker_vm.py +++ b/tests/compute/docker/test_vendor_docker_vm.py @@ -40,7 +40,7 @@ from tests.utils import asyncio_patch, AsyncioMagicMock from gns3server.compute.docker import Docker from gns3server.compute.docker.docker_vm import DockerVM -from gns3server.compute.docker.vendor_docker_vm import VendorDockerVM +from gns3server.compute.docker.vendor_docker_vm import VendorDockerVM, _LazyExecTelnetServer from gns3server.compute.docker.docker_error import DockerError, DockerHttp404Error @@ -457,3 +457,158 @@ def test_cleanup_console_resources_no_writer(compute_project, manager): vm._console_exec_writer = None # must not raise vm._cleanup_console_resources() + + +# --------------------------------------------------------------------------- +# _LazyExecTelnetServer — upstream aliveness + reconnect/recreate logic +# --------------------------------------------------------------------------- + +def _make_lazy_server(compute_project, manager): + """Build a _LazyExecTelnetServer with _create_exec mocked out (no docker).""" + vm = _make_vm(compute_project, manager, environment="GNS3_CONSOLE_CMD=/opt/srlinux/bin/sr_cli") + srv = _LazyExecTelnetServer(vm, manager, "e90e34656842", "/opt/srlinux/bin/sr_cli") + srv._create_exec = AsyncioMagicMock() + srv._on_naws = AsyncioMagicMock() + return srv + + +def _live_writer(): + """A writer mock that reports as open (not closing).""" + w = MagicMock() + w.is_closing.return_value = False + return w + + +def _dead_writer(): + """A writer mock that reports as closing (pty closed).""" + w = MagicMock() + w.is_closing.return_value = True + return w + + +def test_upstream_alive_never_created(compute_project, manager): + + srv = _make_lazy_server(compute_project, manager) + assert srv._upstream_alive() is False + + +def test_upstream_alive_writer_closing(compute_project, manager): + + srv = _make_lazy_server(compute_project, manager) + srv._exec_id = "abc" + srv._writer = _dead_writer() + srv._broadcast_task = MagicMock() + srv._broadcast_task.done.return_value = False + assert srv._upstream_alive() is False + + +def test_upstream_alive_broadcast_done(compute_project, manager): + + srv = _make_lazy_server(compute_project, manager) + srv._exec_id = "abc" + srv._writer = _live_writer() + srv._broadcast_task = MagicMock() + srv._broadcast_task.done.return_value = True # CLI exited → EOF → task ended + assert srv._upstream_alive() is False + + +def test_upstream_alive_live(compute_project, manager): + + srv = _make_lazy_server(compute_project, manager) + srv._exec_id = "abc" + srv._writer = _live_writer() + srv._broadcast_task = MagicMock() + srv._broadcast_task.done.return_value = False + assert srv._upstream_alive() is True + + +@pytest.mark.asyncio +async def test_first_connect_creates_exec(compute_project, manager): + + srv = _make_lazy_server(compute_project, manager) + # never created → must create + await srv.client_connected_hook() + srv._create_exec.assert_called_once() + + +@pytest.mark.asyncio +async def test_reconnect_live_exec_not_recreated(compute_project, manager): + """Reconnecting while the exec is alive must NOT recreate it.""" + + srv = _make_lazy_server(compute_project, manager) + srv._exec_id = "abc" + srv._writer = _live_writer() + srv._broadcast_task = MagicMock() + srv._broadcast_task.done.return_value = False + + await srv.client_connected_hook() + srv._create_exec.assert_not_called() + # Ctrl-L redraw is still sent to the live writer + srv._writer.write.assert_any_call(b"\x0c") + + +@pytest.mark.asyncio +async def test_reconnect_after_death_recreates_exec(compute_project, manager): + """The core reconnect fix: after the CLI exits (broadcast task done), + the next client connection recreates the exec so CPR gets answered.""" + + srv = _make_lazy_server(compute_project, manager) + # simulate a dead upstream: exec existed, but the pty closed / task ended + srv._exec_id = "old-exec" + srv._writer = _dead_writer() + srv._broadcast_task = MagicMock() + srv._broadcast_task.done.return_value = True + + await srv.client_connected_hook() + srv._create_exec.assert_called_once() + + +@pytest.mark.asyncio +async def test_reconnect_closes_half_dead_writer(compute_project, manager): + """If the writer is still open but the broadcast task died, the old writer + must be closed before a new exec is created (no socket leak).""" + + srv = _make_lazy_server(compute_project, manager) + srv._exec_id = "old-exec" + srv._writer = _live_writer() # still open, but... + srv._broadcast_task = MagicMock() + srv._broadcast_task.done.return_value = True # ...task ended + + await srv.client_connected_hook() + srv._writer.close.assert_called_once() + srv._create_exec.assert_called_once() + + +@pytest.mark.asyncio +async def test_create_exec_cmd_has_no_while_true(compute_project, manager): + """The command must NOT be wrapped in a while-true loop (regression guard: + while-true restarts the CLI with no client to answer CPR → blank screen).""" + + vm = _make_vm(compute_project, manager) + manager._server_url = "/var/run/docker.sock" + manager._api_version = "1.40" + srv = _LazyExecTelnetServer(vm, manager, "e90e34656842", "/opt/srlinux/bin/sr_cli") + + captured = {} + + async def fake_query(method, path, data=None, **kw): + captured["data"] = data + return {"Id": "exec123"} + + manager.query = fake_query + + with patch("asyncio.open_unix_connection") as mock_open: + reader = MagicMock() + reader.readuntil = AsyncioMagicMock(return_value=b"HTTP/1.1 101 Upgraded\r\n\r\n") + writer = MagicMock() + writer.is_closing.return_value = False + mock_open.return_value = (reader, writer) + await srv._create_exec() + + cmd = captured["data"]["Cmd"] + assert cmd == ["sh", "-c", "/opt/srlinux/bin/sr_cli"] + assert "while true" not in cmd[2] + # must run as root with a pty and TERM + assert captured["data"]["User"] == "root" + assert captured["data"]["Tty"] is True + assert "TERM=xterm" in captured["data"]["Env"]