Skip to content

Commit b8d3e34

Browse files
nvazqueznvazquez
authored
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
1 parent c569fe9 commit b8d3e34

File tree

9 files changed

+144
-16
lines changed

9 files changed

+144
-16
lines changed

‎engine/api/src/main/java/org/apache/cloudstack/engine/orchestration/service/NetworkOrchestrationService.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import java.util.List;
2121
import java.util.Map;
2222

23+
import com.cloud.dc.DataCenter;
2324
import org.apache.cloudstack.acl.ControlledEntity.ACLType;
2425
import org.apache.cloudstack.framework.config.ConfigKey;
2526
import org.apache.cloudstack.framework.config.ConfigKey.Scope;
@@ -338,7 +339,7 @@ void implementNetworkElementsAndResources(DeployDestination dest, ReservationCon
338339
*/
339340
void cleanupNicDhcpDnsEntry(Network network, VirtualMachineProfile vmProfile, NicProfile nicProfile);
340341

341-
Pair<NicProfile, Integer> importNic(final String macAddress, int deviceId, final Network network, final Boolean isDefaultNic, final VirtualMachine vm, final Network.IpAddresses ipAddresses, boolean forced) throws InsufficientVirtualNetworkCapacityException, InsufficientAddressCapacityException;
342+
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, boolean forced) throws InsufficientVirtualNetworkCapacityException, InsufficientAddressCapacityException;
342343

343344
void unmanageNics(VirtualMachineProfile vm);
344345
}

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

Lines changed: 36 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4564,25 +4564,26 @@ public NicVO savePlaceholderNic(final Network network, final String ip4Address,
45644564

45654565
@DB
45664566
@Override
4567-
public Pair<NicProfile, Integer> importNic(final String macAddress, int deviceId, final Network network, final Boolean isDefaultNic, final VirtualMachine vm, final Network.IpAddresses ipAddresses, final boolean forced)
4567+
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");
45704570
String guestIp = null;
4571+
IPAddressVO freeIpAddress = null;
45714572
if (ipAddresses != null && StringUtils.isNotEmpty(ipAddresses.getIp4Address())) {
45724573
if (ipAddresses.getIp4Address().equals("auto")) {
45734574
ipAddresses.setIp4Address(null);
45744575
}
4575-
if (network.getGuestType() != GuestType.L2) {
4576-
guestIp = _ipAddrMgr.acquireGuestIpAddress(network, ipAddresses.getIp4Address());
4577-
} else {
4578-
guestIp = null;
4576+
freeIpAddress = getGuestIpForNicImport(network, dataCenter, ipAddresses);
4577+
if (freeIpAddress != null && freeIpAddress.getAddress() != null) {
4578+
guestIp = freeIpAddress.getAddress().addr();
45794579
}
45804580
if (guestIp == null && network.getGuestType() != GuestType.L2 && !_networkModel.listNetworkOfferingServices(network.getNetworkOfferingId()).isEmpty()) {
45814581
throw new InsufficientVirtualNetworkCapacityException("Unable to acquire Guest IP address for network " + network, DataCenter.class,
45824582
network.getDataCenterId());
45834583
}
45844584
}
45854585
final String finalGuestIp = guestIp;
4586+
final IPAddressVO freeIp = freeIpAddress;
45864587
final NicVO vo = Transaction.execute(new TransactionCallback<NicVO>() {
45874588
@Override
45884589
public NicVO doInTransaction(TransactionStatus status) {
@@ -4594,12 +4595,13 @@ public NicVO doInTransaction(TransactionStatus status) {
45944595
NicVO vo = new NicVO(network.getGuruName(), vm.getId(), network.getId(), vm.getType());
45954596
vo.setMacAddress(macAddressToPersist);
45964597
vo.setAddressFormat(Networks.AddressFormat.Ip4);
4597-
if (NetUtils.isValidIp4(finalGuestIp) && StringUtils.isNotEmpty(network.getGateway())) {
4598+
Pair<String, String> pair = getNetworkGatewayAndNetmaskForNicImport(network, dataCenter, freeIp);
4599+
String gateway = pair.first();
4600+
String netmask = pair.second();
4601+
if (NetUtils.isValidIp4(finalGuestIp) && StringUtils.isNotEmpty(gateway)) {
45984602
vo.setIPv4Address(finalGuestIp);
4599-
vo.setIPv4Gateway(network.getGateway());
4600-
if (StringUtils.isNotEmpty(network.getCidr())) {
4601-
vo.setIPv4Netmask(NetUtils.cidr2Netmask(network.getCidr()));
4602-
}
4603+
vo.setIPv4Gateway(gateway);
4604+
vo.setIPv4Netmask(netmask);
46034605
}
46044606
vo.setBroadcastUri(network.getBroadcastUri());
46054607
vo.setMode(network.getMode());
@@ -4636,6 +4638,30 @@ public NicVO doInTransaction(TransactionStatus status) {
46364638
return new Pair<NicProfile, Integer>(vmNic, Integer.valueOf(deviceId));
46374639
}
46384640

4641+
protected IPAddressVO getGuestIpForNicImport(Network network, DataCenter dataCenter, Network.IpAddresses ipAddresses) {
4642+
if (network.getGuestType() == GuestType.L2) {
4643+
return null;
4644+
}
4645+
if (dataCenter.getNetworkType() == NetworkType.Advanced) {
4646+
String guestIpAddress = _ipAddrMgr.acquireGuestIpAddress(network, ipAddresses.getIp4Address());
4647+
return _ipAddressDao.findByIp(guestIpAddress);
4648+
}
4649+
return _ipAddressDao.findBySourceNetworkIdAndDatacenterIdAndState(network.getId(), dataCenter.getId(), IpAddress.State.Free);
4650+
}
4651+
4652+
/**
4653+
* Obtain the gateway and netmask for a VM NIC to import
4654+
* If the VM to import is on a Basic Zone, then obtain the information from the vlan table instead of the network
4655+
*/
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);
4662+
return new Pair<>(gateway, netmask);
4663+
}
4664+
46394665
private String generateNewMacAddressIfForced(Network network, String macAddress, boolean forced) {
46404666
if (!forced) {
46414667
throw new CloudRuntimeException("NIC with MAC address = " + macAddress + " exists on network with ID = " + network.getId() +

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

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,9 @@
2929
import java.util.List;
3030
import java.util.Map;
3131

32+
import com.cloud.dc.DataCenter;
33+
import com.cloud.network.IpAddressManager;
34+
import com.cloud.utils.Pair;
3235
import org.apache.log4j.Logger;
3336
import org.junit.Assert;
3437
import org.junit.Before;
@@ -125,6 +128,7 @@ public void setUp() {
125128
testOrchastrator.routerNetworkDao = mock(RouterNetworkDao.class);
126129
testOrchastrator._vpcMgr = mock(VpcManager.class);
127130
testOrchastrator.routerJoinDao = mock(DomainRouterJoinDao.class);
131+
testOrchastrator._ipAddrMgr = mock(IpAddressManager.class);
128132
DhcpServiceProvider provider = mock(DhcpServiceProvider.class);
129133

130134
Map<Network.Capability, String> capabilities = new HashMap<Network.Capability, String>();
@@ -708,4 +712,88 @@ public void testPrepareNicNetworkRouterNoDnsVm() {
708712
Assert.assertEquals(ip6Dns[0], profile.getIPv6Dns1());
709713
Assert.assertEquals(ip6Dns[1], profile.getIPv6Dns2());
710714
}
715+
716+
@Test
717+
public void testGetNetworkGatewayAndNetmaskForNicImportAdvancedZone() {
718+
Network network = Mockito.mock(Network.class);
719+
DataCenter dataCenter = Mockito.mock(DataCenter.class);
720+
IPAddressVO ipAddressVO = Mockito.mock(IPAddressVO.class);
721+
722+
String networkGateway = "10.1.1.1";
723+
String networkNetmask = "255.255.255.0";
724+
String networkCidr = "10.1.1.0/24";
725+
Mockito.when(dataCenter.getNetworkType()).thenReturn(DataCenter.NetworkType.Advanced);
726+
Mockito.when(network.getGateway()).thenReturn(networkGateway);
727+
Mockito.when(network.getCidr()).thenReturn(networkCidr);
728+
Pair<String, String> pair = testOrchastrator.getNetworkGatewayAndNetmaskForNicImport(network, dataCenter, ipAddressVO);
729+
Assert.assertNotNull(pair);
730+
Assert.assertEquals(networkGateway, pair.first());
731+
Assert.assertEquals(networkNetmask, pair.second());
732+
}
733+
734+
@Test
735+
public void testGetNetworkGatewayAndNetmaskForNicImportBasicZone() {
736+
Network network = Mockito.mock(Network.class);
737+
DataCenter dataCenter = Mockito.mock(DataCenter.class);
738+
IPAddressVO ipAddressVO = Mockito.mock(IPAddressVO.class);
739+
740+
String defaultNetworkGateway = "10.1.1.1";
741+
String defaultNetworkNetmask = "255.255.255.0";
742+
VlanVO vlan = Mockito.mock(VlanVO.class);
743+
Mockito.when(vlan.getVlanGateway()).thenReturn(defaultNetworkGateway);
744+
Mockito.when(vlan.getVlanNetmask()).thenReturn(defaultNetworkNetmask);
745+
Mockito.when(dataCenter.getNetworkType()).thenReturn(DataCenter.NetworkType.Basic);
746+
Mockito.when(ipAddressVO.getVlanId()).thenReturn(1L);
747+
Mockito.when(testOrchastrator._vlanDao.findById(1L)).thenReturn(vlan);
748+
Pair<String, String> pair = testOrchastrator.getNetworkGatewayAndNetmaskForNicImport(network, dataCenter, ipAddressVO);
749+
Assert.assertNotNull(pair);
750+
Assert.assertEquals(defaultNetworkGateway, pair.first());
751+
Assert.assertEquals(defaultNetworkNetmask, pair.second());
752+
}
753+
754+
@Test
755+
public void testGetGuestIpForNicImportL2Network() {
756+
Network network = Mockito.mock(Network.class);
757+
DataCenter dataCenter = Mockito.mock(DataCenter.class);
758+
Network.IpAddresses ipAddresses = Mockito.mock(Network.IpAddresses.class);
759+
Mockito.when(network.getGuestType()).thenReturn(GuestType.L2);
760+
Assert.assertNull(testOrchastrator.getGuestIpForNicImport(network, dataCenter, ipAddresses));
761+
}
762+
763+
@Test
764+
public void testGetGuestIpForNicImportAdvancedZone() {
765+
Network network = Mockito.mock(Network.class);
766+
DataCenter dataCenter = Mockito.mock(DataCenter.class);
767+
Network.IpAddresses ipAddresses = Mockito.mock(Network.IpAddresses.class);
768+
Mockito.when(network.getGuestType()).thenReturn(GuestType.Isolated);
769+
Mockito.when(dataCenter.getNetworkType()).thenReturn(DataCenter.NetworkType.Advanced);
770+
String ipAddress = "10.1.10.10";
771+
IPAddressVO ipAddressVO = Mockito.mock(IPAddressVO.class);
772+
Mockito.when(ipAddresses.getIp4Address()).thenReturn(ipAddress);
773+
Mockito.when(testOrchastrator._ipAddrMgr.acquireGuestIpAddress(network, ipAddress)).thenReturn(ipAddress);
774+
Ip ip = mock(Ip.class);
775+
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());
779+
}
780+
781+
@Test
782+
public void testGetGuestIpForNicImportBasicZone() {
783+
Network network = Mockito.mock(Network.class);
784+
DataCenter dataCenter = Mockito.mock(DataCenter.class);
785+
Network.IpAddresses ipAddresses = Mockito.mock(Network.IpAddresses.class);
786+
Mockito.when(network.getGuestType()).thenReturn(GuestType.Isolated);
787+
Mockito.when(dataCenter.getNetworkType()).thenReturn(DataCenter.NetworkType.Basic);
788+
long networkId = 1L;
789+
long dataCenterId = 1L;
790+
IPAddressVO ipAddressVO = Mockito.mock(IPAddressVO.class);
791+
Ip ip = mock(Ip.class);
792+
Mockito.when(ipAddressVO.getAddress()).thenReturn(ip);
793+
Mockito.when(network.getId()).thenReturn(networkId);
794+
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());
798+
}
711799
}

‎engine/schema/src/main/java/com/cloud/network/dao/IPAddressDao.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,4 +103,6 @@ public interface IPAddressDao extends GenericDao<IPAddressVO, Long> {
103103
List<IPAddressVO> listByNetworkId(long networkId);
104104

105105
void buildQuarantineSearchCriteria(SearchCriteria<IPAddressVO> sc);
106+
107+
IPAddressVO findBySourceNetworkIdAndDatacenterIdAndState(long sourceNetworkId, long dataCenterId, State state);
106108
}

‎engine/schema/src/main/java/com/cloud/network/dao/IPAddressDaoImpl.java‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -554,4 +554,13 @@ public void buildQuarantineSearchCriteria(SearchCriteria<IPAddressVO> sc) {
554554

555555
sc.setParametersIfNotNull("quarantinedPublicIpsIdsNIN", quarantinedIpsIdsAllowedToUser);
556556
}
557+
558+
@Override
559+
public IPAddressVO findBySourceNetworkIdAndDatacenterIdAndState(long sourceNetworkId, long dataCenterId, State state) {
560+
SearchCriteria<IPAddressVO> sc = AllFieldsSearch.create();
561+
sc.setParameters("sourcenetwork", sourceNetworkId);
562+
sc.setParameters("dataCenterId", dataCenterId);
563+
sc.setParameters("state", State.Free);
564+
return findOneBy(sc);
565+
}
557566
}

‎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 (hypervisorType == HypervisorType.VMware) {
4584+
if (List.of(HypervisorType.VMware, HypervisorType.KVM).contains(hypervisorType)) {
45854585
vm.setHostId(host.getId());
45864586
}
45874587
if (lastHost != null) {

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -855,7 +855,8 @@ private Pair<DiskProfile, StoragePool> importDisk(UnmanagedInstanceTO.Disk disk,
855855
}
856856

857857
private NicProfile importNic(UnmanagedInstanceTO.Nic nic, VirtualMachine vm, Network network, Network.IpAddresses ipAddresses, int deviceId, boolean isDefaultNic, boolean forced) throws InsufficientVirtualNetworkCapacityException, InsufficientAddressCapacityException {
858-
Pair<NicProfile, Integer> result = networkOrchestrationService.importNic(nic.getMacAddress(), deviceId, network, isDefaultNic, vm, ipAddresses, forced);
858+
DataCenterVO dataCenterVO = dataCenterDao.findById(network.getDataCenterId());
859+
Pair<NicProfile, Integer> result = networkOrchestrationService.importNic(nic.getMacAddress(), deviceId, network, isDefaultNic, vm, ipAddresses, dataCenterVO, forced);
859860
if (result == null) {
860861
throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, String.format("NIC ID: %s import failed", nic.getNicId()));
861862
}
@@ -2371,7 +2372,7 @@ private UserVm importKvmVirtualMachineFromDisk(final ImportSource importSource,
23712372
cleanupFailedImportVM(userVm);
23722373
throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, String.format("Failed to import volumes while importing vm: %s. %s", instanceName, StringUtils.defaultString(e.getMessage())));
23732374
}
2374-
networkOrchestrationService.importNic(macAddress,0,network, true, userVm, requestedIpPair, true);
2375+
networkOrchestrationService.importNic(macAddress,0,network, true, userVm, requestedIpPair, zone, true);
23752376
publishVMUsageUpdateResourceCount(userVm, serviceOffering);
23762377
return userVm;
23772378
}

‎server/src/test/java/com/cloud/vpc/MockNetworkManagerImpl.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import javax.inject.Inject;
2525
import javax.naming.ConfigurationException;
2626

27+
import com.cloud.dc.DataCenter;
2728
import com.cloud.network.PublicIpQuarantine;
2829
import org.apache.cloudstack.acl.ControlledEntity.ACLType;
2930
import org.apache.cloudstack.api.command.admin.address.ReleasePodIpCmdByAdmin;
@@ -1030,7 +1031,7 @@ public AcquirePodIpCmdResponse allocatePodIp(Account account, String zoneId, Str
10301031
}
10311032

10321033
@Override
1033-
public Pair<NicProfile, Integer> importNic(String macAddress, int deviceId, Network network, Boolean isDefaultNic, VirtualMachine vm, IpAddresses ipAddresses, boolean forced) {
1034+
public Pair<NicProfile, Integer> importNic(String macAddress, int deviceId, Network network, Boolean isDefaultNic, VirtualMachine vm, IpAddresses ipAddresses, DataCenter dataCenter, boolean forced) {
10341035
return null;
10351036
}
10361037

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -367,7 +367,7 @@ public void setUp() throws Exception {
367367
NicProfile profile = Mockito.mock(NicProfile.class);
368368
Integer deviceId = 100;
369369
Pair<NicProfile, Integer> pair = new Pair<NicProfile, Integer>(profile, deviceId);
370-
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);
370+
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);
371371
when(volumeDao.findByInstance(Mockito.anyLong())).thenReturn(volumes);
372372
List<UserVmResponse> userVmResponses = new ArrayList<>();
373373
UserVmResponse userVmResponse = new UserVmResponse();

0 commit comments

Comments
 (0)