diff --git a/ironic_python_agent/extensions/image.py b/ironic_python_agent/extensions/image.py index 8aa71e799..96cb9503d 100644 --- a/ironic_python_agent/extensions/image.py +++ b/ironic_python_agent/extensions/image.py @@ -378,8 +378,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 @@ -388,11 +389,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': @@ -417,6 +416,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, @@ -438,26 +438,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-' @@ -481,9 +486,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""" @@ -513,7 +515,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 @@ -550,15 +552,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 @@ -585,7 +586,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 @@ -614,50 +615,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: @@ -789,7 +788,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. @@ -799,8 +798,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. @@ -823,7 +822,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 @@ -903,21 +902,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: @@ -966,18 +965,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 c91de8416..232e4647d 100644 --- a/ironic_python_agent/tests/unit/extensions/test_image.py +++ b/ironic_python_agent/tests/unit/extensions/test_image.py @@ -1200,7 +1200,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) @@ -1526,16 +1526,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') @@ -1548,7 +1549,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'), @@ -1556,10 +1556,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) @@ -1577,9 +1581,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') @@ -1602,10 +1607,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): @@ -1615,16 +1619,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') @@ -1636,7 +1641,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'), @@ -1644,17 +1648,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') @@ -1663,7 +1671,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') @@ -1677,7 +1685,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') @@ -1698,7 +1706,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) @@ -1711,7 +1719,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) @@ -1746,7 +1754,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', + 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, @@ -1760,21 +1768,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/sdb2', - 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/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 000000000..a45a7ff43 --- /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.