From 43984bc5ebc59bd8c5890ed6ba3de162e6698dcc Mon Sep 17 00:00:00 2001 From: "Chris St. Pierre" Date: Tue, 12 Feb 2013 16:02:24 -0500 Subject: better Executor class for client tools --- src/lib/Bcfg2/Client/Tools/__init__.py | 138 +++++++++++++++++++++++---------- 1 file changed, 95 insertions(+), 43 deletions(-) (limited to 'src/lib/Bcfg2/Client/Tools/__init__.py') diff --git a/src/lib/Bcfg2/Client/Tools/__init__.py b/src/lib/Bcfg2/Client/Tools/__init__.py index c0dd60c1e..7014f334f 100644 --- a/src/lib/Bcfg2/Client/Tools/__init__.py +++ b/src/lib/Bcfg2/Client/Tools/__init__.py @@ -4,7 +4,8 @@ import os import sys import stat import select -from subprocess import Popen, PIPE +import logging +import subprocess import Bcfg2.Client.XML from Bcfg2.Compat import input, walk_packages # pylint: disable=W0622 @@ -25,34 +26,89 @@ class ToolInstantiationError(Exception): pass -class Executor: - """ This class runs shell commands. """ +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 __init__(self, logger): - """ - :param logger: The logger to use to produce debug logging - :type logger: logging.Logger - """ - self.logger = logger + 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 run(self, command): - """ Run a command inside a shell. + def __nonzero__(self): + return self.__bool__() - :param command: The command to run, given as a list as to - :class:`subprocess.Popen`. Since the command - will be run within a shell it is particularly - important to pass it as a list. - :type command: list - :returns: tuple of return value (integer) and output (list of - lines) - """ - self.logger.debug("Running: %s" % command) - proc = Popen(command, shell=True, bufsize=16384, - stdin=PIPE, stdout=PIPE, close_fds=True) - output = proc.communicate()[0].splitlines() - for line in output: + 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) - return (proc.wait(), output) + for line in stderr.splitlines(): # pylint: disable=E1103 + self.logger.info(line) + return ExecutorResult(stdout, stderr, proc.wait()) class ClassName(object): @@ -143,7 +199,7 @@ class Tool(object): #: An :class:`Bcfg2.Client.Tools.Executor` object for #: running external commands. - self.cmd = Executor(logger) + self.cmd = Executor() #: A list of entries that have been modified by this tool self.modified = [] @@ -469,10 +525,7 @@ class PkgTool(Tool): pkgargs = " ".join([self.pkgtool[1][0] % datum for datum in data]) self.logger.debug("Installing packages: %s" % pkgargs) - self.logger.debug("Running command: %s" % (self.pkgtool[0] % pkgargs)) - - cmdrc = self.cmd.run(self.pkgtool[0] % pkgargs)[0] - if cmdrc == 0: + if self.cmd.run(self.pkgtool[0] % pkgargs): self.logger.info("Single Pass Succeded") # set all package states to true and flush workqueues pkgnames = [pkg.get('name') for pkg in packages] @@ -497,12 +550,11 @@ class PkgTool(Tool): else: self.logger.info("Installing pkg %s version %s" % (pkg.get('name'), pkg.get('version'))) - cmdrc = self.cmd.run( + if self.cmd.run( self.pkgtool[0] % (self.pkgtool[1][0] % tuple([pkg.get(field) - for field in self.pkgtool[1][1]]))) - if cmdrc[0] == 0: + for field in self.pkgtool[1][1]]))): states[pkg] = True else: self.logger.error("Failed to install package %s" % @@ -557,7 +609,7 @@ class SvcTool(Tool): :class:`Bcfg2.Client.Tools.Executor.run` """ self.logger.debug('Starting service %s' % service.get('name')) - return self.cmd.run(self.get_svc_command(service, 'start'))[0] + return self.cmd.run(self.get_svc_command(service, 'start')) def stop_service(self, service): """ Stop a service. @@ -568,7 +620,7 @@ class SvcTool(Tool): :class:`Bcfg2.Client.Tools.Executor.run` """ self.logger.debug('Stopping service %s' % service.get('name')) - return self.cmd.run(self.get_svc_command(service, 'stop'))[0] + return self.cmd.run(self.get_svc_command(service, 'stop')) def restart_service(self, service): """ Restart a service. @@ -580,7 +632,7 @@ class SvcTool(Tool): """ self.logger.debug('Restarting service %s' % service.get('name')) restart_target = service.get('target', 'restart') - return self.cmd.run(self.get_svc_command(service, restart_target))[0] + return self.cmd.run(self.get_svc_command(service, restart_target)) def check_service(self, service): """ Check the status a service. @@ -590,7 +642,7 @@ class SvcTool(Tool): :returns: bool - True if the status command returned 0, False otherwise """ - return self.cmd.run(self.get_svc_command(service, 'status'))[0] == 0 + return self.cmd.run(self.get_svc_command(service, 'status')) def Remove(self, services): if self.setup['servicemode'] != 'disabled': @@ -610,10 +662,10 @@ class SvcTool(Tool): not self.setup['interactive'])): continue - rv = None + success = False if entry.get('status') == 'on': if self.setup['servicemode'] == 'build': - rv = self.stop_service(entry) + success = self.stop_service(entry) elif entry.get('name') not in self.restarted: if self.setup['interactive']: prompt = ('Restart service %s?: (y/N): ' % @@ -625,12 +677,12 @@ class SvcTool(Tool): ans = input(prompt) if ans not in ['y', 'Y']: continue - rv = self.restart_service(entry) - if not rv: + success = self.restart_service(entry) + if success: self.restarted.append(entry.get('name')) else: - rv = self.stop_service(entry) - if rv: + success = self.stop_service(entry) + if not success: self.logger.error("Failed to manipulate service %s" % (entry.get('name'))) BundleUpdated.__doc__ = Tool.BundleUpdated.__doc__ -- cgit v1.2.3-1-g7c22