From 316dd3f952bd5056e0ba85b7be26e603f547defc Mon Sep 17 00:00:00 2001 From: "raphael.glon" Date: Fri, 20 Sep 2019 15:14:40 +0000 Subject: [PATCH] Revert "Software raid: mbr/gpt partition table alternative" This reverts commit 258d963e406c512bb90295c700ee22e4609abcd0. Remove the mbr/gpt choice from softraid features for now, as it cannot be directly used without additional commits, to be pushed soon. Furthermore, it could even lead to instance spawn misconfiguration, if the disk label specified in instance_info cannot fit with the boot mode + partitioning layout (example: you build softraid over gpt, then you lose the mbr gap. Thus you need an additional bios boot partition, in BIOS boot mode, or an esp, in UEFI boot mode, outside the raid, for grub to install itself inthere, to be able to assemble raid and find root device correctly). Change-Id: I3a0a704ea99a40eb3fc8e879270dfbd356951488 --- ironic_python_agent/hardware.py | 18 +----- .../tests/unit/test_hardware.py | 63 +------------------ .../notes/softraid-msdos-gpt-alternative.yaml | 4 -- 3 files changed, 6 insertions(+), 79 deletions(-) delete mode 100644 releasenotes/notes/softraid-msdos-gpt-alternative.yaml diff --git a/ironic_python_agent/hardware.py b/ironic_python_agent/hardware.py index fbba74e60..3fa136bdf 100644 --- a/ironic_python_agent/hardware.py +++ b/ironic_python_agent/hardware.py @@ -1443,19 +1443,6 @@ class GenericHardwareManager(HardwareManager): LOG.debug("No target_raid_config found") return {} - 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. @@ -1488,14 +1475,15 @@ class GenericHardwareManager(HardwareManager): partitions) raise errors.SoftwareRAIDError(msg) - # Create an MBR or GPT partition table on each disk. parted_start_dict = {} + # Create an MBR partition table on each disk. + # TODO(arne_wiebalck): Check if GPT would work as well. 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', partition_table_type) + 'mklabel', 'msdos') 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 dcb890dcd..62f065a3a 100644 --- a/ironic_python_agent/tests/unit/test_hardware.py +++ b/ironic_python_agent/tests/unit/test_hardware.py @@ -2710,29 +2710,7 @@ 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 - + node = self.node raid_config = { "logical_disks": [ { @@ -2769,10 +2747,10 @@ class TestGenericHardwareManager(base.IronicAgentTest): mocked_execute.assert_has_calls([ mock.call('parted', '/dev/sda', '-s', '--', 'mklabel', - expected_partition_table_type), + 'msdos'), mock.call('sgdisk', '-F', '/dev/sda'), mock.call('parted', '/dev/sdb', '-s', '--', 'mklabel', - expected_partition_table_type), + 'msdos'), mock.call('sgdisk', '-F', '/dev/sdb'), mock.call('parted', '/dev/sda', '-s', '-a', 'optimal', '--', 'mkpart', 'primary', '42s', '10GiB'), @@ -3031,41 +3009,6 @@ 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' - } - } - } - # pass a sensible target_raid_config - raid_config = { - "logical_disks": [ - { - "size_gb": "100", - "raid_level": "1", - "controller": "software", - }, - { - "size_gb": "MAX", - "raid_level": "0", - "controller": "software", - }, - ] - } - node['target_raid_config'] = raid_config - - error_regex = \ - "Invalid disk_label capability. Should either be 'msdos' or 'gpt'" - self.assertRaisesRegex(errors.SoftwareRAIDError, error_regex, - self.hardware.create_configuration, - node, []) - def test_create_configuration_empty_target_raid_config(self): self.node['target_raid_config'] = {} result = self.hardware.create_configuration(self.node, []) diff --git a/releasenotes/notes/softraid-msdos-gpt-alternative.yaml b/releasenotes/notes/softraid-msdos-gpt-alternative.yaml deleted file mode 100644 index 2a1eb83e2..000000000 --- a/releasenotes/notes/softraid-msdos-gpt-alternative.yaml +++ /dev/null @@ -1,4 +0,0 @@ ---- -features: - - Adds the ability to specify the partition table type when creating RAID. - When not specified, the type is set to ``msdos`` (MBR).