From a5c3781f9d84b8b25244a9c58ef486ecb965d309 Mon Sep 17 00:00:00 2001 From: Mike Degatano Date: Fri, 30 Jan 2026 09:34:12 -0500 Subject: [PATCH] Migrate network interactions to aiodocker (#6505) --- supervisor/docker/manager.py | 70 ++++--- supervisor/docker/network.py | 343 ++++++++++++++++---------------- supervisor/docker/supervisor.py | 3 +- tests/conftest.py | 44 +++- tests/docker/test_manager.py | 35 ++-- tests/docker/test_network.py | 326 ++++++++++++++---------------- 6 files changed, 407 insertions(+), 414 deletions(-) diff --git a/supervisor/docker/manager.py b/supervisor/docker/manager.py index d201acac6..3691465db 100644 --- a/supervisor/docker/manager.py +++ b/supervisor/docker/manager.py @@ -13,7 +13,7 @@ import logging import os from pathlib import Path, PurePath import re -from typing import Any, Final, Literal, Self, cast +from typing import Any, Final, Self, cast import aiodocker from aiodocker.containers import DockerContainer, DockerContainers @@ -23,10 +23,8 @@ from aiohttp import ClientTimeout, UnixConnector import attr from awesomeversion import AwesomeVersion, AwesomeVersionCompareException from docker import errors as docker_errors -from docker.api.client import APIClient from docker.client import DockerClient from docker.models.containers import Container -from docker.models.networks import Network from docker.types.daemon import CancellableStream import requests @@ -291,7 +289,7 @@ class DockerAPI(CoreSysAttributes): ) self._info = await DockerInfo.new(self.dockerpy.info()) await self.config.read_data() - self._network = await DockerNetwork(self.dockerpy).post_init( + self._network = await DockerNetwork(self.docker).post_init( self.config.enable_ipv6, self.config.mtu ) return self @@ -320,11 +318,6 @@ class DockerAPI(CoreSysAttributes): """Return API containers.""" return self.docker.containers - @property - def api(self) -> APIClient: - """Return API containers.""" - return self.dockerpy.api - @property def info(self) -> DockerInfo: """Return local docker info.""" @@ -553,13 +546,13 @@ class DockerAPI(CoreSysAttributes): if cidfile_path: await self.sys_run_in_executor(setup_cidfile, cidfile_path) - # Setup network - def setup_network(network_mode: Literal["host"] | None) -> None: + # Setup network for host and default modes + if not networking_config and network_mode in ("host", None): # Attach network if not network_mode: alias = [hostname] if hostname else None try: - self.network.attach_container( + await self.network.attach_container( container.id, name, alias=alias, ipv4=ipv4 ) except DockerError: @@ -568,21 +561,32 @@ class DockerAPI(CoreSysAttributes): ) else: with suppress(DockerError): - self.network.detach_default_bridge(container.id, name) + await self.network.detach_default_bridge(container.id, name) else: - host_network: Network = self.dockerpy.networks.get(DOCKER_NETWORK_HOST) + try: + host_network = await self.docker.networks.get(DOCKER_NETWORK_HOST) + host_network_meta = await host_network.show() + except aiodocker.DockerError as err: + raise DockerError( + f"Can't get host network information from Docker: {err!s}", + _LOGGER.error, + ) from err # Check if container is register on host # https://github.com/moby/moby/issues/23302 if name and name in ( val.get("Name") - for val in host_network.attrs.get("Containers", {}).values() + for val in host_network_meta.get("Containers", {}).values() ): - with suppress(docker_errors.NotFound): - host_network.disconnect(name, force=True) - - if not networking_config and network_mode in ("host", None): - await self.sys_run_in_executor(setup_network, network_mode) + try: + await host_network.disconnect( + {"Container": name, "Force": True} + ) + except aiodocker.DockerError as err: + if err.status != HTTPStatus.NOT_FOUND: + raise DockerError( + f"Can't disconnect container {name} from host network: {err!s}" + ) from err # Run container try: @@ -740,13 +744,13 @@ class DockerAPI(CoreSysAttributes): _LOGGER.info("Fix stale container on hassio network") try: await self.prune_networks(DOCKER_NETWORK) - except docker_errors.APIError as err: + except aiodocker.DockerError as err: _LOGGER.warning("Error for networks hassio prune: %s", err) _LOGGER.info("Fix stale container on host network") try: await self.prune_networks(DOCKER_NETWORK_HOST) - except docker_errors.APIError as err: + except aiodocker.DockerError as err: _LOGGER.warning("Error for networks host prune: %s", err) async def prune_networks(self, network_name: str) -> None: @@ -754,12 +758,10 @@ class DockerAPI(CoreSysAttributes): Fix: https://github.com/moby/moby/issues/23302 """ - network: Network = await self.sys_run_in_executor( - self.dockerpy.networks.get, network_name - ) - corrupt_containers: list[str] = [] + network = await self.docker.networks.get(network_name) + network_meta = await network.show() - for cid, data in network.attrs.get("Containers", {}).items(): + for cid, data in network_meta.get("Containers", {}).items(): try: await self.containers.get(cid) continue @@ -776,14 +778,10 @@ class DockerAPI(CoreSysAttributes): _LOGGER.debug( "Docker network %s is corrupt on container: %s", network_name, cid ) - corrupt_containers.append(data.get("Name", cid)) - - def disconnect_corrupt_containers(): - for name in corrupt_containers: - with suppress(docker_errors.DockerException, requests.RequestException): - network.disconnect(name, force=True) - - await self.sys_run_in_executor(disconnect_corrupt_containers) + with suppress(aiodocker.DockerError): + await network.disconnect( + {"Container": data.get("Name", cid), "Force": True} + ) async def container_is_initialized( self, name: str, image: str, version: AwesomeVersion @@ -1020,7 +1018,7 @@ class DockerAPI(CoreSysAttributes): def export_image(self, image: str, version: AwesomeVersion, tar_file: Path) -> None: """Export current images into a tar file.""" try: - docker_image = self.api.get_image(f"{image}:{version}") + docker_image = self.dockerpy.api.get_image(f"{image}:{version}") except (docker_errors.DockerException, requests.RequestException) as err: raise DockerError( f"Can't fetch image {image}: {err}", _LOGGER.error diff --git a/supervisor/docker/network.py b/supervisor/docker/network.py index fc7cd6b99..391981a1a 100644 --- a/supervisor/docker/network.py +++ b/supervisor/docker/network.py @@ -1,20 +1,18 @@ """Internal network manager for Supervisor.""" -import asyncio from contextlib import suppress +from http import HTTPStatus from ipaddress import IPv4Address import logging -from typing import Self, cast +from typing import Any, Self, cast -import docker -from docker.models.networks import Network -import requests +import aiodocker +from aiodocker.networks import DockerNetwork as AiodockerNetwork from ..const import ( ATTR_AUDIO, ATTR_CLI, ATTR_DNS, - ATTR_ENABLE_IPV6, ATTR_OBSERVER, ATTR_SUPERVISOR, DOCKER_IPV4_NETWORK_MASK, @@ -31,44 +29,112 @@ from ..exceptions import DockerError _LOGGER: logging.Logger = logging.getLogger(__name__) DOCKER_ENABLEIPV6 = "EnableIPv6" -DOCKER_NETWORK_PARAMS = { - "name": DOCKER_NETWORK, - "driver": DOCKER_NETWORK_DRIVER, - "ipam": docker.types.IPAMConfig( - pool_configs=[ - docker.types.IPAMPool(subnet=str(DOCKER_IPV6_NETWORK_MASK)), - docker.types.IPAMPool( - subnet=str(DOCKER_IPV4_NETWORK_MASK), - gateway=str(DOCKER_IPV4_NETWORK_MASK[1]), - iprange=str(DOCKER_IPV4_NETWORK_RANGE), - ), - ] - ), - ATTR_ENABLE_IPV6: True, - "options": {"com.docker.network.bridge.name": DOCKER_NETWORK}, -} - +DOCKER_OPTIONS = "Options" DOCKER_ENABLE_IPV6_DEFAULT = True +DOCKER_NETWORK_PARAMS = { + "Name": DOCKER_NETWORK, + "Driver": DOCKER_NETWORK_DRIVER, + "IPAM": { + "Driver": "default", + "Config": [ + { + "Subnet": str(DOCKER_IPV6_NETWORK_MASK), + }, + { + "Subnet": str(DOCKER_IPV4_NETWORK_MASK), + "Gateway": str(DOCKER_IPV4_NETWORK_MASK[1]), + "IPRange": str(DOCKER_IPV4_NETWORK_RANGE), + }, + ], + }, + DOCKER_ENABLEIPV6: DOCKER_ENABLE_IPV6_DEFAULT, + DOCKER_OPTIONS: {"com.docker.network.bridge.name": DOCKER_NETWORK}, +} class DockerNetwork: - """Internal Supervisor Network. + """Internal Supervisor Network.""" - This class is not AsyncIO safe! - """ - - def __init__(self, docker_client: docker.DockerClient): + def __init__(self, docker_client: aiodocker.Docker): """Initialize internal Supervisor network.""" - self.docker: docker.DockerClient = docker_client - self._network: Network + self.docker: aiodocker.Docker = docker_client + self._network: AiodockerNetwork | None = None + self._network_meta: dict[str, Any] | None = None async def post_init( self, enable_ipv6: bool | None = None, mtu: int | None = None ) -> Self: """Post init actions that must be done in event loop.""" - self._network = await asyncio.get_running_loop().run_in_executor( - None, self._get_network, enable_ipv6, mtu + try: + self._network = network = await self.docker.networks.get(DOCKER_NETWORK) + except aiodocker.DockerError as err: + # If network was not found, create it instead. Can skip further checks since it's new + if err.status == HTTPStatus.NOT_FOUND: + await self._create_supervisor_network(enable_ipv6, mtu) + return self + + raise DockerError( + f"Could not get network from Docker: {err!s}", _LOGGER.error + ) from err + + # Cache metadata for network + await self.reload() + current_ipv6: bool = self.network_meta.get(DOCKER_ENABLEIPV6, False) + current_mtu_str: str | None = self.network_meta.get(DOCKER_OPTIONS, {}).get( + "com.docker.network.driver.mtu" ) + current_mtu = int(current_mtu_str) if current_mtu_str is not None else None + + # Check if we have explicitly provided settings that differ from what is set + changes = [] + if enable_ipv6 is not None and current_ipv6 != enable_ipv6: + changes.append("IPv4/IPv6 Dual-Stack" if enable_ipv6 else "IPv4-Only") + if mtu is not None and current_mtu != mtu: + changes.append(f"MTU {mtu}") + + if not changes: + return self + + _LOGGER.info("Migrating Supervisor network to %s", ", ".join(changes)) + + # System is considered running if any containers besides Supervisor and Observer are found + # A reboot is required then, we won't disconnect those containers to remake network + containers: dict[str, dict[str, Any]] = self.network_meta.get("Containers", {}) + system_running = containers and any( + container.get("Name") not in (OBSERVER_DOCKER_NAME, SUPERVISOR_DOCKER_NAME) + for container in containers.values() + ) + if system_running: + _LOGGER.warning( + "System appears to be running, not applying Supervisor network change. " + "Reboot your system to apply the change." + ) + return self + + # Disconnect all containers in the network + for c_id, meta in containers.items(): + try: + await network.disconnect({"Container": c_id, "Force": True}) + except aiodocker.DockerError: + _LOGGER.warning( + "Cannot apply Supervisor network changes because container %s " + "could not be disconnected. Reboot your system to apply change.", + meta.get("Name"), + ) + return self + + # Remove the network + try: + await network.delete() + except aiodocker.DockerError: + _LOGGER.warning( + "Cannot apply Supervisor network changes because Supervisor network " + "could not be removed and recreated. Reboot your system to apply change." + ) + return self + + # Recreate it with correct settings + await self._create_supervisor_network(enable_ipv6, mtu) return self @property @@ -77,14 +143,23 @@ class DockerNetwork: return DOCKER_NETWORK @property - def network(self) -> Network: + def network(self) -> AiodockerNetwork: """Return docker network.""" + if not self._network: + raise RuntimeError("Network not set!") return self._network @property - def containers(self) -> list[str]: - """Return of connected containers from network.""" - return list(self.network.attrs.get("Containers", {}).keys()) + def network_meta(self) -> dict[str, Any]: + """Return docker network metadata.""" + if not self._network_meta: + raise RuntimeError("Network metadata not set!") + return self._network_meta + + @property + def containers(self) -> dict[str, dict[str, Any]]: + """Return metadata of connected containers to network.""" + return self.network_meta.get("Containers", {}) @property def gateway(self) -> IPv4Address: @@ -116,94 +191,37 @@ class DockerNetwork: """Return observer of the network.""" return DOCKER_IPV4_NETWORK_MASK[6] - def _get_network( + async def _create_supervisor_network( self, enable_ipv6: bool | None = None, mtu: int | None = None - ) -> Network: - """Get supervisor network.""" - try: - if network := self.docker.networks.get(DOCKER_NETWORK): - current_ipv6 = network.attrs.get(DOCKER_ENABLEIPV6, False) - current_mtu = network.attrs.get("Options", {}).get( - "com.docker.network.driver.mtu" - ) - current_mtu = int(current_mtu) if current_mtu else None - - # If the network exists and we don't have explicit settings, - # simply stick with what we have. - if (enable_ipv6 is None or current_ipv6 == enable_ipv6) and ( - mtu is None or current_mtu == mtu - ): - return network - - # We have explicit settings which differ from the current state. - changes = [] - if enable_ipv6 is not None and current_ipv6 != enable_ipv6: - changes.append( - "IPv4/IPv6 Dual-Stack" if enable_ipv6 else "IPv4-Only" - ) - if mtu is not None and current_mtu != mtu: - changes.append(f"MTU {mtu}") - - if changes: - _LOGGER.info( - "Migrating Supervisor network to %s", ", ".join(changes) - ) - - if (containers := network.containers) and ( - containers_all := all( - container.name in (OBSERVER_DOCKER_NAME, SUPERVISOR_DOCKER_NAME) - for container in containers - ) - ): - for container in containers: - with suppress( - docker.errors.APIError, - docker.errors.DockerException, - requests.RequestException, - ): - network.disconnect(container, force=True) - - if not containers or containers_all: - try: - network.remove() - except docker.errors.APIError: - _LOGGER.warning("Failed to remove existing Supervisor network") - return network - else: - _LOGGER.warning( - "System appears to be running, " - "not applying Supervisor network change. " - "Reboot your system to apply the change." - ) - return network - except docker.errors.NotFound: - _LOGGER.info("Can't find Supervisor network, creating a new network") - + ) -> None: + """Create supervisor network.""" network_params = DOCKER_NETWORK_PARAMS.copy() - network_params[ATTR_ENABLE_IPV6] = ( - DOCKER_ENABLE_IPV6_DEFAULT if enable_ipv6 is None else enable_ipv6 - ) + + if enable_ipv6 is not None: + network_params[DOCKER_ENABLEIPV6] = enable_ipv6 # Copy options and add MTU if specified if mtu is not None: - options = cast(dict[str, str], network_params["options"]).copy() + options = cast(dict[str, str], network_params[DOCKER_OPTIONS]).copy() options["com.docker.network.driver.mtu"] = str(mtu) - network_params["options"] = options + network_params[DOCKER_OPTIONS] = options try: - self._network = self.docker.networks.create(**network_params) # type: ignore - except docker.errors.APIError as err: + self._network = await self.docker.networks.create(network_params) + except aiodocker.DockerError as err: raise DockerError( f"Can't create Supervisor network: {err}", _LOGGER.error ) from err + await self.reload() + with suppress(DockerError): - self.attach_container_by_name( + await self.attach_container_by_name( SUPERVISOR_DOCKER_NAME, [ATTR_SUPERVISOR], self.supervisor ) with suppress(DockerError): - self.attach_container_by_name( + await self.attach_container_by_name( OBSERVER_DOCKER_NAME, [ATTR_OBSERVER], self.observer ) @@ -213,105 +231,90 @@ class DockerNetwork: (ATTR_AUDIO, self.audio), ): with suppress(DockerError): - self.attach_container_by_name(f"{DOCKER_PREFIX}_{name}", [name], ip) + await self.attach_container_by_name( + f"{DOCKER_PREFIX}_{name}", [name], ip + ) - return self._network + async def reload(self) -> None: + """Get and cache metadata for supervisor network.""" + try: + self._network_meta = await self.network.show() + except aiodocker.DockerError as err: + raise DockerError( + f"Could not get network metadata from Docker: {err!s}", _LOGGER.error + ) from err - def attach_container( + async def attach_container( self, container_id: str, name: str | None, alias: list[str] | None = None, ipv4: IPv4Address | None = None, ) -> None: - """Attach container to Supervisor network. - - Need run inside executor. - """ + """Attach container to Supervisor network.""" # Reload Network information - with suppress(docker.errors.DockerException, requests.RequestException): - self.network.reload() + with suppress(DockerError): + await self.reload() # Check stale Network - if name and name in ( - val.get("Name") for val in self.network.attrs.get("Containers", {}).values() - ): - self.stale_cleanup(name) + if name and name in (val.get("Name") for val in self.containers.values()): + await self.stale_cleanup(name) # Attach Network + endpoint_config: dict[str, Any] = {} + if alias: + endpoint_config["Aliases"] = alias + if ipv4: + endpoint_config["IPAMConfig"] = {"IPv4Address": str(ipv4)} + try: - self.network.connect( - container_id, aliases=alias, ipv4_address=str(ipv4) if ipv4 else None + await self.network.connect( + { + "Container": container_id, + "EndpointConfig": endpoint_config, + } ) - except ( - docker.errors.NotFound, - docker.errors.APIError, - docker.errors.DockerException, - requests.RequestException, - ) as err: + except aiodocker.DockerError as err: raise DockerError( f"Can't connect {name or container_id} to Supervisor network: {err}", _LOGGER.error, ) from err - def attach_container_by_name( - self, - name: str, - alias: list[str] | None = None, - ipv4: IPv4Address | None = None, + async def attach_container_by_name( + self, name: str, alias: list[str] | None = None, ipv4: IPv4Address | None = None ) -> None: - """Attach container to Supervisor network. - - Need run inside executor. - """ + """Attach container to Supervisor network.""" try: - container = self.docker.containers.get(name) - except ( - docker.errors.NotFound, - docker.errors.APIError, - docker.errors.DockerException, - requests.RequestException, - ) as err: + container = await self.docker.containers.get(name) + except aiodocker.DockerError as err: raise DockerError(f"Can't find {name}: {err}", _LOGGER.error) from err - if not (container_id := container.id): - raise DockerError(f"Received invalid metadata from docker for {name}") + if container.id not in self.containers: + await self.attach_container(container.id, name, alias, ipv4) - if container_id not in self.containers: - self.attach_container(container_id, name, alias, ipv4) - - def detach_default_bridge(self, container_id: str, name: str | None = None) -> None: - """Detach default Docker bridge. - - Need run inside executor. - """ + async def detach_default_bridge( + self, container_id: str, name: str | None = None + ) -> None: + """Detach default Docker bridge.""" try: - default_network = self.docker.networks.get(DOCKER_NETWORK_DRIVER) - default_network.disconnect(container_id) - except docker.errors.NotFound: - pass - except ( - docker.errors.APIError, - docker.errors.DockerException, - requests.RequestException, - ) as err: + default_network = await self.docker.networks.get(DOCKER_NETWORK_DRIVER) + await default_network.disconnect({"Container": container_id}) + except aiodocker.DockerError as err: + if err.status == HTTPStatus.NOT_FOUND: + return raise DockerError( f"Can't disconnect {name or container_id} from default network: {err}", _LOGGER.warning, ) from err - def stale_cleanup(self, name: str) -> None: + async def stale_cleanup(self, name: str) -> None: """Force remove a container from Network. Fix: https://github.com/moby/moby/issues/23302 """ try: - self.network.disconnect(name, force=True) - except ( - docker.errors.APIError, - docker.errors.DockerException, - requests.RequestException, - ) as err: + await self.network.disconnect({"Container": name, "Force": True}) + except aiodocker.DockerError as err: raise DockerError( f"Can't disconnect {name} from Supervisor network: {err}", _LOGGER.warning, diff --git a/supervisor/docker/supervisor.py b/supervisor/docker/supervisor.py index 62d5b530b..8b41d7426 100644 --- a/supervisor/docker/supervisor.py +++ b/supervisor/docker/supervisor.py @@ -69,8 +69,7 @@ class DockerSupervisor(DockerInterface): # Attach to network _LOGGER.info("Connecting Supervisor to hassio-network") - await self.sys_run_in_executor( - self.sys_docker.network.attach_container, + await self.sys_docker.network.attach_container( docker_container.id, self.name, alias=["supervisor"], diff --git a/tests/conftest.py b/tests/conftest.py index fb798ce1a..ad70dbea6 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -12,6 +12,7 @@ from uuid import uuid4 from aiodocker.containers import DockerContainer, DockerContainers from aiodocker.docker import DockerImages from aiodocker.execs import Exec +from aiodocker.networks import DockerNetwork, DockerNetworks from aiohttp import ClientSession, web from aiohttp.test_utils import TestClient from awesomeversion import AwesomeVersion @@ -125,26 +126,53 @@ async def docker() -> DockerAPI: "State": {"ExitCode": 0, "Status": "stopped", "Running": False}, "Image": "abc123", } + network_inspect = { + "Name": "hassio", + "Id": "hassio123", + "EnableIPv4": True, + "EnableIPv6": False, + "IPAM": { + "Driver": "default", + "Options": None, + "Config": [ + { + "Subnet": "172.30.32.0/23", + "IPRange": "172.30.33.0/24", + "Gateway": "172.30.32.1", + } + ], + }, + "Containers": {}, + } with ( patch("supervisor.docker.manager.DockerClient", return_value=MagicMock()), - patch("supervisor.docker.manager.DockerAPI.api", return_value=MagicMock()), patch("supervisor.docker.manager.DockerAPI.info", return_value=MagicMock()), patch("supervisor.docker.manager.DockerAPI.unload"), - patch("supervisor.docker.manager.aiodocker.Docker", return_value=MagicMock()), + patch( + "supervisor.docker.manager.aiodocker.Docker", + return_value=( + docker_client := MagicMock( + networks=MagicMock(spec=DockerNetworks), + images=(docker_images := MagicMock(spec=DockerImages)), + containers=(docker_containers := MagicMock(spec=DockerContainers)), + ) + ), + ), patch( "supervisor.docker.manager.DockerAPI.images", - new=PropertyMock( - return_value=(docker_images := MagicMock(spec=DockerImages)) - ), + new=PropertyMock(return_value=docker_images), ), patch( "supervisor.docker.manager.DockerAPI.containers", - new=PropertyMock( - return_value=(docker_containers := MagicMock(spec=DockerContainers)) - ), + new=PropertyMock(return_value=docker_containers), ), ): + docker_client.networks.get.return_value = docker_network = MagicMock( + spec=DockerNetwork + ) + docker_network.show.return_value = network_inspect + docker_obj = await DockerAPI(MagicMock()).post_init() docker_obj.config._data = {"registries": {}} with patch("supervisor.docker.monitor.DockerMonitor.load"): diff --git a/tests/docker/test_manager.py b/tests/docker/test_manager.py index 1a5993b6e..3919179c2 100644 --- a/tests/docker/test_manager.py +++ b/tests/docker/test_manager.py @@ -7,7 +7,8 @@ from unittest.mock import AsyncMock, MagicMock, patch import aiodocker from aiodocker.containers import DockerContainer -from docker.errors import APIError, NotFound +from aiodocker.networks import DockerNetwork +from docker.errors import APIError import pytest from supervisor.const import DNS_SUFFIX @@ -434,18 +435,18 @@ async def test_repair( coresys: CoreSys, caplog: pytest.LogCaptureFixture, container: DockerContainer ): """Test repair API.""" - coresys.docker.dockerpy.networks.get.side_effect = [ - hassio := MagicMock( - attrs={ - "Containers": { - "good": {"Name": "good"}, - "corrupt": {"Name": "corrupt"}, - "fail": {"Name": "fail"}, - } - } - ), - host := MagicMock(attrs={"Containers": {}}), + coresys.docker.docker.networks.get.side_effect = [ + hassio := MagicMock(spec=DockerNetwork), + host := MagicMock(spec=DockerNetwork), ] + hassio.show.return_value = { + "Containers": { + "good": {"Name": "good"}, + "corrupt": {"Name": "corrupt"}, + "fail": {"Name": "fail"}, + } + } + host.show.return_value = {"Containers": {}} coresys.docker.containers.get.side_effect = [ container, aiodocker.DockerError(HTTPStatus.NOT_FOUND, {"message": "corrupt"}), @@ -461,7 +462,7 @@ async def test_repair( coresys.docker.dockerpy.api.prune_builds.assert_called_once() coresys.docker.dockerpy.api.prune_volumes.assert_called_once() coresys.docker.dockerpy.api.prune_networks.assert_called_once() - hassio.disconnect.assert_called_once_with("corrupt", force=True) + hassio.disconnect.assert_called_once_with({"Container": "corrupt", "Force": True}) host.disconnect.assert_not_called() assert "Docker fatal error on container fail on hassio" in caplog.text @@ -473,7 +474,9 @@ async def test_repair_failures(coresys: CoreSys, caplog: pytest.LogCaptureFixtur coresys.docker.dockerpy.api.prune_builds.side_effect = APIError("fail") coresys.docker.dockerpy.api.prune_volumes.side_effect = APIError("fail") coresys.docker.dockerpy.api.prune_networks.side_effect = APIError("fail") - coresys.docker.dockerpy.networks.get.side_effect = NotFound("missing") + coresys.docker.docker.networks.get.side_effect = err = aiodocker.DockerError( + HTTPStatus.NOT_FOUND, {"message": "missing"} + ) await coresys.docker.repair() @@ -482,8 +485,8 @@ async def test_repair_failures(coresys: CoreSys, caplog: pytest.LogCaptureFixtur assert "Error for builds prune: fail" in caplog.text assert "Error for volumes prune: fail" in caplog.text assert "Error for networks prune: fail" in caplog.text - assert "Error for networks hassio prune: missing" in caplog.text - assert "Error for networks host prune: missing" in caplog.text + assert f"Error for networks hassio prune: {err!s}" in caplog.text + assert f"Error for networks host prune: {err!s}" in caplog.text @pytest.mark.parametrize("log_starter", [("Loaded image ID"), ("Loaded image")]) diff --git a/tests/docker/test_network.py b/tests/docker/test_network.py index 6c19b7db6..ffdae81aa 100644 --- a/tests/docker/test_network.py +++ b/tests/docker/test_network.py @@ -1,16 +1,18 @@ """Test Internal network manager for Supervisor.""" -from unittest.mock import MagicMock, PropertyMock, patch +from http import HTTPStatus +from unittest.mock import MagicMock -import docker +import aiodocker +from aiodocker.networks import DockerNetwork as AiodockerNetwork import pytest from supervisor.const import ( - ATTR_ENABLE_IPV6, DOCKER_NETWORK, OBSERVER_DOCKER_NAME, SUPERVISOR_DOCKER_NAME, ) +from supervisor.docker.manager import DockerAPI from supervisor.docker.network import ( DOCKER_ENABLEIPV6, DOCKER_NETWORK_PARAMS, @@ -18,212 +20,172 @@ from supervisor.docker.network import ( ) -class MockContainer: - """Mock implementation of a Docker container.""" - - def __init__(self, name: str) -> None: - """Initialize a mock container.""" - self.name = name - - -class MockNetwork: - """Mock implementation of internal network.""" - - def __init__( - self, - raise_error: bool, - containers: list[str], - enableIPv6: bool, - mtu: int | None = None, - ) -> None: - """Initialize a mock network.""" - self.raise_error = raise_error - self.containers = [MockContainer(container) for container in containers or []] - self.attrs = { - DOCKER_ENABLEIPV6: enableIPv6, - "Options": {"com.docker.network.driver.mtu": str(mtu)} if mtu else {}, - } - - def remove(self) -> None: - """Simulate a network removal.""" - if self.raise_error: - raise docker.errors.APIError("Simulated removal error") - - def reload(self, *args, **kwargs) -> None: - """Simulate a network reload.""" - - def connect(self, *args, **kwargs) -> None: - """Simulate a network connection.""" - - def disconnect(self, *args, **kwargs) -> None: - """Simulate a network disconnection.""" - - @pytest.mark.parametrize( - ("raise_error", "containers", f"old_{ATTR_ENABLE_IPV6}", f"new_{ATTR_ENABLE_IPV6}"), + ( + "delete_error", + "disconnect_error", + "containers", + "old_enable_ipv6", + "new_enable_ipv6", + "create_expected", + ), [ - (False, [OBSERVER_DOCKER_NAME, SUPERVISOR_DOCKER_NAME], False, True), - (False, [OBSERVER_DOCKER_NAME, SUPERVISOR_DOCKER_NAME], True, False), - (False, ["test_container"], False, True), - (False, None, False, True), - (True, None, False, True), - (False, None, True, True), + (None, None, [OBSERVER_DOCKER_NAME, SUPERVISOR_DOCKER_NAME], False, True, True), + (None, None, [OBSERVER_DOCKER_NAME, SUPERVISOR_DOCKER_NAME], True, False, True), + (None, None, ["test_container"], False, True, False), + (None, None, [], False, True, True), + ( + None, + aiodocker.DockerError( + HTTPStatus.INTERNAL_SERVER_ERROR, + {"message": "Simulated disconnect error"}, + ), + [OBSERVER_DOCKER_NAME, SUPERVISOR_DOCKER_NAME], + False, + True, + False, + ), + ( + aiodocker.DockerError( + HTTPStatus.INTERNAL_SERVER_ERROR, {"message": "Simulated removal error"} + ), + None, + [], + False, + True, + False, + ), + (None, None, [], True, True, False), ], ) async def test_network_recreation( - raise_error: bool, - containers: list[str] | None, + docker: DockerAPI, + delete_error: aiodocker.DockerError | None, + disconnect_error: aiodocker.DockerError | None, + containers: list[str], old_enable_ipv6: bool, new_enable_ipv6: bool, + create_expected: bool, ): """Test network recreation with IPv6 enabled/disabled.""" + docker.docker.networks.get.return_value = mock_network = MagicMock( + spec=AiodockerNetwork, id="test123" + ) + mock_network.show.return_value = { + "Containers": {name: {"Id": name, "Name": name} for name in containers}, + DOCKER_ENABLEIPV6: old_enable_ipv6, + } + mock_network.delete.side_effect = delete_error + mock_network.disconnect.side_effect = disconnect_error + docker.docker.networks.create.return_value = mock_new_network = MagicMock( + spec=AiodockerNetwork, id="test123" + ) + mock_new_network.show.return_value = { + "Containers": {}, + DOCKER_ENABLEIPV6: new_enable_ipv6, + } - with ( - patch( - "supervisor.docker.network.DockerNetwork.docker", - new_callable=PropertyMock, - return_value=MagicMock(), - create=True, - ), - patch( - "supervisor.docker.network.DockerNetwork.docker.networks", - new_callable=PropertyMock, - return_value=MagicMock(), - create=True, - ), - patch( - "supervisor.docker.network.DockerNetwork.docker.networks.get", - return_value=MockNetwork(raise_error, containers, old_enable_ipv6, None), - ) as mock_get, - patch( - "supervisor.docker.network.DockerNetwork.docker.networks.create", - return_value=MockNetwork(raise_error, containers, new_enable_ipv6, None), - ) as mock_create, - ): - network = (await DockerNetwork(MagicMock()).post_init(new_enable_ipv6)).network + docker_network = await DockerNetwork(docker.docker).post_init(new_enable_ipv6) + docker.docker.networks.get.assert_called_with(DOCKER_NETWORK) - mock_get.assert_called_with(DOCKER_NETWORK) + assert docker_network.network + assert docker_network.network_meta - assert network is not None - assert network.attrs.get(DOCKER_ENABLEIPV6) is ( - new_enable_ipv6 - if not raise_error and (not containers or len(containers) > 1) - else old_enable_ipv6 + if not create_expected: + assert docker_network.network_meta[DOCKER_ENABLEIPV6] is old_enable_ipv6 + docker.docker.networks.create.assert_not_called() + else: + assert docker_network.network_meta[DOCKER_ENABLEIPV6] is new_enable_ipv6 + docker.docker.networks.create.assert_called_once_with( + DOCKER_NETWORK_PARAMS | {DOCKER_ENABLEIPV6: new_enable_ipv6} ) - if ( - not raise_error and (not containers or len(containers) > 1) - ) and old_enable_ipv6 != new_enable_ipv6: - network_params = DOCKER_NETWORK_PARAMS.copy() - network_params[ATTR_ENABLE_IPV6] = new_enable_ipv6 - mock_create.assert_called_with(**network_params) - - -async def test_network_default_ipv6_for_new_installations(): +async def test_network_default_ipv6_for_new_installations(docker: DockerAPI): """Test that IPv6 is enabled by default when no user setting is provided (None).""" - with ( - patch( - "supervisor.docker.network.DockerNetwork.docker", - new_callable=PropertyMock, - return_value=MagicMock(), - create=True, - ), - patch( - "supervisor.docker.network.DockerNetwork.docker.networks", - new_callable=PropertyMock, - return_value=MagicMock(), - create=True, - ), - patch( - "supervisor.docker.network.DockerNetwork.docker.networks.get", - side_effect=docker.errors.NotFound("Network not found"), - ), - patch( - "supervisor.docker.network.DockerNetwork.docker.networks.create", - return_value=MockNetwork(False, None, True, None), - ) as mock_create, - ): - # Pass None as enable_ipv6 to simulate no user setting - network = (await DockerNetwork(MagicMock()).post_init(None)).network + docker.docker.networks.get.side_effect = aiodocker.DockerError( + HTTPStatus.NOT_FOUND, {"message": "Network not found"} + ) + docker.docker.networks.create.return_value = mock_network = MagicMock( + spec=AiodockerNetwork, id="test123" + ) + mock_network.show.return_value = {"Containers": {}, DOCKER_ENABLEIPV6: True} - assert network is not None - assert network.attrs.get(DOCKER_ENABLEIPV6) is True + # Pass None as enable_ipv6 to simulate no user setting + docker_network = await DockerNetwork(docker.docker).post_init(None) - # Verify that create was called with IPv6 enabled by default - expected_params = DOCKER_NETWORK_PARAMS.copy() - expected_params[ATTR_ENABLE_IPV6] = True - mock_create.assert_called_with(**expected_params) + assert docker_network.network + assert docker_network.network_meta + assert docker_network.network_meta[DOCKER_ENABLEIPV6] is True + + # Verify that create was called with IPv6 enabled by default + docker.docker.networks.create.assert_called_with( + DOCKER_NETWORK_PARAMS | {DOCKER_ENABLEIPV6: True} + ) -async def test_network_mtu_recreation(): +async def test_network_mtu_recreation(docker: DockerAPI): """Test network recreation with different MTU settings.""" - with ( - patch( - "supervisor.docker.network.DockerNetwork.docker", - new_callable=PropertyMock, - return_value=MagicMock(), - create=True, - ), - patch( - "supervisor.docker.network.DockerNetwork.docker.networks", - new_callable=PropertyMock, - return_value=MagicMock(), - create=True, - ), - patch( - "supervisor.docker.network.DockerNetwork.docker.networks.get", - return_value=MockNetwork(False, None, True, 1500), # Old MTU 1500 - ) as mock_get, - patch( - "supervisor.docker.network.DockerNetwork.docker.networks.create", - return_value=MockNetwork(False, None, True, 1450), # New MTU 1450 - ) as mock_create, - ): - # Set new MTU to 1450 - network = (await DockerNetwork(MagicMock()).post_init(True, 1450)).network + docker.docker.networks.get.return_value = mock_network = MagicMock( + spec=AiodockerNetwork, id="test123" + ) + mock_network.show.return_value = { + DOCKER_ENABLEIPV6: False, + "Containers": {}, + "Options": {"com.docker.network.driver.mtu": "1500"}, + } + docker.docker.networks.create.return_value = mock_new_network = MagicMock( + spec=AiodockerNetwork, id="test123" + ) + mock_new_network.show.return_value = { + DOCKER_ENABLEIPV6: True, + "Containers": {}, + "Options": {"com.docker.network.driver.mtu": "1450"}, + } - mock_get.assert_called_with(DOCKER_NETWORK) + # Set new MTU to 1450 + docker_network = await DockerNetwork(docker.docker).post_init(True, 1450) - assert network is not None + docker.docker.networks.get.assert_called_with(DOCKER_NETWORK) + assert docker_network.network + assert docker_network.network_meta + assert docker_network.network_meta[DOCKER_ENABLEIPV6] is True + assert ( + docker_network.network_meta["Options"]["com.docker.network.driver.mtu"] + == "1450" + ) - # Verify network was recreated with new MTU - expected_params = DOCKER_NETWORK_PARAMS.copy() - expected_params[ATTR_ENABLE_IPV6] = True - expected_params["options"] = expected_params["options"].copy() - expected_params["options"]["com.docker.network.driver.mtu"] = "1450" - mock_create.assert_called_with(**expected_params) + # Verify network was recreated with new MTU + expected_options = DOCKER_NETWORK_PARAMS["Options"] | { + "com.docker.network.driver.mtu": "1450" + } + docker.docker.networks.create.assert_called_with( + DOCKER_NETWORK_PARAMS | {DOCKER_ENABLEIPV6: True, "Options": expected_options} + ) -async def test_network_mtu_no_change(): +async def test_network_mtu_no_change(docker: DockerAPI): """Test that network is not recreated when MTU hasn't changed.""" - with ( - patch( - "supervisor.docker.network.DockerNetwork.docker", - new_callable=PropertyMock, - return_value=MagicMock(), - create=True, - ), - patch( - "supervisor.docker.network.DockerNetwork.docker.networks", - new_callable=PropertyMock, - return_value=MagicMock(), - create=True, - ), - patch( - "supervisor.docker.network.DockerNetwork.docker.networks.get", - return_value=MockNetwork(False, None, True, 1450), # Existing MTU 1450 - ) as mock_get, - patch( - "supervisor.docker.network.DockerNetwork.docker.networks.create", - ) as mock_create, - ): - # Set same MTU (1450) - network = (await DockerNetwork(MagicMock()).post_init(True, 1450)).network + docker.docker.networks.get.return_value = mock_network = MagicMock( + spec=AiodockerNetwork, id="test123" + ) + mock_network.show.return_value = { + DOCKER_ENABLEIPV6: True, + "Containers": {}, + "Options": {"com.docker.network.driver.mtu": "1450"}, + } - mock_get.assert_called_with(DOCKER_NETWORK) + # Set same MTU (1450) + docker_network = await DockerNetwork(docker.docker).post_init(True, 1450) - # Verify network was NOT recreated since MTU is the same - mock_create.assert_not_called() + docker.docker.networks.get.assert_called_with(DOCKER_NETWORK) + assert docker_network.network + assert docker_network.network_meta + assert docker_network.network_meta[DOCKER_ENABLEIPV6] is True + assert ( + docker_network.network_meta["Options"]["com.docker.network.driver.mtu"] + == "1450" + ) - assert network is not None + # Verify network was NOT recreated since MTU is the same + docker.docker.networks.create.assert_not_called()