From 258d963e406c512bb90295c700ee22e4609abcd0 Mon Sep 17 00:00:00 2001 From: Raphael Glon Date: Tue, 6 Aug 2019 13:52:13 +0200 Subject: [PATCH] Software raid: mbr/gpt partition table alternative GPT/MBR partition table type alternative on softraid creation. Change-Id: Ib3d00034fe687328a1ea3e038f6f86d2746e63a0 --- ironic_python_agent/hardware.py | 18 +++++-- .../tests/unit/test_hardware.py | 51 +++++++++++++++++-- .../notes/softraid-msdos-gpt-alternative.yaml | 4 ++ 3 files changed, 66 insertions(+), 7 deletions(-) create mode 100644 releasenotes/notes/softraid-msdos-gpt-alternative.yaml diff --git a/ironic_python_agent/hardware.py b/ironic_python_agent/hardware.py index 7c0636be7..37e36f7cd 100644 --- a/ironic_python_agent/hardware.py +++ b/ironic_python_agent/hardware.py @@ -1400,6 +1400,19 @@ class GenericHardwareManager(HardwareManager): raid_config = node.get('target_raid_config', {}) + partition_table_type = 'msdos' + + # If explicitely specified by caller let's follow orders + instance_info = node.get('instance_info', {}) + capabilities = instance_info.get('capabilities', {}) + specified_table_type = capabilities.get('disk_label') + if specified_table_type: + if specified_table_type not in ['msdos', 'gpt']: + msg = ("Invalid disk_label capability. " + "Should either be 'msdos' or 'gpt'") + raise errors.SoftwareRAIDError(msg) + partition_table_type = specified_table_type + # No 'software' controller: do nothing. If 'controller' is # set to 'software' on only one of the drives, the validation # code will catch it. @@ -1432,14 +1445,13 @@ class GenericHardwareManager(HardwareManager): partitions) raise errors.SoftwareRAIDError(msg) - # Create an MBR partition table on each disk. - # TODO(arne_wiebalck): Check if GPT would work as well. + # Create an MBR or GPT partition table on each disk. for block_device in block_devices: LOG.info("Creating partition table on {}".format( block_device.name)) try: utils.execute('parted', block_device.name, '-s', '--', - 'mklabel', 'msdos') + 'mklabel', partition_table_type) except processutils.ProcessExecutionError as e: msg = "Failed to create partition table on {}: {}".format( block_device.name, e) diff --git a/ironic_python_agent/tests/unit/test_hardware.py b/ironic_python_agent/tests/unit/test_hardware.py index 2822962cd..a84969349 100644 --- a/ironic_python_agent/tests/unit/test_hardware.py +++ b/ironic_python_agent/tests/unit/test_hardware.py @@ -2568,6 +2568,29 @@ class TestGenericHardwareManager(base.IronicAgentTest): @mock.patch.object(utils, 'execute', autospec=True) def test_create_configuration(self, mocked_execute): + self._test_create_configuration(mocked_execute, 'msdos') + + @mock.patch.object(utils, 'execute', autospec=True) + def test_create_configuration_disk_label_specified( + self, mocked_execute): + + # Override gpt default choice with msdos + node = { + 'uuid': 'hello', + 'instance_info': { + 'capabilities': { + 'disk_label': 'gpt' + } + } + } + self._test_create_configuration(mocked_execute, 'msdos') + self._test_create_configuration(mocked_execute, 'gpt', node) + + def _test_create_configuration(self, mocked_execute, + expected_partition_table_type, node=None): + if node is None: + node = self.node + raid_config = { "logical_disks": [ { @@ -2582,17 +2605,19 @@ class TestGenericHardwareManager(base.IronicAgentTest): }, ] } - self.node['target_raid_config'] = raid_config + node['target_raid_config'] = raid_config device1 = hardware.BlockDevice('/dev/sda', 'sda', 1073741824, True) device2 = hardware.BlockDevice('/dev/sdb', 'sdb', 1073741824, True) self.hardware.list_block_devices = mock.Mock() self.hardware.list_block_devices.return_value = [device1, device2] - result = self.hardware.create_configuration(self.node, []) + result = self.hardware.create_configuration(node, []) mocked_execute.assert_has_calls([ - mock.call('parted', '/dev/sda', '-s', '--', 'mklabel', 'msdos'), - mock.call('parted', '/dev/sdb', '-s', '--', 'mklabel', 'msdos'), + mock.call('parted', '/dev/sda', '-s', '--', 'mklabel', + expected_partition_table_type), + mock.call('parted', '/dev/sdb', '-s', '--', 'mklabel', + expected_partition_table_type), mock.call('parted', '/dev/sda', '-s', '-a', 'optimal', '--', 'mkpart', 'primary', '2048s', 102400), mock.call('partx', '-u', '/dev/sda', check_exit_code=False), @@ -2718,6 +2743,24 @@ class TestGenericHardwareManager(base.IronicAgentTest): self.hardware.create_configuration, self.node, []) + @mock.patch.object(utils, 'execute', autospec=True) + def test_create_configuration_bad_disk_label(self, mocked_execute): + + # Override gpt default choice with msdos + node = { + 'uuid': 'hello', + 'instance_info': { + 'capabilities': { + 'disk_label': 'invalid' + } + } + } + error_regex = \ + "Invalid disk_label capability. Should either be 'msdos' or 'gpt'" + self.assertRaisesRegex(errors.SoftwareRAIDError, error_regex, + self.hardware.create_configuration, + node, []) + @mock.patch.object(utils, 'execute', autospec=True) def test_create_configuration_with_nvme(self, mocked_execute): raid_config = { diff --git a/releasenotes/notes/softraid-msdos-gpt-alternative.yaml b/releasenotes/notes/softraid-msdos-gpt-alternative.yaml new file mode 100644 index 000000000..be302d65a --- /dev/null +++ b/releasenotes/notes/softraid-msdos-gpt-alternative.yaml @@ -0,0 +1,4 @@ +--- +features: + - Adds the ability to specify the partition table type when creating raid. + When not specified, the type is set to msdos.