From af2fda6648cb33fad64651503f2d5465a881840f Mon Sep 17 00:00:00 2001 From: "Grzegorz Grasza (xek)" Date: Tue, 30 Jan 2018 12:39:32 +0100 Subject: [PATCH 1/8] Changing partition command to separate commands This is to avoid error "unable to inform the kernel". Co-Authored-By: Grzegorz Grasza Co-Authored-By: Marta Mucek Co-Authored-By: Piotr Prokop --- ironic_lib/disk_partitioner.py | 8 +-- ironic_lib/tests/test_disk_partitioner.py | 70 ++++++++++++------ ironic_lib/tests/test_disk_utils.py | 115 ++++++++++++++++++------------ 3 files changed, 120 insertions(+), 73 deletions(-) diff --git a/ironic_lib/disk_partitioner.py b/ironic_lib/disk_partitioner.py index 644d05a..cb68059 100644 --- a/ironic_lib/disk_partitioner.py +++ b/ironic_lib/disk_partitioner.py @@ -139,19 +139,19 @@ class DiskPartitioner(object): """Write to the disk.""" LOG.debug("Committing partitions to disk.") cmd_args = ['mklabel', self._disk_label] + self._exec(*cmd_args) # NOTE(lucasagomes): Lead in with 1MiB to allow room for the # partition table itself. start = 1 for num, part in self.get_partitions(): end = start + part['size'] - cmd_args.extend(['mkpart', part['type'], part['fs_type'], - str(start), str(end)]) + cmd_args = ['mkpart', part['type'], part['fs_type'], + str(start), str(end)] if part['boot_flag']: cmd_args.extend(['set', str(num), part['boot_flag'], 'on']) + self._exec(*cmd_args) start = end - self._exec(*cmd_args) - retries = [0] pids = [''] fuser_err = [''] diff --git a/ironic_lib/tests/test_disk_partitioner.py b/ironic_lib/tests/test_disk_partitioner.py index 45b740f..618af21 100644 --- a/ironic_lib/tests/test_disk_partitioner.py +++ b/ironic_lib/tests/test_disk_partitioner.py @@ -74,13 +74,21 @@ class DiskPartitionerTestCase(test_base.BaseTestCase): mock_utils_exc.return_value = (None, None) dp.commit() - mock_disk_partitioner_exec.assert_called_once_with( - mock.ANY, 'mklabel', 'msdos', - 'mkpart', 'fake-type', 'fake-fs-type', '1', '2', - 'mkpart', 'fake-type', 'fake-fs-type', '2', '3', - 'set', '2', 'boot', 'on', - 'mkpart', 'fake-type', 'fake-fs-type', '3', '4', - 'set', '3', 'bios_grub', 'on') + mock_disk_partitioner_exec.assert_any_call(mock.ANY, 'mklabel', + 'msdos') + mock_disk_partitioner_exec.assert_any_call(mock.ANY, 'mkpart', + 'fake-type', + 'fake-fs-type', '2', + '3', 'set', '2', 'boot', + 'on') + mock_disk_partitioner_exec.assert_any_call(mock.ANY, 'mkpart', + 'fake-type', + 'fake-fs-type', '1', + '2') + mock_disk_partitioner_exec.assert_any_call(mock.ANY, 'mkpart', + 'fake-type', 'fake-fs-type', + '3', '4', 'set', '3', + 'bios_grub', 'on') mock_utils_exc.assert_called_once_with( 'fuser', '/dev/fake', run_as_root=True, check_exit_code=[0, 1]) @@ -106,11 +114,17 @@ class DiskPartitionerTestCase(test_base.BaseTestCase): mock_utils_exc.side_effect = fuser_outputs dp.commit() - mock_disk_partitioner_exec.assert_called_once_with( - mock.ANY, 'mklabel', 'msdos', - 'mkpart', 'fake-type', 'fake-fs-type', '1', '2', - 'mkpart', 'fake-type', 'fake-fs-type', '2', '3', - 'set', '2', 'boot', 'on') + mock_disk_partitioner_exec.assert_any_call(mock.ANY, 'mklabel', + 'msdos') + mock_disk_partitioner_exec.assert_any_call(mock.ANY, 'mkpart', + 'fake-type', + 'fake-fs-type', '2', + '3', 'set', '2', 'boot', + 'on') + mock_disk_partitioner_exec.assert_any_call(mock.ANY, 'mkpart', + 'fake-type', + 'fake-fs-type', '1', + '2') mock_utils_exc.assert_called_with( 'fuser', '/dev/fake', run_as_root=True, check_exit_code=[0, 1]) self.assertEqual(2, mock_utils_exc.call_count) @@ -136,11 +150,17 @@ class DiskPartitionerTestCase(test_base.BaseTestCase): mock_utils_exc.return_value = ("/dev/fake: 10000 10001", None) self.assertRaises(exception.InstanceDeployFailure, dp.commit) - mock_disk_partitioner_exec.assert_called_once_with( - mock.ANY, 'mklabel', 'msdos', - 'mkpart', 'fake-type', 'fake-fs-type', '1', '2', - 'mkpart', 'fake-type', 'fake-fs-type', '2', '3', - 'set', '2', 'boot', 'on') + mock_disk_partitioner_exec.assert_any_call(mock.ANY, 'mklabel', + 'msdos') + mock_disk_partitioner_exec.assert_any_call(mock.ANY, 'mkpart', + 'fake-type', + 'fake-fs-type', '2', + '3', 'set', '2', 'boot', + 'on') + mock_disk_partitioner_exec.assert_any_call(mock.ANY, 'mkpart', + 'fake-type', + 'fake-fs-type', '1', + '2') mock_utils_exc.assert_called_with( 'fuser', '/dev/fake', run_as_root=True, check_exit_code=[0, 1]) self.assertEqual(20, mock_utils_exc.call_count) @@ -168,11 +188,17 @@ class DiskPartitionerTestCase(test_base.BaseTestCase): " does not exist.") self.assertRaises(exception.InstanceDeployFailure, dp.commit) - mock_disk_partitioner_exec.assert_called_once_with( - mock.ANY, 'mklabel', 'msdos', - 'mkpart', 'fake-type', 'fake-fs-type', '1', '2', - 'mkpart', 'fake-type', 'fake-fs-type', '2', '3', - 'set', '2', 'boot', 'on') + mock_disk_partitioner_exec.assert_any_call(mock.ANY, 'mklabel', + 'msdos') + mock_disk_partitioner_exec.assert_any_call(mock.ANY, 'mkpart', + 'fake-type', + 'fake-fs-type', '2', + '3', 'set', '2', 'boot', + 'on') + mock_disk_partitioner_exec.assert_any_call(mock.ANY, 'mkpart', + 'fake-type', + 'fake-fs-type', '1', + '2') mock_utils_exc.assert_called_with( 'fuser', '/dev/fake', run_as_root=True, check_exit_code=[0, 1]) self.assertEqual(20, mock_utils_exc.call_count) diff --git a/ironic_lib/tests/test_disk_utils.py b/ironic_lib/tests/test_disk_utils.py index 5d05f23..a46f985 100644 --- a/ironic_lib/tests/test_disk_utils.py +++ b/ironic_lib/tests/test_disk_utils.py @@ -307,12 +307,32 @@ class MakePartitionsTestCase(test_base.BaseTestCase): self.efi_size = CONF.disk_utils.efi_system_partition_size self.bios_size = CONF.disk_utils.bios_boot_partition_size - def _get_parted_cmd(self, dev, label=None): + def _get_parted_cmd(self, dev, label=None, make_label=None): if label is None: label = 'msdos' + if make_label: + return ['parted', '-a', 'optimal', '-s', dev, + '--', 'unit', 'MiB', 'mklabel', label] + return ['parted', '-a', 'optimal', '-s', dev, - '--', 'unit', 'MiB', 'mklabel', label] + '--', 'unit', 'MiB'] + + def _get_calls(self, expected_mkpart, label=None): + calls = [] + # make partition table + cmd = self._get_parted_cmd(self.dev, label=label, make_label=True) + calls.append(mock.call(*cmd, use_standard_locale=True, + run_as_root=True, check_exit_code=[0])) + + # make partitions + for mkpart in expected_mkpart: + cmd = self._get_parted_cmd(self.dev, label=label) + list(mkpart) + parted_call = mock.call(*cmd, use_standard_locale=True, + run_as_root=True, check_exit_code=[0]) + calls.append(parted_call) + + return calls def _test_make_partitions(self, mock_exc, boot_option, boot_mode='bios', disk_label=None): @@ -326,41 +346,42 @@ class MakePartitionsTestCase(test_base.BaseTestCase): if boot_option == "local" and boot_mode == "uefi": add_efi_sz = lambda x: str(_s(x, self.efi_size)) - expected_mkpart = ['mkpart', 'primary', 'fat32', '1', - add_efi_sz(1), - 'set', '1', 'boot', 'on', - 'mkpart', 'primary', 'linux-swap', - add_efi_sz(1), add_efi_sz(513), 'mkpart', - 'primary', '', add_efi_sz(513), - add_efi_sz(1537)] + expected_mkpart = [('mkpart', 'primary', 'fat32', '1', + add_efi_sz(1), 'set', '1', 'boot', 'on'), + ('mkpart', 'primary', 'linux-swap', + add_efi_sz(1), add_efi_sz(513)), + ('mkpart', 'primary', '', add_efi_sz(513), + add_efi_sz(1537))] else: if boot_option == "local": if disk_label == "gpt": add_bios_sz = lambda x: str(_s(x, self.bios_size)) - expected_mkpart = ['mkpart', 'primary', '', '1', + expected_mkpart = [('mkpart', 'primary', '', '1', add_bios_sz(1), - 'set', '1', 'bios_grub', 'on', - 'mkpart', 'primary', 'linux-swap', - add_bios_sz(1), add_bios_sz(513), - 'mkpart', 'primary', '', - add_bios_sz(513), add_bios_sz(1537)] + 'set', '1', 'bios_grub', 'on'), + ('mkpart', 'primary', 'linux-swap', + add_bios_sz(1), add_bios_sz(513)), + ('mkpart', 'primary', '', + add_bios_sz(513), add_bios_sz(1537))] else: - expected_mkpart = ['mkpart', 'primary', 'linux-swap', '1', - '513', 'mkpart', 'primary', '', '513', - '1537', 'set', '2', 'boot', 'on'] + expected_mkpart = [('mkpart', 'primary', 'linux-swap', '1', + '513'), + ('mkpart', 'primary', '', '513', + '1537', 'set', '2', 'boot', 'on')] else: - expected_mkpart = ['mkpart', 'primary', 'linux-swap', '1', - '513', 'mkpart', 'primary', '', '513', - '1537'] + expected_mkpart = [('mkpart', 'primary', 'linux-swap', '1', + '513'), ('mkpart', 'primary', '', '513', + '1537')] self.dev = 'fake-dev' - parted_cmd = (self._get_parted_cmd(self.dev, disk_label) + - expected_mkpart) - parted_call = mock.call(*parted_cmd, use_standard_locale=True, - run_as_root=True, check_exit_code=[0]) fuser_cmd = ['fuser', 'fake-dev'] fuser_call = mock.call(*fuser_cmd, run_as_root=True, check_exit_code=[0, 1]) - mock_exc.assert_has_calls([parted_call, fuser_call]) + + calls = self._get_calls(expected_mkpart, label=disk_label) + + calls.append(fuser_call) + + mock_exc.assert_has_calls(calls) def test_make_partitions(self, mock_exc): self._test_make_partitions(mock_exc, boot_option="netboot") @@ -382,25 +403,24 @@ class MakePartitionsTestCase(test_base.BaseTestCase): def test_make_partitions_with_ephemeral(self, mock_exc): self.ephemeral_mb = 2048 - expected_mkpart = ['mkpart', 'primary', '', '1', '2049', - 'mkpart', 'primary', 'linux-swap', '2049', '2561', - 'mkpart', 'primary', '', '2561', '3585'] + expected_mkpart = [('mkpart', 'primary', '', '1', '2049'), + ('mkpart', 'primary', 'linux-swap', '2049', '2561'), + ('mkpart', 'primary', '', '2561', '3585')] self.dev = 'fake-dev' - cmd = self._get_parted_cmd(self.dev) + expected_mkpart mock_exc.return_value = (None, None) disk_utils.make_partitions(self.dev, self.root_mb, self.swap_mb, self.ephemeral_mb, self.configdrive_mb, self.node_uuid) - parted_call = mock.call(*cmd, use_standard_locale=True, - run_as_root=True, check_exit_code=[0]) - mock_exc.assert_has_calls([parted_call]) + calls = self._get_calls(expected_mkpart) + + mock_exc.assert_has_calls(calls) def test_make_partitions_with_iscsi_device(self, mock_exc): self.ephemeral_mb = 2048 - expected_mkpart = ['mkpart', 'primary', '', '1', '2049', - 'mkpart', 'primary', 'linux-swap', '2049', '2561', - 'mkpart', 'primary', '', '2561', '3585'] + expected_mkpart = [('mkpart', 'primary', '', '1', '2049'), + ('mkpart', 'primary', 'linux-swap', '2049', '2561'), + ('mkpart', 'primary', '', '2561', '3585')] self.dev = '/dev/iqn.2008-10.org.openstack:%s.fake-9' % self.node_uuid ep = '/dev/iqn.2008-10.org.openstack:%s.fake-9-part1' % self.node_uuid swap = ('/dev/iqn.2008-10.org.openstack:%s.fake-9-part2' @@ -410,35 +430,36 @@ class MakePartitionsTestCase(test_base.BaseTestCase): expected_result = {'ephemeral': ep, 'swap': swap, 'root': root} - cmd = self._get_parted_cmd(self.dev) + expected_mkpart + mock_exc.return_value = (None, None) result = disk_utils.make_partitions( self.dev, self.root_mb, self.swap_mb, self.ephemeral_mb, self.configdrive_mb, self.node_uuid) - parted_call = mock.call(*cmd, use_standard_locale=True, - run_as_root=True, check_exit_code=[0]) - mock_exc.assert_has_calls([parted_call]) + calls = self._get_calls(expected_mkpart) + + mock_exc.assert_has_calls(calls) + self.assertEqual(expected_result, result) def test_make_partitions_with_local_device(self, mock_exc): self.ephemeral_mb = 2048 - expected_mkpart = ['mkpart', 'primary', '', '1', '2049', - 'mkpart', 'primary', 'linux-swap', '2049', '2561', - 'mkpart', 'primary', '', '2561', '3585'] + expected_mkpart = [('mkpart', 'primary', '', '1', '2049'), + ('mkpart', 'primary', 'linux-swap', '2049', '2561'), + ('mkpart', 'primary', '', '2561', '3585')] self.dev = 'fake-dev' expected_result = {'ephemeral': 'fake-dev1', 'swap': 'fake-dev2', 'root': 'fake-dev3'} - cmd = self._get_parted_cmd(self.dev) + expected_mkpart mock_exc.return_value = (None, None) result = disk_utils.make_partitions( self.dev, self.root_mb, self.swap_mb, self.ephemeral_mb, self.configdrive_mb, self.node_uuid) - parted_call = mock.call(*cmd, use_standard_locale=True, - run_as_root=True, check_exit_code=[0]) - mock_exc.assert_has_calls([parted_call]) + calls = self._get_calls(expected_mkpart) + + mock_exc.assert_has_calls(calls) + self.assertEqual(expected_result, result) -- 2.14.3