From 91a3bfc8cb1228f2fbb116cc2d254feec13f1afa Mon Sep 17 00:00:00 2001 From: Andrija Panic Date: Thu, 7 May 2026 20:39:16 +0200 Subject: [PATCH 1/3] Allow preserving duplicate MACs during VM import --- .../apache/cloudstack/api/ApiConstants.java | 1 + .../admin/vm/ImportUnmanagedInstanceCmd.java | 12 +++- .../service/NetworkOrchestrationService.java | 2 +- .../orchestration/NetworkOrchestrator.java | 4 +- .../NetworkOrchestratorTest.java | 8 +-- .../vm/UnmanagedVMsManagerImpl.java | 56 ++++++++------- .../com/cloud/vpc/MockNetworkManagerImpl.java | 2 +- .../vm/UnmanagedVMsManagerImplTest.java | 2 +- ui/public/locales/en.json | 1 + .../views/tools/ImportUnmanagedInstance.vue | 68 +++++++++++++++++-- 10 files changed, 116 insertions(+), 40 deletions(-) diff --git a/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java b/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java index 7485029f22cb..83c89b5a1f8f 100644 --- a/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java +++ b/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java @@ -274,6 +274,7 @@ public class ApiConstants { public static final String FILESYSTEM = "filesystem"; public static final String FIRSTNAME = "firstname"; public static final String FORCED = "forced"; + public static final String ALLOW_DUPLICATE_MAC_ADDRESSES = "allowduplicatemacaddresses"; public static final String FORCED_DESTROY_LOCAL_STORAGE = "forcedestroylocalstorage"; public static final String FORCE_CONVERT_TO_POOL = "forceconverttopool"; diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/ImportUnmanagedInstanceCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/ImportUnmanagedInstanceCmd.java index b42692ce4cf7..34fedaf5066b 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/ImportUnmanagedInstanceCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/ImportUnmanagedInstanceCmd.java @@ -152,9 +152,15 @@ public class ImportUnmanagedInstanceCmd extends BaseAsyncCmd { @Parameter(name = ApiConstants.FORCED, type = CommandType.BOOLEAN, - description = "Instance is imported despite some of its NIC's MAC addresses are already present, in case the MAC address exists then a new MAC address is generated") + description = "Instance is imported even if some NIC MAC addresses already exist. If a MAC address exists, a new MAC address is generated") private Boolean forced; + @Parameter(name = ApiConstants.ALLOW_DUPLICATE_MAC_ADDRESSES, + type = CommandType.BOOLEAN, + since = "4.23.0", + description = "Preserve imported NIC MAC addresses even when they already exist on the target network. Intended for migration cutover workflows where the source and imported VMs are not active on the same L2 network at the same time") + private Boolean allowDuplicateMacAddresses; + ///////////////////////////////////////////////////// /////////////////// Accessors /////////////////////// ///////////////////////////////////////////////////// @@ -278,6 +284,10 @@ public boolean isForced() { return BooleanUtils.isTrue(forced); } + public boolean isAllowDuplicateMacAddresses() { + return BooleanUtils.isTrue(allowDuplicateMacAddresses); + } + ///////////////////////////////////////////////////// /////////////// API Implementation/////////////////// ///////////////////////////////////////////////////// diff --git a/engine/api/src/main/java/org/apache/cloudstack/engine/orchestration/service/NetworkOrchestrationService.java b/engine/api/src/main/java/org/apache/cloudstack/engine/orchestration/service/NetworkOrchestrationService.java index 109a44488ec4..112debda5ebb 100644 --- a/engine/api/src/main/java/org/apache/cloudstack/engine/orchestration/service/NetworkOrchestrationService.java +++ b/engine/api/src/main/java/org/apache/cloudstack/engine/orchestration/service/NetworkOrchestrationService.java @@ -381,7 +381,7 @@ void implementNetworkElementsAndResources(DeployDestination dest, ReservationCon */ void cleanupNicDhcpDnsEntry(Network network, VirtualMachineProfile vmProfile, NicProfile nicProfile); - Pair importNic(final String macAddress, int deviceId, final Network network, final Boolean isDefaultNic, final VirtualMachine vm, final Network.IpAddresses ipAddresses, final DataCenter datacenter, boolean forced) throws InsufficientVirtualNetworkCapacityException, InsufficientAddressCapacityException; + Pair importNic(final String macAddress, int deviceId, final Network network, final Boolean isDefaultNic, final VirtualMachine vm, final Network.IpAddresses ipAddresses, final DataCenter datacenter, boolean forced, boolean allowDuplicateMacAddress) throws InsufficientVirtualNetworkCapacityException, InsufficientAddressCapacityException; void unmanageNics(VirtualMachineProfile vm); diff --git a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java index c1697819174d..95521dd46aba 100644 --- a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java +++ b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java @@ -4929,7 +4929,7 @@ public NicVO savePlaceholderNic(final Network network, final String ip4Address, @DB @Override - public Pair importNic(final String macAddress, int deviceId, final Network network, final Boolean isDefaultNic, final VirtualMachine vm, final Network.IpAddresses ipAddresses, final DataCenter dataCenter, final boolean forced) + public Pair importNic(final String macAddress, int deviceId, final Network network, final Boolean isDefaultNic, final VirtualMachine vm, final Network.IpAddresses ipAddresses, final DataCenter dataCenter, final boolean forced, final boolean allowDuplicateMacAddress) throws ConcurrentOperationException, InsufficientVirtualNetworkCapacityException, InsufficientAddressCapacityException { logger.debug("Allocating NIC for Instance {} in Network {} during import", vm, network); String selectedIp = null; @@ -4955,7 +4955,7 @@ public NicVO doInTransaction(TransactionStatus status) { throw new CloudRuntimeException("Invalid mac address: " + macAddressToPersist); } NicVO existingNic = _nicDao.findByNetworkIdAndMacAddress(network.getId(), macAddressToPersist); - if (existingNic != null) { + if (existingNic != null && !allowDuplicateMacAddress) { macAddressToPersist = generateNewMacAddressIfForced(network, macAddressToPersist, forced); } NicVO vo = new NicVO(network.getGuruName(), vm.getId(), network.getId(), vm.getType()); diff --git a/engine/orchestration/src/test/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestratorTest.java b/engine/orchestration/src/test/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestratorTest.java index 66f5b699cc46..139bf3e21b38 100644 --- a/engine/orchestration/src/test/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestratorTest.java +++ b/engine/orchestration/src/test/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestratorTest.java @@ -923,7 +923,7 @@ public void testImportNicAcquireGuestIPFailed() throws Exception { Mockito.when(testOrchestrator._networkModel.listNetworkOfferingServices(networkOfferingId)).thenReturn(Arrays.asList(Service.Dns, Service.Dhcp)); String macAddress = "02:01:01:82:00:01"; int deviceId = 0; - testOrchestrator.importNic(macAddress, deviceId, network, true, vm, ipAddresses, dataCenter, false); + testOrchestrator.importNic(macAddress, deviceId, network, true, vm, ipAddresses, dataCenter, false, false); } @Test(expected = InsufficientVirtualNetworkCapacityException.class) @@ -942,7 +942,7 @@ public void testImportNicAutoAcquireGuestIPFailed() throws Exception { Mockito.when(testOrchestrator._networkModel.listNetworkOfferingServices(networkOfferingId)).thenReturn(Arrays.asList(Service.Dns, Service.Dhcp)); String macAddress = "02:01:01:82:00:01"; int deviceId = 0; - testOrchestrator.importNic(macAddress, deviceId, network, true, vm, ipAddresses, dataCenter, false); + testOrchestrator.importNic(macAddress, deviceId, network, true, vm, ipAddresses, dataCenter, false, false); } @Test @@ -969,7 +969,7 @@ public void testImportNicNoIP4Address() throws Exception { Mockito.when(testOrchestrator._networkModel.getNetworkTag(hypervisorType, network)).thenReturn("testtag"); try (MockedStatic transactionMocked = Mockito.mockStatic(Transaction.class)) { transactionMocked.when(() -> Transaction.execute(any(TransactionCallback.class))).thenReturn(nic); - Pair nicProfileIntegerPair = testOrchestrator.importNic(macAddress, deviceId, network, true, vm, ipAddresses, dataCenter, false); + Pair nicProfileIntegerPair = testOrchestrator.importNic(macAddress, deviceId, network, true, vm, ipAddresses, dataCenter, false, false); verify(testOrchestrator._networkModel, times(1)).getNetworkRate(networkId, vmId); verify(testOrchestrator._networkModel, times(1)).isSecurityGroupSupportedInNetwork(network); verify(testOrchestrator._networkModel, times(1)).getNetworkTag(Hypervisor.HypervisorType.KVM, network); @@ -1007,7 +1007,7 @@ public void testImportNicWithIP4Address() throws Exception { Mockito.when(testOrchestrator._networkModel.getNetworkTag(hypervisorType, network)).thenReturn("testtag"); try (MockedStatic transactionMocked = Mockito.mockStatic(Transaction.class)) { transactionMocked.when(() -> Transaction.execute(any(TransactionCallback.class))).thenReturn(nic); - Pair nicProfileIntegerPair = testOrchestrator.importNic(macAddress, deviceId, network, true, vm, ipAddresses, dataCenter, false); + Pair nicProfileIntegerPair = testOrchestrator.importNic(macAddress, deviceId, network, true, vm, ipAddresses, dataCenter, false, false); verify(testOrchestrator, times(1)).getSelectedIpForNicImport(network, dataCenter, ipAddresses); verify(testOrchestrator._networkModel, times(1)).getNetworkRate(networkId, vmId); verify(testOrchestrator._networkModel, times(1)).isSecurityGroupSupportedInNetwork(network); diff --git a/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java b/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java index c0e17a064a39..a8dda30782e0 100644 --- a/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java +++ b/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java @@ -955,9 +955,10 @@ private long getNextDeviceId(long deviceId, Set reservedDeviceIds) { return deviceId; } - private NicProfile importNic(UnmanagedInstanceTO.Nic nic, VirtualMachine vm, Network network, Network.IpAddresses ipAddresses, int deviceId, boolean isDefaultNic, boolean forced) throws InsufficientVirtualNetworkCapacityException, InsufficientAddressCapacityException { + private NicProfile importNic(UnmanagedInstanceTO.Nic nic, VirtualMachine vm, Network network, Network.IpAddresses ipAddresses, int deviceId, boolean isDefaultNic, + boolean forced, boolean allowDuplicateMacAddress) throws InsufficientVirtualNetworkCapacityException, InsufficientAddressCapacityException { DataCenterVO dataCenterVO = dataCenterDao.findById(network.getDataCenterId()); - Pair result = networkOrchestrationService.importNic(nic.getMacAddress(), deviceId, network, isDefaultNic, vm, ipAddresses, dataCenterVO, forced); + Pair result = networkOrchestrationService.importNic(nic.getMacAddress(), deviceId, network, isDefaultNic, vm, ipAddresses, dataCenterVO, forced, allowDuplicateMacAddress); if (result == null) { throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, String.format("NIC ID: %s import failed", nic.getNicId())); } @@ -1164,17 +1165,19 @@ private UserVm importVirtualMachineInternal(final UnmanagedInstanceTO unmanagedI final VirtualMachineTemplate template, final String displayName, final String hostName, final Account caller, final Account owner, final Long userId, final ServiceOfferingVO serviceOffering, final Map dataDiskOfferingMap, final Map nicNetworkMap, final Map callerNicIpAddressMap, final Long guestOsId, - final Map details, final boolean migrateAllowed, final boolean forced, final boolean isImportUnmanagedFromSameHypervisor) { + final Map details, final boolean migrateAllowed, final boolean forced, final boolean allowDuplicateMacAddress, + final boolean isImportUnmanagedFromSameHypervisor) { return importVirtualMachineInternal(unmanagedInstance, instanceNameInternal, zone, cluster, host, template, displayName, hostName, caller, owner, userId, serviceOffering, dataDiskOfferingMap, nicNetworkMap, callerNicIpAddressMap, guestOsId, details, migrateAllowed, - forced, isImportUnmanagedFromSameHypervisor, null); + forced, allowDuplicateMacAddress, isImportUnmanagedFromSameHypervisor, null); } private UserVm importVirtualMachineInternal(final UnmanagedInstanceTO unmanagedInstance, final String instanceNameInternal, final DataCenter zone, final Cluster cluster, final HostVO host, final VirtualMachineTemplate template, final String displayName, final String hostName, final Account caller, final Account owner, final Long userId, final ServiceOfferingVO serviceOffering, final Map dataDiskOfferingMap, final Map nicNetworkMap, final Map callerNicIpAddressMap, final Long guestOsId, - final Map details, final boolean migrateAllowed, final boolean forced, final boolean isImportUnmanagedFromSameHypervisor, + final Map details, final boolean migrateAllowed, final boolean forced, final boolean allowDuplicateMacAddress, + final boolean isImportUnmanagedFromSameHypervisor, final Long storagePoolId) { logger.debug(LogUtils.logGsonWithoutException("Trying to import VM [%s] with name [%s], in zone [%s], cluster [%s], and host [%s], using template [%s], service offering [%s], disks map [%s], NICs map [%s] and details [%s].", unmanagedInstance, displayName, zone, cluster, host, template, serviceOffering, dataDiskOfferingMap, nicNetworkMap, details)); @@ -1306,7 +1309,7 @@ private UserVm importVirtualMachineInternal(final UnmanagedInstanceTO unmanagedI for (UnmanagedInstanceTO.Nic nic : unmanagedInstance.getNics()) { Network network = networkDao.findById(allNicNetworkMap.get(nic.getNicId())); Network.IpAddresses ipAddresses = nicIpAddressMap.get(nic.getNicId()); - importNic(nic, userVm, network, ipAddresses, nicIndex, nicIndex == 0, forced); + importNic(nic, userVm, network, ipAddresses, nicIndex, nicIndex == 0, forced, allowDuplicateMacAddress); nicIndex++; } } catch (Exception e) { @@ -1446,6 +1449,10 @@ private UserVmResponse baseImportInstance(ImportUnmanagedInstanceCmd cmd) { final Map dataDiskOfferingMap = cmd.getDataDiskToDiskOfferingList(); final Map details = cmd.getDetails(); final boolean forced = cmd.isForced(); + final boolean allowDuplicateMacAddress = cmd.isAllowDuplicateMacAddresses(); + if (forced && allowDuplicateMacAddress) { + throw new InvalidParameterValueException(String.format("%s and %s are mutually exclusive", ApiConstants.FORCED, ApiConstants.ALLOW_DUPLICATE_MAC_ADDRESSES)); + } List hosts = resourceManager.listHostsInClusterByStatus(clusterId, Status.Up); UserVm userVm = null; List additionalNameFilters = getAdditionalNameFilters(cluster); @@ -1469,7 +1476,7 @@ private UserVmResponse baseImportInstance(ImportUnmanagedInstanceCmd cmd) { template, instanceName, displayName, hostName, caller, owner, userId, serviceOffering, dataDiskOfferingMap, nicNetworkMap, nicIpAddressMap, - details, importVmCmd, forced); + details, importVmCmd, forced, allowDuplicateMacAddress); } } else { if (List.of(Hypervisor.HypervisorType.VMware, Hypervisor.HypervisorType.KVM).contains(cluster.getHypervisorType())) { @@ -1477,7 +1484,7 @@ private UserVmResponse baseImportInstance(ImportUnmanagedInstanceCmd cmd) { template, instanceName, displayName, hostName, caller, owner, userId, serviceOffering, dataDiskOfferingMap, nicNetworkMap, nicIpAddressMap, - details, cmd.getMigrateAllowed(), managedVms, forced); + details, cmd.getMigrateAllowed(), managedVms, forced, allowDuplicateMacAddress); } } @@ -1624,7 +1631,8 @@ private UserVm importUnmanagedInstanceFromHypervisor(DataCenter zone, Cluster cl String hostName, Account caller, Account owner, long userId, ServiceOfferingVO serviceOffering, Map dataDiskOfferingMap, Map nicNetworkMap, Map nicIpAddressMap, - Map details, Boolean migrateAllowed, List managedVms, boolean forced) throws ResourceAllocationException { + Map details, Boolean migrateAllowed, List managedVms, boolean forced, + boolean allowDuplicateMacAddress) throws ResourceAllocationException { UserVm userVm = null; for (HostVO host : hosts) { HashMap unmanagedInstances = getUnmanagedInstancesForHost(host, instanceName, managedVms); @@ -1676,7 +1684,7 @@ private UserVm importUnmanagedInstanceFromHypervisor(DataCenter zone, Cluster cl template, displayName, hostName, CallContext.current().getCallingAccount(), owner, userId, serviceOffering, dataDiskOfferingMap, nicNetworkMap, nicIpAddressMap, null, - details, migrateAllowed, forced, true); + details, migrateAllowed, forced, allowDuplicateMacAddress, true); } finally { ReservationHelper.closeAll(reservations); } @@ -1787,7 +1795,8 @@ protected UserVm importUnmanagedInstanceFromVmwareToKvm(DataCenter zone, Cluster Account caller, Account owner, long userId, ServiceOfferingVO serviceOffering, Map dataDiskOfferingMap, Map nicNetworkMap, Map nicIpAddressMap, - Map details, ImportVmCmd cmd, boolean forced) throws ResourceAllocationException { + Map details, ImportVmCmd cmd, boolean forced, + boolean allowDuplicateMacAddress) throws ResourceAllocationException { Long existingVcenterId = cmd.getExistingVcenterId(); String vcenter = cmd.getVcenter(); String datacenterName = cmd.getDatacenterName(); @@ -1896,7 +1905,7 @@ protected UserVm importUnmanagedInstanceFromVmwareToKvm(DataCenter zone, Cluster } nicIpAddressMap = autoFillStaticNicIpAddresses(nicNetworkMap, nicIpAddressMap, buildSourceNicCidrMap(sourceVMwareInstance)); - checkNetworkingBeforeConvertingVmwareInstance(zone, owner, displayName, hostName, sourceVMwareInstance, nicNetworkMap, nicIpAddressMap, forced); + checkNetworkingBeforeConvertingVmwareInstance(zone, owner, displayName, hostName, sourceVMwareInstance, nicNetworkMap, nicIpAddressMap, forced, allowDuplicateMacAddress); UnmanagedInstanceTO convertedInstance; if (!useVddk && (forceMsToImportVmFiles || !isOvfExportSupported)) { // Uses MS for OVF export to temporary conversion location @@ -1927,7 +1936,7 @@ protected UserVm importUnmanagedInstanceFromVmwareToKvm(DataCenter zone, Cluster template, displayName, hostName, caller, owner, userId, serviceOffering, dataDiskOfferingMap, nicNetworkMap, nicIpAddressMap, guestOsId, - applyVmwareSourceHardwareDetails(details, sourceVMwareInstance), false, forced, false); + applyVmwareSourceHardwareDetails(details, sourceVMwareInstance), false, forced, allowDuplicateMacAddress, false); long timeElapsedInSecs = (System.currentTimeMillis() - importStartTime) / 1000; logger.debug(String.format("VMware VM %s imported successfully to CloudStack instance %s (%s), Time taken: %d secs, OVF files imported from %s, Source VMware VM details - OS: %s, PowerState: %s, Disks: %s, NICs: %s", sourceVMName, displayName, displayName, timeElapsedInSecs, (ovfTemplateOnConvertLocation != null)? "MS" : "KVM Host", sourceVMwareInstance.getOperatingSystem(), sourceVMwareInstance.getPowerState(), sourceVMwareInstance.getDisks(), sourceVMwareInstance.getNics())); @@ -1983,7 +1992,7 @@ public UserVm importConvertedVmwareCbtInstanceToKvm(String vcenter, String datac Map effectiveNicIpAddressMap = autoFillStaticNicIpAddresses(nicNetworkMap, nicIpAddressMap, buildSourceNicCidrMap(sourceVMwareInstance)); checkNetworkingBeforeConvertingVmwareInstance(zone, owner, resolvedDisplayName, resolvedHostName, - sourceVMwareInstance, nicNetworkMap, effectiveNicIpAddressMap, forced); + sourceVMwareInstance, nicNetworkMap, effectiveNicIpAddressMap, forced, false); sanitizeConvertedInstance(convertedInstance, sourceVMwareInstance); if (guestOsId == null) { @@ -1992,7 +2001,7 @@ public UserVm importConvertedVmwareCbtInstanceToKvm(String vcenter, String datac return importVirtualMachineInternal(convertedInstance, null, zone, destinationCluster, null, template, resolvedDisplayName, resolvedHostName, caller, owner, userId, serviceOffering, dataDiskOfferingMap, nicNetworkMap, effectiveNicIpAddressMap, guestOsId, - applyVmwareSourceHardwareDetails(details, sourceVMwareInstance), false, forced, false, storagePoolId); + applyVmwareSourceHardwareDetails(details, sourceVMwareInstance), false, forced, false, false, storagePoolId); } catch (CloudRuntimeException e) { throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, e.getMessage()); } catch (ResourceAllocationException e) { @@ -2096,7 +2105,7 @@ private void checkNetworkingBeforeConvertingVmwareInstance(DataCenter zone, Acco String hostName, UnmanagedInstanceTO sourceVMwareInstance, Map nicNetworkMap, Map nicIpAddressMap, - boolean forced) { + boolean forced, boolean allowDuplicateMacAddress) { List nics = sourceVMwareInstance.getNics(); List networkIds = new ArrayList<>(nicNetworkMap.values()); if (nics.size() != networkIds.size()) { @@ -2119,18 +2128,19 @@ private void checkNetworkingBeforeConvertingVmwareInstance(DataCenter zone, Acco ipAddresses = nicIpAddressMap.get(nic.getNicId()); } boolean autoImport = ipAddresses != null && ipAddresses.getIp4Address() != null && ipAddresses.getIp4Address().equalsIgnoreCase("auto"); - checkUnmanagedNicAndNetworkMacAddressForImport(network, nic, forced); + checkUnmanagedNicAndNetworkMacAddressForImport(network, nic, forced, allowDuplicateMacAddress); checkUnmanagedNicAndNetworkForImport(displayName, nic, network, zone, owner, autoImport, Hypervisor.HypervisorType.KVM); checkUnmanagedNicAndNetworkHostnameForImport(displayName, nic, network, hostName); checkUnmanagedNicIpAndNetworkForImport(displayName, nic, network, ipAddresses); } } - private void checkUnmanagedNicAndNetworkMacAddressForImport(NetworkVO network, UnmanagedInstanceTO.Nic nic, boolean forced) { + private void checkUnmanagedNicAndNetworkMacAddressForImport(NetworkVO network, UnmanagedInstanceTO.Nic nic, boolean forced, boolean allowDuplicateMacAddress) { NicVO existingNic = nicDao.findByNetworkIdAndMacAddress(network.getId(), nic.getMacAddress()); - if (existingNic != null && !forced) { - String err = String.format("NIC %s with MAC address %s already exists on network %s and forced flag is disabled. " + - "Retry with forced flag enabled if a new MAC address to be generated.", nic, nic.getMacAddress(), network); + if (existingNic != null && !forced && !allowDuplicateMacAddress) { + String err = String.format("NIC %s with MAC address %s already exists on network %s. " + + "Retry with %s=true to generate a new MAC address or %s=true to preserve the duplicate MAC address.", + nic, nic.getMacAddress(), network, ApiConstants.FORCED, ApiConstants.ALLOW_DUPLICATE_MAC_ADDRESSES); logger.error(err); throw new CloudRuntimeException(err); } @@ -3476,7 +3486,7 @@ private UserVm importExternalKvmVirtualMachine(final UnmanagedInstanceTO unmanag for (UnmanagedInstanceTO.Nic nic : unmanagedInstance.getNics()) { Network network = networkDao.findById(allNicNetworkMap.get(nic.getNicId())); Network.IpAddresses ipAddresses = nicIpAddressMap.get(nic.getNicId()); - importNic(nic, userVm, network, ipAddresses, nicIndex, nicIndex==0, true); + importNic(nic, userVm, network, ipAddresses, nicIndex, nicIndex==0, true, false); nicIndex++; } } catch (Exception e) { @@ -3666,7 +3676,7 @@ private UserVm importKvmVirtualMachineFromDisk(final ImportSource importSource, cleanupFailedImportVM(userVm); throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, String.format("Failed to import volumes while importing vm: %s. %s", instanceName, StringUtils.defaultString(e.getMessage()))); } - networkOrchestrationService.importNic(macAddress, 0, network, true, userVm, requestedIpPair, zone, true); + networkOrchestrationService.importNic(macAddress, 0, network, true, userVm, requestedIpPair, zone, true, false); publishVMUsageUpdateResourceCount(userVm, dummyOffering, template); return userVm; diff --git a/server/src/test/java/com/cloud/vpc/MockNetworkManagerImpl.java b/server/src/test/java/com/cloud/vpc/MockNetworkManagerImpl.java index 1239dd23e7ec..059512337820 100644 --- a/server/src/test/java/com/cloud/vpc/MockNetworkManagerImpl.java +++ b/server/src/test/java/com/cloud/vpc/MockNetworkManagerImpl.java @@ -1076,7 +1076,7 @@ public AcquirePodIpCmdResponse allocatePodIp(Account account, String zoneId, Str } @Override - public Pair importNic(String macAddress, int deviceId, Network network, Boolean isDefaultNic, VirtualMachine vm, IpAddresses ipAddresses, DataCenter dataCenter, boolean forced) { + public Pair importNic(String macAddress, int deviceId, Network network, Boolean isDefaultNic, VirtualMachine vm, IpAddresses ipAddresses, DataCenter dataCenter, boolean forced, boolean allowDuplicateMacAddress) { return null; } diff --git a/server/src/test/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImplTest.java b/server/src/test/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImplTest.java index c5267033dfb4..6d8ed2d36ccc 100644 --- a/server/src/test/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImplTest.java +++ b/server/src/test/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImplTest.java @@ -430,7 +430,7 @@ public void setUp() throws Exception { NicProfile profile = Mockito.mock(NicProfile.class); Integer deviceId = 100; Pair pair = new Pair<>(profile, deviceId); - when(networkOrchestrationService.importNic(nullable(String.class), nullable(Integer.class), nullable(Network.class), nullable(Boolean.class), nullable(VirtualMachine.class), nullable(Network.IpAddresses.class), nullable(DataCenter.class), Mockito.anyBoolean())).thenReturn(pair); + when(networkOrchestrationService.importNic(nullable(String.class), nullable(Integer.class), nullable(Network.class), nullable(Boolean.class), nullable(VirtualMachine.class), nullable(Network.IpAddresses.class), nullable(DataCenter.class), Mockito.anyBoolean(), Mockito.anyBoolean())).thenReturn(pair); when(volumeDao.findByInstance(Mockito.anyLong())).thenReturn(volumes); List userVmResponses = new ArrayList<>(); UserVmResponse userVmResponse = new UserVmResponse(); diff --git a/ui/public/locales/en.json b/ui/public/locales/en.json index 0acafc3b8015..7f3577febecb 100644 --- a/ui/public/locales/en.json +++ b/ui/public/locales/en.json @@ -1210,6 +1210,7 @@ "label.forcks": "For CKS", "label.forbidden": "Forbidden", "label.forced": "Force", +"label.generate.new.mac.if.required": "Generate new MAC if required", "label.force.convert.to.pool": "Force converting to storage pool directly (not using temporary storage for conversion)", "label.force.ms.to.import.vm.files": "Enable to force OVF Download via Management Server. Disable to use KVM Host ovftool (if installed)", "label.force.update.os.type": "Force update OS type", diff --git a/ui/src/views/tools/ImportUnmanagedInstance.vue b/ui/src/views/tools/ImportUnmanagedInstance.vue index 14c06288f308..12c6cd991eba 100644 --- a/ui/src/views/tools/ImportUnmanagedInstance.vue +++ b/ui/src/views/tools/ImportUnmanagedInstance.vue @@ -360,15 +360,25 @@ {{ $t('message.ip.address.changes.effect.after.vm.restart') }} - + - + + + + + + + @@ -422,14 +432,24 @@ - + - + + + + + + + @@ -727,6 +747,15 @@ export default { canUseVmwareCbt () { return 'startVmwareCbtMigration' in this.$store.getters.apis }, + hasSourceNicMacAddresses () { + return !this.isDiskImport && this.resource?.nic?.some(nic => nic.macaddress || nic.mac) + }, + showMacConflictOptions () { + return this.hasSourceNicMacAddresses + }, + showAllowDuplicateMacAddresses () { + return this.showMacConflictOptions && this.apiParams.allowduplicatemacaddresses + }, domainSelectOptions () { var domains = this.options.domains.map((domain) => { return { @@ -861,6 +890,7 @@ export default { usevddk: false, migrateallowed: this.switches.migrateAllowed, forced: this.switches.forced, + allowduplicatemacaddresses: this.switches.allowDuplicateMacAddresses, forcemstoimportvmfiles: this.switches.forceMsToImportVmFiles, forceconverttopool: this.switches.forceConvertToPool, domainid: null, @@ -1245,6 +1275,22 @@ export default { this.form.vmwaremigrationmode = mode this.onUseVddkChange(mode === 'vddk' || mode === 'cbt', isUserChange) }, + onForcedMacConflictChange (val) { + this.switches.forced = val + this.form.forced = val + if (val) { + this.switches.allowDuplicateMacAddresses = false + this.form.allowduplicatemacaddresses = false + } + }, + onAllowDuplicateMacAddressesChange (val) { + this.switches.allowDuplicateMacAddresses = val + this.form.allowduplicatemacaddresses = val + if (val) { + this.switches.forced = false + this.form.forced = false + } + }, onUseVddkChange (val, isUserChange = true) { if (isUserChange) { this.userModifiedVddkSetting = true @@ -1413,7 +1459,13 @@ export default { params.forceconverttopool = values.forceconverttopool } } - var keys = ['hostname', 'domainid', 'projectid', 'account', 'migrateallowed', 'forced', 'osid'] + var keys = ['hostname', 'domainid', 'projectid', 'account', 'migrateallowed', 'osid'] + if (this.showMacConflictOptions) { + keys.push('forced') + } + if (this.showAllowDuplicateMacAddresses) { + keys.push('allowduplicatemacaddresses') + } if (this.templateType !== 'auto') { keys.push('templateid') } @@ -1688,6 +1740,8 @@ export default { this.form.usevddk = false this.form.forceconverttopool = false this.form.forcemstoimportvmfiles = false + this.form.forced = false + this.form.allowduplicatemacaddresses = false this.userModifiedVddkSetting = false this.resetStorageOptionsForConversion() }, From a4855a22426ea5e4591df3b5b5f0beb38507e0c6 Mon Sep 17 00:00:00 2001 From: andrijapanicsb Date: Tue, 6 Oct 2026 06:20:22 +0200 Subject: [PATCH 2/3] Fix MAC conflict options for remote KVM VM imports Remote KVM imports always generated a replacement MAC on conflict because the import path hardcoded forced=true and duplicate preservation=false. Preserve the historical replacement default when forced is omitted, while honoring explicit forced=false and allowduplicatemacaddresses=true. Reject incompatible options before import side effects and forward the resolved policy to NIC allocation. Update the UI to show the remote KVM default, preserve mutual exclusion, and submit forced=false explicitly. Add backend, allocator, and UI coverage for the supported conflict policies. --- .../admin/vm/ImportUnmanagedInstanceCmd.java | 4 + .../NetworkOrchestratorTest.java | 82 +++++++++++++++++ .../vm/UnmanagedVMsManagerImpl.java | 19 +++- .../vm/UnmanagedVMsManagerImplTest.java | 79 +++++++++++++++- ui/src/utils/importVmMacAddressPolicy.js | 42 +++++++++ .../views/tools/ImportUnmanagedInstance.vue | 51 ++++++----- .../utils/importVmMacAddressPolicy.spec.js | 89 +++++++++++++++++++ 7 files changed, 340 insertions(+), 26 deletions(-) create mode 100644 ui/src/utils/importVmMacAddressPolicy.js create mode 100644 ui/tests/unit/utils/importVmMacAddressPolicy.spec.js diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/ImportUnmanagedInstanceCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/ImportUnmanagedInstanceCmd.java index 34fedaf5066b..52305d65ac28 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/ImportUnmanagedInstanceCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/ImportUnmanagedInstanceCmd.java @@ -284,6 +284,10 @@ public boolean isForced() { return BooleanUtils.isTrue(forced); } + public Boolean getForced() { + return forced; + } + public boolean isAllowDuplicateMacAddresses() { return BooleanUtils.isTrue(allowDuplicateMacAddresses); } diff --git a/engine/orchestration/src/test/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestratorTest.java b/engine/orchestration/src/test/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestratorTest.java index 139bf3e21b38..c1919fecf172 100644 --- a/engine/orchestration/src/test/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestratorTest.java +++ b/engine/orchestration/src/test/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestratorTest.java @@ -82,6 +82,7 @@ import com.cloud.utils.db.EntityManager; import com.cloud.utils.db.Transaction; import com.cloud.utils.db.TransactionCallback; +import com.cloud.utils.db.TransactionStatus; import com.cloud.utils.exception.CloudRuntimeException; import com.cloud.utils.net.Ip; import com.cloud.vm.DomainRouterVO; @@ -945,6 +946,23 @@ public void testImportNicAutoAcquireGuestIPFailed() throws Exception { testOrchestrator.importNic(macAddress, deviceId, network, true, vm, ipAddresses, dataCenter, false, false); } + @Test(expected = InsufficientVirtualNetworkCapacityException.class) + public void testImportNicDuplicateMacAllowanceDoesNotBypassIpAllocation() throws Exception { + DataCenter dataCenter = Mockito.mock(DataCenter.class); + VirtualMachine vm = mock(VirtualMachine.class); + Network network = Mockito.mock(Network.class); + Mockito.when(network.getGuestType()).thenReturn(GuestType.Isolated); + Mockito.when(network.getNetworkOfferingId()).thenReturn(networkOfferingId); + long dataCenterId = 1L; + Mockito.when(network.getDataCenterId()).thenReturn(dataCenterId); + Network.IpAddresses ipAddresses = Mockito.mock(Network.IpAddresses.class); + Mockito.when(ipAddresses.getIp4Address()).thenReturn("10.1.10.10"); + Mockito.when(testOrchestrator.getSelectedIpForNicImport(network, dataCenter, ipAddresses)).thenReturn(null); + Mockito.when(testOrchestrator._networkModel.listNetworkOfferingServices(networkOfferingId)).thenReturn(Arrays.asList(Service.Dns, Service.Dhcp)); + + testOrchestrator.importNic("02:01:01:82:00:01", 0, network, true, vm, ipAddresses, dataCenter, false, true); + } + @Test public void testImportNicNoIP4Address() throws Exception { DataCenter dataCenter = Mockito.mock(DataCenter.class); @@ -1021,6 +1039,70 @@ public void testImportNicWithIP4Address() throws Exception { } } + @Test + public void testImportNicReplacesConflictingMacWhenForced() throws Exception { + String sourceMacAddress = "02:01:01:82:00:01"; + String replacementMacAddress = "02:01:01:82:00:02"; + when(testOrchestrator._networkModel.getNextAvailableMacAddressInNetwork(1L)).thenReturn(replacementMacAddress); + + NicProfile nicProfile = importNicWithMacPolicy(sourceMacAddress, true, true, false); + + assertEquals(replacementMacAddress, nicProfile.getMacAddress()); + } + + @Test(expected = CloudRuntimeException.class) + public void testImportNicRejectsConflictingMacWhenNotForced() throws Exception { + importNicWithMacPolicy("02:01:01:82:00:01", true, false, false); + } + + @Test + public void testImportNicPreservesAllowedDuplicateMac() throws Exception { + String sourceMacAddress = "02:01:01:82:00:01"; + + NicProfile nicProfile = importNicWithMacPolicy(sourceMacAddress, true, false, true); + + assertEquals(sourceMacAddress, nicProfile.getMacAddress()); + verify(testOrchestrator._networkModel, never()).getNextAvailableMacAddressInNetwork(1L); + } + + @Test + public void testImportNicPreservesUnusedSourceMac() throws Exception { + String sourceMacAddress = "02:01:01:82:00:01"; + + NicProfile nicProfile = importNicWithMacPolicy(sourceMacAddress, false, true, false); + + assertEquals(sourceMacAddress, nicProfile.getMacAddress()); + verify(testOrchestrator._networkModel, never()).getNextAvailableMacAddressInNetwork(1L); + } + + @SuppressWarnings("unchecked") + private NicProfile importNicWithMacPolicy(String macAddress, boolean macAddressExists, boolean forced, + boolean allowDuplicateMacAddress) throws Exception { + long networkId = 1L; + long vmId = 1L; + DataCenter dataCenter = mock(DataCenter.class); + Network network = mock(Network.class); + VirtualMachine vm = mock(VirtualMachine.class); + when(network.getId()).thenReturn(networkId); + when(network.getGuestType()).thenReturn(GuestType.L2); + when(vm.getId()).thenReturn(vmId); + when(vm.getType()).thenReturn(Type.User); + when(vm.getHypervisorType()).thenReturn(Hypervisor.HypervisorType.KVM); + if (macAddressExists) { + when(testOrchestrator._nicDao.findByNetworkIdAndMacAddress(networkId, macAddress)).thenReturn(mock(NicVO.class)); + } + when(testOrchestrator._nicDao.persist(any(NicVO.class))).thenAnswer(invocation -> invocation.getArgument(0)); + + try (MockedStatic transactionMocked = Mockito.mockStatic(Transaction.class)) { + transactionMocked.when(() -> Transaction.execute(any(TransactionCallback.class))).thenAnswer(invocation -> { + TransactionCallback callback = invocation.getArgument(0); + return callback.doInTransaction(mock(TransactionStatus.class)); + }); + return testOrchestrator.importNic(macAddress, 0, network, true, vm, null, dataCenter, + forced, allowDuplicateMacAddress).first(); + } + } + // ----------------------------------------------------------------------- // Tests for getNetworkElementsIncludingExtensions // ----------------------------------------------------------------------- diff --git a/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java b/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java index a8dda30782e0..dbf94767b168 100644 --- a/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java +++ b/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java @@ -3272,6 +3272,17 @@ private UserVmResponse importKvmInstance(ImportVmCmd cmd) { Long poolId = cmd.getStoragePoolId(); Long networkId = cmd.getNetworkId(); + boolean forced = false; + boolean allowDuplicateMacAddress = false; + if (ImportSource.EXTERNAL == importSource) { + Boolean requestedForced = cmd.getForced(); + allowDuplicateMacAddress = cmd.isAllowDuplicateMacAddresses(); + if (BooleanUtils.isTrue(requestedForced) && allowDuplicateMacAddress) { + throw new InvalidParameterValueException(String.format("%s and %s are mutually exclusive", ApiConstants.FORCED, ApiConstants.ALLOW_DUPLICATE_MAC_ADDRESSES)); + } + forced = !allowDuplicateMacAddress && (requestedForced == null || requestedForced); + } + UnmanagedInstanceTO unmanagedInstanceTO = null; if (ImportSource.EXTERNAL == importSource) { if (StringUtils.isBlank(cmd.getUsername())) { @@ -3355,7 +3366,8 @@ private UserVmResponse importKvmInstance(ImportVmCmd cmd) { userVm = importExternalKvmVirtualMachine(unmanagedInstanceTO, instanceName, zone, template, displayName, hostName, caller, owner, userId, serviceOffering, dataDiskOfferingMap, - nicNetworkMap, nicIpAddressMap, remoteUrl, username, password, tmpPath, details); + nicNetworkMap, nicIpAddressMap, remoteUrl, username, password, tmpPath, details, + forced, allowDuplicateMacAddress); } else if (ImportSource.SHARED == importSource || ImportSource.LOCAL == importSource) { try { userVm = importKvmVirtualMachineFromDisk(importSource, instanceName, zone, @@ -3387,7 +3399,8 @@ private UserVm importExternalKvmVirtualMachine(final UnmanagedInstanceTO unmanag final VirtualMachineTemplate template, final String displayName, final String hostName, final Account caller, final Account owner, final Long userId, final ServiceOfferingVO serviceOffering, final Map dataDiskOfferingMap, final Map nicNetworkMap, final Map callerNicIpAddressMap, - final String remoteUrl, String username, String password, String tmpPath, final Map details) throws ResourceAllocationException { + final String remoteUrl, String username, String password, String tmpPath, final Map details, + final boolean forced, final boolean allowDuplicateMacAddress) throws ResourceAllocationException { UserVm userVm = null; Map allDetails = new HashMap<>(details); @@ -3486,7 +3499,7 @@ private UserVm importExternalKvmVirtualMachine(final UnmanagedInstanceTO unmanag for (UnmanagedInstanceTO.Nic nic : unmanagedInstance.getNics()) { Network network = networkDao.findById(allNicNetworkMap.get(nic.getNicId())); Network.IpAddresses ipAddresses = nicIpAddressMap.get(nic.getNicId()); - importNic(nic, userVm, network, ipAddresses, nicIndex, nicIndex==0, true, false); + importNic(nic, userVm, network, ipAddresses, nicIndex, nicIndex == 0, forced, allowDuplicateMacAddress); nicIndex++; } } catch (Exception e) { diff --git a/server/src/test/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImplTest.java b/server/src/test/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImplTest.java index 6d8ed2d36ccc..0e119199ca44 100644 --- a/server/src/test/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImplTest.java +++ b/server/src/test/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImplTest.java @@ -27,6 +27,7 @@ import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.lenient; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -509,6 +510,24 @@ public void importUnmanagedInstanceTest() { } } + @Test + public void importUnmanagedVmwareInstanceForwardsDuplicateMacPolicy() { + ImportUnmanagedInstanceCmd cmd = Mockito.mock(ImportUnmanagedInstanceCmd.class); + when(cmd.getName()).thenReturn("TestInstance"); + when(cmd.getDomainId()).thenReturn(null); + when(cmd.isAllowDuplicateMacAddresses()).thenReturn(true); + when(volumeApiService.doesStoragePoolSupportDiskOffering(any(StoragePool.class), any())).thenReturn(true); + + try (MockedStatic ignored = Mockito.mockStatic(UsageEventUtils.class); + MockedConstruction mockCheckedReservation = Mockito.mockConstruction(CheckedReservation.class)) { + unmanagedVMsManager.importUnmanagedInstance(cmd); + } + + verify(networkOrchestrationService).importNic(Mockito.eq(instance.getNics().get(0).getMacAddress()), anyInt(), + any(Network.class), anyBoolean(), any(VirtualMachine.class), nullable(Network.IpAddresses.class), + any(DataCenter.class), Mockito.eq(false), Mockito.eq(true)); + } + @Test(expected = InvalidParameterValueException.class) public void importUnmanagedInstanceInvalidHostnameTest() { ImportUnmanagedInstanceCmd importUnmanageInstanceCmd = Mockito.mock(ImportUnmanagedInstanceCmd.class); @@ -695,15 +714,63 @@ public void testListRemoteInstancesTestNonKVM() { unmanagedVMsManager.listVmsForImport(cmd); } @Test - public void testImportFromExternalTest() throws InsufficientServerCapacityException { - String vmname = "TestInstance"; + public void testImportFromExternalDefaultsToReplacingConflictingMac() throws InsufficientServerCapacityException { + ImportVmCmd cmd = createExternalImportCmd(null, false); + importExternalKvmVm(cmd); + verifyExternalKvmMacPolicy(true, false); + } + + @Test + public void testImportFromExternalForwardsExplicitForcedTrue() throws InsufficientServerCapacityException { + ImportVmCmd cmd = createExternalImportCmd(true, false); + importExternalKvmVm(cmd); + verifyExternalKvmMacPolicy(true, false); + } + + @Test + public void testImportFromExternalForwardsExplicitForcedFalse() throws InsufficientServerCapacityException { + ImportVmCmd cmd = createExternalImportCmd(false, false); + importExternalKvmVm(cmd); + verifyExternalKvmMacPolicy(false, false); + } + + @Test + public void testImportFromExternalForwardsDuplicateMacAllowance() throws InsufficientServerCapacityException { + ImportVmCmd cmd = createExternalImportCmd(false, true); + importExternalKvmVm(cmd); + verifyExternalKvmMacPolicy(false, true); + } + + @Test + public void testImportFromExternalRejectsIncompatibleMacOptionsBeforeImportSideEffects() { + ImportVmCmd cmd = createExternalImportCmd(true, true); + + try { + unmanagedVMsManager.importVm(cmd); + Assert.fail("Expected incompatible MAC address conflict options to be rejected"); + } catch (InvalidParameterValueException expected) { + Assert.assertTrue(expected.getMessage().contains(ApiConstants.FORCED)); + Assert.assertTrue(expected.getMessage().contains(ApiConstants.ALLOW_DUPLICATE_MAC_ADDRESSES)); + } + + verify(agentManager, never()).easySend(anyLong(), any(GetRemoteVmsCommand.class)); + verify(volumeManager, never()).allocateRawVolume(any(), any(), any(), any(), any(), any(), any(), any(), any(), any(), any(), anyBoolean()); + } + + private ImportVmCmd createExternalImportCmd(Boolean forced, boolean allowDuplicateMacAddresses) { ImportVmCmd cmd = Mockito.mock(ImportVmCmd.class); when(cmd.getHypervisor()).thenReturn(Hypervisor.HypervisorType.KVM.toString()); - when(cmd.getName()).thenReturn(vmname); + when(cmd.getName()).thenReturn("TestInstance"); when(cmd.getUsername()).thenReturn("user"); when(cmd.getPassword()).thenReturn("pass"); when(cmd.getImportSource()).thenReturn("external"); when(cmd.getDomainId()).thenReturn(null); + when(cmd.getForced()).thenReturn(forced); + when(cmd.isAllowDuplicateMacAddresses()).thenReturn(allowDuplicateMacAddresses); + return cmd; + } + + private void importExternalKvmVm(ImportVmCmd cmd) throws InsufficientServerCapacityException { HostVO host = Mockito.mock(HostVO.class); DeployDestination mockDest = Mockito.mock(DeployDestination.class); when(deploymentPlanningManager.planDeployment(any(), any(), any(), any())).thenReturn(mockDest); @@ -726,6 +793,12 @@ public void testImportFromExternalTest() throws InsufficientServerCapacityExcept } } + private void verifyExternalKvmMacPolicy(boolean forced, boolean allowDuplicateMacAddresses) { + verify(networkOrchestrationService).importNic(Mockito.eq(instance.getNics().get(0).getMacAddress()), anyInt(), + any(Network.class), anyBoolean(), any(VirtualMachine.class), nullable(Network.IpAddresses.class), + any(DataCenter.class), Mockito.eq(forced), Mockito.eq(allowDuplicateMacAddresses)); + } + private void baseBasicParametersCheckForImportInstance(String name, Long domainId, String accountName) { unmanagedVMsManager.basicParametersCheckForImportInstance(name, domainId, accountName); } diff --git a/ui/src/utils/importVmMacAddressPolicy.js b/ui/src/utils/importVmMacAddressPolicy.js new file mode 100644 index 000000000000..8b299b19ed00 --- /dev/null +++ b/ui/src/utils/importVmMacAddressPolicy.js @@ -0,0 +1,42 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +export const getDefaultMacAddressConflictOptions = (isExternalKvmImport) => ({ + forced: Boolean(isExternalKvmImport), + allowDuplicateMacAddresses: false +}) + +export const selectForcedMacAddressConflictOption = (options, forced) => ({ + forced, + allowDuplicateMacAddresses: forced ? false : options.allowDuplicateMacAddresses +}) + +export const selectAllowDuplicateMacAddressesOption = (options, allowDuplicateMacAddresses) => ({ + forced: allowDuplicateMacAddresses ? false : options.forced, + allowDuplicateMacAddresses +}) + +export const getMacAddressConflictApiParams = (options, isExternalKvmImport, showForced, showAllowDuplicateMacAddresses) => { + const params = {} + if (showForced && (isExternalKvmImport || options.forced)) { + params.forced = Boolean(options.forced) + } + if (showAllowDuplicateMacAddresses && options.allowDuplicateMacAddresses) { + params.allowduplicatemacaddresses = true + } + return params +} diff --git a/ui/src/views/tools/ImportUnmanagedInstance.vue b/ui/src/views/tools/ImportUnmanagedInstance.vue index 12c6cd991eba..49af6c16ffb1 100644 --- a/ui/src/views/tools/ImportUnmanagedInstance.vue +++ b/ui/src/views/tools/ImportUnmanagedInstance.vue @@ -481,6 +481,12 @@ import OsLogo from '@/components/widgets/OsLogo' import ResourceIcon from '@/components/view/ResourceIcon' import CheckBoxSelectPair from '@/components/CheckBoxSelectPair' import { formatKvmConversionHostName } from '@/utils/vmware' +import { + getDefaultMacAddressConflictOptions, + getMacAddressConflictApiParams, + selectAllowDuplicateMacAddressesOption, + selectForcedMacAddressConflictOption +} from '@/utils/importVmMacAddressPolicy' export default { name: 'ImportUnmanagedInstances', @@ -738,6 +744,9 @@ export default { } return false }, + isExternalKvmImport () { + return this.isExternalImport && this.hypervisor?.toLowerCase() === 'kvm' + }, isKVMUnmanage () { return this.hypervisor && this.hypervisor === 'kvm' && (this.importsource === 'unmanaged' || this.importsource === 'external') }, @@ -884,6 +893,9 @@ export default { methods: { initForm () { this.formRef = ref() + const macAddressConflictOptions = getDefaultMacAddressConflictOptions(this.isExternalKvmImport) + this.switches.forced = macAddressConflictOptions.forced + this.switches.allowDuplicateMacAddresses = macAddressConflictOptions.allowDuplicateMacAddresses this.form = reactive({ rootdiskid: 0, vmwaremigrationmode: this.defaultVmwareMigrationMode(), @@ -1276,20 +1288,18 @@ export default { this.onUseVddkChange(mode === 'vddk' || mode === 'cbt', isUserChange) }, onForcedMacConflictChange (val) { - this.switches.forced = val - this.form.forced = val - if (val) { - this.switches.allowDuplicateMacAddresses = false - this.form.allowduplicatemacaddresses = false - } + const options = selectForcedMacAddressConflictOption(this.switches, val) + this.switches.forced = options.forced + this.switches.allowDuplicateMacAddresses = options.allowDuplicateMacAddresses + this.form.forced = options.forced + this.form.allowduplicatemacaddresses = options.allowDuplicateMacAddresses }, onAllowDuplicateMacAddressesChange (val) { - this.switches.allowDuplicateMacAddresses = val - this.form.allowduplicatemacaddresses = val - if (val) { - this.switches.forced = false - this.form.forced = false - } + const options = selectAllowDuplicateMacAddressesOption(this.switches, val) + this.switches.forced = options.forced + this.switches.allowDuplicateMacAddresses = options.allowDuplicateMacAddresses + this.form.forced = options.forced + this.form.allowduplicatemacaddresses = options.allowDuplicateMacAddresses }, onUseVddkChange (val, isUserChange = true) { if (isUserChange) { @@ -1460,12 +1470,10 @@ export default { } } var keys = ['hostname', 'domainid', 'projectid', 'account', 'migrateallowed', 'osid'] - if (this.showMacConflictOptions) { - keys.push('forced') - } - if (this.showAllowDuplicateMacAddresses) { - keys.push('allowduplicatemacaddresses') - } + Object.assign(params, getMacAddressConflictApiParams({ + forced: values.forced, + allowDuplicateMacAddresses: values.allowduplicatemacaddresses + }, this.isExternalKvmImport, this.showMacConflictOptions, this.showAllowDuplicateMacAddresses)) if (this.templateType !== 'auto') { keys.push('templateid') } @@ -1737,11 +1745,14 @@ export default { this.updateComputeOffering(undefined) this.switches = {} this.form.vmwaremigrationmode = this.defaultVmwareMigrationMode() + const macAddressConflictOptions = getDefaultMacAddressConflictOptions(this.isExternalKvmImport) + this.switches.forced = macAddressConflictOptions.forced + this.switches.allowDuplicateMacAddresses = macAddressConflictOptions.allowDuplicateMacAddresses this.form.usevddk = false this.form.forceconverttopool = false this.form.forcemstoimportvmfiles = false - this.form.forced = false - this.form.allowduplicatemacaddresses = false + this.form.forced = macAddressConflictOptions.forced + this.form.allowduplicatemacaddresses = macAddressConflictOptions.allowDuplicateMacAddresses this.userModifiedVddkSetting = false this.resetStorageOptionsForConversion() }, diff --git a/ui/tests/unit/utils/importVmMacAddressPolicy.spec.js b/ui/tests/unit/utils/importVmMacAddressPolicy.spec.js new file mode 100644 index 000000000000..8ee797159aec --- /dev/null +++ b/ui/tests/unit/utils/importVmMacAddressPolicy.spec.js @@ -0,0 +1,89 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +import { + getDefaultMacAddressConflictOptions, + getMacAddressConflictApiParams, + selectAllowDuplicateMacAddressesOption, + selectForcedMacAddressConflictOption +} from '@/utils/importVmMacAddressPolicy' + +describe('remote KVM MAC address conflict options', () => { + it('defaults remote KVM imports to replacement and leaves other imports unchanged', () => { + expect(getDefaultMacAddressConflictOptions(true)).toEqual({ + forced: true, + allowDuplicateMacAddresses: false + }) + expect(getDefaultMacAddressConflictOptions(false)).toEqual({ + forced: false, + allowDuplicateMacAddresses: false + }) + }) + + it('keeps replacement and duplicate preservation mutually exclusive', () => { + expect(selectAllowDuplicateMacAddressesOption({ + forced: true, + allowDuplicateMacAddresses: false + }, true)).toEqual({ + forced: false, + allowDuplicateMacAddresses: true + }) + + expect(selectForcedMacAddressConflictOption({ + forced: false, + allowDuplicateMacAddresses: true + }, true)).toEqual({ + forced: true, + allowDuplicateMacAddresses: false + }) + }) + + it('submits an explicit false when replacement is disabled for remote KVM', () => { + expect(getMacAddressConflictApiParams({ + forced: false, + allowDuplicateMacAddresses: false + }, true, true, true)).toEqual({ forced: false }) + }) + + it('submits duplicate preservation together with replacement disabled', () => { + expect(getMacAddressConflictApiParams({ + forced: false, + allowDuplicateMacAddresses: true + }, true, true, true)).toEqual({ + forced: false, + allowduplicatemacaddresses: true + }) + }) + + it('preserves truthy-only serialization for other import paths', () => { + expect(getMacAddressConflictApiParams({ + forced: false, + allowDuplicateMacAddresses: false + }, false, true, true)).toEqual({}) + expect(getMacAddressConflictApiParams({ + forced: true, + allowDuplicateMacAddresses: false + }, false, true, true)).toEqual({ forced: true }) + }) + + it('does not submit duplicate preservation when the API capability is absent', () => { + expect(getMacAddressConflictApiParams({ + forced: false, + allowDuplicateMacAddresses: true + }, true, true, false)).toEqual({ forced: false }) + }) +}) From 691343402b70055621ac50bf6a156f7c23cf1612 Mon Sep 17 00:00:00 2001 From: andrijapanicsb Date: Tue, 6 Oct 2026 06:38:21 +0200 Subject: [PATCH 3/3] Fix VM import test compilation Declare the checked exceptions raised by importNic Mockito verifications and cover remote KVM duplicate-MAC forwarding when forced is omitted. --- .../vm/UnmanagedVMsManagerImplTest.java | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/server/src/test/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImplTest.java b/server/src/test/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImplTest.java index 0e119199ca44..1d59fff55346 100644 --- a/server/src/test/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImplTest.java +++ b/server/src/test/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImplTest.java @@ -511,7 +511,7 @@ public void importUnmanagedInstanceTest() { } @Test - public void importUnmanagedVmwareInstanceForwardsDuplicateMacPolicy() { + public void importUnmanagedVmwareInstanceForwardsDuplicateMacPolicy() throws Exception { ImportUnmanagedInstanceCmd cmd = Mockito.mock(ImportUnmanagedInstanceCmd.class); when(cmd.getName()).thenReturn("TestInstance"); when(cmd.getDomainId()).thenReturn(null); @@ -714,33 +714,40 @@ public void testListRemoteInstancesTestNonKVM() { unmanagedVMsManager.listVmsForImport(cmd); } @Test - public void testImportFromExternalDefaultsToReplacingConflictingMac() throws InsufficientServerCapacityException { + public void testImportFromExternalDefaultsToReplacingConflictingMac() throws Exception { ImportVmCmd cmd = createExternalImportCmd(null, false); importExternalKvmVm(cmd); verifyExternalKvmMacPolicy(true, false); } @Test - public void testImportFromExternalForwardsExplicitForcedTrue() throws InsufficientServerCapacityException { + public void testImportFromExternalForwardsExplicitForcedTrue() throws Exception { ImportVmCmd cmd = createExternalImportCmd(true, false); importExternalKvmVm(cmd); verifyExternalKvmMacPolicy(true, false); } @Test - public void testImportFromExternalForwardsExplicitForcedFalse() throws InsufficientServerCapacityException { + public void testImportFromExternalForwardsExplicitForcedFalse() throws Exception { ImportVmCmd cmd = createExternalImportCmd(false, false); importExternalKvmVm(cmd); verifyExternalKvmMacPolicy(false, false); } @Test - public void testImportFromExternalForwardsDuplicateMacAllowance() throws InsufficientServerCapacityException { + public void testImportFromExternalForwardsDuplicateMacAllowance() throws Exception { ImportVmCmd cmd = createExternalImportCmd(false, true); importExternalKvmVm(cmd); verifyExternalKvmMacPolicy(false, true); } + @Test + public void testImportFromExternalForwardsDuplicateMacAllowanceWhenForcedIsOmitted() throws Exception { + ImportVmCmd cmd = createExternalImportCmd(null, true); + importExternalKvmVm(cmd); + verifyExternalKvmMacPolicy(false, true); + } + @Test public void testImportFromExternalRejectsIncompatibleMacOptionsBeforeImportSideEffects() { ImportVmCmd cmd = createExternalImportCmd(true, true); @@ -793,7 +800,7 @@ private void importExternalKvmVm(ImportVmCmd cmd) throws InsufficientServerCapac } } - private void verifyExternalKvmMacPolicy(boolean forced, boolean allowDuplicateMacAddresses) { + private void verifyExternalKvmMacPolicy(boolean forced, boolean allowDuplicateMacAddresses) throws Exception { verify(networkOrchestrationService).importNic(Mockito.eq(instance.getNics().get(0).getMacAddress()), anyInt(), any(Network.class), anyBoolean(), any(VirtualMachine.class), nullable(Network.IpAddresses.class), any(DataCenter.class), Mockito.eq(forced), Mockito.eq(allowDuplicateMacAddresses));