From df0c007dd8586c155bbf0305481d185450a2b379 Mon Sep 17 00:00:00 2001 From: tyderian1978 Date: Tue, 3 Nov 2026 11:18:17 +1100 Subject: [PATCH] sources: assign an IP that moved to another machine in the run that sees the move When an address moves from one running machine to another (a Windows failover cluster's or SQL listener's address, a keepalived/ucarp VIP, an address handed to a replacement VM), the next run logged Current interface 'X' for IP 'a.b.c.d' and this one 'Y' are both enabled. IP assignment skipped because it is unclear which one is the correct one! and left the address assigned to nothing, tagged orphaned, until the run after that assigned it to the new machine. The old machine no longer reports the address, so add_update_interface() queues its removal (remove_interface_association() -> unset_attribute()). That only records the removal for the NetBox update phase; the object's data still names the old interface. The matching loop for the new machine, and the reevaluation pass meant for exactly this case, therefore kept seeing the address on an enabled interface of another active machine. Treat an address whose assignment is queued for removal as free, and drop the queued removal when it is assigned again, so NetBox gets a single PATCH to the new interface. An address both machines still report stays ambiguous and is skipped as before. --- module/netbox/object_classes.py | 6 + module/sources/common/source_base.py | 7 +- tests/test_ip_moved_between_machines.py | 163 ++++++++++++++++++++++++ 3 files changed, 174 insertions(+), 2 deletions(-) create mode 100644 tests/test_ip_moved_between_machines.py diff --git a/module/netbox/object_classes.py b/module/netbox/object_classes.py index c382d326..1264254e 100644 --- a/module/netbox/object_classes.py +++ b/module/netbox/object_classes.py @@ -2233,6 +2233,12 @@ def update(self, data=None, read_from_netbox=False, source=None): if grab(previous_ip_device_vm, "data.primary_ip6") is self: is_primary_ipv6_of_previous_device = True + # a new assignment replaces a removal queued earlier in this run (the IP moved to + # another interface); otherwise the IP would be unassigned first and reassigned after + for attribute_name in ["assigned_object_id", "assigned_object_type"]: + if attribute_name in self.unset_items: + self.unset_items.remove(attribute_name) + if not isinstance(assigned_object, NetBoxObject): data["assigned_object_id"] = \ diff --git a/module/sources/common/source_base.py b/module/sources/common/source_base.py index 7538e5de..8db8dca2 100644 --- a/module/sources/common/source_base.py +++ b/module/sources/common/source_base.py @@ -646,8 +646,11 @@ def add_update_interface(self, interface_object, device_object, interface_data, f"possible VRF '{possible_ip_vrf_str}'") continue - # IP address is not assigned to any interface - if not isinstance(current_ip_nic, (NBInterface, NBVMInterface)): + # IP address is not assigned to any interface, or the interface it is assigned to + # stopped reporting it in this run and its removal is queued (it moved, e.g. a + # failover address): either way it is free to be assigned here + if not isinstance(current_ip_nic, (NBInterface, NBVMInterface)) or \ + "assigned_object_id" in ip.unset_items: this_ip_object = ip break diff --git a/tests/test_ip_moved_between_machines.py b/tests/test_ip_moved_between_machines.py new file mode 100644 index 00000000..5e32a5ba --- /dev/null +++ b/tests/test_ip_moved_between_machines.py @@ -0,0 +1,163 @@ +# -*- coding: utf-8 -*- +# Copyright (c) 2020 - 2026 netbox-sync team. All rights reserved. +# +# netbox-sync.py +# +# This work is licensed under the terms of the MIT license. +# For a copy, see file LICENSE.txt included in this +# repository or visit: . + +""" +An address that moves from one running machine to another (a failover cluster's +address, a keepalived/ucarp VIP, an address handed to a replacement VM) has to follow +in the run that sees the move. The old machine no longer reports it, so its removal +is queued; that queued removal must make the address free for the new machine, +whichever of the two is parsed first. An address both machines still report stays +ambiguous and is skipped, as before. +""" +from types import SimpleNamespace + +import pytest + +from module.netbox.inventory import NetBoxInventory +from module.netbox.object_classes import ( + NBSite, NBClusterType, NBCluster, NBVM, NBVMInterface, NBIPAddress, +) +from module.sources.common.source_base import SourceBase + +SHARED = "10.0.0.5/24" +OWN_A = "10.0.0.6/24" +OWN_B = "10.0.0.7/24" + +# vmware object whose guest tools report as running, so IP handling is not skipped +_VM_TOOLS_RUNNING = SimpleNamespace(guest=SimpleNamespace(toolsRunningStatus="guestToolsRunning")) + + +@pytest.fixture +def inventory(): + def _reset(): + inv = NetBoxInventory() + inv.base_structure = {} + inv.source_list = [] + inv.init() + inv.netbox_api_version = "4.0.0" + return inv + + inv = _reset() + yield inv + _reset() + + +def _make_source(inventory): + src = SourceBase() + src.inventory = inventory + src.name = "test" + src.source_tag = "Source: test" + src.settings = SimpleNamespace( + skip_fhrp_group_ips=False, + preserve_primary_ips=False, + ip_tenant_inheritance_order=["disabled"], + disable_vlan_sync=True, + vlan_group_relation_by_id=None, + vlan_group_relation_by_name=None, + vlan_sync_exclude_by_id=None, + vlan_sync_exclude_by_name=None, + ) + src.return_longest_matching_prefix_for_ip = lambda *a, **k: None + return src + + +def _vm_and_nic(inv, name): + site = inv.add_object(NBSite, data={"name": "site1"}, read_from_netbox=True) + ctype = inv.add_object(NBClusterType, data={"name": "vmware"}, read_from_netbox=True) + cluster = inv.add_object(NBCluster, data={"name": "c1", "type": ctype, "scope": site}, + read_from_netbox=True) + vm = inv.add_object(NBVM, data={"name": name, "cluster": cluster, "status": "active"}, + read_from_netbox=True) + nic = inv.add_object(NBVMInterface, data={"name": "eth0", "virtual_machine": vm, "enabled": True}, + read_from_netbox=True) + return vm, nic + + +def _add_ip(inv, nic, address=SHARED): + return inv.add_object(NBIPAddress, data={ + "address": address, + "assigned_object_type": "virtualization.vminterface", + "assigned_object_id": nic, + }, read_from_netbox=True) + + +def _sync(src, vm, nic, addresses): + _iface, ip_objects = src.add_update_interface( + interface_object=nic, device_object=vm, + interface_data={"name": "eth0"}, interface_ips=addresses, + vmware_object=_VM_TOOLS_RUNNING, + ) + return ip_objects + + +@pytest.fixture +def two_machines(inventory): + # NetBox has the shared address on vm-a; in vCenter it has moved to vm-b + vm_a, nic_a = _vm_and_nic(inventory, "vm-a") + vm_b, nic_b = _vm_and_nic(inventory, "vm-b") + shared = _add_ip(inventory, nic_a) + return SimpleNamespace(src=_make_source(inventory), vm_a=vm_a, nic_a=nic_a, + vm_b=vm_b, nic_b=nic_b, shared=shared) + + +def _assert_moved_to_b(m, ip_objects): + assert m.shared in ip_objects, "the moved address was not assigned to the new machine" + assert m.shared.get_interface() is m.nic_b + assert m.shared not in m.nic_a.get_ip_addresses() + # one PATCH to the new interface: no queued removal left to send first + assert "assigned_object_id" in m.shared.updated_items + assert "assigned_object_id" not in m.shared.unset_items + assert "assigned_object_type" not in m.shared.unset_items + assert len(list(m.src.inventory.get_all_items(NBIPAddress))) == 3, "a duplicate address was created" + + +def test_moved_address_follows_when_old_machine_is_parsed_first(two_machines): + m = two_machines + + _sync(m.src, m.vm_a, m.nic_a, [OWN_A]) # vm-a no longer reports it: removal queued + ip_objects = _sync(m.src, m.vm_b, m.nic_b, [SHARED, OWN_B]) + + _assert_moved_to_b(m, ip_objects) + + +def test_moved_address_follows_on_reevaluation_when_new_machine_is_parsed_first(two_machines): + m = two_machines + + # vm-b first: the address is still on vm-a's enabled interface, so it is skipped for now + first = _sync(m.src, m.vm_b, m.nic_b, [SHARED, OWN_B]) + assert m.shared not in first + _sync(m.src, m.vm_a, m.nic_a, [OWN_A]) # removal queued + # the reevaluation pass parses vm-b again + ip_objects = _sync(m.src, m.vm_b, m.nic_b, [SHARED, OWN_B]) + + _assert_moved_to_b(m, ip_objects) + + +def test_address_reported_by_both_machines_is_still_skipped(two_machines): + # control: while vm-a still reports the address it stays ambiguous and stays on vm-a + m = two_machines + + _sync(m.src, m.vm_a, m.nic_a, [SHARED, OWN_A]) + ip_objects = _sync(m.src, m.vm_b, m.nic_b, [SHARED, OWN_B]) + + assert m.shared not in ip_objects + assert m.shared.get_interface() is m.nic_a + assert m.shared.unset_items == [] + + +def test_moved_primary_address_is_unset_on_the_old_machine(two_machines): + # control: the old machine still loses the address as its primary IP + m = two_machines + m.vm_a.data["primary_ip4"] = m.shared + + _sync(m.src, m.vm_a, m.nic_a, [OWN_A]) + _sync(m.src, m.vm_b, m.nic_b, [SHARED, OWN_B]) + + assert m.shared.get_interface() is m.nic_b + assert "primary_ip4" in m.vm_a.unset_items