From aeed2df155105384bda377d2dba32726b87b026a Mon Sep 17 00:00:00 2001 From: Arne Wiebalck Date: Mon, 24 Jan 2022 15:00:05 +0100 Subject: SoftwareRAID: Use efibootmgr (and drop grub2-install) Move the software RAID code path from grub2-install to efibootmgr: - remove the UEFI efibootmgr exception for software RAID - create and populate the ESPs on the holder disks - update the NVRAM with all ESPs (the component devices of the ESP mirror, use unique labels to avoid unintentional deduplication of entries in the NVRAM) In addition, use canonical device names for the RAID device for the ESP: it seems like tinyIPA silently replaces /dev/md/esp with /dev/md127. Find the next free /dev/md device and use it instead. Also rescan the resulting device before copying files. Co-authored-by: Dmitry Tantsur Story: #2009794 Change-Id: I7ed34e595215194a589c2f1cd0b39ff0336da8f1 (cherry picked from commit 62c5674a600baeeef0af3b12baeab486870eb103) --- ironic_python_agent/extensions/image.py | 88 +++++++++--- ironic_python_agent/hardware.py | 6 +- ironic_python_agent/raid_utils.py | 14 ++ .../tests/unit/extensions/test_image.py | 155 ++++++++++++++++----- ironic_python_agent/tests/unit/test_hardware.py | 14 +- ironic_python_agent/tests/unit/test_raid_utils.py | 23 +++ ...move_swraid_to_efibootmgr-d87c1bfde1661fb5.yaml | 7 + 7 files changed, 243 insertions(+), 64 deletions(-) create mode 100644 releasenotes/notes/move_swraid_to_efibootmgr-d87c1bfde1661fb5.yaml diff --git a/ironic_python_agent/extensions/image.py b/ironic_python_agent/extensions/image.py index 576163fd..0ec6de9d 100644 --- a/ironic_python_agent/extensions/image.py +++ b/ironic_python_agent/extensions/image.py @@ -20,6 +20,7 @@ import shutil import stat import tempfile +from ironic_lib import disk_utils from ironic_lib import utils as ilib_utils from oslo_concurrency import processutils from oslo_config import cfg @@ -261,7 +262,7 @@ def _get_efi_bootloaders(location): def _run_efibootmgr(valid_efi_bootloaders, device, efi_partition, - mount_point): + mount_point, label_suffix=None): """Executes efibootmgr and removes duplicate entries. :param valid_efi_bootloaders: the list of valid efi bootloaders @@ -270,6 +271,9 @@ def _run_efibootmgr(valid_efi_bootloaders, device, efi_partition, :param mount_point: The mountpoint for the EFI partition so we can read contents of files if necessary to perform proper bootloader injection operations. + :param label_suffix: a string to be appended to the EFI label, + mainly used in the case of software to uniqify + the entries for the md components. """ # Before updating let's get information about the bootorder @@ -292,9 +296,13 @@ def _run_efibootmgr(valid_efi_bootloaders, device, efi_partition, v_efi_bl_path = v_bl.replace(csv_filename, str(csv_contents[0])) v_efi_bl_path = '\\' + v_efi_bl_path.replace('/', '\\') label = csv_contents[1] + if label_suffix: + label = label + " " + str(label_suffix) else: v_efi_bl_path = '\\' + v_bl.replace('/', '\\') label = 'ironic' + str(label_id) + if label_suffix: + label = label + " " + str(label_suffix) # Iterate through standard out, and look for duplicates for line in original_efi_output[0].split('\n'): @@ -307,9 +315,11 @@ def _run_efibootmgr(valid_efi_bootloaders, device, efi_partition, LOG.debug("Found bootnum %s matching label", boot_num) utils.execute('efibootmgr', '-b', boot_num, '-B') - LOG.debug("Adding loader %(path)s on partition %(part)s of device " - " %(dev)s", {'path': v_efi_bl_path, 'part': efi_partition, - 'dev': device}) + LOG.info("Adding loader %(path)s on partition %(part)s of device " + " %(dev)s with label %(label)s", + {'path': v_efi_bl_path, 'part': efi_partition, + 'dev': device, 'label': label}) + # Update the nvram using efibootmgr # https://linux.die.net/man/8/efibootmgr utils.execute('efibootmgr', '-v', '-c', '-d', device, @@ -380,16 +390,60 @@ def _manage_uefi(device, efi_system_part_uuid=None): efi_mounted = True valid_efi_bootloaders = _get_efi_bootloaders(efi_partition_mount_point) - if valid_efi_bootloaders: - _run_efibootmgr(valid_efi_bootloaders, device, efi_partition, - efi_partition_mount_point) - return True - else: + if not valid_efi_bootloaders: # NOTE(dtantsur): if we have an empty EFI partition, try to use # grub-install to populate it. LOG.warning('Empty EFI partition detected.') return False + if not hardware.is_md_device(device): + efi_devices = [device] + efi_partition_numbers = [efi_partition] + efi_label_suffix = '' + else: + # umount to allow for signature removal (to avoid confusion about + # which ESP to mount once the instance is deployed) + utils.execute('umount', efi_partition_mount_point, attempts=3, + delay_on_retry=True) + efi_mounted = False + + holders = hardware.get_holder_disks(device) + efi_md_device = prepare_boot_partitions_for_softraid( + device, holders, efi_device_part, target_boot_mode='uefi' + ) + efi_devices = hardware.get_component_devices(efi_md_device) + efi_partition_numbers = [] + _PARTITION_NUMBER = re.compile(r'(\d+)$') + for dev in efi_devices: + match = _PARTITION_NUMBER.search(dev) + if match: + partition_number = match.group(1) + efi_partition_numbers.append(partition_number) + else: + raise errors.DeviceNotFound( + "Could not extract the partition number " + "from %s!" % dev) + efi_label_suffix = "(RAID, part%s)" + + # remount for _run_efibootmgr + utils.execute('mount', efi_device_part, efi_partition_mount_point) + efi_mounted = True + + efi_dev_part = zip(efi_devices, efi_partition_numbers) + for i, (efi_dev, efi_part) in enumerate(efi_dev_part): + LOG.debug("Calling efibootmgr with dev %s part %s", + efi_dev, efi_part) + if efi_label_suffix: + # NOTE (arne_wiebalck): uniqify the labels to prevent + # unintentional boot entry cleanup + _run_efibootmgr(valid_efi_bootloaders, efi_dev, efi_part, + efi_partition_mount_point, + efi_label_suffix % i) + else: + _run_efibootmgr(valid_efi_bootloaders, efi_dev, efi_part, + efi_partition_mount_point) + return True + except processutils.ProcessExecutionError as e: error_msg = ('Could not verify uefi on device %(dev)s' 'failed with %(err)s.' % {'dev': device, 'err': e}) @@ -422,8 +476,8 @@ def _manage_uefi(device, efi_system_part_uuid=None): # TODO(rg): handle PreP boot parts relocation as well -def _prepare_boot_partitions_for_softraid(device, holders, efi_part, - target_boot_mode): +def prepare_boot_partitions_for_softraid(device, holders, efi_part, + target_boot_mode): """Prepare boot partitions when relevant. Create either a RAIDed EFI partition or bios boot partitions for software @@ -492,15 +546,17 @@ def _prepare_boot_partitions_for_softraid(device, holders, efi_part, target_part, holder) # RAID the ESPs, metadata=1.0 is mandatory to be able to boot - md_device = '/dev/md/esp' + md_device = raid_utils.get_next_free_raid_device() LOG.debug("Creating md device %(md_device)s for the ESPs " "on %(efi_partitions)s", {'md_device': md_device, 'efi_partitions': efi_partitions}) utils.execute('mdadm', '--create', md_device, '--force', '--run', '--metadata=1.0', '--level', '1', - '--raid-devices', len(efi_partitions), + '--name', 'esp', '--raid-devices', len(efi_partitions), *efi_partitions) + disk_utils.trigger_device_rescan(md_device) + if efi_part: # Blockdev copy the source ESP and erase it LOG.debug("Relocating EFI %s to %s", efi_part, md_device) @@ -627,7 +683,7 @@ def _install_grub2(device, root_uuid, efi_system_part_uuid=None, efi_partition = efi_part if hardware.is_md_device(device): holders = hardware.get_holder_disks(device) - efi_partition = _prepare_boot_partitions_for_softraid( + efi_partition = prepare_boot_partitions_for_softraid( device, holders, efi_part, target_boot_mode ) @@ -1004,9 +1060,7 @@ def _efi_boot_setup(device, efi_system_part_uuid=None, target_boot_mode=None): {'target': target_boot_mode, 'current': boot.current_boot_mode}) - # FIXME(arne_wiebalck): make software RAID work with efibootmgr - if (boot.current_boot_mode == 'uefi' - and not hardware.is_md_device(device)): + if boot.current_boot_mode == 'uefi': try: utils.execute('efibootmgr', '--version') except FileNotFoundError: diff --git a/ironic_python_agent/hardware.py b/ironic_python_agent/hardware.py index 34173582..352704a0 100644 --- a/ironic_python_agent/hardware.py +++ b/ironic_python_agent/hardware.py @@ -181,7 +181,7 @@ def _get_md_uuid(raid_device): return match.group(1) -def _get_component_devices(raid_device): +def get_component_devices(raid_device): """Get the component devices of a Software RAID device. Get the UUID of the md device and scan all other devices @@ -325,7 +325,7 @@ def md_restart(raid_device): """ try: LOG.debug('Restarting software RAID device %s', raid_device) - component_devices = _get_component_devices(raid_device) + component_devices = get_component_devices(raid_device) utils.execute('mdadm', '--stop', raid_device) utils.execute('mdadm', '--assemble', raid_device, *component_devices) @@ -2235,7 +2235,7 @@ class GenericHardwareManager(HardwareManager): def _delete_config_pass(self, raid_devices): all_holder_disks = [] for raid_device in raid_devices: - component_devices = _get_component_devices(raid_device.name) + component_devices = get_component_devices(raid_device.name) if not component_devices: # A "Software RAID device" without components is usually # a partition on an md device (as, for instance, created diff --git a/ironic_python_agent/raid_utils.py b/ironic_python_agent/raid_utils.py index 962a6e9a..d4c338a8 100644 --- a/ironic_python_agent/raid_utils.py +++ b/ironic_python_agent/raid_utils.py @@ -248,3 +248,17 @@ def create_raid_device(index, logical_disk): msg = "Failed re-add {} to {}: {}".format( dev, md_device, e) raise errors.SoftwareRAIDError(msg) + + +def get_next_free_raid_device(): + """Get a device name that is still free.""" + from ironic_python_agent import hardware + + names = {dev.name for dev in + hardware.dispatch_to_managers('list_block_devices')} + for idx in range(128): + name = f'/dev/md{idx}' + if name not in names: + return name + + raise errors.SoftwareRAIDError("No free md (RAID) devices are left") diff --git a/ironic_python_agent/tests/unit/extensions/test_image.py b/ironic_python_agent/tests/unit/extensions/test_image.py index 3cc32d65..b36110b2 100644 --- a/ironic_python_agent/tests/unit/extensions/test_image.py +++ b/ironic_python_agent/tests/unit/extensions/test_image.py @@ -18,12 +18,14 @@ import shutil import tempfile from unittest import mock +from ironic_lib import disk_utils from ironic_lib import utils as ilib_utils from oslo_concurrency import processutils from ironic_python_agent import errors from ironic_python_agent.extensions import image from ironic_python_agent import hardware +from ironic_python_agent import raid_utils from ironic_python_agent.tests.unit import base from ironic_python_agent import utils @@ -1690,9 +1692,13 @@ Boot0004* ironic1 HD(1,GPT,55db8d03-c8f6-4a5b-9155-790dddc348fa,0x800,0x640 uuid=self.fake_root_uuid) self.assertFalse(mock_dispatch.called) + @mock.patch.object(disk_utils, 'trigger_device_rescan', autospec=True) + @mock.patch.object(raid_utils, 'get_next_free_raid_device', autospec=True, + return_value='/dev/md42') @mock.patch.object(utils, 'get_efi_part_on_device', autospec=True) - def test__prepare_boot_partitions_for_softraid_uefi_gpt( - self, mock_efi_part, mock_execute, mock_dispatch): + def test_prepare_boot_partitions_for_softraid_uefi_gpt( + self, mock_efi_part, mock_free_raid_device, mock_rescan, + mock_execute, mock_dispatch): mock_efi_part.return_value = '12' mock_execute.side_effect = [ ('451', None), # sgdisk -F @@ -1710,7 +1716,7 @@ Boot0004* ironic1 HD(1,GPT,55db8d03-c8f6-4a5b-9155-790dddc348fa,0x800,0x640 (None, None), # wipefs ] - efi_part = image._prepare_boot_partitions_for_softraid( + efi_part = image.prepare_boot_partitions_for_softraid( '/dev/md0', ['/dev/sda', '/dev/sdb'], None, target_boot_mode='uefi') @@ -1730,19 +1736,24 @@ Boot0004* ironic1 HD(1,GPT,55db8d03-c8f6-4a5b-9155-790dddc348fa,0x800,0x640 mock.call('blkid'), mock.call('blkid', '-l', '-t', 'PARTLABEL=uefi-holder-1', '/dev/sdb'), - mock.call('mdadm', '--create', '/dev/md/esp', '--force', '--run', - '--metadata=1.0', '--level', '1', '--raid-devices', 2, - '/dev/sda12', '/dev/sdb14'), - mock.call('cp', '/dev/md0p12', '/dev/md/esp'), + mock.call('mdadm', '--create', '/dev/md42', '--force', '--run', + '--metadata=1.0', '--level', '1', '--name', 'esp', + '--raid-devices', 2, '/dev/sda12', '/dev/sdb14'), + mock.call('cp', '/dev/md0p12', '/dev/md42'), mock.call('wipefs', '-a', '/dev/md0p12') ] mock_execute.assert_has_calls(expected, any_order=False) - self.assertEqual(efi_part, '/dev/md/esp') + self.assertEqual(efi_part, '/dev/md42') + mock_rescan.assert_called_once_with('/dev/md42') + @mock.patch.object(disk_utils, 'trigger_device_rescan', autospec=True) + @mock.patch.object(raid_utils, 'get_next_free_raid_device', autospec=True, + return_value='/dev/md42') @mock.patch.object(utils, 'get_efi_part_on_device', autospec=True) @mock.patch.object(ilib_utils, 'mkfs', autospec=True) - def test__prepare_boot_partitions_for_softraid_uefi_gpt_esp_not_found( - self, mock_mkfs, mock_efi_part, mock_execute, mock_dispatch): + def test_prepare_boot_partitions_for_softraid_uefi_gpt_esp_not_found( + self, mock_mkfs, mock_efi_part, mock_free_raid_device, + mock_rescan, mock_execute, mock_dispatch): mock_efi_part.return_value = None mock_execute.side_effect = [ ('451', None), # sgdisk -F @@ -1758,7 +1769,7 @@ Boot0004* ironic1 HD(1,GPT,55db8d03-c8f6-4a5b-9155-790dddc348fa,0x800,0x640 (None, None), # mdadm ] - efi_part = image._prepare_boot_partitions_for_softraid( + efi_part = image.prepare_boot_partitions_for_softraid( '/dev/md0', ['/dev/sda', '/dev/sdb'], None, target_boot_mode='uefi') @@ -1781,12 +1792,17 @@ Boot0004* ironic1 HD(1,GPT,55db8d03-c8f6-4a5b-9155-790dddc348fa,0x800,0x640 ] mock_execute.assert_has_calls(expected, any_order=False) mock_mkfs.assert_has_calls([ - mock.call(path='/dev/md/esp', label='efi-part', fs='vfat'), + mock.call(path='/dev/md42', label='efi-part', fs='vfat'), ], any_order=False) - self.assertEqual(efi_part, '/dev/md/esp') - - def test__prepare_boot_partitions_for_softraid_uefi_gpt_efi_provided( - self, mock_execute, mock_dispatch): + self.assertEqual(efi_part, '/dev/md42') + mock_rescan.assert_called_once_with('/dev/md42') + + @mock.patch.object(disk_utils, 'trigger_device_rescan', autospec=True) + @mock.patch.object(raid_utils, 'get_next_free_raid_device', autospec=True, + return_value='/dev/md42') + def test_prepare_boot_partitions_for_softraid_uefi_gpt_efi_provided( + self, mock_free_raid_device, mock_rescan, + mock_execute, mock_dispatch): mock_execute.side_effect = [ ('451', None), # sgdisk -F (None, None), # sgdisk create part @@ -1803,7 +1819,7 @@ Boot0004* ironic1 HD(1,GPT,55db8d03-c8f6-4a5b-9155-790dddc348fa,0x800,0x640 (None, None), # wipefs ] - efi_part = image._prepare_boot_partitions_for_softraid( + efi_part = image.prepare_boot_partitions_for_softraid( '/dev/md0', ['/dev/sda', '/dev/sdb'], '/dev/md0p15', target_boot_mode='uefi') @@ -1822,21 +1838,21 @@ Boot0004* ironic1 HD(1,GPT,55db8d03-c8f6-4a5b-9155-790dddc348fa,0x800,0x640 mock.call('blkid'), mock.call('blkid', '-l', '-t', 'PARTLABEL=uefi-holder-1', '/dev/sdb'), - mock.call('mdadm', '--create', '/dev/md/esp', '--force', '--run', - '--metadata=1.0', '--level', '1', '--raid-devices', 2, - '/dev/sda12', '/dev/sdb14'), - mock.call('cp', '/dev/md0p15', '/dev/md/esp'), + mock.call('mdadm', '--create', '/dev/md42', '--force', '--run', + '--metadata=1.0', '--level', '1', '--name', 'esp', + '--raid-devices', 2, '/dev/sda12', '/dev/sdb14'), + mock.call('cp', '/dev/md0p15', '/dev/md42'), mock.call('wipefs', '-a', '/dev/md0p15') ] mock_execute.assert_has_calls(expected, any_order=False) - self.assertEqual(efi_part, '/dev/md/esp') + self.assertEqual(efi_part, '/dev/md42') @mock.patch.object(utils, 'scan_partition_table_type', autospec=True, return_value='msdos') - def test__prepare_boot_partitions_for_softraid_bios_msdos( + def test_prepare_boot_partitions_for_softraid_bios_msdos( self, mock_label_scan, mock_execute, mock_dispatch): - efi_part = image._prepare_boot_partitions_for_softraid( + efi_part = image.prepare_boot_partitions_for_softraid( '/dev/md0', ['/dev/sda', '/dev/sdb'], 'notusedanyway', target_boot_mode='bios') @@ -1849,7 +1865,7 @@ Boot0004* ironic1 HD(1,GPT,55db8d03-c8f6-4a5b-9155-790dddc348fa,0x800,0x640 @mock.patch.object(utils, 'scan_partition_table_type', autospec=True, return_value='gpt') - def test__prepare_boot_partitions_for_softraid_bios_gpt( + def test_prepare_boot_partitions_for_softraid_bios_gpt( self, mock_label_scan, mock_execute, mock_dispatch): mock_execute.side_effect = [ @@ -1859,7 +1875,7 @@ Boot0004* ironic1 HD(1,GPT,55db8d03-c8f6-4a5b-9155-790dddc348fa,0x800,0x640 (None, None), # bios boot grub ] - efi_part = image._prepare_boot_partitions_for_softraid( + efi_part = image.prepare_boot_partitions_for_softraid( '/dev/md0', ['/dev/sda', '/dev/sdb'], 'notusedanyway', target_boot_mode='bios') @@ -1891,7 +1907,7 @@ Boot0004* ironic1 HD(1,GPT,55db8d03-c8f6-4a5b-9155-790dddc348fa,0x800,0x640 @mock.patch.object(os, 'environ', autospec=True) @mock.patch.object(os, 'makedirs', autospec=True) @mock.patch.object(image, '_get_partition', autospec=True) - @mock.patch.object(image, '_prepare_boot_partitions_for_softraid', + @mock.patch.object(image, 'prepare_boot_partitions_for_softraid', autospec=True, return_value='/dev/md/esp') @mock.patch.object(image, '_has_dracut', @@ -2009,7 +2025,7 @@ Boot0004* ironic1 HD(1,GPT,55db8d03-c8f6-4a5b-9155-790dddc348fa,0x800,0x640 @mock.patch.object(os, 'environ', autospec=True) @mock.patch.object(os, 'makedirs', autospec=True) @mock.patch.object(image, '_get_partition', autospec=True) - @mock.patch.object(image, '_prepare_boot_partitions_for_softraid', + @mock.patch.object(image, 'prepare_boot_partitions_for_softraid', autospec=True, return_value=[]) @mock.patch.object(image, '_has_dracut', @@ -2308,16 +2324,17 @@ Boot0004* ironic1 HD(1,GPT,55db8d03-c8f6-4a5b-9155-790dddc348fa,0x800,0x640 self.assertFalse(result) @mock.patch.object(os.path, 'exists', lambda *_: False) + @mock.patch.object(hardware, 'is_md_device', autospec=True) @mock.patch.object(image, '_get_efi_bootloaders', autospec=True) @mock.patch.object(image, '_get_partition', autospec=True) @mock.patch.object(utils, 'get_efi_part_on_device', autospec=True) @mock.patch.object(os, 'makedirs', autospec=True) def test__manage_uefi(self, mkdir_mock, mock_utils_efi_part, - mock_get_part_uuid, mock_efi_bl, mock_execute, - mock_dispatch): + mock_get_part_uuid, mock_efi_bl, mock_is_md_device, + mock_execute, mock_dispatch): mock_utils_efi_part.return_value = '1' mock_get_part_uuid.return_value = self.fake_dev - + mock_is_md_device.return_value = False mock_efi_bl.return_value = ['EFI/BOOT/BOOTX64.EFI'] mock_execute.side_effect = iter([('', ''), ('', ''), @@ -2347,15 +2364,18 @@ Boot0004* ironic1 HD(1,GPT,55db8d03-c8f6-4a5b-9155-790dddc348fa,0x800,0x640 self.assertEqual(7, mock_execute.call_count) @mock.patch.object(os.path, 'exists', lambda *_: False) + @mock.patch.object(hardware, 'is_md_device', autospec=True) @mock.patch.object(image, '_get_efi_bootloaders', autospec=True) @mock.patch.object(image, '_get_partition', autospec=True) @mock.patch.object(utils, 'get_efi_part_on_device', autospec=True) @mock.patch.object(os, 'makedirs', autospec=True) def test__manage_uefi_found_csv(self, mkdir_mock, mock_utils_efi_part, mock_get_part_uuid, mock_efi_bl, - mock_execute, mock_dispatch): + mock_is_md_device, mock_execute, + mock_dispatch): mock_utils_efi_part.return_value = '1' mock_get_part_uuid.return_value = self.fake_dev + mock_is_md_device.return_value = False mock_efi_bl.return_value = ['EFI/vendor/BOOTX64.CSV'] # Format is ,,,humanfriendlytextnotused @@ -2405,16 +2425,18 @@ Boot0002: VENDMAGIC FvFile(9f3c6294-bf9b-4208-9808-be45dfc34b51) self.assertEqual(9, mock_execute.call_count) @mock.patch.object(os.path, 'exists', lambda *_: False) + @mock.patch.object(hardware, 'is_md_device', autospec=True) @mock.patch.object(image, '_get_efi_bootloaders', autospec=True) @mock.patch.object(image, '_get_partition', autospec=True) @mock.patch.object(utils, 'get_efi_part_on_device', autospec=True) @mock.patch.object(os, 'makedirs', autospec=True) def test__manage_uefi_nvme_device(self, mkdir_mock, mock_utils_efi_part, mock_get_part_uuid, mock_efi_bl, - mock_execute, mock_dispatch): + mock_is_md_device, mock_execute, + mock_dispatch): mock_utils_efi_part.return_value = '1' mock_get_part_uuid.return_value = '/dev/fakenvme0p1' - + mock_is_md_device.return_value = False mock_efi_bl.return_value = ['EFI/BOOT/BOOTX64.EFI'] mock_execute.side_effect = iter([('', ''), ('', ''), @@ -2444,17 +2466,18 @@ Boot0002: VENDMAGIC FvFile(9f3c6294-bf9b-4208-9808-be45dfc34b51) self.assertEqual(7, mock_execute.call_count) @mock.patch.object(os.path, 'exists', lambda *_: False) + @mock.patch.object(hardware, 'is_md_device', autospec=True) @mock.patch.object(image, '_get_efi_bootloaders', autospec=True) @mock.patch.object(image, '_get_partition', autospec=True) @mock.patch.object(utils, 'get_efi_part_on_device', autospec=True) @mock.patch.object(os, 'makedirs', autospec=True) def test__manage_uefi_wholedisk( self, mkdir_mock, mock_utils_efi_part, - mock_get_part_uuid, mock_efi_bl, mock_execute, - mock_dispatch): + mock_get_part_uuid, mock_efi_bl, mock_is_md_device, + mock_execute, mock_dispatch): mock_utils_efi_part.return_value = '1' mock_get_part_uuid.side_effect = Exception - + mock_is_md_device.return_value = False mock_efi_bl.return_value = ['EFI/BOOT/BOOTX64.EFI'] mock_execute.side_effect = iter([('', ''), ('', ''), @@ -2483,6 +2506,64 @@ Boot0002: VENDMAGIC FvFile(9f3c6294-bf9b-4208-9808-be45dfc34b51) mock_execute.assert_has_calls(expected) self.assertEqual(7, mock_execute.call_count) + @mock.patch.object(os.path, 'exists', lambda *_: False) + @mock.patch.object(hardware, 'get_component_devices', autospec=True) + @mock.patch.object(image, + 'prepare_boot_partitions_for_softraid', + autospec=True) + @mock.patch.object(hardware, 'get_holder_disks', autospec=True) + @mock.patch.object(hardware, 'is_md_device', autospec=True) + @mock.patch.object(image, '_get_efi_bootloaders', autospec=True) + @mock.patch.object(image, '_get_partition', autospec=True) + @mock.patch.object(utils, 'get_efi_part_on_device', autospec=True) + @mock.patch.object(os, 'makedirs', autospec=True) + def test__manage_uefi_software_raid( + self, mkdir_mock, mock_utils_efi_part, + mock_get_part_uuid, mock_efi_bl, mock_is_md_device, + mock_get_holder_disks, mock_prepare, mock_get_component_devices, + mock_execute, mock_dispatch): + mock_utils_efi_part.return_value = '1' + mock_get_part_uuid.side_effect = Exception + mock_efi_bl.return_value = ['EFI/BOOT/BOOTX64.EFI'] + mock_is_md_device.return_value = True + mock_get_holder_disks.return_value = ['/dev/sda', '/dev/sdb'] + mock_prepare.return_value = '/dev/md125' + mock_get_component_devices.return_value = ['/dev/sda3', '/dev/sdb3'] + + mock_execute.side_effect = iter([('', ''), ('', ''), + ('', ''), ('', ''), + ('', ''), ('', ''), + ('', ''), ('', ''), + ('', ''), ('', ''), + ('', '')]) + + expected = [mock.call('partx', '-a', '/dev/fake', attempts=3, + delay_on_retry=True), + mock.call('udevadm', 'settle'), + mock.call('mount', self.fake_efi_system_part, + self.fake_dir + '/boot/efi'), + mock.call('umount', self.fake_dir + '/boot/efi', + attempts=3, delay_on_retry=True), + mock.call('mount', self.fake_efi_system_part, + self.fake_dir + '/boot/efi'), + mock.call('efibootmgr', '-v'), + mock.call('efibootmgr', '-v', '-c', '-d', '/dev/sda3', + '-p', '3', '-w', '-L', 'ironic1 (RAID, part0)', + '-l', '\\EFI\\BOOT\\BOOTX64.EFI'), + mock.call('efibootmgr', '-v'), + mock.call('efibootmgr', '-v', '-c', '-d', '/dev/sdb3', + '-p', '3', '-w', '-L', 'ironic1 (RAID, part1)', + '-l', '\\EFI\\BOOT\\BOOTX64.EFI'), + mock.call('umount', self.fake_dir + '/boot/efi', + attempts=3, delay_on_retry=True), + mock.call('sync')] + + result = image._manage_uefi(self.fake_dev, None) + self.assertTrue(result) + mkdir_mock.assert_called_once_with(self.fake_dir + '/boot/efi') + mock_efi_bl.assert_called_once_with(self.fake_dir + '/boot/efi') + mock_execute.assert_has_calls(expected) + @mock.patch.object(os, 'walk', autospec=True) @mock.patch.object(os, 'access', autospec=False) def test__no_efi_bootloaders(self, mock_access, mock_walk, mock_execute, diff --git a/ironic_python_agent/tests/unit/test_hardware.py b/ironic_python_agent/tests/unit/test_hardware.py index 9e629e6c..0efa5555 100644 --- a/ironic_python_agent/tests/unit/test_hardware.py +++ b/ironic_python_agent/tests/unit/test_hardware.py @@ -3597,9 +3597,9 @@ class TestGenericHardwareManager(base.IronicAgentTest): @mock.patch.object(hardware, '_get_md_uuid', autospec=True) @mock.patch.object(hardware, 'list_all_block_devices', autospec=True) @mock.patch.object(utils, 'execute', autospec=True) - def test__get_component_devices(self, mocked_execute, - mocked_list_all_block_devices, - mocked_md_uuid): + def test_get_component_devices(self, mocked_execute, + mocked_list_all_block_devices, + mocked_md_uuid): raid_device1 = hardware.BlockDevice('/dev/md0', 'RAID-1', 107374182400, True) sda = hardware.BlockDevice('/dev/sda', 'model12', 21, True) @@ -3619,7 +3619,7 @@ class TestGenericHardwareManager(base.IronicAgentTest): [hws.MDADM_EXAMINE_OUTPUT_NON_MEMBER, '_'], ] - component_devices = hardware._get_component_devices(raid_device1) + component_devices = hardware.get_component_devices(raid_device1) self.assertEqual(['/dev/sda1'], component_devices) mocked_execute.assert_has_calls([ mock.call('mdadm', '--examine', '/dev/sda', @@ -3676,7 +3676,7 @@ class TestGenericHardwareManager(base.IronicAgentTest): self.assertEqual(['/dev/sda'], holder_disks) @mock.patch.object(hardware, 'get_holder_disks', autospec=True) - @mock.patch.object(hardware, '_get_component_devices', autospec=True) + @mock.patch.object(hardware, 'get_component_devices', autospec=True) @mock.patch.object(hardware, 'list_all_block_devices', autospec=True) @mock.patch.object(utils, 'execute', autospec=True) def test_delete_configuration(self, mocked_execute, mocked_list, @@ -3764,7 +3764,7 @@ class TestGenericHardwareManager(base.IronicAgentTest): mock.call('mdadm', '--assemble', '--scan', check_exit_code=False), ]) - @mock.patch.object(hardware, '_get_component_devices', autospec=True) + @mock.patch.object(hardware, 'get_component_devices', autospec=True) @mock.patch.object(hardware, 'list_all_block_devices', autospec=True) @mock.patch.object(utils, 'execute', autospec=True) def test_delete_configuration_partition(self, mocked_execute, mocked_list, @@ -3789,7 +3789,7 @@ class TestGenericHardwareManager(base.IronicAgentTest): mock.call('mdadm', '--assemble', '--scan', check_exit_code=False), ]) - @mock.patch.object(hardware, '_get_component_devices', autospec=True) + @mock.patch.object(hardware, 'get_component_devices', autospec=True) @mock.patch.object(hardware, 'list_all_block_devices', autospec=True) @mock.patch.object(utils, 'execute', autospec=True) def test_delete_configuration_failure_blocks_remaining( diff --git a/ironic_python_agent/tests/unit/test_raid_utils.py b/ironic_python_agent/tests/unit/test_raid_utils.py index 30a13065..a20bd13e 100644 --- a/ironic_python_agent/tests/unit/test_raid_utils.py +++ b/ironic_python_agent/tests/unit/test_raid_utils.py @@ -15,9 +15,11 @@ from unittest import mock from oslo_concurrency import processutils from ironic_python_agent import errors +from ironic_python_agent import hardware from ironic_python_agent import raid_utils from ironic_python_agent.tests.unit import base from ironic_python_agent.tests.unit.samples import hardware_samples as hws +from ironic_python_agent.tests.unit import test_hardware from ironic_python_agent import utils @@ -112,3 +114,24 @@ class TestRaidUtils(base.IronicAgentTest): "Failed re-add /dev/sdb1 to /dev/md0", raid_utils.create_raid_device, 0, logical_disk) + + +@mock.patch.object(hardware, 'dispatch_to_managers', autospec=True) +class TestGetNextFreeRaidDevice(base.IronicAgentTest): + + def test_ok(self, mock_dispatch): + mock_dispatch.return_value = \ + test_hardware.RAID_BLK_DEVICE_TEMPLATE_DEVICES + result = raid_utils.get_next_free_raid_device() + self.assertEqual('/dev/md2', result) + mock_dispatch.assert_called_once_with('list_block_devices') + + def test_no_device(self, mock_dispatch): + mock_dispatch.return_value = [ + hardware.BlockDevice(name=f'/dev/md{idx}', model='RAID', + size=1765517033470, rotational=False, + vendor="FooTastic", uuid="") + for idx in range(128) + ] + self.assertRaises(errors.SoftwareRAIDError, + raid_utils.get_next_free_raid_device) diff --git a/releasenotes/notes/move_swraid_to_efibootmgr-d87c1bfde1661fb5.yaml b/releasenotes/notes/move_swraid_to_efibootmgr-d87c1bfde1661fb5.yaml new file mode 100644 index 00000000..3fa29284 --- /dev/null +++ b/releasenotes/notes/move_swraid_to_efibootmgr-d87c1bfde1661fb5.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - | + Use efibootmgr instead of grub2-install for software RAID. + This fixes an issue with images which include newer versions + of grub2-install as they refuse bootloader installations in + UEFI boot mode due to the lack of secure boot support. -- cgit v1.2.1