From 42b5e4cd061f480c8136baddd03b54722da3d9c3 Mon Sep 17 00:00:00 2001 From: anti Date: Sun, 19 Apr 2026 18:12:28 -0400 Subject: [PATCH] fix(network): replace decnet_lan when driver differs (macvlan<->ipvlan) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The create helpers short-circuited on name alone, so a prior macvlan deploy left Docker's decnet_lan network in place. A subsequent ipvlan deploy would no-op the network create, then container attach would try to add a macvlan port on enp0s3 that already had an ipvlan slave — EBUSY, agent 500, docker ps empty. Now: when the existing network's driver disagrees with the requested one, disconnect any live containers and DROP it before recreating. Parent-NIC can host one driver at a time. Also: setup_host_{macvlan,ipvlan} opportunistically delete the opposite host-side helper so we don't leave cruft across driver swaps. --- decnet/network.py | 90 +++++++++++++++++++++++++++++-------------- tests/test_network.py | 34 ++++++++++++---- 2 files changed, 89 insertions(+), 35 deletions(-) diff --git a/decnet/network.py b/decnet/network.py index aa88432..1d183be 100644 --- a/decnet/network.py +++ b/decnet/network.py @@ -126,22 +126,46 @@ def allocate_ips( # Docker MACVLAN network # --------------------------------------------------------------------------- -def create_macvlan_network( +def _ensure_network( client: docker.DockerClient, + *, + driver: str, interface: str, subnet: str, gateway: str, ip_range: str, + extra_options: dict | None = None, ) -> None: - """Create the MACVLAN Docker network. No-op if it already exists.""" - existing = [n.name for n in client.networks.list()] - if MACVLAN_NETWORK_NAME in existing: - return + """Create the decnet docker network with ``driver``, replacing any + existing network of the same name that was built with a different driver. + + Why the replace-on-driver-mismatch: macvlan and ipvlan slaves can't + coexist on the same parent interface. If an earlier run left behind a + macvlan-driver network and we're now asked for ipvlan (or vice versa), + short-circuiting on name alone leaves Docker attaching new containers + to the old driver and the host NIC ends up EBUSY on the next port + create. So: when driver disagrees, disconnect everything and DROP it. + """ + options = {"parent": interface} + if extra_options: + options.update(extra_options) + + for net in client.networks.list(names=[MACVLAN_NETWORK_NAME]): + if net.attrs.get("Driver") == driver: + return # right driver, leave it alone + # Wrong driver — tear it down. Disconnect any live containers first + # so `remove()` doesn't refuse with ErrNetworkInUse. + for cid in (net.attrs.get("Containers") or {}): + try: + net.disconnect(cid, force=True) + except docker.errors.APIError: + pass + net.remove() client.networks.create( name=MACVLAN_NETWORK_NAME, - driver="macvlan", - options={"parent": interface}, + driver=driver, + options=options, ipam=docker.types.IPAMConfig( driver="default", pool_configs=[ @@ -155,6 +179,21 @@ def create_macvlan_network( ) +def create_macvlan_network( + client: docker.DockerClient, + interface: str, + subnet: str, + gateway: str, + ip_range: str, +) -> None: + """Create the MACVLAN Docker network, replacing an ipvlan-driver one of + the same name if necessary (parent-NIC can't host both drivers).""" + _ensure_network( + client, driver="macvlan", interface=interface, + subnet=subnet, gateway=gateway, ip_range=ip_range, + ) + + def create_ipvlan_network( client: docker.DockerClient, interface: str, @@ -162,25 +201,12 @@ def create_ipvlan_network( gateway: str, ip_range: str, ) -> None: - """Create an IPvlan L2 Docker network. No-op if it already exists.""" - existing = [n.name for n in client.networks.list()] - if MACVLAN_NETWORK_NAME in existing: - return - - client.networks.create( - name=MACVLAN_NETWORK_NAME, - driver="ipvlan", - options={"parent": interface, "ipvlan_mode": "l2"}, - ipam=docker.types.IPAMConfig( - driver="default", - pool_configs=[ - docker.types.IPAMPool( - subnet=subnet, - gateway=gateway, - iprange=ip_range, - ) - ], - ), + """Create an IPvlan L2 Docker network, replacing a macvlan-driver one of + the same name if necessary (parent-NIC can't host both drivers).""" + _ensure_network( + client, driver="ipvlan", interface=interface, + subnet=subnet, gateway=gateway, ip_range=ip_range, + extra_options={"ipvlan_mode": "l2"}, ) @@ -204,10 +230,14 @@ def _require_root() -> None: def setup_host_macvlan(interface: str, host_macvlan_ip: str, decky_ip_range: str) -> None: """ Create a macvlan interface on the host so the deployer can reach deckies. - Idempotent — skips steps that are already done. + Idempotent — skips steps that are already done. Drops a stale ipvlan + host-helper first: the two drivers can share a parent NIC on paper but + leaving the opposite helper in place is just cruft after a driver swap. """ _require_root() + _run(["ip", "link", "del", HOST_IPVLAN_IFACE], check=False) + # Check if interface already exists result = _run(["ip", "link", "show", HOST_MACVLAN_IFACE], check=False) if result.returncode != 0: @@ -227,10 +257,14 @@ def teardown_host_macvlan(decky_ip_range: str) -> None: def setup_host_ipvlan(interface: str, host_ipvlan_ip: str, decky_ip_range: str) -> None: """ Create an IPvlan interface on the host so the deployer can reach deckies. - Idempotent — skips steps that are already done. + Idempotent — skips steps that are already done. Drops a stale macvlan + host-helper first so a prior macvlan deploy doesn't leave its slave + dangling on the parent NIC after the driver swap. """ _require_root() + _run(["ip", "link", "del", HOST_MACVLAN_IFACE], check=False) + result = _run(["ip", "link", "show", HOST_IPVLAN_IFACE], check=False) if result.returncode != 0: _run(["ip", "link", "add", HOST_IPVLAN_IFACE, "link", interface, "type", "ipvlan", "mode", "l2"]) diff --git a/tests/test_network.py b/tests/test_network.py index af99534..fa6a744 100644 --- a/tests/test_network.py +++ b/tests/test_network.py @@ -76,11 +76,12 @@ class TestIpsToRange: # --------------------------------------------------------------------------- class TestCreateMacvlanNetwork: - def _make_client(self, existing=None): + def _make_client(self, existing=None, existing_driver="macvlan"): client = MagicMock() nets = [MagicMock(name=n) for n in (existing or [])] for net, n in zip(nets, (existing or [])): net.name = n + net.attrs = {"Driver": existing_driver, "Containers": {}} client.networks.list.return_value = nets return client @@ -104,11 +105,12 @@ class TestCreateMacvlanNetwork: # --------------------------------------------------------------------------- class TestCreateIpvlanNetwork: - def _make_client(self, existing=None): + def _make_client(self, existing=None, existing_driver="ipvlan"): client = MagicMock() nets = [MagicMock(name=n) for n in (existing or [])] for net, n in zip(nets, (existing or [])): net.name = n + net.attrs = {"Driver": existing_driver, "Containers": {}} client.networks.list.return_value = nets return client @@ -126,6 +128,18 @@ class TestCreateIpvlanNetwork: create_ipvlan_network(client, "wlan0", "192.168.1.0/24", "192.168.1.1", "192.168.1.96/27") client.networks.create.assert_not_called() + def test_replaces_macvlan_network_with_ipvlan(self): + """If an old macvlan-driver net exists under the same name, remove+recreate. + Short-circuiting on name alone leaves Docker attaching containers to the + wrong driver — on the next port create the parent NIC goes EBUSY because + macvlan and ipvlan slaves can't share it.""" + client = self._make_client([MACVLAN_NETWORK_NAME], existing_driver="macvlan") + old_net = client.networks.list.return_value[0] + create_ipvlan_network(client, "wlan0", "192.168.1.0/24", "192.168.1.1", "192.168.1.96/27") + old_net.remove.assert_called_once() + client.networks.create.assert_called_once() + assert client.networks.create.call_args[1]["driver"] == "ipvlan" + def test_uses_same_network_name_as_macvlan(self): """Both drivers share the same logical network name so compose files are identical.""" client = self._make_client([]) @@ -154,8 +168,10 @@ class TestSetupHostMacvlan: mock_run.return_value = MagicMock(returncode=0) setup_host_macvlan("eth0", "192.168.1.5", "192.168.1.96/27") calls = [c[0][0] for c in mock_run.call_args_list] - # "ip link add link ..." should not be called when iface exists - assert not any("link" in cmd and "add" in cmd and HOST_MACVLAN_IFACE in cmd for cmd in calls) + # "ip link add link ..." should not be called when iface exists. + # (The opportunistic `ip link del decnet_ipvlan0` cleanup is allowed.) + add_cmds = [cmd for cmd in calls if cmd[:3] == ["ip", "link", "add"]] + assert not any(HOST_MACVLAN_IFACE in cmd for cmd in add_cmds) @patch("decnet.network.os.geteuid", return_value=1) def test_requires_root(self, _): @@ -182,9 +198,13 @@ class TestSetupHostIpvlan: def test_uses_ipvlan_iface_name(self, mock_run, _): mock_run.side_effect = lambda cmd, **kw: MagicMock(returncode=1) if "show" in cmd else MagicMock(returncode=0) setup_host_ipvlan("wlan0", "192.168.1.5", "192.168.1.96/27") - calls = [str(c) for c in mock_run.call_args_list] - assert any(HOST_IPVLAN_IFACE in c for c in calls) - assert not any(HOST_MACVLAN_IFACE in c for c in calls) + calls = [c[0][0] for c in mock_run.call_args_list] + # Primary interface created is the ipvlan slave. + assert any("add" in cmd and HOST_IPVLAN_IFACE in cmd for cmd in calls) + # The only macvlan reference allowed is the opportunistic `del` + # that cleans up a stale helper from a prior macvlan deploy. + macvlan_refs = [cmd for cmd in calls if HOST_MACVLAN_IFACE in cmd] + assert all(cmd[:3] == ["ip", "link", "del"] for cmd in macvlan_refs) @patch("decnet.network.os.geteuid", return_value=1) def test_requires_root(self, _):