Remove unused import & improve VR NIC validation

Henrique Sato · Henrique Sato · commit e86f28b9023a · 2024-04-10T15:42:43.000-03:00
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
@@ -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
@@ -37,4 +37,6 @@ public interface DomainRouterJoinDao extends GenericDao<DomainRouterJoinVO, Long
     List<DomainRouterJoinVO> searchByIds(Long... ids);
 
     List<DomainRouterJoinVO> 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
@@ -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<DomainRouterJoinVO,
     private AnnotationDao annotationDao;
 
     private final SearchBuilder<DomainRouterJoinVO> vrSearch;
-
     private final SearchBuilder<DomainRouterJoinVO> vrIdSearch;
     private final SearchBuilder<DomainRouterJoinVO> vrIdTrafficSearch;
+    private final GenericSearchBuilder<DomainRouterJoinVO, Integer> 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<DomainRouterJoinVO> getRouterByIdAndTrafficType(Long id, TrafficType
         return searchIncludingRemoved(sc, null, null, false);
     }
 
+    @Override
+    public int countDefaultNetworksById(long routerId) {
+        SearchCriteria<Integer> sc = defaultNetworksSearch.create();
+        sc.setParameters("id", routerId);
+        final List<Integer> count = customSearch(sc, null);
+        return count.get(0);
+    }
+
     @Override
     public List<DomainRouterJoinVO> 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
@@ -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<VpcProvider> 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
@@ -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);

Original file line number	Diff line number	Diff line change
`@@ -107,6 +107,8 @@ protected void init() {`
`107`	`107`	`PeerRouterSearch.and("macAddress", PeerRouterSearch.entity().getMacAddress(), Op.EQ);`
`108`	`108`	`PeerRouterSearch.and("vmType", PeerRouterSearch.entity().getVmType(), Op.EQ);`
`109`	`109`	`PeerRouterSearch.done();`
	`110`	`+`
	`111`	`+`
`110`	`112`	`}`
`111`	`113`
`112`	`114`	`@Override`
Original file line number	Diff line number	Diff line change
`@@ -37,4 +37,6 @@ public interface DomainRouterJoinDao extends GenericDao<DomainRouterJoinVO, Long`
`37`	`37`	`List<DomainRouterJoinVO> searchByIds(Long... ids);`
`38`	`38`
`39`	`39`	`List<DomainRouterJoinVO> getRouterByIdAndTrafficType(Long id, Networks.TrafficType... trafficType);`
	`40`	`+`
	`41`	`+ int countDefaultNetworksById(long routerId);`
`40`	`42`	`}`