From f91163abed4aa739f7f8b772eabb403f01b94a88 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 +--------------------- src/lib/Bcfg2/Options.py | 24 ++++- src/lib/Bcfg2/Server/Plugin/base.py | 13 +-- src/lib/Bcfg2/Utils.py | 156 ++++++++++++++++++++++++++++++++- 6 files changed, 204 insertions(+), 143 deletions(-) (limited to 'src/lib/Bcfg2') diff --git a/src/lib/Bcfg2/Client/Client.py b/src/lib/Bcfg2/Client/Client.py index 6aa9ace1f..0488fcf21 100644 --- a/src/lib/Bcfg2/Client/Client.py +++ b/src/lib/Bcfg2/Client/Client.py @@ -14,10 +14,9 @@ import Bcfg2.Client.XML import Bcfg2.Client.Proxy 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 = [] diff --git a/src/lib/Bcfg2/Options.py b/src/lib/Bcfg2/Options.py index fe9a480a4..3f4e9a83c 100644 --- a/src/lib/Bcfg2/Options.py +++ b/src/lib/Bcfg2/Options.py @@ -334,6 +334,16 @@ def get_bool(val): raise ValueError("Not a boolean value", val) +def get_timeout(val): + """ convert the timeout value into a float or None """ + if val is None: + return val + timeout = float(val) # pass ValueError up the stack + if timeout <= 0: + return None + return timeout + + def get_size(value): """ Given a number of bytes in a human-readable format (e.g., '512m', '2g'), get the absolute number of bytes as an integer """ @@ -801,6 +811,16 @@ CLIENT_EXIT_ON_PROBE_FAILURE = \ long_arg=True, cf=('client', 'exit_on_probe_failure'), cook=get_bool) +CLIENT_PROBE_TIMEOUT = \ + Option("Timeout when running client probes", + default=None, + cf=('client', 'probe_timeout'), + cook=get_timeout) +CLIENT_COMMAND_TIMEOUT = \ + Option("Timeout when client runs other external commands (not probes)", + default=None, + cf=('client', 'command_timeout'), + cook=get_timeout) # bcfg2-test and bcfg2-lint options TEST_NOSEOPTS = \ @@ -1131,7 +1151,9 @@ CLIENT_COMMON_OPTIONS = \ serverCN=CLIENT_SCNS, timeout=CLIENT_TIMEOUT, decision_list=CLIENT_DECISION_LIST, - probe_exit=CLIENT_EXIT_ON_PROBE_FAILURE) + probe_exit=CLIENT_EXIT_ON_PROBE_FAILURE, + probe_timeout=CLIENT_PROBE_TIMEOUT, + command_timeout=CLIENT_COMMAND_TIMEOUT) CLIENT_COMMON_OPTIONS.update(DRIVER_OPTIONS) CLIENT_COMMON_OPTIONS.update(CLI_COMMON_OPTIONS) diff --git a/src/lib/Bcfg2/Server/Plugin/base.py b/src/lib/Bcfg2/Server/Plugin/base.py index e74909ee9..25a687874 100644 --- a/src/lib/Bcfg2/Server/Plugin/base.py +++ b/src/lib/Bcfg2/Server/Plugin/base.py @@ -2,6 +2,7 @@ import os import logging +from Bcfg2.Utils import ClassName class Debuggable(object): @@ -59,18 +60,6 @@ class Debuggable(object): self.logger.error(message) -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 plugin 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 Plugin(Debuggable): """ The base class for all Bcfg2 Server plugins. """ diff --git a/src/lib/Bcfg2/Utils.py b/src/lib/Bcfg2/Utils.py index 247e4f16b..3b1559528 100644 --- a/src/lib/Bcfg2/Utils.py +++ b/src/lib/Bcfg2/Utils.py @@ -3,9 +3,25 @@ used by both client and server. Stuff that doesn't fit anywhere else. """ import fcntl +import logging +import threading +import subprocess from Bcfg2.Compat import any # pylint: disable=W0622 +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 server plugin and every client 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 and tool isn't repeating its own + name.""" + + def __get__(self, inst, owner): + return owner.__name__ + + class PackedDigitRange(object): """ Representation of a set of integer ranges. A range is described by a comma-delimited string of integers and ranges, @@ -69,9 +85,147 @@ class PackedDigitRange(object): def locked(fd): - """ Acquire a lock on a file """ + """ Acquire a lock on a file. + + :param fd: The file descriptor to lock + :type fd: int + :returns: bool - True if the file is already locked, False + otherwise """ try: fcntl.lockf(fd, fcntl.LOCK_EX | fcntl.LOCK_NB) except IOError: return True return False + + +class ExecutorResult(object): + """ Returned as the result of a call to + :func:`Bcfg2.Utils.Executor.run`. The result can be accessed via + the instance variables, documented below, as a boolean (which is + equivalent to :attr:`Bcfg2.Utils.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 convenient way to run external commands with + :class:`subprocess.Popen` """ + + def __init__(self, timeout=None): + """ + :param timeout: Set a default timeout for all commands run by + this Executor object + :type timeout: float + """ + self.logger = logging.getLogger(self.__class__.__name__) + self.timeout = timeout + + def _timeout_callback(self, proc): + """ Get a callback (suitable for passing to + :class:`threading.Timer`) that kills the given process. + + :param proc: The process to kill upon timeout. + :type proc: subprocess.Popen + :returns: function """ + def _timeout(): + """ Callback that kills ``proc`` """ + if proc.poll() == None: + try: + proc.kill() + self.logger.warning("Process exceeeded timeout, killing") + except OSError: + pass + + return _timeout + + def run(self, command, inputdata=None, shell=False, timeout=None): + """ Run a command, given as a list, optionally giving it the + specified input data. + + :param command: The command to run, as a list (preferred) or + as a string. See :class:`subprocess.Popen` for + details. + :type command: list or string + :param inputdata: Data to pass to the command on stdin + :type inputdata: string + :param shell: Run the given command in a shell (not recommended) + :type shell: bool + :param timeout: Kill the command if it runs longer than this + many seconds. Set to 0 or -1 to explicitly + override a default timeout. + :type timeout: float + :returns: :class:`Bcfg2.Utils.ExecutorResult` + """ + 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 timeout is None: + timeout = self.timeout + if timeout is not None: + timer = threading.Timer(float(timeout), + self._timeout_callback(proc), + [proc]) + timer.start() + try: + 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()) + finally: + if timeout is not None: + timer.cancel() -- cgit v1.2.3-1-g7c22