From 1893c50204bd5ce3fcd85ffcfbff77475996962f Mon Sep 17 00:00:00 2001 From: Martin Magr Date: Tue, 12 Feb 2013 19:29:28 +0100 Subject: [PATCH] Fixed temp dir conflict Fixed invalid validator Change-Id: I72d6e00d895a0fb360d3d05da9b44e0e63322869 --- packstack/installer/basedefs.py | 8 +++--- packstack/installer/common_utils.py | 2 +- packstack/installer/engine_validators.py | 20 ++++++++++++++ packstack/installer/run_setup.py | 10 +++---- packstack/installer/setup_controller.py | 11 +++++--- packstack/modules/ospluginutils.py | 11 ++++---- packstack/plugins/puppet_950.py | 34 +++++++++++++++++------- packstack/plugins/serverprep_901.py | 34 ++++++++---------------- 8 files changed, 79 insertions(+), 51 deletions(-) diff --git a/packstack/installer/basedefs.py b/packstack/installer/basedefs.py index 45454e1c7..e4f33bbb2 100644 --- a/packstack/installer/basedefs.py +++ b/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" diff --git a/packstack/installer/common_utils.py b/packstack/installer/common_utils.py index 926819a65..c71a83cca 100644 --- a/packstack/installer/common_utils.py +++ b/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: diff --git a/packstack/installer/engine_validators.py b/packstack/installer/engine_validators.py index 35053e855..f048e0c7a 100644 --- a/packstack/installer/engine_validators.py +++ b/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) diff --git a/packstack/installer/run_setup.py b/packstack/installer/run_setup.py index 47581d2dc..11158ab34 100644 --- a/packstack/installer/run_setup.py +++ b/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 - # 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)) + server.append('rm -rf %s' % host_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)) diff --git a/packstack/installer/setup_controller.py b/packstack/installer/setup_controller.py index 49791dbd7..53d453fd9 100644 --- a/packstack/installer/setup_controller.py +++ b/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): diff --git a/packstack/modules/ospluginutils.py b/packstack/modules/ospluginutils.py index 47c95b2cb..446798906 100644 --- a/packstack/modules/ospluginutils.py +++ b/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(): - fd = os.open(file, os.O_WRONLY | os.O_CREAT | os.O_EXCL, 0600) + for fname, data in self.data.items(): + 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(manifestfile, marker, data) + manifestfiles.addFile(manifest_name, marker, data) def gethostlist(CONF): diff --git a/packstack/plugins/puppet_950.py b/packstack/plugins/puppet_950.py index 5071d66c0..ae3fe0614 100644 --- a/packstack/plugins/puppet_950.py +++ b/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 - finished_logfile = "%s.finished" % manifest + man_path = os.path.join(controller.temp_map[hostname], + 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("touch %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) + server.append("export FACTERLIB=$FACTERLIB:%s/facts" % host_dir) + server.append("touch %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 &" % (host_dir, host_dir, man_path, running_logfile, running_logfile, finished_logfile) server.append(command) server.execute() diff --git a/packstack/plugins/serverprep_901.py b/packstack/plugins/serverprep_901.py index 3a001852f..7b550e91e 100644 --- a/packstack/plugins/serverprep_901.py +++ b/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) - # Separately create the tmp directory for this packstack run, this will fail if - # the directory already exists - server.append("mkdir --mode 0700 %s" % basedefs.VAR_DIR) + if hostname not in controller.temp_map: + # TO-DO: Move this to packstack.installer.setup_controller + server.append("mkdir -p %s" % basedefs.PACKSTACK_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")