KVM VMware import: request automatic IP allocation when a source IP cannot be preserved

When no explicit NIC IP is given and the source address cannot be preserved
(missing Tools data, address outside the target network CIDR, gateway clash or
address already in use), the import into an isolated or shared network used to
fail with "NIC needs a valid IP address". Fill in the "auto" marker instead, so
the network orchestrator picks a free address for that NIC. Lab-validated with
a two-NIC Windows guest (static NIC preserved, DHCP NIC auto-allocated).

Author: andrijapanicsb

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

@@ -2194,39 +2194,47 @@ protected Map<String, List<String>> buildSourceNicCidrMap(UnmanagedInstanceTO so
     public Map<String, Network.IpAddresses> autoFillStaticNicIpAddresses(Map<String, Long> nicNetworkMap,
             Map<String, Network.IpAddresses> nicIpAddressMap, Map<String, List<String>> nicIdToIpv4Cidrs) {
         Map<String, Network.IpAddresses> effective = nicIpAddressMap == null ? new HashMap<>() : new HashMap<>(nicIpAddressMap);
-        if (MapUtils.isEmpty(nicNetworkMap) || MapUtils.isEmpty(nicIdToIpv4Cidrs)) {
+        if (MapUtils.isEmpty(nicNetworkMap)) {
             return effective;
         }
         for (Map.Entry<String, Long> mapping : nicNetworkMap.entrySet()) {
             String nicId = mapping.getKey();
             if (effective.containsKey(nicId)) {
                 continue;
             }
-            List<String> sourceCidrs = nicIdToIpv4Cidrs.get(nicId);
-            if (CollectionUtils.isEmpty(sourceCidrs)) {
-                continue;
-            }
             NetworkVO network = networkDao.findById(mapping.getValue());
-            if (network == null || StringUtils.isBlank(network.getCidr())) {
+            if (network == null || network.getGuestType() == Network.GuestType.L2 || StringUtils.isBlank(network.getCidr())) {
                 continue;
             }
-            for (String sourceCidr : sourceCidrs) {
-                String sourceIp = sourceCidr.contains("/") ? sourceCidr.substring(0, sourceCidr.indexOf('/')) : sourceCidr;
-                if (!NetUtils.isValidIp4(sourceIp) || !NetUtils.isIpWithInCidrRange(sourceIp, network.getCidr())) {
-                    continue;
-                }
-                if (sourceIp.equals(network.getGateway())) {
-                    logger.warn("Not preserving source IP {} for NIC {}: it is the gateway of network {}", sourceIp, nicId, network.getUuid());
-                    continue;
-                }
-                if (nicDao.findByIp4AddressAndNetworkId(sourceIp, network.getId()) != null) {
-                    logger.warn("Not preserving source IP {} for NIC {}: it is already in use in network {}; the NIC falls back to automatic allocation",
-                            sourceIp, nicId, network.getUuid());
-                    continue;
+            String preservedIp = null;
+            List<String> sourceCidrs = nicIdToIpv4Cidrs == null ? null : nicIdToIpv4Cidrs.get(nicId);
+            if (CollectionUtils.isNotEmpty(sourceCidrs)) {
+                for (String sourceCidr : sourceCidrs) {
+                    String sourceIp = sourceCidr.contains("/") ? sourceCidr.substring(0, sourceCidr.indexOf('/')) : sourceCidr;
+                    if (!NetUtils.isValidIp4(sourceIp) || !NetUtils.isIpWithInCidrRange(sourceIp, network.getCidr())) {
+                        continue;
+                    }
+                    if (sourceIp.equals(network.getGateway())) {
+                        logger.warn("Not preserving source IP {} for NIC {}: it is the gateway of network {}", sourceIp, nicId, network.getUuid());
+                        continue;
+                    }
+                    if (nicDao.findByIp4AddressAndNetworkId(sourceIp, network.getId()) != null) {
+                        logger.warn("Not preserving source IP {} for NIC {}: it is already in use in network {}; the NIC falls back to automatic allocation",
+                                sourceIp, nicId, network.getUuid());
+                        continue;
+                    }
+                    preservedIp = sourceIp;
+                    break;
                 }
-                logger.info("Preserving static source IP {} for NIC {} in network {}", sourceIp, nicId, network.getUuid());
-                effective.put(nicId, new Network.IpAddresses(sourceIp, null));
-                break;
+            }
+            if (preservedIp != null) {
+                logger.info("Preserving static source IP {} for NIC {} in network {}", preservedIp, nicId, network.getUuid());
+                effective.put(nicId, new Network.IpAddresses(preservedIp, null));
+            } else {
+                // Isolated/shared networks reject import NICs without an IP; "auto" makes the
+                // network orchestrator pick a free address instead (see allocateNic import path).
+                logger.info("No preservable source IP for NIC {}; falling back to automatic allocation in network {}", nicId, network.getUuid());
+                effective.put(nicId, new Network.IpAddresses("auto", null));
             }
         }
         return effective;

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

@@ -1956,27 +1956,37 @@ public void testAutoFillKeepsCallerProvidedIp() {
     }
 
     @Test
-    public void testAutoFillSkipsIpOutsideTargetCidr() {
+    public void testAutoFillFallsBackToAutoForIpOutsideTargetCidr() {
         staticIpTestNetwork(200L, "10.1.1.0/24", "10.1.1.1");
         Map<String, com.cloud.network.Network.IpAddresses> result = unmanagedVMsManager.autoFillStaticNicIpAddresses(
                 Map.of("Network adapter 2", 200L), null,
                 Map.of("Network adapter 2", java.util.List.of("192.168.77.5/24")));
-        Assert.assertFalse(result.containsKey("Network adapter 2"));
+        Assert.assertEquals("auto", result.get("Network adapter 2").getIp4Address());
     }
 
     @Test
-    public void testAutoFillSkipsGatewayAndInUseIps() {
+    public void testAutoFillFallsBackToAutoForGatewayAndInUseIps() {
         NetworkVO network = staticIpTestNetwork(200L, "10.1.1.0/24", "10.1.1.1");
         Mockito.when(nicDao.findByIp4AddressAndNetworkId("10.1.1.151", 200L)).thenReturn(Mockito.mock(com.cloud.vm.NicVO.class));
         Map<String, com.cloud.network.Network.IpAddresses> result = unmanagedVMsManager.autoFillStaticNicIpAddresses(
                 Map.of("Network adapter 1", 200L, "Network adapter 2", 200L), null,
                 Map.of("Network adapter 1", java.util.List.of("10.1.1.1/24"),
                        "Network adapter 2", java.util.List.of("10.1.1.151/24")));
-        Assert.assertFalse(result.containsKey("Network adapter 1"));
-        Assert.assertFalse(result.containsKey("Network adapter 2"));
+        Assert.assertEquals("auto", result.get("Network adapter 1").getIp4Address());
+        Assert.assertEquals("auto", result.get("Network adapter 2").getIp4Address());
         Mockito.verify(network, Mockito.atLeastOnce()).getCidr();
     }
 
+    @Test
+    public void testAutoFillFallsBackToAutoWhenSourceReportsNoAddresses() {
+        // A NIC with no Tools-reported address at all (DHCP guest before lease, Tools down)
+        // must still be importable into an isolated network - via automatic allocation.
+        staticIpTestNetwork(200L, "10.1.1.0/24", "10.1.1.1");
+        Map<String, com.cloud.network.Network.IpAddresses> result = unmanagedVMsManager.autoFillStaticNicIpAddresses(
+                Map.of("Network adapter 1", 200L), null, new HashMap<>());
+        Assert.assertEquals("auto", result.get("Network adapter 1").getIp4Address());
+    }
+
     @Test
     public void testAutoFillAcceptsBareAddressWithoutPrefix() {
         // Older or limited VMware Tools report addresses without ipConfig, so the captured