diff --git a/nova/tests/unit/test_crypto.py b/nova/tests/unit/test_crypto.py index b8ed5474e3ee..0929f70881b6 100644 --- a/nova/tests/unit/test_crypto.py +++ b/nova/tests/unit/test_crypto.py @@ -74,12 +74,12 @@ e6fCXWECgYEAqgpGvva5kJ1ISgNwnJbwiNw0sOT9BMOsdNZBElf0kJIIy6FMPvap with open(sshkey, 'w') as f: f.write(ssh_private_key) try: - dec, _err = utils.execute('openssl', - 'rsautl', - '-decrypt', - '-inkey', sshkey, - process_input=text, - binary=True) + dec, _err = processutils.execute('openssl', + 'rsautl', + '-decrypt', + '-inkey', sshkey, + process_input=text, + binary=True) return dec except processutils.ProcessExecutionError as exc: raise exception.DecryptionFailure(reason=exc.stderr) diff --git a/nova/tests/unit/test_utils.py b/nova/tests/unit/test_utils.py index af1fc9110573..ec0c2eb6cd94 100644 --- a/nova/tests/unit/test_utils.py +++ b/nova/tests/unit/test_utils.py @@ -25,11 +25,9 @@ from keystoneauth1.identity import base as ks_identity from keystoneauth1 import session as ks_session import mock import netaddr -from oslo_concurrency import processutils from oslo_config import cfg from oslo_context import context as common_context from oslo_context import fixture as context_fixture -from oslo_log import log as logging from oslo_utils import encodeutils from oslo_utils import fixture as utils_fixture from oslo_utils import units @@ -219,23 +217,12 @@ class GenericUtilsTestCase(test.NoDBTestCase): self.assertIs(six.text_type, type(utils.get_obj_repr_unicode(instance))) - def test_use_rootwrap(self): - self.flags(disable_rootwrap=False, group='workarounds') - self.flags(rootwrap_config='foo') - cmd = utils.get_root_helper() - self.assertEqual('sudo nova-rootwrap foo', cmd) - - def test_use_sudo(self): - self.flags(disable_rootwrap=True, group='workarounds') - cmd = utils.get_root_helper() - self.assertEqual('sudo', cmd) - - def test_ssh_execute(self): + @mock.patch('oslo_concurrency.processutils.execute') + def test_ssh_execute(self, mock_execute): expected_args = ('ssh', '-o', 'BatchMode=yes', 'remotehost', 'ls', '-l') - with mock.patch('nova.utils.execute') as mock_method: - utils.ssh_execute('remotehost', 'ls', '-l') - mock_method.assert_called_once_with(*expected_args) + utils.ssh_execute('remotehost', 'ls', '-l') + mock_execute.assert_called_once_with(*expected_args) def test_generate_hostid(self): host = 'host' @@ -291,174 +278,6 @@ class TestCachedFile(test.NoDBTestCase): self.assertNotIn(filename, utils._FILE_CACHE) -class RootwrapDaemonTestCase(test.NoDBTestCase): - @mock.patch('oslo_rootwrap.client.Client') - def test_get_client(self, mock_client): - mock_conf = mock.MagicMock() - utils.RootwrapDaemonHelper(mock_conf) - mock_client.assert_called_once_with( - ["sudo", "nova-rootwrap-daemon", mock_conf]) - - @mock.patch('nova.utils.LOG.info') - def test_execute(self, mock_info): - mock_conf = mock.MagicMock() - daemon = utils.RootwrapDaemonHelper(mock_conf) - daemon.client = mock.MagicMock() - daemon.client.execute = mock.Mock(return_value=(0, None, None)) - - daemon.execute('a', 1, foo='bar', run_as_root=True) - daemon.client.execute.assert_called_once_with(['a', '1'], None) - mock_info.assert_has_calls([mock.call( - u'Executing RootwrapDaemonHelper.execute cmd=[%(cmd)r] ' - u'kwargs=[%(kwargs)r]', - {'cmd': u'a 1', 'kwargs': {'run_as_root': True, 'foo': 'bar'}})]) - - def test_execute_with_kwargs(self): - mock_conf = mock.MagicMock() - daemon = utils.RootwrapDaemonHelper(mock_conf) - daemon.client = mock.MagicMock() - daemon.client.execute = mock.Mock(return_value=(0, None, None)) - - daemon.execute('a', 1, foo='bar', run_as_root=True, process_input=True) - daemon.client.execute.assert_called_once_with(['a', '1'], True) - - def test_execute_fail(self): - mock_conf = mock.MagicMock() - daemon = utils.RootwrapDaemonHelper(mock_conf) - daemon.client = mock.MagicMock() - daemon.client.execute = mock.Mock(return_value=(-2, None, None)) - - self.assertRaises(processutils.ProcessExecutionError, - daemon.execute, 'b', 2) - - def test_execute_pass_with_check_exit_code(self): - mock_conf = mock.MagicMock() - daemon = utils.RootwrapDaemonHelper(mock_conf) - daemon.client = mock.MagicMock() - daemon.client.execute = mock.Mock(return_value=(-2, None, None)) - daemon.execute('b', 2, check_exit_code=[-2]) - - @mock.patch('time.sleep', new=mock.Mock()) - def test_execute_fail_with_retry(self): - mock_conf = mock.MagicMock() - daemon = utils.RootwrapDaemonHelper(mock_conf) - daemon.client = mock.MagicMock() - daemon.client.execute = mock.Mock(return_value=(-2, None, None)) - - self.assertRaises(processutils.ProcessExecutionError, - daemon.execute, 'b', 2, attempts=2) - daemon.client.execute.assert_has_calls( - [mock.call(['b', '2'], None), - mock.call(['b', '2'], None)]) - - @mock.patch('time.sleep', new=mock.Mock()) - @mock.patch('nova.utils.LOG.log') - def test_execute_fail_and_logging(self, mock_log): - mock_conf = mock.MagicMock() - daemon = utils.RootwrapDaemonHelper(mock_conf) - daemon.client = mock.MagicMock() - daemon.client.execute = mock.Mock(return_value=(-2, None, None)) - - self.assertRaises(processutils.ProcessExecutionError, - daemon.execute, 'b', 2, - attempts=2, - loglevel=logging.CRITICAL, - log_errors=processutils.LOG_ALL_ERRORS) - mock_log.assert_has_calls( - [ - mock.call(logging.CRITICAL, u'Running cmd (subprocess): %s', - u'b 2'), - mock.call(logging.CRITICAL, - 'CMD "%(sanitized_cmd)s" returned: %(return_code)s ' - 'in %(end_time)0.3fs', - {'sanitized_cmd': u'b 2', 'return_code': -2, - 'end_time': mock.ANY}), - mock.call(logging.CRITICAL, - u'%(desc)r\ncommand: %(cmd)r\nexit code: %(code)r' - u'\nstdout: %(stdout)r\nstderr: %(stderr)r', - {'code': -2, 'cmd': u'b 2', 'stdout': u'None', - 'stderr': u'None', 'desc': None}), - mock.call(logging.CRITICAL, u'%r failed. Retrying.', u'b 2'), - mock.call(logging.CRITICAL, u'Running cmd (subprocess): %s', - u'b 2'), - mock.call(logging.CRITICAL, - 'CMD "%(sanitized_cmd)s" returned: %(return_code)s ' - 'in %(end_time)0.3fs', - {'sanitized_cmd': u'b 2', 'return_code': -2, - 'end_time': mock.ANY}), - mock.call(logging.CRITICAL, - u'%(desc)r\ncommand: %(cmd)r\nexit code: %(code)r' - u'\nstdout: %(stdout)r\nstderr: %(stderr)r', - {'code': -2, 'cmd': u'b 2', 'stdout': u'None', - 'stderr': u'None', 'desc': None}), - mock.call(logging.CRITICAL, u'%r failed. Not Retrying.', - u'b 2')] - ) - - def test_trycmd(self): - mock_conf = mock.MagicMock() - daemon = utils.RootwrapDaemonHelper(mock_conf) - daemon.client = mock.MagicMock() - daemon.client.execute = mock.Mock(return_value=(0, None, None)) - - daemon.trycmd('a', 1, foo='bar', run_as_root=True) - daemon.client.execute.assert_called_once_with(['a', '1'], None) - - def test_trycmd_with_kwargs(self): - mock_conf = mock.MagicMock() - daemon = utils.RootwrapDaemonHelper(mock_conf) - daemon.execute = mock.Mock(return_value=('out', 'err')) - - daemon.trycmd('a', 1, foo='bar', run_as_root=True, - loglevel=logging.WARN, - log_errors=True, - process_input=True, - delay_on_retry=False, - attempts=5, - check_exit_code=[200]) - daemon.execute.assert_called_once_with('a', 1, attempts=5, - check_exit_code=[200], - delay_on_retry=False, foo='bar', - log_errors=True, loglevel=30, - process_input=True, - run_as_root=True) - - def test_trycmd_fail(self): - mock_conf = mock.MagicMock() - daemon = utils.RootwrapDaemonHelper(mock_conf) - daemon.client = mock.MagicMock() - daemon.client.execute = mock.Mock(return_value=(-2, None, None)) - - expected_err = six.text_type('''\ -Unexpected error while running command. -Command: a 1 -Exit code: -2''') - - out, err = daemon.trycmd('a', 1, foo='bar', run_as_root=True) - daemon.client.execute.assert_called_once_with(['a', '1'], None) - self.assertIn(expected_err, err) - - @mock.patch('time.sleep', new=mock.Mock()) - def test_trycmd_fail_with_retry(self): - mock_conf = mock.MagicMock() - daemon = utils.RootwrapDaemonHelper(mock_conf) - daemon.client = mock.MagicMock() - daemon.client.execute = mock.Mock(return_value=(-2, None, None)) - - expected_err = six.text_type('''\ -Unexpected error while running command. -Command: a 1 -Exit code: -2''') - - out, err = daemon.trycmd('a', 1, foo='bar', run_as_root=True, - attempts=3) - self.assertIn(expected_err, err) - daemon.client.execute.assert_has_calls( - [mock.call(['a', '1'], None), - mock.call(['a', '1'], None), - mock.call(['a', '1'], None)]) - - class AuditPeriodTest(test.NoDBTestCase): def setUp(self): diff --git a/nova/tests/unit/virt/disk/mount/test_loop.py b/nova/tests/unit/virt/disk/mount/test_loop.py index b7c5e5a9c620..3c0c18fa60f2 100644 --- a/nova/tests/unit/virt/disk/mount/test_loop.py +++ b/nova/tests/unit/virt/disk/mount/test_loop.py @@ -38,8 +38,6 @@ class LoopTestCase(test.NoDBTestCase): def test_get_dev(self, mock_loopremove, mock_loopsetup): tempdir = self.useFixture(fixtures.TempDir()).path mount = loop.LoopMount(self.file, tempdir) - self.useFixture(fixtures.MonkeyPatch('nova.utils.execute', - _fake_noop)) # No error logged, device consumed self.assertTrue(mount.get_dev()) diff --git a/nova/tests/unit/virt/disk/mount/test_nbd.py b/nova/tests/unit/virt/disk/mount/test_nbd.py index e406b75f984d..7bd48ff2c8ca 100644 --- a/nova/tests/unit/virt/disk/mount/test_nbd.py +++ b/nova/tests/unit/virt/disk/mount/test_nbd.py @@ -224,7 +224,6 @@ class NbdTestCase(test.NoDBTestCase): # something we don't have tempdir = self.useFixture(fixtures.TempDir()).path n = nbd.NbdMount(self.file, tempdir) - self.useFixture(fixtures.MonkeyPatch('nova.utils.execute', _fake_noop)) n.unget_dev() @mock.patch('time.sleep', new=mock.Mock()) diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py index 2dd38d958ba8..66fb0be57eab 100644 --- a/nova/tests/unit/virt/libvirt/test_driver.py +++ b/nova/tests/unit/virt/libvirt/test_driver.py @@ -13463,7 +13463,7 @@ class LibvirtConnTestCase(test.NoDBTestCase, disk_info['mapping'].pop('disk.local') with test.nested( - mock.patch.object(utils, 'execute'), + mock.patch('oslo_concurrency.processutils.execute'), mock.patch.object(drvr, 'get_info'), mock.patch.object(drvr, '_create_domain_and_network'), mock.patch.object(imagebackend.Image, 'verify_base_size'), @@ -16481,7 +16481,7 @@ class LibvirtConnTestCase(test.NoDBTestCase, @mock.patch.object(os, 'unlink') @mock.patch.object(os.path, 'exists') - @mock.patch.object(utils, 'execute') + @mock.patch('oslo_concurrency.processutils.execute') @mock.patch.object(libvirt_driver.LibvirtDriver, 'get_host_ip_addr', return_value='foo') def test_shared_storage_detection_easy(self, mock_get, mock_exec, diff --git a/nova/tests/unit/virt/libvirt/test_imagebackend.py b/nova/tests/unit/virt/libvirt/test_imagebackend.py index c7f4c5208ef7..cebeb5eb0017 100644 --- a/nova/tests/unit/virt/libvirt/test_imagebackend.py +++ b/nova/tests/unit/virt/libvirt/test_imagebackend.py @@ -1095,7 +1095,8 @@ class EncryptedLvmTestCase(_ImageTestCase, test.NoDBTestCase): mock.patch.object(self.dmcrypt, 'create_volume', mock.Mock()), mock.patch.object(self.dmcrypt, 'delete_volume', mock.Mock()), mock.patch.object(self.dmcrypt, 'list_volumes', mock.Mock()), - mock.patch.object(self.utils, 'execute', mock.Mock())): + mock.patch('oslo_concurrency.processutils.execute', + mock.Mock())): fn = mock.Mock() self.lvm.create_volume.side_effect = RuntimeError() @@ -1132,7 +1133,8 @@ class EncryptedLvmTestCase(_ImageTestCase, test.NoDBTestCase): mock.patch.object(self.dmcrypt, 'create_volume', mock.Mock()), mock.patch.object(self.dmcrypt, 'delete_volume', mock.Mock()), mock.patch.object(self.dmcrypt, 'list_volumes', mock.Mock()), - mock.patch.object(self.utils, 'execute', mock.Mock())): + mock.patch('oslo_concurrency.processutils.execute', + mock.Mock())): fn = mock.Mock() self.dmcrypt.create_volume.side_effect = RuntimeError() @@ -1174,7 +1176,8 @@ class EncryptedLvmTestCase(_ImageTestCase, test.NoDBTestCase): mock.patch.object(self.dmcrypt, 'create_volume', mock.Mock()), mock.patch.object(self.dmcrypt, 'delete_volume', mock.Mock()), mock.patch.object(self.dmcrypt, 'list_volumes', mock.Mock()), - mock.patch.object(self.utils, 'execute', mock.Mock())): + mock.patch('oslo_concurrency.processutils.execute', + mock.Mock())): fn = mock.Mock() fn.side_effect = RuntimeError() @@ -1216,7 +1219,8 @@ class EncryptedLvmTestCase(_ImageTestCase, test.NoDBTestCase): mock.patch.object(self.dmcrypt, 'create_volume', mock.Mock()), mock.patch.object(self.dmcrypt, 'delete_volume', mock.Mock()), mock.patch.object(self.dmcrypt, 'list_volumes', mock.Mock()), - mock.patch.object(self.utils, 'execute', mock.Mock())): + mock.patch('oslo_concurrency.processutils.execute', + mock.Mock())): fn = mock.Mock() fn.side_effect = RuntimeError() diff --git a/nova/tests/unit/virt/libvirt/test_vif.py b/nova/tests/unit/virt/libvirt/test_vif.py index 0d88b9694843..bff9e1a60453 100644 --- a/nova/tests/unit/virt/libvirt/test_vif.py +++ b/nova/tests/unit/virt/libvirt/test_vif.py @@ -505,13 +505,6 @@ class LibvirtVifTestCase(test.NoDBTestCase): # os_vif.initialize is typically done in nova-compute startup os_vif.initialize() self.setup_os_vif_objects() - self.executes = [] - - def fake_execute(*cmd, **kwargs): - self.executes.append(cmd) - return None, None - - self.stub_out('nova.utils.execute', fake_execute) def _get_node(self, xml): doc = etree.fromstring(xml) diff --git a/nova/tests/unit/virt/libvirt/volume/test_volume.py b/nova/tests/unit/virt/libvirt/volume/test_volume.py index 34758850c728..34b9e45cfb92 100644 --- a/nova/tests/unit/virt/libvirt/volume/test_volume.py +++ b/nova/tests/unit/virt/libvirt/volume/test_volume.py @@ -67,13 +67,6 @@ class LibvirtVolumeBaseTestCase(test.NoDBTestCase): def setUp(self): super(LibvirtVolumeBaseTestCase, self).setUp() - self.executes = [] - - def fake_execute(*cmd, **kwargs): - self.executes.append(cmd) - return None, None - - self.stub_out('nova.utils.execute', fake_execute) self.useFixture(fakelibvirt.FakeLibvirtFixture()) diff --git a/nova/tests/unit/virt/test_virt.py b/nova/tests/unit/virt/test_virt.py index b01df08e535c..6e817d5f1004 100644 --- a/nova/tests/unit/virt/test_virt.py +++ b/nova/tests/unit/virt/test_virt.py @@ -181,18 +181,6 @@ class TestDiskImage(test.NoDBTestCase): class TestVirtDisk(test.NoDBTestCase): - def setUp(self): - super(TestVirtDisk, self).setUp() - - # TODO(mikal): this can probably be removed post privsep cleanup. - self.executes = [] - - def fake_execute(*cmd, **kwargs): - self.executes.append(cmd) - return None, None - - self.stub_out('nova.utils.execute', fake_execute) - def test_lxc_setup_container(self): image = '/tmp/fake-image' container_dir = '/mnt/fake_rootfs/' diff --git a/nova/utils.py b/nova/utils.py index 05658f2f3ed7..2a6c5caa9ad3 100644 --- a/nova/utils.py +++ b/nova/utils.py @@ -28,7 +28,6 @@ import random import re import shutil import tempfile -import time import eventlet from keystoneauth1 import exceptions as ks_exc @@ -53,7 +52,7 @@ from six.moves import range import nova.conf from nova import exception -from nova.i18n import _, _LE, _LI, _LW +from nova.i18n import _LE, _LW import nova.network from nova import safe_utils @@ -99,6 +98,8 @@ else: getargspec = inspect.getargspec +# NOTE(mikal): this seems to have to stay for now to handle os-brick +# requirements. This makes me a sad panda. def get_root_helper(): if CONF.workarounds.disable_rootwrap: cmd = 'sudo' @@ -107,140 +108,12 @@ def get_root_helper(): return cmd -class RootwrapProcessHelper(object): - def trycmd(self, *cmd, **kwargs): - kwargs['root_helper'] = get_root_helper() - return processutils.trycmd(*cmd, **kwargs) - - def execute(self, *cmd, **kwargs): - kwargs['root_helper'] = get_root_helper() - return processutils.execute(*cmd, **kwargs) - - -class RootwrapDaemonHelper(RootwrapProcessHelper): - _clients = {} - - @synchronized('daemon-client-lock') - def _get_client(cls, rootwrap_config): - try: - return cls._clients[rootwrap_config] - except KeyError: - from oslo_rootwrap import client - new_client = client.Client([ - "sudo", "nova-rootwrap-daemon", rootwrap_config]) - cls._clients[rootwrap_config] = new_client - return new_client - - def __init__(self, rootwrap_config): - self.client = self._get_client(rootwrap_config) - - def trycmd(self, *args, **kwargs): - discard_warnings = kwargs.pop('discard_warnings', False) - try: - out, err = self.execute(*args, **kwargs) - failed = False - except processutils.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 execute(self, *cmd, **kwargs): - # NOTE(dims): This method is to provide compatibility with the - # processutils.execute interface. So that calling daemon or direct - # rootwrap to honor the same set of flags in kwargs and to ensure - # that we don't regress any current behavior. - cmd = [str(c) for c in cmd] - loglevel = kwargs.pop('loglevel', logging.DEBUG) - log_errors = kwargs.pop('log_errors', None) - process_input = kwargs.pop('process_input', None) - delay_on_retry = kwargs.pop('delay_on_retry', True) - attempts = kwargs.pop('attempts', 1) - 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] - - sanitized_cmd = strutils.mask_password(' '.join(cmd)) - LOG.info(_LI('Executing RootwrapDaemonHelper.execute ' - 'cmd=[%(cmd)r] kwargs=[%(kwargs)r]'), - {'cmd': sanitized_cmd, 'kwargs': kwargs}) - - while attempts > 0: - attempts -= 1 - try: - start_time = time.time() - LOG.log(loglevel, _('Running cmd (subprocess): %s'), - sanitized_cmd) - - (returncode, out, err) = self.client.execute( - cmd, process_input) - - end_time = time.time() - start_time - LOG.log(loglevel, - 'CMD "%(sanitized_cmd)s" returned: %(return_code)s ' - 'in %(end_time)0.3fs', - {'sanitized_cmd': sanitized_cmd, - 'return_code': returncode, - 'end_time': end_time}) - - if not ignore_exit_code and returncode not in check_exit_code: - out = strutils.mask_password(out) - err = strutils.mask_password(err) - raise processutils.ProcessExecutionError( - exit_code=returncode, - stdout=out, - stderr=err, - cmd=sanitized_cmd) - return (out, err) - - except processutils.ProcessExecutionError as err: - # if we want to always log the errors or if this is - # the final attempt that failed and we want to log that. - if log_errors == processutils.LOG_ALL_ERRORS or ( - log_errors == processutils.LOG_FINAL_ERROR and - not attempts): - format = _('%(desc)r\ncommand: %(cmd)r\n' - 'exit code: %(code)r\nstdout: %(stdout)r\n' - 'stderr: %(stderr)r') - LOG.log(loglevel, format, {"desc": err.description, - "cmd": err.cmd, - "code": err.exit_code, - "stdout": err.stdout, - "stderr": err.stderr}) - if not attempts: - LOG.log(loglevel, _('%r failed. Not Retrying.'), - sanitized_cmd) - raise - else: - LOG.log(loglevel, _('%r failed. Retrying.'), - sanitized_cmd) - if delay_on_retry: - time.sleep(random.randint(20, 200) / 100.0) - - -def execute(*cmd, **kwargs): - """Convenience wrapper around oslo's execute() method.""" - if 'run_as_root' in kwargs and kwargs.get('run_as_root'): - if CONF.use_rootwrap_daemon: - return RootwrapDaemonHelper(CONF.rootwrap_config).execute( - *cmd, **kwargs) - else: - return RootwrapProcessHelper().execute(*cmd, **kwargs) - return processutils.execute(*cmd, **kwargs) - - def ssh_execute(dest, *cmd, **kwargs): """Convenience wrapper to execute ssh command.""" ssh_cmd = ['ssh', '-o', 'BatchMode=yes'] ssh_cmd.append(dest) ssh_cmd.extend(cmd) - return execute(*ssh_cmd, **kwargs) + return processutils.execute(*ssh_cmd, **kwargs) def generate_uid(topic, size=8):