fix(proxmox): keep a migrated guest on its existing inventory row
Excluding claimed rows from both fallbacks was too broad. The synthetic ieee embeds the host name, so a live migration or a node rename rewrites it for what is the same guest (`pve-pve1-802` → `pve-pve2-802`). Its exact match then missed, MAC and IP both skipped the existing row as claimed, and the import filed a second row — leaving the first orphaned on a host that no longer runs the guest. A MAC is an identity; an IP is not, and only the IP caused #419. So the MAC fallback now matches a claimed row too, and `_adopted_ieee` re-points it to the new host. Re-pointing is the half that matters beyond the duplicate: links are rebuilt keyed on the ieee, so a row left on the stale one stops resolving as a host→guest endpoint. A mesh ieee is a real hardware address and is never overwritten. Accepted trade-off, pinned by a test: two guests sharing a NIC MAC now collapse into one row. That is a misconfiguration which already breaks their networking, where the alternative orphans a row on every migration. ha-relevant: no
This commit is contained in:
committed by
Pouzor - Rémy Jardient
parent
29525bca87
commit
080de69543
@@ -339,15 +339,22 @@ async def _find_pending(
|
||||
|
||||
1. The synthetic ``ieee_address`` (``pve-{host}-{vmid}``). Unique per guest
|
||||
and the only key that identifies one.
|
||||
2. Else MAC, else IP — merging into a row an IP/ARP scan found first.
|
||||
2. Else the NIC MAC, else the IP — merging into a row an IP/ARP scan found
|
||||
first, or into this same guest under its previous host name.
|
||||
|
||||
An IP is not an identity: a duplicated static lease, a re-used DHCP address,
|
||||
two guests behind one NAT, or a container-bridge address several guests
|
||||
report to the agent all make one address describe several machines. So the
|
||||
fallback skips any row already claimed by a *different* Proxmox guest —
|
||||
matching one used to overwrite that guest's hostname, VMID and specs, and
|
||||
IP fallback skips any row already claimed by a Proxmox guest — matching one
|
||||
used to overwrite that guest's hostname, VMID and specs, and
|
||||
non-deterministically, since one flat OR with no ordering returned whichever
|
||||
row the database yielded first. Oldest row wins, so a re-import is stable.
|
||||
|
||||
A MAC *is* an identity, so its fallback matches a claimed row too. It has to:
|
||||
the synthetic ieee embeds the host name, and a live migration or a node
|
||||
rename changes it for what is the same guest (``pve-pve1-802`` →
|
||||
``pve-pve2-802``). Excluding claimed rows there would file the migrated
|
||||
guest as a new device and orphan its old row. See ``_adopted_ieee``.
|
||||
"""
|
||||
exact = (
|
||||
await db.execute(
|
||||
@@ -361,13 +368,17 @@ async def _find_pending(
|
||||
InventoryDevice.ieee_address.is_(None),
|
||||
~InventoryDevice.ieee_address.startswith(_PVE_IEEE_PREFIX),
|
||||
)
|
||||
for column, value in ((InventoryDevice.mac, mac), (InventoryDevice.ip, ip)):
|
||||
for column, value, claimed_ok in (
|
||||
(InventoryDevice.mac, mac, True),
|
||||
(InventoryDevice.ip, ip, False),
|
||||
):
|
||||
if not value:
|
||||
continue
|
||||
where = [column == value] if claimed_ok else [column == value, unclaimed]
|
||||
row = (
|
||||
await db.execute(
|
||||
select(InventoryDevice)
|
||||
.where(column == value, unclaimed)
|
||||
.where(*where)
|
||||
.order_by(InventoryDevice.discovered_at, InventoryDevice.id)
|
||||
)
|
||||
).scalars().first()
|
||||
@@ -376,6 +387,20 @@ async def _find_pending(
|
||||
return None
|
||||
|
||||
|
||||
def _adopted_ieee(current: str | None, ieee: str) -> str:
|
||||
"""The ``ieee_address`` a row should carry once this guest merges into it.
|
||||
|
||||
Re-point a row that already belongs to a Proxmox guest: matched by NIC MAC,
|
||||
it is this same guest under a stale host name (migrated, or the node was
|
||||
renamed). Links are rebuilt keyed on the ieee, so a row left on the old one
|
||||
stops resolving as a host→guest endpoint. A mesh ieee is a real hardware
|
||||
address and is never overwritten.
|
||||
"""
|
||||
if not current or current.startswith(_PVE_IEEE_PREFIX):
|
||||
return ieee
|
||||
return current
|
||||
|
||||
|
||||
def _new_pending(
|
||||
ieee: str,
|
||||
ip: str | None,
|
||||
@@ -432,7 +457,7 @@ def _refresh_pending(
|
||||
) -> None:
|
||||
# Compute sources before adopting the pve ieee (needs the pre-merge origin).
|
||||
pending.discovery_sources = _sources_after_merge(pending)
|
||||
pending.ieee_address = pending.ieee_address or ieee
|
||||
pending.ieee_address = _adopted_ieee(pending.ieee_address, ieee)
|
||||
pending.ip = ip or pending.ip
|
||||
pending.mac = pending.mac or mac
|
||||
pending.hostname = n.get("hostname") or pending.hostname
|
||||
@@ -464,7 +489,7 @@ async def _ensure_inventory_row(
|
||||
else:
|
||||
# Compute sources before adopting the pve ieee (needs the pre-merge origin).
|
||||
inv.discovery_sources = _sources_after_merge(inv)
|
||||
inv.ieee_address = inv.ieee_address or ieee
|
||||
inv.ieee_address = _adopted_ieee(inv.ieee_address, ieee)
|
||||
inv.ip = ip or inv.ip
|
||||
inv.mac = inv.mac or mac
|
||||
inv.hostname = n.get("hostname") or inv.hostname
|
||||
|
||||
@@ -40,14 +40,17 @@ def _host_node() -> dict:
|
||||
}
|
||||
|
||||
|
||||
def _guest_node(vmid: int, ip: str | None, status: str = "online", mac: str | None = None) -> dict:
|
||||
def _guest_node(
|
||||
vmid: int, ip: str | None, status: str = "online", mac: str | None = None,
|
||||
host: str = "pve1",
|
||||
) -> dict:
|
||||
return {
|
||||
"id": f"pve-pve1-{vmid}", "label": f"vm{vmid}", "type": "vm",
|
||||
"ieee_address": f"pve-pve1-{vmid}", "hostname": f"vm{vmid}", "ip": ip,
|
||||
"id": f"pve-{host}-{vmid}", "label": f"vm{vmid}", "type": "vm",
|
||||
"ieee_address": f"pve-{host}-{vmid}", "hostname": f"vm{vmid}", "ip": ip,
|
||||
"mac": mac,
|
||||
"status": status, "cpu_count": 2, "ram_gb": 4.0, "disk_gb": 32.0,
|
||||
"vendor": "Proxmox VE", "model": "QEMU", "vmid": vmid,
|
||||
"parent_ieee": "pve-node-pve1",
|
||||
"parent_ieee": f"pve-node-{host}",
|
||||
}
|
||||
|
||||
|
||||
@@ -341,6 +344,67 @@ async def test_persist_merges_the_oldest_scan_row_when_two_share_an_ip(db_sessio
|
||||
assert merged.mac == "aa:bb:cc:00:00:02" # the older row
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_persist_merges_a_guest_that_moved_to_another_host(db_session) -> None:
|
||||
# The synthetic ieee embeds the host name, so a live migration or a node
|
||||
# rename changes it for what is the same guest. The NIC MAC survives both:
|
||||
# merge and re-point the row, rather than filing a new device and orphaning
|
||||
# the old row (which would then never resolve as a host→guest link end).
|
||||
await _persist_pending_import(
|
||||
db_session, [_guest_node(802, "10.0.0.5", mac="bc:24:11:aa:bb:cc")], []
|
||||
)
|
||||
row = (await db_session.execute(select(InventoryDevice))).scalars().one()
|
||||
row.status = "approved"
|
||||
await db_session.commit()
|
||||
|
||||
await _persist_pending_import(
|
||||
db_session,
|
||||
[_guest_node(802, "10.0.0.5", mac="bc:24:11:aa:bb:cc", host="pve2")],
|
||||
[],
|
||||
)
|
||||
|
||||
rows = (await db_session.execute(select(InventoryDevice))).scalars().all()
|
||||
assert len(rows) == 1 # no duplicate, no orphan
|
||||
assert rows[0].ieee_address == "pve-pve2-802" # re-pointed to the new host
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_persist_never_repoints_a_mesh_ieee(db_session) -> None:
|
||||
# A zigbee/zwave ieee is a real hardware address. A Proxmox import that
|
||||
# merges into such a row by MAC must not overwrite it.
|
||||
db_session.add(InventoryDevice(
|
||||
id=str(uuid.uuid4()), ieee_address="0x00124b0022a1b2c3",
|
||||
mac="bc:24:11:aa:bb:cc", suggested_type="zigbee_device", status="pending",
|
||||
discovery_source="zigbee", discovery_sources=["zigbee"],
|
||||
))
|
||||
await db_session.commit()
|
||||
|
||||
await _persist_pending_import(
|
||||
db_session, [_guest_node(101, None, mac="bc:24:11:aa:bb:cc")], []
|
||||
)
|
||||
|
||||
rows = (await db_session.execute(select(InventoryDevice))).scalars().all()
|
||||
assert len(rows) == 1
|
||||
assert rows[0].ieee_address == "0x00124b0022a1b2c3"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_persist_merges_two_guests_that_share_a_nic_mac(db_session) -> None:
|
||||
# Accepted trade-off of matching on MAC: two guests configured with the same
|
||||
# NIC MAC collapse into one row. That is a misconfiguration which already
|
||||
# breaks their networking, and the alternative — excluding claimed rows from
|
||||
# the MAC fallback — orphans every migrated guest. Shared *IPs* stay split;
|
||||
# see test_persist_never_overwrites_another_guest_row_on_a_shared_ip.
|
||||
nodes = [
|
||||
_guest_node(802, "10.0.0.5", mac="bc:24:11:aa:bb:cc"),
|
||||
_guest_node(812, "10.0.0.6", mac="bc:24:11:aa:bb:cc"),
|
||||
]
|
||||
await _persist_pending_import(db_session, nodes, [])
|
||||
|
||||
rows = (await db_session.execute(select(InventoryDevice))).scalars().all()
|
||||
assert len(rows) == 1
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_persist_preserves_ip_tag_for_legacy_null_source_row(db_session) -> None:
|
||||
# Legacy inventory row from an old IP scan, before discovery_source(s) were
|
||||
|
||||
Reference in New Issue
Block a user