From 72865693c6ac921476445b1c7d36a93d5d9f29e4 Mon Sep 17 00:00:00 2001 From: Sergey Sannikov Date: Thu, 10 Sep 2026 02:39:43 +0400 Subject: [PATCH] fix: do not adopt a same-named cluster another source keeps in a different site (#501) --- module/sources/vmware/connection.py | 32 +++++ tests/test_vmware_cluster_matching.py | 183 ++++++++++++++++++++++++++ 2 files changed, 215 insertions(+) create mode 100644 tests/test_vmware_cluster_matching.py diff --git a/module/sources/vmware/connection.py b/module/sources/vmware/connection.py index 26ebbb9..7670a7c 100644 --- a/module/sources/vmware/connection.py +++ b/module/sources/vmware/connection.py @@ -1504,6 +1504,29 @@ def add_datacenter(self, obj): self.add_object_to_cache(obj, self.inventory.add_update_object(NBClusterGroup, data=object_data, source=self)) + def object_synced_by_other_source(self, nb_object): + """ + Check if a NetBox object is maintained by a different configured source. During + this run that is the source which touched the object, for objects read from NetBox + the source tags decide. + + Parameters + ---------- + nb_object: NetBoxObject + object to check + + Returns + ------- + bool: True if another configured source synced this object + """ + + if nb_object.source is not None: + return nb_object.source is not self + + other_source_tags = [x.source_tag for x in self.inventory.source_list if x is not self] + + return len(set(nb_object.get_tags()).intersection(other_source_tags)) > 0 + def add_cluster(self, obj): """ Add a vCenter cluster as a NBCluster to NetBox. Cluster name is checked against @@ -1598,6 +1621,15 @@ def add_cluster(self, obj): if grab(cluster_candidate, "data.name") != name: continue + # a cluster which a different source keeps in a different site is not this cluster. + # NetBox refuses to move a cluster away from the site of its hosts, so adopting it + # would fail on every run and attach this vCenter's hosts to the other cluster. + if site_name is not None and cluster_candidate.get_site_name() not in [None, site_name] and \ + self.object_synced_by_other_source(cluster_candidate) is True: + log.debug2(f"Skipping cluster '{name}' in site '{cluster_candidate.get_site_name()}' " + f"as it is synced by a different source") + continue + if site_name is not None: # try to find a cluster with matching site if cluster_candidate.get_site_name() == site_name: diff --git a/tests/test_vmware_cluster_matching.py b/tests/test_vmware_cluster_matching.py new file mode 100644 index 0000000..6eaba4b --- /dev/null +++ b/tests/test_vmware_cluster_matching.py @@ -0,0 +1,183 @@ +""" +Regression tests for matching a vCenter cluster to an existing NetBox cluster when several +VMware sources are configured (issue #501, related to #472). + +Different vCenters commonly carry the same datacenter and cluster names. add_cluster() used +to adopt a same-named cluster which another source had already scoped to a different site. +NetBox rejects the resulting re-scope ("N devices are assigned as hosts for this cluster but +are not in site ...") on every run and the second vCenter's hosts end up attached to the +first vCenter's cluster. + +These tests drive the real VMWareHandler.add_datacenter()/add_cluster() against the in-memory +NetBoxInventory with minimal pyVmomi-typed stand-ins, so neither a vCenter nor a NetBox is +needed. +""" +import re +from types import SimpleNamespace + +from pyVmomi import vim + +from module.netbox.object_classes import NBCluster, NBClusterGroup, NBClusterType, NBSite +from module.sources.vmware.connection import VMWareHandler + + +class _Datacenter(vim.Datacenter): + """vim.Datacenter with a plain name and no vCenter connection behind it.""" + + def __init__(self, moid, name): + super().__init__(moid) + object.__setattr__(self, "_name", name) + + name = property(lambda self: self._name) + + +class _Cluster(vim.ClusterComputeResource): + """vim.ClusterComputeResource with a plain name and parent.""" + + def __init__(self, moid, name, parent): + super().__init__(moid) + object.__setattr__(self, "_name", name) + object.__setattr__(self, "_parent", parent) + + name = property(lambda self: self._name) + parent = property(lambda self: self._parent) + + +def _relation(assigned_name): + """A parsed '.* = ' relation, or no relation at all.""" + if assigned_name is None: + return [] + return [{"object_regex": re.compile(".*"), "assigned_name": assigned_name}] + + +def _make_source(inventory, name, site=None, source_name_as_group=False): + """ + A VMWareHandler carrying just the state add_datacenter()/add_cluster() touch, + registered with the inventory like instantiate_sources() does. + """ + src = VMWareHandler.__new__(VMWareHandler) + src.inventory = inventory + src.name = name + src.source_tag = f"Source: {name}" + src.site_name = f"vCenter: {name}" + src.tag_session = None + src.object_cache = {} + src.recursion_level = 0 + src.settings = SimpleNamespace( + set_source_name_as_cluster_group=source_name_as_group, + strip_host_domain_name=False, + cluster_include_filter=None, + cluster_exclude_filter=None, + cluster_site_relation=_relation(site), + cluster_scope_type_relation=[], + cluster_scope_id_relation=[], + cluster_tenant_relation=[], + cluster_tag_relation=[], + cluster_tag_source=None, + ) + inventory.add_source(src) + return src + + +def _sync_cluster(source, dc_name="Datacenter01", cluster_name="Cluster01"): + """Run one vCenter's datacenter and cluster through the source, return the NBCluster it settled on.""" + datacenter = _Datacenter(f"datacenter-{source.name}", dc_name) + cluster = _Cluster(f"domain-{source.name}", cluster_name, datacenter) + source.add_datacenter(datacenter) + source.add_cluster(cluster) + return source.get_object_from_cache(cluster) + + +def _netbox_cluster(inventory, nb_id, name, group_name, site_name, source_tags): + """A cluster as it looks after being read from NetBox: id, group, site scope and source tags.""" + group = inventory.add_update_object(NBClusterGroup, data={"name": group_name}, read_from_netbox=True) + site = inventory.add_update_object(NBSite, data={"name": site_name}, read_from_netbox=True) + cluster_type = inventory.add_update_object(NBClusterType, data={"name": "VMware ESXi"}, read_from_netbox=True) + cluster = inventory.add_object(NBCluster, data={ + "id": nb_id, + "name": name, + "type": cluster_type, + "group": group, + "scope_type": "dcim.site", + "scope_id": site, + }, read_from_netbox=True) + cluster.add_tags(["NetBox-synced"] + list(source_tags)) + cluster.updated_items = [] + return cluster + + +def test_first_run_creates_one_cluster_per_vcenter(inventory): + # issue #501: both vCenters have Datacenter01/Cluster01, each source scopes it to its own site + src_a = _make_source(inventory, "vc-sr1", site="SR1") + src_b = _make_source(inventory, "vc-sr2", site="SR2") + + cluster_a = _sync_cluster(src_a) + cluster_b = _sync_cluster(src_b) + + assert cluster_b is not cluster_a + assert len(inventory.get_all_items(NBCluster)) == 2 + assert cluster_a.get_site_name() == "SR1" + assert cluster_b.get_site_name() == "SR2" + + +def test_existing_clusters_are_matched_by_site_not_by_shared_datacenter_name(inventory): + # steady state of #501: both clusters exist in NetBox, the other vCenter's cluster is listed + # first and shares the cluster group (datacenter name) + src_a = _make_source(inventory, "vc-sr1", site="SR1") + src_b = _make_source(inventory, "vc-sr2", site="SR2") + nb_a = _netbox_cluster(inventory, 3, "Cluster01", "Datacenter01", "SR1", [src_a.source_tag]) + nb_b = _netbox_cluster(inventory, 4, "Cluster01", "Datacenter01", "SR2", [src_b.source_tag]) + + assert _sync_cluster(src_b) is nb_b + assert _sync_cluster(src_a) is nb_a + assert len(inventory.get_all_items(NBCluster)) == 2 + assert nb_a.get_site_name() == "SR1" + assert nb_b.get_site_name() == "SR2" + + +def test_cluster_already_shared_by_both_sources_stays_at_its_site(inventory): + # the state #501 leaves behind: one cluster tagged by both sources, scoped to the site of the + # hosts it holds. The source configured for the other site must not try to re-scope it. + src_a = _make_source(inventory, "vc-sr1", site="SR1") + src_b = _make_source(inventory, "vc-sr2", site="SR2") + shared = _netbox_cluster(inventory, 3, "Cluster01", "Datacenter01", "SR2", + [src_a.source_tag, src_b.source_tag]) + + cluster_a = _sync_cluster(src_a) + + assert cluster_a is not shared + assert shared.get_site_name() == "SR2" + assert shared.updated_items == [] + assert _sync_cluster(src_b) is shared + + +def test_single_source_still_moves_its_cluster_to_a_new_site(inventory): + # one source whose cluster_site_relation changed: the cluster is re-scoped, not duplicated + src = _make_source(inventory, "vc", site="SR2") + existing = _netbox_cluster(inventory, 3, "Cluster01", "Datacenter01", "SR1", [src.source_tag]) + + assert _sync_cluster(src) is existing + assert existing.get_site_name() == "SR2" + assert len(inventory.get_all_items(NBCluster)) == 1 + + +def test_cluster_of_a_source_no_longer_configured_is_adopted(inventory): + # a renamed source section: the tag on the cluster belongs to no configured source, so the + # cluster is adopted and re-scoped as before + src = _make_source(inventory, "vc-new", site="SR2") + existing = _netbox_cluster(inventory, 3, "Cluster01", "Datacenter01", "SR1", ["Source: vc-old"]) + + assert _sync_cluster(src) is existing + assert existing.get_site_name() == "SR2" + assert len(inventory.get_all_items(NBCluster)) == 1 + + +def test_other_sources_cluster_without_site_is_still_adopted(inventory): + # nothing tells the clusters apart when the first source assigns no site: unchanged behaviour + src_a = _make_source(inventory, "vc-a") + src_b = _make_source(inventory, "vc-b", site="SR2") + + cluster_a = _sync_cluster(src_a) + + assert _sync_cluster(src_b) is cluster_a + assert len(inventory.get_all_items(NBCluster)) == 1