From f389773f5255051f2f1cff4d0b0a08eafc0ac147 Mon Sep 17 00:00:00 2001 From: Michael Still Date: Fri, 29 Mar 2019 04:09:40 +0000 Subject: [PATCH] Improve test coverage of nova.privsep.fs, continued. Using the new PrivsepFixture, improve test coverage of filesystem utilities. This finishes off coverage for this file. Change-Id: I4af57ced168cf285cd3872d20dc2a369b3ac2e14 --- nova/privsep/fs.py | 23 +++++---- nova/tests/unit/privsep/test_fs.py | 83 +++++++++++++++++++++++++++++- 2 files changed, 94 insertions(+), 12 deletions(-) diff --git a/nova/privsep/fs.py b/nova/privsep/fs.py index 638fd50ad48f..0c1cc94dc423 100644 --- a/nova/privsep/fs.py +++ b/nova/privsep/fs.py @@ -142,7 +142,9 @@ def e2fsck(image, flags='-fp'): unprivileged_e2fsck(image, flags=flags) -# NOTE(mikal): this method is deliberately not wrapped in a privsep entrypoint +# NOTE(mikal): this method is deliberately not wrapped in a privsep +# entrypoint. This is not for unit testing, there are some callers who do +# not require elevated permissions when calling this. def unprivileged_e2fsck(image, flags='-fp'): processutils.execute('e2fsck', flags, image, check_exit_code=[0, 1, 2]) @@ -152,7 +154,9 @@ def resize2fs(image, check_exit_code, size=None): unprivileged_resize2fs(image, check_exit_code=check_exit_code, size=size) -# NOTE(mikal): this method is deliberately not wrapped in a privsep entrypoint +# NOTE(mikal): this method is deliberately not wrapped in a privsep +# entrypoint. This is not for unit testing, there are some callers who do +# not require elevated permissions when calling this. def unprivileged_resize2fs(image, check_exit_code, size=None): if size: cmd = ['resize2fs', image, size] @@ -180,7 +184,9 @@ def list_partitions(device): return unprivileged_list_partitions(device) -# NOTE(mikal): this method is deliberately not wrapped in a privsep entrypoint +# NOTE(mikal): this method is deliberately not wrapped in a privsep +# entrypoint. This is not for unit testing, there are some callers who do +# not require elevated permissions when calling this. def unprivileged_list_partitions(device): """Return partition information (num, size, type) for a device.""" @@ -206,11 +212,6 @@ def unprivileged_list_partitions(device): @nova.privsep.sys_admin_pctxt.entrypoint def resize_partition(device, start, end, bootable): - return unprivileged_resize_partition(device, start, end, bootable) - - -# NOTE(mikal): this method is deliberately not wrapped in a privsep entrypoint -def unprivileged_resize_partition(device, start, end, bootable): processutils.execute('parted', '--script', device, 'rm', '1') processutils.execute('parted', '--script', device, 'mkpart', 'primary', '%ds' % start, '%ds' % end) @@ -237,7 +238,7 @@ def ext_journal_enable(device): # NOTE(mikal): I really feel like this whole thing should be deprecated, I # just don't think its a great idea to let people specify a command in a -# configuration option to run a root. +# configuration option to run as root. _MKFS_COMMAND = {} _DEFAULT_MKFS_COMMAND = None @@ -309,7 +310,9 @@ def mkfs(fs, path, label=None): unprivileged_mkfs(fs, path, label=None) -# NOTE(mikal): this method is deliberately not wrapped in a privsep entrypoint +# NOTE(mikal): this method is deliberately not wrapped in a privsep +# entrypoint. This is not for unit testing, there are some callers who do +# not require elevated permissions when calling this. def unprivileged_mkfs(fs, path, label=None): """Format a file or block device diff --git a/nova/tests/unit/privsep/test_fs.py b/nova/tests/unit/privsep/test_fs.py index 1486869094f9..53748bb18128 100644 --- a/nova/tests/unit/privsep/test_fs.py +++ b/nova/tests/unit/privsep/test_fs.py @@ -150,18 +150,97 @@ class PrivsepFilesystemHelpersTestCase(test.NoDBTestCase): check_exit_code=[0, 2]) @mock.patch('oslo_concurrency.processutils.execute') - def test_list_partitions(self, mock_execute): + def test_privileged_e2fsck(self, mock_execute): + nova.privsep.fs.e2fsck('/path/nosuch') + mock_execute.assert_called_with('e2fsck', '-fp', '/path/nosuch', + check_exit_code=[0, 1, 2]) + + @mock.patch('oslo_concurrency.processutils.execute') + def test_privileged_e2fsck_with_flags(self, mock_execute): + nova.privsep.fs.e2fsck('/path/nosuch', flags='festive') + mock_execute.assert_called_with('e2fsck', 'festive', '/path/nosuch', + check_exit_code=[0, 1, 2]) + + @mock.patch('oslo_concurrency.processutils.execute') + def test_unprivileged_e2fsck(self, mock_execute): + nova.privsep.fs.unprivileged_e2fsck('/path/nosuch') + mock_execute.assert_called_with('e2fsck', '-fp', '/path/nosuch', + check_exit_code=[0, 1, 2]) + + @mock.patch('oslo_concurrency.processutils.execute') + def test_unprivileged_e2fsck_with_flags(self, mock_execute): + nova.privsep.fs.unprivileged_e2fsck('/path/nosuch', flags='festive') + mock_execute.assert_called_with('e2fsck', 'festive', '/path/nosuch', + check_exit_code=[0, 1, 2]) + + @mock.patch('oslo_concurrency.processutils.execute') + def test_privileged_resize2fs(self, mock_execute): + nova.privsep.fs.resize2fs('/path/nosuch', [0, 1, 2]) + mock_execute.assert_called_with('resize2fs', '/path/nosuch', + check_exit_code=[0, 1, 2]) + + @mock.patch('oslo_concurrency.processutils.execute') + def test_privileged_resize2fs_with_size(self, mock_execute): + nova.privsep.fs.resize2fs('/path/nosuch', [0, 1, 2], 1024) + mock_execute.assert_called_with('resize2fs', '/path/nosuch', 1024, + check_exit_code=[0, 1, 2]) + + @mock.patch('oslo_concurrency.processutils.execute') + def test_unprivileged_resize2fs(self, mock_execute): + nova.privsep.fs.unprivileged_resize2fs('/path/nosuch', [0, 1, 2]) + mock_execute.assert_called_with('resize2fs', '/path/nosuch', + check_exit_code=[0, 1, 2]) + + @mock.patch('oslo_concurrency.processutils.execute') + def test_unprivileged_resize2fs_with_size(self, mock_execute): + nova.privsep.fs.unprivileged_resize2fs('/path/nosuch', [0, 1, 2], 1024) + mock_execute.assert_called_with('resize2fs', '/path/nosuch', 1024, + check_exit_code=[0, 1, 2]) + + @mock.patch('oslo_concurrency.processutils.execute') + def test_create_partition_table(self, mock_execute): + nova.privsep.fs.create_partition_table('/dev/nosuch', 'style') + mock_execute.assert_called_with('parted', '--script', '/dev/nosuch', + 'mklabel', 'style', + check_exit_code=True) + + @mock.patch('oslo_concurrency.processutils.execute') + def test_create_partition(self, mock_execute): + nova.privsep.fs.create_partition('/dev/nosuch', 'style', 0, 100) + mock_execute.assert_called_with('parted', '--script', '/dev/nosuch', + '--', 'mkpart', 'style', 0, 100, + check_exit_code=True) + + @mock.patch('oslo_concurrency.processutils.execute') + def _test_list_partitions(self, meth, mock_execute): parted_return = "BYT;\n...\n" parted_return += "1:2s:11s:10s:ext3::boot;\n" parted_return += "2:20s:11s:10s::bob:;\n" mock_execute.return_value = (parted_return, None) - partitions = nova.privsep.fs.unprivileged_list_partitions("abc") + partitions = meth("abc") self.assertEqual(2, len(partitions)) self.assertEqual((1, 2, 10, "ext3", "", "boot"), partitions[0]) self.assertEqual((2, 20, 10, "", "bob", ""), partitions[1]) + def test_privileged_list_partitions(self): + self._test_list_partitions(nova.privsep.fs.list_partitions) + + def test_unprivileged_list_partitions(self): + self._test_list_partitions( + nova.privsep.fs.unprivileged_list_partitions) + + @mock.patch('oslo_concurrency.processutils.execute') + def test_resize_partition(self, mock_execute): + nova.privsep.fs.resize_partition('/dev/nosuch', 0, 100, True) + mock_execute.assert_has_calls([ + mock.call('parted', '--script', '/dev/nosuch', 'rm', '1'), + mock.call('parted', '--script', '/dev/nosuch', 'mkpart', + 'primary', '0s', '100s'), + mock.call('parted', '--script', '/dev/nosuch', + 'set', '1', 'boot', 'on')]) + class MkfsTestCase(test.NoDBTestCase): @mock.patch('oslo_concurrency.processutils.execute')