Fixed temp dir conflict

Fixed invalid validator

Change-Id: I72d6e00d895a0fb360d3d05da9b44e0e63322869

packstack/installer/basedefs.py

@@ -10,17 +10,17 @@ APP_NAME = "Installer"
 
 FILE_YUM_VERSION_LOCK = "/etc/yum/pluginconf.d/versionlock.list"
 
-_tmpdirprefix = datetime.datetime.now().strftime('%Y%m%d-%H%M%S-')
-
 PACKSTACK_VAR_DIR = "/var/tmp/packstack"
 try:
     os.mkdir(PACKSTACK_VAR_DIR, 0700)
 except:
     pass
 
+_tmpdirprefix = datetime.datetime.now().strftime('%Y%m%d-%H%M%S-')
 VAR_DIR = tempfile.mkdtemp(prefix = _tmpdirprefix, dir = PACKSTACK_VAR_DIR)
 DIR_LOG = VAR_DIR
-PUPPET_MANIFEST_DIR = os.path.join(VAR_DIR, "manifests")
+PUPPET_MANIFEST_RELATIVE = "manifests"
+PUPPET_MANIFEST_DIR = os.path.join(VAR_DIR, PUPPET_MANIFEST_RELATIVE)
 
 FILE_INSTALLER_LOG = "setup.log"
 
@@ -28,6 +28,8 @@ DIR_PROJECT_DIR = os.environ.get('INSTALLER_PROJECT_DIR', os.path.join(os.path.s
 DIR_PLUGINS = os.path.join(DIR_PROJECT_DIR, "plugins")
 DIR_MODULES = os.path.join(DIR_PROJECT_DIR, "modules")
 
+
+
 EXEC_RPM = "rpm"
 EXEC_SEMANAGE = "semanage"
 EXEC_NSLOOKUP = "nslookup"

packstack/installer/common_utils.py

@@ -332,7 +332,7 @@ def getLocalhostIP():
 
 def host2ip(hostname, allow_localhost=False):
     """
-    Converts given hostname to IP address. Raises HostnameConvertError
+    Converts given hostname to IP address. Raises NetworkError
     if conversion failed.
     """
     try:

packstack/installer/engine_validators.py

@@ -88,6 +88,7 @@ def validate_options(param, options=None):
     """
     options = options or []
 
+    # TO-DO: to be more flexible, remove this and exit in case param is empty
     validate_not_empty(param, options)
     if param not in options:
         logging.debug('validate_options(%s, options=%s) failed.' %
@@ -96,6 +97,21 @@ def validate_options(param, options=None):
         raise ParamValidationError(msg % (options, param))
 
 
+def validate_multi_options(param, options=None):
+    """
+    Validates if comma separated values given in params are members
+    of options.
+    """
+    if not param:
+        return
+    options = options or []
+    for i in param.split(','):
+        if i.strip() not in options:
+            logging.debug('validate_multi_options(%s, options=%s) '
+                          'failed.' % (param, options))
+            msg = 'Given value is not member of allowed values %s: %s'
+            raise ParamValidationError(msg % (options, param))
+
 def validate_ip(param, options=None):
     """
     Raises ParamValidationError if given parameter value is not in IPv4
@@ -129,6 +145,7 @@ def validate_file(param, options=None):
     Raises ParamValidationError if provided file in param does not exist.
     """
     options = options or []
+    # TO-DO: to be more flexible, remove this and exit in case param is empty
     validate_not_empty(param)
 
     if not os.path.isfile(param):
@@ -144,6 +161,7 @@ def validate_ping(param, options=None):
     echo request.
     """
     options = options or []
+    # TO-DO: to be more flexible, remove this and exit in case param is empty
     validate_not_empty(param)
 
     cmd = ["/bin/ping", "-c", "1", str(param)]
@@ -161,6 +179,7 @@ def validate_multi_ping(param, options=None):
     do not answer to ICMP echo request.
     """
     options = options or []
+    # TO-DO: to be more flexible, remove this and exit in case param is empty
     validate_not_empty(param)
     for host in param.split(","):
         validate_ping(host.strip())
@@ -202,6 +221,7 @@ def validate_multi_ssh(param, options=None):
     in param do not listen on port 22.
     """
     options = options or []
+    # TO-DO: to be more flexible, remove this and exit in case param is empty
     validate_not_empty(param)
     for host in param.split(","):
         validate_ssh(host)

packstack/installer/run_setup.py

@@ -612,16 +612,14 @@ def remove_remote_var_dirs():
     doesn't remove data on localhost
     """
     for host in gethostlist(controller.CONF):
-        logging.info(output_messages.INFO_REMOVE_REMOTE_VAR%(basedefs.VAR_DIR, host))
+        host_dir = controller.temp_map[host]
+        logging.info(output_messages.INFO_REMOVE_REMOTE_VAR % (host_dir, host))
         server = utils.ScriptRunner(host)
-        # We don't want to remove the tmp dir on the host that was used to
+        server.append('rm -rf %s' % host_dir)
-        # run packstack, this host has the logfile, if it doesn't have a
-        # logfile its a remote host, so we remove the temp files
-        server.append('ls -l %s || rm -rf %s'%(logFile, basedefs.VAR_DIR))
         try:
             server.execute()
         except Exception, e:
-            msg = output_messages.ERR_REMOVE_REMOTE_VAR%(basedefs.VAR_DIR, host)
+            msg = output_messages.ERR_REMOVE_REMOTE_VAR % (host_dir, host)
             logging.error(msg)
             logging.exception(e)
             controller.MESSAGES.append(utils.getColoredText(msg, basedefs.RED))

packstack/installer/setup_controller.py

@@ -13,7 +13,7 @@ class Controller(object):
     MESSAGES=[]
     CONF={}
 
-    __single = None # the one, true Singleton
+    __single = None # the one, true Singleton ... for god's sake why ??? :)
 
     def __new__(self, *args, **kwargs):
         """
@@ -22,12 +22,17 @@ class Controller(object):
         which means that we will not invoke this singleton if someone tries to create a new
         instance from a class which inherit Controller.
         did not use isinstance because inheritence makes it behave erratically.
-        """ 
+        """
         if self != type(self.__single):
             self.__single = object.__new__(self, *args, **kwargs)
         return self.__single
 
-    def __init__(self): pass
+    def __init__(self):
+        # XXX: Right now this will only hold all temp dirs on each host.
+        #      Method for temp dir creation should be implemented in this
+        #      class, when it will start behaving like controller and not
+        #      only like data container
+        self.temp_map = {}
 
     # PLugins
     def addPlugin(self, plugObj):

packstack/modules/ospluginutils.py

@@ -47,7 +47,8 @@ class ManifestFiles(object):
         for f, p in self.filelist:
             if f == filename:
                 return
-        self.filelist.append((filename, marker,))
+
+        self.filelist.append((filename, marker))
 
     def getFiles(self):
         return [f for f in self.filelist]
@@ -58,8 +59,9 @@ class ManifestFiles(object):
         write before the puppet manifests are copied to the various servers
         """
         os.mkdir(basedefs.PUPPET_MANIFEST_DIR, 0700)
-        for file, data in self.data.items():
+        for fname, data in self.data.items():
-            fd = os.open(file, os.O_WRONLY | os.O_CREAT | os.O_EXCL, 0600)
+            path = os.path.join(basedefs.PUPPET_MANIFEST_DIR, fname)
+            fd = os.open(path, os.O_WRONLY | os.O_CREAT | os.O_EXCL, 0600)
             with os.fdopen(fd, 'w') as fp:
                 fp.write(data)
 manifestfiles = ManifestFiles()
@@ -71,8 +73,7 @@ def getManifestTemplate(template_name):
 
 
 def appendManifestFile(manifest_name, data, marker=''):
-    manifestfile = os.path.join(basedefs.PUPPET_MANIFEST_DIR, manifest_name)
+    manifestfiles.addFile(manifest_name, marker, data)
-    manifestfiles.addFile(manifestfile, marker, data)
 
 
 def gethostlist(CONF):

packstack/plugins/puppet_950.py

@@ -81,10 +81,18 @@ def copyPuppetModules():
     if platform.linux_distribution()[0] == "Fedora":
         tar_opts += "--exclude create_resources "
     for hostname in gethostlist(controller.CONF):
+        host_dir = controller.temp_map[hostname]
+        puppet_dir = os.path.join(host_dir, basedefs.PUPPET_MANIFEST_RELATIVE)
         server.append("cd %s/puppet" % basedefs.DIR_PROJECT_DIR)
-        server.append("tar %s --dereference -cpzf - modules facts | ssh -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null root@%s tar -C %s -xpzf -" % (tar_opts, hostname, basedefs.VAR_DIR))
+        server.append("tar %s --dereference -cpzf - modules facts | "
+                      "ssh -o StrictHostKeyChecking=no "
+                          "-o UserKnownHostsFile=/dev/null "
+                          "root@%s tar -C %s -xpzf -" % (tar_opts, hostname, host_dir))
         server.append("cd %s" % basedefs.PUPPET_MANIFEST_DIR)
-        server.append("tar %s --dereference -cpzf - ../manifests | ssh -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null root@%s tar -C %s -xpzf -" % (tar_opts, hostname, basedefs.VAR_DIR))
+        server.append("tar %s --dereference -cpzf - ../manifests | "
+                      "ssh -o StrictHostKeyChecking=no "
+                          "-o UserKnownHostsFile=/dev/null "
+                          "root@%s tar -C %s -xpzf -" % (tar_opts, hostname, host_dir))
     server.execute()
 
 
@@ -96,7 +104,8 @@ def waitforpuppet(currently_running):
                 # Once a remote puppet run has finished, we retrieve the log
                 # file and check it for errors
                 local_server = utils.ScriptRunner()
-                log = finished_logfile.replace(".finished", ".log")
+                log = os.path.join(basedefs.PUPPET_MANIFEST_DIR,
+                                   os.path.basename(finished_logfile).replace(".finished", ".log"))
                 local_server.append('scp -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null root@%s:%s %s' % (hostname, finished_logfile, log))
                 # Errors are expected here if the puppet run isn't finished so we suppress logging them
                 local_server.execute(logerrors=False)
@@ -128,20 +137,25 @@ def applyPuppetManifest():
         lastmarker = marker
 
         for hostname in gethostlist(controller.CONF):
-            if "/%s_" % hostname not in manifest:
+            if "%s_" % hostname not in manifest:
                 continue
 
+            host_dir = controller.temp_map[hostname]
             print "Applying " + manifest
             server = utils.ScriptRunner(hostname)
 
-            running_logfile = "%s.running" % manifest
+            man_path = os.path.join(controller.temp_map[hostname],
-            finished_logfile = "%s.finished" % manifest
+                                    basedefs.PUPPET_MANIFEST_RELATIVE,
+                                    manifest)
+
+            running_logfile = "%s.running" % man_path
+            finished_logfile = "%s.finished" % man_path
             currently_running.append((hostname, finished_logfile))
             if not manifest.endswith('_horizon.pp'):
-                server.append("export FACTERLIB=$FACTERLIB:%s/facts" % basedefs.VAR_DIR)
+                server.append("export FACTERLIB=$FACTERLIB:%s/facts" % host_dir)
-            server.append("touch %s"%running_logfile)
+            server.append("touch %s" % running_logfile)
-            server.append("chmod 600 %s"%running_logfile)
+            server.append("chmod 600 %s" % running_logfile)
-            command = "( flock %s/ps.lock puppet apply --modulepath %s/modules %s > %s 2>&1 < /dev/null ; mv %s %s ) > /dev/null 2>&1 < /dev/null &" % (basedefs.VAR_DIR, basedefs.VAR_DIR, manifest, running_logfile, running_logfile, finished_logfile)
+            command = "( flock %s/ps.lock puppet apply --modulepath %s/modules %s > %s 2>&1 < /dev/null ; mv %s %s ) > /dev/null 2>&1 < /dev/null &" % (host_dir, host_dir, man_path, running_logfile, running_logfile, finished_logfile)
             server.append(command)
             server.execute()
 

packstack/plugins/serverprep_901.py

@@ -3,8 +3,9 @@ prepare server
 """
 
 import os
+import uuid
 import logging
-import os
+import datetime
 
 from packstack.installer import basedefs
 from packstack.installer import common_utils as utils
@@ -200,7 +201,7 @@ def initConfig(controllerObject):
                                         "flags are: novirtinfo, norhnsd, nopackages"),
                    "PROMPT"          : "Enter comma separated list of flags passed to rhnreg_ks",
                    "OPTION_LIST"     : ['novirtinfo', 'norhnsd', 'nopackages'],
-                   "VALIDATORS"      : [validate_multi_options],
+                   "VALIDATORS"      : [validate.validate_multi_options],
                    "DEFAULT_VALUE"   : "",
                    "MASK_INPUT"      : True,
                    "LOOSE_VALIDATION": False,
@@ -225,23 +226,6 @@ def initConfig(controllerObject):
         controller.addGroup(group, paramList)
 
 
-def validate_multi_options(param, options=None):
-    """
-    Validates if comma separated values given in params are members
-    of options.
-    """
-    # TO-DO: Move this to packstack.installer.setup_validators and modify
-    #        it to raise exception as soon as I1bb3486e will be accepted
-    options = options or []
-    for i in param.split(','):
-        if i.strip() not in options:
-            msg = 'Given value is not member of allowed values %s: %s'
-            # TO-DO: raise ParamValidationError(msg % (options, param))
-            print msg % (options, param)
-            return False
-    return True
-
-
 def run_rhn_reg(host, server_url, username=None, password=None,
                 cacert=None, activation_key=None, profile_name=None,
                 proxy_host=None, proxy_user=None, proxy_pass=None,
@@ -378,10 +362,14 @@ def serverprep():
             server.append("rm -rf $REPOFILE")
 
         # Create the packstack tmp directory
-        server.append("mkdir -p %s" % basedefs.PACKSTACK_VAR_DIR)
+        if hostname not in controller.temp_map:
-        # Separately create the tmp directory for this packstack run, this will fail if
+            # TO-DO: Move this to packstack.installer.setup_controller
-        # the directory already exists
+            server.append("mkdir -p %s" % basedefs.PACKSTACK_VAR_DIR)
-        server.append("mkdir --mode 0700 %s" % basedefs.VAR_DIR)
+            # Separately create the tmp directory for this packstack run, this will fail if
+            # the directory already exists
+            host_dir = os.path.join(basedefs.PACKSTACK_VAR_DIR, uuid.uuid4().hex)
+            server.append("mkdir --mode 0700 %s" % host_dir)
+            controller.temp_map[hostname] = host_dir
 
         # set highest priority of RHOS repository if EPEL is installed
         server.append("rpm -q epel-release && yum install -y yum-plugin-priorities || true")