fix(network): replace decnet_lan when driver differs (macvlan<->ipvlan)
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.
This commit is contained in:
@@ -126,22 +126,46 @@ def allocate_ips(
|
|||||||
# Docker MACVLAN network
|
# Docker MACVLAN network
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
|
|
||||||
def create_macvlan_network(
|
def _ensure_network(
|
||||||
client: docker.DockerClient,
|
client: docker.DockerClient,
|
||||||
|
*,
|
||||||
|
driver: str,
|
||||||
interface: str,
|
interface: str,
|
||||||
subnet: str,
|
subnet: str,
|
||||||
gateway: str,
|
gateway: str,
|
||||||
ip_range: str,
|
ip_range: str,
|
||||||
|
extra_options: dict | None = None,
|
||||||
) -> None:
|
) -> None:
|
||||||
"""Create the MACVLAN Docker network. No-op if it already exists."""
|
"""Create the decnet docker network with ``driver``, replacing any
|
||||||
existing = [n.name for n in client.networks.list()]
|
existing network of the same name that was built with a different driver.
|
||||||
if MACVLAN_NETWORK_NAME in existing:
|
|
||||||
return
|
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(
|
client.networks.create(
|
||||||
name=MACVLAN_NETWORK_NAME,
|
name=MACVLAN_NETWORK_NAME,
|
||||||
driver="macvlan",
|
driver=driver,
|
||||||
options={"parent": interface},
|
options=options,
|
||||||
ipam=docker.types.IPAMConfig(
|
ipam=docker.types.IPAMConfig(
|
||||||
driver="default",
|
driver="default",
|
||||||
pool_configs=[
|
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(
|
def create_ipvlan_network(
|
||||||
client: docker.DockerClient,
|
client: docker.DockerClient,
|
||||||
interface: str,
|
interface: str,
|
||||||
@@ -162,25 +201,12 @@ def create_ipvlan_network(
|
|||||||
gateway: str,
|
gateway: str,
|
||||||
ip_range: str,
|
ip_range: str,
|
||||||
) -> None:
|
) -> None:
|
||||||
"""Create an IPvlan L2 Docker network. No-op if it already exists."""
|
"""Create an IPvlan L2 Docker network, replacing a macvlan-driver one of
|
||||||
existing = [n.name for n in client.networks.list()]
|
the same name if necessary (parent-NIC can't host both drivers)."""
|
||||||
if MACVLAN_NETWORK_NAME in existing:
|
_ensure_network(
|
||||||
return
|
client, driver="ipvlan", interface=interface,
|
||||||
|
subnet=subnet, gateway=gateway, ip_range=ip_range,
|
||||||
client.networks.create(
|
extra_options={"ipvlan_mode": "l2"},
|
||||||
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,
|
|
||||||
)
|
|
||||||
],
|
|
||||||
),
|
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
@@ -204,10 +230,14 @@ def _require_root() -> None:
|
|||||||
def setup_host_macvlan(interface: str, host_macvlan_ip: str, decky_ip_range: str) -> 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.
|
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()
|
_require_root()
|
||||||
|
|
||||||
|
_run(["ip", "link", "del", HOST_IPVLAN_IFACE], check=False)
|
||||||
|
|
||||||
# Check if interface already exists
|
# Check if interface already exists
|
||||||
result = _run(["ip", "link", "show", HOST_MACVLAN_IFACE], check=False)
|
result = _run(["ip", "link", "show", HOST_MACVLAN_IFACE], check=False)
|
||||||
if result.returncode != 0:
|
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:
|
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.
|
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()
|
_require_root()
|
||||||
|
|
||||||
|
_run(["ip", "link", "del", HOST_MACVLAN_IFACE], check=False)
|
||||||
|
|
||||||
result = _run(["ip", "link", "show", HOST_IPVLAN_IFACE], check=False)
|
result = _run(["ip", "link", "show", HOST_IPVLAN_IFACE], check=False)
|
||||||
if result.returncode != 0:
|
if result.returncode != 0:
|
||||||
_run(["ip", "link", "add", HOST_IPVLAN_IFACE, "link", interface, "type", "ipvlan", "mode", "l2"])
|
_run(["ip", "link", "add", HOST_IPVLAN_IFACE, "link", interface, "type", "ipvlan", "mode", "l2"])
|
||||||
|
|||||||
@@ -76,11 +76,12 @@ class TestIpsToRange:
|
|||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
|
|
||||||
class TestCreateMacvlanNetwork:
|
class TestCreateMacvlanNetwork:
|
||||||
def _make_client(self, existing=None):
|
def _make_client(self, existing=None, existing_driver="macvlan"):
|
||||||
client = MagicMock()
|
client = MagicMock()
|
||||||
nets = [MagicMock(name=n) for n in (existing or [])]
|
nets = [MagicMock(name=n) for n in (existing or [])]
|
||||||
for net, n in zip(nets, (existing or [])):
|
for net, n in zip(nets, (existing or [])):
|
||||||
net.name = n
|
net.name = n
|
||||||
|
net.attrs = {"Driver": existing_driver, "Containers": {}}
|
||||||
client.networks.list.return_value = nets
|
client.networks.list.return_value = nets
|
||||||
return client
|
return client
|
||||||
|
|
||||||
@@ -104,11 +105,12 @@ class TestCreateMacvlanNetwork:
|
|||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
|
|
||||||
class TestCreateIpvlanNetwork:
|
class TestCreateIpvlanNetwork:
|
||||||
def _make_client(self, existing=None):
|
def _make_client(self, existing=None, existing_driver="ipvlan"):
|
||||||
client = MagicMock()
|
client = MagicMock()
|
||||||
nets = [MagicMock(name=n) for n in (existing or [])]
|
nets = [MagicMock(name=n) for n in (existing or [])]
|
||||||
for net, n in zip(nets, (existing or [])):
|
for net, n in zip(nets, (existing or [])):
|
||||||
net.name = n
|
net.name = n
|
||||||
|
net.attrs = {"Driver": existing_driver, "Containers": {}}
|
||||||
client.networks.list.return_value = nets
|
client.networks.list.return_value = nets
|
||||||
return client
|
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")
|
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()
|
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):
|
def test_uses_same_network_name_as_macvlan(self):
|
||||||
"""Both drivers share the same logical network name so compose files are identical."""
|
"""Both drivers share the same logical network name so compose files are identical."""
|
||||||
client = self._make_client([])
|
client = self._make_client([])
|
||||||
@@ -154,8 +168,10 @@ class TestSetupHostMacvlan:
|
|||||||
mock_run.return_value = MagicMock(returncode=0)
|
mock_run.return_value = MagicMock(returncode=0)
|
||||||
setup_host_macvlan("eth0", "192.168.1.5", "192.168.1.96/27")
|
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]
|
calls = [c[0][0] for c in mock_run.call_args_list]
|
||||||
# "ip link add <iface> link ..." should not be called when iface exists
|
# "ip link add <iface> 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)
|
# (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)
|
@patch("decnet.network.os.geteuid", return_value=1)
|
||||||
def test_requires_root(self, _):
|
def test_requires_root(self, _):
|
||||||
@@ -182,9 +198,13 @@ class TestSetupHostIpvlan:
|
|||||||
def test_uses_ipvlan_iface_name(self, mock_run, _):
|
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)
|
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")
|
setup_host_ipvlan("wlan0", "192.168.1.5", "192.168.1.96/27")
|
||||||
calls = [str(c) for c in mock_run.call_args_list]
|
calls = [c[0][0] for c in mock_run.call_args_list]
|
||||||
assert any(HOST_IPVLAN_IFACE in c for c in calls)
|
# Primary interface created is the ipvlan slave.
|
||||||
assert not any(HOST_MACVLAN_IFACE in c for c in calls)
|
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)
|
@patch("decnet.network.os.geteuid", return_value=1)
|
||||||
def test_requires_root(self, _):
|
def test_requires_root(self, _):
|
||||||
|
|||||||
Reference in New Issue
Block a user