Skip to content

Fixes for KVM unmanaged instances import on advanced network and VNC password #8492

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
shwstppr merged 7 commits into from
Jan 12, 2024
Merged

Fixes for KVM unmanaged instances import on advanced network and VNC password #8492

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
ca67e32
Refactor VNC password set on KVM unmanaged instance
nvazquez Jan 10, 2024
1548c5e
Fix IP assignment for advanced and basic zones
nvazquez Jan 10, 2024
2f688c2
Fix random characters for VNC password to use only letters
nvazquez Jan 10, 2024
51d96e9
Fix for manual IP assignment on basic zones
nvazquez Jan 10, 2024
6c264c9
Refactor method
nvazquez Jan 10, 2024
1a1ef27
Merge branch 'main' into kvm-unmanage-fixes-advanced-andpass
nvazquez Jan 11, 2024
f28f9b5
Add validation to prevent NPE on setting host
nvazquez Jan 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 33 additions & 23 deletions ...tration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4567,23 +4567,18 @@ public NicVO savePlaceholderNic(final Network network, final String ip4Address,
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)
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<NicVO>() {
@Override
public NicVO doInTransaction(TransactionStatus status) {
Expand All @@ -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<String, String> pair = getNetworkGatewayAndNetmaskForNicImport(network, dataCenter, freeIp);
Pair<String, String> 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);
}
Expand Down Expand Up @@ -4638,27 +4633,42 @@ public NicVO doInTransaction(TransactionStatus status) {
return new Pair<NicProfile, Integer>(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);
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 _ipAddressDao.findBySourceNetworkIdAndDatacenterIdAndState(network.getId(), dataCenter.getId(), IpAddress.State.Free);
return ipAddressVO.getAddress() != null ? ipAddressVO.getAddress().addr() : null;
}

/**
* 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<String, String> 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<String, String> 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);
}

Expand Down
74 changes: 60 additions & 14 deletions ...ion/src/test/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestratorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -717,15 +717,15 @@ 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";
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<String, String> pair = testOrchastrator.getNetworkGatewayAndNetmaskForNicImport(network, dataCenter, ipAddressVO);
Pair<String, String> pair = testOrchastrator.getNetworkGatewayAndNetmaskForNicImport(network, dataCenter, ipAddress);
Assert.assertNotNull(pair);
Assert.assertEquals(networkGateway, pair.first());
Assert.assertEquals(networkNetmask, pair.second());
Expand All @@ -736,16 +736,18 @@ 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);
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<String, String> pair = testOrchastrator.getNetworkGatewayAndNetmaskForNicImport(network, dataCenter, ipAddressVO);
Mockito.when(testOrchastrator._ipAddressDao.findByIp(ipAddress)).thenReturn(ipAddressVO);
Pair<String, String> pair = testOrchastrator.getNetworkGatewayAndNetmaskForNicImport(network, dataCenter, ipAddress);
Assert.assertNotNull(pair);
Assert.assertEquals(defaultNetworkGateway, pair.first());
Assert.assertEquals(defaultNetworkNetmask, pair.second());
Expand All @@ -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
Expand All @@ -768,32 +770,76 @@ 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);
String guestIp = testOrchastrator.getSelectedIpForNicImport(network, dataCenter, ipAddresses);
Assert.assertEquals(ipAddress, guestIp);
}

@Test
public void testGetGuestIpForNicImportBasicZoneAutomaticIP() {
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 freeIp = "172.10.10.10";
IPAddressVO ipAddressVO = Mockito.mock(IPAddressVO.class);
Ip ip = mock(Ip.class);
Mockito.when(ip.addr()).thenReturn(freeIp);
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());
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(freeIp, ipAddress);
}

@Test
public void testGetGuestIpForNicImportBasicZone() {
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.Isolated);
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(testOrchastrator._ipAddressDao.findBySourceNetworkIdAndDatacenterIdAndState(networkId, dataCenterId, State.Free)).thenReturn(ipAddressVO);
IPAddressVO guestIp = testOrchastrator.getGuestIpForNicImport(network, dataCenter, ipAddresses);
Assert.assertEquals(ip, guestIp.getAddress());
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);
}
}
13 changes: 12 additions & 1 deletion ...com/cloud/hypervisor/kvm/resource/wrapper/LibvirtGetUnmanagedInstancesCommandWrapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -43,6 +44,8 @@
public final class LibvirtGetUnmanagedInstancesCommandWrapper extends CommandWrapper<GetUnmanagedInstancesCommand, GetUnmanagedInstancesAnswer, LibvirtComputingResource> {
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");
Expand Down Expand Up @@ -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) {
Expand All @@ -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(), true, false);
return String.format("%s%s", vncPasswd, randomChars);
}

private UnmanagedInstanceTO.PowerState getPowerState(VirtualMachine.PowerState vmPowerState) {
switch (vmPowerState) {
case PowerOn:
Expand Down
2 changes: 1 addition & 1 deletion server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down