Fix /pull network reconnect for aliases, static IPs and links

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
 

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]:

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()

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"]