CLOUDSTACK-9317: Update review comments and rule state column

Author: jayapalu

engine/schema/src/com/cloud/network/dao/IPAddressVO.java

@@ -117,6 +117,7 @@ public class IPAddressVO implements IpAddress {
     @Column(name = "display", updatable = true, nullable = false)
     protected boolean display = true;
 
+    //static nat rule state
     @Enumerated(value = EnumType.STRING)
     @Column(name = "rule_state")
     State ruleState;

plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java

@@ -1771,7 +1771,7 @@ protected ExecutionResult cleanupNetworkElementCommand(final IpAssocCommand cmd)
                 }
                 nicNum = broadcastUriAllocatedToVM.get(ip.getBroadcastUri());
 
-                if (lastIp != null && !ip.isAdd()) {
+                if (org.apache.commons.lang.StringUtils.equalsIgnoreCase(lastIp, "true") && !ip.isAdd()) {
                     // in isolated network eth2 is the default public interface. We don't want to delete it.
                     if (nicNum != 2) {
                         vifHotUnPlug(conn, routerName, ip.getVifMacAddress());

plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/CitrixResourceBase.java

@@ -627,7 +627,7 @@ protected ExecutionResult cleanupNetworkElementCommand(final IpAssocCommand cmd)
 
                 // there is only one ip in this public vlan and removing it, so
                 // remove the nic
-                if (lastIp != null && !ip.isAdd()) {
+                if (org.apache.commons.lang.StringUtils.equalsIgnoreCase(lastIp, "true") && !ip.isAdd()) {
                     final VIF correctVif = getCorrectVif(conn, router, network);
                     // in isolated network eth2 is the default public interface. We don't want to delete it.
                     if (correctVif != null && !correctVif.getDevice(conn).equals("2")) {

server/src/com/cloud/network/router/CommandSetupHelper.java

@@ -176,8 +176,6 @@ public class CommandSetupHelper {
     @Inject
     private IPAddressDao _ipAddressDao;
     @Inject
-    private FirewallRulesDao _firewallsDao;
-    @Inject
     private GuestOSDao _guestOSDao;
 
     @Inject
@@ -854,11 +852,12 @@ public int compare(final PublicIpAddress o1, final PublicIpAddress o2) {
             int ipsWithrules = 0;
             int ipsStaticNat = 0;
             for (IPAddressVO ip : userIps) {
-                if ( _firewallsDao.countRulesByIpIdAndState(ip.getId(), FirewallRule.State.Active) > 0 ) {
+                if ( _rulesDao.countRulesByIpIdAndState(ip.getId(), FirewallRule.State.Active) > 0){
                     ipsWithrules++;
                 }
 
-                // check onetoonenat and also check if the ip "add":false. If there are 2 PF remove 1 static nat add
+                // check onetoonenat and also check if the ip "add":false. If there are 2 PF rules remove and
+                // 1 static nat rule add
                 if (ip.isOneToOneNat() && ip.getRuleState() == null) {
                     ipsStaticNat++;
                 }
@@ -871,10 +870,12 @@ public int compare(final PublicIpAddress o1, final PublicIpAddress o2) {
             final DataCenterVO dcVo = _dcDao.findById(router.getDataCenterId());
             cmd.setAccessDetail(NetworkElementCommand.ZONE_NETWORK_TYPE, dcVo.getNetworkType().toString());
 
-            // if there 1 static nat then it will be checked for remove at the resource
-            if (ipsWithrules == 0 && ipsStaticNat == 0 ) {
+            // if there is 1 static nat then it will be checked for remove at the resource
+            if (ipsWithrules == 0 && ipsStaticNat == 0) {
                 // there is only one ip address for the network.
                 cmd.setAccessDetail(NetworkElementCommand.NETWORK_PUB_LAST_IP, "true");
+            } else {
+                cmd.setAccessDetail(NetworkElementCommand.NETWORK_PUB_LAST_IP, "false");
             }
 
             cmds.addCommand(ipAssocCommand, cmd);