From 5096a0b93169312a83f9255e3e4f95817798c449 Mon Sep 17 00:00:00 2001 From: Henrique Sato Date: Wed, 17 Jan 2024 14:22:50 -0300 Subject: [PATCH 01/16] Update vpc.max.networks setting --- .../com/cloud/network/vpc/VpcManager.java | 10 +++ .../api/query/dao/DomainRouterJoinDao.java | 2 + .../query/dao/DomainRouterJoinDaoImpl.java | 14 ++++ .../java/com/cloud/configuration/Config.java | 1 - .../com/cloud/network/vpc/VpcManagerImpl.java | 61 ++++++++++++++- .../cloud/network/vpc/VpcManagerImplTest.java | 75 ++++++++++++++++++- 6 files changed, 157 insertions(+), 6 deletions(-) diff --git a/engine/components-api/src/main/java/com/cloud/network/vpc/VpcManager.java b/engine/components-api/src/main/java/com/cloud/network/vpc/VpcManager.java index 24012a2487b8..11c0247f16a0 100644 --- a/engine/components-api/src/main/java/com/cloud/network/vpc/VpcManager.java +++ b/engine/components-api/src/main/java/com/cloud/network/vpc/VpcManager.java @@ -22,6 +22,7 @@ import com.cloud.utils.Pair; import org.apache.cloudstack.acl.ControlledEntity.ACLType; +import org.apache.cloudstack.framework.config.ConfigKey; import com.cloud.exception.ConcurrentOperationException; import com.cloud.exception.InsufficientAddressCapacityException; @@ -39,6 +40,15 @@ import com.cloud.user.Account; public interface VpcManager { + ConfigKey VpcMaxNetworks = new ConfigKey<>( + "Advanced", + Integer.class, + "vpc.max.networks", + "3", + "Maximum number of networks per VPC. Bear in mind that this value will depend on the hypervisor where the VR was/will be deployed.", + true, + ConfigKey.Scope.Cluster); + /** * Returns all the Guest networks that are part of VPC * diff --git a/server/src/main/java/com/cloud/api/query/dao/DomainRouterJoinDao.java b/server/src/main/java/com/cloud/api/query/dao/DomainRouterJoinDao.java index 509d02dd71c5..4dbe4be882bc 100644 --- a/server/src/main/java/com/cloud/api/query/dao/DomainRouterJoinDao.java +++ b/server/src/main/java/com/cloud/api/query/dao/DomainRouterJoinDao.java @@ -37,4 +37,6 @@ public interface DomainRouterJoinDao extends GenericDao searchByIds(Long... ids); List getRouterByIdAndTrafficType(Long id, Networks.TrafficType... trafficType); + + List listByVpcId(long vpcId); } diff --git a/server/src/main/java/com/cloud/api/query/dao/DomainRouterJoinDaoImpl.java b/server/src/main/java/com/cloud/api/query/dao/DomainRouterJoinDaoImpl.java index c6041c3e3732..7dfa4c3968fd 100644 --- a/server/src/main/java/com/cloud/api/query/dao/DomainRouterJoinDaoImpl.java +++ b/server/src/main/java/com/cloud/api/query/dao/DomainRouterJoinDaoImpl.java @@ -62,6 +62,7 @@ public class DomainRouterJoinDaoImpl extends GenericDaoBase vrIdSearch; private final SearchBuilder vrIdTrafficSearch; + private final SearchBuilder vrVpcIdSearch; protected DomainRouterJoinDaoImpl() { @@ -78,6 +79,12 @@ protected DomainRouterJoinDaoImpl() { vrIdTrafficSearch.and("trafficType", vrIdTrafficSearch.entity().getTrafficType(), SearchCriteria.Op.IN); vrIdTrafficSearch.done(); + vrVpcIdSearch = createSearchBuilder(); + vrVpcIdSearch.and("vpcId", vrVpcIdSearch.entity().getVpcId(), SearchCriteria.Op.EQ); + vrVpcIdSearch.and("clusterId", vrVpcIdSearch.entity().getClusterId(), SearchCriteria.Op.NNULL); + vrVpcIdSearch.groupBy(vrVpcIdSearch.entity().getClusterId()); + vrVpcIdSearch.done(); + _count = "select count(distinct id) from domain_router_view WHERE "; } @@ -357,4 +364,11 @@ public List newDomainRouterView(VirtualRouter vr) { return searchIncludingRemoved(sc, null, null, false); } + @Override + public List listByVpcId(long vpcId) { + SearchCriteria sc = vrVpcIdSearch.create(); + sc.setParameters("vpcId", vpcId); + + return listBy(sc); + } } diff --git a/server/src/main/java/com/cloud/configuration/Config.java b/server/src/main/java/com/cloud/configuration/Config.java index 2d677042b62c..2a85c0c8b939 100644 --- a/server/src/main/java/com/cloud/configuration/Config.java +++ b/server/src/main/java/com/cloud/configuration/Config.java @@ -1631,7 +1631,6 @@ public enum Config { "3600", "The interval (in seconds) between cleanup for Inactive VPCs", null), - VpcMaxNetworks("Advanced", ManagementServer.class, Integer.class, "vpc.max.networks", "3", "Maximum number of networks per vpc", null), DetailBatchQuerySize("Advanced", ManagementServer.class, Integer.class, "detail.batch.query.size", "2000", "Default entity detail batch query size for listing", null), NetworkIPv6SearchRetryMax( "Network", diff --git a/server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java b/server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java index e313b1bfdf04..f6a20bab7f4f 100644 --- a/server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java +++ b/server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java @@ -37,6 +37,11 @@ import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.TimeUnit; +import com.cloud.api.query.dao.DomainRouterJoinDao; +import com.cloud.api.query.vo.DomainRouterJoinVO; +import com.cloud.dc.ClusterVO; +import com.cloud.dc.dao.ClusterDao; + import javax.annotation.PostConstruct; import javax.inject.Inject; import javax.naming.ConfigurationException; @@ -59,6 +64,8 @@ import org.apache.cloudstack.api.command.user.vpc.UpdateVPCCmd; import org.apache.cloudstack.context.CallContext; import org.apache.cloudstack.engine.orchestration.service.NetworkOrchestrationService; +import org.apache.cloudstack.framework.config.ConfigKey; +import org.apache.cloudstack.framework.config.Configurable; import org.apache.cloudstack.framework.config.dao.ConfigurationDao; import org.apache.cloudstack.managed.context.ManagedContextRunnable; import org.apache.cloudstack.query.QueryService; @@ -177,7 +184,7 @@ import com.cloud.vm.dao.DomainRouterDao; import com.cloud.vm.dao.NicDao; -public class VpcManagerImpl extends ManagerBase implements VpcManager, VpcProvisioningService, VpcService { +public class VpcManagerImpl extends ManagerBase implements VpcManager, VpcProvisioningService, VpcService, Configurable { public static final String SERVICE = "service"; public static final String CAPABILITYTYPE = "capabilitytype"; @@ -256,6 +263,10 @@ public class VpcManagerImpl extends ManagerBase implements VpcManager, VpcProvis @Inject NicDao nicDao; @Inject + DomainRouterJoinDao domainRouterJoinDao; + @Inject + ClusterDao clusterDao; + @Inject AlertManager alertManager; @Inject CommandSetupHelper commandSetupHelper; @@ -372,8 +383,7 @@ public void doInTransactionWithoutResult(final TransactionStatus status) { final String value = configs.get(Config.VpcCleanupInterval.key()); _cleanupInterval = NumbersUtil.parseInt(value, 60 * 60); // 1 hour - final String maxNtwks = configs.get(Config.VpcMaxNetworks.key()); - _maxNetworks = NumbersUtil.parseInt(maxNtwks, 3); // max=3 is default + _maxNetworks = VpcMaxNetworks.value(); IpAddressSearch = _ipAddressDao.createSearchBuilder(); IpAddressSearch.and("accountId", IpAddressSearch.entity().getAllocatedToAccountId(), Op.EQ); @@ -1830,10 +1840,12 @@ public void doInTransactionWithoutResult(final TransactionStatus status) { throw new CloudRuntimeException("Unable to acquire lock on " + vpc); } + _maxNetworks = getVpcMaxNetworksValue(vpc.getId()); + try { // check number of active networks in vpc if (_ntwkDao.countVpcNetworks(vpc.getId()) >= _maxNetworks) { - logger.warn(String.format("Failed to create a new VPC Guest Network because the number of networks per VPC has reached its maximum capacity of [%s]. Increase it by modifying global config [%s].", _maxNetworks, Config.VpcMaxNetworks)); + logger.warn("Failed to create a new VPC Guest Network because the number of networks per VPC has reached its maximum capacity of {}. Increase it by modifying global or cluster config {}. Bear in mind that the maximum number of networks per VPC depends on the hypervisor where the VR is deployed; therefore, using the global value as reference might cause errors during the VR's deploy, if this situation is not considered.", _maxNetworks, VpcMaxNetworks); throw new CloudRuntimeException(String.format("Number of networks per VPC cannot surpass [%s].", _maxNetworks)); } @@ -1878,6 +1890,37 @@ public void doInTransactionWithoutResult(final TransactionStatus status) { }); } + /** + * Searches for the {@link #VpcMaxNetworks} value in the VPC domain routers cluster and returns the smallest value. + * If the VPC does not have any domain routers, it returns the {@link #VpcMaxNetworks} global configuration value. + * + * @param vpcId ID from VPC. + * @return The {@link #VpcMaxNetworks} configuration value. + */ + protected int getVpcMaxNetworksValue(long vpcId) { + List domainRouters = domainRouterJoinDao.listByVpcId(vpcId); + + if (domainRouters.isEmpty()) { + logger.warn("Using {} global configuration value {} as VPC does not have any VRs deployed within a cluster. Bear in mind that the maximum number of networks per VPC depends on the hypervisor where the VR is deployed; therefore, using the global value as reference might cause errors during the VR's deploy, if this situation is not considered.", VpcMaxNetworks, VpcMaxNetworks.value()); + return VpcMaxNetworks.value(); + } + + ClusterVO vpcCluster = clusterDao.findById(domainRouters.get(0).getClusterId()); + int configValue = VpcMaxNetworks.valueIn(vpcCluster.getId()); + logger.debug("Using {} configuration value {} from cluster {}, which is using the {} hypervisor.", VpcMaxNetworks, configValue, vpcCluster.getUuid(), vpcCluster.getHypervisorType()); + + for (DomainRouterJoinVO domainRouter : domainRouters) { + int clusterConfigValue = VpcMaxNetworks.valueIn(domainRouter.getClusterId()); + if (configValue > clusterConfigValue) { + configValue = clusterConfigValue; + vpcCluster = clusterDao.findById(domainRouter.getClusterId()); + logger.warn("Using value {} as max number of networks per VPC as the VRs of VPC {} are deployed in different clusters. This value is referent to the cluster's {} configuration, which has the smallest value and uses the {} hypervisor.", configValue, domainRouter.getVpcUuid(), vpcCluster.getUuid(), vpcCluster.getHypervisorType()); + } + } + + return configValue; + } + private void CheckAccountsAccess(Vpc vpc, Account networkAccount) { Account vpcaccount = _accountMgr.getAccount(vpc.getAccountId()); try { @@ -3179,4 +3222,14 @@ protected boolean isGlobalAcl(Long aclVpcId) { protected boolean isDefaultAcl(long aclId) { return aclId == NetworkACL.DEFAULT_ALLOW || aclId == NetworkACL.DEFAULT_DENY; } + + @Override + public String getConfigComponentName() { + return VpcManager.class.getSimpleName(); + } + + @Override + public ConfigKey[] getConfigKeys() { + return new ConfigKey[] {VpcMaxNetworks}; + } } diff --git a/server/src/test/java/com/cloud/network/vpc/VpcManagerImplTest.java b/server/src/test/java/com/cloud/network/vpc/VpcManagerImplTest.java index deffb165f29f..45989817e7c6 100644 --- a/server/src/test/java/com/cloud/network/vpc/VpcManagerImplTest.java +++ b/server/src/test/java/com/cloud/network/vpc/VpcManagerImplTest.java @@ -23,9 +23,13 @@ import com.cloud.agent.api.to.IpAddressTO; import com.cloud.agent.manager.Commands; import com.cloud.alert.AlertManager; +import com.cloud.api.query.dao.DomainRouterJoinDao; +import com.cloud.api.query.vo.DomainRouterJoinVO; import com.cloud.configuration.Resource; +import com.cloud.dc.ClusterVO; import com.cloud.dc.DataCenterVO; import com.cloud.dc.VlanVO; +import com.cloud.dc.dao.ClusterDao; import com.cloud.dc.dao.DataCenterDao; import com.cloud.dc.dao.VlanDao; import com.cloud.exception.InsufficientCapacityException; @@ -85,6 +89,7 @@ import org.springframework.test.util.ReflectionTestUtils; import java.lang.reflect.Field; +import java.lang.reflect.Modifier; import java.util.ArrayList; import java.util.HashMap; import java.util.HashSet; @@ -155,6 +160,20 @@ public class VpcManagerImplTest { FirewallRulesDao firewallDao; @Mock NetworkACLVO networkACLVOMock; + @Mock + ClusterDao clusterDaoMock; + @Mock + ClusterVO clusterVO1Mock; + @Mock + ClusterVO clusterVO2Mock; + @Mock + DomainRouterJoinDao domainRouterJoinDaoMock; + @Mock + DomainRouterJoinVO domainRouterJoinVO1Mock; + @Mock + DomainRouterJoinVO domainRouterJoinVO2Mock; + @Mock + ConfigKey vpcMaxNetworksMock; public static final long ACCOUNT_ID = 1; private AccountVO account; @@ -212,6 +231,8 @@ public void setup() throws NoSuchFieldException, IllegalAccessException { manager._ntwkSvc = networkServiceMock; manager._firewallDao = firewallDao; manager._networkAclDao = networkACLDaoMock; + manager.clusterDao = clusterDaoMock; + manager.domainRouterJoinDao = domainRouterJoinDaoMock; CallContext.register(Mockito.mock(User.class), Mockito.mock(Account.class)); registerCallContext(); overrideDefaultConfigValue(NetworkService.AllowUsersToSpecifyVRMtu, "_defaultValue", "false"); @@ -510,4 +531,56 @@ public void validateVpcPrivateGatewayTestAclFromDifferentVpcThrowsInvalidParamet Assert.assertThrows(InvalidParameterValueException.class, () -> manager.validateVpcPrivateGatewayAclId(vpcId, differentVpcAclId)); } -} + @Test + public void getVpcMaxNetworksValueEmptyDomainRoutersReturnsGlobalConfigurationDefaultValue() throws NoSuchFieldException, IllegalAccessException { + Mockito.when(vpcMaxNetworksMock.value()).thenReturn(3); + Mockito.when(domainRouterJoinDaoMock.listByVpcId(vpcId)).thenReturn(new ArrayList<>()); + updateFinalStaticField(manager.getClass().getField("VpcMaxNetworks"), vpcMaxNetworksMock); + + int vpcMaxNetworks = manager.getVpcMaxNetworksValue(vpcId); + + Assert.assertEquals(3, vpcMaxNetworks); + } + + @Test + public void getVpcMaxNetworksValueDomainRoutersSingleClusterReturnsClusterConfigurationValue() throws NoSuchFieldException, IllegalAccessException { + Mockito.when(vpcMaxNetworksMock.valueIn(1L)).thenReturn(5); + Mockito.when(clusterDaoMock.findById(anyLong())).thenReturn(clusterVO1Mock); + Mockito.when(clusterVO1Mock.getId()).thenReturn(1L); + Mockito.when(domainRouterJoinVO1Mock.getClusterId()).thenReturn(1L); + Mockito.when(domainRouterJoinVO2Mock.getClusterId()).thenReturn(1L); + Mockito.when(domainRouterJoinDaoMock.listByVpcId(vpcId)).thenReturn(List.of(domainRouterJoinVO1Mock, domainRouterJoinVO2Mock)); + updateFinalStaticField(manager.getClass().getField("VpcMaxNetworks"), vpcMaxNetworksMock); + + int vpcMaxNetworks = manager.getVpcMaxNetworksValue(vpcId); + + Assert.assertEquals(5, vpcMaxNetworks); + } + + @Test + public void getVpcMaxNetworksValueDomainRoutersWithDifferentClustersReturnsClustersSmallestConfigurationValue() throws NoSuchFieldException, IllegalAccessException { + Mockito.when(vpcMaxNetworksMock.valueIn(1L)).thenReturn(5); + Mockito.when(vpcMaxNetworksMock.valueIn(2L)).thenReturn(1); + Mockito.when(clusterDaoMock.findById(1L)).thenReturn(clusterVO1Mock); + Mockito.when(clusterDaoMock.findById(2L)).thenReturn(clusterVO2Mock); + Mockito.when(clusterVO1Mock.getId()).thenReturn(1L); + Mockito.when(domainRouterJoinVO1Mock.getClusterId()).thenReturn(1L); + Mockito.when(domainRouterJoinVO2Mock.getClusterId()).thenReturn(2L); + Mockito.when(domainRouterJoinDaoMock.listByVpcId(vpcId)).thenReturn(List.of(domainRouterJoinVO1Mock, domainRouterJoinVO2Mock)); + updateFinalStaticField(manager.getClass().getField("VpcMaxNetworks"), vpcMaxNetworksMock); + + int vpcMaxNetworks = manager.getVpcMaxNetworksValue(vpcId); + + Assert.assertEquals(1, vpcMaxNetworks); + } + + void updateFinalStaticField(Field field, Object newValue) throws IllegalArgumentException, IllegalAccessException, NoSuchFieldException, SecurityException { + field.setAccessible(true); + + Field modifiersField = Field.class.getDeclaredField("modifiers"); + modifiersField.setAccessible(true); + modifiersField.setInt(field, field.getModifiers() & ~Modifier.FINAL); + + field.set(null, newValue); + } + } From 1b34e0302d8958aae4b2b810ef2077d46ab4630e Mon Sep 17 00:00:00 2001 From: Henrique Sato Date: Fri, 1 Mar 2024 18:05:25 -0300 Subject: [PATCH 02/16] Change vpc.max.networks to account scope & create vm.max.nics settings --- .../service/NetworkOrchestrationService.java | 8 + .../com/cloud/network/vpc/VpcManager.java | 10 +- .../orchestration/NetworkOrchestrator.java | 90 +++++++- .../NetworkOrchestratorTest.java | 197 ++++++++++++++++++ .../api/query/dao/DomainRouterJoinDao.java | 2 - .../query/dao/DomainRouterJoinDaoImpl.java | 14 -- .../com/cloud/network/vpc/VpcManagerImpl.java | 37 +--- .../cloud/network/vpc/VpcManagerImplTest.java | 73 ------- 8 files changed, 298 insertions(+), 133 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 2005b70b4394..059ce11b4bb5 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 @@ -104,6 +104,14 @@ public interface NetworkOrchestrationService { static final ConfigKey TUNGSTEN_ENABLED = new ConfigKey<>(Boolean.class, "tungsten.plugin.enable", "Advanced", "false", "Indicates whether to enable the Tungsten plugin", false, ConfigKey.Scope.Zone, null); + ConfigKey VirtualMachineMaxNicsKvm = new ConfigKey<>("Advanced", Integer.class, "virtual.machine.max.nics.kvm", "23", + "The maximum number of NICs supported by the KVM hypervsior.", true, Scope.Cluster); + + ConfigKey VirtualMachineMaxNicsVmware = new ConfigKey<>("Advanced", Integer.class, "virtual.machine.max.nics.vmware", "10", + "The maximum number of NICs supported by the VMware hypervsior.", true, Scope.Cluster); + + ConfigKey VirtualMachineMaxNicsXenserver = new ConfigKey<>("Advanced", Integer.class, "virtual.machine.max.nics.xenserver", "7", + "The maximum number of NICs supported by the XenServer hypervsior.", true, Scope.Cluster); List setupNetwork(Account owner, NetworkOffering offering, DeploymentPlan plan, String name, String displayText, boolean isDefault) throws ConcurrentOperationException; diff --git a/engine/components-api/src/main/java/com/cloud/network/vpc/VpcManager.java b/engine/components-api/src/main/java/com/cloud/network/vpc/VpcManager.java index 11c0247f16a0..edc06dff9ec2 100644 --- a/engine/components-api/src/main/java/com/cloud/network/vpc/VpcManager.java +++ b/engine/components-api/src/main/java/com/cloud/network/vpc/VpcManager.java @@ -40,14 +40,8 @@ import com.cloud.user.Account; public interface VpcManager { - ConfigKey VpcMaxNetworks = new ConfigKey<>( - "Advanced", - Integer.class, - "vpc.max.networks", - "3", - "Maximum number of networks per VPC. Bear in mind that this value will depend on the hypervisor where the VR was/will be deployed.", - true, - ConfigKey.Scope.Cluster); + ConfigKey VpcMaxNetworks = new ConfigKey<>("Advanced", Integer.class, "vpc.max.networks", "3", + "Maximum number of networks per VPC. Bear in mind that this value will depend on the hypervisor where the VR was/will be deployed.", true, ConfigKey.Scope.Account); /** * Returns all the Guest networks that are part of VPC 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 6d858a5dd12b..254964e2e484 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 @@ -253,6 +253,7 @@ import com.cloud.vm.dao.UserVmDao; import com.cloud.vm.dao.VMInstanceDao; import com.googlecode.ipv6.IPv6Address; +import org.apache.commons.lang3.math.NumberUtils; /** * NetworkManagerImpl implements NetworkManager. @@ -1013,6 +1014,13 @@ public void saveExtraDhcpOptions(final String networkUuid, final Long nicId, fin public Pair allocateNic(final NicProfile requested, final Network network, final Boolean isDefaultNic, int deviceId, final VirtualMachineProfile vm) throws InsufficientVirtualNetworkCapacityException, InsufficientAddressCapacityException, ConcurrentOperationException { + Integer virtualMachineMaxNicsValue = getVirtualMachineMaxNicsValue(vm); + List nics = _nicDao.listByVmId(vm.getId()); + + if (nics.size() > virtualMachineMaxNicsValue) { + throw new CloudRuntimeException(String.format("Failed to allocate NIC on network [%s] because VM [%s] has reached its maximum NIC capacity [%s].", network.getUuid(), vm.getUuid(), virtualMachineMaxNicsValue)); + } + final NetworkVO ntwkVO = _networksDao.findById(network.getId()); logger.debug("Allocating nic for vm " + vm.getVirtualMachine() + " in network " + network + " with requested profile " + requested); final NetworkGuru guru = AdapterBase.getAdapterByName(networkGurus, ntwkVO.getGuruName()); @@ -1057,6 +1065,86 @@ public Pair allocateNic(final NicProfile requested, final N return new Pair(vmNic, Integer.valueOf(deviceId)); } + /** + * Returns the maximum number of NICs that the given virtual machine can have considering its hypervisor. + *

+ * First we try to retrieve the setting value from the cluster where the virtual machine is deployed. If the cluster does not exist, we try to retrieve the setting value from the virtual machine hypervisor type. + * In last case, if the virtual machine does not have a hypervisor type, we retrieve the smallest value between the {@link NetworkOrchestrationService#VirtualMachineMaxNicsKvm}, {@link NetworkOrchestrationService#VirtualMachineMaxNicsVmware} + * and {@link NetworkOrchestrationService#VirtualMachineMaxNicsXenserver} global settings. + * + * @param virtualMachine Virtual machine to get the maximum number of NICs. + * @return The maximum number of NICs that the virtual machine can have. + */ + protected Integer getVirtualMachineMaxNicsValue(VirtualMachineProfile virtualMachine) { + Integer virtualMachineMaxNicsValue = getVirtualMachineMaxNicsValueFromCluster(virtualMachine); + + if (virtualMachineMaxNicsValue != null) { + return virtualMachineMaxNicsValue; + } + + if (virtualMachine.getVirtualMachine().getHypervisorType() == null) { + logger.debug("Using the smallest setting global value between {}, {} and {} as the VM {} does not have a hypervisor type and is not deployed on either a host or a cluster.", VirtualMachineMaxNicsKvm, VirtualMachineMaxNicsVmware, VirtualMachineMaxNicsXenserver, virtualMachine.getUuid()); + return NumberUtils.min(VirtualMachineMaxNicsKvm.value(), VirtualMachineMaxNicsVmware.value(), VirtualMachineMaxNicsXenserver.value()); + } + + return getVirtualMachineMaxNicsValueFromVmHypervisorType(virtualMachine); + } + + /** + * Searches the maximum virtual machine NICs based on the hypervisor type of the cluster where the instance is deployed. + * + * @param virtualMachine Virtual machine to get the cluster. + * @return The maximum number of NICs that the virtual machine can have. + */ + protected Integer getVirtualMachineMaxNicsValueFromCluster(VirtualMachineProfile virtualMachine) { + HostVO host = _hostDao.findById(virtualMachine.getHostId()); + if (host == null) { + return null; + } + + ClusterVO cluster = clusterDao.findById(host.getClusterId()); + if (cluster == null) { + return null; + } + + int virtualMachineMaxNicsValue; + HypervisorType hypervisor = cluster.getHypervisorType(); + + if (HypervisorType.KVM.equals(hypervisor)) { + virtualMachineMaxNicsValue = VirtualMachineMaxNicsKvm.valueIn(cluster.getId()); + logger.debug("The cluster {} where the VM is deployed uses the {} hypervisor. Therefore, the {} setting value [{}] will be used.", cluster.getName(), hypervisor, VirtualMachineMaxNicsKvm, virtualMachineMaxNicsValue); + } else if (HypervisorType.VMware.equals(hypervisor)) { + virtualMachineMaxNicsValue = VirtualMachineMaxNicsVmware.valueIn(cluster.getId()); + logger.debug("The cluster {} where the VM is deployed uses the {} hypervisor. Therefore, the {} setting value [{}] will be used.", cluster.getName(), hypervisor, VirtualMachineMaxNicsVmware, virtualMachineMaxNicsValue); + } else { + virtualMachineMaxNicsValue = VirtualMachineMaxNicsXenserver.valueIn(cluster.getId()); + logger.debug("The cluster {} where the VM is deployed uses the {} hypervisor. Therefore, the {} setting value [{}] will be used.", cluster.getName(), hypervisor, VirtualMachineMaxNicsXenserver, virtualMachineMaxNicsValue); + } + + return virtualMachineMaxNicsValue; + } + + /** + * Searches the maximum virtual machine NICs based on the virtual machine hypervisor type. + * + * @param virtualMachine Virtual machine to get the hypervisor type. + * @return The maximum number of NICs that the virtual machine can have. + */ + protected Integer getVirtualMachineMaxNicsValueFromVmHypervisorType(VirtualMachineProfile virtualMachine) { + HypervisorType virtualMachineHypervisorType = virtualMachine.getVirtualMachine().getHypervisorType(); + + if (HypervisorType.KVM.equals(virtualMachineHypervisorType)) { + logger.debug("Using the {} setting global value {} as the VM {} has the {} hypervisor type and is not deployed on either a host or a cluster.", VirtualMachineMaxNicsKvm, VirtualMachineMaxNicsKvm.value(), virtualMachine.getUuid(), virtualMachineHypervisorType); + return VirtualMachineMaxNicsKvm.value(); + } else if (HypervisorType.VMware.equals(virtualMachineHypervisorType)) { + logger.debug("Using the {} setting global value {} as the VM {} has the {} hypervisor type and is not deployed on either a host or a cluster.", VirtualMachineMaxNicsVmware, VirtualMachineMaxNicsVmware.value(), virtualMachine.getUuid(), virtualMachineHypervisorType); + return VirtualMachineMaxNicsVmware.value(); + } + + logger.debug("Using the {} setting global value {} as the VM {} has the {} hypervisor type and is not deployed on either a host or a cluster.", VirtualMachineMaxNicsXenserver, VirtualMachineMaxNicsXenserver.value(), virtualMachine.getUuid(), virtualMachineHypervisorType); + return VirtualMachineMaxNicsXenserver.value(); + } + private void setMtuDetailsInVRNic(final Pair networks, Network network, NicVO vo) { if (TrafficType.Public == network.getTrafficType()) { if (networks == null) { @@ -4721,6 +4809,6 @@ public ConfigKey[] getConfigKeys() { return new ConfigKey[]{NetworkGcWait, NetworkGcInterval, NetworkLockTimeout, GuestDomainSuffix, NetworkThrottlingRate, MinVRVersion, PromiscuousMode, MacAddressChanges, ForgedTransmits, MacLearning, RollingRestartEnabled, - TUNGSTEN_ENABLED }; + TUNGSTEN_ENABLED, VirtualMachineMaxNicsKvm, VirtualMachineMaxNicsVmware, VirtualMachineMaxNicsXenserver }; } } 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 d1532cdbef14..374c92152210 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 @@ -22,6 +22,8 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import java.lang.reflect.Field; +import java.lang.reflect.Modifier; import java.net.URI; import java.util.ArrayList; import java.util.Arrays; @@ -29,9 +31,14 @@ import java.util.List; import java.util.Map; +import com.cloud.dc.ClusterVO; import com.cloud.dc.DataCenter; +import com.cloud.dc.dao.ClusterDao; +import com.cloud.host.HostVO; +import com.cloud.host.dao.HostDao; import com.cloud.network.IpAddressManager; import com.cloud.utils.Pair; +import org.apache.cloudstack.framework.config.ConfigKey; import org.junit.Assert; import org.junit.Before; import org.junit.Test; @@ -127,6 +134,8 @@ public void setUp() { testOrchastrator._vpcMgr = mock(VpcManager.class); testOrchastrator.routerJoinDao = mock(DomainRouterJoinDao.class); testOrchastrator._ipAddrMgr = mock(IpAddressManager.class); + testOrchastrator._hostDao = mock(HostDao.class); + testOrchastrator.clusterDao = mock(ClusterDao.class); DhcpServiceProvider provider = mock(DhcpServiceProvider.class); Map capabilities = new HashMap(); @@ -840,4 +849,192 @@ public void testGetGuestIpForNicImportBasicUsedIP() { Mockito.when(testOrchastrator._ipAddressDao.findByIp(requestedIp)).thenReturn(ipAddressVO); testOrchastrator.getSelectedIpForNicImport(network, dataCenter, ipAddresses); } + + @Test + public void getVirtualMachineMaxNicsValueTestVirtualMachineDeployedReturnsVirtualMachineClusterMaxNics() { + VirtualMachineProfile virtualMachineProfileMock = Mockito.mock(VirtualMachineProfile.class); + Mockito.doReturn(44).when(testOrchastrator).getVirtualMachineMaxNicsValueFromCluster(virtualMachineProfileMock); + + Integer virtualMachineMaxNicsValue = testOrchastrator.getVirtualMachineMaxNicsValue(virtualMachineProfileMock); + + Mockito.verify(testOrchastrator, Mockito.times(1)).getVirtualMachineMaxNicsValueFromCluster(Mockito.any()); + Assert.assertEquals((Integer) 44, virtualMachineMaxNicsValue); + } + + @Test + public void getVirtualMachineMaxNicsValueTestVirtualMachineWithoutDeployAndTemplateReturnsSmallestGlobalValueBetweenVirtualMachineMaxNicsSettings() throws NoSuchFieldException, IllegalAccessException { + VirtualMachineProfile virtualMachineProfileMock = Mockito.mock(VirtualMachineProfile.class); + VirtualMachine virtualMachineMock = Mockito.mock(VirtualMachine.class); + ConfigKey virtualMachineMaxNicsKvmMock = Mockito.mock(ConfigKey.class); + ConfigKey virtualMachineMaxNicsVmwareMock = Mockito.mock(ConfigKey.class); + ConfigKey virtualMachineMaxNicsXenserverMock = Mockito.mock(ConfigKey.class); + Mockito.doReturn(virtualMachineMock).when(virtualMachineProfileMock).getVirtualMachine(); + Mockito.doReturn(null).when(virtualMachineMock).getHypervisorType(); + Mockito.doReturn(null).when(testOrchastrator).getVirtualMachineMaxNicsValueFromCluster(virtualMachineProfileMock); + Mockito.doReturn(23).when(virtualMachineMaxNicsKvmMock).value(); + Mockito.doReturn(10).when(virtualMachineMaxNicsVmwareMock).value(); + Mockito.doReturn(7).when(virtualMachineMaxNicsXenserverMock).value(); + updateFinalStaticField(testOrchastrator.getClass().getField("VirtualMachineMaxNicsKvm"), virtualMachineMaxNicsKvmMock); + updateFinalStaticField(testOrchastrator.getClass().getField("VirtualMachineMaxNicsVmware"), virtualMachineMaxNicsVmwareMock); + updateFinalStaticField(testOrchastrator.getClass().getField("VirtualMachineMaxNicsXenserver"), virtualMachineMaxNicsXenserverMock); + + Integer virtualMachineMaxNicsValue = testOrchastrator.getVirtualMachineMaxNicsValue(virtualMachineProfileMock); + + Assert.assertEquals((Integer) 7, virtualMachineMaxNicsValue); + } + + @Test + public void getVirtualMachineMaxNicsValueTestVirtualMachineWithoutDeployReturnsVirtualMachineHypervisorTypeMaxNics() { + VirtualMachineProfile virtualMachineProfileMock = Mockito.mock(VirtualMachineProfile.class); + VirtualMachine virtualMachineMock = Mockito.mock(VirtualMachine.class); + Mockito.doReturn(virtualMachineMock).when(virtualMachineProfileMock).getVirtualMachine(); + Mockito.doReturn(Hypervisor.HypervisorType.KVM).when(virtualMachineMock).getHypervisorType(); + Mockito.doReturn(null).when(testOrchastrator).getVirtualMachineMaxNicsValueFromCluster(virtualMachineProfileMock); + Mockito.doReturn(33).when(testOrchastrator).getVirtualMachineMaxNicsValueFromVmHypervisorType(virtualMachineProfileMock); + + Integer virtualMachineMaxNicsValue = testOrchastrator.getVirtualMachineMaxNicsValue(virtualMachineProfileMock); + + Mockito.verify(testOrchastrator, Mockito.times(1)).getVirtualMachineMaxNicsValueFromVmHypervisorType(Mockito.any()); + Assert.assertEquals((Integer) 33, virtualMachineMaxNicsValue); + } + + @Test + public void getVirtualMachineMaxNicsValueFromClusterTestHostDoesNotExistReturnsNull() { + VirtualMachineProfile virtualMachineProfileMock = Mockito.mock(VirtualMachineProfile.class); + Mockito.doReturn(100L).when(virtualMachineProfileMock).getHostId(); + Mockito.doReturn(null).when(testOrchastrator._hostDao).findById(100L); + + Integer virtualMachineMaxNicsValue = testOrchastrator.getVirtualMachineMaxNicsValueFromCluster(virtualMachineProfileMock); + + Assert.assertNull(virtualMachineMaxNicsValue); + } + + @Test + public void getVirtualMachineMaxNicsValueFromClusterTestClusterDoesNotExistReturnsNull() { + VirtualMachineProfile virtualMachineProfileMock = Mockito.mock(VirtualMachineProfile.class); + HostVO hostVoMock = Mockito.mock(HostVO.class); + Mockito.doReturn(1L).when(virtualMachineProfileMock).getHostId(); + Mockito.doReturn(hostVoMock).when(testOrchastrator._hostDao).findById(1L); + Mockito.doReturn(100L).when(hostVoMock).getClusterId(); + Mockito.doReturn(null).when(testOrchastrator.clusterDao).findById(100L); + + Integer virtualMachineMaxNicsValue = testOrchastrator.getVirtualMachineMaxNicsValueFromCluster(virtualMachineProfileMock); + + Assert.assertNull(virtualMachineMaxNicsValue); + } + + @Test + public void getVirtualMachineMaxNicsValueFromClusterTestKvmClusterReturnsVirtualMachineMaxNicsKvmClusterValue() throws NoSuchFieldException, IllegalAccessException { + VirtualMachineProfile virtualMachineProfileMock = Mockito.mock(VirtualMachineProfile.class); + HostVO hostVoMock = Mockito.mock(HostVO.class); + ClusterVO clusterVoMock = Mockito.mock(ClusterVO.class); + ConfigKey virtualMachineMaxNicsKvmMock = Mockito.mock(ConfigKey.class); + Mockito.doReturn(1L).when(virtualMachineProfileMock).getHostId(); + Mockito.doReturn(hostVoMock).when(testOrchastrator._hostDao).findById(1L); + Mockito.doReturn(1L).when(hostVoMock).getClusterId(); + Mockito.doReturn(clusterVoMock).when(testOrchastrator.clusterDao).findById(1L); + Mockito.doReturn(1L).when(clusterVoMock).getId(); + Mockito.doReturn(Hypervisor.HypervisorType.KVM).when(clusterVoMock).getHypervisorType(); + Mockito.doReturn(33).when(virtualMachineMaxNicsKvmMock).valueIn(1L); + updateFinalStaticField(testOrchastrator.getClass().getField("VirtualMachineMaxNicsKvm"), virtualMachineMaxNicsKvmMock); + + Integer virtualMachineMaxNicsValue = testOrchastrator.getVirtualMachineMaxNicsValueFromCluster(virtualMachineProfileMock); + + Assert.assertEquals((Integer) 33, virtualMachineMaxNicsValue); + } + + @Test + public void getVirtualMachineMaxNicsValueFromClusterTestVmwareClusterReturnsVirtualMachineMaxNicsVmwareClusterValue() throws NoSuchFieldException, IllegalAccessException { + VirtualMachineProfile virtualMachineProfileMock = Mockito.mock(VirtualMachineProfile.class); + HostVO hostVoMock = Mockito.mock(HostVO.class); + ClusterVO clusterVoMock = Mockito.mock(ClusterVO.class); + ConfigKey virtualMachineMaxNicsVmwareMock = Mockito.mock(ConfigKey.class); + Mockito.doReturn(1L).when(virtualMachineProfileMock).getHostId(); + Mockito.doReturn(hostVoMock).when(testOrchastrator._hostDao).findById(1L); + Mockito.doReturn(1L).when(hostVoMock).getClusterId(); + Mockito.doReturn(clusterVoMock).when(testOrchastrator.clusterDao).findById(1L); + Mockito.doReturn(1L).when(clusterVoMock).getId(); + Mockito.doReturn(Hypervisor.HypervisorType.VMware).when(clusterVoMock).getHypervisorType(); + Mockito.doReturn(22).when(virtualMachineMaxNicsVmwareMock).valueIn(1L); + updateFinalStaticField(testOrchastrator.getClass().getField("VirtualMachineMaxNicsVmware"), virtualMachineMaxNicsVmwareMock); + + Integer virtualMachineMaxNicsValue = testOrchastrator.getVirtualMachineMaxNicsValueFromCluster(virtualMachineProfileMock); + + Assert.assertEquals((Integer) 22, virtualMachineMaxNicsValue); + } + + @Test + public void getVirtualMachineMaxNicsValueFromClusterTestXenserverClusterReturnsVirtualMachineMaxNicsXenserverClusterValue() throws NoSuchFieldException, IllegalAccessException { + VirtualMachineProfile virtualMachineProfileMock = Mockito.mock(VirtualMachineProfile.class); + HostVO hostVoMock = Mockito.mock(HostVO.class); + ClusterVO clusterVoMock = Mockito.mock(ClusterVO.class); + ConfigKey virtualMachineMaxNicsXenserverMock = Mockito.mock(ConfigKey.class); + Mockito.doReturn(1L).when(virtualMachineProfileMock).getHostId(); + Mockito.doReturn(hostVoMock).when(testOrchastrator._hostDao).findById(1L); + Mockito.doReturn(1L).when(hostVoMock).getClusterId(); + Mockito.doReturn(clusterVoMock).when(testOrchastrator.clusterDao).findById(1L); + Mockito.doReturn(1L).when(clusterVoMock).getId(); + Mockito.doReturn(Hypervisor.HypervisorType.XenServer).when(clusterVoMock).getHypervisorType(); + Mockito.doReturn(11).when(virtualMachineMaxNicsXenserverMock).valueIn(1L); + updateFinalStaticField(testOrchastrator.getClass().getField("VirtualMachineMaxNicsXenserver"), virtualMachineMaxNicsXenserverMock); + + Integer virtualMachineMaxNicsValue = testOrchastrator.getVirtualMachineMaxNicsValueFromCluster(virtualMachineProfileMock); + + Assert.assertEquals((Integer) 11, virtualMachineMaxNicsValue); + } + + @Test + public void getVirtualMachineMaxNicsValueFromVmHypervisorTypeTestKvmHypervisorReturnsVirtualMachineMaxNicsKvmGlobalValue() throws NoSuchFieldException, IllegalAccessException { + VirtualMachineProfile virtualMachineProfileMock = Mockito.mock(VirtualMachineProfile.class); + VirtualMachine virtualMachineMock = Mockito.mock(VirtualMachine.class); + ConfigKey virtualMachineMaxNicsKvmMock = Mockito.mock(ConfigKey.class); + Mockito.doReturn(virtualMachineMock).when(virtualMachineProfileMock).getVirtualMachine(); + Mockito.doReturn(Hypervisor.HypervisorType.KVM).when(virtualMachineMock).getHypervisorType(); + Mockito.doReturn(23).when(virtualMachineMaxNicsKvmMock).value(); + updateFinalStaticField(testOrchastrator.getClass().getField("VirtualMachineMaxNicsKvm"), virtualMachineMaxNicsKvmMock); + + Integer virtualMachineMaxNicsValue = testOrchastrator.getVirtualMachineMaxNicsValueFromVmHypervisorType(virtualMachineProfileMock); + + Assert.assertEquals((Integer) 23, virtualMachineMaxNicsValue); + } + + @Test + public void getVirtualMachineMaxNicsValueFromVmHypervisorTypeTestVmwareHypervisorReturnsVirtualMachineMaxNicsVmwareGlobalValue() throws NoSuchFieldException, IllegalAccessException { + VirtualMachineProfile virtualMachineProfileMock = Mockito.mock(VirtualMachineProfile.class); + VirtualMachine virtualMachineMock = Mockito.mock(VirtualMachine.class); + ConfigKey virtualMachineMaxNicsVmwareMock = Mockito.mock(ConfigKey.class); + Mockito.doReturn(virtualMachineMock).when(virtualMachineProfileMock).getVirtualMachine(); + Mockito.doReturn(Hypervisor.HypervisorType.VMware).when(virtualMachineMock).getHypervisorType(); + Mockito.doReturn(10).when(virtualMachineMaxNicsVmwareMock).value(); + updateFinalStaticField(testOrchastrator.getClass().getField("VirtualMachineMaxNicsVmware"), virtualMachineMaxNicsVmwareMock); + + Integer virtualMachineMaxNicsValue = testOrchastrator.getVirtualMachineMaxNicsValueFromVmHypervisorType(virtualMachineProfileMock); + + Assert.assertEquals((Integer) 10, virtualMachineMaxNicsValue); + } + + @Test + public void getVirtualMachineMaxNicsValueFromVmHypervisorTypeTestXenserverHypervisorReturnsVirtualMachineMaxNicsXenserverGlobalValue() throws NoSuchFieldException, IllegalAccessException { + VirtualMachineProfile virtualMachineProfileMock = Mockito.mock(VirtualMachineProfile.class); + VirtualMachine virtualMachineMock = Mockito.mock(VirtualMachine.class); + ConfigKey virtualMachineMaxNicsXenserverMock = Mockito.mock(ConfigKey.class); + Mockito.doReturn(virtualMachineMock).when(virtualMachineProfileMock).getVirtualMachine(); + Mockito.doReturn(Hypervisor.HypervisorType.XenServer).when(virtualMachineMock).getHypervisorType(); + Mockito.doReturn(7).when(virtualMachineMaxNicsXenserverMock).value(); + updateFinalStaticField(testOrchastrator.getClass().getField("VirtualMachineMaxNicsXenserver"), virtualMachineMaxNicsXenserverMock); + + Integer virtualMachineMaxNicsValue = testOrchastrator.getVirtualMachineMaxNicsValueFromVmHypervisorType(virtualMachineProfileMock); + + Assert.assertEquals((Integer) 7, virtualMachineMaxNicsValue); + } + + void updateFinalStaticField(Field field, Object newValue) throws IllegalArgumentException, IllegalAccessException, NoSuchFieldException, SecurityException { + field.setAccessible(true); + + Field modifiersField = Field.class.getDeclaredField("modifiers"); + modifiersField.setAccessible(true); + modifiersField.setInt(field, field.getModifiers() & ~Modifier.FINAL); + + field.set(null, newValue); + } } diff --git a/server/src/main/java/com/cloud/api/query/dao/DomainRouterJoinDao.java b/server/src/main/java/com/cloud/api/query/dao/DomainRouterJoinDao.java index 4dbe4be882bc..509d02dd71c5 100644 --- a/server/src/main/java/com/cloud/api/query/dao/DomainRouterJoinDao.java +++ b/server/src/main/java/com/cloud/api/query/dao/DomainRouterJoinDao.java @@ -37,6 +37,4 @@ public interface DomainRouterJoinDao extends GenericDao searchByIds(Long... ids); List getRouterByIdAndTrafficType(Long id, Networks.TrafficType... trafficType); - - List listByVpcId(long vpcId); } diff --git a/server/src/main/java/com/cloud/api/query/dao/DomainRouterJoinDaoImpl.java b/server/src/main/java/com/cloud/api/query/dao/DomainRouterJoinDaoImpl.java index 7dfa4c3968fd..c6041c3e3732 100644 --- a/server/src/main/java/com/cloud/api/query/dao/DomainRouterJoinDaoImpl.java +++ b/server/src/main/java/com/cloud/api/query/dao/DomainRouterJoinDaoImpl.java @@ -62,7 +62,6 @@ public class DomainRouterJoinDaoImpl extends GenericDaoBase vrIdSearch; private final SearchBuilder vrIdTrafficSearch; - private final SearchBuilder vrVpcIdSearch; protected DomainRouterJoinDaoImpl() { @@ -79,12 +78,6 @@ protected DomainRouterJoinDaoImpl() { vrIdTrafficSearch.and("trafficType", vrIdTrafficSearch.entity().getTrafficType(), SearchCriteria.Op.IN); vrIdTrafficSearch.done(); - vrVpcIdSearch = createSearchBuilder(); - vrVpcIdSearch.and("vpcId", vrVpcIdSearch.entity().getVpcId(), SearchCriteria.Op.EQ); - vrVpcIdSearch.and("clusterId", vrVpcIdSearch.entity().getClusterId(), SearchCriteria.Op.NNULL); - vrVpcIdSearch.groupBy(vrVpcIdSearch.entity().getClusterId()); - vrVpcIdSearch.done(); - _count = "select count(distinct id) from domain_router_view WHERE "; } @@ -364,11 +357,4 @@ public List newDomainRouterView(VirtualRouter vr) { return searchIncludingRemoved(sc, null, null, false); } - @Override - public List listByVpcId(long vpcId) { - SearchCriteria sc = vrVpcIdSearch.create(); - sc.setParameters("vpcId", vpcId); - - return listBy(sc); - } } diff --git a/server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java b/server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java index f6a20bab7f4f..e9840ebddae1 100644 --- a/server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java +++ b/server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java @@ -37,11 +37,6 @@ import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.TimeUnit; -import com.cloud.api.query.dao.DomainRouterJoinDao; -import com.cloud.api.query.vo.DomainRouterJoinVO; -import com.cloud.dc.ClusterVO; -import com.cloud.dc.dao.ClusterDao; - import javax.annotation.PostConstruct; import javax.inject.Inject; import javax.naming.ConfigurationException; @@ -263,10 +258,6 @@ public class VpcManagerImpl extends ManagerBase implements VpcManager, VpcProvis @Inject NicDao nicDao; @Inject - DomainRouterJoinDao domainRouterJoinDao; - @Inject - ClusterDao clusterDao; - @Inject AlertManager alertManager; @Inject CommandSetupHelper commandSetupHelper; @@ -1840,12 +1831,12 @@ public void doInTransactionWithoutResult(final TransactionStatus status) { throw new CloudRuntimeException("Unable to acquire lock on " + vpc); } - _maxNetworks = getVpcMaxNetworksValue(vpc.getId()); + _maxNetworks = VpcMaxNetworks.valueIn(vpc.getAccountId()); try { // check number of active networks in vpc if (_ntwkDao.countVpcNetworks(vpc.getId()) >= _maxNetworks) { - logger.warn("Failed to create a new VPC Guest Network because the number of networks per VPC has reached its maximum capacity of {}. Increase it by modifying global or cluster config {}. Bear in mind that the maximum number of networks per VPC depends on the hypervisor where the VR is deployed; therefore, using the global value as reference might cause errors during the VR's deploy, if this situation is not considered.", _maxNetworks, VpcMaxNetworks); + logger.warn("Failed to create a new VPC Guest Network because the number of networks per VPC has reached its maximum capacity of {}. Increase it by modifying global or account config {}.", _maxNetworks, VpcMaxNetworks); throw new CloudRuntimeException(String.format("Number of networks per VPC cannot surpass [%s].", _maxNetworks)); } @@ -1897,30 +1888,6 @@ public void doInTransactionWithoutResult(final TransactionStatus status) { * @param vpcId ID from VPC. * @return The {@link #VpcMaxNetworks} configuration value. */ - protected int getVpcMaxNetworksValue(long vpcId) { - List domainRouters = domainRouterJoinDao.listByVpcId(vpcId); - - if (domainRouters.isEmpty()) { - logger.warn("Using {} global configuration value {} as VPC does not have any VRs deployed within a cluster. Bear in mind that the maximum number of networks per VPC depends on the hypervisor where the VR is deployed; therefore, using the global value as reference might cause errors during the VR's deploy, if this situation is not considered.", VpcMaxNetworks, VpcMaxNetworks.value()); - return VpcMaxNetworks.value(); - } - - ClusterVO vpcCluster = clusterDao.findById(domainRouters.get(0).getClusterId()); - int configValue = VpcMaxNetworks.valueIn(vpcCluster.getId()); - logger.debug("Using {} configuration value {} from cluster {}, which is using the {} hypervisor.", VpcMaxNetworks, configValue, vpcCluster.getUuid(), vpcCluster.getHypervisorType()); - - for (DomainRouterJoinVO domainRouter : domainRouters) { - int clusterConfigValue = VpcMaxNetworks.valueIn(domainRouter.getClusterId()); - if (configValue > clusterConfigValue) { - configValue = clusterConfigValue; - vpcCluster = clusterDao.findById(domainRouter.getClusterId()); - logger.warn("Using value {} as max number of networks per VPC as the VRs of VPC {} are deployed in different clusters. This value is referent to the cluster's {} configuration, which has the smallest value and uses the {} hypervisor.", configValue, domainRouter.getVpcUuid(), vpcCluster.getUuid(), vpcCluster.getHypervisorType()); - } - } - - return configValue; - } - private void CheckAccountsAccess(Vpc vpc, Account networkAccount) { Account vpcaccount = _accountMgr.getAccount(vpc.getAccountId()); try { diff --git a/server/src/test/java/com/cloud/network/vpc/VpcManagerImplTest.java b/server/src/test/java/com/cloud/network/vpc/VpcManagerImplTest.java index 45989817e7c6..ac7ab51fc9ab 100644 --- a/server/src/test/java/com/cloud/network/vpc/VpcManagerImplTest.java +++ b/server/src/test/java/com/cloud/network/vpc/VpcManagerImplTest.java @@ -23,13 +23,9 @@ import com.cloud.agent.api.to.IpAddressTO; import com.cloud.agent.manager.Commands; import com.cloud.alert.AlertManager; -import com.cloud.api.query.dao.DomainRouterJoinDao; -import com.cloud.api.query.vo.DomainRouterJoinVO; import com.cloud.configuration.Resource; -import com.cloud.dc.ClusterVO; import com.cloud.dc.DataCenterVO; import com.cloud.dc.VlanVO; -import com.cloud.dc.dao.ClusterDao; import com.cloud.dc.dao.DataCenterDao; import com.cloud.dc.dao.VlanDao; import com.cloud.exception.InsufficientCapacityException; @@ -89,7 +85,6 @@ import org.springframework.test.util.ReflectionTestUtils; import java.lang.reflect.Field; -import java.lang.reflect.Modifier; import java.util.ArrayList; import java.util.HashMap; import java.util.HashSet; @@ -160,20 +155,6 @@ public class VpcManagerImplTest { FirewallRulesDao firewallDao; @Mock NetworkACLVO networkACLVOMock; - @Mock - ClusterDao clusterDaoMock; - @Mock - ClusterVO clusterVO1Mock; - @Mock - ClusterVO clusterVO2Mock; - @Mock - DomainRouterJoinDao domainRouterJoinDaoMock; - @Mock - DomainRouterJoinVO domainRouterJoinVO1Mock; - @Mock - DomainRouterJoinVO domainRouterJoinVO2Mock; - @Mock - ConfigKey vpcMaxNetworksMock; public static final long ACCOUNT_ID = 1; private AccountVO account; @@ -231,8 +212,6 @@ public void setup() throws NoSuchFieldException, IllegalAccessException { manager._ntwkSvc = networkServiceMock; manager._firewallDao = firewallDao; manager._networkAclDao = networkACLDaoMock; - manager.clusterDao = clusterDaoMock; - manager.domainRouterJoinDao = domainRouterJoinDaoMock; CallContext.register(Mockito.mock(User.class), Mockito.mock(Account.class)); registerCallContext(); overrideDefaultConfigValue(NetworkService.AllowUsersToSpecifyVRMtu, "_defaultValue", "false"); @@ -531,56 +510,4 @@ public void validateVpcPrivateGatewayTestAclFromDifferentVpcThrowsInvalidParamet Assert.assertThrows(InvalidParameterValueException.class, () -> manager.validateVpcPrivateGatewayAclId(vpcId, differentVpcAclId)); } - @Test - public void getVpcMaxNetworksValueEmptyDomainRoutersReturnsGlobalConfigurationDefaultValue() throws NoSuchFieldException, IllegalAccessException { - Mockito.when(vpcMaxNetworksMock.value()).thenReturn(3); - Mockito.when(domainRouterJoinDaoMock.listByVpcId(vpcId)).thenReturn(new ArrayList<>()); - updateFinalStaticField(manager.getClass().getField("VpcMaxNetworks"), vpcMaxNetworksMock); - - int vpcMaxNetworks = manager.getVpcMaxNetworksValue(vpcId); - - Assert.assertEquals(3, vpcMaxNetworks); - } - - @Test - public void getVpcMaxNetworksValueDomainRoutersSingleClusterReturnsClusterConfigurationValue() throws NoSuchFieldException, IllegalAccessException { - Mockito.when(vpcMaxNetworksMock.valueIn(1L)).thenReturn(5); - Mockito.when(clusterDaoMock.findById(anyLong())).thenReturn(clusterVO1Mock); - Mockito.when(clusterVO1Mock.getId()).thenReturn(1L); - Mockito.when(domainRouterJoinVO1Mock.getClusterId()).thenReturn(1L); - Mockito.when(domainRouterJoinVO2Mock.getClusterId()).thenReturn(1L); - Mockito.when(domainRouterJoinDaoMock.listByVpcId(vpcId)).thenReturn(List.of(domainRouterJoinVO1Mock, domainRouterJoinVO2Mock)); - updateFinalStaticField(manager.getClass().getField("VpcMaxNetworks"), vpcMaxNetworksMock); - - int vpcMaxNetworks = manager.getVpcMaxNetworksValue(vpcId); - - Assert.assertEquals(5, vpcMaxNetworks); - } - - @Test - public void getVpcMaxNetworksValueDomainRoutersWithDifferentClustersReturnsClustersSmallestConfigurationValue() throws NoSuchFieldException, IllegalAccessException { - Mockito.when(vpcMaxNetworksMock.valueIn(1L)).thenReturn(5); - Mockito.when(vpcMaxNetworksMock.valueIn(2L)).thenReturn(1); - Mockito.when(clusterDaoMock.findById(1L)).thenReturn(clusterVO1Mock); - Mockito.when(clusterDaoMock.findById(2L)).thenReturn(clusterVO2Mock); - Mockito.when(clusterVO1Mock.getId()).thenReturn(1L); - Mockito.when(domainRouterJoinVO1Mock.getClusterId()).thenReturn(1L); - Mockito.when(domainRouterJoinVO2Mock.getClusterId()).thenReturn(2L); - Mockito.when(domainRouterJoinDaoMock.listByVpcId(vpcId)).thenReturn(List.of(domainRouterJoinVO1Mock, domainRouterJoinVO2Mock)); - updateFinalStaticField(manager.getClass().getField("VpcMaxNetworks"), vpcMaxNetworksMock); - - int vpcMaxNetworks = manager.getVpcMaxNetworksValue(vpcId); - - Assert.assertEquals(1, vpcMaxNetworks); - } - - void updateFinalStaticField(Field field, Object newValue) throws IllegalArgumentException, IllegalAccessException, NoSuchFieldException, SecurityException { - field.setAccessible(true); - - Field modifiersField = Field.class.getDeclaredField("modifiers"); - modifiersField.setAccessible(true); - modifiersField.setInt(field, field.getModifiers() & ~Modifier.FINAL); - - field.set(null, newValue); - } } From e0cddb72f2cd331d83912da654b8eb59ae0248a1 Mon Sep 17 00:00:00 2001 From: Henrique Sato Date: Fri, 1 Mar 2024 18:08:13 -0300 Subject: [PATCH 03/16] Adjusts --- .../src/main/java/com/cloud/network/vpc/VpcManager.java | 2 +- .../main/java/com/cloud/network/vpc/VpcManagerImpl.java | 7 ------- .../java/com/cloud/network/vpc/VpcManagerImplTest.java | 2 +- 3 files changed, 2 insertions(+), 9 deletions(-) diff --git a/engine/components-api/src/main/java/com/cloud/network/vpc/VpcManager.java b/engine/components-api/src/main/java/com/cloud/network/vpc/VpcManager.java index edc06dff9ec2..21779983e7a1 100644 --- a/engine/components-api/src/main/java/com/cloud/network/vpc/VpcManager.java +++ b/engine/components-api/src/main/java/com/cloud/network/vpc/VpcManager.java @@ -41,7 +41,7 @@ public interface VpcManager { ConfigKey VpcMaxNetworks = new ConfigKey<>("Advanced", Integer.class, "vpc.max.networks", "3", - "Maximum number of networks per VPC. Bear in mind that this value will depend on the hypervisor where the VR was/will be deployed.", true, ConfigKey.Scope.Account); + "Maximum number of networks per VPC.", true, ConfigKey.Scope.Account); /** * Returns all the Guest networks that are part of VPC diff --git a/server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java b/server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java index e9840ebddae1..24ebccff6b94 100644 --- a/server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java +++ b/server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java @@ -1881,13 +1881,6 @@ public void doInTransactionWithoutResult(final TransactionStatus status) { }); } - /** - * Searches for the {@link #VpcMaxNetworks} value in the VPC domain routers cluster and returns the smallest value. - * If the VPC does not have any domain routers, it returns the {@link #VpcMaxNetworks} global configuration value. - * - * @param vpcId ID from VPC. - * @return The {@link #VpcMaxNetworks} configuration value. - */ private void CheckAccountsAccess(Vpc vpc, Account networkAccount) { Account vpcaccount = _accountMgr.getAccount(vpc.getAccountId()); try { diff --git a/server/src/test/java/com/cloud/network/vpc/VpcManagerImplTest.java b/server/src/test/java/com/cloud/network/vpc/VpcManagerImplTest.java index ac7ab51fc9ab..deffb165f29f 100644 --- a/server/src/test/java/com/cloud/network/vpc/VpcManagerImplTest.java +++ b/server/src/test/java/com/cloud/network/vpc/VpcManagerImplTest.java @@ -510,4 +510,4 @@ public void validateVpcPrivateGatewayTestAclFromDifferentVpcThrowsInvalidParamet Assert.assertThrows(InvalidParameterValueException.class, () -> manager.validateVpcPrivateGatewayAclId(vpcId, differentVpcAclId)); } - } +} From e0e580158a35c64873d204f12923ac268aae2691 Mon Sep 17 00:00:00 2001 From: Henrique Sato Date: Thu, 21 Mar 2024 16:08:27 -0300 Subject: [PATCH 04/16] Limit number of tiers --- .../service/NetworkOrchestrationService.java | 12 +++ .../orchestration/NetworkOrchestrator.java | 25 ++---- .../NetworkOrchestratorTest.java | 76 +++++++++---------- .../com/cloud/network/vpc/VpcManagerImpl.java | 32 +++++++- .../cloud/network/vpc/VpcManagerImplTest.java | 12 ++- .../com/cloud/vpc/MockNetworkManagerImpl.java | 5 ++ 6 files changed, 99 insertions(+), 63 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 059ce11b4bb5..a04a485ab737 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 @@ -353,4 +353,16 @@ void implementNetworkElementsAndResources(DeployDestination dest, ReservationCon Pair 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); + + /** + * Returns the maximum number of NICs that the given virtual machine can have considering its hypervisor. + *

+ * First we try to retrieve the setting value from the cluster where the virtual machine is deployed. If the cluster does not exist, we try to retrieve the setting value from the virtual machine hypervisor type. + * In last case, if the virtual machine does not have a hypervisor type, we retrieve the smallest value between the {@link NetworkOrchestrationService#VirtualMachineMaxNicsKvm}, {@link NetworkOrchestrationService#VirtualMachineMaxNicsVmware} + * and {@link NetworkOrchestrationService#VirtualMachineMaxNicsXenserver} global settings. + * + * @param virtualMachine Virtual machine to get the maximum number of NICs. + * @return The maximum number of NICs that the virtual machine can have. + */ + int getVirtualMachineMaxNicsValue(VirtualMachine virtualMachine); } 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 254964e2e484..f11514fd95a3 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 @@ -1014,10 +1014,10 @@ public void saveExtraDhcpOptions(final String networkUuid, final Long nicId, fin public Pair allocateNic(final NicProfile requested, final Network network, final Boolean isDefaultNic, int deviceId, final VirtualMachineProfile vm) throws InsufficientVirtualNetworkCapacityException, InsufficientAddressCapacityException, ConcurrentOperationException { - Integer virtualMachineMaxNicsValue = getVirtualMachineMaxNicsValue(vm); + int virtualMachineMaxNicsValue = getVirtualMachineMaxNicsValue(vm.getVirtualMachine()); List nics = _nicDao.listByVmId(vm.getId()); - if (nics.size() > virtualMachineMaxNicsValue) { + if (nics.size() >= virtualMachineMaxNicsValue) { throw new CloudRuntimeException(String.format("Failed to allocate NIC on network [%s] because VM [%s] has reached its maximum NIC capacity [%s].", network.getUuid(), vm.getUuid(), virtualMachineMaxNicsValue)); } @@ -1065,24 +1065,15 @@ public Pair allocateNic(final NicProfile requested, final N return new Pair(vmNic, Integer.valueOf(deviceId)); } - /** - * Returns the maximum number of NICs that the given virtual machine can have considering its hypervisor. - *

- * First we try to retrieve the setting value from the cluster where the virtual machine is deployed. If the cluster does not exist, we try to retrieve the setting value from the virtual machine hypervisor type. - * In last case, if the virtual machine does not have a hypervisor type, we retrieve the smallest value between the {@link NetworkOrchestrationService#VirtualMachineMaxNicsKvm}, {@link NetworkOrchestrationService#VirtualMachineMaxNicsVmware} - * and {@link NetworkOrchestrationService#VirtualMachineMaxNicsXenserver} global settings. - * - * @param virtualMachine Virtual machine to get the maximum number of NICs. - * @return The maximum number of NICs that the virtual machine can have. - */ - protected Integer getVirtualMachineMaxNicsValue(VirtualMachineProfile virtualMachine) { + @Override + public int getVirtualMachineMaxNicsValue(VirtualMachine virtualMachine) { Integer virtualMachineMaxNicsValue = getVirtualMachineMaxNicsValueFromCluster(virtualMachine); if (virtualMachineMaxNicsValue != null) { return virtualMachineMaxNicsValue; } - if (virtualMachine.getVirtualMachine().getHypervisorType() == null) { + if (virtualMachine.getHypervisorType() == null) { logger.debug("Using the smallest setting global value between {}, {} and {} as the VM {} does not have a hypervisor type and is not deployed on either a host or a cluster.", VirtualMachineMaxNicsKvm, VirtualMachineMaxNicsVmware, VirtualMachineMaxNicsXenserver, virtualMachine.getUuid()); return NumberUtils.min(VirtualMachineMaxNicsKvm.value(), VirtualMachineMaxNicsVmware.value(), VirtualMachineMaxNicsXenserver.value()); } @@ -1096,7 +1087,7 @@ protected Integer getVirtualMachineMaxNicsValue(VirtualMachineProfile virtualMac * @param virtualMachine Virtual machine to get the cluster. * @return The maximum number of NICs that the virtual machine can have. */ - protected Integer getVirtualMachineMaxNicsValueFromCluster(VirtualMachineProfile virtualMachine) { + protected Integer getVirtualMachineMaxNicsValueFromCluster(VirtualMachine virtualMachine) { HostVO host = _hostDao.findById(virtualMachine.getHostId()); if (host == null) { return null; @@ -1130,8 +1121,8 @@ protected Integer getVirtualMachineMaxNicsValueFromCluster(VirtualMachineProfile * @param virtualMachine Virtual machine to get the hypervisor type. * @return The maximum number of NICs that the virtual machine can have. */ - protected Integer getVirtualMachineMaxNicsValueFromVmHypervisorType(VirtualMachineProfile virtualMachine) { - HypervisorType virtualMachineHypervisorType = virtualMachine.getVirtualMachine().getHypervisorType(); + protected int getVirtualMachineMaxNicsValueFromVmHypervisorType(VirtualMachine virtualMachine) { + HypervisorType virtualMachineHypervisorType = virtualMachine.getHypervisorType(); if (HypervisorType.KVM.equals(virtualMachineHypervisorType)) { logger.debug("Using the {} setting global value {} as the VM {} has the {} hypervisor type and is not deployed on either a host or a cluster.", VirtualMachineMaxNicsKvm, VirtualMachineMaxNicsKvm.value(), virtualMachine.getUuid(), virtualMachineHypervisorType); 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 374c92152210..e88b56444703 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 @@ -852,13 +852,13 @@ public void testGetGuestIpForNicImportBasicUsedIP() { @Test public void getVirtualMachineMaxNicsValueTestVirtualMachineDeployedReturnsVirtualMachineClusterMaxNics() { - VirtualMachineProfile virtualMachineProfileMock = Mockito.mock(VirtualMachineProfile.class); - Mockito.doReturn(44).when(testOrchastrator).getVirtualMachineMaxNicsValueFromCluster(virtualMachineProfileMock); + VirtualMachine virtualMachineMock = Mockito.mock(VirtualMachine.class); + Mockito.doReturn(44).when(testOrchastrator).getVirtualMachineMaxNicsValueFromCluster(virtualMachineMock); - Integer virtualMachineMaxNicsValue = testOrchastrator.getVirtualMachineMaxNicsValue(virtualMachineProfileMock); + int virtualMachineMaxNicsValue = testOrchastrator.getVirtualMachineMaxNicsValue(virtualMachineMock); Mockito.verify(testOrchastrator, Mockito.times(1)).getVirtualMachineMaxNicsValueFromCluster(Mockito.any()); - Assert.assertEquals((Integer) 44, virtualMachineMaxNicsValue); + Assert.assertEquals(44, virtualMachineMaxNicsValue); } @Test @@ -870,7 +870,7 @@ public void getVirtualMachineMaxNicsValueTestVirtualMachineWithoutDeployAndTempl ConfigKey virtualMachineMaxNicsXenserverMock = Mockito.mock(ConfigKey.class); Mockito.doReturn(virtualMachineMock).when(virtualMachineProfileMock).getVirtualMachine(); Mockito.doReturn(null).when(virtualMachineMock).getHypervisorType(); - Mockito.doReturn(null).when(testOrchastrator).getVirtualMachineMaxNicsValueFromCluster(virtualMachineProfileMock); + Mockito.doReturn(null).when(testOrchastrator).getVirtualMachineMaxNicsValueFromCluster(virtualMachineMock); Mockito.doReturn(23).when(virtualMachineMaxNicsKvmMock).value(); Mockito.doReturn(10).when(virtualMachineMaxNicsVmwareMock).value(); Mockito.doReturn(7).when(virtualMachineMaxNicsXenserverMock).value(); @@ -878,9 +878,9 @@ public void getVirtualMachineMaxNicsValueTestVirtualMachineWithoutDeployAndTempl updateFinalStaticField(testOrchastrator.getClass().getField("VirtualMachineMaxNicsVmware"), virtualMachineMaxNicsVmwareMock); updateFinalStaticField(testOrchastrator.getClass().getField("VirtualMachineMaxNicsXenserver"), virtualMachineMaxNicsXenserverMock); - Integer virtualMachineMaxNicsValue = testOrchastrator.getVirtualMachineMaxNicsValue(virtualMachineProfileMock); + int virtualMachineMaxNicsValue = testOrchastrator.getVirtualMachineMaxNicsValue(virtualMachineMock); - Assert.assertEquals((Integer) 7, virtualMachineMaxNicsValue); + Assert.assertEquals(7, virtualMachineMaxNicsValue); } @Test @@ -889,47 +889,47 @@ public void getVirtualMachineMaxNicsValueTestVirtualMachineWithoutDeployReturnsV VirtualMachine virtualMachineMock = Mockito.mock(VirtualMachine.class); Mockito.doReturn(virtualMachineMock).when(virtualMachineProfileMock).getVirtualMachine(); Mockito.doReturn(Hypervisor.HypervisorType.KVM).when(virtualMachineMock).getHypervisorType(); - Mockito.doReturn(null).when(testOrchastrator).getVirtualMachineMaxNicsValueFromCluster(virtualMachineProfileMock); - Mockito.doReturn(33).when(testOrchastrator).getVirtualMachineMaxNicsValueFromVmHypervisorType(virtualMachineProfileMock); + Mockito.doReturn(null).when(testOrchastrator).getVirtualMachineMaxNicsValueFromCluster(virtualMachineMock); + Mockito.doReturn(33).when(testOrchastrator).getVirtualMachineMaxNicsValueFromVmHypervisorType(virtualMachineMock); - Integer virtualMachineMaxNicsValue = testOrchastrator.getVirtualMachineMaxNicsValue(virtualMachineProfileMock); + int virtualMachineMaxNicsValue = testOrchastrator.getVirtualMachineMaxNicsValue(virtualMachineMock); Mockito.verify(testOrchastrator, Mockito.times(1)).getVirtualMachineMaxNicsValueFromVmHypervisorType(Mockito.any()); - Assert.assertEquals((Integer) 33, virtualMachineMaxNicsValue); + Assert.assertEquals(33, virtualMachineMaxNicsValue); } @Test public void getVirtualMachineMaxNicsValueFromClusterTestHostDoesNotExistReturnsNull() { - VirtualMachineProfile virtualMachineProfileMock = Mockito.mock(VirtualMachineProfile.class); - Mockito.doReturn(100L).when(virtualMachineProfileMock).getHostId(); + VirtualMachine virtualMachineMock = Mockito.mock(VirtualMachine.class); + Mockito.doReturn(100L).when(virtualMachineMock).getHostId(); Mockito.doReturn(null).when(testOrchastrator._hostDao).findById(100L); - Integer virtualMachineMaxNicsValue = testOrchastrator.getVirtualMachineMaxNicsValueFromCluster(virtualMachineProfileMock); + Integer virtualMachineMaxNicsValue = testOrchastrator.getVirtualMachineMaxNicsValueFromCluster(virtualMachineMock); Assert.assertNull(virtualMachineMaxNicsValue); } @Test public void getVirtualMachineMaxNicsValueFromClusterTestClusterDoesNotExistReturnsNull() { - VirtualMachineProfile virtualMachineProfileMock = Mockito.mock(VirtualMachineProfile.class); + VirtualMachine virtualMachineMock = Mockito.mock(VirtualMachine.class); HostVO hostVoMock = Mockito.mock(HostVO.class); - Mockito.doReturn(1L).when(virtualMachineProfileMock).getHostId(); + Mockito.doReturn(1L).when(virtualMachineMock).getHostId(); Mockito.doReturn(hostVoMock).when(testOrchastrator._hostDao).findById(1L); Mockito.doReturn(100L).when(hostVoMock).getClusterId(); Mockito.doReturn(null).when(testOrchastrator.clusterDao).findById(100L); - Integer virtualMachineMaxNicsValue = testOrchastrator.getVirtualMachineMaxNicsValueFromCluster(virtualMachineProfileMock); + Integer virtualMachineMaxNicsValue = testOrchastrator.getVirtualMachineMaxNicsValueFromCluster(virtualMachineMock); Assert.assertNull(virtualMachineMaxNicsValue); } @Test public void getVirtualMachineMaxNicsValueFromClusterTestKvmClusterReturnsVirtualMachineMaxNicsKvmClusterValue() throws NoSuchFieldException, IllegalAccessException { - VirtualMachineProfile virtualMachineProfileMock = Mockito.mock(VirtualMachineProfile.class); + VirtualMachine virtualMachineMock = Mockito.mock(VirtualMachine.class); HostVO hostVoMock = Mockito.mock(HostVO.class); ClusterVO clusterVoMock = Mockito.mock(ClusterVO.class); ConfigKey virtualMachineMaxNicsKvmMock = Mockito.mock(ConfigKey.class); - Mockito.doReturn(1L).when(virtualMachineProfileMock).getHostId(); + Mockito.doReturn(1L).when(virtualMachineMock).getHostId(); Mockito.doReturn(hostVoMock).when(testOrchastrator._hostDao).findById(1L); Mockito.doReturn(1L).when(hostVoMock).getClusterId(); Mockito.doReturn(clusterVoMock).when(testOrchastrator.clusterDao).findById(1L); @@ -938,18 +938,18 @@ public void getVirtualMachineMaxNicsValueFromClusterTestKvmClusterReturnsVirtual Mockito.doReturn(33).when(virtualMachineMaxNicsKvmMock).valueIn(1L); updateFinalStaticField(testOrchastrator.getClass().getField("VirtualMachineMaxNicsKvm"), virtualMachineMaxNicsKvmMock); - Integer virtualMachineMaxNicsValue = testOrchastrator.getVirtualMachineMaxNicsValueFromCluster(virtualMachineProfileMock); + int virtualMachineMaxNicsValue = testOrchastrator.getVirtualMachineMaxNicsValueFromCluster(virtualMachineMock); - Assert.assertEquals((Integer) 33, virtualMachineMaxNicsValue); + Assert.assertEquals(33, virtualMachineMaxNicsValue); } @Test public void getVirtualMachineMaxNicsValueFromClusterTestVmwareClusterReturnsVirtualMachineMaxNicsVmwareClusterValue() throws NoSuchFieldException, IllegalAccessException { - VirtualMachineProfile virtualMachineProfileMock = Mockito.mock(VirtualMachineProfile.class); + VirtualMachine virtualMachineMock = Mockito.mock(VirtualMachine.class); HostVO hostVoMock = Mockito.mock(HostVO.class); ClusterVO clusterVoMock = Mockito.mock(ClusterVO.class); ConfigKey virtualMachineMaxNicsVmwareMock = Mockito.mock(ConfigKey.class); - Mockito.doReturn(1L).when(virtualMachineProfileMock).getHostId(); + Mockito.doReturn(1L).when(virtualMachineMock).getHostId(); Mockito.doReturn(hostVoMock).when(testOrchastrator._hostDao).findById(1L); Mockito.doReturn(1L).when(hostVoMock).getClusterId(); Mockito.doReturn(clusterVoMock).when(testOrchastrator.clusterDao).findById(1L); @@ -958,18 +958,18 @@ public void getVirtualMachineMaxNicsValueFromClusterTestVmwareClusterReturnsVirt Mockito.doReturn(22).when(virtualMachineMaxNicsVmwareMock).valueIn(1L); updateFinalStaticField(testOrchastrator.getClass().getField("VirtualMachineMaxNicsVmware"), virtualMachineMaxNicsVmwareMock); - Integer virtualMachineMaxNicsValue = testOrchastrator.getVirtualMachineMaxNicsValueFromCluster(virtualMachineProfileMock); + int virtualMachineMaxNicsValue = testOrchastrator.getVirtualMachineMaxNicsValueFromCluster(virtualMachineMock); - Assert.assertEquals((Integer) 22, virtualMachineMaxNicsValue); + Assert.assertEquals(22, virtualMachineMaxNicsValue); } @Test public void getVirtualMachineMaxNicsValueFromClusterTestXenserverClusterReturnsVirtualMachineMaxNicsXenserverClusterValue() throws NoSuchFieldException, IllegalAccessException { - VirtualMachineProfile virtualMachineProfileMock = Mockito.mock(VirtualMachineProfile.class); + VirtualMachine virtualMachineMock = Mockito.mock(VirtualMachine.class); HostVO hostVoMock = Mockito.mock(HostVO.class); ClusterVO clusterVoMock = Mockito.mock(ClusterVO.class); ConfigKey virtualMachineMaxNicsXenserverMock = Mockito.mock(ConfigKey.class); - Mockito.doReturn(1L).when(virtualMachineProfileMock).getHostId(); + Mockito.doReturn(1L).when(virtualMachineMock).getHostId(); Mockito.doReturn(hostVoMock).when(testOrchastrator._hostDao).findById(1L); Mockito.doReturn(1L).when(hostVoMock).getClusterId(); Mockito.doReturn(clusterVoMock).when(testOrchastrator.clusterDao).findById(1L); @@ -978,54 +978,48 @@ public void getVirtualMachineMaxNicsValueFromClusterTestXenserverClusterReturnsV Mockito.doReturn(11).when(virtualMachineMaxNicsXenserverMock).valueIn(1L); updateFinalStaticField(testOrchastrator.getClass().getField("VirtualMachineMaxNicsXenserver"), virtualMachineMaxNicsXenserverMock); - Integer virtualMachineMaxNicsValue = testOrchastrator.getVirtualMachineMaxNicsValueFromCluster(virtualMachineProfileMock); + int virtualMachineMaxNicsValue = testOrchastrator.getVirtualMachineMaxNicsValueFromCluster(virtualMachineMock); - Assert.assertEquals((Integer) 11, virtualMachineMaxNicsValue); + Assert.assertEquals(11, virtualMachineMaxNicsValue); } @Test public void getVirtualMachineMaxNicsValueFromVmHypervisorTypeTestKvmHypervisorReturnsVirtualMachineMaxNicsKvmGlobalValue() throws NoSuchFieldException, IllegalAccessException { - VirtualMachineProfile virtualMachineProfileMock = Mockito.mock(VirtualMachineProfile.class); VirtualMachine virtualMachineMock = Mockito.mock(VirtualMachine.class); ConfigKey virtualMachineMaxNicsKvmMock = Mockito.mock(ConfigKey.class); - Mockito.doReturn(virtualMachineMock).when(virtualMachineProfileMock).getVirtualMachine(); Mockito.doReturn(Hypervisor.HypervisorType.KVM).when(virtualMachineMock).getHypervisorType(); Mockito.doReturn(23).when(virtualMachineMaxNicsKvmMock).value(); updateFinalStaticField(testOrchastrator.getClass().getField("VirtualMachineMaxNicsKvm"), virtualMachineMaxNicsKvmMock); - Integer virtualMachineMaxNicsValue = testOrchastrator.getVirtualMachineMaxNicsValueFromVmHypervisorType(virtualMachineProfileMock); + int virtualMachineMaxNicsValue = testOrchastrator.getVirtualMachineMaxNicsValueFromVmHypervisorType(virtualMachineMock); - Assert.assertEquals((Integer) 23, virtualMachineMaxNicsValue); + Assert.assertEquals(23, virtualMachineMaxNicsValue); } @Test public void getVirtualMachineMaxNicsValueFromVmHypervisorTypeTestVmwareHypervisorReturnsVirtualMachineMaxNicsVmwareGlobalValue() throws NoSuchFieldException, IllegalAccessException { - VirtualMachineProfile virtualMachineProfileMock = Mockito.mock(VirtualMachineProfile.class); VirtualMachine virtualMachineMock = Mockito.mock(VirtualMachine.class); ConfigKey virtualMachineMaxNicsVmwareMock = Mockito.mock(ConfigKey.class); - Mockito.doReturn(virtualMachineMock).when(virtualMachineProfileMock).getVirtualMachine(); Mockito.doReturn(Hypervisor.HypervisorType.VMware).when(virtualMachineMock).getHypervisorType(); Mockito.doReturn(10).when(virtualMachineMaxNicsVmwareMock).value(); updateFinalStaticField(testOrchastrator.getClass().getField("VirtualMachineMaxNicsVmware"), virtualMachineMaxNicsVmwareMock); - Integer virtualMachineMaxNicsValue = testOrchastrator.getVirtualMachineMaxNicsValueFromVmHypervisorType(virtualMachineProfileMock); + int virtualMachineMaxNicsValue = testOrchastrator.getVirtualMachineMaxNicsValueFromVmHypervisorType(virtualMachineMock); - Assert.assertEquals((Integer) 10, virtualMachineMaxNicsValue); + Assert.assertEquals(10, virtualMachineMaxNicsValue); } @Test public void getVirtualMachineMaxNicsValueFromVmHypervisorTypeTestXenserverHypervisorReturnsVirtualMachineMaxNicsXenserverGlobalValue() throws NoSuchFieldException, IllegalAccessException { - VirtualMachineProfile virtualMachineProfileMock = Mockito.mock(VirtualMachineProfile.class); VirtualMachine virtualMachineMock = Mockito.mock(VirtualMachine.class); ConfigKey virtualMachineMaxNicsXenserverMock = Mockito.mock(ConfigKey.class); - Mockito.doReturn(virtualMachineMock).when(virtualMachineProfileMock).getVirtualMachine(); Mockito.doReturn(Hypervisor.HypervisorType.XenServer).when(virtualMachineMock).getHypervisorType(); Mockito.doReturn(7).when(virtualMachineMaxNicsXenserverMock).value(); updateFinalStaticField(testOrchastrator.getClass().getField("VirtualMachineMaxNicsXenserver"), virtualMachineMaxNicsXenserverMock); - Integer virtualMachineMaxNicsValue = testOrchastrator.getVirtualMachineMaxNicsValueFromVmHypervisorType(virtualMachineProfileMock); + int virtualMachineMaxNicsValue = testOrchastrator.getVirtualMachineMaxNicsValueFromVmHypervisorType(virtualMachineMock); - Assert.assertEquals((Integer) 7, virtualMachineMaxNicsValue); + Assert.assertEquals(7, virtualMachineMaxNicsValue); } void updateFinalStaticField(Field field, Object newValue) throws IllegalArgumentException, IllegalAccessException, NoSuchFieldException, SecurityException { diff --git a/server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java b/server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java index 24ebccff6b94..5ccd03ffc761 100644 --- a/server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java +++ b/server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java @@ -261,6 +261,8 @@ public class VpcManagerImpl extends ManagerBase implements VpcManager, VpcProvis AlertManager alertManager; @Inject CommandSetupHelper commandSetupHelper; + @Inject + NetworkOrchestrationService networkOrchestrationService; @Autowired @Qualifier("networkHelper") protected NetworkHelper networkHelper; @@ -1826,18 +1828,25 @@ protected void validateNewVpcGuestNetwork(final String cidr, final String gatewa Transaction.execute(new TransactionCallbackNoReturn() { @Override public void doInTransactionWithoutResult(final TransactionStatus status) { - final Vpc locked = vpcDao.acquireInLockTable(vpc.getId()); + long vpcId = vpc.getId(); + final Vpc locked = vpcDao.acquireInLockTable(vpcId); if (locked == null) { throw new CloudRuntimeException("Unable to acquire lock on " + vpc); } _maxNetworks = VpcMaxNetworks.valueIn(vpc.getAccountId()); + Account account = _accountMgr.getAccount(vpc.getAccountId()); try { // check number of active networks in vpc - if (_ntwkDao.countVpcNetworks(vpc.getId()) >= _maxNetworks) { + if (_ntwkDao.countVpcNetworks(vpcId) >= _maxNetworks) { logger.warn("Failed to create a new VPC Guest Network because the number of networks per VPC has reached its maximum capacity of {}. Increase it by modifying global or account config {}.", _maxNetworks, VpcMaxNetworks); - throw new CloudRuntimeException(String.format("Number of networks per VPC cannot surpass [%s].", _maxNetworks)); + throw new CloudRuntimeException(String.format("Number of networks per VPC cannot surpass [%s] for account [%s].", _maxNetworks, account.getAccountName())); + } + + if (!existsVpcDomainRouterWithSufficientNicCapacity(vpc.getId())) { + logger.warn("Failed to create a new VPC Guest Network because no virtual routers were found with sufficient NIC capacity. The number of VPC Guest networks cannot exceed the number of NICs a virtual router can have."); + throw new CloudRuntimeException(String.format("No available virtual router found to deploy new Guest Network on VPC [%s].", vpc.getName())); } // 1) CIDR is required @@ -1852,7 +1861,7 @@ public void doInTransactionWithoutResult(final TransactionStatus status) { // 3) Network cidr shouldn't cross the cidr of other vpc // network cidrs - final List ntwks = _ntwkDao.listByVpc(vpc.getId()); + final List ntwks = _ntwkDao.listByVpc(vpcId); for (final Network ntwk : ntwks) { assert cidr != null : "Why the network cidr is null when it belongs to vpc?"; @@ -1881,6 +1890,21 @@ public void doInTransactionWithoutResult(final TransactionStatus status) { }); } + protected boolean existsVpcDomainRouterWithSufficientNicCapacity(long vpcId) { + long countVpcNetworks = _ntwkDao.countVpcNetworks(vpcId); + List vpcDomainRoutersVO = routerDao.listByVpcId(vpcId); + int availableRouters = vpcDomainRoutersVO.size(); + + for (DomainRouterVO domainRouter : vpcDomainRoutersVO) { + if (countVpcNetworks >= networkOrchestrationService.getVirtualMachineMaxNicsValue(domainRouter)) { + logger.debug("Virtual router [%s] is unable to reserve the NIC for the new VPC Guest Network.", domainRouter.getName()); + availableRouters--; + } + } + + return availableRouters > 0; + } + private void CheckAccountsAccess(Vpc vpc, Account networkAccount) { Account vpcaccount = _accountMgr.getAccount(vpc.getAccountId()); try { diff --git a/server/src/test/java/com/cloud/network/vpc/VpcManagerImplTest.java b/server/src/test/java/com/cloud/network/vpc/VpcManagerImplTest.java index deffb165f29f..8bd0298e08db 100644 --- a/server/src/test/java/com/cloud/network/vpc/VpcManagerImplTest.java +++ b/server/src/test/java/com/cloud/network/vpc/VpcManagerImplTest.java @@ -81,6 +81,7 @@ import org.mockito.Mock; import org.mockito.Mockito; import org.mockito.MockitoAnnotations; +import org.mockito.Spy; import org.mockito.junit.MockitoJUnitRunner; import org.springframework.test.util.ReflectionTestUtils; @@ -122,6 +123,7 @@ public class VpcManagerImplTest { DataCenterDao dataCenterDao; @Mock VpcOfferingServiceMapDao vpcOfferingServiceMapDao; + @Spy VpcManagerImpl manager; @Mock EntityManager entityMgr; @@ -155,6 +157,10 @@ public class VpcManagerImplTest { FirewallRulesDao firewallDao; @Mock NetworkACLVO networkACLVOMock; + @Mock + NetworkOrchestrationService networkOrchestrationServiceMock; + @Mock + DomainRouterVO domainRouterVOMock; public static final long ACCOUNT_ID = 1; private AccountVO account; @@ -190,7 +196,6 @@ private void registerCallContext() { @Before public void setup() throws NoSuchFieldException, IllegalAccessException { closeable = MockitoAnnotations.openMocks(this); - manager = new VpcManagerImpl(); manager._vpcOffSvcMapDao = vpcOfferingServiceMapDao; manager.vpcDao = vpcDao; manager._ntwkMgr = networkMgr; @@ -212,6 +217,7 @@ public void setup() throws NoSuchFieldException, IllegalAccessException { manager._ntwkSvc = networkServiceMock; manager._firewallDao = firewallDao; manager._networkAclDao = networkACLDaoMock; + manager.networkOrchestrationService = networkOrchestrationServiceMock; CallContext.register(Mockito.mock(User.class), Mockito.mock(Account.class)); registerCallContext(); overrideDefaultConfigValue(NetworkService.AllowUsersToSpecifyVRMtu, "_defaultValue", "false"); @@ -358,6 +364,10 @@ public void testCreateVpcNetwork() throws InsufficientCapacityException, Resourc Mockito.when(networkOfferingServiceMapDao.listByNetworkOfferingId(anyLong())).thenReturn(serviceMap); Mockito.when(vpcMock.getCidr()).thenReturn("10.0.0.0/8"); Mockito.when(vpcMock.getNetworkDomain()).thenReturn("cs1cloud.internal"); + Mockito.when(routerDao.listByVpcId(VPC_ID)).thenReturn(List.of(domainRouterVOMock)); + Mockito.when(networkOrchestrationServiceMock.getVirtualMachineMaxNicsValue(domainRouterVOMock)).thenReturn(3); + Mockito.when(networkDao.countVpcNetworks(VPC_ID)).thenReturn(1L); + Mockito.when(manager.existsVpcDomainRouterWithSufficientNicCapacity(VPC_ID)).thenReturn(true); manager.validateNewVpcGuestNetwork("10.10.10.0/24", "10.10.10.1", accountMock, vpcMock, "cs1cloud.internal"); manager.validateNtwkOffForNtwkInVpc(2L, 1, "10.10.10.0/24", "111-", vpcMock, "10.1.1.1", new AccountVO(), null); diff --git a/server/src/test/java/com/cloud/vpc/MockNetworkManagerImpl.java b/server/src/test/java/com/cloud/vpc/MockNetworkManagerImpl.java index 434c644f5969..c4ad7064da4b 100644 --- a/server/src/test/java/com/cloud/vpc/MockNetworkManagerImpl.java +++ b/server/src/test/java/com/cloud/vpc/MockNetworkManagerImpl.java @@ -1043,6 +1043,11 @@ public Pair importNic(String macAddress, int deviceId, Netw public void unmanageNics(VirtualMachineProfile vm) { } + @Override + public int getVirtualMachineMaxNicsValue(VirtualMachine virtualMachine) { + return 0; + } + @Override public Pair, Integer> listGuestVlans(ListGuestVlansCmd cmd) { return null; From 0c137849cfb24dc60ecd5e859a55f60567891c94 Mon Sep 17 00:00:00 2001 From: Henrique Sato Date: Thu, 21 Mar 2024 17:56:56 -0300 Subject: [PATCH 05/16] Fix validation --- .../com/cloud/network/vpc/VpcManagerImpl.java | 10 +++--- .../cloud/network/vpc/VpcManagerImplTest.java | 31 +++++++++++++++++-- 2 files changed, 32 insertions(+), 9 deletions(-) diff --git a/server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java b/server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java index 6a194592dbe8..9fad8ffc56fe 100644 --- a/server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java +++ b/server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java @@ -1996,18 +1996,16 @@ public void doInTransactionWithoutResult(final TransactionStatus status) { } protected boolean existsVpcDomainRouterWithSufficientNicCapacity(long vpcId) { + int countRouterDefaultNics = 2; long countVpcNetworks = _ntwkDao.countVpcNetworks(vpcId); List vpcDomainRoutersVO = routerDao.listByVpcId(vpcId); - int availableRouters = vpcDomainRoutersVO.size(); + int totalNicsAvailable = 0; for (DomainRouterVO domainRouter : vpcDomainRoutersVO) { - if (countVpcNetworks >= networkOrchestrationService.getVirtualMachineMaxNicsValue(domainRouter)) { - logger.debug("Virtual router [%s] is unable to reserve the NIC for the new VPC Guest Network.", domainRouter.getName()); - availableRouters--; - } + totalNicsAvailable += networkOrchestrationService.getVirtualMachineMaxNicsValue(domainRouter) - countRouterDefaultNics; } - return availableRouters > 0; + return totalNicsAvailable > countVpcNetworks; } private void CheckAccountsAccess(Vpc vpc, Account networkAccount) { diff --git a/server/src/test/java/com/cloud/network/vpc/VpcManagerImplTest.java b/server/src/test/java/com/cloud/network/vpc/VpcManagerImplTest.java index 8bd0298e08db..1461d7feb15e 100644 --- a/server/src/test/java/com/cloud/network/vpc/VpcManagerImplTest.java +++ b/server/src/test/java/com/cloud/network/vpc/VpcManagerImplTest.java @@ -160,7 +160,9 @@ public class VpcManagerImplTest { @Mock NetworkOrchestrationService networkOrchestrationServiceMock; @Mock - DomainRouterVO domainRouterVOMock; + DomainRouterVO domainRouter1VOMock; + @Mock + DomainRouterVO domainRouter2VOMock; public static final long ACCOUNT_ID = 1; private AccountVO account; @@ -364,8 +366,8 @@ public void testCreateVpcNetwork() throws InsufficientCapacityException, Resourc Mockito.when(networkOfferingServiceMapDao.listByNetworkOfferingId(anyLong())).thenReturn(serviceMap); Mockito.when(vpcMock.getCidr()).thenReturn("10.0.0.0/8"); Mockito.when(vpcMock.getNetworkDomain()).thenReturn("cs1cloud.internal"); - Mockito.when(routerDao.listByVpcId(VPC_ID)).thenReturn(List.of(domainRouterVOMock)); - Mockito.when(networkOrchestrationServiceMock.getVirtualMachineMaxNicsValue(domainRouterVOMock)).thenReturn(3); + Mockito.when(routerDao.listByVpcId(VPC_ID)).thenReturn(List.of(domainRouter1VOMock)); + Mockito.when(networkOrchestrationServiceMock.getVirtualMachineMaxNicsValue(domainRouter1VOMock)).thenReturn(3); Mockito.when(networkDao.countVpcNetworks(VPC_ID)).thenReturn(1L); Mockito.when(manager.existsVpcDomainRouterWithSufficientNicCapacity(VPC_ID)).thenReturn(true); @@ -520,4 +522,27 @@ public void validateVpcPrivateGatewayTestAclFromDifferentVpcThrowsInvalidParamet Assert.assertThrows(InvalidParameterValueException.class, () -> manager.validateVpcPrivateGatewayAclId(vpcId, differentVpcAclId)); } + @Test + public void existsVpcDomainRouterWithSufficientNicCapacityTestUnavailableRoutersReturnsFalse() { + Mockito.when(networkDao.countVpcNetworks(vpcId)).thenReturn(8L); + Mockito.when(routerDao.listByVpcId(vpcId)).thenReturn(List.of(domainRouter1VOMock, domainRouter2VOMock)); + Mockito.when(networkOrchestrationServiceMock.getVirtualMachineMaxNicsValue(domainRouter1VOMock)).thenReturn(6); + Mockito.when(networkOrchestrationServiceMock.getVirtualMachineMaxNicsValue(domainRouter2VOMock)).thenReturn(6); + + boolean result = manager.existsVpcDomainRouterWithSufficientNicCapacity(vpcId); + + Assert.assertFalse(result); + } + + @Test + public void existsVpcDomainRouterWithSufficientNicCapacityTestAvailableRouterReturnsTrue() { + Mockito.when(networkDao.countVpcNetworks(vpcId)).thenReturn(7L); + Mockito.when(routerDao.listByVpcId(vpcId)).thenReturn(List.of(domainRouter1VOMock, domainRouter2VOMock)); + Mockito.when(networkOrchestrationServiceMock.getVirtualMachineMaxNicsValue(domainRouter1VOMock)).thenReturn(6); + Mockito.when(networkOrchestrationServiceMock.getVirtualMachineMaxNicsValue(domainRouter2VOMock)).thenReturn(6); + + boolean result = manager.existsVpcDomainRouterWithSufficientNicCapacity(vpcId); + + Assert.assertTrue(result); + } } From 10d58f07434b2e2fd41a1f9131248ed3f7a50b67 Mon Sep 17 00:00:00 2001 From: Henrique Sato Date: Thu, 21 Mar 2024 19:23:31 -0300 Subject: [PATCH 06/16] Desconsider redundant routers in validation --- .../java/com/cloud/vm/dao/DomainRouterDao.java | 2 ++ .../com/cloud/vm/dao/DomainRouterDaoImpl.java | 16 ++++++++++++++++ .../com/cloud/network/vpc/VpcManagerImpl.java | 2 +- .../cloud/network/vpc/VpcManagerImplTest.java | 7 ++----- 4 files changed, 21 insertions(+), 6 deletions(-) diff --git a/engine/schema/src/main/java/com/cloud/vm/dao/DomainRouterDao.java b/engine/schema/src/main/java/com/cloud/vm/dao/DomainRouterDao.java index 8e965b210beb..778198ede522 100644 --- a/engine/schema/src/main/java/com/cloud/vm/dao/DomainRouterDao.java +++ b/engine/schema/src/main/java/com/cloud/vm/dao/DomainRouterDao.java @@ -165,4 +165,6 @@ public interface DomainRouterDao extends GenericDao { List listStopped(long networkId); List listIncludingRemovedByVpcId(long vpcId); + + List listNoRedundantRouterByVpcId(long vpcId); } diff --git a/engine/schema/src/main/java/com/cloud/vm/dao/DomainRouterDaoImpl.java b/engine/schema/src/main/java/com/cloud/vm/dao/DomainRouterDaoImpl.java index 63cdc042b26f..1a8a0cf13982 100644 --- a/engine/schema/src/main/java/com/cloud/vm/dao/DomainRouterDaoImpl.java +++ b/engine/schema/src/main/java/com/cloud/vm/dao/DomainRouterDaoImpl.java @@ -61,6 +61,7 @@ public class DomainRouterDaoImpl extends GenericDaoBase im protected SearchBuilder OutsidePodSearch; protected SearchBuilder clusterSearch; protected SearchBuilder SearchByStateAndManagementServerId; + protected SearchBuilder VpcNotRedundantRouterSearch; @Inject HostDao _hostsDao; @Inject @@ -98,6 +99,12 @@ protected void init() { VpcSearch.and("vpcId", VpcSearch.entity().getVpcId(), Op.EQ); VpcSearch.done(); + VpcNotRedundantRouterSearch = createSearchBuilder(); + VpcNotRedundantRouterSearch.and("role", VpcNotRedundantRouterSearch.entity().getRole(), Op.EQ); + VpcNotRedundantRouterSearch.and("vpcId", VpcNotRedundantRouterSearch.entity().getVpcId(), Op.EQ); + VpcNotRedundantRouterSearch.and("isRedundantRouter", VpcNotRedundantRouterSearch.entity().getIsRedundantRouter(), Op.EQ); + VpcNotRedundantRouterSearch.done(); + IdNetworkIdStatesSearch = createSearchBuilder(); IdNetworkIdStatesSearch.and("id", IdNetworkIdStatesSearch.entity().getId(), Op.EQ); final SearchBuilder joinRouterNetwork1 = _routerNetworkDao.createSearchBuilder(); @@ -450,4 +457,13 @@ public List listIncludingRemovedByVpcId(long vpcId) { sc.setParameters("role", Role.VIRTUAL_ROUTER); return listIncludingRemovedBy(sc); } + + @Override + public List listNoRedundantRouterByVpcId(final long vpcId) { + final SearchCriteria sc = VpcNotRedundantRouterSearch.create(); + sc.setParameters("vpcId", vpcId); + sc.setParameters("role", Role.VIRTUAL_ROUTER); + sc.setParameters("isRedundantRouter", false); + return listBy(sc); + } } diff --git a/server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java b/server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java index 9fad8ffc56fe..0c2bea6c4c0b 100644 --- a/server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java +++ b/server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java @@ -1998,7 +1998,7 @@ public void doInTransactionWithoutResult(final TransactionStatus status) { protected boolean existsVpcDomainRouterWithSufficientNicCapacity(long vpcId) { int countRouterDefaultNics = 2; long countVpcNetworks = _ntwkDao.countVpcNetworks(vpcId); - List vpcDomainRoutersVO = routerDao.listByVpcId(vpcId); + List vpcDomainRoutersVO = routerDao.listNoRedundantRouterByVpcId(vpcId); int totalNicsAvailable = 0; for (DomainRouterVO domainRouter : vpcDomainRoutersVO) { diff --git a/server/src/test/java/com/cloud/network/vpc/VpcManagerImplTest.java b/server/src/test/java/com/cloud/network/vpc/VpcManagerImplTest.java index 1461d7feb15e..8869eb1f1e1d 100644 --- a/server/src/test/java/com/cloud/network/vpc/VpcManagerImplTest.java +++ b/server/src/test/java/com/cloud/network/vpc/VpcManagerImplTest.java @@ -366,9 +366,6 @@ public void testCreateVpcNetwork() throws InsufficientCapacityException, Resourc Mockito.when(networkOfferingServiceMapDao.listByNetworkOfferingId(anyLong())).thenReturn(serviceMap); Mockito.when(vpcMock.getCidr()).thenReturn("10.0.0.0/8"); Mockito.when(vpcMock.getNetworkDomain()).thenReturn("cs1cloud.internal"); - Mockito.when(routerDao.listByVpcId(VPC_ID)).thenReturn(List.of(domainRouter1VOMock)); - Mockito.when(networkOrchestrationServiceMock.getVirtualMachineMaxNicsValue(domainRouter1VOMock)).thenReturn(3); - Mockito.when(networkDao.countVpcNetworks(VPC_ID)).thenReturn(1L); Mockito.when(manager.existsVpcDomainRouterWithSufficientNicCapacity(VPC_ID)).thenReturn(true); manager.validateNewVpcGuestNetwork("10.10.10.0/24", "10.10.10.1", accountMock, vpcMock, "cs1cloud.internal"); @@ -525,7 +522,7 @@ public void validateVpcPrivateGatewayTestAclFromDifferentVpcThrowsInvalidParamet @Test public void existsVpcDomainRouterWithSufficientNicCapacityTestUnavailableRoutersReturnsFalse() { Mockito.when(networkDao.countVpcNetworks(vpcId)).thenReturn(8L); - Mockito.when(routerDao.listByVpcId(vpcId)).thenReturn(List.of(domainRouter1VOMock, domainRouter2VOMock)); + Mockito.when(routerDao.listNoRedundantRouterByVpcId(vpcId)).thenReturn(List.of(domainRouter1VOMock, domainRouter2VOMock)); Mockito.when(networkOrchestrationServiceMock.getVirtualMachineMaxNicsValue(domainRouter1VOMock)).thenReturn(6); Mockito.when(networkOrchestrationServiceMock.getVirtualMachineMaxNicsValue(domainRouter2VOMock)).thenReturn(6); @@ -537,7 +534,7 @@ public void existsVpcDomainRouterWithSufficientNicCapacityTestUnavailableRouters @Test public void existsVpcDomainRouterWithSufficientNicCapacityTestAvailableRouterReturnsTrue() { Mockito.when(networkDao.countVpcNetworks(vpcId)).thenReturn(7L); - Mockito.when(routerDao.listByVpcId(vpcId)).thenReturn(List.of(domainRouter1VOMock, domainRouter2VOMock)); + Mockito.when(routerDao.listNoRedundantRouterByVpcId(vpcId)).thenReturn(List.of(domainRouter1VOMock, domainRouter2VOMock)); Mockito.when(networkOrchestrationServiceMock.getVirtualMachineMaxNicsValue(domainRouter1VOMock)).thenReturn(6); Mockito.when(networkOrchestrationServiceMock.getVirtualMachineMaxNicsValue(domainRouter2VOMock)).thenReturn(6); From 88d92eeb8b74a03078f68e95c05a543b2fbfe672 Mon Sep 17 00:00:00 2001 From: Henrique Sato Date: Fri, 22 Mar 2024 17:19:45 -0300 Subject: [PATCH 07/16] Change query --- .../java/com/cloud/vm/dao/DomainRouterDao.java | 2 +- .../com/cloud/vm/dao/DomainRouterDaoImpl.java | 17 ++++++++--------- .../com/cloud/network/vpc/VpcManagerImpl.java | 2 +- .../cloud/network/vpc/VpcManagerImplTest.java | 4 ++-- 4 files changed, 12 insertions(+), 13 deletions(-) diff --git a/engine/schema/src/main/java/com/cloud/vm/dao/DomainRouterDao.java b/engine/schema/src/main/java/com/cloud/vm/dao/DomainRouterDao.java index 778198ede522..9b7109f6f398 100644 --- a/engine/schema/src/main/java/com/cloud/vm/dao/DomainRouterDao.java +++ b/engine/schema/src/main/java/com/cloud/vm/dao/DomainRouterDao.java @@ -166,5 +166,5 @@ public interface DomainRouterDao extends GenericDao { List listIncludingRemovedByVpcId(long vpcId); - List listNoRedundantRouterByVpcId(long vpcId); + List listDistinctRouterByVpcId(long vpcId); } diff --git a/engine/schema/src/main/java/com/cloud/vm/dao/DomainRouterDaoImpl.java b/engine/schema/src/main/java/com/cloud/vm/dao/DomainRouterDaoImpl.java index 1a8a0cf13982..5d79d54f5eb0 100644 --- a/engine/schema/src/main/java/com/cloud/vm/dao/DomainRouterDaoImpl.java +++ b/engine/schema/src/main/java/com/cloud/vm/dao/DomainRouterDaoImpl.java @@ -61,7 +61,7 @@ public class DomainRouterDaoImpl extends GenericDaoBase im protected SearchBuilder OutsidePodSearch; protected SearchBuilder clusterSearch; protected SearchBuilder SearchByStateAndManagementServerId; - protected SearchBuilder VpcNotRedundantRouterSearch; + protected SearchBuilder VpcDistinctRouterSearch; @Inject HostDao _hostsDao; @Inject @@ -99,11 +99,11 @@ protected void init() { VpcSearch.and("vpcId", VpcSearch.entity().getVpcId(), Op.EQ); VpcSearch.done(); - VpcNotRedundantRouterSearch = createSearchBuilder(); - VpcNotRedundantRouterSearch.and("role", VpcNotRedundantRouterSearch.entity().getRole(), Op.EQ); - VpcNotRedundantRouterSearch.and("vpcId", VpcNotRedundantRouterSearch.entity().getVpcId(), Op.EQ); - VpcNotRedundantRouterSearch.and("isRedundantRouter", VpcNotRedundantRouterSearch.entity().getIsRedundantRouter(), Op.EQ); - VpcNotRedundantRouterSearch.done(); + VpcDistinctRouterSearch = createSearchBuilder(); + VpcDistinctRouterSearch.and("role", VpcDistinctRouterSearch.entity().getRole(), Op.EQ); + VpcDistinctRouterSearch.and("vpcId", VpcDistinctRouterSearch.entity().getVpcId(), Op.EQ); + VpcDistinctRouterSearch.groupBy(VpcDistinctRouterSearch.entity().getPublicMacAddress()); + VpcDistinctRouterSearch.done(); IdNetworkIdStatesSearch = createSearchBuilder(); IdNetworkIdStatesSearch.and("id", IdNetworkIdStatesSearch.entity().getId(), Op.EQ); @@ -459,11 +459,10 @@ public List listIncludingRemovedByVpcId(long vpcId) { } @Override - public List listNoRedundantRouterByVpcId(final long vpcId) { - final SearchCriteria sc = VpcNotRedundantRouterSearch.create(); + public List listDistinctRouterByVpcId(final long vpcId) { + final SearchCriteria sc = VpcDistinctRouterSearch.create(); sc.setParameters("vpcId", vpcId); sc.setParameters("role", Role.VIRTUAL_ROUTER); - sc.setParameters("isRedundantRouter", false); return listBy(sc); } } diff --git a/server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java b/server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java index 0c2bea6c4c0b..e4c6aec65403 100644 --- a/server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java +++ b/server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java @@ -1998,7 +1998,7 @@ public void doInTransactionWithoutResult(final TransactionStatus status) { protected boolean existsVpcDomainRouterWithSufficientNicCapacity(long vpcId) { int countRouterDefaultNics = 2; long countVpcNetworks = _ntwkDao.countVpcNetworks(vpcId); - List vpcDomainRoutersVO = routerDao.listNoRedundantRouterByVpcId(vpcId); + List vpcDomainRoutersVO = routerDao.listDistinctRouterByVpcId(vpcId); int totalNicsAvailable = 0; for (DomainRouterVO domainRouter : vpcDomainRoutersVO) { diff --git a/server/src/test/java/com/cloud/network/vpc/VpcManagerImplTest.java b/server/src/test/java/com/cloud/network/vpc/VpcManagerImplTest.java index 8869eb1f1e1d..ddbb755e438d 100644 --- a/server/src/test/java/com/cloud/network/vpc/VpcManagerImplTest.java +++ b/server/src/test/java/com/cloud/network/vpc/VpcManagerImplTest.java @@ -522,7 +522,7 @@ public void validateVpcPrivateGatewayTestAclFromDifferentVpcThrowsInvalidParamet @Test public void existsVpcDomainRouterWithSufficientNicCapacityTestUnavailableRoutersReturnsFalse() { Mockito.when(networkDao.countVpcNetworks(vpcId)).thenReturn(8L); - Mockito.when(routerDao.listNoRedundantRouterByVpcId(vpcId)).thenReturn(List.of(domainRouter1VOMock, domainRouter2VOMock)); + Mockito.when(routerDao.listDistinctRouterByVpcId(vpcId)).thenReturn(List.of(domainRouter1VOMock, domainRouter2VOMock)); Mockito.when(networkOrchestrationServiceMock.getVirtualMachineMaxNicsValue(domainRouter1VOMock)).thenReturn(6); Mockito.when(networkOrchestrationServiceMock.getVirtualMachineMaxNicsValue(domainRouter2VOMock)).thenReturn(6); @@ -534,7 +534,7 @@ public void existsVpcDomainRouterWithSufficientNicCapacityTestUnavailableRouters @Test public void existsVpcDomainRouterWithSufficientNicCapacityTestAvailableRouterReturnsTrue() { Mockito.when(networkDao.countVpcNetworks(vpcId)).thenReturn(7L); - Mockito.when(routerDao.listNoRedundantRouterByVpcId(vpcId)).thenReturn(List.of(domainRouter1VOMock, domainRouter2VOMock)); + Mockito.when(routerDao.listDistinctRouterByVpcId(vpcId)).thenReturn(List.of(domainRouter1VOMock, domainRouter2VOMock)); Mockito.when(networkOrchestrationServiceMock.getVirtualMachineMaxNicsValue(domainRouter1VOMock)).thenReturn(6); Mockito.when(networkOrchestrationServiceMock.getVirtualMachineMaxNicsValue(domainRouter2VOMock)).thenReturn(6); From 63ab94aaec84a7f6aa4c16f1112a3614d4d0279d Mon Sep 17 00:00:00 2001 From: Henrique Sato Date: Tue, 26 Mar 2024 16:00:32 -0300 Subject: [PATCH 08/16] Compare single VR --- .../com/cloud/vm/dao/DomainRouterDao.java | 2 +- .../com/cloud/vm/dao/DomainRouterDaoImpl.java | 18 ++++++------ .../com/cloud/network/vpc/VpcManagerImpl.java | 9 +++--- .../cloud/network/vpc/VpcManagerImplTest.java | 28 +++++++++++-------- 4 files changed, 32 insertions(+), 25 deletions(-) diff --git a/engine/schema/src/main/java/com/cloud/vm/dao/DomainRouterDao.java b/engine/schema/src/main/java/com/cloud/vm/dao/DomainRouterDao.java index 9b7109f6f398..1191c4aba348 100644 --- a/engine/schema/src/main/java/com/cloud/vm/dao/DomainRouterDao.java +++ b/engine/schema/src/main/java/com/cloud/vm/dao/DomainRouterDao.java @@ -166,5 +166,5 @@ public interface DomainRouterDao extends GenericDao { List listIncludingRemovedByVpcId(long vpcId); - List listDistinctRouterByVpcId(long vpcId); + DomainRouterVO findOneByVpcId(long vpcId); } diff --git a/engine/schema/src/main/java/com/cloud/vm/dao/DomainRouterDaoImpl.java b/engine/schema/src/main/java/com/cloud/vm/dao/DomainRouterDaoImpl.java index 5d79d54f5eb0..b4f04a3c47f3 100644 --- a/engine/schema/src/main/java/com/cloud/vm/dao/DomainRouterDaoImpl.java +++ b/engine/schema/src/main/java/com/cloud/vm/dao/DomainRouterDaoImpl.java @@ -61,7 +61,7 @@ public class DomainRouterDaoImpl extends GenericDaoBase im protected SearchBuilder OutsidePodSearch; protected SearchBuilder clusterSearch; protected SearchBuilder SearchByStateAndManagementServerId; - protected SearchBuilder VpcDistinctRouterSearch; + protected SearchBuilder VpcSingleRouterSearch; @Inject HostDao _hostsDao; @Inject @@ -99,11 +99,11 @@ protected void init() { VpcSearch.and("vpcId", VpcSearch.entity().getVpcId(), Op.EQ); VpcSearch.done(); - VpcDistinctRouterSearch = createSearchBuilder(); - VpcDistinctRouterSearch.and("role", VpcDistinctRouterSearch.entity().getRole(), Op.EQ); - VpcDistinctRouterSearch.and("vpcId", VpcDistinctRouterSearch.entity().getVpcId(), Op.EQ); - VpcDistinctRouterSearch.groupBy(VpcDistinctRouterSearch.entity().getPublicMacAddress()); - VpcDistinctRouterSearch.done(); + VpcSingleRouterSearch = createSearchBuilder(); + VpcSingleRouterSearch.and("role", VpcSingleRouterSearch.entity().getRole(), Op.EQ); + VpcSingleRouterSearch.and("vpcId", VpcSingleRouterSearch.entity().getVpcId(), Op.EQ); + VpcSingleRouterSearch.groupBy(VpcSingleRouterSearch.entity().getVpcId()); + VpcSingleRouterSearch.done(); IdNetworkIdStatesSearch = createSearchBuilder(); IdNetworkIdStatesSearch.and("id", IdNetworkIdStatesSearch.entity().getId(), Op.EQ); @@ -459,10 +459,10 @@ public List listIncludingRemovedByVpcId(long vpcId) { } @Override - public List listDistinctRouterByVpcId(final long vpcId) { - final SearchCriteria sc = VpcDistinctRouterSearch.create(); + public DomainRouterVO findOneByVpcId(final long vpcId) { + final SearchCriteria sc = VpcSingleRouterSearch.create(); sc.setParameters("vpcId", vpcId); sc.setParameters("role", Role.VIRTUAL_ROUTER); - return listBy(sc); + return findOneBy(sc); } } diff --git a/server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java b/server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java index e4c6aec65403..b4620c98ec21 100644 --- a/server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java +++ b/server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java @@ -1998,13 +1998,14 @@ public void doInTransactionWithoutResult(final TransactionStatus status) { protected boolean existsVpcDomainRouterWithSufficientNicCapacity(long vpcId) { int countRouterDefaultNics = 2; long countVpcNetworks = _ntwkDao.countVpcNetworks(vpcId); - List vpcDomainRoutersVO = routerDao.listDistinctRouterByVpcId(vpcId); - int totalNicsAvailable = 0; + DomainRouterVO vpcDomainRouter = routerDao.findOneByVpcId(vpcId); - for (DomainRouterVO domainRouter : vpcDomainRoutersVO) { - totalNicsAvailable += networkOrchestrationService.getVirtualMachineMaxNicsValue(domainRouter) - countRouterDefaultNics; + if (vpcDomainRouter == null) { + return false; } + int totalNicsAvailable = networkOrchestrationService.getVirtualMachineMaxNicsValue(vpcDomainRouter) - countRouterDefaultNics; + return totalNicsAvailable > countVpcNetworks; } diff --git a/server/src/test/java/com/cloud/network/vpc/VpcManagerImplTest.java b/server/src/test/java/com/cloud/network/vpc/VpcManagerImplTest.java index ddbb755e438d..508eceaa05a6 100644 --- a/server/src/test/java/com/cloud/network/vpc/VpcManagerImplTest.java +++ b/server/src/test/java/com/cloud/network/vpc/VpcManagerImplTest.java @@ -160,9 +160,7 @@ public class VpcManagerImplTest { @Mock NetworkOrchestrationService networkOrchestrationServiceMock; @Mock - DomainRouterVO domainRouter1VOMock; - @Mock - DomainRouterVO domainRouter2VOMock; + DomainRouterVO domainRouterVOMock; public static final long ACCOUNT_ID = 1; private AccountVO account; @@ -521,10 +519,9 @@ public void validateVpcPrivateGatewayTestAclFromDifferentVpcThrowsInvalidParamet @Test public void existsVpcDomainRouterWithSufficientNicCapacityTestUnavailableRoutersReturnsFalse() { - Mockito.when(networkDao.countVpcNetworks(vpcId)).thenReturn(8L); - Mockito.when(routerDao.listDistinctRouterByVpcId(vpcId)).thenReturn(List.of(domainRouter1VOMock, domainRouter2VOMock)); - Mockito.when(networkOrchestrationServiceMock.getVirtualMachineMaxNicsValue(domainRouter1VOMock)).thenReturn(6); - Mockito.when(networkOrchestrationServiceMock.getVirtualMachineMaxNicsValue(domainRouter2VOMock)).thenReturn(6); + Mockito.when(networkDao.countVpcNetworks(vpcId)).thenReturn(7L); + Mockito.when(routerDao.findOneByVpcId(vpcId)).thenReturn(domainRouterVOMock); + Mockito.when(networkOrchestrationServiceMock.getVirtualMachineMaxNicsValue(domainRouterVOMock)).thenReturn(9); boolean result = manager.existsVpcDomainRouterWithSufficientNicCapacity(vpcId); @@ -533,13 +530,22 @@ public void existsVpcDomainRouterWithSufficientNicCapacityTestUnavailableRouters @Test public void existsVpcDomainRouterWithSufficientNicCapacityTestAvailableRouterReturnsTrue() { - Mockito.when(networkDao.countVpcNetworks(vpcId)).thenReturn(7L); - Mockito.when(routerDao.listDistinctRouterByVpcId(vpcId)).thenReturn(List.of(domainRouter1VOMock, domainRouter2VOMock)); - Mockito.when(networkOrchestrationServiceMock.getVirtualMachineMaxNicsValue(domainRouter1VOMock)).thenReturn(6); - Mockito.when(networkOrchestrationServiceMock.getVirtualMachineMaxNicsValue(domainRouter2VOMock)).thenReturn(6); + Mockito.when(networkDao.countVpcNetworks(vpcId)).thenReturn(6L); + Mockito.when(routerDao.findOneByVpcId(vpcId)).thenReturn(domainRouterVOMock); + Mockito.when(networkOrchestrationServiceMock.getVirtualMachineMaxNicsValue(domainRouterVOMock)).thenReturn(9); boolean result = manager.existsVpcDomainRouterWithSufficientNicCapacity(vpcId); Assert.assertTrue(result); } + + @Test + public void existsVpcDomainRouterWithSufficientNicCapacityTestNullRouterReturnsFalse() { + Mockito.when(networkDao.countVpcNetworks(vpcId)).thenReturn(6L); + Mockito.when(routerDao.findOneByVpcId(vpcId)).thenReturn(null); + + boolean result = manager.existsVpcDomainRouterWithSufficientNicCapacity(vpcId); + + Assert.assertFalse(result); + } } From b9c4142eab65de769abf79cc82cd1a0462b8dfb3 Mon Sep 17 00:00:00 2001 From: Henrique Sato Date: Tue, 9 Apr 2024 18:36:28 -0300 Subject: [PATCH 09/16] Refactor code --- .../orchestration/NetworkOrchestrator.java | 14 ++- .../NetworkOrchestratorTest.java | 26 +---- .../com/cloud/network/vpc/VpcManagerImpl.java | 107 ++++++++++-------- .../cloud/network/vpc/VpcManagerImplTest.java | 58 +++++++++- 4 files changed, 135 insertions(+), 70 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 36a6b268c5fe..aaeedf1fdc57 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 @@ -1115,7 +1115,7 @@ public int getVirtualMachineMaxNicsValue(VirtualMachine virtualMachine) { } /** - * Searches the maximum virtual machine NICs based on the hypervisor type of the cluster where the instance is deployed. + * Searches the maximum virtual machine NICs based on the cluster where the instance is deployed. * * @param virtualMachine Virtual machine to get the cluster. * @return The maximum number of NICs that the virtual machine can have. @@ -1131,7 +1131,17 @@ protected Integer getVirtualMachineMaxNicsValueFromCluster(VirtualMachine virtua return null; } - int virtualMachineMaxNicsValue; + return getVirtualMachineMaxNicsValueFromCluster(cluster); + } + + /** + * Searches the maximum virtual machine NICs based on the hypervisor type of the cluster where the instance is deployed. + * + * @param cluster Cluster to get the virtual machine max NICs value from. + * @return The maximum number of NICs that the virtual machine can have. + */ + protected int getVirtualMachineMaxNicsValueFromCluster(ClusterVO cluster) { + int virtualMachineMaxNicsValue = 0; HypervisorType hypervisor = cluster.getHypervisorType(); if (HypervisorType.KVM.equals(hypervisor)) { 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 e88b56444703..330aaac261e0 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 @@ -857,7 +857,7 @@ public void getVirtualMachineMaxNicsValueTestVirtualMachineDeployedReturnsVirtua int virtualMachineMaxNicsValue = testOrchastrator.getVirtualMachineMaxNicsValue(virtualMachineMock); - Mockito.verify(testOrchastrator, Mockito.times(1)).getVirtualMachineMaxNicsValueFromCluster(Mockito.any()); + Mockito.verify(testOrchastrator, Mockito.times(1)).getVirtualMachineMaxNicsValueFromCluster((VirtualMachine) Mockito.any()); Assert.assertEquals(44, virtualMachineMaxNicsValue); } @@ -925,60 +925,42 @@ public void getVirtualMachineMaxNicsValueFromClusterTestClusterDoesNotExistRetur @Test public void getVirtualMachineMaxNicsValueFromClusterTestKvmClusterReturnsVirtualMachineMaxNicsKvmClusterValue() throws NoSuchFieldException, IllegalAccessException { - VirtualMachine virtualMachineMock = Mockito.mock(VirtualMachine.class); - HostVO hostVoMock = Mockito.mock(HostVO.class); ClusterVO clusterVoMock = Mockito.mock(ClusterVO.class); ConfigKey virtualMachineMaxNicsKvmMock = Mockito.mock(ConfigKey.class); - Mockito.doReturn(1L).when(virtualMachineMock).getHostId(); - Mockito.doReturn(hostVoMock).when(testOrchastrator._hostDao).findById(1L); - Mockito.doReturn(1L).when(hostVoMock).getClusterId(); - Mockito.doReturn(clusterVoMock).when(testOrchastrator.clusterDao).findById(1L); Mockito.doReturn(1L).when(clusterVoMock).getId(); Mockito.doReturn(Hypervisor.HypervisorType.KVM).when(clusterVoMock).getHypervisorType(); Mockito.doReturn(33).when(virtualMachineMaxNicsKvmMock).valueIn(1L); updateFinalStaticField(testOrchastrator.getClass().getField("VirtualMachineMaxNicsKvm"), virtualMachineMaxNicsKvmMock); - int virtualMachineMaxNicsValue = testOrchastrator.getVirtualMachineMaxNicsValueFromCluster(virtualMachineMock); + int virtualMachineMaxNicsValue = testOrchastrator.getVirtualMachineMaxNicsValueFromCluster(clusterVoMock); Assert.assertEquals(33, virtualMachineMaxNicsValue); } @Test public void getVirtualMachineMaxNicsValueFromClusterTestVmwareClusterReturnsVirtualMachineMaxNicsVmwareClusterValue() throws NoSuchFieldException, IllegalAccessException { - VirtualMachine virtualMachineMock = Mockito.mock(VirtualMachine.class); - HostVO hostVoMock = Mockito.mock(HostVO.class); ClusterVO clusterVoMock = Mockito.mock(ClusterVO.class); ConfigKey virtualMachineMaxNicsVmwareMock = Mockito.mock(ConfigKey.class); - Mockito.doReturn(1L).when(virtualMachineMock).getHostId(); - Mockito.doReturn(hostVoMock).when(testOrchastrator._hostDao).findById(1L); - Mockito.doReturn(1L).when(hostVoMock).getClusterId(); - Mockito.doReturn(clusterVoMock).when(testOrchastrator.clusterDao).findById(1L); Mockito.doReturn(1L).when(clusterVoMock).getId(); Mockito.doReturn(Hypervisor.HypervisorType.VMware).when(clusterVoMock).getHypervisorType(); Mockito.doReturn(22).when(virtualMachineMaxNicsVmwareMock).valueIn(1L); updateFinalStaticField(testOrchastrator.getClass().getField("VirtualMachineMaxNicsVmware"), virtualMachineMaxNicsVmwareMock); - int virtualMachineMaxNicsValue = testOrchastrator.getVirtualMachineMaxNicsValueFromCluster(virtualMachineMock); + int virtualMachineMaxNicsValue = testOrchastrator.getVirtualMachineMaxNicsValueFromCluster(clusterVoMock); Assert.assertEquals(22, virtualMachineMaxNicsValue); } @Test public void getVirtualMachineMaxNicsValueFromClusterTestXenserverClusterReturnsVirtualMachineMaxNicsXenserverClusterValue() throws NoSuchFieldException, IllegalAccessException { - VirtualMachine virtualMachineMock = Mockito.mock(VirtualMachine.class); - HostVO hostVoMock = Mockito.mock(HostVO.class); ClusterVO clusterVoMock = Mockito.mock(ClusterVO.class); ConfigKey virtualMachineMaxNicsXenserverMock = Mockito.mock(ConfigKey.class); - Mockito.doReturn(1L).when(virtualMachineMock).getHostId(); - Mockito.doReturn(hostVoMock).when(testOrchastrator._hostDao).findById(1L); - Mockito.doReturn(1L).when(hostVoMock).getClusterId(); - Mockito.doReturn(clusterVoMock).when(testOrchastrator.clusterDao).findById(1L); Mockito.doReturn(1L).when(clusterVoMock).getId(); Mockito.doReturn(Hypervisor.HypervisorType.XenServer).when(clusterVoMock).getHypervisorType(); Mockito.doReturn(11).when(virtualMachineMaxNicsXenserverMock).valueIn(1L); updateFinalStaticField(testOrchastrator.getClass().getField("VirtualMachineMaxNicsXenserver"), virtualMachineMaxNicsXenserverMock); - int virtualMachineMaxNicsValue = testOrchastrator.getVirtualMachineMaxNicsValueFromCluster(virtualMachineMock); + int virtualMachineMaxNicsValue = testOrchastrator.getVirtualMachineMaxNicsValueFromCluster(clusterVoMock); Assert.assertEquals(11, virtualMachineMaxNicsValue); } diff --git a/server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java b/server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java index b4620c98ec21..163cd449888b 100644 --- a/server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java +++ b/server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java @@ -278,7 +278,6 @@ public class VpcManagerImpl extends ManagerBase implements VpcManager, VpcProvis Provider.JuniperContrailVpcRouter, Provider.Ovs, Provider.BigSwitchBcf, Provider.ConfigDrive, Provider.Nsx); int _cleanupInterval; - int _maxNetworks; SearchBuilder IpAddressSearch; protected final List hTypes = new ArrayList(); @@ -416,8 +415,6 @@ public void doInTransactionWithoutResult(final TransactionStatus status) { final String value = configs.get(Config.VpcCleanupInterval.key()); _cleanupInterval = NumbersUtil.parseInt(value, 60 * 60); // 1 hour - _maxNetworks = VpcMaxNetworks.value(); - IpAddressSearch = _ipAddressDao.createSearchBuilder(); IpAddressSearch.and("accountId", IpAddressSearch.entity().getAllocatedToAccountId(), Op.EQ); IpAddressSearch.and("dataCenterId", IpAddressSearch.entity().getDataCenterId(), Op.EQ); @@ -1929,7 +1926,6 @@ public void validateNtwkOffForVpc(final NetworkOffering guestNtwkOff, final List @DB protected void validateNewVpcGuestNetwork(final String cidr, final String gateway, final Account networkOwner, final Vpc vpc, final String networkDomain) { - Transaction.execute(new TransactionCallbackNoReturn() { @Override public void doInTransactionWithoutResult(final TransactionStatus status) { @@ -1939,56 +1935,29 @@ public void doInTransactionWithoutResult(final TransactionStatus status) { throw new CloudRuntimeException("Unable to acquire lock on " + vpc); } - _maxNetworks = VpcMaxNetworks.valueIn(vpc.getAccountId()); - Account account = _accountMgr.getAccount(vpc.getAccountId()); - try { - // check number of active networks in vpc - if (_ntwkDao.countVpcNetworks(vpcId) >= _maxNetworks) { - logger.warn("Failed to create a new VPC Guest Network because the number of networks per VPC has reached its maximum capacity of {}. Increase it by modifying global or account config {}.", _maxNetworks, VpcMaxNetworks); - throw new CloudRuntimeException(String.format("Number of networks per VPC cannot surpass [%s] for account [%s].", _maxNetworks, account.getAccountName())); - } + Account account = _accountMgr.getAccount(vpc.getAccountId()); + int maxNetworks = VpcMaxNetworks.valueIn(vpc.getAccountId()); - if (!existsVpcDomainRouterWithSufficientNicCapacity(vpc.getId())) { - logger.warn("Failed to create a new VPC Guest Network because no virtual routers were found with sufficient NIC capacity. The number of VPC Guest networks cannot exceed the number of NICs a virtual router can have."); - throw new CloudRuntimeException(String.format("No available virtual router found to deploy new Guest Network on VPC [%s].", vpc.getName())); - } + checkIfVpcNumberOfTiersIsNotExceeded(vpcId, maxNetworks, account); + + checkIfVpcHasDomainRouterWithSufficientNicCapacity(vpc); - // 1) CIDR is required if (cidr == null) { throw new InvalidParameterValueException("Gateway/netmask are required when create network for VPC"); } - // 2) Network cidr should be within vpcCidr - if (!NetUtils.isNetworkAWithinNetworkB(cidr, vpc.getCidr())) { - throw new InvalidParameterValueException("Network cidr " + cidr + " is not within vpc " + vpc + " cidr"); - } - - // 3) Network cidr shouldn't cross the cidr of other vpc - // network cidrs - final List ntwks = _ntwkDao.listByVpc(vpcId); - for (final Network ntwk : ntwks) { - assert cidr != null : "Why the network cidr is null when it belongs to vpc?"; + checkIfNetworkCidrIsWithinVpcCidr(cidr, vpc); - if (NetUtils.isNetworkAWithinNetworkB(ntwk.getCidr(), cidr) || NetUtils.isNetworkAWithinNetworkB(cidr, ntwk.getCidr())) { - throw new InvalidParameterValueException("Network cidr " + cidr + " crosses other network cidr " + ntwk + " belonging to the same vpc " + vpc); - } - } + checkIfNetworkCidrNotCrossesOtherVpcNetworksCidr(cidr, vpc); - // 4) Vpc's account should be able to access network owner's account - CheckAccountsAccess(vpc, networkOwner); + checkAccountsAccess(vpc, networkOwner); - // 5) network domain should be the same as VPC's - if (!networkDomain.equalsIgnoreCase(vpc.getNetworkDomain())) { - throw new InvalidParameterValueException("Network domain of the new network should match network" + " domain of vpc " + vpc); - } + checkIfNetworkAndVpcDomainsAreTheSame(networkDomain, vpc); - // 6) gateway should never be equal to the cidr subnet - if (NetUtils.getCidrSubNet(cidr).equalsIgnoreCase(gateway)) { - throw new InvalidParameterValueException("Invalid gateway specified. It should never be equal to the cidr subnet value"); - } + checkIfGatewayIsDifferentFromCidrSubnet(cidr, gateway); } finally { - logger.debug("Releasing lock for " + locked); + logger.debug("Releasing lock for {}.", locked); vpcDao.releaseFromLockTable(locked.getId()); } } @@ -2009,10 +1978,46 @@ protected boolean existsVpcDomainRouterWithSufficientNicCapacity(long vpcId) { return totalNicsAvailable > countVpcNetworks; } - private void CheckAccountsAccess(Vpc vpc, Account networkAccount) { - Account vpcaccount = _accountMgr.getAccount(vpc.getAccountId()); + protected void checkIfVpcNumberOfTiersIsNotExceeded(long vpcId, int maxNetworks, Account account) { + if (_ntwkDao.countVpcNetworks(vpcId) >= maxNetworks) { + logger.warn("Failed to create a new VPC Guest Network because the number of networks per VPC has reached its maximum capacity of {}. Increase it by modifying global or account config {}.", maxNetworks, VpcMaxNetworks); + throw new CloudRuntimeException(String.format("Number of networks per VPC cannot surpass [%s] for account [%s].", maxNetworks, account.getAccountName())); + } + } + + protected void checkIfVpcHasDomainRouterWithSufficientNicCapacity(Vpc vpc) { + if (!existsVpcDomainRouterWithSufficientNicCapacity(vpc.getId())) { + logger.warn("Failed to create a new VPC Guest Network because no virtual routers were found with sufficient NIC capacity. The number of VPC Guest networks cannot exceed the number of NICs a virtual router can have."); + throw new CloudRuntimeException(String.format("No available virtual router found to deploy new Guest Network on VPC [%s].", vpc.getName())); + } + } + + protected void checkIfNetworkCidrIsWithinVpcCidr(String cidr, Vpc vpc) { + if (!NetUtils.isNetworkAWithinNetworkB(cidr, vpc.getCidr())) { + throw new InvalidParameterValueException("Network cidr " + cidr + " is not within vpc " + vpc + " cidr"); + } + } + + protected void checkIfNetworkCidrNotCrossesOtherVpcNetworksCidr(String cidr, Vpc vpc) { + final List networks = _ntwkDao.listByVpc(vpc.getId()); + + for (final Network network : networks) { + if (NetUtils.isNetworkAWithinNetworkB(network.getCidr(), cidr) || NetUtils.isNetworkAWithinNetworkB(cidr, network.getCidr())) { + throw new InvalidParameterValueException("Network cidr " + cidr + " crosses other network cidr " + network + " belonging to the same vpc " + vpc); + } + } + } + + /** + * Checks if the VPC account has access to the account for which the tier is being created. + * + * @param vpc Vpc to get the account. + * @param networkAccount Tier owner account. + */ + private void checkAccountsAccess(Vpc vpc, Account networkAccount) { + Account vpcAccount = _accountMgr.getAccount(vpc.getAccountId()); try { - _accountMgr.checkAccess(vpcaccount, null, false, networkAccount); + _accountMgr.checkAccess(vpcAccount, null, false, networkAccount); } catch (PermissionDeniedException e) { logger.error(e.getMessage()); @@ -2020,6 +2025,18 @@ private void CheckAccountsAccess(Vpc vpc, Account networkAccount) { } } + protected void checkIfNetworkAndVpcDomainsAreTheSame(String networkDomain, Vpc vpc) { + if (!networkDomain.equalsIgnoreCase(vpc.getNetworkDomain())) { + throw new InvalidParameterValueException(String.format("Network domain of the new network should match VPC domain [%s].", vpc)); + } + } + + protected void checkIfGatewayIsDifferentFromCidrSubnet(String cidr, String gateway) { + if (NetUtils.getCidrSubNet(cidr).equalsIgnoreCase(gateway)) { + throw new InvalidParameterValueException("Invalid gateway specified. It should never be equal to the CIDR subnet value."); + } + } + public List getVpcElements() { if (vpcElements == null) { vpcElements = new ArrayList(); diff --git a/server/src/test/java/com/cloud/network/vpc/VpcManagerImplTest.java b/server/src/test/java/com/cloud/network/vpc/VpcManagerImplTest.java index 508eceaa05a6..e9f82d32d3e8 100644 --- a/server/src/test/java/com/cloud/network/vpc/VpcManagerImplTest.java +++ b/server/src/test/java/com/cloud/network/vpc/VpcManagerImplTest.java @@ -62,6 +62,7 @@ import com.cloud.user.UserVO; import com.cloud.utils.Pair; import com.cloud.utils.db.EntityManager; +import com.cloud.utils.exception.CloudRuntimeException; import com.cloud.utils.net.Ip; import com.cloud.utils.net.NetUtils; import com.cloud.vm.DomainRouterVO; @@ -85,6 +86,7 @@ import org.mockito.junit.MockitoJUnitRunner; import org.springframework.test.util.ReflectionTestUtils; +import javax.management.InvalidApplicationException; import java.lang.reflect.Field; import java.util.ArrayList; import java.util.HashMap; @@ -161,6 +163,8 @@ public class VpcManagerImplTest { NetworkOrchestrationService networkOrchestrationServiceMock; @Mock DomainRouterVO domainRouterVOMock; + @Mock + Vpc vpcMock; public static final long ACCOUNT_ID = 1; private AccountVO account; @@ -342,7 +346,6 @@ protected Set prepareVpcManagerForCheckingCapabilityPerService @Test public void testCreateVpcNetwork() throws InsufficientCapacityException, ResourceAllocationException { final long VPC_ID = 201L; - manager._maxNetworks = 3; VpcVO vpcMockVO = Mockito.mock(VpcVO.class); Vpc vpcMock = Mockito.mock(Vpc.class); Account accountMock = Mockito.mock(Account.class); @@ -548,4 +551,57 @@ public void existsVpcDomainRouterWithSufficientNicCapacityTestNullRouterReturnsF Assert.assertFalse(result); } + + @Test + public void checkIfVpcNumberOfTiersIsNotExceededTestExceededTiersThrowCloudRuntimeException() { + int maxNetworks = 3; + AccountVO accountMock = Mockito.mock(AccountVO.class); + Mockito.doReturn(5L).when(networkDao).countVpcNetworks(1L); + + Assert.assertThrows(CloudRuntimeException.class, () -> manager.checkIfVpcNumberOfTiersIsNotExceeded(vpcId, maxNetworks, accountMock)); + } + + @Test + public void checkIfVpcHasDomainRouterWithSufficientNicCapacityTestDomainRoutersWithoutCapacityThrowsCloudRuntimeException() { + Mockito.doReturn(vpcId).when(vpcMock).getId(); + Mockito.doReturn(false).when(manager).existsVpcDomainRouterWithSufficientNicCapacity(vpcId); + + Assert.assertThrows(CloudRuntimeException.class, () -> manager.checkIfVpcHasDomainRouterWithSufficientNicCapacity(vpcMock)); + } + + @Test + public void checkIfNetworkCidrIsWithinVpcCidrTestNetworkCidrOutsideOfVpcCidrThrowInvalidParameterValueException() { + String cidr = "10.0.0.1/24"; + Mockito.doReturn("192.168.0.0/24").when(vpcMock).getCidr(); + + Assert.assertThrows(InvalidParameterValueException.class, () -> manager.checkIfNetworkCidrIsWithinVpcCidr(cidr, vpcMock)); + } + + @Test + public void checkIfNetworkCidrNotCrossesOtherVpcNetworksCidrNetworkCidrCrossesOtherVpcNetworkCidr() { + String cidr = "192.168.0.0/24"; + Network networkMock = Mockito.mock(Network.class); + Mockito.doReturn("192.168.0.0/24").when(networkMock).getCidr(); + Mockito.doReturn(vpcId).when(vpcMock).getId(); + Mockito.doReturn(List.of(networkMock)).when(networkDao).listByVpc(vpcId); + + Assert.assertThrows(InvalidParameterValueException.class, () -> manager.checkIfNetworkCidrNotCrossesOtherVpcNetworksCidr(cidr, vpcMock)); + } + + @Test + public void checkIfNetworkAndVpcDomainsAreTheSameTestDifferentDomainsThrowInvalidParameterValueException() { + String domain = "domain"; + Mockito.doReturn("anotherDomain").when(vpcMock).getNetworkDomain(); + + Assert.assertThrows(InvalidParameterValueException.class, () -> manager.checkIfNetworkAndVpcDomainsAreTheSame(domain, vpcMock)); + } + + @Test + public void checkIfGatewayIsDifferentFromCidrSubnetTestGatewayEqualsCidrSubnetThrowInvalidParameterValueException() { + String gateway = "192.168.0.0"; + String cidr = "192.168.0.1/24"; + + Assert.assertThrows(InvalidParameterValueException.class, () -> manager.checkIfGatewayIsDifferentFromCidrSubnet(cidr, gateway)); + } + } From e86f28b9023a194b5e3e37d8549a4e5fe74d6982 Mon Sep 17 00:00:00 2001 From: Henrique Sato Date: Wed, 10 Apr 2024 15:42:43 -0300 Subject: [PATCH 10/16] Remove unused import & improve VR NIC validation --- .../java/com/cloud/vm/dao/NicDaoImpl.java | 2 ++ .../api/query/dao/DomainRouterJoinDao.java | 2 ++ .../query/dao/DomainRouterJoinDaoImpl.java | 18 +++++++++++++++- .../com/cloud/network/vpc/VpcManagerImpl.java | 21 ++++++++++++------- .../cloud/network/vpc/VpcManagerImplTest.java | 10 +++++++-- 5 files changed, 42 insertions(+), 11 deletions(-) diff --git a/engine/schema/src/main/java/com/cloud/vm/dao/NicDaoImpl.java b/engine/schema/src/main/java/com/cloud/vm/dao/NicDaoImpl.java index 3eee1d4e749d..7f266edd5015 100644 --- a/engine/schema/src/main/java/com/cloud/vm/dao/NicDaoImpl.java +++ b/engine/schema/src/main/java/com/cloud/vm/dao/NicDaoImpl.java @@ -107,6 +107,8 @@ protected void init() { PeerRouterSearch.and("macAddress", PeerRouterSearch.entity().getMacAddress(), Op.EQ); PeerRouterSearch.and("vmType", PeerRouterSearch.entity().getVmType(), Op.EQ); PeerRouterSearch.done(); + + } @Override diff --git a/server/src/main/java/com/cloud/api/query/dao/DomainRouterJoinDao.java b/server/src/main/java/com/cloud/api/query/dao/DomainRouterJoinDao.java index 509d02dd71c5..feb608797cdc 100644 --- a/server/src/main/java/com/cloud/api/query/dao/DomainRouterJoinDao.java +++ b/server/src/main/java/com/cloud/api/query/dao/DomainRouterJoinDao.java @@ -37,4 +37,6 @@ public interface DomainRouterJoinDao extends GenericDao searchByIds(Long... ids); List getRouterByIdAndTrafficType(Long id, Networks.TrafficType... trafficType); + + int countDefaultNetworksById(long routerId); } diff --git a/server/src/main/java/com/cloud/api/query/dao/DomainRouterJoinDaoImpl.java b/server/src/main/java/com/cloud/api/query/dao/DomainRouterJoinDaoImpl.java index c6041c3e3732..53b5758389b3 100644 --- a/server/src/main/java/com/cloud/api/query/dao/DomainRouterJoinDaoImpl.java +++ b/server/src/main/java/com/cloud/api/query/dao/DomainRouterJoinDaoImpl.java @@ -21,6 +21,7 @@ import javax.inject.Inject; +import com.cloud.utils.db.GenericSearchBuilder; import org.apache.cloudstack.annotation.AnnotationService; import org.apache.cloudstack.annotation.dao.AnnotationDao; import org.apache.cloudstack.context.CallContext; @@ -59,9 +60,9 @@ public class DomainRouterJoinDaoImpl extends GenericDaoBase vrSearch; - private final SearchBuilder vrIdSearch; private final SearchBuilder vrIdTrafficSearch; + private final GenericSearchBuilder defaultNetworksSearch; protected DomainRouterJoinDaoImpl() { @@ -78,6 +79,13 @@ protected DomainRouterJoinDaoImpl() { vrIdTrafficSearch.and("trafficType", vrIdTrafficSearch.entity().getTrafficType(), SearchCriteria.Op.IN); vrIdTrafficSearch.done(); + defaultNetworksSearch = createSearchBuilder(Integer.class); + defaultNetworksSearch.select(null, SearchCriteria.Func.COUNT, defaultNetworksSearch.entity().getId()); + defaultNetworksSearch.and("id", defaultNetworksSearch.entity().getId(), SearchCriteria.Op.EQ); + defaultNetworksSearch.and("network_name", defaultNetworksSearch.entity().getNetworkName(), SearchCriteria.Op.NULL); + defaultNetworksSearch.and("removed", defaultNetworksSearch.entity().getRemoved(), SearchCriteria.Op.NULL); + defaultNetworksSearch.done(); + _count = "select count(distinct id) from domain_router_view WHERE "; } @@ -349,6 +357,14 @@ public List getRouterByIdAndTrafficType(Long id, TrafficType return searchIncludingRemoved(sc, null, null, false); } + @Override + public int countDefaultNetworksById(long routerId) { + SearchCriteria sc = defaultNetworksSearch.create(); + sc.setParameters("id", routerId); + final List count = customSearch(sc, null); + return count.get(0); + } + @Override public List newDomainRouterView(VirtualRouter vr) { diff --git a/server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java b/server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java index 163cd449888b..4d7c004e1934 100644 --- a/server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java +++ b/server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java @@ -41,6 +41,7 @@ import javax.inject.Inject; import javax.naming.ConfigurationException; +import com.cloud.api.query.dao.DomainRouterJoinDao; import com.cloud.configuration.ConfigurationManager; import org.apache.cloudstack.acl.ControlledEntity.ACLType; import org.apache.cloudstack.alert.AlertService; @@ -270,6 +271,8 @@ public class VpcManagerImpl extends ManagerBase implements VpcManager, VpcProvis protected NetworkHelper networkHelper; @Inject private VpcPrivateGatewayTransactionCallable vpcTxCallable; + @Inject + protected DomainRouterJoinDao domainRouterJoinDao; private final ScheduledExecutorService _executor = Executors.newScheduledThreadPool(1, new NamedThreadFactory("VpcChecker")); private List vpcElements = null; @@ -1937,9 +1940,8 @@ public void doInTransactionWithoutResult(final TransactionStatus status) { try { Account account = _accountMgr.getAccount(vpc.getAccountId()); - int maxNetworks = VpcMaxNetworks.valueIn(vpc.getAccountId()); - checkIfVpcNumberOfTiersIsNotExceeded(vpcId, maxNetworks, account); + checkIfVpcNumberOfTiersIsNotExceeded(vpcId, account); checkIfVpcHasDomainRouterWithSufficientNicCapacity(vpc); @@ -1965,20 +1967,23 @@ public void doInTransactionWithoutResult(final TransactionStatus status) { } protected boolean existsVpcDomainRouterWithSufficientNicCapacity(long vpcId) { - int countRouterDefaultNics = 2; - long countVpcNetworks = _ntwkDao.countVpcNetworks(vpcId); DomainRouterVO vpcDomainRouter = routerDao.findOneByVpcId(vpcId); if (vpcDomainRouter == null) { return false; } - int totalNicsAvailable = networkOrchestrationService.getVirtualMachineMaxNicsValue(vpcDomainRouter) - countRouterDefaultNics; + int countRouterDefaultNetworks = domainRouterJoinDao.countDefaultNetworksById(vpcDomainRouter.getId()); + long countVpcNetworks = _ntwkDao.countVpcNetworks(vpcId); + + int totalNicsAvailable = networkOrchestrationService.getVirtualMachineMaxNicsValue(vpcDomainRouter) - countRouterDefaultNetworks; return totalNicsAvailable > countVpcNetworks; } - protected void checkIfVpcNumberOfTiersIsNotExceeded(long vpcId, int maxNetworks, Account account) { + protected void checkIfVpcNumberOfTiersIsNotExceeded(long vpcId, Account account) { + int maxNetworks = VpcMaxNetworks.valueIn(account.getId()); + if (_ntwkDao.countVpcNetworks(vpcId) >= maxNetworks) { logger.warn("Failed to create a new VPC Guest Network because the number of networks per VPC has reached its maximum capacity of {}. Increase it by modifying global or account config {}.", maxNetworks, VpcMaxNetworks); throw new CloudRuntimeException(String.format("Number of networks per VPC cannot surpass [%s] for account [%s].", maxNetworks, account.getAccountName())); @@ -1994,7 +1999,7 @@ protected void checkIfVpcHasDomainRouterWithSufficientNicCapacity(Vpc vpc) { protected void checkIfNetworkCidrIsWithinVpcCidr(String cidr, Vpc vpc) { if (!NetUtils.isNetworkAWithinNetworkB(cidr, vpc.getCidr())) { - throw new InvalidParameterValueException("Network cidr " + cidr + " is not within vpc " + vpc + " cidr"); + throw new InvalidParameterValueException(String.format("Network CIDR %s is not within VPC %s CIDR", cidr, vpc)); } } @@ -2003,7 +2008,7 @@ protected void checkIfNetworkCidrNotCrossesOtherVpcNetworksCidr(String cidr, Vpc for (final Network network : networks) { if (NetUtils.isNetworkAWithinNetworkB(network.getCidr(), cidr) || NetUtils.isNetworkAWithinNetworkB(cidr, network.getCidr())) { - throw new InvalidParameterValueException("Network cidr " + cidr + " crosses other network cidr " + network + " belonging to the same vpc " + vpc); + throw new InvalidParameterValueException(String.format("Network CIDR %s crosses other network CIDR %s belonging to the same VPC %s.", cidr, network, vpc)); } } } diff --git a/server/src/test/java/com/cloud/network/vpc/VpcManagerImplTest.java b/server/src/test/java/com/cloud/network/vpc/VpcManagerImplTest.java index e9f82d32d3e8..13d60429ae62 100644 --- a/server/src/test/java/com/cloud/network/vpc/VpcManagerImplTest.java +++ b/server/src/test/java/com/cloud/network/vpc/VpcManagerImplTest.java @@ -23,6 +23,7 @@ import com.cloud.agent.api.to.IpAddressTO; import com.cloud.agent.manager.Commands; import com.cloud.alert.AlertManager; +import com.cloud.api.query.dao.DomainRouterJoinDao; import com.cloud.configuration.Resource; import com.cloud.dc.DataCenterVO; import com.cloud.dc.VlanVO; @@ -86,7 +87,6 @@ import org.mockito.junit.MockitoJUnitRunner; import org.springframework.test.util.ReflectionTestUtils; -import javax.management.InvalidApplicationException; import java.lang.reflect.Field; import java.util.ArrayList; import java.util.HashMap; @@ -165,6 +165,8 @@ public class VpcManagerImplTest { DomainRouterVO domainRouterVOMock; @Mock Vpc vpcMock; + @Mock + DomainRouterJoinDao domainRouterJoinDaoMock; public static final long ACCOUNT_ID = 1; private AccountVO account; @@ -222,6 +224,7 @@ public void setup() throws NoSuchFieldException, IllegalAccessException { manager._firewallDao = firewallDao; manager._networkAclDao = networkACLDaoMock; manager.networkOrchestrationService = networkOrchestrationServiceMock; + manager.domainRouterJoinDao = domainRouterJoinDaoMock; CallContext.register(Mockito.mock(User.class), Mockito.mock(Account.class)); registerCallContext(); overrideDefaultConfigValue(NetworkService.AllowUsersToSpecifyVRMtu, "_defaultValue", "false"); @@ -524,6 +527,8 @@ public void validateVpcPrivateGatewayTestAclFromDifferentVpcThrowsInvalidParamet public void existsVpcDomainRouterWithSufficientNicCapacityTestUnavailableRoutersReturnsFalse() { Mockito.when(networkDao.countVpcNetworks(vpcId)).thenReturn(7L); Mockito.when(routerDao.findOneByVpcId(vpcId)).thenReturn(domainRouterVOMock); + Mockito.when(domainRouterVOMock.getId()).thenReturn(1L); + Mockito.when(domainRouterJoinDaoMock.countDefaultNetworksById(1L)).thenReturn(2); Mockito.when(networkOrchestrationServiceMock.getVirtualMachineMaxNicsValue(domainRouterVOMock)).thenReturn(9); boolean result = manager.existsVpcDomainRouterWithSufficientNicCapacity(vpcId); @@ -535,6 +540,8 @@ public void existsVpcDomainRouterWithSufficientNicCapacityTestUnavailableRouters public void existsVpcDomainRouterWithSufficientNicCapacityTestAvailableRouterReturnsTrue() { Mockito.when(networkDao.countVpcNetworks(vpcId)).thenReturn(6L); Mockito.when(routerDao.findOneByVpcId(vpcId)).thenReturn(domainRouterVOMock); + Mockito.when(domainRouterVOMock.getId()).thenReturn(1L); + Mockito.when(domainRouterJoinDaoMock.countDefaultNetworksById(1L)).thenReturn(2); Mockito.when(networkOrchestrationServiceMock.getVirtualMachineMaxNicsValue(domainRouterVOMock)).thenReturn(9); boolean result = manager.existsVpcDomainRouterWithSufficientNicCapacity(vpcId); @@ -544,7 +551,6 @@ public void existsVpcDomainRouterWithSufficientNicCapacityTestAvailableRouterRet @Test public void existsVpcDomainRouterWithSufficientNicCapacityTestNullRouterReturnsFalse() { - Mockito.when(networkDao.countVpcNetworks(vpcId)).thenReturn(6L); Mockito.when(routerDao.findOneByVpcId(vpcId)).thenReturn(null); boolean result = manager.existsVpcDomainRouterWithSufficientNicCapacity(vpcId); From 5535203196dd631c16ae005c849997fcf814c691 Mon Sep 17 00:00:00 2001 From: Henrique Sato Date: Wed, 10 Apr 2024 15:46:01 -0300 Subject: [PATCH 11/16] Remove whitespaces --- engine/schema/src/main/java/com/cloud/vm/dao/NicDaoImpl.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/engine/schema/src/main/java/com/cloud/vm/dao/NicDaoImpl.java b/engine/schema/src/main/java/com/cloud/vm/dao/NicDaoImpl.java index 7f266edd5015..3eee1d4e749d 100644 --- a/engine/schema/src/main/java/com/cloud/vm/dao/NicDaoImpl.java +++ b/engine/schema/src/main/java/com/cloud/vm/dao/NicDaoImpl.java @@ -107,8 +107,6 @@ protected void init() { PeerRouterSearch.and("macAddress", PeerRouterSearch.entity().getMacAddress(), Op.EQ); PeerRouterSearch.and("vmType", PeerRouterSearch.entity().getVmType(), Op.EQ); PeerRouterSearch.done(); - - } @Override From 20877f3bb1e02db32cc678e749887d6544ac4175 Mon Sep 17 00:00:00 2001 From: Henrique Sato Date: Fri, 12 Apr 2024 17:51:09 -0300 Subject: [PATCH 12/16] Fix tests --- .../test/java/com/cloud/network/vpc/VpcManagerImplTest.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/server/src/test/java/com/cloud/network/vpc/VpcManagerImplTest.java b/server/src/test/java/com/cloud/network/vpc/VpcManagerImplTest.java index 13d60429ae62..d325295c9b17 100644 --- a/server/src/test/java/com/cloud/network/vpc/VpcManagerImplTest.java +++ b/server/src/test/java/com/cloud/network/vpc/VpcManagerImplTest.java @@ -364,13 +364,13 @@ public void testCreateVpcNetwork() throws InsufficientCapacityException, Resourc Mockito.when(entityMgr.findById(NetworkOffering.class, 1L)).thenReturn(offering); Mockito.when(vpcMock.getId()).thenReturn(VPC_ID); Mockito.when(vpcDao.acquireInLockTable(VPC_ID)).thenReturn(vpcMockVO); - Mockito.when(networkDao.countVpcNetworks(anyLong())).thenReturn(1L); Mockito.when(offering.getGuestType()).thenReturn(Network.GuestType.Isolated); Mockito.when(networkModel.listNetworkOfferingServices(anyLong())).thenReturn(services); Mockito.when(networkOfferingServiceMapDao.listByNetworkOfferingId(anyLong())).thenReturn(serviceMap); Mockito.when(vpcMock.getCidr()).thenReturn("10.0.0.0/8"); Mockito.when(vpcMock.getNetworkDomain()).thenReturn("cs1cloud.internal"); Mockito.when(manager.existsVpcDomainRouterWithSufficientNicCapacity(VPC_ID)).thenReturn(true); + Mockito.doNothing().when(manager).checkIfVpcNumberOfTiersIsNotExceeded(Mockito.anyLong(), Mockito.any()); manager.validateNewVpcGuestNetwork("10.10.10.0/24", "10.10.10.1", accountMock, vpcMock, "cs1cloud.internal"); manager.validateNtwkOffForNtwkInVpc(2L, 1, "10.10.10.0/24", "111-", vpcMock, "10.1.1.1", new AccountVO(), null); @@ -560,11 +560,10 @@ public void existsVpcDomainRouterWithSufficientNicCapacityTestNullRouterReturnsF @Test public void checkIfVpcNumberOfTiersIsNotExceededTestExceededTiersThrowCloudRuntimeException() { - int maxNetworks = 3; AccountVO accountMock = Mockito.mock(AccountVO.class); Mockito.doReturn(5L).when(networkDao).countVpcNetworks(1L); - Assert.assertThrows(CloudRuntimeException.class, () -> manager.checkIfVpcNumberOfTiersIsNotExceeded(vpcId, maxNetworks, accountMock)); + Assert.assertThrows(CloudRuntimeException.class, () -> manager.checkIfVpcNumberOfTiersIsNotExceeded(vpcId, accountMock)); } @Test From aa4271ba553acd87695e7ae366d769040f6a0165 Mon Sep 17 00:00:00 2001 From: Henrique Sato Date: Mon, 22 Apr 2024 17:13:17 -0300 Subject: [PATCH 13/16] Add integration tests --- .../integration/component/test_vpc_network.py | 315 +++++++++++++++++- .../component/test_vpc_vms_deployment.py | 7 +- 2 files changed, 315 insertions(+), 7 deletions(-) diff --git a/test/integration/component/test_vpc_network.py b/test/integration/component/test_vpc_network.py index db436bbd3983..c3e571f16c3d 100644 --- a/test/integration/component/test_vpc_network.py +++ b/test/integration/component/test_vpc_network.py @@ -35,7 +35,11 @@ Router, StaticNATRule, NetworkACL, - PublicIPAddress) + PublicIPAddress, + Host, + Cluster, + Configurations, + Resources) from marvin.lib.common import (get_zone, get_domain, get_template, @@ -51,7 +55,6 @@ class Services: - """Test VPC network services """ @@ -187,7 +190,9 @@ def __init__(self): "network": { "name": "Test Network", "displaytext": "Test Network", - "netmask": '255.255.255.0' + "netmask": '255.255.255.0', + # Max networks allowed per VPC: Xenserver -> 7, VMWare -> 10 & KVM -> 27 + "limit": 5, }, "lbrule": { "name": "SSH", @@ -2756,3 +2761,307 @@ def test_stop_start_vpc_router(self): if (exceptionOccurred or (not isRouterInDesiredState)): self.fail(exceptionMessage) return + + +class TestVPCNetworkHypervisorLimit(cloudstackTestCase): + + @classmethod + def setUpClass(cls): + cls.testClient = super(TestVPCNetworkHypervisorLimit, cls).getClsTestClient() + cls.api_client = cls.testClient.getApiClient() + + cls.services = Services().services + + cls.domain = get_domain(cls.api_client) + cls.zone = get_zone(cls.api_client, cls.testClient.getZoneForTests()) + + cls._cleanup = [] + + return + + def setUp(self): + self.apiclient = self.testClient.getApiClient() + self.dbclient = self.testClient.getDbConnection() + self.account = Account.create( + self.apiclient, + self.services["account"], + admin=True, + domainid=self.domain.id + ) + self.cleanup = [self.account] + + return + + @classmethod + def tearDownClass(cls): + super(TestVPCNetworkHypervisorLimit, cls).tearDownClass() + + def tearDown(self): + super(TestVPCNetworkHypervisorLimit, self).tearDown() + + def validate_vpc_network(self, network, state=None): + """Validates the VPC network""" + + self.debug("Checking if the VPC network was created successfully.") + vpc_networks = VPC.list( + self.apiclient, + id=network.id + ) + self.assertEqual( + isinstance(vpc_networks, list), + True, + "List VPC network should return a valid list" + ) + self.assertEqual( + network.name, + vpc_networks[0].name, + "Name of the VPC network should match with listVPC data" + ) + if state: + self.assertEqual( + vpc_networks[0].state, + state, + "VPC state should be '%s'" % state + ) + self.debug("VPC network validated - %s" % network.name) + + return + + def validate_vpc_offering(self, vpc_offering): + """Validates the VPC offering""" + + self.debug("Checking if the VPC offering was created successfully.") + vpc_offs = VpcOffering.list( + self.apiclient, + id=vpc_offering.id + ) + self.assertEqual( + isinstance(vpc_offs, list), + True, + "List VPC offerings should return a valid list" + ) + self.assertEqual( + vpc_offering.name, + vpc_offs[0].name, + "Name of the VPC offering should match with listVPCOff data" + ) + self.debug("VPC offering was created successfully - %s" % vpc_offering.name) + + return + + def create_network_max_limit_hypervisor_test(self, hypervisor): + Configurations.update( + self.apiclient, + accountid=self.account.id, + name="vpc.max.networks", + value=30 + ) + + self.debug("Creating a VPC offering.") + vpc_off = VpcOffering.create( + self.apiclient, + self.services["vpc_offering"] + ) + + self._cleanup.append(vpc_off) + self.validate_vpc_offering(vpc_off) + + self.debug("Enabling the VPC offering.") + vpc_off.update(self.apiclient, state='Enabled') + + self.debug("Creating a VPC network in the account: %s" % self.account.name) + self.services["vpc"]["cidr"] = '10.1.1.1/16' + vpc = VPC.create( + self.apiclient, + self.services["vpc"], + vpcofferingid=vpc_off.id, + zoneid=self.zone.id, + account=self.account.name, + domainid=self.account.domainid + ) + self.validate_vpc_network(vpc) + + vpc_router = Router.list( + self.apiclient, + vpcId=vpc.id, + listAll=True + )[0] + + host = Host.list( + self.apiclient, + id=vpc_router.hostId + )[0] + + cluster = Cluster.list( + self.apiclient, + id=host.clusterId + )[0] + + cluster_old_hypervisor_type = cluster.hypervisortype + + self.debug("Updating cluster %s hypervisor from %s to %s." % + (cluster.name, cluster_old_hypervisor_type, hypervisor)) + Cluster.update( + self.apiclient, + id=cluster.id, + hypervisor=hypervisor + ) + + network_offering = NetworkOffering.create( + self.apiclient, + self.services["network_offering"], + conservemode=False + ) + # Enable Network offering + self._cleanup.append(network_offering) + network_offering.update(self.apiclient, state='Enabled') + + # Empty list to store all networks + networks = [] + + # Creating network using the network offering created + self.debug("Creating network with network offering: %s." % network_offering.id) + network_1 = Network.create( + self.apiclient, + self.services["network"], + accountid=self.account.name, + domainid=self.account.domainid, + networkofferingid=network_offering.id, + zoneid=self.zone.id, + gateway='10.1.0.1', + vpcid=vpc.id + ) + + self.debug("Created network with ID: %s." % network_1.id) + + config_name = "virtual.machine.max.nics.%s" % hypervisor.lower() + + configs = Configurations.list( + self.apiclient, + name=config_name, + listall=True + ) + if not isinstance(configs, list): + raise Exception("Failed to find %s virtual machine max NICs." % hypervisor) + + self.services["network"]["limit"] = int(configs[0].value) + + self.debug("Updating network resource limit for account %s with value %s." % ( + self.account.name, self.services["network"]["limit"])) + Resources.updateLimit( + self.apiclient, + resourcetype=6, + account=self.account.name, + domainid=self.domain.id, + max=self.services["network"]["limit"] + ) + + # Create networks till max limit of hypervisor + for i in range(self.services["network"]["limit"] - 3): + # Creating network using the network offering created + self.debug("Creating network with network offering: %s." % + network_offering.id) + gateway = '10.1.' + str(i + 1) + '.1' + self.debug("Gateway for new network: %s." % gateway) + + network = Network.create( + self.apiclient, + self.services["network"], + accountid=self.account.name, + domainid=self.account.domainid, + networkofferingid=network_offering.id, + zoneid=self.zone.id, + gateway=gateway, + vpcid=vpc.id + ) + self.debug("Created network with ID: %s." % network.id) + networks.append(network) + + self.debug( + "Trying to create one more network than %s hypervisor limit in VPC: %s." % (hypervisor, vpc.name)) + gateway = '10.1.' + str(self.services["network"]["limit"]) + '.1' + + with self.assertRaises(Exception): + Network.create( + self.apiclient, + self.services["network"], + accountid=self.account.name, + domainid=self.account.domainid, + networkofferingid=network_offering.id, + zoneid=self.zone.id, + gateway=gateway, + vpcid=vpc.id + ) + + self.debug("Deleting one of the existing networks.") + try: + network_1.delete(self.apiclient) + except Exception as e: + self.fail("Failed to delete network: %s - %s." % + (network_1.name, e)) + + self.debug("Creating a new network in VPC: %s." % vpc.name) + network = Network.create( + self.apiclient, + self.services["network"], + accountid=self.account.name, + domainid=self.account.domainid, + networkofferingid=network_offering.id, + zoneid=self.zone.id, + gateway=gateway, + vpcid=vpc.id + ) + self.debug("Created a new network: %s." % network.name) + networks.append(network) + + self.debug( + "Updating cluster %s hypervisor to its old hypervisor %s." % (cluster.name, cluster_old_hypervisor_type)) + Cluster.update( + self.apiclient, + id=cluster.id, + hypervisor=cluster_old_hypervisor_type + ) + + return + + @attr(tags=["advanced", "intervlan"], required_hardware="true") + def test_01_create_network_max_limit_xenserver(self): + """ Test create networks in VPC up to maximum limit for XenServer hypervisor. + """ + + # Validate the following + # 1. Create a VPC and add maximum # of supported networks to the VPC; + # 2. After reaching the maximum networks, adding another network to the VPC must raise an exception; + # 3. Delete a network from VPC; + # 4. Add a new network to the VPC. + + self.create_network_max_limit_hypervisor_test("XenServer") + return + + @attr(tags=["advanced", "intervlan"], required_hardware="true") + def test_02_create_network_max_limit_vmware(self): + """ Test create networks in VPC up to maximum limit for VMware hypervisor. + """ + + # Validate the following + # 1. Create a VPC and add maximum # of supported networks to the VPC; + # 2. After reaching the maximum networks, adding another network to the VPC must raise an exception; + # 3. Delete a network from VPC; + # 4. Add a new network to the VPC. + + self.create_network_max_limit_hypervisor_test("VMware") + return + + @attr(tags=["advanced", "intervlan"], required_hardware="true") + def test_03_create_network_max_limit_kvm(self): + """ Test create networks in VPC up to maximum limit for KVM hypervisor. + """ + + # Validate the following + # 1. Create a VPC and add maximum # of supported networks to the VPC; + # 2. After reaching the maximum networks, adding another network to the VPC must raise an exception; + # 3. Delete a network from VPC; + # 4. Add a new network to the VPC. + + self.create_network_max_limit_hypervisor_test("KVM") + return diff --git a/test/integration/component/test_vpc_vms_deployment.py b/test/integration/component/test_vpc_vms_deployment.py index 4d3f93471d1a..6f74a3c23623 100644 --- a/test/integration/component/test_vpc_vms_deployment.py +++ b/test/integration/component/test_vpc_vms_deployment.py @@ -123,8 +123,7 @@ def __init__(self): "displaytext": "Test Network", "netmask": '255.255.255.0', "limit": 5, - # Max networks allowed as per hypervisor - # Xenserver -> 5, VMWare -> 9 + # Max networks allowed per VPC }, "natrule": { "privateport": 22, @@ -1344,8 +1343,8 @@ def test_04_deploy_vms_delete_add_network_noLb(self): return @attr(tags=["advanced", "intervlan"], required_hardware="false") - def test_05_create_network_max_limit(self): - """ Test create networks in VPC upto maximum limit for hypervisor + def test_05_create_network_max_limit_vpc(self): + """ Test create networks in VPC up to maximum limit for VPC. """ # Validate the following From b9dd4da2d5c92d334e9d33cf832046502d5ef1bb Mon Sep 17 00:00:00 2001 From: Henrique Sato Date: Mon, 22 Apr 2024 18:19:50 -0300 Subject: [PATCH 14/16] Refactor --- .../service/NetworkOrchestrationService.java | 5 +- .../orchestration/NetworkOrchestrator.java | 65 +++++++++--------- .../NetworkOrchestratorTest.java | 68 +++++++++++-------- .../com/cloud/network/vpc/VpcManagerImpl.java | 8 ++- .../cloud/network/vpc/VpcManagerImplTest.java | 12 ++++ .../com/cloud/vpc/MockNetworkManagerImpl.java | 2 +- .../integration/component/test_vpc_network.py | 2 +- 7 files changed, 97 insertions(+), 65 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 a3665ccbf312..d24d942af9ca 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 @@ -361,11 +361,10 @@ void implementNetworkElementsAndResources(DeployDestination dest, ReservationCon * Returns the maximum number of NICs that the given virtual machine can have considering its hypervisor. *

* First we try to retrieve the setting value from the cluster where the virtual machine is deployed. If the cluster does not exist, we try to retrieve the setting value from the virtual machine hypervisor type. - * In last case, if the virtual machine does not have a hypervisor type, we retrieve the smallest value between the {@link NetworkOrchestrationService#VirtualMachineMaxNicsKvm}, {@link NetworkOrchestrationService#VirtualMachineMaxNicsVmware} - * and {@link NetworkOrchestrationService#VirtualMachineMaxNicsXenserver} global settings. + * Returns null if the setting value could not be extracted. * * @param virtualMachine Virtual machine to get the maximum number of NICs. * @return The maximum number of NICs that the virtual machine can have. */ - int getVirtualMachineMaxNicsValue(VirtualMachine virtualMachine); + Integer getVirtualMachineMaxNicsValue(VirtualMachine virtualMachine); } 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 aaeedf1fdc57..6d1f5f70a84b 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 @@ -257,7 +257,6 @@ import com.cloud.vm.dao.UserVmDao; import com.cloud.vm.dao.VMInstanceDao; import com.googlecode.ipv6.IPv6Address; -import org.apache.commons.lang3.math.NumberUtils; import org.jetbrains.annotations.NotNull; /** @@ -1041,10 +1040,10 @@ public void saveExtraDhcpOptions(final String networkUuid, final Long nicId, fin public Pair allocateNic(final NicProfile requested, final Network network, final Boolean isDefaultNic, int deviceId, final VirtualMachineProfile vm) throws InsufficientVirtualNetworkCapacityException, InsufficientAddressCapacityException, ConcurrentOperationException { - int virtualMachineMaxNicsValue = getVirtualMachineMaxNicsValue(vm.getVirtualMachine()); + Integer virtualMachineMaxNicsValue = getVirtualMachineMaxNicsValue(vm.getVirtualMachine()); List nics = _nicDao.listByVmId(vm.getId()); - if (nics.size() >= virtualMachineMaxNicsValue) { + if (virtualMachineMaxNicsValue != null && nics.size() >= virtualMachineMaxNicsValue) { throw new CloudRuntimeException(String.format("Failed to allocate NIC on network [%s] because VM [%s] has reached its maximum NIC capacity [%s].", network.getUuid(), vm.getUuid(), virtualMachineMaxNicsValue)); } @@ -1099,7 +1098,7 @@ public Pair allocateNic(final NicProfile requested, final N } @Override - public int getVirtualMachineMaxNicsValue(VirtualMachine virtualMachine) { + public Integer getVirtualMachineMaxNicsValue(VirtualMachine virtualMachine) { Integer virtualMachineMaxNicsValue = getVirtualMachineMaxNicsValueFromCluster(virtualMachine); if (virtualMachineMaxNicsValue != null) { @@ -1107,8 +1106,8 @@ public int getVirtualMachineMaxNicsValue(VirtualMachine virtualMachine) { } if (virtualMachine.getHypervisorType() == null) { - logger.debug("Using the smallest setting global value between {}, {} and {} as the VM {} does not have a hypervisor type and is not deployed on either a host or a cluster.", VirtualMachineMaxNicsKvm, VirtualMachineMaxNicsVmware, VirtualMachineMaxNicsXenserver, virtualMachine.getUuid()); - return NumberUtils.min(VirtualMachineMaxNicsKvm.value(), VirtualMachineMaxNicsVmware.value(), VirtualMachineMaxNicsXenserver.value()); + logger.debug("Not considering the hypervisor maximum limits as we were unable to find a compatible hypervisor on the VM and VM cluster for virtual machine maximum NICs settings."); + return null; } return getVirtualMachineMaxNicsValueFromVmHypervisorType(virtualMachine); @@ -1140,22 +1139,22 @@ protected Integer getVirtualMachineMaxNicsValueFromCluster(VirtualMachine virtua * @param cluster Cluster to get the virtual machine max NICs value from. * @return The maximum number of NICs that the virtual machine can have. */ - protected int getVirtualMachineMaxNicsValueFromCluster(ClusterVO cluster) { - int virtualMachineMaxNicsValue = 0; - HypervisorType hypervisor = cluster.getHypervisorType(); - - if (HypervisorType.KVM.equals(hypervisor)) { - virtualMachineMaxNicsValue = VirtualMachineMaxNicsKvm.valueIn(cluster.getId()); - logger.debug("The cluster {} where the VM is deployed uses the {} hypervisor. Therefore, the {} setting value [{}] will be used.", cluster.getName(), hypervisor, VirtualMachineMaxNicsKvm, virtualMachineMaxNicsValue); - } else if (HypervisorType.VMware.equals(hypervisor)) { - virtualMachineMaxNicsValue = VirtualMachineMaxNicsVmware.valueIn(cluster.getId()); - logger.debug("The cluster {} where the VM is deployed uses the {} hypervisor. Therefore, the {} setting value [{}] will be used.", cluster.getName(), hypervisor, VirtualMachineMaxNicsVmware, virtualMachineMaxNicsValue); - } else { - virtualMachineMaxNicsValue = VirtualMachineMaxNicsXenserver.valueIn(cluster.getId()); - logger.debug("The cluster {} where the VM is deployed uses the {} hypervisor. Therefore, the {} setting value [{}] will be used.", cluster.getName(), hypervisor, VirtualMachineMaxNicsXenserver, virtualMachineMaxNicsValue); + protected Integer getVirtualMachineMaxNicsValueFromCluster(ClusterVO cluster) { + HypervisorType clusterHypervisor = cluster.getHypervisorType(); + + switch (clusterHypervisor) { + case KVM: + logger.debug("The cluster {} where the VM is deployed uses the {} hypervisor. Therefore, the {} setting value [{}] will be used.", cluster.getName(), clusterHypervisor, VirtualMachineMaxNicsKvm, VirtualMachineMaxNicsKvm.valueIn(cluster.getId())); + return VirtualMachineMaxNicsKvm.valueIn(cluster.getId()); + case VMware: + logger.debug("The cluster {} where the VM is deployed uses the {} hypervisor. Therefore, the {} setting value [{}] will be used.", cluster.getName(), clusterHypervisor, VirtualMachineMaxNicsKvm, VirtualMachineMaxNicsVmware.valueIn(cluster.getId())); + return VirtualMachineMaxNicsVmware.valueIn(cluster.getId()); + case XenServer: + logger.debug("The cluster {} where the VM is deployed uses the {} hypervisor. Therefore, the {} setting value [{}] will be used.", cluster.getName(), clusterHypervisor, VirtualMachineMaxNicsXenserver, VirtualMachineMaxNicsXenserver.valueIn(cluster.getId())); + return VirtualMachineMaxNicsXenserver.valueIn(cluster.getId()); + default: + return null; } - - return virtualMachineMaxNicsValue; } /** @@ -1164,19 +1163,23 @@ protected int getVirtualMachineMaxNicsValueFromCluster(ClusterVO cluster) { * @param virtualMachine Virtual machine to get the hypervisor type. * @return The maximum number of NICs that the virtual machine can have. */ - protected int getVirtualMachineMaxNicsValueFromVmHypervisorType(VirtualMachine virtualMachine) { + protected Integer getVirtualMachineMaxNicsValueFromVmHypervisorType(VirtualMachine virtualMachine) { HypervisorType virtualMachineHypervisorType = virtualMachine.getHypervisorType(); - if (HypervisorType.KVM.equals(virtualMachineHypervisorType)) { - logger.debug("Using the {} setting global value {} as the VM {} has the {} hypervisor type and is not deployed on either a host or a cluster.", VirtualMachineMaxNicsKvm, VirtualMachineMaxNicsKvm.value(), virtualMachine.getUuid(), virtualMachineHypervisorType); - return VirtualMachineMaxNicsKvm.value(); - } else if (HypervisorType.VMware.equals(virtualMachineHypervisorType)) { - logger.debug("Using the {} setting global value {} as the VM {} has the {} hypervisor type and is not deployed on either a host or a cluster.", VirtualMachineMaxNicsVmware, VirtualMachineMaxNicsVmware.value(), virtualMachine.getUuid(), virtualMachineHypervisorType); - return VirtualMachineMaxNicsVmware.value(); + switch (virtualMachineHypervisorType) { + case KVM: + logger.debug("Using the {} setting global value {} as the VM {} has the {} hypervisor type and is not deployed on either a host or a cluster.", VirtualMachineMaxNicsKvm, VirtualMachineMaxNicsKvm.value(), virtualMachine.getUuid(), virtualMachineHypervisorType); + return VirtualMachineMaxNicsKvm.value(); + case VMware: + logger.debug("Using the {} setting global value {} as the VM {} has the {} hypervisor type and is not deployed on either a host or a cluster.", VirtualMachineMaxNicsVmware, VirtualMachineMaxNicsVmware.value(), virtualMachine.getUuid(), virtualMachineHypervisorType); + return VirtualMachineMaxNicsVmware.value(); + case XenServer: + logger.debug("Using the {} setting global value {} as the VM {} has the {} hypervisor type and is not deployed on either a host or a cluster.", VirtualMachineMaxNicsXenserver, VirtualMachineMaxNicsXenserver.value(), virtualMachine.getUuid(), virtualMachineHypervisorType); + return VirtualMachineMaxNicsXenserver.value(); + default: + logger.debug("Not considering the hypervisor maximum limits as we were unable to find a compatible hypervisor on the VM and VM cluster for virtual machine maximum NICs configurations."); + return null; } - - logger.debug("Using the {} setting global value {} as the VM {} has the {} hypervisor type and is not deployed on either a host or a cluster.", VirtualMachineMaxNicsXenserver, VirtualMachineMaxNicsXenserver.value(), virtualMachine.getUuid(), virtualMachineHypervisorType); - return VirtualMachineMaxNicsXenserver.value(); } private boolean isNicAllocatedForNsxPublicNetworkOnVR(Network network, NicProfile requested, VirtualMachineProfile vm) { 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 330aaac261e0..c91e65598759 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 @@ -855,32 +855,23 @@ public void getVirtualMachineMaxNicsValueTestVirtualMachineDeployedReturnsVirtua VirtualMachine virtualMachineMock = Mockito.mock(VirtualMachine.class); Mockito.doReturn(44).when(testOrchastrator).getVirtualMachineMaxNicsValueFromCluster(virtualMachineMock); - int virtualMachineMaxNicsValue = testOrchastrator.getVirtualMachineMaxNicsValue(virtualMachineMock); + Integer virtualMachineMaxNicsValue = testOrchastrator.getVirtualMachineMaxNicsValue(virtualMachineMock); Mockito.verify(testOrchastrator, Mockito.times(1)).getVirtualMachineMaxNicsValueFromCluster((VirtualMachine) Mockito.any()); - Assert.assertEquals(44, virtualMachineMaxNicsValue); + Assert.assertEquals((Integer) 44, virtualMachineMaxNicsValue); } @Test - public void getVirtualMachineMaxNicsValueTestVirtualMachineWithoutDeployAndTemplateReturnsSmallestGlobalValueBetweenVirtualMachineMaxNicsSettings() throws NoSuchFieldException, IllegalAccessException { + public void getVirtualMachineMaxNicsValueTestVirtualMachineIncompatibleHypervisorReturnsNull() { VirtualMachineProfile virtualMachineProfileMock = Mockito.mock(VirtualMachineProfile.class); VirtualMachine virtualMachineMock = Mockito.mock(VirtualMachine.class); - ConfigKey virtualMachineMaxNicsKvmMock = Mockito.mock(ConfigKey.class); - ConfigKey virtualMachineMaxNicsVmwareMock = Mockito.mock(ConfigKey.class); - ConfigKey virtualMachineMaxNicsXenserverMock = Mockito.mock(ConfigKey.class); Mockito.doReturn(virtualMachineMock).when(virtualMachineProfileMock).getVirtualMachine(); Mockito.doReturn(null).when(virtualMachineMock).getHypervisorType(); Mockito.doReturn(null).when(testOrchastrator).getVirtualMachineMaxNicsValueFromCluster(virtualMachineMock); - Mockito.doReturn(23).when(virtualMachineMaxNicsKvmMock).value(); - Mockito.doReturn(10).when(virtualMachineMaxNicsVmwareMock).value(); - Mockito.doReturn(7).when(virtualMachineMaxNicsXenserverMock).value(); - updateFinalStaticField(testOrchastrator.getClass().getField("VirtualMachineMaxNicsKvm"), virtualMachineMaxNicsKvmMock); - updateFinalStaticField(testOrchastrator.getClass().getField("VirtualMachineMaxNicsVmware"), virtualMachineMaxNicsVmwareMock); - updateFinalStaticField(testOrchastrator.getClass().getField("VirtualMachineMaxNicsXenserver"), virtualMachineMaxNicsXenserverMock); - int virtualMachineMaxNicsValue = testOrchastrator.getVirtualMachineMaxNicsValue(virtualMachineMock); + Integer virtualMachineMaxNicsValue = testOrchastrator.getVirtualMachineMaxNicsValue(virtualMachineMock); - Assert.assertEquals(7, virtualMachineMaxNicsValue); + Assert.assertNull(virtualMachineMaxNicsValue); } @Test @@ -892,10 +883,10 @@ public void getVirtualMachineMaxNicsValueTestVirtualMachineWithoutDeployReturnsV Mockito.doReturn(null).when(testOrchastrator).getVirtualMachineMaxNicsValueFromCluster(virtualMachineMock); Mockito.doReturn(33).when(testOrchastrator).getVirtualMachineMaxNicsValueFromVmHypervisorType(virtualMachineMock); - int virtualMachineMaxNicsValue = testOrchastrator.getVirtualMachineMaxNicsValue(virtualMachineMock); + Integer virtualMachineMaxNicsValue = testOrchastrator.getVirtualMachineMaxNicsValue(virtualMachineMock); Mockito.verify(testOrchastrator, Mockito.times(1)).getVirtualMachineMaxNicsValueFromVmHypervisorType(Mockito.any()); - Assert.assertEquals(33, virtualMachineMaxNicsValue); + Assert.assertEquals((Integer) 33, virtualMachineMaxNicsValue); } @Test @@ -932,9 +923,9 @@ public void getVirtualMachineMaxNicsValueFromClusterTestKvmClusterReturnsVirtual Mockito.doReturn(33).when(virtualMachineMaxNicsKvmMock).valueIn(1L); updateFinalStaticField(testOrchastrator.getClass().getField("VirtualMachineMaxNicsKvm"), virtualMachineMaxNicsKvmMock); - int virtualMachineMaxNicsValue = testOrchastrator.getVirtualMachineMaxNicsValueFromCluster(clusterVoMock); + Integer virtualMachineMaxNicsValue = testOrchastrator.getVirtualMachineMaxNicsValueFromCluster(clusterVoMock); - Assert.assertEquals(33, virtualMachineMaxNicsValue); + Assert.assertEquals((Integer) 33, virtualMachineMaxNicsValue); } @Test @@ -946,9 +937,9 @@ public void getVirtualMachineMaxNicsValueFromClusterTestVmwareClusterReturnsVirt Mockito.doReturn(22).when(virtualMachineMaxNicsVmwareMock).valueIn(1L); updateFinalStaticField(testOrchastrator.getClass().getField("VirtualMachineMaxNicsVmware"), virtualMachineMaxNicsVmwareMock); - int virtualMachineMaxNicsValue = testOrchastrator.getVirtualMachineMaxNicsValueFromCluster(clusterVoMock); + Integer virtualMachineMaxNicsValue = testOrchastrator.getVirtualMachineMaxNicsValueFromCluster(clusterVoMock); - Assert.assertEquals(22, virtualMachineMaxNicsValue); + Assert.assertEquals((Integer) 22, virtualMachineMaxNicsValue); } @Test @@ -960,9 +951,20 @@ public void getVirtualMachineMaxNicsValueFromClusterTestXenserverClusterReturnsV Mockito.doReturn(11).when(virtualMachineMaxNicsXenserverMock).valueIn(1L); updateFinalStaticField(testOrchastrator.getClass().getField("VirtualMachineMaxNicsXenserver"), virtualMachineMaxNicsXenserverMock); - int virtualMachineMaxNicsValue = testOrchastrator.getVirtualMachineMaxNicsValueFromCluster(clusterVoMock); + Integer virtualMachineMaxNicsValue = testOrchastrator.getVirtualMachineMaxNicsValueFromCluster(clusterVoMock); - Assert.assertEquals(11, virtualMachineMaxNicsValue); + Assert.assertEquals((Integer) 11, virtualMachineMaxNicsValue); + } + + @Test + public void getVirtualMachineMaxNicsValueFromClusterTestIncompatibleHypervisorReturnsNull() { + ClusterVO clusterVoMock = Mockito.mock(ClusterVO.class); + Mockito.doReturn(1L).when(clusterVoMock).getId(); + Mockito.doReturn(Hypervisor.HypervisorType.Hyperv).when(clusterVoMock).getHypervisorType(); + + Integer virtualMachineMaxNicsValue = testOrchastrator.getVirtualMachineMaxNicsValueFromCluster(clusterVoMock); + + Assert.assertNull(virtualMachineMaxNicsValue); } @Test @@ -973,9 +975,9 @@ public void getVirtualMachineMaxNicsValueFromVmHypervisorTypeTestKvmHypervisorRe Mockito.doReturn(23).when(virtualMachineMaxNicsKvmMock).value(); updateFinalStaticField(testOrchastrator.getClass().getField("VirtualMachineMaxNicsKvm"), virtualMachineMaxNicsKvmMock); - int virtualMachineMaxNicsValue = testOrchastrator.getVirtualMachineMaxNicsValueFromVmHypervisorType(virtualMachineMock); + Integer virtualMachineMaxNicsValue = testOrchastrator.getVirtualMachineMaxNicsValueFromVmHypervisorType(virtualMachineMock); - Assert.assertEquals(23, virtualMachineMaxNicsValue); + Assert.assertEquals((Integer) 23, virtualMachineMaxNicsValue); } @Test @@ -986,9 +988,9 @@ public void getVirtualMachineMaxNicsValueFromVmHypervisorTypeTestVmwareHyperviso Mockito.doReturn(10).when(virtualMachineMaxNicsVmwareMock).value(); updateFinalStaticField(testOrchastrator.getClass().getField("VirtualMachineMaxNicsVmware"), virtualMachineMaxNicsVmwareMock); - int virtualMachineMaxNicsValue = testOrchastrator.getVirtualMachineMaxNicsValueFromVmHypervisorType(virtualMachineMock); + Integer virtualMachineMaxNicsValue = testOrchastrator.getVirtualMachineMaxNicsValueFromVmHypervisorType(virtualMachineMock); - Assert.assertEquals(10, virtualMachineMaxNicsValue); + Assert.assertEquals((Integer) 10, virtualMachineMaxNicsValue); } @Test @@ -999,9 +1001,19 @@ public void getVirtualMachineMaxNicsValueFromVmHypervisorTypeTestXenserverHyperv Mockito.doReturn(7).when(virtualMachineMaxNicsXenserverMock).value(); updateFinalStaticField(testOrchastrator.getClass().getField("VirtualMachineMaxNicsXenserver"), virtualMachineMaxNicsXenserverMock); - int virtualMachineMaxNicsValue = testOrchastrator.getVirtualMachineMaxNicsValueFromVmHypervisorType(virtualMachineMock); + Integer virtualMachineMaxNicsValue = testOrchastrator.getVirtualMachineMaxNicsValueFromVmHypervisorType(virtualMachineMock); + + Assert.assertEquals((Integer) 7, virtualMachineMaxNicsValue); + } + + @Test + public void getVirtualMachineMaxNicsValueFromVmHypervisorTypeTestIncompatibleHypervisorReturnsNull() { + VirtualMachine virtualMachineMock = Mockito.mock(VirtualMachine.class); + Mockito.doReturn(Hypervisor.HypervisorType.Hyperv).when(virtualMachineMock).getHypervisorType(); + + Integer virtualMachineMaxNicsValue = testOrchastrator.getVirtualMachineMaxNicsValueFromVmHypervisorType(virtualMachineMock); - Assert.assertEquals(7, virtualMachineMaxNicsValue); + Assert.assertNull(virtualMachineMaxNicsValue); } void updateFinalStaticField(Field field, Object newValue) throws IllegalArgumentException, IllegalAccessException, NoSuchFieldException, SecurityException { diff --git a/server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java b/server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java index 4d7c004e1934..ddc3077c6a5d 100644 --- a/server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java +++ b/server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java @@ -1976,7 +1976,13 @@ protected boolean existsVpcDomainRouterWithSufficientNicCapacity(long vpcId) { int countRouterDefaultNetworks = domainRouterJoinDao.countDefaultNetworksById(vpcDomainRouter.getId()); long countVpcNetworks = _ntwkDao.countVpcNetworks(vpcId); - int totalNicsAvailable = networkOrchestrationService.getVirtualMachineMaxNicsValue(vpcDomainRouter) - countRouterDefaultNetworks; + Integer routerMaxNicsValue = networkOrchestrationService.getVirtualMachineMaxNicsValue(vpcDomainRouter); + + if (routerMaxNicsValue == null) { + return true; + } + + int totalNicsAvailable = routerMaxNicsValue - countRouterDefaultNetworks; return totalNicsAvailable > countVpcNetworks; } diff --git a/server/src/test/java/com/cloud/network/vpc/VpcManagerImplTest.java b/server/src/test/java/com/cloud/network/vpc/VpcManagerImplTest.java index d325295c9b17..4b6dd5d53dbc 100644 --- a/server/src/test/java/com/cloud/network/vpc/VpcManagerImplTest.java +++ b/server/src/test/java/com/cloud/network/vpc/VpcManagerImplTest.java @@ -536,6 +536,18 @@ public void existsVpcDomainRouterWithSufficientNicCapacityTestUnavailableRouters Assert.assertFalse(result); } + @Test + public void existsVpcDomainRouterWithSufficientNicCapacityTestRouterIncompatibleHypervisorTypeReturnsTrue() { + Mockito.when(routerDao.findOneByVpcId(vpcId)).thenReturn(domainRouterVOMock); + Mockito.when(domainRouterVOMock.getId()).thenReturn(1L); + Mockito.when(domainRouterJoinDaoMock.countDefaultNetworksById(1L)).thenReturn(2); + Mockito.when(networkOrchestrationServiceMock.getVirtualMachineMaxNicsValue(domainRouterVOMock)).thenReturn(null); + + boolean result = manager.existsVpcDomainRouterWithSufficientNicCapacity(vpcId); + + Assert.assertTrue(result); + } + @Test public void existsVpcDomainRouterWithSufficientNicCapacityTestAvailableRouterReturnsTrue() { Mockito.when(networkDao.countVpcNetworks(vpcId)).thenReturn(6L); diff --git a/server/src/test/java/com/cloud/vpc/MockNetworkManagerImpl.java b/server/src/test/java/com/cloud/vpc/MockNetworkManagerImpl.java index 853756686b4d..6ae32b413bed 100644 --- a/server/src/test/java/com/cloud/vpc/MockNetworkManagerImpl.java +++ b/server/src/test/java/com/cloud/vpc/MockNetworkManagerImpl.java @@ -1051,7 +1051,7 @@ public void unmanageNics(VirtualMachineProfile vm) { } @Override - public int getVirtualMachineMaxNicsValue(VirtualMachine virtualMachine) { + public Integer getVirtualMachineMaxNicsValue(VirtualMachine virtualMachine) { return 0; } diff --git a/test/integration/component/test_vpc_network.py b/test/integration/component/test_vpc_network.py index c3e571f16c3d..377e88a7ba75 100644 --- a/test/integration/component/test_vpc_network.py +++ b/test/integration/component/test_vpc_network.py @@ -191,7 +191,7 @@ def __init__(self): "name": "Test Network", "displaytext": "Test Network", "netmask": '255.255.255.0', - # Max networks allowed per VPC: Xenserver -> 7, VMWare -> 10 & KVM -> 27 + # Max networks allowed per hypervisor: Xenserver -> 7, VMWare -> 10 & KVM -> 27 "limit": 5, }, "lbrule": { From af3f0a8364e2329aa76160d56566c10e64d9cf7b Mon Sep 17 00:00:00 2001 From: Henrique Sato Date: Thu, 2 May 2024 17:46:32 -0300 Subject: [PATCH 15/16] Change tests file --- .github/workflows/ci.yml | 3 +- .../integration/component/test_vpc_network.py | 313 +------------- .../smoke/test_vpc_hypervisor_max_networks.py | 391 ++++++++++++++++++ 3 files changed, 394 insertions(+), 313 deletions(-) create mode 100644 test/integration/smoke/test_vpc_hypervisor_max_networks.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c4196f06d4b3..a8c126bfc80f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -140,7 +140,8 @@ jobs: smoke/test_vpc_ipv6 smoke/test_vpc_redundant smoke/test_vpc_router_nics - smoke/test_vpc_vpn", + smoke/test_vpc_vpn + smoke/test_vpc_hypervisor_max_networks", "component/find_hosts_for_migration component/test_acl_isolatednetwork component/test_acl_isolatednetwork_delete diff --git a/test/integration/component/test_vpc_network.py b/test/integration/component/test_vpc_network.py index 377e88a7ba75..c8f03794af9b 100644 --- a/test/integration/component/test_vpc_network.py +++ b/test/integration/component/test_vpc_network.py @@ -34,12 +34,7 @@ LoadBalancerRule, Router, StaticNATRule, - NetworkACL, - PublicIPAddress, - Host, - Cluster, - Configurations, - Resources) + NetworkACL) from marvin.lib.common import (get_zone, get_domain, get_template, @@ -191,8 +186,6 @@ def __init__(self): "name": "Test Network", "displaytext": "Test Network", "netmask": '255.255.255.0', - # Max networks allowed per hypervisor: Xenserver -> 7, VMWare -> 10 & KVM -> 27 - "limit": 5, }, "lbrule": { "name": "SSH", @@ -2761,307 +2754,3 @@ def test_stop_start_vpc_router(self): if (exceptionOccurred or (not isRouterInDesiredState)): self.fail(exceptionMessage) return - - -class TestVPCNetworkHypervisorLimit(cloudstackTestCase): - - @classmethod - def setUpClass(cls): - cls.testClient = super(TestVPCNetworkHypervisorLimit, cls).getClsTestClient() - cls.api_client = cls.testClient.getApiClient() - - cls.services = Services().services - - cls.domain = get_domain(cls.api_client) - cls.zone = get_zone(cls.api_client, cls.testClient.getZoneForTests()) - - cls._cleanup = [] - - return - - def setUp(self): - self.apiclient = self.testClient.getApiClient() - self.dbclient = self.testClient.getDbConnection() - self.account = Account.create( - self.apiclient, - self.services["account"], - admin=True, - domainid=self.domain.id - ) - self.cleanup = [self.account] - - return - - @classmethod - def tearDownClass(cls): - super(TestVPCNetworkHypervisorLimit, cls).tearDownClass() - - def tearDown(self): - super(TestVPCNetworkHypervisorLimit, self).tearDown() - - def validate_vpc_network(self, network, state=None): - """Validates the VPC network""" - - self.debug("Checking if the VPC network was created successfully.") - vpc_networks = VPC.list( - self.apiclient, - id=network.id - ) - self.assertEqual( - isinstance(vpc_networks, list), - True, - "List VPC network should return a valid list" - ) - self.assertEqual( - network.name, - vpc_networks[0].name, - "Name of the VPC network should match with listVPC data" - ) - if state: - self.assertEqual( - vpc_networks[0].state, - state, - "VPC state should be '%s'" % state - ) - self.debug("VPC network validated - %s" % network.name) - - return - - def validate_vpc_offering(self, vpc_offering): - """Validates the VPC offering""" - - self.debug("Checking if the VPC offering was created successfully.") - vpc_offs = VpcOffering.list( - self.apiclient, - id=vpc_offering.id - ) - self.assertEqual( - isinstance(vpc_offs, list), - True, - "List VPC offerings should return a valid list" - ) - self.assertEqual( - vpc_offering.name, - vpc_offs[0].name, - "Name of the VPC offering should match with listVPCOff data" - ) - self.debug("VPC offering was created successfully - %s" % vpc_offering.name) - - return - - def create_network_max_limit_hypervisor_test(self, hypervisor): - Configurations.update( - self.apiclient, - accountid=self.account.id, - name="vpc.max.networks", - value=30 - ) - - self.debug("Creating a VPC offering.") - vpc_off = VpcOffering.create( - self.apiclient, - self.services["vpc_offering"] - ) - - self._cleanup.append(vpc_off) - self.validate_vpc_offering(vpc_off) - - self.debug("Enabling the VPC offering.") - vpc_off.update(self.apiclient, state='Enabled') - - self.debug("Creating a VPC network in the account: %s" % self.account.name) - self.services["vpc"]["cidr"] = '10.1.1.1/16' - vpc = VPC.create( - self.apiclient, - self.services["vpc"], - vpcofferingid=vpc_off.id, - zoneid=self.zone.id, - account=self.account.name, - domainid=self.account.domainid - ) - self.validate_vpc_network(vpc) - - vpc_router = Router.list( - self.apiclient, - vpcId=vpc.id, - listAll=True - )[0] - - host = Host.list( - self.apiclient, - id=vpc_router.hostId - )[0] - - cluster = Cluster.list( - self.apiclient, - id=host.clusterId - )[0] - - cluster_old_hypervisor_type = cluster.hypervisortype - - self.debug("Updating cluster %s hypervisor from %s to %s." % - (cluster.name, cluster_old_hypervisor_type, hypervisor)) - Cluster.update( - self.apiclient, - id=cluster.id, - hypervisor=hypervisor - ) - - network_offering = NetworkOffering.create( - self.apiclient, - self.services["network_offering"], - conservemode=False - ) - # Enable Network offering - self._cleanup.append(network_offering) - network_offering.update(self.apiclient, state='Enabled') - - # Empty list to store all networks - networks = [] - - # Creating network using the network offering created - self.debug("Creating network with network offering: %s." % network_offering.id) - network_1 = Network.create( - self.apiclient, - self.services["network"], - accountid=self.account.name, - domainid=self.account.domainid, - networkofferingid=network_offering.id, - zoneid=self.zone.id, - gateway='10.1.0.1', - vpcid=vpc.id - ) - - self.debug("Created network with ID: %s." % network_1.id) - - config_name = "virtual.machine.max.nics.%s" % hypervisor.lower() - - configs = Configurations.list( - self.apiclient, - name=config_name, - listall=True - ) - if not isinstance(configs, list): - raise Exception("Failed to find %s virtual machine max NICs." % hypervisor) - - self.services["network"]["limit"] = int(configs[0].value) - - self.debug("Updating network resource limit for account %s with value %s." % ( - self.account.name, self.services["network"]["limit"])) - Resources.updateLimit( - self.apiclient, - resourcetype=6, - account=self.account.name, - domainid=self.domain.id, - max=self.services["network"]["limit"] - ) - - # Create networks till max limit of hypervisor - for i in range(self.services["network"]["limit"] - 3): - # Creating network using the network offering created - self.debug("Creating network with network offering: %s." % - network_offering.id) - gateway = '10.1.' + str(i + 1) + '.1' - self.debug("Gateway for new network: %s." % gateway) - - network = Network.create( - self.apiclient, - self.services["network"], - accountid=self.account.name, - domainid=self.account.domainid, - networkofferingid=network_offering.id, - zoneid=self.zone.id, - gateway=gateway, - vpcid=vpc.id - ) - self.debug("Created network with ID: %s." % network.id) - networks.append(network) - - self.debug( - "Trying to create one more network than %s hypervisor limit in VPC: %s." % (hypervisor, vpc.name)) - gateway = '10.1.' + str(self.services["network"]["limit"]) + '.1' - - with self.assertRaises(Exception): - Network.create( - self.apiclient, - self.services["network"], - accountid=self.account.name, - domainid=self.account.domainid, - networkofferingid=network_offering.id, - zoneid=self.zone.id, - gateway=gateway, - vpcid=vpc.id - ) - - self.debug("Deleting one of the existing networks.") - try: - network_1.delete(self.apiclient) - except Exception as e: - self.fail("Failed to delete network: %s - %s." % - (network_1.name, e)) - - self.debug("Creating a new network in VPC: %s." % vpc.name) - network = Network.create( - self.apiclient, - self.services["network"], - accountid=self.account.name, - domainid=self.account.domainid, - networkofferingid=network_offering.id, - zoneid=self.zone.id, - gateway=gateway, - vpcid=vpc.id - ) - self.debug("Created a new network: %s." % network.name) - networks.append(network) - - self.debug( - "Updating cluster %s hypervisor to its old hypervisor %s." % (cluster.name, cluster_old_hypervisor_type)) - Cluster.update( - self.apiclient, - id=cluster.id, - hypervisor=cluster_old_hypervisor_type - ) - - return - - @attr(tags=["advanced", "intervlan"], required_hardware="true") - def test_01_create_network_max_limit_xenserver(self): - """ Test create networks in VPC up to maximum limit for XenServer hypervisor. - """ - - # Validate the following - # 1. Create a VPC and add maximum # of supported networks to the VPC; - # 2. After reaching the maximum networks, adding another network to the VPC must raise an exception; - # 3. Delete a network from VPC; - # 4. Add a new network to the VPC. - - self.create_network_max_limit_hypervisor_test("XenServer") - return - - @attr(tags=["advanced", "intervlan"], required_hardware="true") - def test_02_create_network_max_limit_vmware(self): - """ Test create networks in VPC up to maximum limit for VMware hypervisor. - """ - - # Validate the following - # 1. Create a VPC and add maximum # of supported networks to the VPC; - # 2. After reaching the maximum networks, adding another network to the VPC must raise an exception; - # 3. Delete a network from VPC; - # 4. Add a new network to the VPC. - - self.create_network_max_limit_hypervisor_test("VMware") - return - - @attr(tags=["advanced", "intervlan"], required_hardware="true") - def test_03_create_network_max_limit_kvm(self): - """ Test create networks in VPC up to maximum limit for KVM hypervisor. - """ - - # Validate the following - # 1. Create a VPC and add maximum # of supported networks to the VPC; - # 2. After reaching the maximum networks, adding another network to the VPC must raise an exception; - # 3. Delete a network from VPC; - # 4. Add a new network to the VPC. - - self.create_network_max_limit_hypervisor_test("KVM") - return diff --git a/test/integration/smoke/test_vpc_hypervisor_max_networks.py b/test/integration/smoke/test_vpc_hypervisor_max_networks.py new file mode 100644 index 000000000000..9618d1ac18f0 --- /dev/null +++ b/test/integration/smoke/test_vpc_hypervisor_max_networks.py @@ -0,0 +1,391 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +""" Test VPC max networks based on the hypervisor limits """ + +from nose.plugins.attrib import attr +from marvin.cloudstackTestCase import cloudstackTestCase +from marvin.lib.base import (Account, + NetworkOffering, + Network, + VPC, + VpcOffering, + Router, + Host, + Cluster, + Configurations, + Resources) +from marvin.lib.common import (get_zone, + get_domain) + + +class Services: + def __init__(self): + self.services = { + "account": { + "email": "test@test.com", + "firstname": "Test", + "lastname": "User", + "username": "test", + # Random characters are appended for unique + # username + "password": "password", + }, + "network_offering": { + "name": 'VPC Network offering', + "displaytext": 'VPC Network off', + "guestiptype": 'Isolated', + "supportedservices": 'Vpn,Dhcp,Dns,SourceNat,PortForwarding,Lb,UserData,StaticNat,NetworkACL', + "traffictype": 'GUEST', + "availability": 'Optional', + "useVpc": 'on', + "serviceProviderList": { + "Vpn": 'VpcVirtualRouter', + "Dhcp": 'VpcVirtualRouter', + "Dns": 'VpcVirtualRouter', + "SourceNat": 'VpcVirtualRouter', + "PortForwarding": 'VpcVirtualRouter', + "Lb": 'VpcVirtualRouter', + "UserData": 'VpcVirtualRouter', + "StaticNat": 'VpcVirtualRouter', + "NetworkACL": 'VpcVirtualRouter' + }, + "serviceCapabilityList": { + "SourceNat": {"SupportedSourceNatTypes": "peraccount"}, + }, + }, + "vpc_offering": { + "name": 'VPC off', + "displaytext": 'VPC off', + "supportedservices": 'Dhcp,Dns,SourceNat,PortForwarding,Vpn,Lb,UserData,StaticNat', + }, + "vpc": { + "name": "TestVPC", + "displaytext": "TestVPC", + "cidr": '10.0.0.1/24' + }, + "network": { + "name": "Test Network", + "displaytext": "Test Network", + "netmask": '255.255.255.0', + # Max networks allowed per hypervisor: Xenserver -> 7, VMWare -> 10 & KVM -> 27 + "limit": 5, + } + } + + +class TestVpcHypervisorMaxNetworks(cloudstackTestCase): + @classmethod + def setUpClass(cls): + cls.testClient = super(TestVpcHypervisorMaxNetworks, cls).getClsTestClient() + cls.api_client = cls.testClient.getApiClient() + + cls.services = Services().services + + cls.domain = get_domain(cls.api_client) + cls.zone = get_zone(cls.api_client, cls.testClient.getZoneForTests()) + + cls._cleanup = [] + + return + + def setUp(self): + self.apiclient = self.testClient.getApiClient() + self.dbclient = self.testClient.getDbConnection() + self.account = Account.create( + self.apiclient, + self.services["account"], + admin=True, + domainid=self.domain.id + ) + self.cleanup = [self.account] + + return + + @classmethod + def tearDownClass(cls): + super(TestVpcHypervisorMaxNetworks, cls).tearDownClass() + + def tearDown(self): + super(TestVpcHypervisorMaxNetworks, self).tearDown() + + def validate_vpc_network(self, network, state=None): + """Validates the VPC network""" + + self.debug("Checking if the VPC network was created successfully.") + vpc_networks = VPC.list( + self.apiclient, + id=network.id + ) + self.assertEqual( + isinstance(vpc_networks, list), + True, + "List VPC network should return a valid list" + ) + self.assertEqual( + network.name, + vpc_networks[0].name, + "Name of the VPC network should match with listVPC data" + ) + if state: + self.assertEqual( + vpc_networks[0].state, + state, + "VPC state should be '%s'" % state + ) + self.debug("VPC network validated - %s" % network.name) + + return + + def validate_vpc_offering(self, vpc_offering): + """Validates the VPC offering""" + + self.debug("Checking if the VPC offering was created successfully.") + vpc_offs = VpcOffering.list( + self.apiclient, + id=vpc_offering.id + ) + self.assertEqual( + isinstance(vpc_offs, list), + True, + "List VPC offerings should return a valid list" + ) + self.assertEqual( + vpc_offering.name, + vpc_offs[0].name, + "Name of the VPC offering should match with listVPCOff data" + ) + self.debug("VPC offering was created successfully - %s" % vpc_offering.name) + + return + + def create_network_max_limit_hypervisor_test(self, hypervisor): + Configurations.update( + self.apiclient, + accountid=self.account.id, + name="vpc.max.networks", + value=30 + ) + + self.debug("Creating a VPC offering.") + vpc_off = VpcOffering.create( + self.apiclient, + self.services["vpc_offering"] + ) + + self._cleanup.append(vpc_off) + self.validate_vpc_offering(vpc_off) + + self.debug("Enabling the VPC offering.") + vpc_off.update(self.apiclient, state='Enabled') + + self.debug("Creating a VPC network in the account: %s" % self.account.name) + self.services["vpc"]["cidr"] = '10.1.1.1/16' + vpc = VPC.create( + self.apiclient, + self.services["vpc"], + vpcofferingid=vpc_off.id, + zoneid=self.zone.id, + account=self.account.name, + domainid=self.account.domainid + ) + self.validate_vpc_network(vpc) + + vpc_router = Router.list( + self.apiclient, + vpcId=vpc.id, + listAll=True + )[0] + + host = Host.list( + self.apiclient, + id=vpc_router.hostId + )[0] + + cluster = Cluster.list( + self.apiclient, + id=host.clusterId + )[0] + + cluster_old_hypervisor_type = cluster.hypervisortype + + self.debug("Updating cluster %s hypervisor from %s to %s." % + (cluster.name, cluster_old_hypervisor_type, hypervisor)) + Cluster.update( + self.apiclient, + id=cluster.id, + hypervisor=hypervisor + ) + + network_offering = NetworkOffering.create( + self.apiclient, + self.services["network_offering"], + conservemode=False + ) + # Enable Network offering + self._cleanup.append(network_offering) + network_offering.update(self.apiclient, state='Enabled') + + # Empty list to store all networks + networks = [] + + # Creating network using the network offering created + self.debug("Creating network with network offering: %s." % network_offering.id) + network_1 = Network.create( + self.apiclient, + self.services["network"], + accountid=self.account.name, + domainid=self.account.domainid, + networkofferingid=network_offering.id, + zoneid=self.zone.id, + gateway='10.1.0.1', + vpcid=vpc.id + ) + + self.debug("Created network with ID: %s." % network_1.id) + + config_name = "virtual.machine.max.nics.%s" % hypervisor.lower() + + configs = Configurations.list( + self.apiclient, + name=config_name, + listall=True + ) + if not isinstance(configs, list): + raise Exception("Failed to find %s virtual machine max NICs." % hypervisor) + + self.services["network"]["limit"] = int(configs[0].value) + + self.debug("Updating network resource limit for account %s with value %s." % ( + self.account.name, self.services["network"]["limit"])) + Resources.updateLimit( + self.apiclient, + resourcetype=6, + account=self.account.name, + domainid=self.domain.id, + max=self.services["network"]["limit"] + ) + + # Create networks till max limit of hypervisor + for i in range(self.services["network"]["limit"] - 3): + # Creating network using the network offering created + self.debug("Creating network with network offering: %s." % + network_offering.id) + gateway = '10.1.' + str(i + 1) + '.1' + self.debug("Gateway for new network: %s." % gateway) + + network = Network.create( + self.apiclient, + self.services["network"], + accountid=self.account.name, + domainid=self.account.domainid, + networkofferingid=network_offering.id, + zoneid=self.zone.id, + gateway=gateway, + vpcid=vpc.id + ) + self.debug("Created network with ID: %s." % network.id) + networks.append(network) + + self.debug( + "Trying to create one more network than %s hypervisor limit in VPC: %s." % (hypervisor, vpc.name)) + gateway = '10.1.' + str(self.services["network"]["limit"]) + '.1' + + with self.assertRaises(Exception): + Network.create( + self.apiclient, + self.services["network"], + accountid=self.account.name, + domainid=self.account.domainid, + networkofferingid=network_offering.id, + zoneid=self.zone.id, + gateway=gateway, + vpcid=vpc.id + ) + + self.debug("Deleting one of the existing networks.") + try: + network_1.delete(self.apiclient) + except Exception as e: + self.fail("Failed to delete network: %s - %s." % + (network_1.name, e)) + + self.debug("Creating a new network in VPC: %s." % vpc.name) + network = Network.create( + self.apiclient, + self.services["network"], + accountid=self.account.name, + domainid=self.account.domainid, + networkofferingid=network_offering.id, + zoneid=self.zone.id, + gateway=gateway, + vpcid=vpc.id + ) + self.debug("Created a new network: %s." % network.name) + networks.append(network) + + self.debug( + "Updating cluster %s hypervisor to its old hypervisor %s." % (cluster.name, cluster_old_hypervisor_type)) + Cluster.update( + self.apiclient, + id=cluster.id, + hypervisor=cluster_old_hypervisor_type + ) + + return + + @attr(tags=["advanced", "intervlan"], required_hardware="true") + def test_01_create_network_max_limit_xenserver(self): + """ Test create networks in VPC up to maximum limit for XenServer hypervisor. + """ + + # Validate the following + # 1. Create a VPC and add maximum # of supported networks to the VPC; + # 2. After reaching the maximum networks, adding another network to the VPC must raise an exception; + # 3. Delete a network from VPC; + # 4. Add a new network to the VPC. + + self.create_network_max_limit_hypervisor_test("XenServer") + return + + @attr(tags=["advanced", "intervlan"], required_hardware="true") + def test_02_create_network_max_limit_vmware(self): + """ Test create networks in VPC up to maximum limit for VMware hypervisor. + """ + + # Validate the following + # 1. Create a VPC and add maximum # of supported networks to the VPC; + # 2. After reaching the maximum networks, adding another network to the VPC must raise an exception; + # 3. Delete a network from VPC; + # 4. Add a new network to the VPC. + + self.create_network_max_limit_hypervisor_test("VMware") + return + + @attr(tags=["advanced", "intervlan"], required_hardware="true") + def test_03_create_network_max_limit_kvm(self): + """ Test create networks in VPC up to maximum limit for KVM hypervisor. + """ + + # Validate the following + # 1. Create a VPC and add maximum # of supported networks to the VPC; + # 2. After reaching the maximum networks, adding another network to the VPC must raise an exception; + # 3. Delete a network from VPC; + # 4. Add a new network to the VPC. + + self.create_network_max_limit_hypervisor_test("KVM") + return From b241a4c69f56e9a98d62cd5f64721d691409bef1 Mon Sep 17 00:00:00 2001 From: Henrique Sato Date: Thu, 2 May 2024 17:51:05 -0300 Subject: [PATCH 16/16] Restore old tests file --- test/integration/component/test_vpc_network.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/test/integration/component/test_vpc_network.py b/test/integration/component/test_vpc_network.py index c8f03794af9b..db436bbd3983 100644 --- a/test/integration/component/test_vpc_network.py +++ b/test/integration/component/test_vpc_network.py @@ -34,7 +34,8 @@ LoadBalancerRule, Router, StaticNATRule, - NetworkACL) + NetworkACL, + PublicIPAddress) from marvin.lib.common import (get_zone, get_domain, get_template, @@ -50,6 +51,7 @@ class Services: + """Test VPC network services """ @@ -185,7 +187,7 @@ def __init__(self): "network": { "name": "Test Network", "displaytext": "Test Network", - "netmask": '255.255.255.0', + "netmask": '255.255.255.0' }, "lbrule": { "name": "SSH",