From 90df4e4df0ccece0693cc862c2f59214f99518cf Mon Sep 17 00:00:00 2001
From: Alena Prokharchyk <alena.prokharchyk@citrix.com>
Date: Tue, 4 Jun 2013 14:24:49 -0700
Subject: [PATCH] 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

---
 .../dao/ApplicationLoadBalancerRuleDao.java   |  1 +
 .../ApplicationLoadBalancerRuleDaoImpl.java   | 22 +++++++++++++++++--
 .../element/InternalLoadBalancerElement.java  | 18 +++++----------
 3 files changed, 26 insertions(+), 15 deletions(-)

diff --git a/engine/schema/src/org/apache/cloudstack/lb/dao/ApplicationLoadBalancerRuleDao.java b/engine/schema/src/org/apache/cloudstack/lb/dao/ApplicationLoadBalancerRuleDao.java
index c385e62f6ab..47f1d361216 100644
--- 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
index 880c67e732c..6036b5a2d60 100644
--- 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.apache.cloudstack.lb.ApplicationLoadBalancerRuleVO;
 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 @@ public class ApplicationLoadBalancerRuleDaoImpl extends GenericDaoBase<Applicati
         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 class ApplicationLoadBalancerRuleDaoImpl extends GenericDaoBase<Applicati
         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
index 4b9308b6606..14b616cb749 100644
--- 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.element.VirtualRouterProviderVO;
 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 @@ public class InternalLoadBalancerElement extends AdapterBase implements LoadBala
         //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;
     }