Skip to content

Commit a3a4833

Browse files
nvazqueznvazquez
authored
Fixes for KVM unmanaged instances import on advanced network and VNC password (apache#8492)
This PR fixes a regression caused by apache#8465 on advanced zones, import fails with: 2024-01-10 12:13:33,234 DEBUG [o.a.c.e.o.NetworkOrchestrator] (API-Job-Executor-3:ctx-991bbe9f job-128 ctx-f49517d4) (logid:d7b8e716) Allocating nic for vm 142272e8-9e2e-407b-9d7e-e9a03b81653c in network Network {"id": 204, "name": "Isolated", "uuid": "9679fac5-e3ac-4694-a57b-beb635340f39", "networkofferingid": 10} during import 2024-01-10 12:13:33,239 ERROR [o.a.c.v.UnmanagedVMsManagerImpl] (API-Job-Executor-3:ctx-991bbe9f job-128 ctx-f49517d4) (logid:d7b8e716) Failed to import NICs while importing vm: i-2-31-VM com.cloud.exception.InsufficientVirtualNetworkCapacityException: Unable to acquire Guest IP address for network Network {"id": 204, "name": "Isolated", "uuid": "9679fac5-e3ac-4694-a57b-beb635340f39", "networkofferingid": 10}Scope=interface com.cloud.dc.DataCenter; id=1 at org.apache.cloudstack.engine.orchestration.NetworkOrchestrator.importNic(NetworkOrchestrator.java:4582) at org.apache.cloudstack.vm.UnmanagedVMsManagerImpl.importNic(UnmanagedVMsManagerImpl.java:859) at org.apache.cloudstack.vm.UnmanagedVMsManagerImpl.importVirtualMachineInternal(UnmanagedVMsManagerImpl.java:1198) at org.apache.cloudstack.vm.UnmanagedVMsManagerImpl.importUnmanagedInstanceFromHypervisor(UnmanagedVMsManagerImpl.java:1511) at org.apache.cloudstack.vm.UnmanagedVMsManagerImpl.baseImportInstance(UnmanagedVMsManagerImpl.java:1342) at org.apache.cloudstack.vm.UnmanagedVMsManagerImpl.importUnmanagedInstance(UnmanagedVMsManagerImpl.java:1282) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method) Also, addresses the VNC password field set instead of a fixed string
1 parent 59e78cb commit a3a4833

4 files changed

Lines changed: 106 additions & 39 deletions

File tree

‎engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java‎

Lines changed: 33 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -4567,23 +4567,18 @@ public NicVO savePlaceholderNic(final Network network, final String ip4Address,
45674567
public Pair<NicProfile, Integer> 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)
45684568
throws ConcurrentOperationException, InsufficientVirtualNetworkCapacityException, InsufficientAddressCapacityException {
45694569
s_logger.debug("Allocating nic for vm " + vm.getUuid() + " in network " + network + " during import");
4570-
String guestIp = null;
4571-
IPAddressVO freeIpAddress = null;
4570+
String selectedIp = null;
45724571
if (ipAddresses != null && StringUtils.isNotEmpty(ipAddresses.getIp4Address())) {
45734572
if (ipAddresses.getIp4Address().equals("auto")) {
45744573
ipAddresses.setIp4Address(null);
45754574
}
4576-
freeIpAddress = getGuestIpForNicImport(network, dataCenter, ipAddresses);
4577-
if (freeIpAddress != null && freeIpAddress.getAddress() != null) {
4578-
guestIp = freeIpAddress.getAddress().addr();
4579-
}
4580-
if (guestIp == null && network.getGuestType() != GuestType.L2 && !_networkModel.listNetworkOfferingServices(network.getNetworkOfferingId()).isEmpty()) {
4575+
selectedIp = getSelectedIpForNicImport(network, dataCenter, ipAddresses);
4576+
if (selectedIp == null && network.getGuestType() != GuestType.L2 && !_networkModel.listNetworkOfferingServices(network.getNetworkOfferingId()).isEmpty()) {
45814577
throw new InsufficientVirtualNetworkCapacityException("Unable to acquire Guest IP address for network " + network, DataCenter.class,
45824578
network.getDataCenterId());
45834579
}
45844580
}
4585-
final String finalGuestIp = guestIp;
4586-
final IPAddressVO freeIp = freeIpAddress;
4581+
final String finalSelectedIp = selectedIp;
45874582
final NicVO vo = Transaction.execute(new TransactionCallback<NicVO>() {
45884583
@Override
45894584
public NicVO doInTransaction(TransactionStatus status) {
@@ -4595,11 +4590,11 @@ public NicVO doInTransaction(TransactionStatus status) {
45954590
NicVO vo = new NicVO(network.getGuruName(), vm.getId(), network.getId(), vm.getType());
45964591
vo.setMacAddress(macAddressToPersist);
45974592
vo.setAddressFormat(Networks.AddressFormat.Ip4);
4598-
Pair<String, String> pair = getNetworkGatewayAndNetmaskForNicImport(network, dataCenter, freeIp);
4593+
Pair<String, String> pair = getNetworkGatewayAndNetmaskForNicImport(network, dataCenter, finalSelectedIp);
45994594
String gateway = pair.first();
46004595
String netmask = pair.second();
4601-
if (NetUtils.isValidIp4(finalGuestIp) && StringUtils.isNotEmpty(gateway)) {
4602-
vo.setIPv4Address(finalGuestIp);
4596+
if (NetUtils.isValidIp4(finalSelectedIp) && StringUtils.isNotEmpty(gateway)) {
4597+
vo.setIPv4Address(finalSelectedIp);
46034598
vo.setIPv4Gateway(gateway);
46044599
vo.setIPv4Netmask(netmask);
46054600
}
@@ -4638,27 +4633,42 @@ public NicVO doInTransaction(TransactionStatus status) {
46384633
return new Pair<NicProfile, Integer>(vmNic, Integer.valueOf(deviceId));
46394634
}
46404635

4641-
protected IPAddressVO getGuestIpForNicImport(Network network, DataCenter dataCenter, Network.IpAddresses ipAddresses) {
4636+
protected String getSelectedIpForNicImport(Network network, DataCenter dataCenter, Network.IpAddresses ipAddresses) {
46424637
if (network.getGuestType() == GuestType.L2) {
46434638
return null;
46444639
}
4645-
if (dataCenter.getNetworkType() == NetworkType.Advanced) {
4646-
String guestIpAddress = _ipAddrMgr.acquireGuestIpAddress(network, ipAddresses.getIp4Address());
4647-
return _ipAddressDao.findByIp(guestIpAddress);
4640+
return dataCenter.getNetworkType() == NetworkType.Basic ?
4641+
getSelectedIpForNicImportOnBasicZone(ipAddresses.getIp4Address(), network, dataCenter):
4642+
_ipAddrMgr.acquireGuestIpAddress(network, ipAddresses.getIp4Address());
4643+
}
4644+
4645+
protected String getSelectedIpForNicImportOnBasicZone(String requestedIp, Network network, DataCenter dataCenter) {
4646+
IPAddressVO ipAddressVO = StringUtils.isBlank(requestedIp) ?
4647+
_ipAddressDao.findBySourceNetworkIdAndDatacenterIdAndState(network.getId(), dataCenter.getId(), IpAddress.State.Free):
4648+
_ipAddressDao.findByIp(requestedIp);
4649+
if (ipAddressVO == null || ipAddressVO.getState() != IpAddress.State.Free) {
4650+
String msg = String.format("Cannot find a free IP to assign to VM NIC on network %s", network.getName());
4651+
s_logger.error(msg);
4652+
throw new CloudRuntimeException(msg);
46484653
}
4649-
return _ipAddressDao.findBySourceNetworkIdAndDatacenterIdAndState(network.getId(), dataCenter.getId(), IpAddress.State.Free);
4654+
return ipAddressVO.getAddress() != null ? ipAddressVO.getAddress().addr() : null;
46504655
}
46514656

46524657
/**
46534658
* Obtain the gateway and netmask for a VM NIC to import
46544659
* If the VM to import is on a Basic Zone, then obtain the information from the vlan table instead of the network
46554660
*/
4656-
protected Pair<String, String> getNetworkGatewayAndNetmaskForNicImport(Network network, DataCenter dataCenter, IPAddressVO freeIp) {
4657-
VlanVO vlan = dataCenter.getNetworkType() == NetworkType.Basic && freeIp != null ?
4658-
_vlanDao.findById(freeIp.getVlanId()) : null;
4659-
String gateway = vlan != null ? vlan.getVlanGateway() : network.getGateway();
4660-
String netmask = vlan != null ? vlan.getVlanNetmask() :
4661-
(StringUtils.isNotEmpty(network.getCidr()) ? NetUtils.cidr2Netmask(network.getCidr()) : null);
4661+
protected Pair<String, String> getNetworkGatewayAndNetmaskForNicImport(Network network, DataCenter dataCenter, String selectedIp) {
4662+
String gateway = network.getGateway();
4663+
String netmask = StringUtils.isNotEmpty(network.getCidr()) ? NetUtils.cidr2Netmask(network.getCidr()) : null;
4664+
if (dataCenter.getNetworkType() == NetworkType.Basic) {
4665+
IPAddressVO freeIp = _ipAddressDao.findByIp(selectedIp);
4666+
if (freeIp != null) {
4667+
VlanVO vlan = _vlanDao.findById(freeIp.getVlanId());
4668+
gateway = vlan != null ? vlan.getVlanGateway() : null;
4669+
netmask = vlan != null ? vlan.getVlanNetmask() : null;
4670+
}
4671+
}
46624672
return new Pair<>(gateway, netmask);
46634673
}
46644674

‎engine/orchestration/src/test/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestratorTest.java‎

Lines changed: 60 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -717,15 +717,15 @@ public void testPrepareNicNetworkRouterNoDnsVm() {
717717
public void testGetNetworkGatewayAndNetmaskForNicImportAdvancedZone() {
718718
Network network = Mockito.mock(Network.class);
719719
DataCenter dataCenter = Mockito.mock(DataCenter.class);
720-
IPAddressVO ipAddressVO = Mockito.mock(IPAddressVO.class);
720+
String ipAddress = "10.1.1.10";
721721

722722
String networkGateway = "10.1.1.1";
723723
String networkNetmask = "255.255.255.0";
724724
String networkCidr = "10.1.1.0/24";
725725
Mockito.when(dataCenter.getNetworkType()).thenReturn(DataCenter.NetworkType.Advanced);
726726
Mockito.when(network.getGateway()).thenReturn(networkGateway);
727727
Mockito.when(network.getCidr()).thenReturn(networkCidr);
728-
Pair<String, String> pair = testOrchastrator.getNetworkGatewayAndNetmaskForNicImport(network, dataCenter, ipAddressVO);
728+
Pair<String, String> pair = testOrchastrator.getNetworkGatewayAndNetmaskForNicImport(network, dataCenter, ipAddress);
729729
Assert.assertNotNull(pair);
730730
Assert.assertEquals(networkGateway, pair.first());
731731
Assert.assertEquals(networkNetmask, pair.second());
@@ -736,16 +736,18 @@ public void testGetNetworkGatewayAndNetmaskForNicImportBasicZone() {
736736
Network network = Mockito.mock(Network.class);
737737
DataCenter dataCenter = Mockito.mock(DataCenter.class);
738738
IPAddressVO ipAddressVO = Mockito.mock(IPAddressVO.class);
739+
String ipAddress = "172.1.1.10";
739740

740-
String defaultNetworkGateway = "10.1.1.1";
741+
String defaultNetworkGateway = "172.1.1.1";
741742
String defaultNetworkNetmask = "255.255.255.0";
742743
VlanVO vlan = Mockito.mock(VlanVO.class);
743744
Mockito.when(vlan.getVlanGateway()).thenReturn(defaultNetworkGateway);
744745
Mockito.when(vlan.getVlanNetmask()).thenReturn(defaultNetworkNetmask);
745746
Mockito.when(dataCenter.getNetworkType()).thenReturn(DataCenter.NetworkType.Basic);
746747
Mockito.when(ipAddressVO.getVlanId()).thenReturn(1L);
747748
Mockito.when(testOrchastrator._vlanDao.findById(1L)).thenReturn(vlan);
748-
Pair<String, String> pair = testOrchastrator.getNetworkGatewayAndNetmaskForNicImport(network, dataCenter, ipAddressVO);
749+
Mockito.when(testOrchastrator._ipAddressDao.findByIp(ipAddress)).thenReturn(ipAddressVO);
750+
Pair<String, String> pair = testOrchastrator.getNetworkGatewayAndNetmaskForNicImport(network, dataCenter, ipAddress);
749751
Assert.assertNotNull(pair);
750752
Assert.assertEquals(defaultNetworkGateway, pair.first());
751753
Assert.assertEquals(defaultNetworkNetmask, pair.second());
@@ -757,7 +759,7 @@ public void testGetGuestIpForNicImportL2Network() {
757759
DataCenter dataCenter = Mockito.mock(DataCenter.class);
758760
Network.IpAddresses ipAddresses = Mockito.mock(Network.IpAddresses.class);
759761
Mockito.when(network.getGuestType()).thenReturn(GuestType.L2);
760-
Assert.assertNull(testOrchastrator.getGuestIpForNicImport(network, dataCenter, ipAddresses));
762+
Assert.assertNull(testOrchastrator.getSelectedIpForNicImport(network, dataCenter, ipAddresses));
761763
}
762764

763765
@Test
@@ -768,32 +770,76 @@ public void testGetGuestIpForNicImportAdvancedZone() {
768770
Mockito.when(network.getGuestType()).thenReturn(GuestType.Isolated);
769771
Mockito.when(dataCenter.getNetworkType()).thenReturn(DataCenter.NetworkType.Advanced);
770772
String ipAddress = "10.1.10.10";
771-
IPAddressVO ipAddressVO = Mockito.mock(IPAddressVO.class);
772773
Mockito.when(ipAddresses.getIp4Address()).thenReturn(ipAddress);
773774
Mockito.when(testOrchastrator._ipAddrMgr.acquireGuestIpAddress(network, ipAddress)).thenReturn(ipAddress);
775+
String guestIp = testOrchastrator.getSelectedIpForNicImport(network, dataCenter, ipAddresses);
776+
Assert.assertEquals(ipAddress, guestIp);
777+
}
778+
779+
@Test
780+
public void testGetGuestIpForNicImportBasicZoneAutomaticIP() {
781+
Network network = Mockito.mock(Network.class);
782+
DataCenter dataCenter = Mockito.mock(DataCenter.class);
783+
Network.IpAddresses ipAddresses = Mockito.mock(Network.IpAddresses.class);
784+
Mockito.when(network.getGuestType()).thenReturn(GuestType.Shared);
785+
Mockito.when(dataCenter.getNetworkType()).thenReturn(DataCenter.NetworkType.Basic);
786+
long networkId = 1L;
787+
long dataCenterId = 1L;
788+
String freeIp = "172.10.10.10";
789+
IPAddressVO ipAddressVO = Mockito.mock(IPAddressVO.class);
774790
Ip ip = mock(Ip.class);
791+
Mockito.when(ip.addr()).thenReturn(freeIp);
775792
Mockito.when(ipAddressVO.getAddress()).thenReturn(ip);
776-
Mockito.when(testOrchastrator._ipAddressDao.findByIp(ipAddress)).thenReturn(ipAddressVO);
777-
IPAddressVO guestIp = testOrchastrator.getGuestIpForNicImport(network, dataCenter, ipAddresses);
778-
Assert.assertEquals(ip, guestIp.getAddress());
793+
Mockito.when(ipAddressVO.getState()).thenReturn(State.Free);
794+
Mockito.when(network.getId()).thenReturn(networkId);
795+
Mockito.when(dataCenter.getId()).thenReturn(dataCenterId);
796+
Mockito.when(testOrchastrator._ipAddressDao.findBySourceNetworkIdAndDatacenterIdAndState(networkId, dataCenterId, State.Free)).thenReturn(ipAddressVO);
797+
String ipAddress = testOrchastrator.getSelectedIpForNicImport(network, dataCenter, ipAddresses);
798+
Assert.assertEquals(freeIp, ipAddress);
779799
}
780800

781801
@Test
782-
public void testGetGuestIpForNicImportBasicZone() {
802+
public void testGetGuestIpForNicImportBasicZoneManualIP() {
783803
Network network = Mockito.mock(Network.class);
784804
DataCenter dataCenter = Mockito.mock(DataCenter.class);
785805
Network.IpAddresses ipAddresses = Mockito.mock(Network.IpAddresses.class);
786-
Mockito.when(network.getGuestType()).thenReturn(GuestType.Isolated);
806+
Mockito.when(network.getGuestType()).thenReturn(GuestType.Shared);
787807
Mockito.when(dataCenter.getNetworkType()).thenReturn(DataCenter.NetworkType.Basic);
788808
long networkId = 1L;
789809
long dataCenterId = 1L;
810+
String requestedIp = "172.10.10.10";
790811
IPAddressVO ipAddressVO = Mockito.mock(IPAddressVO.class);
791812
Ip ip = mock(Ip.class);
813+
Mockito.when(ip.addr()).thenReturn(requestedIp);
792814
Mockito.when(ipAddressVO.getAddress()).thenReturn(ip);
815+
Mockito.when(ipAddressVO.getState()).thenReturn(State.Free);
793816
Mockito.when(network.getId()).thenReturn(networkId);
794817
Mockito.when(dataCenter.getId()).thenReturn(dataCenterId);
795-
Mockito.when(testOrchastrator._ipAddressDao.findBySourceNetworkIdAndDatacenterIdAndState(networkId, dataCenterId, State.Free)).thenReturn(ipAddressVO);
796-
IPAddressVO guestIp = testOrchastrator.getGuestIpForNicImport(network, dataCenter, ipAddresses);
797-
Assert.assertEquals(ip, guestIp.getAddress());
818+
Mockito.when(ipAddresses.getIp4Address()).thenReturn(requestedIp);
819+
Mockito.when(testOrchastrator._ipAddressDao.findByIp(requestedIp)).thenReturn(ipAddressVO);
820+
String ipAddress = testOrchastrator.getSelectedIpForNicImport(network, dataCenter, ipAddresses);
821+
Assert.assertEquals(requestedIp, ipAddress);
822+
}
823+
824+
@Test(expected = CloudRuntimeException.class)
825+
public void testGetGuestIpForNicImportBasicUsedIP() {
826+
Network network = Mockito.mock(Network.class);
827+
DataCenter dataCenter = Mockito.mock(DataCenter.class);
828+
Network.IpAddresses ipAddresses = Mockito.mock(Network.IpAddresses.class);
829+
Mockito.when(network.getGuestType()).thenReturn(GuestType.Shared);
830+
Mockito.when(dataCenter.getNetworkType()).thenReturn(DataCenter.NetworkType.Basic);
831+
long networkId = 1L;
832+
long dataCenterId = 1L;
833+
String requestedIp = "172.10.10.10";
834+
IPAddressVO ipAddressVO = Mockito.mock(IPAddressVO.class);
835+
Ip ip = mock(Ip.class);
836+
Mockito.when(ip.addr()).thenReturn(requestedIp);
837+
Mockito.when(ipAddressVO.getAddress()).thenReturn(ip);
838+
Mockito.when(ipAddressVO.getState()).thenReturn(State.Allocated);
839+
Mockito.when(network.getId()).thenReturn(networkId);
840+
Mockito.when(dataCenter.getId()).thenReturn(dataCenterId);
841+
Mockito.when(ipAddresses.getIp4Address()).thenReturn(requestedIp);
842+
Mockito.when(testOrchastrator._ipAddressDao.findByIp(requestedIp)).thenReturn(ipAddressVO);
843+
testOrchastrator.getSelectedIpForNicImport(network, dataCenter, ipAddresses);
798844
}
799845
}

‎plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtGetUnmanagedInstancesCommandWrapper.java‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import com.cloud.utils.exception.CloudRuntimeException;
2929
import com.cloud.vm.VirtualMachine;
3030
import org.apache.cloudstack.vm.UnmanagedInstanceTO;
31+
import org.apache.commons.lang3.RandomStringUtils;
3132
import org.apache.commons.lang3.StringUtils;
3233
import org.apache.log4j.Logger;
3334
import org.libvirt.Connect;
@@ -43,6 +44,8 @@
4344
public final class LibvirtGetUnmanagedInstancesCommandWrapper extends CommandWrapper<GetUnmanagedInstancesCommand, GetUnmanagedInstancesAnswer, LibvirtComputingResource> {
4445
private static final Logger LOGGER = Logger.getLogger(LibvirtGetUnmanagedInstancesCommandWrapper.class);
4546

47+
private static final int requiredVncPasswordLength = 22;
48+
4649
@Override
4750
public GetUnmanagedInstancesAnswer execute(GetUnmanagedInstancesCommand command, LibvirtComputingResource libvirtComputingResource) {
4851
LOGGER.info("Fetching unmanaged instance on host");
@@ -130,7 +133,7 @@ private UnmanagedInstanceTO getUnmanagedInstance(LibvirtComputingResource libvir
130133
instance.setMemory((int) LibvirtComputingResource.getDomainMemory(domain) / 1024);
131134
instance.setNics(getUnmanagedInstanceNics(parser.getInterfaces()));
132135
instance.setDisks(getUnmanagedInstanceDisks(parser.getDisks(),libvirtComputingResource, conn, domain.getName()));
133-
instance.setVncPassword(parser.getVncPasswd() + "aaaaaaaaaaaaaa"); // Suffix back extra characters for DB compatibility
136+
instance.setVncPassword(getFormattedVncPassword(parser.getVncPasswd()));
134137

135138
return instance;
136139
} catch (Exception e) {
@@ -139,6 +142,14 @@ private UnmanagedInstanceTO getUnmanagedInstance(LibvirtComputingResource libvir
139142
}
140143
}
141144

145+
protected String getFormattedVncPassword(String vncPasswd) {
146+
if (StringUtils.isBlank(vncPasswd)) {
147+
return null;
148+
}
149+
String randomChars = RandomStringUtils.random(requiredVncPasswordLength - vncPasswd.length(), true, false);
150+
return String.format("%s%s", vncPasswd, randomChars);
151+
}
152+
142153
private UnmanagedInstanceTO.PowerState getPowerState(VirtualMachine.PowerState vmPowerState) {
143154
switch (vmPowerState) {
144155
case PowerOn:

‎server/src/main/java/com/cloud/vm/UserVmManagerImpl.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4581,7 +4581,7 @@ protected void setVmRequiredFieldsForImport(boolean isImport, UserVmVO vm, DataC
45814581
Host host, Host lastHost, VirtualMachine.PowerState powerState) {
45824582
if (isImport) {
45834583
vm.setDataCenterId(zone.getId());
4584-
if (List.of(HypervisorType.VMware, HypervisorType.KVM).contains(hypervisorType)) {
4584+
if (List.of(HypervisorType.VMware, HypervisorType.KVM).contains(hypervisorType) && host != null) {
45854585
vm.setHostId(host.getId());
45864586
}
45874587
if (lastHost != null) {

0 commit comments

Comments
 (0)