From b7995e7b2cb8e527ab8bb8bd3f01c8fb2fce736a Mon Sep 17 00:00:00 2001 From: "Chris St. Pierre" Date: Wed, 13 Feb 2013 16:08:08 -0500 Subject: extended usage of Executor class, added client-side timeout options --- src/lib/Bcfg2/Client/Client.py | 18 +++--- src/lib/Bcfg2/Client/Tools/SELinux.py | 32 +++++----- src/lib/Bcfg2/Client/Tools/__init__.py | 104 +-------------------------------- 3 files changed, 25 insertions(+), 129 deletions(-) (limited to 'src/lib/Bcfg2/Client') diff --git a/src/lib/Bcfg2/Client/Client.py b/src/lib/Bcfg2/Client/Client.py index b0d877a19..88f3bd6ef 100644 --- a/src/lib/Bcfg2/Client/Client.py +++ b/src/lib/Bcfg2/Client/Client.py @@ -14,10 +14,9 @@ import Bcfg2.Options import Bcfg2.Client.XML import Bcfg2.Client.Frame import Bcfg2.Client.Tools -from Bcfg2.Utils import locked +from Bcfg2.Utils import locked, Executor from Bcfg2.Compat import xmlrpclib from Bcfg2.version import __version__ -from subprocess import Popen, PIPE class Client(object): @@ -42,6 +41,9 @@ class Client(object): to_file=self.setup['logging']) self.logger = logging.getLogger('bcfg2') self.logger.debug(self.setup) + + self.cmd = Executor(self.setup['command_timeout']) + if self.setup['bundle_quick']: if not self.setup['bundle'] and not self.setup['skipbundle']: self.logger.error("-Q option requires -b or -B") @@ -95,16 +97,14 @@ class Client(object): stat.S_IRUSR | stat.S_IRGRP | stat.S_IROTH | stat.S_IXUSR | stat.S_IXGRP | stat.S_IXOTH | stat.S_IWUSR) # 0755 - proc = Popen(scriptname, stdin=PIPE, stdout=PIPE, stderr=PIPE) - ret.text, err = proc.communicate() - rv = proc.wait() - if err: + rv = self.cmd.run(scriptname, timeout=self.setup['timeout']) + if rv.stderr: self.logger.warning("Probe %s has error output: %s" % - (name, err)) - if rv: + (name, rv.stderr)) + if not rv.success: self._probe_failure(name, "Return value %s" % rv) self.logger.info("Probe %s has result:" % name) - self.logger.info(ret.text) + self.logger.info(rv.stdout) finally: os.unlink(scriptname) except SystemExit: diff --git a/src/lib/Bcfg2/Client/Tools/SELinux.py b/src/lib/Bcfg2/Client/Tools/SELinux.py index 414ca1f93..08d943251 100644 --- a/src/lib/Bcfg2/Client/Tools/SELinux.py +++ b/src/lib/Bcfg2/Client/Tools/SELinux.py @@ -12,7 +12,6 @@ import seobject import Bcfg2.Client.XML import Bcfg2.Client.Tools from Bcfg2.Client.Tools.POSIX.File import POSIXFile -from subprocess import Popen, PIPE def pack128(int_val): @@ -734,9 +733,7 @@ class SELinuxSemoduleHandler(SELinuxEntryHandler): self._all = dict() self.logger.debug("SELinux: Getting modules from semodule") try: - proc = Popen(['semodule', '-l'], stdout=PIPE, stderr=PIPE) - out = proc.communicate()[0] - rv = proc.wait() + rv = self.tool.cmd.run(['semodule', '-l']) except OSError: # semanage failed; probably not in $PATH. try to # get the list of modules from the filesystem @@ -745,13 +742,9 @@ class SELinuxSemoduleHandler(SELinuxEntryHandler): err) self._all.update(self._all_records_from_filesystem()) else: - if rv: - self.logger.error("SELinux: Failed to run semodule: %s" - % err) - self._all.update(self._all_records_from_filesystem()) - else: + if rv.success: # ran semodule successfully - for line in out.splitlines(): + for line in rv.stdout.splitlines(): mod, version = line.split() self._all[mod] = (version, 1) @@ -759,6 +752,10 @@ class SELinuxSemoduleHandler(SELinuxEntryHandler): for mod in self._all_records_from_filesystem().keys(): if mod not in self._all: self._all[mod] = ('', 0) + else: + self.logger.error("SELinux: Failed to run semodule: %s" + % rv.error) + self._all.update(self._all_records_from_filesystem()) return self._all def _all_records_from_filesystem(self): @@ -870,26 +867,23 @@ class SELinuxSemoduleHandler(SELinuxEntryHandler): self.logger.debug("Install SELinux module %s with semodule -i %s" % (entry.get('name'), self._filepath(entry))) try: - proc = Popen(['semodule', '-i', self._filepath(entry)], - stdout=PIPE, stderr=PIPE) - err = proc.communicate()[1] - rv = proc.wait() + rv = self.tool.cmd.run(['semodule', '-i', self._filepath(entry)]) except OSError: err = sys.exc_info()[1] self.logger.error("Failed to install SELinux module %s with " "semodule: %s" % (entry.get("name"), err)) return False - if rv: - self.logger.error("Failed to install SELinux module %s with " - "semodule: %s" % (entry.get("name"), err)) - return False - else: + if rv.success: if entry.get("disabled", "false").lower() == "true": self.logger.warning("SELinux: Cannot disable modules with " "semodule") return False else: return True + else: + self.logger.error("Failed to install SELinux module %s with " + "semodule: %s" % (entry.get("name"), rv.error)) + return False def _addargs(self, entry): """ argument list for adding entries """ diff --git a/src/lib/Bcfg2/Client/Tools/__init__.py b/src/lib/Bcfg2/Client/Tools/__init__.py index 7014f334f..cd86a2a4b 100644 --- a/src/lib/Bcfg2/Client/Tools/__init__.py +++ b/src/lib/Bcfg2/Client/Tools/__init__.py @@ -4,9 +4,8 @@ import os import sys import stat import select -import logging -import subprocess import Bcfg2.Client.XML +from Bcfg2.Utils import Executor, ClassName from Bcfg2.Compat import input, walk_packages # pylint: disable=W0622 __all__ = [m[1] for m in walk_packages(path=__path__)] @@ -26,103 +25,6 @@ class ToolInstantiationError(Exception): pass -class ExecutorResult(object): - """ Returned as the result of a call to - :func:`Bcfg2.Client.Tools.Executor.run`. The result can be - accessed via the instance variables, documented below, as a - boolean (which is equivalent to - :attr:`Bcfg2.Client.Tools.ExecutorResult.success`), or as a tuple, - which, for backwards compatibility, is equivalent to - ``(result.retval, result.stdout.splitlines())``. """ - - def __init__(self, stdout, stderr, retval): - #: The output of the command - self.stdout = stdout - - #: The error produced by the command - self.stderr = stderr - - #: The return value of the command. - self.retval = retval - - #: Whether or not the command was successful. If the - #: ExecutorResult is used as a boolean, ``success`` is - #: returned. - self.success = retval == 0 - - #: A friendly error message - self.error = None - if self.retval: - if self.stderr: - self.error = "%s (rv: %s)" % (self.stderr, self.retval) - elif self.stdout: - self.error = "%s (rv: %s)" % (self.stdout, self.retval) - else: - self.error = "No output or error; return value %s" % \ - self.retval - - def __repr__(self): - if self.error: - return "Errored command result: %s" % self.error - elif self.stdout: - return "Successful command result: %s" % self.stdout - else: - return "Successful command result: No output" - - def __getitem__(self, idx): - """ This provides compatibility with the old Executor, which - returned a tuple of (return value, stdout split by lines). """ - return (self.retval, self.stdout.splitlines())[idx] - - def __nonzero__(self): - return self.__bool__() - - def __bool__(self): - return self.success - - -class Executor(object): - """ A better version of Bcfg2.Client.Tool.Executor, which captures - stderr, raises exceptions on error, and doesn't use the shell to - execute by default """ - - def __init__(self): - self.logger = logging.getLogger(self.__class__.__name__) - - def run(self, command, inputdata=None, shell=False): - """ Run a command, given as a list, optionally giving it the - specified input data """ - if isinstance(command, str): - cmdstr = command - else: - cmdstr = " ".join(command) - self.logger.debug("Running: %s" % cmdstr) - proc = subprocess.Popen(command, shell=shell, bufsize=16384, - stdin=subprocess.PIPE, stdout=subprocess.PIPE, - stderr=subprocess.PIPE, close_fds=True) - if inputdata: - for line in inputdata.splitlines(): - self.logger.debug('> %s' % line) - (stdout, stderr) = proc.communicate(input=inputdata) - for line in stdout.splitlines(): # pylint: disable=E1103 - self.logger.debug('< %s' % line) - for line in stderr.splitlines(): # pylint: disable=E1103 - self.logger.info(line) - return ExecutorResult(stdout, stderr, proc.wait()) - - -class ClassName(object): - """ This very simple descriptor class exists only to get the name - of the owner class. This is used because, for historical reasons, - we expect every tool to have a ``name`` attribute that is in - almost all cases the same as the ``__class__.__name__`` attribute - of the plugin object. This makes that more dynamic so that each - plugin isn't repeating its own name.""" - - def __get__(self, inst, owner): - return owner.__name__ - - class Tool(object): """ The base tool class. All tools subclass this. @@ -197,9 +99,9 @@ class Tool(object): #: The XML configuration for this client self.config = config - #: An :class:`Bcfg2.Client.Tools.Executor` object for + #: An :class:`Bcfg2.Utils.Executor` object for #: running external commands. - self.cmd = Executor() + self.cmd = Executor(timeout=self.setup['command_timeout']) #: A list of entries that have been modified by this tool self.modified = [] -- cgit v1.2.3-1-g7c22