From b8d3e342be2134cfe61a654ce2826ad0c1bc9b3e Mon Sep 17 00:00:00 2001 From: Nicolas Vazquez Date: Wed, 10 Jan 2024 04:51:00 -0300 Subject: [PATCH] Fix KVM import unmanaged instances on basic zone (#8465) This PR fixes import unmanaged instances on KVM basic zones, on top of #8433 Fixes: #8439: point 1 --- .../service/NetworkOrchestrationService.java | 3 +- .../orchestration/NetworkOrchestrator.java | 46 +++++++--- .../NetworkOrchestratorTest.java | 88 +++++++++++++++++++ .../com/cloud/network/dao/IPAddressDao.java | 2 + .../cloud/network/dao/IPAddressDaoImpl.java | 9 ++ .../java/com/cloud/vm/UserVmManagerImpl.java | 2 +- .../vm/UnmanagedVMsManagerImpl.java | 5 +- .../com/cloud/vpc/MockNetworkManagerImpl.java | 3 +- .../vm/UnmanagedVMsManagerImplTest.java | 2 +- 9 files changed, 144 insertions(+), 16 deletions(-) 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 6d7c540613b..c691b8b0942 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 @@ -20,6 +20,7 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import com.cloud.dc.DataCenter; import org.apache.cloudstack.acl.ControlledEntity.ACLType; import org.apache.cloudstack.framework.config.ConfigKey; import org.apache.cloudstack.framework.config.ConfigKey.Scope; @@ -338,7 +339,7 @@ public interface NetworkOrchestrationService { */ 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, 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) 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 c6396dbdede..12271f5f105 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 @@ -4564,18 +4564,18 @@ public class NetworkOrchestrator extends ManagerBase implements NetworkOrchestra @DB @Override - public Pair importNic(final String macAddress, int deviceId, final Network network, final Boolean isDefaultNic, final VirtualMachine vm, final Network.IpAddresses ipAddresses, 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) throws ConcurrentOperationException, InsufficientVirtualNetworkCapacityException, InsufficientAddressCapacityException { s_logger.debug("Allocating nic for vm " + vm.getUuid() + " in network " + network + " during import"); String guestIp = null; + IPAddressVO freeIpAddress = null; if (ipAddresses != null && StringUtils.isNotEmpty(ipAddresses.getIp4Address())) { if (ipAddresses.getIp4Address().equals("auto")) { ipAddresses.setIp4Address(null); } - if (network.getGuestType() != GuestType.L2) { - guestIp = _ipAddrMgr.acquireGuestIpAddress(network, ipAddresses.getIp4Address()); - } else { - guestIp = null; + freeIpAddress = getGuestIpForNicImport(network, dataCenter, ipAddresses); + if (freeIpAddress != null && freeIpAddress.getAddress() != null) { + guestIp = freeIpAddress.getAddress().addr(); } if (guestIp == null && network.getGuestType() != GuestType.L2 && !_networkModel.listNetworkOfferingServices(network.getNetworkOfferingId()).isEmpty()) { throw new InsufficientVirtualNetworkCapacityException("Unable to acquire Guest IP address for network " + network, DataCenter.class, @@ -4583,6 +4583,7 @@ public class NetworkOrchestrator extends ManagerBase implements NetworkOrchestra } } final String finalGuestIp = guestIp; + final IPAddressVO freeIp = freeIpAddress; final NicVO vo = Transaction.execute(new TransactionCallback() { @Override public NicVO doInTransaction(TransactionStatus status) { @@ -4594,12 +4595,13 @@ public class NetworkOrchestrator extends ManagerBase implements NetworkOrchestra NicVO vo = new NicVO(network.getGuruName(), vm.getId(), network.getId(), vm.getType()); vo.setMacAddress(macAddressToPersist); vo.setAddressFormat(Networks.AddressFormat.Ip4); - if (NetUtils.isValidIp4(finalGuestIp) && StringUtils.isNotEmpty(network.getGateway())) { + Pair pair = getNetworkGatewayAndNetmaskForNicImport(network, dataCenter, freeIp); + String gateway = pair.first(); + String netmask = pair.second(); + if (NetUtils.isValidIp4(finalGuestIp) && StringUtils.isNotEmpty(gateway)) { vo.setIPv4Address(finalGuestIp); - vo.setIPv4Gateway(network.getGateway()); - if (StringUtils.isNotEmpty(network.getCidr())) { - vo.setIPv4Netmask(NetUtils.cidr2Netmask(network.getCidr())); - } + vo.setIPv4Gateway(gateway); + vo.setIPv4Netmask(netmask); } vo.setBroadcastUri(network.getBroadcastUri()); vo.setMode(network.getMode()); @@ -4636,6 +4638,30 @@ public class NetworkOrchestrator extends ManagerBase implements NetworkOrchestra return new Pair(vmNic, Integer.valueOf(deviceId)); } + protected IPAddressVO getGuestIpForNicImport(Network network, DataCenter dataCenter, Network.IpAddresses ipAddresses) { + if (network.getGuestType() == GuestType.L2) { + return null; + } + if (dataCenter.getNetworkType() == NetworkType.Advanced) { + String guestIpAddress = _ipAddrMgr.acquireGuestIpAddress(network, ipAddresses.getIp4Address()); + return _ipAddressDao.findByIp(guestIpAddress); + } + return _ipAddressDao.findBySourceNetworkIdAndDatacenterIdAndState(network.getId(), dataCenter.getId(), IpAddress.State.Free); + } + + /** + * Obtain the gateway and netmask for a VM NIC to import + * If the VM to import is on a Basic Zone, then obtain the information from the vlan table instead of the network + */ + protected Pair getNetworkGatewayAndNetmaskForNicImport(Network network, DataCenter dataCenter, IPAddressVO freeIp) { + VlanVO vlan = dataCenter.getNetworkType() == NetworkType.Basic && freeIp != null ? + _vlanDao.findById(freeIp.getVlanId()) : null; + String gateway = vlan != null ? vlan.getVlanGateway() : network.getGateway(); + String netmask = vlan != null ? vlan.getVlanNetmask() : + (StringUtils.isNotEmpty(network.getCidr()) ? NetUtils.cidr2Netmask(network.getCidr()) : null); + return new Pair<>(gateway, netmask); + } + private String generateNewMacAddressIfForced(Network network, String macAddress, boolean forced) { if (!forced) { throw new CloudRuntimeException("NIC with MAC address = " + macAddress + " exists on network with ID = " + network.getId() + 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 0aed5a2c259..3d2c96b083d 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 @@ -29,6 +29,9 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import com.cloud.dc.DataCenter; +import com.cloud.network.IpAddressManager; +import com.cloud.utils.Pair; import org.apache.log4j.Logger; import org.junit.Assert; import org.junit.Before; @@ -125,6 +128,7 @@ public class NetworkOrchestratorTest extends TestCase { testOrchastrator.routerNetworkDao = mock(RouterNetworkDao.class); testOrchastrator._vpcMgr = mock(VpcManager.class); testOrchastrator.routerJoinDao = mock(DomainRouterJoinDao.class); + testOrchastrator._ipAddrMgr = mock(IpAddressManager.class); DhcpServiceProvider provider = mock(DhcpServiceProvider.class); Map capabilities = new HashMap(); @@ -708,4 +712,88 @@ public class NetworkOrchestratorTest extends TestCase { Assert.assertEquals(ip6Dns[0], profile.getIPv6Dns1()); Assert.assertEquals(ip6Dns[1], profile.getIPv6Dns2()); } + + @Test + public void testGetNetworkGatewayAndNetmaskForNicImportAdvancedZone() { + Network network = Mockito.mock(Network.class); + DataCenter dataCenter = Mockito.mock(DataCenter.class); + IPAddressVO ipAddressVO = Mockito.mock(IPAddressVO.class); + + String networkGateway = "10.1.1.1"; + String networkNetmask = "255.255.255.0"; + String networkCidr = "10.1.1.0/24"; + Mockito.when(dataCenter.getNetworkType()).thenReturn(DataCenter.NetworkType.Advanced); + Mockito.when(network.getGateway()).thenReturn(networkGateway); + Mockito.when(network.getCidr()).thenReturn(networkCidr); + Pair pair = testOrchastrator.getNetworkGatewayAndNetmaskForNicImport(network, dataCenter, ipAddressVO); + Assert.assertNotNull(pair); + Assert.assertEquals(networkGateway, pair.first()); + Assert.assertEquals(networkNetmask, pair.second()); + } + + @Test + public void testGetNetworkGatewayAndNetmaskForNicImportBasicZone() { + Network network = Mockito.mock(Network.class); + DataCenter dataCenter = Mockito.mock(DataCenter.class); + IPAddressVO ipAddressVO = Mockito.mock(IPAddressVO.class); + + String defaultNetworkGateway = "10.1.1.1"; + String defaultNetworkNetmask = "255.255.255.0"; + VlanVO vlan = Mockito.mock(VlanVO.class); + Mockito.when(vlan.getVlanGateway()).thenReturn(defaultNetworkGateway); + Mockito.when(vlan.getVlanNetmask()).thenReturn(defaultNetworkNetmask); + Mockito.when(dataCenter.getNetworkType()).thenReturn(DataCenter.NetworkType.Basic); + Mockito.when(ipAddressVO.getVlanId()).thenReturn(1L); + Mockito.when(testOrchastrator._vlanDao.findById(1L)).thenReturn(vlan); + Pair pair = testOrchastrator.getNetworkGatewayAndNetmaskForNicImport(network, dataCenter, ipAddressVO); + Assert.assertNotNull(pair); + Assert.assertEquals(defaultNetworkGateway, pair.first()); + Assert.assertEquals(defaultNetworkNetmask, pair.second()); + } + + @Test + public void testGetGuestIpForNicImportL2Network() { + Network network = Mockito.mock(Network.class); + DataCenter dataCenter = Mockito.mock(DataCenter.class); + Network.IpAddresses ipAddresses = Mockito.mock(Network.IpAddresses.class); + Mockito.when(network.getGuestType()).thenReturn(GuestType.L2); + Assert.assertNull(testOrchastrator.getGuestIpForNicImport(network, dataCenter, ipAddresses)); + } + + @Test + public void testGetGuestIpForNicImportAdvancedZone() { + Network network = Mockito.mock(Network.class); + DataCenter dataCenter = Mockito.mock(DataCenter.class); + Network.IpAddresses ipAddresses = Mockito.mock(Network.IpAddresses.class); + Mockito.when(network.getGuestType()).thenReturn(GuestType.Isolated); + Mockito.when(dataCenter.getNetworkType()).thenReturn(DataCenter.NetworkType.Advanced); + String ipAddress = "10.1.10.10"; + IPAddressVO ipAddressVO = Mockito.mock(IPAddressVO.class); + Mockito.when(ipAddresses.getIp4Address()).thenReturn(ipAddress); + Mockito.when(testOrchastrator._ipAddrMgr.acquireGuestIpAddress(network, ipAddress)).thenReturn(ipAddress); + Ip ip = mock(Ip.class); + Mockito.when(ipAddressVO.getAddress()).thenReturn(ip); + Mockito.when(testOrchastrator._ipAddressDao.findByIp(ipAddress)).thenReturn(ipAddressVO); + IPAddressVO guestIp = testOrchastrator.getGuestIpForNicImport(network, dataCenter, ipAddresses); + Assert.assertEquals(ip, guestIp.getAddress()); + } + + @Test + public void testGetGuestIpForNicImportBasicZone() { + Network network = Mockito.mock(Network.class); + DataCenter dataCenter = Mockito.mock(DataCenter.class); + Network.IpAddresses ipAddresses = Mockito.mock(Network.IpAddresses.class); + Mockito.when(network.getGuestType()).thenReturn(GuestType.Isolated); + Mockito.when(dataCenter.getNetworkType()).thenReturn(DataCenter.NetworkType.Basic); + long networkId = 1L; + long dataCenterId = 1L; + IPAddressVO ipAddressVO = Mockito.mock(IPAddressVO.class); + Ip ip = mock(Ip.class); + Mockito.when(ipAddressVO.getAddress()).thenReturn(ip); + Mockito.when(network.getId()).thenReturn(networkId); + Mockito.when(dataCenter.getId()).thenReturn(dataCenterId); + Mockito.when(testOrchastrator._ipAddressDao.findBySourceNetworkIdAndDatacenterIdAndState(networkId, dataCenterId, State.Free)).thenReturn(ipAddressVO); + IPAddressVO guestIp = testOrchastrator.getGuestIpForNicImport(network, dataCenter, ipAddresses); + Assert.assertEquals(ip, guestIp.getAddress()); + } } diff --git a/engine/schema/src/main/java/com/cloud/network/dao/IPAddressDao.java b/engine/schema/src/main/java/com/cloud/network/dao/IPAddressDao.java index f75dc8a6661..b1b1e1cf757 100644 --- a/engine/schema/src/main/java/com/cloud/network/dao/IPAddressDao.java +++ b/engine/schema/src/main/java/com/cloud/network/dao/IPAddressDao.java @@ -103,4 +103,6 @@ public interface IPAddressDao extends GenericDao { List listByNetworkId(long networkId); void buildQuarantineSearchCriteria(SearchCriteria sc); + + IPAddressVO findBySourceNetworkIdAndDatacenterIdAndState(long sourceNetworkId, long dataCenterId, State state); } diff --git a/engine/schema/src/main/java/com/cloud/network/dao/IPAddressDaoImpl.java b/engine/schema/src/main/java/com/cloud/network/dao/IPAddressDaoImpl.java index 9ffc4c9159c..d1427522715 100644 --- a/engine/schema/src/main/java/com/cloud/network/dao/IPAddressDaoImpl.java +++ b/engine/schema/src/main/java/com/cloud/network/dao/IPAddressDaoImpl.java @@ -554,4 +554,13 @@ public class IPAddressDaoImpl extends GenericDaoBase implemen sc.setParametersIfNotNull("quarantinedPublicIpsIdsNIN", quarantinedIpsIdsAllowedToUser); } + + @Override + public IPAddressVO findBySourceNetworkIdAndDatacenterIdAndState(long sourceNetworkId, long dataCenterId, State state) { + SearchCriteria sc = AllFieldsSearch.create(); + sc.setParameters("sourcenetwork", sourceNetworkId); + sc.setParameters("dataCenterId", dataCenterId); + sc.setParameters("state", State.Free); + return findOneBy(sc); + } } diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index 121ca95e365..b7bd528e6e9 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java @@ -4581,7 +4581,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir Host host, Host lastHost, VirtualMachine.PowerState powerState) { if (isImport) { vm.setDataCenterId(zone.getId()); - if (hypervisorType == HypervisorType.VMware) { + if (List.of(HypervisorType.VMware, HypervisorType.KVM).contains(hypervisorType)) { vm.setHostId(host.getId()); } if (lastHost != null) { 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 559b6c7af06..85f2119b4ca 100644 --- a/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java +++ b/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java @@ -855,7 +855,8 @@ public class UnmanagedVMsManagerImpl implements UnmanagedVMsManager { } private NicProfile importNic(UnmanagedInstanceTO.Nic nic, VirtualMachine vm, Network network, Network.IpAddresses ipAddresses, int deviceId, boolean isDefaultNic, boolean forced) throws InsufficientVirtualNetworkCapacityException, InsufficientAddressCapacityException { - Pair result = networkOrchestrationService.importNic(nic.getMacAddress(), deviceId, network, isDefaultNic, vm, ipAddresses, forced); + DataCenterVO dataCenterVO = dataCenterDao.findById(network.getDataCenterId()); + Pair result = networkOrchestrationService.importNic(nic.getMacAddress(), deviceId, network, isDefaultNic, vm, ipAddresses, dataCenterVO, forced); if (result == null) { throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, String.format("NIC ID: %s import failed", nic.getNicId())); } @@ -2371,7 +2372,7 @@ public class UnmanagedVMsManagerImpl implements UnmanagedVMsManager { 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, true); + networkOrchestrationService.importNic(macAddress,0,network, true, userVm, requestedIpPair, zone, true); publishVMUsageUpdateResourceCount(userVm, serviceOffering); 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 7535841974e..04556c49ab2 100644 --- a/server/src/test/java/com/cloud/vpc/MockNetworkManagerImpl.java +++ b/server/src/test/java/com/cloud/vpc/MockNetworkManagerImpl.java @@ -24,6 +24,7 @@ import java.util.Map; import javax.inject.Inject; import javax.naming.ConfigurationException; +import com.cloud.dc.DataCenter; import com.cloud.network.PublicIpQuarantine; import org.apache.cloudstack.acl.ControlledEntity.ACLType; import org.apache.cloudstack.api.command.admin.address.ReleasePodIpCmdByAdmin; @@ -1030,7 +1031,7 @@ public class MockNetworkManagerImpl extends ManagerBase implements NetworkOrches } @Override - public Pair importNic(String macAddress, int deviceId, Network network, Boolean isDefaultNic, VirtualMachine vm, IpAddresses ipAddresses, boolean forced) { + public Pair importNic(String macAddress, int deviceId, Network network, Boolean isDefaultNic, VirtualMachine vm, IpAddresses ipAddresses, DataCenter dataCenter, boolean forced) { 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 e7831998353..229e59b1a7a 100644 --- a/server/src/test/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImplTest.java +++ b/server/src/test/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImplTest.java @@ -367,7 +367,7 @@ public class UnmanagedVMsManagerImplTest { 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), 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())).thenReturn(pair); when(volumeDao.findByInstance(Mockito.anyLong())).thenReturn(volumes); List userVmResponses = new ArrayList<>(); UserVmResponse userVmResponse = new UserVmResponse();