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)

Story: #2009794

Change-Id: I7ed34e595215194a589c2f1cd0b39ff0336da8f1

Author: arnewiebalck

ironic_python_agent/efi_utils.py

@@ -20,6 +20,8 @@
 from oslo_log import log
 
 from ironic_python_agent import errors
+from ironic_python_agent.extensions import image
+from ironic_python_agent import hardware
 from ironic_python_agent import partition_utils
 from ironic_python_agent import utils
 
@@ -92,16 +94,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 = image.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})
@@ -227,7 +273,7 @@ def remove_boot_record(boot_num):
 
 
 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
@@ -236,6 +282,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
@@ -255,9 +304,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 boot_num, boot_rec in boot_records:
@@ -268,9 +321,11 @@ def _run_efibootmgr(valid_efi_bootloaders, device, efi_partition,
                 LOG.debug("Found bootnum %s matching label", boot_num)
                 remove_boot_record(boot_num)
 
-        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
         add_boot_record(device, efi_partition, v_efi_bl_path, label)
         # Increment the ID in case the loop runs again.

ironic_python_agent/extensions/image.py

@@ -105,8 +105,8 @@ def _is_bootloader_loaded(dev):
 
 
 # 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
@@ -311,7 +311,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
             )
 
@@ -648,9 +648,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:

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)
         il_utils.execute('mdadm', '--stop', raid_device)
         il_utils.execute('mdadm', '--assemble', raid_device,
                          *component_devices)
@@ -2221,7 +2221,7 @@ def _scan_raids():
     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

ironic_python_agent/tests/unit/extensions/test_image.py

@@ -1654,7 +1654,7 @@ def test__install_grub2_command_fail(self, mock_get_part_uuid,
         self.assertFalse(mock_dispatch.called)
 
     @mock.patch.object(disk_utils, 'find_efi_partition', autospec=True)
-    def test__prepare_boot_partitions_for_softraid_uefi_gpt(
+    def test_prepare_boot_partitions_for_softraid_uefi_gpt(
             self, mock_efi_part, mock_execute, mock_dispatch):
         mock_efi_part.return_value = {'number': '12'}
         mock_execute.side_effect = [
@@ -1673,7 +1673,7 @@ def test__prepare_boot_partitions_for_softraid_uefi_gpt(
             (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')
 
@@ -1704,7 +1704,7 @@ def test__prepare_boot_partitions_for_softraid_uefi_gpt(
 
     @mock.patch.object(disk_utils, 'find_efi_partition', autospec=True)
     @mock.patch.object(ilib_utils, 'mkfs', autospec=True)
-    def test__prepare_boot_partitions_for_softraid_uefi_gpt_esp_not_found(
+    def test_prepare_boot_partitions_for_softraid_uefi_gpt_esp_not_found(
             self, mock_mkfs, mock_efi_part, mock_execute, mock_dispatch):
         mock_efi_part.return_value = None
         mock_execute.side_effect = [
@@ -1721,7 +1721,7 @@ def test__prepare_boot_partitions_for_softraid_uefi_gpt_esp_not_found(
             (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')
 
@@ -1748,7 +1748,7 @@ def test__prepare_boot_partitions_for_softraid_uefi_gpt_esp_not_found(
         ], any_order=False)
         self.assertEqual(efi_part, '/dev/md/esp')
 
-    def test__prepare_boot_partitions_for_softraid_uefi_gpt_efi_provided(
+    def test_prepare_boot_partitions_for_softraid_uefi_gpt_efi_provided(
             self, mock_execute, mock_dispatch):
         mock_execute.side_effect = [
             ('451', None),  # sgdisk -F
@@ -1766,7 +1766,7 @@ def test__prepare_boot_partitions_for_softraid_uefi_gpt_efi_provided(
             (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')
 
@@ -1796,10 +1796,10 @@ def test__prepare_boot_partitions_for_softraid_uefi_gpt_efi_provided(
 
     @mock.patch.object(disk_utils, 'get_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')
 
@@ -1812,7 +1812,7 @@ def test__prepare_boot_partitions_for_softraid_bios_msdos(
 
     @mock.patch.object(disk_utils, 'get_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 = [
@@ -1822,7 +1822,7 @@ def test__prepare_boot_partitions_for_softraid_bios_gpt(
             (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')
 
@@ -1854,7 +1854,7 @@ def test__prepare_boot_partitions_for_softraid_bios_gpt(
     @mock.patch.object(os, 'environ', autospec=True)
     @mock.patch.object(os, 'makedirs', autospec=True)
     @mock.patch.object(partition_utils, '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',
@@ -1972,7 +1972,7 @@ def test__install_grub2_softraid_uefi_gpt(
     @mock.patch.object(os, 'environ', autospec=True)
     @mock.patch.object(os, 'makedirs', autospec=True)
     @mock.patch.object(partition_utils, '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',