From 01f65c6a74b865c772408deb2d0b41d35ff4ecea Mon Sep 17 00:00:00 2001 From: Marco Sinhoreli Date: Wed, 29 Apr 2026 12:09:25 +0200 Subject: [PATCH] network: ExternalGuestNetworkGuru should defer to specialized gurus MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ExternalGuestNetworkGuru.canHandle currently accepts any Advanced / Guest / Isolated|L2 offering whose physical network advertises GRE / L3 / VLAN as one of its isolation methods, regardless of the provider list on the offering. When the same physical network also advertises a SDN-specific isolation method (OVN, NSX, NETRIS, ...), operators routinely keep VLAN registered alongside it for legacy offerings — and that single fact is enough to make this guru claim modern, specialized offerings too. NetworkOrchestrator.setupNetwork iterates every NetworkGuru and persists one NetworkVO row per matching guru. With the current canHandle, the specialized guru (e.g. OvnGuestNetworkGuru) creates the real row that gets implemented, while ExternalGuestNetworkGuru silently creates a second, parallel row that stays in Allocated state forever: no broadcast_uri, no implement() call, but visible in listNetworks and the UI as a phantom tier / network. This breaks user-visible state in two ways: - listNetworks returns duplicate entries for every isolated network or VPC tier on a zone whose physical network keeps VLAN registered alongside the specialized isolation method. - Cleanup paths can leave the orphaned row behind even after the real network is destroyed, because it was never linked to any backend state. The fix is to make ExternalGuestNetworkGuru bail out of canHandle when the offering binds any of its services to a provider that ships its own dedicated NetworkGuru: Ovn, Netris, Nsx, Tungsten, BigSwitchBcf, NiciraNvp, Opendaylight, BrocadeVcs, Ovs. Those providers have their own canHandle that already gates on their isolation method *and* their provider in networkOfferingServiceMapDao — so the specialized guru is always the canonical claimant for those offerings. The list is intentionally explicit and conservative: it covers every in-tree guest NetworkGuru that lives outside ExternalGuestNetworkGuru and that targets the same Isolated / L2 offerings on the Advanced zone. SspGuestNetworkGuru and VxlanGuestNetworkGuru are not in the list because they use isolation methods (SSP, VXLAN) that ExternalGuestNetworkGuru never matched in the first place, so there was no overlap to fix there. Lab-validated end to end on an OVN-backed zone whose physical network had OVN+VLAN registered: every newly created VPC tier and isolated network now produces exactly one networks row instead of two. --- .../guru/ExternalGuestNetworkGuru.java | 40 +++++++++++++++++-- 1 file changed, 36 insertions(+), 4 deletions(-) diff --git a/server/src/main/java/com/cloud/network/guru/ExternalGuestNetworkGuru.java b/server/src/main/java/com/cloud/network/guru/ExternalGuestNetworkGuru.java index 49404068051c..96eb0a22d4fd 100644 --- a/server/src/main/java/com/cloud/network/guru/ExternalGuestNetworkGuru.java +++ b/server/src/main/java/com/cloud/network/guru/ExternalGuestNetworkGuru.java @@ -93,18 +93,50 @@ public ExternalGuestNetworkGuru() { _isolationMethods = new IsolationMethod[] {new IsolationMethod("GRE"), new IsolationMethod("L3"), new IsolationMethod("VLAN")}; } + /** + * Network providers that ship their own {@link com.cloud.network.guru.NetworkGuru}. When an + * offering binds any of its services to one of these providers we must let that provider's + * guru claim the network exclusively. Otherwise this generic guru also matches (because the + * physical network usually still carries a VLAN/GRE/L3 isolation method as a fallback) and + * persists a duplicate, never-implemented row alongside the real one — breaking idempotency + * and confusing both the UI and the cleanup paths. + * + *

Looked up by name (rather than referencing the {@link Provider} constants directly) so + * this list keeps compiling against branches where one of the optional providers is not yet + * present in the API module — e.g. {@code Provider.Ovn} only landed in 4.23.

+ */ + private static final java.util.Set SPECIALIZED_GUEST_GURU_PROVIDER_NAMES = + java.util.Collections.unmodifiableSet(new java.util.HashSet<>(java.util.Arrays.asList( + "Ovn", "Netris", "Nsx", "Tungsten", "BigSwitchBcf", + "NiciraNvp", "Opendaylight", "BrocadeVcs", "Ovs"))); + @Override protected boolean canHandle(NetworkOffering offering, final NetworkType networkType, final PhysicalNetwork physicalNetwork) { // This guru handles only Guest Isolated network that supports Source // nat service - if (networkType == NetworkType.Advanced && isMyTrafficType(offering.getTrafficType()) + if (!(networkType == NetworkType.Advanced && isMyTrafficType(offering.getTrafficType()) && (offering.getGuestType() == Network.GuestType.Isolated || offering.getGuestType() == GuestType.L2) - && isMyIsolationMethod(physicalNetwork) && !offering.isSystemOnly()) { - return true; - } else { + && isMyIsolationMethod(physicalNetwork) && !offering.isSystemOnly())) { logger.trace("We only take care of Guest networks of type " + GuestType.Isolated + " in zone of type " + NetworkType.Advanced); return false; } + // If any service in the offering is bound to a provider that owns its own NetworkGuru, + // bail. Otherwise both this guru and the specialized one would match the same offering + // (the physical network carries VLAN as a fallback isolation method alongside OVN/etc.), + // and setupNetwork would persist two NetworkVO rows — the specialized guru implements + // its row, this guru's row stays Allocated forever and clutters listNetworks. + java.util.List offeringProviders = networkOfferingServiceMapDao.getDistinctProviders(offering.getId()); + if (offeringProviders != null) { + for (String providerName : offeringProviders) { + if (providerName != null && SPECIALIZED_GUEST_GURU_PROVIDER_NAMES.contains(providerName)) { + logger.debug("Offering {} declares provider {} which owns a dedicated NetworkGuru; " + + "ExternalGuestNetworkGuru declines to handle.", + offering.getId(), providerName); + return false; + } + } + } + return true; } @Override