This is an automated email from the ASF dual-hosted git repository.
shwstppr pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
The following commit(s) were added to refs/heads/main by this push:
new b8d3e342be2 Fix KVM import unmanaged instances on basic zone (#8465)
b8d3e342be2 is described below
commit b8d3e342be2134cfe61a654ce2826ad0c1bc9b3e
Author: Nicolas Vazquez <[email protected]>
AuthorDate: Wed Jan 10 04:51:00 2024 -0300
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 +-
.../engine/orchestration/NetworkOrchestrator.java | 46 ++++++++---
.../orchestration/NetworkOrchestratorTest.java | 88 ++++++++++++++++++++++
.../java/com/cloud/network/dao/IPAddressDao.java | 2 +
.../com/cloud/network/dao/IPAddressDaoImpl.java | 9 +++
.../main/java/com/cloud/vm/UserVmManagerImpl.java | 2 +-
.../cloudstack/vm/UnmanagedVMsManagerImpl.java | 5 +-
.../java/com/cloud/vpc/MockNetworkManagerImpl.java | 3 +-
.../cloudstack/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<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;
+ 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;
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<NicProfile, Integer> importNic(final String macAddress, int
deviceId, final Network network, final Boolean isDefaultNic, final
VirtualMachine vm, final Network.IpAddresses ipAddresses, final boolean forced)
+ 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;
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<NicVO>() {
@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<String, String> 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<NicProfile, Integer>(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<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);
+ 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<Network.Capability, String> capabilities = new
HashMap<Network.Capability, String>();
@@ -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<String, String> 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<String, String> 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<IPAddressVO, Long> {
List<IPAddressVO> listByNetworkId(long networkId);
void buildQuarantineSearchCriteria(SearchCriteria<IPAddressVO> 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<IPAddressVO, Long> implemen
sc.setParametersIfNotNull("quarantinedPublicIpsIdsNIN",
quarantinedIpsIdsAllowedToUser);
}
+
+ @Override
+ public IPAddressVO findBySourceNetworkIdAndDatacenterIdAndState(long
sourceNetworkId, long dataCenterId, State state) {
+ SearchCriteria<IPAddressVO> 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<NicProfile, Integer> result =
networkOrchestrationService.importNic(nic.getMacAddress(), deviceId, network,
isDefaultNic, vm, ipAddresses, forced);
+ DataCenterVO dataCenterVO =
dataCenterDao.findById(network.getDataCenterId());
+ Pair<NicProfile, Integer> 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<NicProfile, Integer> importNic(String macAddress, int
deviceId, Network network, Boolean isDefaultNic, VirtualMachine vm, IpAddresses
ipAddresses, boolean forced) {
+ public Pair<NicProfile, Integer> 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<NicProfile, Integer> pair = new Pair<NicProfile,
Integer>(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<UserVmResponse> userVmResponses = new ArrayList<>();
UserVmResponse userVmResponse = new UserVmResponse();
