]> review.fuel-infra Code Review - openstack-build/neutron-build.git/commitdiff
Have L3 agent catch the correct exception
authorTerry Wilson <twilson@redhat.com>
Tue, 9 Dec 2014 22:24:37 +0000 (15:24 -0700)
committerTerry Wilson <twilson@redhat.com>
Thu, 11 Dec 2014 18:09:04 +0000 (18:09 +0000)
L3 agent imports the processutils module to catch exceptions that
wouldn't ever be thrown because the underlying execute() being
called is the one from neutron.agent.linux.utils which raises a
RuntimeError on failure.

Also, processutils is now part of oslo.concurrency. So when we
actually start using it, we'll use it from there.

Closes-Bug: 1401626
Change-Id: I43874e1b63a0ba7b01415cafe0538f4343057066

neutron/agent/l3/agent.py
neutron/openstack/common/processutils.py [deleted file]
neutron/tests/unit/test_l3_agent.py
openstack-common.conf

index 9ddf430f92a7bcad4decc34179b2b7db405cd309..290d8c3cdbf37530f130bae61bf8e25bfdd6c27b 100644 (file)
@@ -50,7 +50,6 @@ from neutron import manager
 from neutron.openstack.common import log as logging
 from neutron.openstack.common import loopingcall
 from neutron.openstack.common import periodic_task
-from neutron.openstack.common import processutils
 from neutron.openstack.common import service
 from neutron import service as neutron_service
 try:
@@ -817,8 +816,7 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback,
             net = netaddr.IPNetwork(ip_cidr)
             try:
                 device.addr.add(net.version, ip_cidr, str(net.broadcast))
-            except (processutils.UnknownArgumentError,
-                    processutils.ProcessExecutionError):
+            except RuntimeError:
                 # any exception occurred here should cause the floating IP
                 # to be set in error state
                 LOG.warn(_LW("Unable to configure IP address for "
diff --git a/neutron/openstack/common/processutils.py b/neutron/openstack/common/processutils.py
deleted file mode 100644 (file)
index 7b85b97..0000000
+++ /dev/null
@@ -1,289 +0,0 @@
-# Copyright 2011 OpenStack Foundation.
-# All Rights Reserved.
-#
-#    Licensed under the Apache License, Version 2.0 (the "License"); you may
-#    not use this file except in compliance with the License. You may obtain
-#    a copy of the License at
-#
-#         http://www.apache.org/licenses/LICENSE-2.0
-#
-#    Unless required by applicable law or agreed to in writing, software
-#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
-#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
-#    License for the specific language governing permissions and limitations
-#    under the License.
-
-"""
-System-level utilities and helper functions.
-"""
-
-import errno
-import logging
-import multiprocessing
-import os
-import random
-import shlex
-import signal
-
-from eventlet.green import subprocess
-from eventlet import greenthread
-from oslo.utils import strutils
-import six
-
-from neutron.openstack.common._i18n import _
-
-
-LOG = logging.getLogger(__name__)
-
-
-class InvalidArgumentError(Exception):
-    def __init__(self, message=None):
-        super(InvalidArgumentError, self).__init__(message)
-
-
-class UnknownArgumentError(Exception):
-    def __init__(self, message=None):
-        super(UnknownArgumentError, self).__init__(message)
-
-
-class ProcessExecutionError(Exception):
-    def __init__(self, stdout=None, stderr=None, exit_code=None, cmd=None,
-                 description=None):
-        self.exit_code = exit_code
-        self.stderr = stderr
-        self.stdout = stdout
-        self.cmd = cmd
-        self.description = description
-
-        if description is None:
-            description = _("Unexpected error while running command.")
-        if exit_code is None:
-            exit_code = '-'
-        message = _('%(description)s\n'
-                    'Command: %(cmd)s\n'
-                    'Exit code: %(exit_code)s\n'
-                    'Stdout: %(stdout)r\n'
-                    'Stderr: %(stderr)r') % {'description': description,
-                                             'cmd': cmd,
-                                             'exit_code': exit_code,
-                                             'stdout': stdout,
-                                             'stderr': stderr}
-        super(ProcessExecutionError, self).__init__(message)
-
-
-class NoRootWrapSpecified(Exception):
-    def __init__(self, message=None):
-        super(NoRootWrapSpecified, self).__init__(message)
-
-
-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 shell out and execute a command through subprocess.
-
-    Allows optional retry.
-
-    :param cmd:             Passed to subprocess.Popen.
-    :type cmd:              string
-    :param process_input:   Send to opened process.
-    :type process_input:    string
-    :param env_variables:   Environment variables and their values that
-                            will be set for the process.
-    :type env_variables:    dict
-    :param check_exit_code: Single bool, int, or list of allowed exit
-                            codes.  Defaults to [0].  Raise
-                            :class:`ProcessExecutionError` unless
-                            program exits with one of these code.
-    :type check_exit_code:  boolean, int, or [int]
-    :param delay_on_retry:  True | False. Defaults to True. If set to True,
-                            wait a short amount of time before retrying.
-    :type delay_on_retry:   boolean
-    :param attempts:        How many times to retry cmd.
-    :type attempts:         int
-    :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 kwarg.
-    :type run_as_root:      boolean
-    :param root_helper:     command to prefix to commands called with
-                            run_as_root=True
-    :type root_helper:      string
-    :param shell:           whether or not there should be a shell used to
-                            execute this command. Defaults to false.
-    :type shell:            boolean
-    :param loglevel:        log level for execute commands.
-    :type loglevel:         int.  (Should be logging.DEBUG or logging.INFO)
-    :returns:               (stdout, stderr) from process execution
-    :raises:                :class:`UnknownArgumentError` on
-                            receiving unknown arguments
-    :raises:                :class:`ProcessExecutionError`
-    """
-
-    process_input = kwargs.pop('process_input', None)
-    env_variables = kwargs.pop('env_variables', None)
-    check_exit_code = kwargs.pop('check_exit_code', [0])
-    ignore_exit_code = False
-    delay_on_retry = kwargs.pop('delay_on_retry', True)
-    attempts = kwargs.pop('attempts', 1)
-    run_as_root = kwargs.pop('run_as_root', False)
-    root_helper = kwargs.pop('root_helper', '')
-    shell = kwargs.pop('shell', False)
-    loglevel = kwargs.pop('loglevel', logging.DEBUG)
-
-    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]
-
-    if kwargs:
-        raise UnknownArgumentError(_('Got unknown keyword args: %r') % kwargs)
-
-    if run_as_root and hasattr(os, 'geteuid') and os.geteuid() != 0:
-        if not root_helper:
-            raise NoRootWrapSpecified(
-                message=_('Command requested root, but did not '
-                          'specify a root helper.'))
-        cmd = shlex.split(root_helper) + list(cmd)
-
-    cmd = map(str, cmd)
-    sanitized_cmd = strutils.mask_password(' '.join(cmd))
-
-    while attempts > 0:
-        attempts -= 1
-        try:
-            LOG.log(loglevel, _('Running cmd (subprocess): %s'), sanitized_cmd)
-            _PIPE = subprocess.PIPE  # pylint: disable=E1101
-
-            if os.name == 'nt':
-                preexec_fn = None
-                close_fds = False
-            else:
-                preexec_fn = _subprocess_setup
-                close_fds = True
-
-            obj = subprocess.Popen(cmd,
-                                   stdin=_PIPE,
-                                   stdout=_PIPE,
-                                   stderr=_PIPE,
-                                   close_fds=close_fds,
-                                   preexec_fn=preexec_fn,
-                                   shell=shell,
-                                   env=env_variables)
-            result = None
-            for _i in six.moves.range(20):
-                # NOTE(russellb) 20 is an arbitrary number of retries to
-                # prevent any chance of looping forever here.
-                try:
-                    if process_input is not None:
-                        result = obj.communicate(process_input)
-                    else:
-                        result = obj.communicate()
-                except OSError as e:
-                    if e.errno in (errno.EAGAIN, errno.EINTR):
-                        continue
-                    raise
-                break
-            obj.stdin.close()  # pylint: disable=E1101
-            _returncode = obj.returncode  # pylint: disable=E1101
-            LOG.log(loglevel, 'Result was %s' % _returncode)
-            if not ignore_exit_code and _returncode not in check_exit_code:
-                (stdout, stderr) = result
-                sanitized_stdout = strutils.mask_password(stdout)
-                sanitized_stderr = strutils.mask_password(stderr)
-                raise ProcessExecutionError(exit_code=_returncode,
-                                            stdout=sanitized_stdout,
-                                            stderr=sanitized_stderr,
-                                            cmd=sanitized_cmd)
-            return result
-        except ProcessExecutionError:
-            if not attempts:
-                raise
-            else:
-                LOG.log(loglevel, _('%r failed. Retrying.'), sanitized_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)
-
-
-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)
-
-    try:
-        out, err = execute(*args, **kwargs)
-        failed = False
-    except ProcessExecutionError as exn:
-        out, err = '', six.text_type(exn)
-        failed = True
-
-    if not failed and discard_warnings and err:
-        # Handle commands that output to stderr but otherwise succeed
-        err = ''
-
-    return out, err
-
-
-def ssh_execute(ssh, cmd, process_input=None,
-                addl_env=None, check_exit_code=True):
-    sanitized_cmd = strutils.mask_password(cmd)
-    LOG.debug('Running cmd (SSH): %s', sanitized_cmd)
-    if addl_env:
-        raise InvalidArgumentError(_('Environment not supported over SSH'))
-
-    if process_input:
-        # This is (probably) fixable if we need it...
-        raise InvalidArgumentError(_('process_input not supported over SSH'))
-
-    stdin_stream, stdout_stream, stderr_stream = ssh.exec_command(cmd)
-    channel = stdout_stream.channel
-
-    # NOTE(justinsb): This seems suspicious...
-    # ...other SSH clients have buffering issues with this approach
-    stdout = stdout_stream.read()
-    sanitized_stdout = strutils.mask_password(stdout)
-    stderr = stderr_stream.read()
-    sanitized_stderr = strutils.mask_password(stderr)
-
-    stdin_stream.close()
-
-    exit_status = channel.recv_exit_status()
-
-    # exit_status == -1 if no exit code was returned
-    if exit_status != -1:
-        LOG.debug('Result was %s' % exit_status)
-        if check_exit_code and exit_status != 0:
-            raise ProcessExecutionError(exit_code=exit_status,
-                                        stdout=sanitized_stdout,
-                                        stderr=sanitized_stderr,
-                                        cmd=sanitized_cmd)
-
-    return (sanitized_stdout, sanitized_stderr)
-
-
-def get_worker_count():
-    """Utility to get the default worker count.
-
-    @return: The number of CPUs if that can be determined, else a default
-             worker count of 1 is returned.
-    """
-    try:
-        return multiprocessing.cpu_count()
-    except NotImplementedError:
-        return 1
index e50eda7cc00aef1119f083500bb39b989fe9edeb..7c0fe49690fe5f81014b077c84926a65b417815b 100644 (file)
@@ -32,7 +32,6 @@ from neutron.common import config as base_config
 from neutron.common import constants as l3_constants
 from neutron.common import exceptions as n_exc
 from neutron.i18n import _LE
-from neutron.openstack.common import processutils
 from neutron.openstack.common import uuidutils
 from neutron.plugins.common import constants as p_const
 from neutron.tests import base
@@ -1110,7 +1109,7 @@ vrrp_instance VR_1 {
     @mock.patch('neutron.agent.linux.ip_lib.IPDevice')
     def test_process_router_floating_ip_with_device_add_error(self, IPDevice):
         IPDevice.return_value = device = mock.Mock()
-        device.addr.add.side_effect = processutils.ProcessExecutionError
+        device.addr.add.side_effect = RuntimeError()
         device.addr.list.return_value = []
         fip_id = _uuid()
         fip = {
index 7bba50dfae89a0d0b9e756b37c317e7959381d69..0afcafa74e9cedf150e2c11f9675922e9b0475f9 100644 (file)
@@ -13,7 +13,6 @@ module=loopingcall
 module=middleware
 module=periodic_task
 module=policy
-module=processutils
 module=service
 module=systemd
 module=threadgroup