From fd205ce2ef1989b58e9af7bdffab668051a552c1 Mon Sep 17 00:00:00 2001 From: Copilot <198982749+Copilot@users.noreply.github.com> Date: Tue, 12 Aug 2025 09:19:12 +0200 Subject: [PATCH] Add Docker MTU configuration support for networks with non-standard MTU (#6079) * Initial plan * Implement Docker MTU support - core functionality Co-authored-by: agners <34061+agners@users.noreply.github.com> * Add comprehensive MTU tests and documentation Co-authored-by: agners <34061+agners@users.noreply.github.com> * Fix final linting issue in test file Co-authored-by: agners <34061+agners@users.noreply.github.com> * Apply suggestions from code review * Implement reboot_required flag pattern and fix MyPy typing issue Co-authored-by: agners <34061+agners@users.noreply.github.com> * Update supervisor/api/docker.py * Update supervisor/docker/manager.py Co-authored-by: Mike Degatano --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: agners <34061+agners@users.noreply.github.com> Co-authored-by: Stefan Agner Co-authored-by: Mike Degatano --- supervisor/api/docker.py | 22 ++++++++- supervisor/const.py | 1 + supervisor/docker/manager.py | 13 +++++- supervisor/docker/network.py | 45 +++++++++++++----- supervisor/validate.py | 4 ++ tests/api/test_docker.py | 46 ++++++++++++++++++ tests/docker/test_network.py | 90 ++++++++++++++++++++++++++++++++++-- 7 files changed, 202 insertions(+), 19 deletions(-) diff --git a/supervisor/api/docker.py b/supervisor/api/docker.py index e751cc006..8a62054ef 100644 --- a/supervisor/api/docker.py +++ b/supervisor/api/docker.py @@ -12,6 +12,7 @@ from ..const import ( ATTR_ENABLE_IPV6, ATTR_HOSTNAME, ATTR_LOGGING, + ATTR_MTU, ATTR_PASSWORD, ATTR_REGISTRIES, ATTR_STORAGE, @@ -34,7 +35,12 @@ SCHEMA_DOCKER_REGISTRY = vol.Schema( ) # pylint: disable=no-value-for-parameter -SCHEMA_OPTIONS = vol.Schema({vol.Optional(ATTR_ENABLE_IPV6): vol.Maybe(vol.Boolean())}) +SCHEMA_OPTIONS = vol.Schema( + { + vol.Optional(ATTR_ENABLE_IPV6): vol.Maybe(vol.Boolean()), + vol.Optional(ATTR_MTU): vol.Maybe(vol.All(int, vol.Range(min=68, max=65535))), + } +) class APIDocker(CoreSysAttributes): @@ -51,6 +57,7 @@ class APIDocker(CoreSysAttributes): return { ATTR_VERSION: self.sys_docker.info.version, ATTR_ENABLE_IPV6: self.sys_docker.config.enable_ipv6, + ATTR_MTU: self.sys_docker.config.mtu, ATTR_STORAGE: self.sys_docker.info.storage, ATTR_LOGGING: self.sys_docker.info.logging, ATTR_REGISTRIES: data_registries, @@ -61,12 +68,23 @@ class APIDocker(CoreSysAttributes): """Set docker options.""" body = await api_validate(SCHEMA_OPTIONS, request) + reboot_required = False + if ( ATTR_ENABLE_IPV6 in body and self.sys_docker.config.enable_ipv6 != body[ATTR_ENABLE_IPV6] ): self.sys_docker.config.enable_ipv6 = body[ATTR_ENABLE_IPV6] - _LOGGER.info("Host system reboot required to apply new IPv6 configuration") + reboot_required = True + + if ATTR_MTU in body and self.sys_docker.config.mtu != body[ATTR_MTU]: + self.sys_docker.config.mtu = body[ATTR_MTU] + reboot_required = True + + if reboot_required: + _LOGGER.info( + "Host system reboot required to apply Docker configuration changes" + ) self.sys_resolution.create_issue( IssueType.REBOOT_REQUIRED, ContextType.SYSTEM, diff --git a/supervisor/const.py b/supervisor/const.py index ba95aee06..2441fbddf 100644 --- a/supervisor/const.py +++ b/supervisor/const.py @@ -180,6 +180,7 @@ ATTR_DOMAINS = "domains" ATTR_ENABLE = "enable" ATTR_ENABLE_IPV6 = "enable_ipv6" ATTR_ENABLED = "enabled" +ATTR_MTU = "mtu" ATTR_ENVIRONMENT = "environment" ATTR_EVENT = "event" ATTR_EXCLUDE_DATABASE = "exclude_database" diff --git a/supervisor/docker/manager.py b/supervisor/docker/manager.py index 5bf678f4f..ae4ad7827 100644 --- a/supervisor/docker/manager.py +++ b/supervisor/docker/manager.py @@ -23,6 +23,7 @@ import requests from ..const import ( ATTR_ENABLE_IPV6, + ATTR_MTU, ATTR_REGISTRIES, DNS_SUFFIX, DOCKER_NETWORK, @@ -104,6 +105,16 @@ class DockerConfig(FileConfiguration): """Set IPv6 configuration for docker network.""" self._data[ATTR_ENABLE_IPV6] = value + @property + def mtu(self) -> int | None: + """Return MTU configuration for docker network.""" + return self._data.get(ATTR_MTU) + + @mtu.setter + def mtu(self, value: int | None) -> None: + """Set MTU configuration for docker network.""" + self._data[ATTR_MTU] = value + @property def registries(self) -> dict[str, Any]: """Return credentials for docker registries.""" @@ -138,7 +149,7 @@ class DockerAPI: self._info = DockerInfo.new(self.docker.info()) await self.config.read_data() self._network = await DockerNetwork(self.docker).post_init( - self.config.enable_ipv6 + self.config.enable_ipv6, self.config.mtu ) return self diff --git a/supervisor/docker/network.py b/supervisor/docker/network.py index e4e8c22a9..903825bcc 100644 --- a/supervisor/docker/network.py +++ b/supervisor/docker/network.py @@ -4,7 +4,7 @@ import asyncio from contextlib import suppress from ipaddress import IPv4Address import logging -from typing import Self +from typing import Self, cast import docker import requests @@ -61,10 +61,12 @@ class DockerNetwork: self.docker: docker.DockerClient = docker_client self._network: docker.models.networks.Network - async def post_init(self, enable_ipv6: bool | None = None) -> Self: + 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 + None, self._get_network, enable_ipv6, mtu ) return self @@ -114,22 +116,37 @@ class DockerNetwork: return DOCKER_IPV4_NETWORK_MASK[6] def _get_network( - self, enable_ipv6: bool | None = None + self, enable_ipv6: bool | None = None, mtu: int | None = None ) -> docker.models.networks.Network: """Get supervisor network.""" try: if network := self.docker.networks.get(DOCKER_NETWORK): current_ipv6 = network.attrs.get(DOCKER_ENABLEIPV6, False) - # If the network exists and we don't have an explicit setting, + 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: + if (enable_ipv6 is None or current_ipv6 == enable_ipv6) and ( + mtu is None or current_mtu == mtu + ): return network - # We have an explicit setting which differs from the current state. - _LOGGER.info( - "Migrating Supervisor network to %s", - "IPv4/IPv6 Dual-Stack" if enable_ipv6 else "IPv4-Only", - ) + # 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( @@ -166,6 +183,12 @@ class DockerNetwork: DOCKER_ENABLE_IPV6_DEFAULT if enable_ipv6 is None else enable_ipv6 ) + # Copy options and add MTU if specified + if mtu is not None: + options = cast(dict[str, str], network_params["options"]).copy() + options["com.docker.network.driver.mtu"] = str(mtu) + network_params["options"] = options + try: self._network = self.docker.networks.create(**network_params) # type: ignore except docker.errors.APIError as err: diff --git a/supervisor/validate.py b/supervisor/validate.py index 394c2c008..e435ab074 100644 --- a/supervisor/validate.py +++ b/supervisor/validate.py @@ -29,6 +29,7 @@ from .const import ( ATTR_IMAGE, ATTR_LAST_BOOT, ATTR_LOGGING, + ATTR_MTU, ATTR_MULTICAST, ATTR_OBSERVER, ATTR_OTA, @@ -185,6 +186,9 @@ SCHEMA_DOCKER_CONFIG = vol.Schema( } ), vol.Optional(ATTR_ENABLE_IPV6, default=None): vol.Maybe(vol.Boolean()), + vol.Optional(ATTR_MTU, default=None): vol.Maybe( + vol.All(int, vol.Range(min=68, max=65535)) + ), } ) diff --git a/tests/api/test_docker.py b/tests/api/test_docker.py index 81d183d31..8c10aab40 100644 --- a/tests/api/test_docker.py +++ b/tests/api/test_docker.py @@ -32,6 +32,52 @@ async def test_api_network_enable_ipv6(coresys: CoreSys, api_client: TestClient) assert body["data"]["enable_ipv6"] is True +async def test_api_network_mtu(coresys: CoreSys, api_client: TestClient): + """Test setting docker network MTU.""" + assert coresys.docker.config.mtu is None + + resp = await api_client.post("/docker/options", json={"mtu": 1450}) + assert resp.status == 200 + + assert coresys.docker.config.mtu == 1450 + + resp = await api_client.get("/docker/info") + assert resp.status == 200 + body = await resp.json() + assert body["data"]["mtu"] == 1450 + + # Test setting MTU to None + resp = await api_client.post("/docker/options", json={"mtu": None}) + assert resp.status == 200 + + assert coresys.docker.config.mtu is None + + resp = await api_client.get("/docker/info") + assert resp.status == 200 + body = await resp.json() + assert body["data"]["mtu"] is None + + +async def test_api_network_combined_options(coresys: CoreSys, api_client: TestClient): + """Test setting both IPv6 and MTU together.""" + assert coresys.docker.config.enable_ipv6 is None + assert coresys.docker.config.mtu is None + + resp = await api_client.post( + "/docker/options", json={"enable_ipv6": True, "mtu": 1400} + ) + assert resp.status == 200 + + assert coresys.docker.config.enable_ipv6 is True + assert coresys.docker.config.mtu == 1400 + + resp = await api_client.get("/docker/info") + assert resp.status == 200 + body = await resp.json() + assert body["data"]["enable_ipv6"] is True + assert body["data"]["mtu"] == 1400 + + async def test_registry_not_found(api_client: TestClient): """Test registry not found error.""" resp = await api_client.delete("/docker/registries/bad") diff --git a/tests/docker/test_network.py b/tests/docker/test_network.py index 7da89bf2c..6c19b7db6 100644 --- a/tests/docker/test_network.py +++ b/tests/docker/test_network.py @@ -30,12 +30,19 @@ class MockNetwork: """Mock implementation of internal network.""" def __init__( - self, raise_error: bool, containers: list[str], enableIPv6: bool + 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} + self.attrs = { + DOCKER_ENABLEIPV6: enableIPv6, + "Options": {"com.docker.network.driver.mtu": str(mtu)} if mtu else {}, + } def remove(self) -> None: """Simulate a network removal.""" @@ -86,11 +93,11 @@ async def test_network_recreation( ), patch( "supervisor.docker.network.DockerNetwork.docker.networks.get", - return_value=MockNetwork(raise_error, containers, old_enable_ipv6), + 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), + return_value=MockNetwork(raise_error, containers, new_enable_ipv6, None), ) as mock_create, ): network = (await DockerNetwork(MagicMock()).post_init(new_enable_ipv6)).network @@ -134,7 +141,7 @@ async def test_network_default_ipv6_for_new_installations(): ), patch( "supervisor.docker.network.DockerNetwork.docker.networks.create", - return_value=MockNetwork(False, None, True), + return_value=MockNetwork(False, None, True, None), ) as mock_create, ): # Pass None as enable_ipv6 to simulate no user setting @@ -147,3 +154,76 @@ async def test_network_default_ipv6_for_new_installations(): expected_params = DOCKER_NETWORK_PARAMS.copy() expected_params[ATTR_ENABLE_IPV6] = True mock_create.assert_called_with(**expected_params) + + +async def test_network_mtu_recreation(): + """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 + + mock_get.assert_called_with(DOCKER_NETWORK) + + assert network is not None + + # 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) + + +async def test_network_mtu_no_change(): + """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 + + mock_get.assert_called_with(DOCKER_NETWORK) + + # Verify network was NOT recreated since MTU is the same + mock_create.assert_not_called() + + assert network is not None