diff options
author | Michael Still <mikal@stillhq.com> | 2017-09-27 06:59:01 +1000 |
---|---|---|
committer | Michael Still <mikal@stillhq.com> | 2017-10-24 18:50:34 +1100 |
commit | c7dae4e19bfb45a3cd6cbf78fa771f5e34e2651c (patch) | |
tree | 53b98f96941463d2d2a4aa042d01571981965d60 | |
parent | fd4b2aa4cbff797ed7904c0c3b7ebb72e8d6da47 (diff) | |
download | nova-c7dae4e19bfb45a3cd6cbf78fa771f5e34e2651c.tar.gz |
Move nbd commands to privsep.
The same pattern as previous patches. Some of these unit tests
are starting to be a bit simpler as we finish the transition.
Change-Id: If0e1fe4c0466f2f88525dc575af2ef366d4bb59d
blueprint: hurrah-for-privsep
-rw-r--r-- | etc/nova/rootwrap.d/compute.filters | 4 | ||||
-rw-r--r-- | nova/privsep/fs.py | 10 | ||||
-rw-r--r-- | nova/tests/unit/virt/disk/mount/test_nbd.py | 118 | ||||
-rw-r--r-- | nova/tests/unit/virt/test_virt.py | 28 | ||||
-rw-r--r-- | nova/virt/disk/api.py | 4 | ||||
-rw-r--r-- | nova/virt/disk/mount/nbd.py | 20 | ||||
-rw-r--r-- | releasenotes/notes/privsep-queens-rootwrap-adds-907aa1bc8e3eb2ca.yaml | 4 |
7 files changed, 88 insertions, 100 deletions
diff --git a/etc/nova/rootwrap.d/compute.filters b/etc/nova/rootwrap.d/compute.filters index 2ed7bcf37d..0260670caf 100644 --- a/etc/nova/rootwrap.d/compute.filters +++ b/etc/nova/rootwrap.d/compute.filters @@ -10,10 +10,6 @@ kpartx: CommandFilter, kpartx, root # nova/virt/xenapi/vm_utils.py: tune2fs, -j, partition_path tune2fs: CommandFilter, tune2fs, root -# nova/virt/disk/mount/nbd.py: 'qemu-nbd', '-c', device, image -# nova/virt/disk/mount/nbd.py: 'qemu-nbd', '-d', device -qemu-nbd: CommandFilter, qemu-nbd, root - # nova/virt/disk/vfs/localfs.py: 'blkid', '-o', 'value', '-s', 'TYPE', device blkid: CommandFilter, blkid, root diff --git a/nova/privsep/fs.py b/nova/privsep/fs.py index 8b1fb707eb..c97ed1afa4 100644 --- a/nova/privsep/fs.py +++ b/nova/privsep/fs.py @@ -97,3 +97,13 @@ def loopsetup(path): @nova.privsep.sys_admin_pctxt.entrypoint def loopremove(device): return processutils.execute('losetup', '--detach', device, attempts=3) + + +@nova.privsep.sys_admin_pctxt.entrypoint +def nbd_connect(device, image): + return processutils.execute('qemu-nbd', '-c', device, image) + + +@nova.privsep.sys_admin_pctxt.entrypoint +def nbd_disconnect(device): + return processutils.execute('qemu-nbd', '-d', device) diff --git a/nova/tests/unit/virt/disk/mount/test_nbd.py b/nova/tests/unit/virt/disk/mount/test_nbd.py index de9c6f83ec..2048766b2c 100644 --- a/nova/tests/unit/virt/disk/mount/test_nbd.py +++ b/nova/tests/unit/virt/disk/mount/test_nbd.py @@ -143,70 +143,45 @@ class NbdTestCase(test.NoDBTestCase): n = nbd.NbdMount(self.file, tempdir) self.assertFalse(n._inner_get_dev()) - def test_inner_get_dev_qemu_fails(self): + @mock.patch('nova.privsep.fs.nbd_connect', return_value=('', 'broken')) + def test_inner_get_dev_qemu_fails(self, mock_nbd_connect): tempdir = self.useFixture(fixtures.TempDir()).path n = nbd.NbdMount(self.file, tempdir) self.useFixture(fixtures.MonkeyPatch('os.path.exists', _fake_exists_no_users)) - # We have a trycmd that always fails - def fake_trycmd(*args, **kwargs): - return '', 'broken' - self.useFixture(fixtures.MonkeyPatch('nova.utils.trycmd', fake_trycmd)) - # Error logged, no device consumed self.assertFalse(n._inner_get_dev()) self.assertTrue(n.error.startswith('qemu-nbd error')) - def test_inner_get_dev_qemu_timeout(self): + @mock.patch('random.shuffle') + @mock.patch('os.path.exists', side_effect=[True, False, False, False, + False, False, False, False]) + @mock.patch('os.listdir', return_value=['nbd0', 'nbd1', 'loop0']) + @mock.patch('nova.privsep.fs.nbd_connect', return_value=('', '')) + @mock.patch('nova.privsep.fs.nbd_disconnect', return_value=('', '')) + @mock.patch('time.sleep') + def test_inner_get_dev_qemu_timeout(self, mock_sleep, mock_nbd_disconnct, + mock_nbd_connect, mock_exists, + mock_listdir, mock_shuffle): + self.flags(timeout_nbd=3) tempdir = self.useFixture(fixtures.TempDir()).path n = nbd.NbdMount(self.file, tempdir) - self.useFixture(fixtures.MonkeyPatch('os.path.exists', - _fake_exists_no_users)) - - # We have a trycmd that always passed - def fake_trycmd(*args, **kwargs): - return '', '' - self.useFixture(fixtures.MonkeyPatch('nova.utils.trycmd', fake_trycmd)) - self.useFixture(fixtures.MonkeyPatch('time.sleep', _fake_noop)) # Error logged, no device consumed self.assertFalse(n._inner_get_dev()) self.assertTrue(n.error.endswith('did not show up')) - def fake_exists_one(self, path): - # We need the pid file for the device which is allocated to exist, but - # only once it is allocated to us - if path.startswith('/sys/block/nbd'): - if path == '/sys/block/nbd1/pid': - return False - if path.endswith('pid'): - return False - return True - return ORIG_EXISTS(path) - - def fake_trycmd_creates_pid(self, *args, **kwargs): - def fake_exists_two(path): - if path.startswith('/sys/block/nbd'): - if path == '/sys/block/nbd0/pid': - return True - if path.endswith('pid'): - return False - return True - return ORIG_EXISTS(path) - self.useFixture(fixtures.MonkeyPatch('os.path.exists', - fake_exists_two)) - return '', '' - - def test_inner_get_dev_works(self): + @mock.patch('random.shuffle') + @mock.patch('os.path.exists', side_effect=[True, False, False, False, + False, True]) + @mock.patch('os.listdir', return_value=['nbd0', 'nbd1', 'loop0']) + @mock.patch('nova.privsep.fs.nbd_connect', return_value=('', '')) + @mock.patch('nova.privsep.fs.nbd_disconnect') + def test_inner_get_dev_works(self, mock_nbd_disconnect, mock_nbd_connect, + mock_exists, mock_listdir, mock_shuffle): tempdir = self.useFixture(fixtures.TempDir()).path n = nbd.NbdMount(self.file, tempdir) - self.useFixture(fixtures.MonkeyPatch('random.shuffle', _fake_noop)) - self.useFixture(fixtures.MonkeyPatch('os.path.exists', - self.fake_exists_one)) - self.useFixture(fixtures.MonkeyPatch('nova.utils.trycmd', - self.fake_trycmd_creates_pid)) - self.useFixture(fixtures.MonkeyPatch('nova.utils.execute', _fake_noop)) # No error logged, device consumed self.assertTrue(n._inner_get_dev()) @@ -228,15 +203,16 @@ class NbdTestCase(test.NoDBTestCase): self.useFixture(fixtures.MonkeyPatch('nova.utils.execute', _fake_noop)) n.unget_dev() - def test_get_dev(self): + @mock.patch('random.shuffle') + @mock.patch('os.path.exists', side_effect=[True, False, False, False, + False, True]) + @mock.patch('os.listdir', return_value=['nbd0', 'nbd1', 'loop0']) + @mock.patch('nova.privsep.fs.nbd_connect', return_value=('', '')) + @mock.patch('nova.privsep.fs.nbd_disconnect') + def test_get_dev(self, mock_nbd_disconnect, mock_nbd_connect, + mock_exists, mock_listdir, mock_shuffle): tempdir = self.useFixture(fixtures.TempDir()).path n = nbd.NbdMount(self.file, tempdir) - self.useFixture(fixtures.MonkeyPatch('random.shuffle', _fake_noop)) - self.useFixture(fixtures.MonkeyPatch('nova.utils.execute', _fake_noop)) - self.useFixture(fixtures.MonkeyPatch('os.path.exists', - self.fake_exists_one)) - self.useFixture(fixtures.MonkeyPatch('nova.utils.trycmd', - self.fake_trycmd_creates_pid)) # No error logged, device consumed self.assertTrue(n.get_dev()) @@ -250,22 +226,18 @@ class NbdTestCase(test.NoDBTestCase): self.assertEqual('', n.error) self.assertIsNone(n.device) - def test_get_dev_timeout(self): - # Always fail to get a device - def fake_get_dev_fails(self): - return False - self.stub_out('nova.virt.disk.mount.nbd.NbdMount._inner_get_dev', - fake_get_dev_fails) - + @mock.patch('random.shuffle') + @mock.patch('time.sleep') + @mock.patch('nova.privsep.fs.nbd_connect') + @mock.patch('nova.privsep.fs.nbd_disconnect') + @mock.patch('os.path.exists', return_value=True) + @mock.patch('nova.virt.disk.mount.nbd.NbdMount._inner_get_dev', + return_value=False) + def test_get_dev_timeout(self, mock_get_dev, mock_exists, + mock_nbd_disconnect, mock_nbd_connect, + mock_sleep, mock_shuffle): tempdir = self.useFixture(fixtures.TempDir()).path n = nbd.NbdMount(self.file, tempdir) - self.useFixture(fixtures.MonkeyPatch('random.shuffle', _fake_noop)) - self.useFixture(fixtures.MonkeyPatch('time.sleep', _fake_noop)) - self.useFixture(fixtures.MonkeyPatch('nova.utils.execute', _fake_noop)) - self.useFixture(fixtures.MonkeyPatch('os.path.exists', - self.fake_exists_one)) - self.useFixture(fixtures.MonkeyPatch('nova.utils.trycmd', - self.fake_trycmd_creates_pid)) self.useFixture(fixtures.MonkeyPatch(('nova.virt.disk.mount.api.' 'MAX_DEVICE_WAIT'), -10)) @@ -288,7 +260,11 @@ class NbdTestCase(test.NoDBTestCase): self.assertFalse(mount.do_mount()) - def test_device_creation_race(self): + @mock.patch('nova.privsep.fs.nbd_connect') + @mock.patch('nova.privsep.fs.nbd_disconnect') + @mock.patch('os.path.exists') + def test_device_creation_race(self, mock_exists, mock_nbd_disconnect, + mock_nbd_connect): # Make sure that even if two threads create instances at the same time # they cannot choose the same nbd number (see bug 1207422) @@ -316,10 +292,8 @@ class NbdTestCase(test.NoDBTestCase): self.stub_out('nova.virt.disk.mount.nbd.NbdMount._allocate_nbd', fake_find_unused) - self.useFixture(fixtures.MonkeyPatch('nova.utils.trycmd', - delay_and_remove_device)) - self.useFixture(fixtures.MonkeyPatch('os.path.exists', - pid_exists)) + mock_nbd_connect.side_effect = delay_and_remove_device + mock_exists.side_effect = pid_exists def get_a_device(): n = nbd.NbdMount(self.file, tempdir) diff --git a/nova/tests/unit/virt/test_virt.py b/nova/tests/unit/virt/test_virt.py index f54e217a53..6743c0dac6 100644 --- a/nova/tests/unit/virt/test_virt.py +++ b/nova/tests/unit/virt/test_virt.py @@ -214,8 +214,10 @@ class TestVirtDisk(test.NoDBTestCase): @mock.patch('os.path.exists', return_value=True) @mock.patch('nova.privsep.fs.loopremove') @mock.patch('nova.privsep.fs.umount') + @mock.patch('nova.privsep.fs.nbd_disconnect') def test_lxc_teardown_container( - self, mock_umount, mock_loopremove, mock_exist): + self, mock_nbd_disconnect, mock_umount, mock_loopremove, + mock_exist): def proc_mounts(mount_point): mount_points = { @@ -248,18 +250,20 @@ class TestVirtDisk(test.NoDBTestCase): disk_api.teardown_container('/mnt/nbd/nopart') expected_commands += [ ('blockdev', '--flushbufs', '/dev/nbd15'), - ('qemu-nbd', '-d', '/dev/nbd15'), ] + mock_nbd_disconnect.assert_has_calls([mock.call('/dev/nbd15')]) mock_umount.assert_has_calls([mock.call('/dev/nbd15')]) + mock_nbd_disconnect.reset_mock() mock_umount.reset_mock() disk_api.teardown_container('/mnt/nbd/part') expected_commands += [ ('blockdev', '--flushbufs', '/dev/nbd15'), ('kpartx', '-d', '/dev/nbd15'), - ('qemu-nbd', '-d', '/dev/nbd15'), ] + mock_nbd_disconnect.assert_has_calls([mock.call('/dev/nbd15')]) mock_umount.assert_has_calls([mock.call('/dev/mapper/nbd15p1')]) + mock_nbd_disconnect.reset_mock() mock_umount.reset_mock() # NOTE(thomasem): Not adding any commands in this case, because we're @@ -274,10 +278,10 @@ class TestVirtDisk(test.NoDBTestCase): @mock.patch('nova.virt.disk.api._DiskImage._device_for_path', return_value=None) @mock.patch('nova.privsep.fs.loopremove') + @mock.patch('nova.privsep.fs.nbd_disconnect') def test_lxc_teardown_container_with_namespace_cleaned( - self, mock_loopremove, mock_device_for_path, mock_exists): - - expected_commands = [] + self, mock_nbd_disconnect, mock_loopremove, mock_device_for_path, + mock_exists): disk_api.teardown_container('/mnt/loop/nopart', '/dev/loop0') mock_loopremove.assert_has_calls([mock.call('/dev/loop0')]) @@ -288,13 +292,9 @@ class TestVirtDisk(test.NoDBTestCase): mock_loopremove.reset_mock() disk_api.teardown_container('/mnt/nbd/nopart', '/dev/nbd15') - expected_commands += [ - ('qemu-nbd', '-d', '/dev/nbd15'), - ] + mock_nbd_disconnect.assert_has_calls([mock.call('/dev/nbd15')]) + mock_nbd_disconnect.reset_mock() disk_api.teardown_container('/mnt/nbd/part', '/dev/nbd15') - expected_commands += [ - ('qemu-nbd', '-d', '/dev/nbd15'), - ] - - self.assertEqual(self.executes, expected_commands) + mock_nbd_disconnect.assert_has_calls([mock.call('/dev/nbd15')]) + mock_nbd_disconnect.reset_mock() diff --git a/nova/virt/disk/api.py b/nova/virt/disk/api.py index 2d45d86fec..a3ac3f28b7 100644 --- a/nova/virt/disk/api.py +++ b/nova/virt/disk/api.py @@ -37,6 +37,7 @@ from oslo_serialization import jsonutils import nova.conf from nova import exception from nova.i18n import _ +import nova.privsep.fs import nova.privsep.libvirt from nova import utils from nova.virt.disk.mount import api as mount @@ -450,8 +451,7 @@ def teardown_container(container_dir, container_root_device=None): nova.privsep.fs.loopremove(container_root_device) elif 'nbd' in container_root_device: LOG.debug('Release nbd device %s', container_root_device) - utils.execute('qemu-nbd', '-d', container_root_device, - run_as_root=True) + nova.privsep.fs.nbd_disconnect(container_root_device) else: LOG.debug('No release necessary for block device %s', container_root_device) diff --git a/nova/virt/disk/mount/nbd.py b/nova/virt/disk/mount/nbd.py index c7f5683627..8010578fb2 100644 --- a/nova/virt/disk/mount/nbd.py +++ b/nova/virt/disk/mount/nbd.py @@ -18,10 +18,13 @@ import random import re import time +from oslo_concurrency import processutils from oslo_log import log as logging +import six import nova.conf from nova.i18n import _ +import nova.privsep.fs from nova import utils from nova.virt.disk.mount import api @@ -76,9 +79,11 @@ class NbdMount(api.Mount): # already in use. LOG.debug('Get nbd device %(dev)s for %(imgfile)s', {'dev': device, 'imgfile': self.image.path}) - _out, err = utils.trycmd('qemu-nbd', '-c', device, - self.image.path, - run_as_root=True) + try: + _out, err = nova.privsep.fs.nbd_connect(device, self.image.path) + except processutils.ProcessExecutionError as exc: + err = six.text_type(exc) + if err: self.error = _('qemu-nbd error: %s') % err LOG.info('NBD mount error: %s', self.error) @@ -97,8 +102,11 @@ class NbdMount(api.Mount): LOG.info('NBD mount error: %s', self.error) # Cleanup - _out, err = utils.trycmd('qemu-nbd', '-d', device, - run_as_root=True) + try: + _out, err = nova.privsep.fs.nbd_disconnect(device) + except processutils.ProcessExecutionError as exc: + err = six.text_type(exc) + if err: LOG.warning('Detaching from erroneous nbd device returned ' 'error: %s', err) @@ -116,7 +124,7 @@ class NbdMount(api.Mount): if not self.linked: return LOG.debug('Release nbd device %s', self.device) - utils.execute('qemu-nbd', '-d', self.device, run_as_root=True) + nova.privsep.fs.nbd_disconnect(self.device) self.linked = False self.device = None diff --git a/releasenotes/notes/privsep-queens-rootwrap-adds-907aa1bc8e3eb2ca.yaml b/releasenotes/notes/privsep-queens-rootwrap-adds-907aa1bc8e3eb2ca.yaml index d72b220ca6..3bd98ebb7a 100644 --- a/releasenotes/notes/privsep-queens-rootwrap-adds-907aa1bc8e3eb2ca.yaml +++ b/releasenotes/notes/privsep-queens-rootwrap-adds-907aa1bc8e3eb2ca.yaml @@ -11,5 +11,5 @@ upgrade: - | The following commands are no longer required to be listed in your rootwrap configuration: cat; chown; cryptsetup; dd; losetup; lvcreate; lvremove; - lvs; mkdir; mount; nova-idmapshift; ploop; prl_disk_tool; readlink; shred; - tee; touch; umount; vgs; and xend. + lvs; mkdir; mount; nova-idmapshift; ploop; prl_disk_tool; qemu-nbd; + readlink; shred; tee; touch; umount; vgs; and xend. |