From 403d2f06c670475b95de2bd971dcfcb83f166529 Mon Sep 17 00:00:00 2001 From: Dmitry Tantsur Date: Tue, 26 Jan 2021 18:27:52 +0100 Subject: [PATCH] Fix error message with UEFI-incompatible images It's somewhat confusing at the moment, since we're trying to find a UEFI partition by UUID "None". Don't search for partition if we don't know its UUID, and provide a better error message. Change-Id: Ief874084132797a445ddae8009264712a05facfd --- ironic_python_agent/extensions/image.py | 48 +++++++++++-------- .../tests/unit/extensions/test_image.py | 10 ++++ .../notes/uefi-images-38c8536db189ffc1.yaml | 5 ++ 3 files changed, 43 insertions(+), 20 deletions(-) create mode 100644 releasenotes/notes/uefi-images-38c8536db189ffc1.yaml diff --git a/ironic_python_agent/extensions/image.py b/ironic_python_agent/extensions/image.py index 0fcfb882b..c78b6ce4c 100644 --- a/ironic_python_agent/extensions/image.py +++ b/ironic_python_agent/extensions/image.py @@ -280,6 +280,7 @@ def _manage_uefi(device, efi_system_part_uuid=None): :param device: the device to be checked. :param efi_system_part_uuid: efi partition uuid. + :raises: DeviceNotFound if the efi partition cannot be found. :return: True - if it founds any efi bootloader and the nvram was updated using the efibootmgr. False - if no efi bootloader is found. @@ -295,38 +296,45 @@ def _manage_uefi(device, efi_system_part_uuid=None): local_path = tempfile.mkdtemp() # Trust the contents on the disk in the event of a whole disk image. efi_partition = utils.get_efi_part_on_device(device) - if not efi_partition: + if not efi_partition and efi_system_part_uuid: # _get_partition returns + and we only need the # partition number partition = _get_partition(device, uuid=efi_system_part_uuid) efi_partition = int(partition.replace(device, "")) - if efi_partition: - efi_partition_mount_point = os.path.join(local_path, "boot/efi") - if not os.path.exists(efi_partition_mount_point): - os.makedirs(efi_partition_mount_point) + if not efi_partition: + # NOTE(dtantsur): we cannot have a valid EFI deployment without an + # EFI partition at all. This code path is easily hit when using an + # image that is not UEFI compatible (which sadly applies to most + # cloud images out there, with a nice exception of Ubuntu). + raise errors.DeviceNotFound( + "No EFI partition could be detected on device %s and " + "EFI partition UUID has not been recorded during deployment " + "(which is often the case for whole disk images). " + "Are you using a UEFI-compatible image?" % device) - # The mount needs the device with the partition, in case the - # device ends with a digit we add a `p` and the partition number we - # found, otherwise we just join the device and the partition number - if device[-1].isdigit(): - efi_device_part = '{}p{}'.format(device, efi_partition) - utils.execute('mount', efi_device_part, - efi_partition_mount_point) - else: - efi_device_part = '{}{}'.format(device, efi_partition) - utils.execute('mount', efi_device_part, - efi_partition_mount_point) - efi_mounted = True + efi_partition_mount_point = os.path.join(local_path, "boot/efi") + if not os.path.exists(efi_partition_mount_point): + os.makedirs(efi_partition_mount_point) + + # The mount needs the device with the partition, in case the + # device ends with a digit we add a `p` and the partition number we + # found, otherwise we just join the device and the partition number + if device[-1].isdigit(): + efi_device_part = '{}p{}'.format(device, efi_partition) + utils.execute('mount', efi_device_part, efi_partition_mount_point) else: - # If we can't find the partition we need to decide what should - # happen - return False + efi_device_part = '{}{}'.format(device, efi_partition) + utils.execute('mount', efi_device_part, efi_partition_mount_point) + 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) return True else: + # NOTE(dtantsur): if we have an empty EFI partition, try to use + # grub-install to populate it. return False except processutils.ProcessExecutionError as e: diff --git a/ironic_python_agent/tests/unit/extensions/test_image.py b/ironic_python_agent/tests/unit/extensions/test_image.py index 55ee60f77..d7093b6b2 100644 --- a/ironic_python_agent/tests/unit/extensions/test_image.py +++ b/ironic_python_agent/tests/unit/extensions/test_image.py @@ -2107,6 +2107,16 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n mock_get_part_uuid, mock_execute, mock_dispatch): mock_utils_efi_part.return_value = None + self.assertRaises(errors.DeviceNotFound, + image._manage_uefi, self.fake_dev, None) + self.assertFalse(mock_get_part_uuid.called) + + @mock.patch.object(image, '_get_partition', autospec=True) + @mock.patch.object(utils, 'get_efi_part_on_device', autospec=True) + def test__manage_uefi_empty_partition_by_uuid(self, mock_utils_efi_part, + mock_get_part_uuid, + mock_execute, mock_dispatch): + mock_utils_efi_part.return_value = None mock_get_part_uuid.return_value = self.fake_root_part result = image._manage_uefi(self.fake_dev, self.fake_root_uuid) self.assertFalse(result) diff --git a/releasenotes/notes/uefi-images-38c8536db189ffc1.yaml b/releasenotes/notes/uefi-images-38c8536db189ffc1.yaml new file mode 100644 index 000000000..31f6814e5 --- /dev/null +++ b/releasenotes/notes/uefi-images-38c8536db189ffc1.yaml @@ -0,0 +1,5 @@ +--- +fixes: + - | + Provides a more specific error message if a UEFI-incompatible image + is used in the UEFI mode.