From ca67e326cedd2c0aec291aed3125410138968a6c Mon Sep 17 00:00:00 2001 From: nvazquez Date: Wed, 10 Jan 2024 09:30:16 -0300 Subject: [PATCH 1/6] Refactor VNC password set on KVM unmanaged instance --- .../LibvirtGetUnmanagedInstancesCommandWrapper.java | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtGetUnmanagedInstancesCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtGetUnmanagedInstancesCommandWrapper.java index 9a4498b12fd1..c238fd50e35b 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtGetUnmanagedInstancesCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtGetUnmanagedInstancesCommandWrapper.java @@ -28,6 +28,7 @@ import com.cloud.utils.exception.CloudRuntimeException; import com.cloud.vm.VirtualMachine; import org.apache.cloudstack.vm.UnmanagedInstanceTO; +import org.apache.commons.lang3.RandomStringUtils; import org.apache.commons.lang3.StringUtils; import org.apache.log4j.Logger; import org.libvirt.Connect; @@ -43,6 +44,8 @@ public final class LibvirtGetUnmanagedInstancesCommandWrapper extends CommandWrapper { private static final Logger LOGGER = Logger.getLogger(LibvirtGetUnmanagedInstancesCommandWrapper.class); + private static final int requiredVncPasswordLength = 22; + @Override public GetUnmanagedInstancesAnswer execute(GetUnmanagedInstancesCommand command, LibvirtComputingResource libvirtComputingResource) { LOGGER.info("Fetching unmanaged instance on host"); @@ -130,7 +133,7 @@ private UnmanagedInstanceTO getUnmanagedInstance(LibvirtComputingResource libvir instance.setMemory((int) LibvirtComputingResource.getDomainMemory(domain) / 1024); instance.setNics(getUnmanagedInstanceNics(parser.getInterfaces())); instance.setDisks(getUnmanagedInstanceDisks(parser.getDisks(),libvirtComputingResource, conn, domain.getName())); - instance.setVncPassword(parser.getVncPasswd() + "aaaaaaaaaaaaaa"); // Suffix back extra characters for DB compatibility + instance.setVncPassword(getFormattedVncPassword(parser.getVncPasswd())); return instance; } catch (Exception e) { @@ -139,6 +142,14 @@ private UnmanagedInstanceTO getUnmanagedInstance(LibvirtComputingResource libvir } } + protected String getFormattedVncPassword(String vncPasswd) { + if (StringUtils.isBlank(vncPasswd)) { + return null; + } + String randomChars = RandomStringUtils.random(requiredVncPasswordLength - vncPasswd.length()); + return String.format("%s%s", vncPasswd, randomChars); + } + private UnmanagedInstanceTO.PowerState getPowerState(VirtualMachine.PowerState vmPowerState) { switch (vmPowerState) { case PowerOn: From 1548c5ea9012c34f0538a8f7b010acd52a9d5543 Mon Sep 17 00:00:00 2001 From: nvazquez Date: Wed, 10 Jan 2024 09:59:46 -0300 Subject: [PATCH 2/6] Fix IP assignment for advanced and basic zones --- .../orchestration/NetworkOrchestrator.java | 46 +++++++++---------- .../NetworkOrchestratorTest.java | 28 +++++------ 2 files changed, 37 insertions(+), 37 deletions(-) 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 12271f5f105a..0d3c6046a700 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 @@ -4567,23 +4567,18 @@ public NicVO savePlaceholderNic(final Network network, final String ip4Address, 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; + String selectedIp = null; if (ipAddresses != null && StringUtils.isNotEmpty(ipAddresses.getIp4Address())) { if (ipAddresses.getIp4Address().equals("auto")) { ipAddresses.setIp4Address(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()) { + selectedIp = getSelectedIpForNicImport(network, dataCenter, ipAddresses); + if (selectedIp == null && network.getGuestType() != GuestType.L2 && !_networkModel.listNetworkOfferingServices(network.getNetworkOfferingId()).isEmpty()) { throw new InsufficientVirtualNetworkCapacityException("Unable to acquire Guest IP address for network " + network, DataCenter.class, network.getDataCenterId()); } } - final String finalGuestIp = guestIp; - final IPAddressVO freeIp = freeIpAddress; + final String finalSelectedIp = selectedIp; final NicVO vo = Transaction.execute(new TransactionCallback() { @Override public NicVO doInTransaction(TransactionStatus status) { @@ -4595,11 +4590,11 @@ public NicVO doInTransaction(TransactionStatus status) { NicVO vo = new NicVO(network.getGuruName(), vm.getId(), network.getId(), vm.getType()); vo.setMacAddress(macAddressToPersist); vo.setAddressFormat(Networks.AddressFormat.Ip4); - Pair pair = getNetworkGatewayAndNetmaskForNicImport(network, dataCenter, freeIp); + Pair pair = getNetworkGatewayAndNetmaskForNicImport(network, dataCenter, finalSelectedIp); String gateway = pair.first(); String netmask = pair.second(); - if (NetUtils.isValidIp4(finalGuestIp) && StringUtils.isNotEmpty(gateway)) { - vo.setIPv4Address(finalGuestIp); + if (NetUtils.isValidIp4(finalSelectedIp) && StringUtils.isNotEmpty(gateway)) { + vo.setIPv4Address(finalSelectedIp); vo.setIPv4Gateway(gateway); vo.setIPv4Netmask(netmask); } @@ -4638,27 +4633,32 @@ public NicVO doInTransaction(TransactionStatus status) { return new Pair(vmNic, Integer.valueOf(deviceId)); } - protected IPAddressVO getGuestIpForNicImport(Network network, DataCenter dataCenter, Network.IpAddresses ipAddresses) { + protected String getSelectedIpForNicImport(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); + if (dataCenter.getNetworkType() == NetworkType.Basic) { + IPAddressVO ipAddressVO = _ipAddressDao.findBySourceNetworkIdAndDatacenterIdAndState(network.getId(), dataCenter.getId(), IpAddress.State.Free); + return ipAddressVO != null && ipAddressVO.getAddress() != null ? ipAddressVO.getAddress().addr() : null; } - return _ipAddressDao.findBySourceNetworkIdAndDatacenterIdAndState(network.getId(), dataCenter.getId(), IpAddress.State.Free); + return _ipAddrMgr.acquireGuestIpAddress(network, ipAddresses.getIp4Address()); } /** * 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); + protected Pair getNetworkGatewayAndNetmaskForNicImport(Network network, DataCenter dataCenter, String selectedIp) { + String gateway = network.getGateway(); + String netmask = StringUtils.isNotEmpty(network.getCidr()) ? NetUtils.cidr2Netmask(network.getCidr()) : null; + if (dataCenter.getNetworkType() == NetworkType.Basic) { + IPAddressVO freeIp = _ipAddressDao.findByIp(selectedIp); + if (freeIp != null) { + VlanVO vlan = _vlanDao.findById(freeIp.getVlanId()); + gateway = vlan != null ? vlan.getVlanGateway() : null; + netmask = vlan != null ? vlan.getVlanNetmask() : null; + } + } return new Pair<>(gateway, netmask); } 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 3d2c96b083d2..22c0b7db4ad1 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 @@ -717,7 +717,7 @@ public void testPrepareNicNetworkRouterNoDnsVm() { public void testGetNetworkGatewayAndNetmaskForNicImportAdvancedZone() { Network network = Mockito.mock(Network.class); DataCenter dataCenter = Mockito.mock(DataCenter.class); - IPAddressVO ipAddressVO = Mockito.mock(IPAddressVO.class); + String ipAddress = "10.1.1.10"; String networkGateway = "10.1.1.1"; String networkNetmask = "255.255.255.0"; @@ -725,7 +725,7 @@ public void testGetNetworkGatewayAndNetmaskForNicImportAdvancedZone() { 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); + Pair pair = testOrchastrator.getNetworkGatewayAndNetmaskForNicImport(network, dataCenter, ipAddress); Assert.assertNotNull(pair); Assert.assertEquals(networkGateway, pair.first()); Assert.assertEquals(networkNetmask, pair.second()); @@ -736,8 +736,9 @@ public void testGetNetworkGatewayAndNetmaskForNicImportBasicZone() { Network network = Mockito.mock(Network.class); DataCenter dataCenter = Mockito.mock(DataCenter.class); IPAddressVO ipAddressVO = Mockito.mock(IPAddressVO.class); + String ipAddress = "172.1.1.10"; - String defaultNetworkGateway = "10.1.1.1"; + String defaultNetworkGateway = "172.1.1.1"; String defaultNetworkNetmask = "255.255.255.0"; VlanVO vlan = Mockito.mock(VlanVO.class); Mockito.when(vlan.getVlanGateway()).thenReturn(defaultNetworkGateway); @@ -745,7 +746,8 @@ public void testGetNetworkGatewayAndNetmaskForNicImportBasicZone() { 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); + Mockito.when(testOrchastrator._ipAddressDao.findByIp(ipAddress)).thenReturn(ipAddressVO); + Pair pair = testOrchastrator.getNetworkGatewayAndNetmaskForNicImport(network, dataCenter, ipAddress); Assert.assertNotNull(pair); Assert.assertEquals(defaultNetworkGateway, pair.first()); Assert.assertEquals(defaultNetworkNetmask, pair.second()); @@ -757,7 +759,7 @@ public void testGetGuestIpForNicImportL2Network() { 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)); + Assert.assertNull(testOrchastrator.getSelectedIpForNicImport(network, dataCenter, ipAddresses)); } @Test @@ -768,14 +770,10 @@ public void testGetGuestIpForNicImportAdvancedZone() { 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()); + String guestIp = testOrchastrator.getSelectedIpForNicImport(network, dataCenter, ipAddresses); + Assert.assertEquals(ipAddress, guestIp); } @Test @@ -783,17 +781,19 @@ 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(network.getGuestType()).thenReturn(GuestType.Shared); Mockito.when(dataCenter.getNetworkType()).thenReturn(DataCenter.NetworkType.Basic); long networkId = 1L; long dataCenterId = 1L; + String publicIp = "172.10.10.10"; IPAddressVO ipAddressVO = Mockito.mock(IPAddressVO.class); Ip ip = mock(Ip.class); + Mockito.when(ip.addr()).thenReturn(publicIp); 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()); + String ipAddress = testOrchastrator.getSelectedIpForNicImport(network, dataCenter, ipAddresses); + Assert.assertEquals(publicIp, ipAddress); } } From 2f688c20e856b8df8885fbe7ddbee37acf4b71b8 Mon Sep 17 00:00:00 2001 From: nvazquez Date: Wed, 10 Jan 2024 10:25:37 -0300 Subject: [PATCH 3/6] Fix random characters for VNC password to use only letters --- .../wrapper/LibvirtGetUnmanagedInstancesCommandWrapper.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtGetUnmanagedInstancesCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtGetUnmanagedInstancesCommandWrapper.java index c238fd50e35b..e2ef565b502d 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtGetUnmanagedInstancesCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtGetUnmanagedInstancesCommandWrapper.java @@ -146,7 +146,7 @@ protected String getFormattedVncPassword(String vncPasswd) { if (StringUtils.isBlank(vncPasswd)) { return null; } - String randomChars = RandomStringUtils.random(requiredVncPasswordLength - vncPasswd.length()); + String randomChars = RandomStringUtils.random(requiredVncPasswordLength - vncPasswd.length(), true, false); return String.format("%s%s", vncPasswd, randomChars); } From 51d96e98a3b228127b821ac835240db1a80fbbd7 Mon Sep 17 00:00:00 2001 From: nvazquez Date: Wed, 10 Jan 2024 12:47:50 -0300 Subject: [PATCH 4/6] Fix for manual IP assignment on basic zones --- .../orchestration/NetworkOrchestrator.java | 14 +++-- .../NetworkOrchestratorTest.java | 54 +++++++++++++++++-- 2 files changed, 61 insertions(+), 7 deletions(-) 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 0d3c6046a700..1bbf6d2bc68a 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 @@ -4637,11 +4637,19 @@ protected String getSelectedIpForNicImport(Network network, DataCenter dataCente if (network.getGuestType() == GuestType.L2) { return null; } + String requestedIp = ipAddresses.getIp4Address(); if (dataCenter.getNetworkType() == NetworkType.Basic) { - IPAddressVO ipAddressVO = _ipAddressDao.findBySourceNetworkIdAndDatacenterIdAndState(network.getId(), dataCenter.getId(), IpAddress.State.Free); - return ipAddressVO != null && ipAddressVO.getAddress() != null ? ipAddressVO.getAddress().addr() : null; + IPAddressVO ipAddressVO = StringUtils.isBlank(requestedIp) ? + _ipAddressDao.findBySourceNetworkIdAndDatacenterIdAndState(network.getId(), dataCenter.getId(), IpAddress.State.Free): + _ipAddressDao.findByIp(requestedIp); + if (ipAddressVO == null || ipAddressVO.getState() != IpAddress.State.Free) { + String msg = String.format("Cannot find a free IP to assign to VM NIC on network %s", network.getName()); + s_logger.error(msg); + throw new CloudRuntimeException(msg); + } + return ipAddressVO.getAddress() != null ? ipAddressVO.getAddress().addr() : null; } - return _ipAddrMgr.acquireGuestIpAddress(network, ipAddresses.getIp4Address()); + return _ipAddrMgr.acquireGuestIpAddress(network, requestedIp); } /** 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 22c0b7db4ad1..9e64eff18160 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 @@ -777,7 +777,7 @@ public void testGetGuestIpForNicImportAdvancedZone() { } @Test - public void testGetGuestIpForNicImportBasicZone() { + public void testGetGuestIpForNicImportBasicZoneAutomaticIP() { Network network = Mockito.mock(Network.class); DataCenter dataCenter = Mockito.mock(DataCenter.class); Network.IpAddresses ipAddresses = Mockito.mock(Network.IpAddresses.class); @@ -785,15 +785,61 @@ public void testGetGuestIpForNicImportBasicZone() { Mockito.when(dataCenter.getNetworkType()).thenReturn(DataCenter.NetworkType.Basic); long networkId = 1L; long dataCenterId = 1L; - String publicIp = "172.10.10.10"; + String freeIp = "172.10.10.10"; IPAddressVO ipAddressVO = Mockito.mock(IPAddressVO.class); Ip ip = mock(Ip.class); - Mockito.when(ip.addr()).thenReturn(publicIp); + Mockito.when(ip.addr()).thenReturn(freeIp); Mockito.when(ipAddressVO.getAddress()).thenReturn(ip); + Mockito.when(ipAddressVO.getState()).thenReturn(State.Free); Mockito.when(network.getId()).thenReturn(networkId); Mockito.when(dataCenter.getId()).thenReturn(dataCenterId); Mockito.when(testOrchastrator._ipAddressDao.findBySourceNetworkIdAndDatacenterIdAndState(networkId, dataCenterId, State.Free)).thenReturn(ipAddressVO); String ipAddress = testOrchastrator.getSelectedIpForNicImport(network, dataCenter, ipAddresses); - Assert.assertEquals(publicIp, ipAddress); + Assert.assertEquals(freeIp, ipAddress); + } + + @Test + public void testGetGuestIpForNicImportBasicZoneManualIP() { + 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.Shared); + Mockito.when(dataCenter.getNetworkType()).thenReturn(DataCenter.NetworkType.Basic); + long networkId = 1L; + long dataCenterId = 1L; + String requestedIp = "172.10.10.10"; + IPAddressVO ipAddressVO = Mockito.mock(IPAddressVO.class); + Ip ip = mock(Ip.class); + Mockito.when(ip.addr()).thenReturn(requestedIp); + Mockito.when(ipAddressVO.getAddress()).thenReturn(ip); + Mockito.when(ipAddressVO.getState()).thenReturn(State.Free); + Mockito.when(network.getId()).thenReturn(networkId); + Mockito.when(dataCenter.getId()).thenReturn(dataCenterId); + Mockito.when(ipAddresses.getIp4Address()).thenReturn(requestedIp); + Mockito.when(testOrchastrator._ipAddressDao.findByIp(requestedIp)).thenReturn(ipAddressVO); + String ipAddress = testOrchastrator.getSelectedIpForNicImport(network, dataCenter, ipAddresses); + Assert.assertEquals(requestedIp, ipAddress); + } + + @Test(expected = CloudRuntimeException.class) + public void testGetGuestIpForNicImportBasicUsedIP() { + 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.Shared); + Mockito.when(dataCenter.getNetworkType()).thenReturn(DataCenter.NetworkType.Basic); + long networkId = 1L; + long dataCenterId = 1L; + String requestedIp = "172.10.10.10"; + IPAddressVO ipAddressVO = Mockito.mock(IPAddressVO.class); + Ip ip = mock(Ip.class); + Mockito.when(ip.addr()).thenReturn(requestedIp); + Mockito.when(ipAddressVO.getAddress()).thenReturn(ip); + Mockito.when(ipAddressVO.getState()).thenReturn(State.Allocated); + Mockito.when(network.getId()).thenReturn(networkId); + Mockito.when(dataCenter.getId()).thenReturn(dataCenterId); + Mockito.when(ipAddresses.getIp4Address()).thenReturn(requestedIp); + Mockito.when(testOrchastrator._ipAddressDao.findByIp(requestedIp)).thenReturn(ipAddressVO); + testOrchastrator.getSelectedIpForNicImport(network, dataCenter, ipAddresses); } } From 6c264c95bedd7e89e3188040241f3155ea7aab08 Mon Sep 17 00:00:00 2001 From: nvazquez Date: Wed, 10 Jan 2024 13:00:28 -0300 Subject: [PATCH 5/6] Refactor method --- .../orchestration/NetworkOrchestrator.java | 26 ++++++++++--------- 1 file changed, 14 insertions(+), 12 deletions(-) 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 1bbf6d2bc68a..0f6dfe608669 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 @@ -4637,19 +4637,21 @@ protected String getSelectedIpForNicImport(Network network, DataCenter dataCente if (network.getGuestType() == GuestType.L2) { return null; } - String requestedIp = ipAddresses.getIp4Address(); - if (dataCenter.getNetworkType() == NetworkType.Basic) { - IPAddressVO ipAddressVO = StringUtils.isBlank(requestedIp) ? - _ipAddressDao.findBySourceNetworkIdAndDatacenterIdAndState(network.getId(), dataCenter.getId(), IpAddress.State.Free): - _ipAddressDao.findByIp(requestedIp); - if (ipAddressVO == null || ipAddressVO.getState() != IpAddress.State.Free) { - String msg = String.format("Cannot find a free IP to assign to VM NIC on network %s", network.getName()); - s_logger.error(msg); - throw new CloudRuntimeException(msg); - } - return ipAddressVO.getAddress() != null ? ipAddressVO.getAddress().addr() : null; + return dataCenter.getNetworkType() == NetworkType.Basic ? + getSelectedIpForNicImportOnBasicZone(ipAddresses.getIp4Address(), network, dataCenter): + _ipAddrMgr.acquireGuestIpAddress(network, ipAddresses.getIp4Address()); + } + + protected String getSelectedIpForNicImportOnBasicZone(String requestedIp, Network network, DataCenter dataCenter) { + IPAddressVO ipAddressVO = StringUtils.isBlank(requestedIp) ? + _ipAddressDao.findBySourceNetworkIdAndDatacenterIdAndState(network.getId(), dataCenter.getId(), IpAddress.State.Free): + _ipAddressDao.findByIp(requestedIp); + if (ipAddressVO == null || ipAddressVO.getState() != IpAddress.State.Free) { + String msg = String.format("Cannot find a free IP to assign to VM NIC on network %s", network.getName()); + s_logger.error(msg); + throw new CloudRuntimeException(msg); } - return _ipAddrMgr.acquireGuestIpAddress(network, requestedIp); + return ipAddressVO.getAddress() != null ? ipAddressVO.getAddress().addr() : null; } /** From f28f9b5144ac12c083de3b5bb08c052119ccf938 Mon Sep 17 00:00:00 2001 From: nvazquez Date: Thu, 11 Jan 2024 10:36:19 -0300 Subject: [PATCH 6/6] Add validation to prevent NPE on setting host --- server/src/main/java/com/cloud/vm/UserVmManagerImpl.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index b7bd528e6e9d..2927db638abc 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 @@ protected void setVmRequiredFieldsForImport(boolean isImport, UserVmVO vm, DataC Host host, Host lastHost, VirtualMachine.PowerState powerState) { if (isImport) { vm.setDataCenterId(zone.getId()); - if (List.of(HypervisorType.VMware, HypervisorType.KVM).contains(hypervisorType)) { + if (List.of(HypervisorType.VMware, HypervisorType.KVM).contains(hypervisorType) && host != null) { vm.setHostId(host.getId()); } if (lastHost != null) {