CLOUDSTACK-2840: get the latest information from the DB about the number of rules in non-revoked state for the ip address when figuring out if the internal lb vm needs to be destroyed. Instead of relying on the information passed down by the NetworkManager as the network manager might pass only rules in transition state omitting the Active rules

Alena Prokharchyk · Alena Prokharchyk · commit 90df4e4df0cc · 2013-06-04T14:41:23.000-07:00
diff --git a/engine/schema/src/org/apache/cloudstack/lb/dao/ApplicationLoadBalancerRuleDao.java b/engine/schema/src/org/apache/cloudstack/lb/dao/ApplicationLoadBalancerRuleDao.java
@@ -31,5 +31,6 @@ public interface ApplicationLoadBalancerRuleDao extends GenericDao<ApplicationLo
     long countBySourceIp(Ip sourceIp, long sourceIpNetworkId);
     List<ApplicationLoadBalancerRuleVO> listBySourceIpAndNotRevoked(Ip sourceIp, long sourceNetworkId);
     List<String> listLbIpsBySourceIpNetworkIdAndScheme(long sourceIpNetworkId, Scheme scheme);
+    long countBySourceIpAndNotRevoked(Ip sourceIp, long sourceIpNetworkId);
 
 }
diff --git a/engine/schema/src/org/apache/cloudstack/lb/dao/ApplicationLoadBalancerRuleDaoImpl.java b/engine/schema/src/org/apache/cloudstack/lb/dao/ApplicationLoadBalancerRuleDaoImpl.java
@@ -25,6 +25,7 @@
 import org.springframework.stereotype.Component;
 
 import com.cloud.network.rules.FirewallRule;
+import com.cloud.network.rules.FirewallRule.State;
 import com.cloud.network.rules.LoadBalancerContainer.Scheme;
 import com.cloud.utils.db.GenericDaoBase;
 import com.cloud.utils.db.GenericSearchBuilder;
@@ -41,8 +42,8 @@ public class ApplicationLoadBalancerRuleDaoImpl extends GenericDaoBase<Applicati
     final GenericSearchBuilder<ApplicationLoadBalancerRuleVO, String> listIps;
     final GenericSearchBuilder<ApplicationLoadBalancerRuleVO, Long> CountBy;
     protected final SearchBuilder<ApplicationLoadBalancerRuleVO> NotRevokedSearch;
-
-
+    final GenericSearchBuilder<ApplicationLoadBalancerRuleVO, Long> CountNotRevoked;
+    
     
     protected ApplicationLoadBalancerRuleDaoImpl() {
         AllFieldsSearch = createSearchBuilder();
@@ -69,6 +70,13 @@ protected ApplicationLoadBalancerRuleDaoImpl() {
         NotRevokedSearch.and("sourceIpNetworkId", NotRevokedSearch.entity().getSourceIpNetworkId(), SearchCriteria.Op.EQ);
         NotRevokedSearch.and("state", NotRevokedSearch.entity().getState(), SearchCriteria.Op.NEQ);
         NotRevokedSearch.done();
+        
+        CountNotRevoked = createSearchBuilder(Long.class);
+        CountNotRevoked.select(null, Func.COUNT, CountNotRevoked.entity().getId());
+        CountNotRevoked.and("sourceIp", CountNotRevoked.entity().getSourceIp(), Op.EQ);
+        CountNotRevoked.and("state", CountNotRevoked.entity().getState(), Op.NEQ);
+        CountNotRevoked.and("sourceIpNetworkId", CountNotRevoked.entity().getSourceIpNetworkId(), Op.EQ);
+        CountNotRevoked.done();
     }
 
     @Override
@@ -112,4 +120,14 @@ public List<String> listLbIpsBySourceIpNetworkIdAndScheme(long sourceIpNetworkId
         return customSearch(sc, null);
     }
 
+    @Override
+    public long countBySourceIpAndNotRevoked(Ip sourceIp, long sourceIpNetworkId) {
+        SearchCriteria<Long> sc = CountNotRevoked.create();
+        sc.setParameters("sourceIp", sourceIp);
+        sc.setParameters("sourceIpNetworkId", sourceIpNetworkId);
+        sc.setParameters("state", State.Revoke);
+        List<Long> results = customSearch(sc, null);
+        return results.get(0);
+    }
+
 }
diff --git a/plugins/network-elements/internal-loadbalancer/src/org/apache/cloudstack/network/element/InternalLoadBalancerElement.java b/plugins/network-elements/internal-loadbalancer/src/org/apache/cloudstack/network/element/InternalLoadBalancerElement.java
@@ -64,7 +64,6 @@
 import com.cloud.network.lb.LoadBalancingRule;
 import com.cloud.network.router.VirtualRouter;
 import com.cloud.network.router.VirtualRouter.Role;
-import com.cloud.network.rules.FirewallRule;
 import com.cloud.network.rules.LoadBalancerContainer;
 import com.cloud.network.rules.LoadBalancerContainer.Scheme;
 import com.cloud.offering.NetworkOffering;
@@ -394,23 +393,16 @@ protected Set<Ip> getVmsToDestroy(List<LoadBalancingRule> rules) {
         //1) Group rules by the source ip address as NetworkManager always passes the entire network lb config to the element
         Map<Ip, List<LoadBalancingRule>> groupedRules = groupBySourceIp(rules);
 
-        //2) Count rules in revoke state
         Set<Ip> vmsToDestroy = new HashSet<Ip>();
         
         for (Ip sourceIp : groupedRules.keySet()) {
+            //2) Check if there are non revoked rules for the source ip address
             List<LoadBalancingRule> rulesToCheck = groupedRules.get(sourceIp);
-            int revoke = 0;
-            for (LoadBalancingRule ruleToCheck : rulesToCheck) {
-                if (ruleToCheck.getState() == FirewallRule.State.Revoke){
-                    revoke++;
-                }
-            }
-            
-            if (revoke == rulesToCheck.size()) {
-                s_logger.debug("Have to destroy internal lb vm for source ip " + sourceIp);
+            if (_appLbDao.countBySourceIpAndNotRevoked(sourceIp, rulesToCheck.get(0).getNetworkId()) == 0) {
+                s_logger.debug("Have to destroy internal lb vm for source ip " + sourceIp + " as it has 0 rules in non-Revoke state");
                 vmsToDestroy.add(sourceIp);
-            } 
-        }        
+            }
+        }
         return vmsToDestroy;
     }
 

Original file line number	Diff line number	Diff line change
`@@ -31,5 +31,6 @@ public interface ApplicationLoadBalancerRuleDao extends GenericDao<ApplicationLo`
`31`	`31`	`long countBySourceIp(Ip sourceIp, long sourceIpNetworkId);`
`32`	`32`	`List<ApplicationLoadBalancerRuleVO> listBySourceIpAndNotRevoked(Ip sourceIp, long sourceNetworkId);`
`33`	`33`	`List<String> listLbIpsBySourceIpNetworkIdAndScheme(long sourceIpNetworkId, Scheme scheme);`
	`34`	`+ long countBySourceIpAndNotRevoked(Ip sourceIp, long sourceIpNetworkId);`
`34`	`35`
`35`	`36`	`}`