You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by GitBox <gi...@apache.org> on 2017/12/02 19:04:07 UTC

[GitHub] rhtyd closed pull request #1470: Make the generated json files unique to prevent concurrency issues

rhtyd closed pull request #1470: Make the generated json files unique to prevent concurrency issues
URL: https://github.com/apache/cloudstack/pull/1470
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/core/src/com/cloud/agent/resource/virtualnetwork/VirtualRoutingResource.java b/core/src/com/cloud/agent/resource/virtualnetwork/VirtualRoutingResource.java
index f3edc696759..731c895d902 100644
--- a/core/src/com/cloud/agent/resource/virtualnetwork/VirtualRoutingResource.java
+++ b/core/src/com/cloud/agent/resource/virtualnetwork/VirtualRoutingResource.java
@@ -159,6 +159,7 @@ private ExecutionResult applyConfigToVR(String routerAccessIp, ConfigItem c) {
     private ExecutionResult applyConfigToVR(String routerAccessIp, ConfigItem c, int timeout) {
         if (c instanceof FileConfigItem) {
             FileConfigItem configItem = (FileConfigItem)c;
+
             return _vrDeployer.createFileInVR(routerAccessIp, configItem.getFilePath(), configItem.getFileName(), configItem.getFileContents());
         } else if (c instanceof ScriptConfigItem) {
             ScriptConfigItem configItem = (ScriptConfigItem)c;
diff --git a/core/src/com/cloud/agent/resource/virtualnetwork/facade/AbstractConfigItemFacade.java b/core/src/com/cloud/agent/resource/virtualnetwork/facade/AbstractConfigItemFacade.java
index f017384b63c..3c34f15d4a8 100644
--- a/core/src/com/cloud/agent/resource/virtualnetwork/facade/AbstractConfigItemFacade.java
+++ b/core/src/com/cloud/agent/resource/virtualnetwork/facade/AbstractConfigItemFacade.java
@@ -22,6 +22,9 @@
 import java.util.Hashtable;
 import java.util.LinkedList;
 import java.util.List;
+import java.util.UUID;
+
+import org.apache.log4j.Logger;
 
 import com.cloud.agent.api.BumpUpPriorityCommand;
 import com.cloud.agent.api.SetupGuestNetworkCommand;
@@ -58,6 +61,8 @@
 
 public abstract class AbstractConfigItemFacade {
 
+    private static final Logger s_logger = Logger.getLogger(AbstractConfigItemFacade.class);
+
     private final static Gson gson;
 
     private static Hashtable<Class<? extends NetworkElementCommand>, AbstractConfigItemFacade> flyweight = new Hashtable<Class<? extends NetworkElementCommand>, AbstractConfigItemFacade>();
@@ -104,13 +109,25 @@ public static AbstractConfigItemFacade getInstance(final Class<? extends Network
         return instance;
     }
 
+
+    private static String appendUuidToJsonFiles(final String filename) {
+        String remoteFileName = new String(filename);
+        if (remoteFileName.endsWith("json")) {
+            remoteFileName += "." + UUID.randomUUID().toString();
+        }
+        return remoteFileName;
+    }
+
     protected List<ConfigItem> generateConfigItems(final ConfigBase configuration) {
         final List<ConfigItem> cfg = new LinkedList<>();
 
-        final ConfigItem configFile = new FileConfigItem(VRScripts.CONFIG_PERSIST_LOCATION, destinationFile, gson.toJson(configuration));
+        final String remoteFilename = appendUuidToJsonFiles(destinationFile);
+        s_logger.debug("Transformed filename " + destinationFile + " to " + remoteFilename);
+
+        final ConfigItem configFile = new FileConfigItem(VRScripts.CONFIG_PERSIST_LOCATION, remoteFilename, gson.toJson(configuration));
         cfg.add(configFile);
 
-        final ConfigItem updateCommand = new ScriptConfigItem(VRScripts.UPDATE_CONFIG, destinationFile);
+        final ConfigItem updateCommand = new ScriptConfigItem(VRScripts.UPDATE_CONFIG, remoteFilename);
         cfg.add(updateCommand);
 
         return cfg;
diff --git a/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java b/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java
index 883c8c99429..57ce60e8497 100755
--- a/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java
+++ b/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java
@@ -292,6 +292,9 @@ public ExecutionResult executeInVR(final String routerIp, final String script, f
         if (details == null) {
             details = parser.getLines();
         }
+
+        s_logger.debug("Executing script in VR " + script);
+
         return new ExecutionResult(command.getExitValue() == 0, details);
     }
 
@@ -300,6 +303,8 @@ public ExecutionResult createFileInVR(final String routerIp, final String path,
         final File permKey = new File("/root/.ssh/id_rsa.cloud");
         String error = null;
 
+        s_logger.debug("Creating file in VR " + filename);
+
         try {
             SshHelper.scpTo(routerIp, 3922, "root", permKey, null, path, content.getBytes(), filename, null);
         } catch (final Exception e) {
diff --git a/systemvm/patches/debian/config/opt/cloud/bin/configure.py b/systemvm/patches/debian/config/opt/cloud/bin/configure.py
index b5f65e733cb..d4e866334b4 100755
--- a/systemvm/patches/debian/config/opt/cloud/bin/configure.py
+++ b/systemvm/patches/debian/config/opt/cloud/bin/configure.py
@@ -17,33 +17,29 @@
 # specific language governing permissions and limitations
 # under the License.
 import sys
-import os
 import base64
 
-from merge import DataBag
-from pprint import pprint
-import subprocess
+from collections import OrderedDict
+
 import logging
 import re
-import time
-import shutil
+
 import os.path
 import os
 from fcntl import flock, LOCK_EX, LOCK_UN
 
-from cs.CsDatabag import CsDataBag, CsCmdLine
-import cs.CsHelper
+from cs.CsDatabag import CsDataBag
 from cs.CsNetfilter import CsNetfilters
 from cs.CsDhcp import CsDhcp
 from cs.CsRedundant import *
 from cs.CsFile import CsFile
-from cs.CsApp import CsApache, CsDnsmasq
 from cs.CsMonitor import CsMonitor
 from cs.CsLoadBalancer import CsLoadBalancer
 from cs.CsConfig import CsConfig
 from cs.CsProcess import CsProcess
 from cs.CsStaticRoutes import CsStaticRoutes
 
+OCCURRENCES = 1
 
 class CsPassword(CsDataBag):
     
@@ -886,14 +882,49 @@ def processStaticNatRule(self, rule):
 
         self.fw.append(["nat", "front", "-A POSTROUTING -s %s -d %s -j SNAT -o eth0 --to-source %s" % (self.getNetworkByIp(rule['internal_ip']),rule["internal_ip"], self.getGuestIp())])
 
+class IpTablesExecutor:
+
+    config = None
+
+    def __init__(self, config):
+        self.config = config
+
+    def process(self):
+        acls = CsAcl('networkacl', self.config)
+        acls.process()
+
+        acls = CsAcl('firewallrules', self.config)
+        acls.process()
+
+        fwd = CsForwardingRules("forwardingrules", self.config)
+        fwd.process()
+
+        vpns = CsSite2SiteVpn("site2sitevpn", self.config)
+        vpns.process()
+
+        rvpn = CsRemoteAccessVpn("remoteaccessvpn", self.config)
+        rvpn.process()
+
+        lb = CsLoadBalancer("loadbalancer", self.config)
+        lb.process()
+
+        logging.debug("Configuring iptables rules")
+        nf = CsNetfilters(True)
+        nf.compare(self.config.get_fw())
+
+        logging.debug("Configuring iptables rules done ...saving rules")
+
+        # Save iptables configuration - will be loaded on reboot by the iptables-restore that is configured on /etc/rc.local
+        CsHelper.save_iptables("iptables-save", "/etc/iptables/router_rules.v4")
+        CsHelper.save_iptables("ip6tables-save", "/etc/iptables/router_rules.v6")
 
 def main(argv):
     # The file we are currently processing, if it is "cmd_line.json" everything will be processed.
     process_file = argv[1]
 
-    # process_file can be None, if so assume cmd_line.json
     if process_file is None:
-        process_file = "cmd_line.json"
+        logging.debug("No file was received, do not go on processing the other actions. Just leave for now.")
+        return
 
     # Track if changes need to be committed to NetFilter
     iptables_change = False
@@ -912,96 +943,54 @@ def main(argv):
     config.address().compare()
     config.address().process()
 
-    if process_file in ["cmd_line.json", "guest_network.json"]:
-        logging.debug("Configuring Guest Network")
-        iptables_change = True
-
-    if process_file in ["cmd_line.json", "vm_password.json"]:
-        logging.debug("Configuring vmpassword")
-        password = CsPassword("vmpassword", config)
-        password.process()
-
-    if process_file in ["cmd_line.json", "vm_metadata.json"]:
-        logging.debug("Configuring vmdata")
-        metadata = CsVmMetadata('vmdata', config)
-        metadata.process()
-
-    if process_file in ["cmd_line.json", "network_acl.json"]:
-        logging.debug("Configuring networkacl")
-        iptables_change = True
-
-    if process_file in ["cmd_line.json", "firewall_rules.json"]:
-        logging.debug("Configuring firewall rules")
-        iptables_change = True
-
-    if process_file in ["cmd_line.json", "forwarding_rules.json", "staticnat_rules.json"]:
-        logging.debug("Configuring PF rules")
-        iptables_change = True
-
-    if process_file in ["cmd_line.json", "site_2_site_vpn.json"]:
-        logging.debug("Configuring s2s vpn")
-        iptables_change = True
-
-    if process_file in ["cmd_line.json", "remote_access_vpn.json"]:
-        logging.debug("Configuring remote access vpn")
-        iptables_change = True
-
-    if process_file in ["cmd_line.json", "vpn_user_list.json"]:
-        logging.debug("Configuring vpn users list")
-        vpnuser = CsVpnUser("vpnuserlist", config)
-        vpnuser.process()
-
-    if process_file in ["cmd_line.json", "vm_dhcp_entry.json", "dhcp.json"]:
-        logging.debug("Configuring dhcp entry")
-        dhcp = CsDhcp("dhcpentry", config)
-        dhcp.process()
-
-    if process_file in ["cmd_line.json", "load_balancer.json"]:
-        logging.debug("Configuring load balancer")
-        iptables_change = True
-
-    if process_file in ["cmd_line.json", "monitor_service.json"]:
-        logging.debug("Configuring monitor service")
-        mon = CsMonitor("monitorservice", config)
-        mon.process()
-
-    # If iptable rules have changed, apply them.
-    if iptables_change:
-        acls = CsAcl('networkacl', config)
-        acls.process()
-
-        acls = CsAcl('firewallrules', config)
-        acls.process()
-
-        fwd = CsForwardingRules("forwardingrules", config)
-        fwd.process()
-
-        vpns = CsSite2SiteVpn("site2sitevpn", config)
-        vpns.process()
-
-        rvpn = CsRemoteAccessVpn("remoteaccessvpn", config)
-        rvpn.process()
-
-        lb = CsLoadBalancer("loadbalancer", config)
-        lb.process()
-
-        logging.debug("Configuring iptables rules")
-        nf = CsNetfilters()
-        nf.compare(config.get_fw())
-
-        logging.debug("Configuring iptables rules done ...saving rules")
-
-        # Save iptables configuration - will be loaded on reboot by the iptables-restore that is configured on /etc/rc.local
-        CsHelper.save_iptables("iptables-save", "/etc/iptables/router_rules.v4")
-        CsHelper.save_iptables("ip6tables-save", "/etc/iptables/router_rules.v6")
+    databag_map = OrderedDict([("guest_network.json", {"process_iptables" : True, "executor" : IpTablesExecutor(config)}),
+                               ("vm_password.json", {"process_iptables" : False, "executor" : CsPassword("vmpassword", config)}),
+                               ("vm_metadata.json", {"process_iptables" : False, "executor" : CsVmMetadata('vmdata', config)}),
+                               ("network_acl.json", {"process_iptables" : True, "executor" : IpTablesExecutor(config)}),
+                               ("firewall_rules.json", {"process_iptables" : True, "executor" : IpTablesExecutor(config)}),
+                               ("forwarding_rules.json", {"process_iptables" : True, "executor" : IpTablesExecutor(config)}),
+                               ("staticnat_rules.json", {"process_iptables" : True, "executor" : IpTablesExecutor(config)}),
+                               ("site_2_site_vpn.json", {"process_iptables" : True, "executor" : IpTablesExecutor(config)}),
+                               ("remote_access_vpn.json", {"process_iptables" : True, "executor" : IpTablesExecutor(config)}),
+                               ("vpn_user_list.json", {"process_iptables" : False, "executor" : CsVpnUser("vpnuserlist", config)}),
+                               ("vm_dhcp_entry.json", {"process_iptables" : False, "executor" : CsDhcp("dhcpentry", config)}),
+                               ("dhcp.json", {"process_iptables" : False, "executor" : CsDhcp("dhcpentry", config)}),
+                               ("load_balancer.json", {"process_iptables" : True, "executor" : IpTablesExecutor(config)}),
+                               ("monitor_service.json", {"process_iptables" : False, "executor" : CsMonitor("monitorservice", config)}),
+                               ("static_routes.json", {"process_iptables" : False, "executor" : CsStaticRoutes("staticroutes", config)})
+                               ])
+
+    if process_file.count("cmd_line.json") == OCCURRENCES:
+        logging.debug("cmd_line.json changed. All other files will be processed as well.")
+
+        while databag_map:
+            item = databag_map.popitem(last = False)
+            item_name = item[0]
+            item_dict = item[1]
+            if not item_dict["process_iptables"]:
+                executor = item_dict["executor"]
+                executor.process()
+
+        iptables_executor = IpTablesExecutor(config)
+        iptables_executor.process()
+    else:
+        while databag_map:
+            item = databag_map.popitem(last = False)
+            item_name = item[0]
+            item_dict = item[1]
+            if process_file.count(item_name) == OCCURRENCES:
+                executor = item_dict["executor"]
+                executor.process()
+
+                if item_dict["process_iptables"]:
+                    iptables_executor = IpTablesExecutor(config)
+                    iptables_executor.process()
+
+                break
 
     red = CsRedundant(config)
     red.set()
 
-    if process_file in ["cmd_line.json", "static_routes.json"]:
-        logging.debug("Configuring static routes")
-        static_routes = CsStaticRoutes("staticroutes", config)
-        static_routes.process()
 
 if __name__ == "__main__":
     main(sys.argv)
diff --git a/systemvm/patches/debian/config/opt/cloud/bin/merge.py b/systemvm/patches/debian/config/opt/cloud/bin/merge.py
index aa676827adb..b31c0c43242 100755
--- a/systemvm/patches/debian/config/opt/cloud/bin/merge.py
+++ b/systemvm/patches/debian/config/opt/cloud/bin/merge.py
@@ -18,8 +18,10 @@
 
 import json
 import os
-import time
+import uuid
 import logging
+import gzip
+import shutil
 import cs_ip
 import cs_guestnetwork
 import cs_cmdline
@@ -36,8 +38,6 @@
 import cs_vpnusers
 import cs_staticroutes
 
-from pprint import pprint
-
 
 class DataBag:
 
@@ -243,21 +243,25 @@ def load(self, data):
         if data is not None:
             self.data = data
             self.type = self.data["type"]
-            proc = updateDataBag(self)
+            updateDataBag(self)
             return
-        fn = self.configCache + '/' + self.fileName
+        filename = '{cache_location}/{json_file}'.format(cache_location = self.configCache, json_file = self.fileName)
         try:
-            handle = open(fn)
-        except IOError:
-            logging.error("Could not open %s", fn)
+            handle = open(filename)
+        except IOError as exception:
+            error_message = ("Exception occurred with the following exception error '{error}'. Could not open '{file}'. "
+                              "It seems that the file has already been moved.".format(error = exception, file = filename))
+            logging.error(error_message)
         else:
+            logging.info("Continuing with the processing of file '{file}'".format(file = filename))
+
             self.data = json.load(handle)
             self.type = self.data["type"]
             handle.close()
             if self.keep:
-                self.__moveFile(fn, self.configCache + "/processed")
+                self.__moveFile(filename, self.configCache + "/processed")
             else:
-                os.remove(fn)
+                os.remove(filename)
             proc = updateDataBag(self)
 
     def setFile(self, name):
@@ -275,9 +279,19 @@ def setPath(self, path):
     def __moveFile(self, origPath, path):
         if not os.path.exists(path):
             os.makedirs(path)
-        timestamp = str(int(round(time.time())))
-        os.rename(origPath, path + "/" + self.fileName + "." + timestamp)
 
+        originalName = os.path.basename(origPath)
+
+        if originalName.count(".") == 1:
+            originalName += "." + str(uuid.uuid4())
+
+        zipped_file_name = path + "/" + originalName + ".gz"
+
+        with open(origPath, 'rb') as f_in, gzip.open(zipped_file_name, 'wb') as f_out:
+            shutil.copyfileobj(f_in, f_out)
+        os.remove(origPath)
+
+        logging.debug("Processed file written to %s", zipped_file_name)
 
 class PrivateGatewayHack:
 
diff --git a/systemvm/patches/debian/config/opt/cloud/bin/update_config.py b/systemvm/patches/debian/config/opt/cloud/bin/update_config.py
index dddd0c8e3c0..970120cc21c 100755
--- a/systemvm/patches/debian/config/opt/cloud/bin/update_config.py
+++ b/systemvm/patches/debian/config/opt/cloud/bin/update_config.py
@@ -24,8 +24,11 @@
 import os
 import os.path
 import configure
+import glob
 import json
 
+OCCURRENCES = 1
+
 logging.basicConfig(filename='/var/log/cloud.log', level=logging.DEBUG, format='%(asctime)s  %(filename)s %(funcName)s:%(lineno)d %(message)s')
 
 # first commandline argument should be the file to process
@@ -70,7 +73,8 @@ def is_guestnet_configured(guestnet_dict, keys):
         print "[WARN] update_config.py :: Reconfiguring guest network..."
         return False
 
-    file = open(jsonCmdConfigPath)
+    fn = min(glob.iglob(jsonCmdConfigPath + '*'), key=os.path.getctime)
+    file = open(fn)
     new_guestnet_dict = json.load(file)
 
     if not new_guestnet_dict['add']:
@@ -109,19 +113,13 @@ def is_guestnet_configured(guestnet_dict, keys):
 
     return exists
 
-if not (os.path.isfile(jsonCmdConfigPath) and os.access(jsonCmdConfigPath, os.R_OK)):
+filename = min(glob.iglob(jsonCmdConfigPath + '*'), key=os.path.getctime)
+if not (os.path.isfile(filename) and os.access(filename, os.R_OK)):
     print "[ERROR] update_config.py :: You are telling me to process %s, but i can't access it" % jsonCmdConfigPath
     sys.exit(1)
 
-# If the command line json file is unprocessed process it
-# This is important or, the control interfaces will get deleted!
-if os.path.isfile(jsonPath % "cmd_line.json"):
-    qf = QueueFile()
-    qf.setFile("cmd_line.json")
-    qf.load(None)
-
 # If the guest network is already configured and have the same IP, do not try to configure it again otherwise it will break
-if sys.argv[1] == "guest_network.json":
+if sys.argv[1] and sys.argv[1].count("guest_network.json") == OCCURRENCES:
     if os.path.isfile(currentGuestNetConfig):
         file = open(currentGuestNetConfig)
         guestnet_dict = json.load(file)
diff --git a/systemvm/patches/debian/config/opt/cloud/bin/vr_cfg.sh b/systemvm/patches/debian/config/opt/cloud/bin/vr_cfg.sh
index d0eb1fccf79..07c617d4077 100755
--- a/systemvm/patches/debian/config/opt/cloud/bin/vr_cfg.sh
+++ b/systemvm/patches/debian/config/opt/cloud/bin/vr_cfg.sh
@@ -91,7 +91,7 @@ do
 done < $cfg
 
 #remove the configuration file, log file should have all the records as well
-rm -f $cfg
+mv $cfg /var/cache/cloud/processed/
 
 # Flush kernel conntrack table
 log_it "VR config: Flushing conntrack table"


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services