diff options
author | Zuul <zuul@review.opendev.org> | 2021-06-01 19:11:56 +0000 |
---|---|---|
committer | Gerrit Code Review <review@openstack.org> | 2021-06-01 19:11:56 +0000 |
commit | ab97415236e6fbbdc6e1edd828f281a92331fe3f (patch) | |
tree | 8c42c0f0dc28ee222f0627585d622b69d297809b | |
parent | 601b83308697e395df5a129f6403ab9535a909b3 (diff) | |
parent | 6e0d08c8928f7657b27bfa02209433d07ae56998 (diff) | |
download | ironic-python-agent-ab97415236e6fbbdc6e1edd828f281a92331fe3f.tar.gz |
Merge "Software RAID: RAID the ESPs" into stable/victoria
-rw-r--r-- | ironic_python_agent/extensions/image.py | 177 | ||||
-rw-r--r-- | ironic_python_agent/tests/unit/extensions/test_image.py | 66 | ||||
-rw-r--r-- | releasenotes/notes/software-raid-raid-ESPs-25a2aa117b99620a.yaml | 7 |
3 files changed, 124 insertions, 126 deletions
diff --git a/ironic_python_agent/extensions/image.py b/ironic_python_agent/extensions/image.py index db8ca755..8b49ccb2 100644 --- a/ironic_python_agent/extensions/image.py +++ b/ironic_python_agent/extensions/image.py @@ -372,8 +372,9 @@ def _prepare_boot_partitions_for_softraid(device, holders, efi_part, target_boot_mode): """Prepare boot partitions when relevant. - Create either efi partitions or bios boot partitions for softraid, - according to both target boot mode and disk holders partition table types. + Create either a RAIDed EFI partition or bios boot partitions for software + RAID, according to both target boot mode and disk holders partition table + types. :param device: the softraid device path :param holders: the softraid drive members @@ -382,11 +383,9 @@ def _prepare_boot_partitions_for_softraid(device, holders, efi_part, :param target_boot_mode: target boot mode can be bios/uefi/None or anything else for unspecified - :returns: the efi partition paths on softraid disk holders when target - boot mode is uefi, empty list otherwise. + :returns: the path to the ESP md device when target boot mode is uefi, + nothing otherwise. """ - efi_partitions = [] - # Actually any fat partition could be a candidate. Let's assume the # partition also has the esp flag if target_boot_mode == 'uefi': @@ -411,6 +410,7 @@ def _prepare_boot_partitions_for_softraid(device, holders, efi_part, # We could also directly get the EFI partition size. partsize_mib = raid_utils.ESP_SIZE_MIB partlabel_prefix = 'uefi-holder-' + efi_partitions = [] for number, holder in enumerate(holders): # NOTE: see utils.get_partition_table_type_from_specs # for uefi we know that we have setup a gpt partition table, @@ -432,26 +432,31 @@ def _prepare_boot_partitions_for_softraid(device, holders, efi_part, "blkid", "-l", "-t", "PARTLABEL={}".format(partlabel), holder) target_part = target_part.splitlines()[-1].split(':', 1)[0] + efi_partitions.append(target_part) LOG.debug("EFI partition %s created on holder disk %s", target_part, holder) - if efi_part: - LOG.debug("Relocating EFI %s to holder part %s", efi_part, - target_part) - # Blockdev copy - utils.execute("cp", efi_part, target_part) - else: - # Creating a label is just to make life easier - if number == 0: - fslabel = 'efi-part' - else: - # bak, label is limited to 11 chars - fslabel = 'efi-part-b' - ilib_utils.mkfs(fs='vfat', path=target_part, label=fslabel) - efi_partitions.append(target_part) - # TBD: Would not hurt to destroy source efi part when defined, - # for clarity. + # RAID the ESPs, metadata=1.0 is mandatory to be able to boot + md_device = '/dev/md/esp' + LOG.debug("Creating md device {} for the ESPs on {}".format( + md_device, efi_partitions)) + utils.execute('mdadm', '--create', md_device, '--force', + '--run', '--metadata=1.0', '--level', '1', + '--raid-devices', len(efi_partitions), + *efi_partitions) + + if efi_part: + # Blockdev copy the source ESP and erase it + LOG.debug("Relocating EFI %s to %s", efi_part, md_device) + utils.execute('cp', efi_part, md_device) + LOG.debug("Erasing EFI partition %s", efi_part) + utils.execute('wipefs', '-a', efi_part) + else: + fslabel = 'efi-part' + ilib_utils.mkfs(fs='vfat', path=md_device, label=fslabel) + + return md_device elif target_boot_mode == 'bios': partlabel_prefix = 'bios-boot-part-' @@ -475,9 +480,6 @@ def _prepare_boot_partitions_for_softraid(device, holders, efi_part, # Since there is a structural difference, this means it will # fail. - # Just an empty list if not uefi boot mode, nvm, not used anyway - return efi_partitions - def _umount_all_partitions(path, path_variable, umount_warn_msg): """Umount all partitions we may have mounted""" @@ -507,7 +509,7 @@ def _install_grub2(device, root_uuid, efi_system_part_uuid=None, """Install GRUB2 bootloader on a given device.""" LOG.debug("Installing GRUB2 bootloader on device %s", device) - efi_partitions = None + efi_partition = None efi_part = None efi_partition_mount_point = None efi_mounted = False @@ -544,15 +546,14 @@ def _install_grub2(device, root_uuid, efi_system_part_uuid=None, path = tempfile.mkdtemp() if efi_system_part_uuid: efi_part = _get_partition(device, uuid=efi_system_part_uuid) - efi_partitions = [efi_part] - + efi_partition = efi_part if hardware.is_md_device(device): holders = hardware.get_holder_disks(device) - efi_partitions = _prepare_boot_partitions_for_softraid( + efi_partition = _prepare_boot_partitions_for_softraid( device, holders, efi_part, target_boot_mode ) - if efi_partitions: + if efi_partition: efi_partition_mount_point = os.path.join(path, "boot/efi") # For power we want to install grub directly onto the PreP partition @@ -579,7 +580,7 @@ def _install_grub2(device, root_uuid, efi_system_part_uuid=None, # point if we have no efi partitions at all. efi_preserved = _try_preserve_efi_assets( device, path, efi_system_part_uuid, - efi_partitions, efi_partition_mount_point) + efi_partition, efi_partition_mount_point) if efi_preserved: _append_uefi_to_fstab(path, efi_system_part_uuid) # Success preserving efi assets @@ -608,50 +609,48 @@ def _install_grub2(device, root_uuid, efi_system_part_uuid=None, {'path': path}, shell=True, env_variables={'PATH': path_variable}) - if efi_partitions: + if efi_partition: if not os.path.exists(efi_partition_mount_point): os.makedirs(efi_partition_mount_point) - LOG.warning("GRUB2 will be installed for UEFI on efi partitions " + LOG.warning("GRUB2 will be installed for UEFI on efi partition " "%s using the install command which does not place " - "Secure Boot signed binaries.", efi_partitions) - for efi_partition in efi_partitions: - utils.execute( - 'mount', efi_partition, efi_partition_mount_point) - efi_mounted = True - # FIXME(rg): does not work in cross boot mode case (target - # boot mode differs from ramdisk one) - # Probe for the correct target (depends on the arch, example - # --target=x86_64-efi) - utils.execute('chroot %(path)s /bin/sh -c ' - '"%(bin)s-install"' % - {'path': path, 'bin': binary_name}, - shell=True, - env_variables={ - 'PATH': path_variable - }) - # Also run grub-install with --removable, this installs grub to - # the EFI fallback path. Useful if the NVRAM wasn't written - # correctly, was reset or if testing with virt as libvirt - # resets the NVRAM on instance start. - # This operation is essentially a copy operation. Use of the - # --removable flag, per the grub-install source code changes - # the default file to be copied, destination file name, and - # prevents NVRAM from being updated. - # We only run grub2_install for uefi if we can't verify the - # uefi bits - utils.execute('chroot %(path)s /bin/sh -c ' - '"%(bin)s-install --removable"' % - {'path': path, 'bin': binary_name}, - shell=True, - env_variables={ - 'PATH': path_variable - }) - utils.execute('umount', efi_partition_mount_point, attempts=3, - delay_on_retry=True) - efi_mounted = False + "Secure Boot signed binaries.", efi_partition) + utils.execute('mount', efi_partition, efi_partition_mount_point) + efi_mounted = True + # FIXME(rg): does not work in cross boot mode case (target + # boot mode differs from ramdisk one) + # Probe for the correct target (depends on the arch, example + # --target=x86_64-efi) + utils.execute('chroot %(path)s /bin/sh -c ' + '"%(bin)s-install"' % + {'path': path, 'bin': binary_name}, + shell=True, + env_variables={ + 'PATH': path_variable + }) + # Also run grub-install with --removable, this installs grub to + # the EFI fallback path. Useful if the NVRAM wasn't written + # correctly, was reset or if testing with virt as libvirt + # resets the NVRAM on instance start. + # This operation is essentially a copy operation. Use of the + # --removable flag, per the grub-install source code changes + # the default file to be copied, destination file name, and + # prevents NVRAM from being updated. + # We only run grub2_install for uefi if we can't verify the + # uefi bits + utils.execute('chroot %(path)s /bin/sh -c ' + '"%(bin)s-install --removable"' % + {'path': path, 'bin': binary_name}, + shell=True, + env_variables={ + 'PATH': path_variable + }) + utils.execute('umount', efi_partition_mount_point, attempts=3, + delay_on_retry=True) + efi_mounted = False # NOTE: probably never needed for grub-mkconfig, does not hurt in # case of doubt, cleaned in the finally clause anyway - utils.execute('mount', efi_partitions[0], + utils.execute('mount', efi_partition, efi_partition_mount_point) efi_mounted = True else: @@ -783,7 +782,7 @@ def _mount_for_chroot(path): def _try_preserve_efi_assets(device, path, efi_system_part_uuid, - efi_partitions, + efi_partition, efi_partition_mount_point): """Attempt to preserve UEFI boot assets. @@ -793,8 +792,8 @@ def _try_preserve_efi_assets(device, path, which we should examine to preserve assets from. :param efi_system_part_uuid: The partition ID representing the created EFI system partition. - :param efi_partitions: The list of partitions upon wich to - write the preserved assets to. + :param efi_partition: The partitions upon wich to write the preserved + assets to. :param efi_partition_mount_point: The folder at which to mount the assets for the process of preservation. @@ -817,7 +816,7 @@ def _try_preserve_efi_assets(device, path, # But first, if we have grub, we should try to build a grub config! LOG.debug('EFI asset folder detected, attempting to preserve assets.') if _preserve_efi_assets(path, efi_assets_folder, - efi_partitions, + efi_partition, efi_partition_mount_point): try: # Since we have preserved the assets, we should be able @@ -897,21 +896,21 @@ def _efi_boot_setup(device, efi_system_part_uuid=None, target_boot_mode=None): return False -def _preserve_efi_assets(path, efi_assets_folder, efi_partitions, +def _preserve_efi_assets(path, efi_assets_folder, efi_partition, efi_partition_mount_point): """Preserve the EFI assets in a partition image. :param path: The path used for the mounted image filesystem. :param efi_assets_folder: The folder where we can find the UEFI assets required for booting. - :param efi_partitions: The list of partitions upon which to - write the perserved assets to. + :param efi_partition: The partition upon which to write the + perserved assets to. :param efi_partition_mount_point: The folder at which to mount the assets for the process of preservation. :returns: True if EFI assets were able to be located and preserved to their appropriate locations based upon the supplied - efi_partitions list. + efi_partition. False if any error is encountered in this process. """ try: @@ -960,18 +959,16 @@ def _preserve_efi_assets(path, efi_assets_folder, efi_partitions, except (IOError, OSError, shutil.SameFileError) as e: LOG.warning('Failed to copy grubenv file. ' 'Error: %s', e) - # Loop through partitions because software RAID. - for efi_part in efi_partitions: - utils.execute('mount', '-t', 'vfat', efi_part, - efi_partition_mount_point) - shutil.copytree(save_efi, efi_assets_folder) - LOG.debug('Files preserved to %(disk)s for %(part)s. ' - 'Files: %(filelist)s From: %(from)s', - {'disk': efi_part, - 'part': efi_partition_mount_point, - 'filelist': os.listdir(efi_assets_folder), - 'from': save_efi}) - utils.execute('umount', efi_partition_mount_point) + utils.execute('mount', '-t', 'vfat', efi_partition, + efi_partition_mount_point) + shutil.copytree(save_efi, efi_assets_folder) + LOG.debug('Files preserved to %(disk)s for %(part)s. ' + 'Files: %(filelist)s From: %(from)s', + {'disk': efi_partition, + 'part': efi_partition_mount_point, + 'filelist': os.listdir(efi_assets_folder), + 'from': save_efi}) + utils.execute('umount', efi_partition_mount_point) return True except Exception as e: LOG.debug('Failed to preserve EFI assets. Error %s', e) diff --git a/ironic_python_agent/tests/unit/extensions/test_image.py b/ironic_python_agent/tests/unit/extensions/test_image.py index d7a51c59..d9267899 100644 --- a/ironic_python_agent/tests/unit/extensions/test_image.py +++ b/ironic_python_agent/tests/unit/extensions/test_image.py @@ -1199,7 +1199,7 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n mock_preserve_efi_assets.assert_called_with( self.fake_dir, self.fake_dir + '/boot/efi/EFI', - ['/dev/fake1'], + '/dev/fake1', self.fake_dir + '/boot/efi') mock_append_to_fstab.assert_called_with(self.fake_dir, self.fake_efi_system_part_uuid) @@ -1525,16 +1525,17 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n (None, None), # partprobe (None, None), # blkid ('/dev/sda12: dsfkgsdjfg', None), # blkid - (None, None), # cp ('452', None), # sgdisk -F (None, None), # sgdisk create part (None, None), # partprobe (None, None), # blkid ('/dev/sdb14: whatever', None), # blkid + (None, None), # mdadm (None, None), # cp + (None, None), # wipefs ] - efi_parts = 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') @@ -1547,7 +1548,6 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n mock.call('blkid'), mock.call('blkid', '-l', '-t', 'PARTLABEL=uefi-holder-0', '/dev/sda'), - mock.call('cp', '/dev/md0p12', '/dev/sda12'), mock.call('sgdisk', '-F', '/dev/sdb'), mock.call('sgdisk', '-n', '0:452s:+550MiB', '-t', '0:ef00', '-c', '0:uefi-holder-1', '/dev/sdb'), @@ -1555,10 +1555,14 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n mock.call('blkid'), mock.call('blkid', '-l', '-t', 'PARTLABEL=uefi-holder-1', '/dev/sdb'), - mock.call('cp', '/dev/md0p12', '/dev/sdb14') + 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('wipefs', '-a', '/dev/md0p12') ] mock_execute.assert_has_calls(expected, any_order=False) - self.assertEqual(efi_parts, ['/dev/sda12', '/dev/sdb14']) + self.assertEqual(efi_part, '/dev/md/esp') @mock.patch.object(utils, 'get_efi_part_on_device', autospec=True) @mock.patch.object(ilib_utils, 'mkfs', autospec=True) @@ -1576,9 +1580,10 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n (None, None), # partprobe (None, None), # blkid ('/dev/sdb14: whatever', None), # blkid + (None, None), # mdadm ] - efi_parts = 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') @@ -1601,10 +1606,9 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n ] mock_execute.assert_has_calls(expected, any_order=False) mock_mkfs.assert_has_calls([ - mock.call(path='/dev/sda12', label='efi-part', fs='vfat'), - mock.call(path='/dev/sdb14', label='efi-part-b', fs='vfat'), + mock.call(path='/dev/md/esp', label='efi-part', fs='vfat'), ], any_order=False) - self.assertEqual(efi_parts, ['/dev/sda12', '/dev/sdb14']) + self.assertEqual(efi_part, '/dev/md/esp') def test__prepare_boot_partitions_for_softraid_uefi_gpt_efi_provided( self, mock_execute, mock_dispatch): @@ -1614,16 +1618,17 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n (None, None), # partprobe (None, None), # blkid ('/dev/sda12: dsfkgsdjfg', None), # blkid - (None, None), # cp ('452', None), # sgdisk -F (None, None), # sgdisk create part (None, None), # partprobe (None, None), # blkid ('/dev/sdb14: whatever', None), # blkid + (None, None), # mdadm create (None, None), # cp + (None, None), # wipefs ] - efi_parts = 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') @@ -1635,7 +1640,6 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n mock.call('blkid'), mock.call('blkid', '-l', '-t', 'PARTLABEL=uefi-holder-0', '/dev/sda'), - mock.call('cp', '/dev/md0p15', '/dev/sda12'), mock.call('sgdisk', '-F', '/dev/sdb'), mock.call('sgdisk', '-n', '0:452s:+550MiB', '-t', '0:ef00', '-c', '0:uefi-holder-1', '/dev/sdb'), @@ -1643,17 +1647,21 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n mock.call('blkid'), mock.call('blkid', '-l', '-t', 'PARTLABEL=uefi-holder-1', '/dev/sdb'), - mock.call('cp', '/dev/md0p15', '/dev/sdb14') + 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('wipefs', '-a', '/dev/md0p15') ] mock_execute.assert_has_calls(expected, any_order=False) - self.assertEqual(efi_parts, ['/dev/sda12', '/dev/sdb14']) + self.assertEqual(efi_part, '/dev/md/esp') @mock.patch.object(utils, 'scan_partition_table_type', autospec=True, return_value='msdos') def test__prepare_boot_partitions_for_softraid_bios_msdos( self, mock_label_scan, mock_execute, mock_dispatch): - efi_parts = 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') @@ -1662,7 +1670,7 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n mock.call('/dev/sdb'), ] mock_label_scan.assert_has_calls(expected, any_order=False) - self.assertEqual(efi_parts, []) + self.assertIsNone(efi_part) @mock.patch.object(utils, 'scan_partition_table_type', autospec=True, return_value='gpt') @@ -1676,7 +1684,7 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n (None, None), # bios boot grub ] - efi_parts = 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') @@ -1697,7 +1705,7 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n ] mock_execute.assert_has_calls(expected_exec, any_order=False) - self.assertEqual(efi_parts, []) + self.assertIsNone(efi_part) @mock.patch.object(image, '_is_bootloader_loaded', lambda *_: True) @mock.patch.object(hardware, 'is_md_device', autospec=True) @@ -1710,7 +1718,7 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n @mock.patch.object(image, '_get_partition', autospec=True) @mock.patch.object(image, '_prepare_boot_partitions_for_softraid', autospec=True, - return_value=['/dev/sda1', '/dev/sdb2']) + return_value='/dev/md/esp') @mock.patch.object(image, '_has_dracut', autospec=True, return_value=False) @@ -1745,21 +1753,7 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n (self.fake_dir)), shell=True, env_variables={ 'PATH': '/sbin:/bin:/usr/sbin:/sbin'}), - mock.call('mount', '/dev/sda1', - self.fake_dir + '/boot/efi'), - mock.call(('chroot %s /bin/sh -c "grub-install"' % - self.fake_dir), shell=True, - env_variables={ - 'PATH': '/sbin:/bin:/usr/sbin:/sbin'}), - mock.call(('chroot %s /bin/sh -c ' - '"grub-install --removable"' % - self.fake_dir), shell=True, - env_variables={ - 'PATH': '/sbin:/bin:/usr/sbin:/sbin'}), - mock.call( - 'umount', self.fake_dir + '/boot/efi', - attempts=3, delay_on_retry=True), - mock.call('mount', '/dev/sdb2', + mock.call('mount', '/dev/md/esp', self.fake_dir + '/boot/efi'), mock.call(('chroot %s /bin/sh -c "grub-install"' % self.fake_dir), shell=True, @@ -1773,7 +1767,7 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n mock.call( 'umount', self.fake_dir + '/boot/efi', attempts=3, delay_on_retry=True), - mock.call('mount', '/dev/sda1', + mock.call('mount', '/dev/md/esp', '/tmp/fake-dir/boot/efi'), mock.call(('chroot %s /bin/sh -c ' '"grub-mkconfig -o ' diff --git a/releasenotes/notes/software-raid-raid-ESPs-25a2aa117b99620a.yaml b/releasenotes/notes/software-raid-raid-ESPs-25a2aa117b99620a.yaml new file mode 100644 index 00000000..a45a7ff4 --- /dev/null +++ b/releasenotes/notes/software-raid-raid-ESPs-25a2aa117b99620a.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - | + Mirrors the previously disconnected EFI system partitions (ESPs) in UEFI + software RAID setups. Disconnected ESPs can lead to nodes booting with + outdated kernel parameters or the UEFI firmware not finding bootable + kernels at all. |