use of iptables -C option (and some new locatiosn to address

Author: DaanHoogland

services/secondary-storage/controller/src/main/java/org/apache/cloudstack/secondarystorage/SecondaryStorageManagerImpl.java

@@ -531,8 +531,8 @@ public SecondaryStorageVmVO startNew(long dataCenterId, SecondaryStorageVm.Role
 
     /**
      * Get the default network for the secondary storage VM, based on the zone it is in. Delegates to
-     * either {@link #getDefaultNetworkForZone(DataCenter)} or {@link #getDefaultNetworkForAdvancedSGZone(DataCenter)},
-     * depending on the zone network type and whether or not security groups are enabled in the zone.
+     * either {@link #getDefaultNetworkForAdvancedZone(DataCenter)} or {@link #getDefaultNetworkForBasicZone(DataCenter)},
+     * depending on the zone network type and whether security groups are enabled in the zone.
      * @param dc - The zone (DataCenter) of the secondary storage VM.
      * @return The default network for use with the secondary storage VM.
      */

services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java

@@ -2287,28 +2287,39 @@ public synchronized String allowOutgoingOnPrivate(String destCidr) {
         if (!_inSystemVM) {
             return null;
         }
-        Script command = new Script("/bin/bash", s_logger);
         String intf = "eth1";
-        command.add("-c");
-        command.add("iptables -D OUTPUT -o " + intf + " -d " + destCidr + " -p tcp -m state --state NEW -m tcp  -j ACCEPT");
+        String rule =  String.format("OUTPUT -o %s -d %s -p tcp -m state --state NEW -m tcp  -j ACCEPT", intf, destCidr);
 
-        /* ignore the String result =*/ command.execute();
-
-        command = new Script("/bin/bash", s_logger);
-        command.add("-c");
-        command.add("iptables -I OUTPUT -o " + intf + " -d " + destCidr + " -p tcp -m state --state NEW -m tcp  -j ACCEPT");
+        if (ruleNeedsAdding(rule)) {
+            Script command = new Script("/bin/bash", s_logger);
+            command.add("-c");
+            command.add("iptables -I");
+            command.add(rule);
 
-        String result = command.execute();
-        if (result != null) {
-            s_logger.warn("Error in allowing outgoing to " + destCidr + ", err=" + result);
-            return "Error in allowing outgoing to " + destCidr + ", err=" + result;
+            String result = command.execute();
+            if (result != null) {
+                s_logger.warn("Error in allowing outgoing to " + destCidr + ", err=" + result);
+                return "Error in allowing outgoing to " + destCidr + ", err=" + result;
+            }
+        } else {
+            s_logger.warn("Rule already defined in SVM: Error in allowing outgoing to " + destCidr);
         }
 
         addRouteToInternalIpOrCidr(_localgw, _eth1ip, _eth1mask, destCidr);
 
         return null;
     }
 
+    private boolean ruleNeedsAdding(String rule) {
+        Script command = new Script("/bin/bash", s_logger);
+        command.add("-c");
+        command.add("iptables -C");
+        command.add("rule");
+
+        String r1 = command.execute();
+        return (r1 != null && r1.contains("iptables: Bad rule (does a matching rule exist in that chain?)."));
+    }
+
     private Answer execute(SecStorageFirewallCfgCommand cmd) {
         if (!_inSystemVM) {
             return new Answer(cmd, true, null);
@@ -2838,16 +2849,16 @@ private void startAdditionalServices() {
         if (result != null) {
             s_logger.warn("Error in starting sshd service err=" + result);
         }
-        command = new Script("/bin/bash", s_logger);
-        command.add("-c");
-        command.add("iptables -D INPUT -i eth1 -p tcp -m state --state NEW -m tcp --dport 3922 -j ACCEPT");
-        /* ignore result = */ command.execute();
-        command = new Script("/bin/bash", s_logger);
-        command.add("-c");
-        command.add("iptables -I INPUT -i eth1 -p tcp -m state --state NEW -m tcp --dport 3922 -j ACCEPT");
-        result = command.execute();
-        if (result != null) {
-            s_logger.warn("Error in opening up ssh port err=" + result);
+        String rule = "INPUT -i eth1 -p tcp -m state --state NEW -m tcp --dport 3922 -j ACCEPT";
+        if (ruleNeedsAdding(rule)) {
+            command = new Script("/bin/bash", s_logger);
+            command.add("-c");
+            command.add("iptables -I");
+            command.add(rule);
+            result = command.execute();
+            if (result != null) {
+                s_logger.warn("Error in opening up ssh port err=" + result);
+            }
         }
     }
 

services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/template/DownloadManagerImpl.java

@@ -1083,6 +1083,7 @@ public boolean configure(String name, Map<String, Object> params) throws Configu
     }
 
     private void blockOutgoingOnPrivate() {
+        // TODO add a check if rule exists
         Script command = new Script("/bin/bash", LOGGER);
         String intf = "eth1";
         command.add("-c");
@@ -1122,6 +1123,7 @@ private void startAdditionalServices() {
         String port = Integer.toString(TemplateConstants.DEFAULT_TMPLT_COPY_PORT);
         String intf = TemplateConstants.DEFAULT_TMPLT_COPY_INTF;
 
+        // TODO add a check if rule exists
         command = new Script("/bin/bash", LOGGER);
         command.add("-c");
         command.add("iptables -I INPUT -i " + intf + " -p tcp -m state --state NEW -m tcp --dport " + port + " -j ACCEPT;" + "iptables -I INPUT -i " + intf +