]> review.fuel-infra Code Review - openstack-build/cinder-build.git/commitdiff
Use common.processutils.execute
authorJohn Griffith <john.griffith@solidfire.com>
Tue, 4 Jun 2013 23:07:03 +0000 (17:07 -0600)
committerJohn Griffith <john.griffith@solidfire.com>
Sun, 16 Jun 2013 17:12:15 +0000 (11:12 -0600)
This patch replaces the cinder.utils.execute method
with a wrapper around openstack.common.processutils.execute.

With the wrapper I'm also converting the processutils exceptions
to maintain compatability with the existing exceptions that are
caught/raised in the code.

Change-Id: Ibc36a1e83191e7c888ea62c55d28b7023535d9fa

cinder/brick/iscsi/iscsi.py
cinder/utils.py

index 4ddaebf89ed70ef845c1c13a6beb4593eadf99f5..c212771d088f5c35978b1cbf5dee1c0128a5d787 100644 (file)
@@ -385,7 +385,7 @@ class LioAdm(TargetAdmin):
         chap_auth_userid = 'test_id'
         chap_auth_password = 'test_pass'
 
-        if chap_auth != None:
+        if chap_auth is not None:
             (chap_auth_userid, chap_auth_password) = chap_auth.split(' ')[1:]
 
         extra_args = []
index b2464d414e18419fd6a2d5d3e6f3d8f60eca9415..2f5efd829949aa19c19e73edfa765d053741afb1 100644 (file)
@@ -22,7 +22,6 @@
 
 import contextlib
 import datetime
-import errno
 import functools
 import hashlib
 import inspect
@@ -31,9 +30,7 @@ import paramiko
 import pyclbr
 import random
 import re
-import shlex
 import shutil
-import signal
 import sys
 import tempfile
 import time
@@ -44,7 +41,6 @@ from xml.sax import expatreader
 from xml.sax import saxutils
 
 from eventlet import event
-from eventlet.green import subprocess
 from eventlet import greenthread
 from eventlet import pools
 
@@ -55,6 +51,7 @@ from cinder.openstack.common import excutils
 from cinder.openstack.common import importutils
 from cinder.openstack.common import lockutils
 from cinder.openstack.common import log as logging
+from cinder.openstack.common import processutils
 from cinder.openstack.common import timeutils
 
 
@@ -94,143 +91,42 @@ def fetchfile(url, target):
     execute('curl', '--fail', url, '-o', target)
 
 
-def _subprocess_setup():
-    # Python installs a SIGPIPE handler by default. This is usually not what
-    # non-Python subprocesses expect.
-    signal.signal(signal.SIGPIPE, signal.SIG_DFL)
-
-
 def execute(*cmd, **kwargs):
-    """Helper method to execute command with optional retry.
-
-    If you add a run_as_root=True command, don't forget to add the
-    corresponding filter to etc/cinder/rootwrap.d !
-
-    :param cmd:                Passed to subprocess.Popen.
-    :param process_input:      Send to opened process.
-    :param check_exit_code:    Single bool, int, or list of allowed exit
-                               codes.  Defaults to [0].  Raise
-                               exception.ProcessExecutionError unless
-                               program exits with one of these code.
-    :param delay_on_retry:     True | False. Defaults to True. If set to
-                               True, wait a short amount of time
-                               before retrying.
-    :param attempts:           How many times to retry cmd.
-    :param run_as_root:        True | False. Defaults to False. If set to True,
-                               the command is prefixed by the command specified
-                               in the root_helper CONF.
-
-    :raises exception.Error: on receiving unknown arguments
-    :raises exception.ProcessExecutionError:
-
-    :returns: a tuple, (stdout, stderr) from the spawned process, or None if
-             the command fails.
-    """
-
-    process_input = kwargs.pop('process_input', None)
-    check_exit_code = kwargs.pop('check_exit_code', [0])
-    ignore_exit_code = False
-    if isinstance(check_exit_code, bool):
-        ignore_exit_code = not check_exit_code
-        check_exit_code = [0]
-    elif isinstance(check_exit_code, int):
-        check_exit_code = [check_exit_code]
-    delay_on_retry = kwargs.pop('delay_on_retry', True)
-    attempts = kwargs.pop('attempts', 1)
-    run_as_root = kwargs.pop('run_as_root', False)
-    shell = kwargs.pop('shell', False)
-
-    if len(kwargs):
-        raise exception.Error(_('Got unknown keyword args '
-                                'to utils.execute: %r') % kwargs)
-
-    if run_as_root:
-
-        if CONF.rootwrap_config is None or CONF.root_helper != 'sudo':
-            LOG.deprecated(_('The root_helper option (which lets you specify '
-                             'a root wrapper different from cinder-rootwrap, '
-                             'and defaults to using sudo) is now deprecated. '
-                             'You should use the rootwrap_config option '
-                             'instead.'))
-
-        if (CONF.rootwrap_config is not None):
-            cmd = ['sudo', 'cinder-rootwrap',
-                   CONF.rootwrap_config] + list(cmd)
-        else:
-            cmd = shlex.split(CONF.root_helper) + list(cmd)
-    cmd = map(str, cmd)
-
-    while attempts > 0:
-        attempts -= 1
-        try:
-            LOG.debug(_('Running cmd (subprocess): %s'), ' '.join(cmd))
-            _PIPE = subprocess.PIPE  # pylint: disable=E1101
-            obj = subprocess.Popen(cmd,
-                                   stdin=_PIPE,
-                                   stdout=_PIPE,
-                                   stderr=_PIPE,
-                                   close_fds=True,
-                                   preexec_fn=_subprocess_setup,
-                                   shell=shell)
-            result = None
-            if process_input is not None:
-                result = obj.communicate(process_input)
-            else:
-                result = obj.communicate()
-            obj.stdin.close()  # pylint: disable=E1101
-            _returncode = obj.returncode  # pylint: disable=E1101
-            if _returncode:
-                LOG.debug(_('Result was %s') % _returncode)
-                if not ignore_exit_code and _returncode not in check_exit_code:
-                    (stdout, stderr) = result
-                    raise exception.ProcessExecutionError(
-                        exit_code=_returncode,
-                        stdout=stdout,
-                        stderr=stderr,
-                        cmd=' '.join(cmd))
-            return result
-        except exception.ProcessExecutionError:
-            if not attempts:
-                raise
-            else:
-                LOG.debug(_('%r failed. Retrying.'), cmd)
-                if delay_on_retry:
-                    greenthread.sleep(random.randint(20, 200) / 100.0)
-        finally:
-            # NOTE(termie): this appears to be necessary to let the subprocess
-            #               call clean something up in between calls, without
-            #               it two execute calls in a row hangs the second one
-            greenthread.sleep(0)
+    """Convenience wrapper around oslo's execute() method."""
+    if 'run_as_root' in kwargs and not 'root_helper' in kwargs:
+        kwargs['root_helper'] =\
+            'sudo cinder-rootwrap %s' % CONF.rootwrap_config
+    try:
+        (stdout, stderr) = processutils.execute(*cmd, **kwargs)
+    except processutils.ProcessExecutionError as ex:
+        raise exception.ProcessExecutionError(
+            exit_code=ex.exit_code,
+            stderr=ex.stderr,
+            stdout=ex.stdout,
+            cmd=ex.cmd,
+            description=ex.description)
+    except processutils.UnknownArgumentError as ex:
+        raise exception.Error(ex.message)
+    return (stdout, stderr)
 
 
 def trycmd(*args, **kwargs):
-    """
-    A wrapper around execute() to more easily handle warnings and errors.
-
-    Returns an (out, err) tuple of strings containing the output of
-    the command's stdout and stderr.  If 'err' is not empty then the
-    command can be considered to have failed.
-
-    :discard_warnings   True | False. Defaults to False. If set to True,
-                        then for succeeding commands, stderr is cleared
-
-    """
-    discard_warnings = kwargs.pop('discard_warnings', False)
-
+    """Convenience wrapper around oslo's trycmd() method."""
+    if 'run_as_root' in kwargs and not 'root_helper' in kwargs:
+        kwargs['root_helper'] =\
+            'sudo cinder-rootwrap %s' % CONF.rootwrap_config
     try:
-        out, err = execute(*args, **kwargs)
-        failed = False
-    except exception.ProcessExecutionError as exn:
-        out, err = '', str(exn)
-        LOG.debug(err)
-        failed = True
-
-    if not failed and discard_warnings and err:
-        # Handle commands that output to stderr but otherwise succeed
-        LOG.debug(err)
-        err = ''
-
-    return out, err
+        (stdout, stderr) = processutils.trycmd(*args, **kwargs)
+    except processutils.ProcessExecutionError as ex:
+        raise exception.ProcessExecutionError(
+            exit_code=ex.exit_code,
+            stderr=ex.stderr,
+            stdout=ex.stdout,
+            cmd=ex.cmd,
+            description=ex.description)
+    except processutils.UnknownArgumentError as ex:
+        raise exception.Error(ex.message)
+    return (stdout, stderr)
 
 
 def ssh_execute(ssh, cmd, process_input=None,