From 6806c1d58ac5505688174ee78893d9ee3a0017be Mon Sep 17 00:00:00 2001 From: Stefan Agner Date: Tue, 3 Feb 2026 13:36:24 +0100 Subject: [PATCH] Fix Docker exec exit code handling by using detach=False (#6520) * Fix Docker exec exit code handling by using detach=False When executing commands inside containers using `container_run_inside()`, the exec metadata did not contain a valid exit code because `detach=True` starts the exec in the background and returns immediately before completion. Root cause: With `detach=True`, Docker's exec start() returns an awaitable that yields output bytes. However, the await only waits for the HTTP/REST call to complete, NOT for the actual exec command to finish. The command continues running in the background after the HTTP response is received. Calling `inspect()` immediately after returns `ExitCode: None` because the exec hasn't completed yet. Solution: Use `detach=False` which returns a Stream object that: - Automatically waits for exec completion by reading from the stream - Provides actual command output (not just empty bytes) - Makes exit code immediately available after stream closes - No polling needed Changes: - Switch from `detach=True` to `detach=False` in container_run_inside() - Read output from stream using async context manager - Add defensive validation to ensure ExitCode is never None - Update tests to mock the Stream interface using AsyncMock - Add debug log showing exit code after command execution Fixes #6518 Co-Authored-By: Claude Sonnet 4.5 * Address review feedback --------- Co-authored-by: Claude Sonnet 4.5 --- supervisor/addons/addon.py | 6 +++-- supervisor/docker/manager.py | 31 ++++++++++++++++++++--- tests/conftest.py | 49 ++++++++++++++++++++++++++++++++++-- 3 files changed, 79 insertions(+), 7 deletions(-) diff --git a/supervisor/addons/addon.py b/supervisor/addons/addon.py index c7c7944a1..b610a480a 100644 --- a/supervisor/addons/addon.py +++ b/supervisor/addons/addon.py @@ -61,6 +61,7 @@ from ..const import ( from ..coresys import CoreSys from ..docker.addon import DockerAddon from ..docker.const import ContainerState +from ..docker.manager import ExecReturn from ..docker.monitor import DockerContainerStateEvent from ..docker.stats import DockerStats from ..exceptions import ( @@ -1227,10 +1228,11 @@ class Addon(AddonModel): async def _backup_command(self, command: str) -> None: try: - command_return = await self.instance.run_inside(command) + command_return: ExecReturn = await self.instance.run_inside(command) if command_return.exit_code != 0: _LOGGER.debug( - "Pre-/Post backup command failed with: %s", command_return.output + "Pre-/Post backup command failed with: %s", + command_return.output.decode("utf-8", errors="replace"), ) raise AddonPrePostBackupCommandReturnedError( _LOGGER.error, addon=self.slug, exit_code=command_return.exit_code diff --git a/supervisor/docker/manager.py b/supervisor/docker/manager.py index 298b9cfb2..d6af2867d 100644 --- a/supervisor/docker/manager.py +++ b/supervisor/docker/manager.py @@ -18,6 +18,7 @@ from typing import Any, Final, Self, cast import aiodocker from aiodocker.containers import DockerContainer, DockerContainers from aiodocker.images import DockerImages +from aiodocker.stream import Stream from aiodocker.types import JSONObject from aiohttp import ClientTimeout, UnixConnector import attr @@ -967,17 +968,41 @@ class DockerAPI(CoreSysAttributes): f"Can't get container {name} to run command: {err!s}" ) from err - # Execute + # Execute - use detach=False to wait for completion and capture output try: docker_exec = await docker_container.exec(command) - output = await docker_exec.start(detach=True) + # start() with detach=False returns a Stream object (not a coroutine) + stream: Stream = docker_exec.start(detach=False) + + # Read all output from the stream until exec completes + output_parts = [] + async with stream: + while msg := await stream.read_out(): + output_parts.append(msg.data) + + # Combine all output parts + output = b"".join(output_parts) + + # After stream closes, exec is complete and exit code should be available exec_metadata = await docker_exec.inspect() + + # Validate exit code is present + exit_code = exec_metadata.get("ExitCode") + if exit_code is None: + raise DockerError( + f"Exec command '{command}' in {name} completed but ExitCode is None. " + f"Metadata: {exec_metadata}", + _LOGGER.error, + ) except aiodocker.DockerError as err: raise DockerError( f"Can't run command in container {name}: {err!s}" ) from err - return ExecReturn(exec_metadata["ExitCode"], output) + _LOGGER.debug( + "Exec command '%s' in %s exited with %d", command, name, exit_code + ) + return ExecReturn(exit_code, output) async def remove_image( self, image: str, version: AwesomeVersion, latest: bool = True diff --git a/tests/conftest.py b/tests/conftest.py index 34f825491..7f344356b 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -195,7 +195,9 @@ async def docker() -> DockerAPI: docker_container.log = AsyncMock(return_value=[]) docker_container.exec.return_value = docker_exec = MagicMock(spec=Exec) - docker_exec.start = AsyncMock(return_value=b"") + # start() with detach=False returns a Stream (not async) + # Use return_value instead of side_effect to avoid it being replaced by tests + docker_exec.start.return_value = create_mock_exec_stream(output=b"") docker_exec.inspect.return_value = {"ExitCode": 0} docker_obj.info.logging = "journald" @@ -872,10 +874,53 @@ async def os_available(request: pytest.FixtureRequest) -> None: yield +def create_mock_exec_stream(output: bytes = b"") -> AsyncMock: + """Create a mock stream for exec with detach=False.""" + stream = AsyncMock() + # Set up async context manager + stream.__aenter__.return_value = stream + stream.__aexit__.return_value = None + # Set up read_out to return messages then None (EOF) + if output: + Message = type("Message", (), {"data": output}) + stream.read_out.side_effect = [Message(), None] + else: + stream.read_out.return_value = None + return stream + + @pytest.fixture async def container(docker: DockerAPI) -> DockerContainer: """Mock attrs and status for container on attach.""" - yield docker.containers.get.return_value + container_mock = docker.containers.get.return_value + + # Set up exec mock to return a mock stream + # Note: This must be a regular function, not async, to match aiodocker's start() behavior + def mock_exec_start(detach=False): + if detach: + # Old behavior for detach=True (shouldn't be used anymore) + # Would need to return an awaitable + async def _async_start(): + return b"" + + return _async_start() + # Return mock stream for detach=False (synchronous) + return create_mock_exec_stream(output=b"") + + # Replace the mock's start method with a MagicMock (not AsyncMock) + start_mock = MagicMock(side_effect=mock_exec_start) + container_mock.exec.return_value.start = start_mock + + # Store the original side_effect so tests can override it but we can restore if needed + start_mock._original_side_effect = mock_exec_start + + container_mock.exec.return_value.inspect.return_value = { + "Running": False, + "ExitCode": 0, + "Pid": 12345, + } + + yield container_mock @pytest.fixture