diff --git a/api/src/main/java/org/apache/cloudstack/extension/ExtensionHelper.java b/api/src/main/java/org/apache/cloudstack/extension/ExtensionHelper.java index 8f79583216a..84105dfe5ce 100644 --- a/api/src/main/java/org/apache/cloudstack/extension/ExtensionHelper.java +++ b/api/src/main/java/org/apache/cloudstack/extension/ExtensionHelper.java @@ -43,30 +43,6 @@ public interface ExtensionHelper { */ String NETWORK_SERVICE_CAPABILITIES_DETAIL_KEY = "network.service.capabilities"; - /** - * Detail key used by an OVS-backed NetworkOrchestrator extension to declare - * how its Logical Switch Port name should be matched against the OVS - * {@code external_ids:iface-id} written by libvirt on the hypervisor. - * - *

Currently supported value:

- * - * - *

If absent, the framework keeps the network's broadcast type unchanged - * (typically {@code Vlan}) and does not propagate {@code --nic-uuid}.

- */ - String VIF_BINDING_DETAIL_KEY = "vif.binding"; - - /** Value of {@link #VIF_BINDING_DETAIL_KEY} that selects the Lswitch path. */ - String VIF_BINDING_LSWITCH = "lswitch"; - String getExtensionScriptPath(Extension extension); /** diff --git a/framework/extensions/src/main/java/org/apache/cloudstack/framework/extensions/network/NetworkExtensionElement.java b/framework/extensions/src/main/java/org/apache/cloudstack/framework/extensions/network/NetworkExtensionElement.java index 19b713c834c..b2b8862a135 100644 --- a/framework/extensions/src/main/java/org/apache/cloudstack/framework/extensions/network/NetworkExtensionElement.java +++ b/framework/extensions/src/main/java/org/apache/cloudstack/framework/extensions/network/NetworkExtensionElement.java @@ -115,7 +115,9 @@ import com.cloud.vm.dao.NicDao; import com.cloud.vm.dao.UserVmDao; import com.cloud.vm.dao.VMInstanceDetailsDao; import com.google.gson.Gson; +import com.google.gson.JsonElement; import com.google.gson.JsonObject; +import com.google.gson.JsonParser; import org.apache.cloudstack.api.ApiConstants; import org.apache.cloudstack.engine.orchestration.service.NetworkOrchestrationService; @@ -124,6 +126,7 @@ import org.apache.cloudstack.extension.ExtensionHelper; import org.apache.cloudstack.extension.NetworkCustomActionProvider; import org.apache.cloudstack.framework.extensions.dao.ExtensionDetailsDao; import org.apache.cloudstack.resourcedetail.dao.VpcDetailsDao; +import org.apache.commons.lang3.EnumUtils; import org.apache.commons.lang3.StringUtils; import java.nio.charset.StandardCharsets; @@ -499,32 +502,14 @@ public class NetworkExtensionElement extends AdapterBase implements implArgs.add("--extension-ip"); implArgs.add(safeStr(extensionIp)); implArgs.addAll(vpcArgs); - boolean result = executeScript(network, CMD_IMPLEMENT_NETWORK, implArgs.toArray(new String[0])); + Pair result = executeScriptAndReturnOutput(network, CMD_IMPLEMENT_NETWORK, implArgs.toArray(new String[0])); - if (!result) { + if (!result.first()) { return false; } - // When the extension declares vif.binding=lswitch, also update the - // Network row itself so listNetworks / DB queries advertise the - // OVN-flavoured identifier instead of the cosmetic VLAN URI the - // GuestNetworkGuru allocated at design-time. Format follows the - // legacy ovn-plugin convention: ``ovn://cs-net-``. - if (isLswitchVifBinding(network)) { - try { - NetworkVO networkVo = networkDao.findById(network.getId()); - if (networkVo != null) { - URI ovnUri = URI.create("ovn://cs-net-" + network.getId()); - networkVo.setBroadcastDomainType(Networks.BroadcastDomainType.Lswitch); - networkVo.setBroadcastUri(ovnUri); - networkDao.update(networkVo.getId(), networkVo); - logger.debug("implement: applied Lswitch broadcast type and ovn:// URI to network {} per extension vif.binding hint", - network.getId()); - } - } catch (Exception e) { - logger.warn("Failed to persist OVN URI on network {}: {}", network.getId(), e.getMessage()); - } - } + // Update the network properties from the output + applyNetworkUpdateFromScriptOutput(network, result.second()); // Step 3: Configure source NAT for both VPC and non-VPC networks for // compatibility (other network-element providers may also implement VPC tiers). @@ -571,42 +556,8 @@ public class NetworkExtensionElement extends AdapterBase implements return false; } - // VIF binding hint -- when the extension declares vif.binding=lswitch, - // override the NicProfile's broadcast type so OvsVifDriver picks the - // Lswitch path on the KVM agent. That path already emits libvirt - // and - // libvirt sets external_ids:iface-id atomically with tap creation. - // No agent patch is required for this binding mode. - if (isLswitchVifBinding(network)) { - // Override broadcast type + URI on the NicProfile (in-memory), - // and persist the same to the underlying nics row so listNics - // / DB queries report consistent OVN identifiers instead of - // the stale VLAN URI the GuestNetworkGuru allocated at - // design-time. - URI ovnUri = null; - try { - ovnUri = URI.create("ovn://cs-net-" + network.getId()); - } catch (Exception e) { - logger.warn("Failed to build OVN URI for NIC {}: {}", nic.getId(), e.getMessage()); - } - nic.setBroadcastType(Networks.BroadcastDomainType.Lswitch); - if (ovnUri != null) { - nic.setBroadcastUri(ovnUri); - nic.setIsolationUri(ovnUri); - try { - NicVO nicVo = nicDao.findById(nic.getId()); - if (nicVo != null) { - nicVo.setBroadcastUri(ovnUri); - nicVo.setIsolationUri(ovnUri); - nicDao.update(nicVo.getId(), nicVo); - } - } catch (Exception e) { - logger.warn("Failed to persist OVN URI on nics row {}: {}", nic.getId(), e.getMessage()); - } - } - logger.debug("prepare: applied Lswitch broadcast type and ovn:// URI to NIC {} (uuid={}) on network {} per extension vif.binding hint", - nic.getId(), nic.getUuid(), network.getId()); - } + // Sync nic with network + applyNicUpdateFromNetwork(network, nic); final NetworkOfferingVO offering = networkOfferingDao.findById(network.getNetworkOfferingId()); implement(network, offering, dest, context); @@ -615,49 +566,35 @@ public class NetworkExtensionElement extends AdapterBase implements } /** - * Returns {@code true} when the extension that owns the given network - * declares {@code vif.binding=lswitch} in its {@code extension_details}. - * Used by {@link #prepare(Network, NicProfile, VirtualMachineProfile, - * DeployDestination, ReservationContext)} to switch the NIC's - * {@link Networks.BroadcastDomainType} to {@code Lswitch} so the KVM - * agent's existing {@code OvsVifDriver} Lswitch path is exercised -- - * see the framework README for the full contract. + * Returns {@code ["--nic-uuid", ""]} when the extension so the script + * can use the same UUID when needed. */ - private boolean isLswitchVifBinding(Network network) { - try { - Extension extension = resolveExtension(network); - if (extension == null) { - return false; - } - Map details = extensionDetailsDao.listDetailsKeyPairs(extension.getId()); - if (details == null) { - return false; - } - String vifBinding = details.get(ExtensionHelper.VIF_BINDING_DETAIL_KEY); - return ExtensionHelper.VIF_BINDING_LSWITCH.equalsIgnoreCase(vifBinding); - } catch (Exception e) { - logger.debug("Failed to resolve vif.binding for network {}: {}", network.getId(), e.getMessage()); - return false; - } - } - - /** - * Returns {@code ["--nic-uuid", ""]} when the extension prefers the - * Lswitch VIF binding path so the script can use the same UUID as the LSP - * name (matching the {@code interfaceid} that {@code OvsVifDriver} emits). - * Returns an empty list when the extension does not opt in -- existing - * extensions that derive identifiers from the MAC keep working unchanged. - */ - private List getNicUuidArgs(Network network, NicProfile nic) { + private List getNicUuidArgs(NicProfile nic) { if (nic == null || nic.getUuid() == null || nic.getUuid().isBlank()) { return Collections.emptyList(); } - if (!isLswitchVifBinding(network)) { - return Collections.emptyList(); - } return List.of("--nic-uuid", nic.getUuid()); } + private void applyNicUpdateFromNetwork(Network network, NicProfile nic) { + if (nic == null) { + return; + } + try { + NicVO nicVo = nicDao.findById(nic.getId()); + if (nicVo == null) { + return; + } + if (network.getBroadcastUri() != null) { + nicVo.setBroadcastUri(network.getBroadcastUri()); + nicVo.setIsolationUri(network.getBroadcastUri()); + } + nicDao.update(nic.getId(), nicVo); + } catch (Exception e) { + logger.debug("Failed to update nic {}: {}", nic.getId(), e.getMessage()); + } + } + @Override public boolean release(Network network, NicProfile nic, VirtualMachineProfile vm, ReservationContext context) throws ConcurrentOperationException, ResourceUnavailableException { @@ -1071,6 +1008,10 @@ public class NetworkExtensionElement extends AdapterBase implements * */ protected boolean executeScript(Network network, String command, String... args) { + return executeScriptAndReturnOutput(network, command, args).first(); + } + + protected Pair executeScriptAndReturnOutput(Network network, String command, String... args) { Extension extension = resolveExtension(network); File scriptFile = resolveScriptFile(network, extension); @@ -1107,15 +1048,83 @@ public class NetworkExtensionElement extends AdapterBase implements } if (exitCode != 0) { logger.error("Network extension script failed with exit code {}: {}", exitCode, outputStr); - return false; + return new Pair<>(false, outputStr); } - return true; + return new Pair<>(true, outputStr); } catch (Exception e) { logger.error("Failed to execute network extension script: {}", e.getMessage(), e); throw new CloudRuntimeException("Failed to execute network extension script", e); } } + private JsonObject parseJsonOutput(String outputStr) { + if (StringUtils.isBlank(outputStr)) { + return null; + } + try { + JsonElement parsed = JsonParser.parseString(outputStr); + if (!parsed.isJsonObject()) { + logger.debug("Ignoring non-object script output: {}", outputStr); + return null; + } + return parsed.getAsJsonObject(); + } catch (Exception e) { + logger.debug("Ignoring non-JSON script output: {}", outputStr); + return null; + } + } + + private String getJsonString(JsonObject jsonObject, String key) { + if (jsonObject == null || StringUtils.isBlank(key) || !jsonObject.has(key)) { + return null; + } + JsonElement value = jsonObject.get(key); + if (value == null || value.isJsonNull()) { + return null; + } + return value.getAsString(); + } + + private void applyNetworkUpdateFromScriptOutput(Network network, String outputStr) { + JsonObject outputJson = parseJsonOutput(outputStr); + + String networkBroadcastUri = getJsonString(outputJson, "network.broadcast_uri"); + String networkBroadcastDomainType = getJsonString(outputJson, "network.broadcast_domain_type"); + if (networkBroadcastUri == null && networkBroadcastDomainType == null) { + return; + } + + try { + NetworkVO networkVo = networkDao.findById(network.getId()); + if (networkVo == null) { + return; + } + + boolean changed = false; + if (networkBroadcastDomainType != null) { + Networks.BroadcastDomainType domainType = EnumUtils.getEnumIgnoreCase(Networks.BroadcastDomainType.class, networkBroadcastDomainType); + if (domainType != null) { + networkVo.setBroadcastDomainType(domainType); + changed = true; + } else { + logger.warn("Ignoring unknown broadcast domain type '{}' for network {}", + networkBroadcastDomainType, network.getId()); + } + } + + if (networkBroadcastUri != null) { + networkVo.setBroadcastUri(URI.create(networkBroadcastUri)); + changed = true; + } + + if (changed) { + networkDao.update(networkVo.getId(), networkVo); + } + } catch (Exception e) { + logger.warn("Failed to update network {} from script output: {}", network.getId(), e.getMessage()); + } + } + /** * Writes a potentially large payload to a temporary file and passes the file path * to the extension script via {@code payloadArgName}. This avoids argv size limits @@ -1492,7 +1501,7 @@ public class NetworkExtensionElement extends AdapterBase implements args.add("--default-nic"); args.add(String.valueOf(nic.isDefaultNic())); args.add("--domain"); args.add(safeStr(network.getNetworkDomain())); args.add("--extension-ip"); args.add(safeStr(extensionIp)); - args.addAll(getNicUuidArgs(network, nic)); + args.addAll(getNicUuidArgs(nic)); args.addAll(getVpcIdArgs(network)); return executeScript(network, CMD_ADD_DHCP_ENTRY, args.toArray(new String[0])); } @@ -1581,7 +1590,7 @@ public class NetworkExtensionElement extends AdapterBase implements args.add("--mac"); args.add(safeStr(nic.getMacAddress())); args.add("--ip"); args.add(safeStr(nic.getIPv4Address())); args.add("--extension-ip"); args.add(safeStr(extensionIp)); - args.addAll(getNicUuidArgs(network, nic)); + args.addAll(getNicUuidArgs(nic)); args.addAll(getVpcIdArgs(network)); return executeScript(network, CMD_REMOVE_DHCP_ENTRY, args.toArray(new String[0])); } @@ -1604,7 +1613,7 @@ public class NetworkExtensionElement extends AdapterBase implements args.add("--ip"); args.add(safeStr(nic.getIPv4Address())); args.add("--hostname"); args.add(safeStr(hostname)); args.add("--extension-ip"); args.add(safeStr(extensionIp)); - args.addAll(getNicUuidArgs(network, nic)); + args.addAll(getNicUuidArgs(nic)); args.addAll(getVpcIdArgs(network)); return executeScript(network, CMD_ADD_DNS_ENTRY, args.toArray(new String[0])); } @@ -1781,7 +1790,7 @@ public class NetworkExtensionElement extends AdapterBase implements args.add("--ip"); args.add(safeStr(nicIpAddress)); args.add("--gateway"); args.add(safeStr(nic.getIPv4Gateway())); args.add("--extension-ip"); args.add(safeStr(ensureExtensionIp(network))); - args.addAll(getNicUuidArgs(network, nic)); + args.addAll(getNicUuidArgs(nic)); args.addAll(getVpcIdArgs(network)); return executeScriptWithFilePayload(network, CMD_SAVE_VM_DATA, "--vm-data-file", vmDataArg, args.toArray(new String[0])); @@ -1805,7 +1814,7 @@ public class NetworkExtensionElement extends AdapterBase implements args.add("--gateway"); args.add(safeStr(nic.getIPv4Gateway())); args.add("--password"); args.add(password); args.add("--extension-ip"); args.add(safeStr(extensionIp)); - args.addAll(getNicUuidArgs(network, nic)); + args.addAll(getNicUuidArgs(nic)); args.addAll(getVpcIdArgs(network)); return executeScript(network, CMD_SAVE_PASSWORD, args.toArray(new String[0])); } @@ -1832,7 +1841,7 @@ public class NetworkExtensionElement extends AdapterBase implements args.add("--gateway"); args.add(safeStr(nic.getIPv4Gateway())); args.add("--userdata"); args.add(userData); args.add("--extension-ip"); args.add(safeStr(extensionIp)); - args.addAll(getNicUuidArgs(network, nic)); + args.addAll(getNicUuidArgs(nic)); args.addAll(getVpcIdArgs(network)); return executeScript(network, CMD_SAVE_USERDATA, args.toArray(new String[0])); } @@ -1856,7 +1865,7 @@ public class NetworkExtensionElement extends AdapterBase implements args.add("--gateway"); args.add(safeStr(nic.getIPv4Gateway())); args.add("--sshkey"); args.add(sshKeyBase64); args.add("--extension-ip"); args.add(safeStr(extensionIp)); - args.addAll(getNicUuidArgs(network, nic)); + args.addAll(getNicUuidArgs(nic)); args.addAll(getVpcIdArgs(network)); return executeScript(network, CMD_SAVE_SSHKEY, args.toArray(new String[0])); } @@ -1880,7 +1889,7 @@ public class NetworkExtensionElement extends AdapterBase implements args.add("--gateway"); args.add(safeStr(nic.getIPv4Gateway())); args.add("--hypervisor-hostname"); args.add(hostname); args.add("--extension-ip"); args.add(safeStr(extensionIp)); - args.addAll(getNicUuidArgs(network, nic)); + args.addAll(getNicUuidArgs(nic)); args.addAll(getVpcIdArgs(network)); return executeScript(network, CMD_SAVE_HYPERVISOR_HOSTNAME, args.toArray(new String[0])); } diff --git a/framework/extensions/src/main/java/org/apache/cloudstack/framework/extensions/network/README.md b/framework/extensions/src/main/java/org/apache/cloudstack/framework/extensions/network/README.md index 2d7ce01e1e2..5d9a4c36000 100644 --- a/framework/extensions/src/main/java/org/apache/cloudstack/framework/extensions/network/README.md +++ b/framework/extensions/src/main/java/org/apache/cloudstack/framework/extensions/network/README.md @@ -71,9 +71,8 @@ hosts. Use it as a working example. 8. [Capabilities Configuration](#capabilities-configuration) 9. [VPC Networks](#vpc-networks) 10. [Extension IP](#extension-ip) -11. [VIF Binding for OVS-backed Extensions](#vif-binding-for-ovs-backed-extensions) -12. [Exit Codes](#exit-codes) -13. [Minimal Script Skeleton](#minimal-script-skeleton) +11. [Exit Codes](#exit-codes) +12. [Minimal Script Skeleton](#minimal-script-skeleton) --- @@ -637,7 +636,6 @@ network whose DHCP service is provided by this extension. | `--default-nic ` | `true` if this is the VM's default NIC. | | `--domain ` | Network domain suffix (e.g. `cs.example.com`). | | `--extension-ip ` | | -| `--nic-uuid ` | (optional) Present only when the extension declared `vif.binding=lswitch`. Carries `nic.getUuid()` so the extension can use it as the SDN-side port identifier (matches `external_ids:iface-id` set by libvirt on the OVS tap). See [VIF Binding for OVS-backed Extensions](#vif-binding-for-ovs-backed-extensions). | | `--vpc-id ` | (optional) | **`remove-dhcp-entry` arguments:** @@ -1108,77 +1106,6 @@ To use this extension as a VPC provider: --- -## VIF Binding for OVS-backed Extensions - -Extensions that drive OVS-based fabrics (OVN, NSX-OVS, …) need the OVS -tap interface that libvirt creates for each VM NIC to carry the -`external_ids:iface-id` value that the SDN controller expects. CloudStack -already does the right thing for `BroadcastDomainType.Lswitch` networks: -its `OvsVifDriver` emits - -```xml - - - -``` - -and libvirt sets `external_ids:iface-id=` on the tap atomically -with port creation. No agent patch is required. - -To opt into this binding mode an extension declares it as a top-level -capability hint in its `extension_details`: - -```bash -cmk createExtension \ - name=my-ovs-sdn \ - type=NetworkOrchestrator \ - "details[0].key=network.services" \ - "details[0].value=Dhcp,Dns,UserData,SourceNat,…" \ - "details[1].key=network.service.capabilities" \ - "details[1].value=$(cat my-caps.json)" \ - "details[2].key=vif.binding" \ - "details[2].value=lswitch" -``` - -When `vif.binding=lswitch` is present: - -1. **`prepare()` overrides the NIC broadcast type.** - `NetworkExtensionElement.prepare(...)` calls - `nic.setBroadcastType(Networks.BroadcastDomainType.Lswitch)` so - `OvsVifDriver` on the KVM agent picks the existing Lswitch path and - emits the libvirt `` shown above. - -2. **Per-NIC commands receive `--nic-uuid `.** - `add-dhcp-entry`, `remove-dhcp-entry`, `add-dns-entry`, `save-vm-data`, - `save-password`, `save-userdata`, `save-sshkey`, and - `save-hypervisor-hostname` all gain a `--nic-uuid ` argument - carrying `nic.getUuid()`. - -3. **The script must use `--nic-uuid` as the SDN-side port identifier.** - Whatever object the extension creates on its controller (OVN - Logical_Switch_Port, NSX logical port, …) **must be named exactly the - value of `--nic-uuid`**. That is the string libvirt will write to - `external_ids:iface-id` on the tap, so the SDN controller's local - agent (e.g. `ovn-controller`) finds a match and binds the port. - -When the extension does not declare `vif.binding`, the framework keeps -the default `BroadcastDomainType.Vlan` and does not propagate -`--nic-uuid` -- existing reference extensions (e.g. -`network-namespace`) are unaffected. - -### Why not the extension setting `iface-id` remotely? - -The OVS tap only exists *after* libvirt creates the VM, so any remote -write from the extension would race `ovn-controller` on the host. By -letting libvirt do the write atomically with tap creation, the binding -is ready by the time the controller scans the bridge. - -The extension may still talk OVSDB to the host (read-only checks, -`bridge-mappings` setup, post-incident repair) -- but never for the -boot path. - ---- - ## Exit Codes | Exit code | Meaning |