openstack/nova
Code Issues Proposed changes

We no longer need rootwrap.

Browse Source 
(Well, except for starting privsep daemons, oh and of course
os-brick wants to know what our root helper is for some reason
that is a bit beyond me.)

Yes that's right, we now live in the future and no longer need
the run_as_root infrastructure in nova.utils. I'm sure this breaks
some out of tree drivers, but they've had several releases to
notice that we're moving in this direction and its pretty easy
for them to fix themselves these days.

Change-Id: I99c66558938db9beb0bda33d27a8e36b26b8fcac
This commit is contained in:
Michael Still 2019-02-27 05:06:31 +00:00 committed by Stephen Finucane
parent fb84430fd5
commit 1d2c677641
10 changed files with 24 additions and 357 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
nova/tests/unit/test_crypto.py
View File

@ -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)

189
nova/tests/unit/test_utils.py
View File

@ -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):

2
nova/tests/unit/virt/disk/mount/test_loop.py
View File

@ -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())

1
nova/tests/unit/virt/disk/mount/test_nbd.py
View File

@ -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())

4
nova/tests/unit/virt/libvirt/test_driver.py
View File

@ -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,

12
nova/tests/unit/virt/libvirt/test_imagebackend.py
View File

@ -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()

7
nova/tests/unit/virt/libvirt/test_vif.py
View File

@ -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)

7
nova/tests/unit/virt/libvirt/volume/test_volume.py
View File

@ -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())

12
nova/tests/unit/virt/test_virt.py
View File

@ -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/'

135
nova/utils.py
View File

@ -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):