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.

Author: andrijapanicsb

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);
     }

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<Transaction> transactionMocked = Mockito.mockStatic(Transaction.class)) {
+            transactionMocked.when(() -> Transaction.execute(any(TransactionCallback.class))).thenAnswer(invocation -> {
+                TransactionCallback<NicVO> 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
     // -----------------------------------------------------------------------

server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java

@@ -2648,6 +2648,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())) {
@@ -2715,7 +2726,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,
@@ -2747,7 +2759,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<String, Long> dataDiskOfferingMap,
                                                    final Map<String, Long> nicNetworkMap, final Map<String, Network.IpAddresses> callerNicIpAddressMap,
-                                                   final String remoteUrl, String username, String password, String tmpPath, final Map<String, String> details) throws ResourceAllocationException {
+                                                   final String remoteUrl, String username, String password, String tmpPath, final Map<String, String> details,
+                                                   final boolean forced, final boolean allowDuplicateMacAddress) throws ResourceAllocationException {
         UserVm userVm = null;
 
         Map<String, String> allDetails = new HashMap<>(details);
@@ -2845,7 +2858,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) {

server/src/test/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImplTest.java

@@ -26,6 +26,7 @@
 import static org.mockito.ArgumentMatchers.nullable;
 import static org.mockito.Mockito.doNothing;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
@@ -467,6 +468,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<UsageEventUtils> ignored = Mockito.mockStatic(UsageEventUtils.class);
+             MockedConstruction<CheckedReservation> 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);
@@ -653,15 +672,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);
@@ -684,6 +751,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);
     }

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
+}