From e2148cd0640eeb36da616143a494dd0da6a754b5 Mon Sep 17 00:00:00 2001 From: Sam Castillo Date: Thu, 29 Oct 2026 17:26:26 +0000 Subject: [PATCH 1/2] fix: /pull reconnects networks with their aliases, static IPs and links pull_and_recreate passed each secondary network's endpoint from docker inspect straight to docker-py's Network.connect() as Aliases=, IPAMConfig= and Links=. docker-py only accepts snake_case keywords, so any network with an alias, a static IP or a link failed with "unexpected keyword argument" and was dropped. Only networks with none of those reconnected. - Translate the endpoint to aliases, links (name/alias pairs, which normalize_links() unpacks), ipv4_address, ipv6_address, link_local_ips and driver_opt. Keyword names are checked against the installed connect_container_to_network() signature; anything it lacks is logged and skipped. - Drop the old container's short ID from aliases (engines before API 1.45 list it there) and leave the assigned MAC address unpinned. - Skip a network the recreated container already joined, such as bridge when NetworkMode is "default", instead of reporting a failure. - networks.get() now runs off the event loop like the other Docker calls. The previous test asserted connect(container, Aliases=[...]) on a mock and passed. The new tests drive real docker-py (DockerClient, Network.connect, EndpointConfig) and check the request body for alias, static IPv4 and IPv6, links, driver options, none, the short-ID alias and an assigned MAC. --- src/services/container_control.py | 105 +++++++++++++--- tests/test_docker_client.py | 26 ++-- tests/test_pull_network_reconnect.py | 180 +++++++++++++++++++++++++++ 3 files changed, 286 insertions(+), 25 deletions(-) create mode 100644 tests/test_pull_network_reconnect.py diff --git a/src/services/container_control.py b/src/services/container_control.py index c2e90a0..aa0f954 100644 --- a/src/services/container_control.py +++ b/src/services/container_control.py @@ -1,12 +1,86 @@ import asyncio +import inspect import logging from typing import Any import docker +import docker.api.network from docker.types import DeviceRequest logger = logging.getLogger(__name__) +# Keyword arguments the installed docker-py accepts when connecting a container +# to a network. Network.connect() forwards **kwargs to this method, which +# rejects anything else with a TypeError. +_CONNECT_KWARGS: frozenset[str] = frozenset( + name + for name in inspect.signature( + docker.api.network.NetworkApiMixin.connect_container_to_network + ).parameters + if name not in ("self", "container", "net_id") +) + + +def _endpoint_connect_kwargs(endpoint: dict[str, Any], own_short_id: str) -> dict[str, Any]: + """Translate a ``docker inspect`` network endpoint into ``Network.connect()`` kwargs. + + Inspect reports the endpoint in API shape (``Aliases``, ``Links``, + ``IPAMConfig``, ``DriverOpts``); docker-py takes snake_case keywords and + builds that shape itself. Passing the inspect keys through raised + "unexpected keyword argument 'Aliases'", so every secondary network with + an alias or a static IP was dropped after a /pull. + + ``MacAddress`` is left out: inspect reports the address Docker assigned + even when nobody chose one, and pinning it would carry it to the new + container as if it had been configured. + """ + kwargs: dict[str, Any] = {} + + # Engines before API 1.45 also list the container's short ID as an alias. + # It names the old container, so it is not carried over. + aliases = [ + a for a in (endpoint.get("Aliases") or []) + if isinstance(a, str) and a and a != own_short_id + ] + if aliases: + kwargs["aliases"] = aliases + + # Inspect gives "name:alias" strings; docker-py wants (name, alias) pairs + # and unpacks each item, so a plain string fails in normalize_links(). + links: list[tuple[str, str | None]] = [] + for link in endpoint.get("Links") or []: + if isinstance(link, str) and link: + name, _, alias = link.partition(":") + links.append((name, alias or None)) + if links: + kwargs["links"] = links + + ipam = endpoint.get("IPAMConfig") or {} + if ipam.get("IPv4Address"): + kwargs["ipv4_address"] = ipam["IPv4Address"] + if ipam.get("IPv6Address"): + kwargs["ipv6_address"] = ipam["IPv6Address"] + if ipam.get("LinkLocalIPs"): + kwargs["link_local_ips"] = list(ipam["LinkLocalIPs"]) + + if endpoint.get("DriverOpts"): + kwargs["driver_opt"] = dict(endpoint["DriverOpts"]) + + unsupported = set(kwargs) - _CONNECT_KWARGS + for name in unsupported: + logger.warning(f"Installed docker-py cannot set {name} on a network endpoint; skipping it") + del kwargs[name] + return kwargs + + +def _attached_networks(container: Any) -> set[str]: + """Names of the networks a container is already on, per its inspect data.""" + attrs = getattr(container, "attrs", None) + if not isinstance(attrs, dict): + return set() + networks = (attrs.get("NetworkSettings") or {}).get("Networks") or {} + return set(networks) if isinstance(networks, dict) else set() + class ContainerController: """Controls Docker containers with protection support.""" @@ -143,9 +217,16 @@ def _get_container_info() -> tuple[Any, str, str | None]: new_container = await asyncio.to_thread( self.docker_client.containers.get, container_name ) + attached = _attached_networks(new_container) for net_name, endpoint in secondary_networks.items(): + if net_name in attached: + # run() already joined it, e.g. NetworkMode "default" + # is reported as the "bridge" network. + continue try: - network = self.docker_client.networks.get(net_name) + network = await asyncio.to_thread( + self.docker_client.networks.get, net_name + ) await asyncio.to_thread( network.connect, new_container, **endpoint ) @@ -202,27 +283,21 @@ def _get_container_info() -> tuple[Any, str, str | None]: def _get_secondary_networks(attrs: dict[str, Any]) -> dict[str, dict[str, Any]]: """Extract secondary network connections (excluding the primary NetworkMode). - Returns a dict of network_name -> endpoint_config for networks that need - to be reconnected after container creation (since containers.run only - connects the primary network). + Returns a dict of network_name -> keyword arguments for docker-py's + ``Network.connect()``, for networks that need to be reconnected after + container creation (since containers.run only connects the primary + network). """ - host_config = attrs.get("HostConfig", {}) + host_config = attrs.get("HostConfig") or {} primary_network = host_config.get("NetworkMode", "") + own_short_id = (attrs.get("Id") or "")[:12] - networks = attrs.get("NetworkSettings", {}).get("Networks", {}) + networks = (attrs.get("NetworkSettings") or {}).get("Networks") or {} secondary = {} for net_name, net_config in networks.items(): if net_name == primary_network: continue - # Extract reconnection-relevant config - endpoint = {} - if net_config.get("IPAMConfig"): - endpoint["IPAMConfig"] = net_config["IPAMConfig"] - if net_config.get("Aliases"): - endpoint["Aliases"] = net_config["Aliases"] - if net_config.get("Links"): - endpoint["Links"] = net_config["Links"] - secondary[net_name] = endpoint + secondary[net_name] = _endpoint_connect_kwargs(net_config or {}, own_short_id) return secondary def _extract_run_config(self, attrs: dict[str, Any]) -> dict[str, Any]: diff --git a/tests/test_docker_client.py b/tests/test_docker_client.py index aa5e8f0..fc6f8ea 100644 --- a/tests/test_docker_client.py +++ b/tests/test_docker_client.py @@ -121,27 +121,33 @@ def test_host_cpu_cores_fallback_names_the_value_it_used(self, caplog): assert host_cpu_cores(SharedDockerClient(raw)) == 3 assert "using 3 from os.cpu_count()" in caplog.text - async def test_pull_reconnects_secondary_networks_through_the_wrapper(self): + async def test_pull_reaches_networks_through_the_wrapper(self): + """The wrapper must hand out the real client's networks collection. + + Only the proxy is checked here. What gets sent to Docker is checked + through real docker-py in tests/test_pull_network_reconnect.py; the + version of this test that asserted connect(container, Aliases=[...]) + on a mock passed while docker-py rejected that exact call. + """ from src.services.container_control import ContainerController from src.services.docker_client import SharedDockerClient - container = MagicMock() - container.image.tags = ["lscr.io/linuxserver/sonarr:latest"] - container.image.id = "sha256:old" - container.attrs = { + old = MagicMock() + old.image.tags = ["lscr.io/linuxserver/sonarr:latest"] + old.image.id = "sha256:old" + old.attrs = { "Config": {"Image": "lscr.io/linuxserver/sonarr:latest"}, "HostConfig": {"NetworkMode": "bridge"}, "NetworkSettings": {"Networks": {"bridge": {}, "proxynet": {"Aliases": ["sonarr"]}}}, } + new = MagicMock() + new.attrs = {"NetworkSettings": {"Networks": {"bridge": {}}}} raw = _spec_client() - raw.containers.get.return_value = container + raw.containers.get.side_effect = [old, new] controller = ContainerController(SharedDockerClient(raw), []) # type: ignore[arg-type] result = await controller.pull_and_recreate("sonarr") - assert result.startswith("✅ sonarr updated"), result assert "Failed to reconnect" not in result raw.networks.get.assert_called_once_with("proxynet") - raw.networks.get.return_value.connect.assert_called_once_with( - container, Aliases=["sonarr"] - ) + raw.networks.get.return_value.connect.assert_called_once() diff --git a/tests/test_pull_network_reconnect.py b/tests/test_pull_network_reconnect.py new file mode 100644 index 0000000..0f96b76 --- /dev/null +++ b/tests/test_pull_network_reconnect.py @@ -0,0 +1,180 @@ +"""Secondary networks after /pull, driven through real docker-py. + +v0.22.5 passed the endpoint from ``docker inspect`` (``Aliases``, ``IPAMConfig``, +``Links``) straight into ``Network.connect()``, which docker-py rejects with +"unexpected keyword argument 'Aliases'". The test that shipped with it asserted +that exact call on a mock, so it passed. These tests run docker-py's own +``Network.connect()`` -> ``connect_container_to_network()`` -> ``EndpointConfig`` +and capture only the HTTP body, so a wrong keyword or a bad value shape fails here. +""" +import inspect +from typing import Any +from unittest.mock import MagicMock + +import docker +import docker.api.network +import pytest + +from src.services.container_control import ContainerController, _endpoint_connect_kwargs +from src.services.docker_client import SharedDockerClient + +OLD_ID = "a1b2c3d4e5f6" + "0" * 52 + + +def _real_docker_client() -> tuple[docker.DockerClient, list[dict[str, Any]]]: + """A real DockerClient that records what it would POST instead of sending it.""" + client = docker.DockerClient(base_url="tcp://127.0.0.1:9", version="1.56") + sent: list[dict[str, Any]] = [] + + def post_json(url: str, data: dict[str, Any]) -> MagicMock: + sent.append({"url": url, "data": data}) + return MagicMock() + + client.api._post_json = post_json # type: ignore[method-assign] + client.api._raise_for_status = MagicMock() # type: ignore[method-assign] + return client, sent + + +def _endpoint_sent(endpoint: dict[str, Any]) -> dict[str, Any]: + """Run one inspect endpoint through the real connect path; return the EndpointConfig sent.""" + client, sent = _real_docker_client() + network = client.networks.prepare_model({"Id": "netid", "Name": "proxynet"}) + network.connect("newcontainer", **_endpoint_connect_kwargs(endpoint, OLD_ID[:12])) + assert sent[0]["url"].endswith("/networks/netid/connect") + return dict(sent[0]["data"]["EndpointConfig"]) + + +def test_kwargs_names_come_from_the_installed_docker_py(): + params = inspect.signature( + docker.api.network.NetworkApiMixin.connect_container_to_network + ).parameters + for name in ("aliases", "links", "ipv4_address", "ipv6_address", "link_local_ips", + "driver_opt"): + assert name in params + + +@pytest.mark.parametrize( + ("endpoint", "expected"), + [ + pytest.param( + {"Aliases": ["sonarr", "tv"], "IPAMConfig": None, "Links": None}, + {"Aliases": ["sonarr", "tv"]}, + id="alias", + ), + pytest.param( + {"Aliases": [], "IPAMConfig": {"IPv4Address": "172.31.250.5"}}, + {"IPAMConfig": {"IPv4Address": "172.31.250.5"}}, + id="static-ipv4", + ), + pytest.param( + {"IPAMConfig": {"IPv4Address": "172.31.250.5", "IPv6Address": "fd00::5", + "LinkLocalIPs": ["169.254.0.5"]}}, + {"IPAMConfig": {"IPv4Address": "172.31.250.5", "IPv6Address": "fd00::5", + "LinkLocalIPs": ["169.254.0.5"]}}, + id="static-ipv4-ipv6-linklocal", + ), + pytest.param( + {"Links": ["db:database", "cache"]}, + {"Links": ["cache", "db:database"]}, # docker-py sorts them + id="links", + ), + pytest.param( + {"DriverOpts": {"com.example.opt": "1"}}, + {"DriverOpts": {"com.example.opt": "1"}}, + id="driver-opts", + ), + pytest.param( + {"Aliases": [], "IPAMConfig": {}, "Links": None, "DriverOpts": {}}, + {}, + id="none", + ), + pytest.param( + # Engines before API 1.45 add the container's short ID to Aliases. + {"Aliases": ["web", OLD_ID[:12]]}, + {"Aliases": ["web"]}, + id="own-short-id-dropped", + ), + pytest.param( + {"Aliases": ["web"], "MacAddress": "02:42:ac:11:00:05"}, + {"Aliases": ["web"]}, + id="assigned-mac-not-pinned", + ), + ], +) +def test_inspect_endpoint_round_trips_through_docker_py(endpoint, expected): + assert _endpoint_sent(endpoint) == expected + + +def test_old_inspect_style_kwargs_are_rejected_by_docker_py(): + """The v0.22.5 call shape, for the record: docker-py refuses it.""" + client, _ = _real_docker_client() + network = client.networks.prepare_model({"Id": "netid", "Name": "proxynet"}) + with pytest.raises(TypeError, match="Aliases"): + network.connect("newcontainer", Aliases=["sonarr"]) + + +def _old_container(networks: dict[str, Any]) -> MagicMock: + container = MagicMock() + container.image.tags = ["lscr.io/linuxserver/sonarr:latest"] + container.image.id = "sha256:old" + container.attrs = { + "Id": OLD_ID, + "Config": {"Image": "lscr.io/linuxserver/sonarr:latest"}, + "HostConfig": {"NetworkMode": "bridge"}, + "NetworkSettings": {"Networks": networks}, + } + return container + + +async def test_pull_reconnects_each_secondary_network_with_its_settings(): + real, sent = _real_docker_client() + old = _old_container({ + "bridge": {}, + "proxynet": {"Aliases": ["sonarr", OLD_ID[:12]]}, + "ipnet": {"Aliases": [], "IPAMConfig": {"IPv4Address": "172.31.250.5"}}, + "plainnet": {"Aliases": [], "IPAMConfig": {}}, + }) + new = real.containers.prepare_model( + {"Id": "new" + "0" * 61, "NetworkSettings": {"Networks": {"bridge": {}}}} + ) + + raw = MagicMock(spec=docker.DockerClient) + raw.containers.get.side_effect = [old, new] + raw.networks.get.side_effect = lambda name: real.networks.prepare_model( + {"Id": f"id-{name}", "Name": name} + ) + + controller = ContainerController(SharedDockerClient(raw), []) # type: ignore[arg-type] + result = await controller.pull_and_recreate("sonarr") + + assert result == "✅ sonarr updated (pulled lscr.io/linuxserver/sonarr:latest and recreated)" + by_net = {s["url"].rsplit("/", 2)[-2]: s["data"] for s in sent} + assert by_net == { + "id-proxynet": {"Container": new.id, "EndpointConfig": {"Aliases": ["sonarr"]}}, + "id-ipnet": {"Container": new.id, + "EndpointConfig": {"IPAMConfig": {"IPv4Address": "172.31.250.5"}}}, + "id-plainnet": {"Container": new.id, "EndpointConfig": {}}, + } + + +async def test_pull_skips_a_network_the_new_container_already_joined(): + """NetworkMode "default" is reported as the "bridge" network, so bridge looks + secondary; connecting it again would fail and report a false problem.""" + real, sent = _real_docker_client() + old = _old_container({"bridge": {}, "proxynet": {"Aliases": ["sonarr"]}}) + old.attrs["HostConfig"]["NetworkMode"] = "default" + new = real.containers.prepare_model( + {"Id": "new" + "0" * 61, "NetworkSettings": {"Networks": {"bridge": {}}}} + ) + + raw = MagicMock(spec=docker.DockerClient) + raw.containers.get.side_effect = [old, new] + raw.networks.get.side_effect = lambda name: real.networks.prepare_model( + {"Id": f"id-{name}", "Name": name} + ) + + controller = ContainerController(SharedDockerClient(raw), []) # type: ignore[arg-type] + result = await controller.pull_and_recreate("sonarr") + + assert "Failed to reconnect" not in result + assert [s["url"].rsplit("/", 2)[-2] for s in sent] == ["id-proxynet"] From 51feb70ddbfa85ed4e30e2c64f55d271cea2920c Mon Sep 17 00:00:00 2001 From: Sam Castillo Date: Thu, 29 Oct 2026 17:26:26 +0000 Subject: [PATCH 2/2] docs: draft 0.22.6 changelog for the /pull network fix Adds an Unreleased entry and corrects the 0.22.5 entry, which said a /pull reconnected every secondary network. It reconnected only those with no alias, static IP or link. Version stays at 0.22.5. --- CHANGELOG.md | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a9b7407..9c91c96 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,12 +2,20 @@ All notable changes to UnraidMonitor will be documented in this file. +## [Unreleased] + +Draft for 0.22.6. Not released; the version is still 0.22.5. + +### Fixed +- **A /pull still dropped secondary networks that had an alias, a static IP or a link.** 0.22.5 fixed the missing `networks` proxy but not the call behind it: the endpoint from `docker inspect` was passed to docker-py's `Network.connect()` as-is (`Aliases=`, `IPAMConfig=`, `Links=`), and docker-py only takes snake_case keywords, so it failed with "unexpected keyword argument". Only a secondary network with none of those settings reconnected in 0.22.5. The endpoint is now translated to `aliases`, `links` (as name/alias pairs), `ipv4_address`, `ipv6_address`, `link_local_ips` and `driver_opt`. The old container's short ID, which engines before API 1.45 list as an alias, is not carried over, and the MAC address Docker assigned is not pinned. A network the recreated container has already joined is skipped instead of reported as a failure. +- The 0.22.5 test for this asserted the broken call on a mock, so it passed. The tests now run docker-py's real `connect()` and check the request body it builds. + ## [0.22.5] - 2026-10-29 ### Fixed - **Ollama tool support is read from Ollama, not guessed.** Discovery guessed from the model family by prefix, which was wrong both ways: gemma2, gemma3, gemma and opencoder were offered tools that Ollama rejects ("does not support tools"), and models from a family not on the list had working tools withheld. Discovery now asks `/api/show` for each model's `capabilities`, caches the answer per model digest, and falls back to the family guess only for an Ollama too old to report capabilities or a lookup that fails. Lookups run four at a time with short timeouts inside the discovery budget, so a slow Ollama cannot hold up startup. - **The host CPU count warning on every startup.** `SharedDockerClient`, the reconnectable wrapper every consumer holds, did not pass through `info()`, so reading the host core count always failed with "has no attribute 'info'" and fell back to the bot's own count. That count only sets the top of the CPU threshold buttons and was usually the same number, so the visible cost was the warning. -- **Secondary networks were dropped after a /pull.** The same wrapper did not pass through `networks`, so recreating a container after a /pull or an image-update Pull failed to reconnect every network beyond the primary one, and said so. Both are now passed through, and a test checks that every client attribute the code reads is. +- **Secondary networks were dropped after a /pull.** The same wrapper did not pass through `networks`, so recreating a container after a /pull or an image-update Pull failed to reconnect every network beyond the primary one, and said so. Both are now passed through, and a test checks that every client attribute the code reads is. *Correction:* this fixed only secondary networks with no alias, static IP or link. Those still failed one step later, fixed in the next release (see Unreleased). ## [0.22.4] - 2026-10-29