diff --git a/ironic_python_agent/extensions/image.py b/ironic_python_agent/extensions/image.py index f4e38009c..5353e976e 100644 --- a/ironic_python_agent/extensions/image.py +++ b/ironic_python_agent/extensions/image.py @@ -34,6 +34,8 @@ LOG = log.getLogger(__name__) BIND_MOUNTS = ('/dev', '/proc', '/run') +BOOTLOADERS_EFI = ['bootx64.efi', 'grubaa64.efi', 'winload.efi'] + def _get_partition(device, uuid): """Find the partition of a given device.""" @@ -181,6 +183,151 @@ def _is_bootloader_loaded(dev): return False +def _get_efi_bootloaders(location): + """Get all valid efi bootloaders in a given location + + :param location: the location where it should start looking for the + efi files. + :return: a list of valid efi bootloaders + """ + + # Let's find all files with .efi or .EFI extension + LOG.debug('Looking for all efi files on %s', location) + valid_bootloaders = [] + for root, dirs, files in os.walk(location): + efi_files = [f for f in files if f.lower() in BOOTLOADERS_EFI] + LOG.debug('efi files found in %(location)s : %(efi_files)s', + {'location': location, 'efi_files': str(efi_files)}) + for name in efi_files: + efi_f = os.path.join(root, name) + LOG.debug('Checking if %s is executable', efi_f) + if os.access(efi_f, os.X_OK): + v_bl = efi_f.split('/boot/efi')[-1].replace('/', '\\') + LOG.debug('%s is a valid bootloader', v_bl) + valid_bootloaders.append(v_bl) + return valid_bootloaders + + +def _run_efibootmgr(valid_efi_bootloaders, device, efi_partition): + """Executes efibootmgr and removes duplicate entries. + + :param valid_efi_bootloaders: the list of valid efi bootloaders + :param device: the device to be used + :param efi_partition: the efi partition on the device + """ + + # Before updating let's get information about the bootorder + LOG.debug("Getting information about boot order") + utils.execute('efibootmgr') + # NOTE(iurygregory): regex used to identify the Warning in the stderr after + # we add the new entry. Example: + # "efibootmgr: ** Warning ** : Boot0004 has same label ironic" + duplicated_label = re.compile(r'^.*:\s\*\*.*\*\*\s:\s.*' + r'Boot([0-9a-f-A-F]+)\s.*$') + label_id = 1 + for v_efi_bl_path in valid_efi_bootloaders: + # Update the nvram using efibootmgr + # https://linux.die.net/man/8/efibootmgr + label = 'ironic' + str(label_id) + LOG.debug("Adding loader %(path)s on partition %(part)s of device " + " %(dev)s", {'path': v_efi_bl_path, 'part': efi_partition, + 'dev': device}) + cmd = utils.execute('efibootmgr', '-c', '-d', device, + '-p', efi_partition, '-w', '-L', label, + '-l', v_efi_bl_path) + for line in cmd[1].split('\n'): + match = duplicated_label.match(line) + if match: + boot_num = match.group(1) + LOG.debug("Found bootnum %s matching label", boot_num) + utils.execute('efibootmgr', '-b', boot_num, '-B') + label_id += 1 + + +def _manage_uefi(device, efi_system_part_uuid=None): + """Manage the device looking for valid efi bootloaders to update the nvram. + + This method checks for valid efi bootloaders in the device, if they exists + it updates the nvram using the efibootmgr. + + :param device: the device to be checked. + :param efi_system_part_uuid: efi partition uuid. + :return: True - if it founds any efi bootloader and the nvram was updated + using the efibootmgr. + False - if no efi bootloader is found. + """ + efi_partition = None + efi_partition_mount_point = None + efi_mounted = False + + try: + 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: + # _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) + + # 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 + else: + # If we can't find the partition we need to decide what should + # happen + return False + 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: + return False + + except processutils.ProcessExecutionError as e: + error_msg = ('Could not verify uefi on device %(dev)s' + 'failed with %(err)s.' % {'dev': device, 'err': e}) + LOG.error(error_msg) + raise errors.CommandExecutionError(error_msg) + finally: + umount_warn_msg = "Unable to umount %(local_path)s. Error: %(error)s" + + try: + if efi_mounted: + utils.execute('umount', efi_partition_mount_point, + attempts=3, delay_on_retry=True) + except processutils.ProcessExecutionError as e: + error_msg = ('Umounting efi system partition failed. ' + 'Attempted 3 times. Error: %s' % e) + LOG.error(error_msg) + raise errors.CommandExecutionError(error_msg) + + else: + # If umounting the binds succeed then we can try to delete it + try: + utils.execute('sync') + except processutils.ProcessExecutionError as e: + LOG.warning(umount_warn_msg, {'path': local_path, 'error': e}) + else: + # After everything is umounted we can then remove the + # temporary directory + shutil.rmtree(local_path) + + def _install_grub2(device, root_uuid, efi_system_part_uuid=None, prep_boot_part_uuid=None): """Install GRUB2 bootloader on a given device.""" @@ -265,6 +412,7 @@ def _install_grub2(device, root_uuid, efi_system_part_uuid=None, # --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 if efi_partition: utils.execute('chroot %(path)s /bin/sh -c ' '"%(bin)s-install %(dev)s --removable"' % @@ -367,6 +515,22 @@ class ImageExtension(base.BaseAgentExtension): """ device = hardware.dispatch_to_managers('get_os_install_device') iscsi.clean_up(device) + boot = hardware.dispatch_to_managers('get_boot_info') + if boot.current_boot_mode == 'uefi': + has_efibootmgr = True + try: + utils.execute('efibootmgr', '--version') + except errors.CommandExecutionError: + LOG.warning("efibootmgr is not available in the ramdisk") + has_efibootmgr = False + + if has_efibootmgr: + if _manage_uefi(device, + efi_system_part_uuid=efi_system_part_uuid): + return + + # In case we can't use efibootmgr for uefi we will continue using grub2 + LOG.debug('Using grub2-install to set up boot files') _install_grub2(device, root_uuid=root_uuid, efi_system_part_uuid=efi_system_part_uuid, diff --git a/ironic_python_agent/tests/unit/extensions/test_image.py b/ironic_python_agent/tests/unit/extensions/test_image.py index aa8022755..44dce7b7a 100644 --- a/ironic_python_agent/tests/unit/extensions/test_image.py +++ b/ironic_python_agent/tests/unit/extensions/test_image.py @@ -48,25 +48,36 @@ class TestImageExtension(base.IronicAgentTest): @mock.patch.object(iscsi, 'clean_up', autospec=True) @mock.patch.object(image, '_install_grub2', autospec=True) - def test_install_bootloader_bios(self, mock_grub2, mock_iscsi_clean, - mock_execute, mock_dispatch): - mock_dispatch.return_value = self.fake_dev + def test__install_bootloader_bios(self, mock_grub2, mock_iscsi_clean, + mock_execute, mock_dispatch): + mock_dispatch.side_effect = [ + self.fake_dev, hardware.BootInfo(current_boot_mode='bios') + ] self.agent_extension.install_bootloader(root_uuid=self.fake_root_uuid) - mock_dispatch.assert_called_once_with('get_os_install_device') + mock_dispatch.assert_any_call('get_os_install_device') + mock_dispatch.assert_any_call('get_boot_info') + self.assertEqual(2, mock_dispatch.call_count) mock_grub2.assert_called_once_with( self.fake_dev, root_uuid=self.fake_root_uuid, efi_system_part_uuid=None, prep_boot_part_uuid=None) mock_iscsi_clean.assert_called_once_with(self.fake_dev) @mock.patch.object(iscsi, 'clean_up', autospec=True) + @mock.patch.object(image, '_manage_uefi', autospec=True) @mock.patch.object(image, '_install_grub2', autospec=True) - def test_install_bootloader_uefi(self, mock_grub2, mock_iscsi_clean, - mock_execute, mock_dispatch): - mock_dispatch.return_value = self.fake_dev + def test__install_bootloader_uefi(self, mock_grub2, mock_uefi, + mock_iscsi_clean, + mock_execute, mock_dispatch): + mock_dispatch.side_effect = [ + self.fake_dev, hardware.BootInfo(current_boot_mode='uefi') + ] + mock_uefi.return_value = False self.agent_extension.install_bootloader( root_uuid=self.fake_root_uuid, efi_system_part_uuid=self.fake_efi_system_part_uuid) - mock_dispatch.assert_called_once_with('get_os_install_device') + mock_dispatch.assert_any_call('get_os_install_device') + mock_dispatch.assert_any_call('get_boot_info') + self.assertEqual(2, mock_dispatch.call_count) mock_grub2.assert_called_once_with( self.fake_dev, root_uuid=self.fake_root_uuid, @@ -74,16 +85,207 @@ class TestImageExtension(base.IronicAgentTest): prep_boot_part_uuid=None) mock_iscsi_clean.assert_called_once_with(self.fake_dev) + @mock.patch.object(os.path, 'exists', lambda *_: False) + @mock.patch.object(iscsi, 'clean_up', 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=False) + @mock.patch.object(os, 'makedirs', autospec=True) + def test__uefi_bootloader_given_partition( + self, mkdir_mock, mock_utils_efi_part, mock_partition, + mock_efi_bl, mock_iscsi_clean, mock_execute, mock_dispatch): + mock_dispatch.side_effect = [ + self.fake_dev, hardware.BootInfo(current_boot_mode='uefi') + ] + mock_partition.side_effect = [self.fake_dev, self.fake_efi_system_part] + mock_efi_bl.return_value = ['\\EFI\\BOOT\\BOOTX64.EFI'] + mock_utils_efi_part.return_value = '1' + + mock_execute.side_effect = iter([('', ''), ('', ''), + ('', ''), ('', ''), + ('', ''), ('', '')]) + + expected = [mock.call('efibootmgr', '--version'), + mock.call('mount', self.fake_efi_system_part, + self.fake_dir + '/boot/efi'), + mock.call('efibootmgr'), + mock.call('efibootmgr', '-c', '-d', self.fake_dev, + '-p', '1', '-w', + '-L', 'ironic1', '-l', + '\\EFI\\BOOT\\BOOTX64.EFI'), + mock.call('umount', self.fake_dir + '/boot/efi', + attempts=3, delay_on_retry=True), + mock.call('sync')] + + self.agent_extension.install_bootloader( + root_uuid=self.fake_root_uuid, + efi_system_part_uuid=self.fake_efi_system_part_uuid) + + mock_dispatch.assert_any_call('get_os_install_device') + mock_dispatch.assert_any_call('get_boot_info') + 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_utils_efi_part.assert_called_once_with(self.fake_dev) + self.assertEqual(6, mock_execute.call_count) + + @mock.patch.object(os.path, 'exists', lambda *_: False) + @mock.patch.object(iscsi, 'clean_up', 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__uefi_bootloader_find_partition( + self, mkdir_mock, mock_utils_efi_part, mock_partition, + mock_efi_bl, mock_iscsi_clean, mock_execute, mock_dispatch): + mock_dispatch.side_effect = [ + self.fake_dev, hardware.BootInfo(current_boot_mode='uefi') + ] + mock_partition.return_value = self.fake_dev + mock_utils_efi_part.return_value = '1' + mock_efi_bl.return_value = ['\\EFI\\BOOT\\BOOTX64.EFI'] + mock_execute.side_effect = iter([('', ''), ('', ''), + ('', ''), ('', ''), + ('', ''), ('', '')]) + + expected = [mock.call('efibootmgr', '--version'), + mock.call('mount', self.fake_efi_system_part, + self.fake_dir + '/boot/efi'), + mock.call('efibootmgr'), + mock.call('efibootmgr', '-c', '-d', self.fake_dev, + '-p', '1', '-w', + '-L', 'ironic1', '-l', + '\\EFI\\BOOT\\BOOTX64.EFI'), + mock.call('umount', self.fake_dir + '/boot/efi', + attempts=3, delay_on_retry=True), + mock.call('sync')] + + self.agent_extension.install_bootloader( + root_uuid=self.fake_root_uuid, + efi_system_part_uuid=None) + + mock_dispatch.assert_any_call('get_os_install_device') + mock_dispatch.assert_any_call('get_boot_info') + 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_utils_efi_part.assert_called_once_with(self.fake_dev) + self.assertEqual(6, mock_execute.call_count) + + @mock.patch.object(os.path, 'exists', lambda *_: False) + @mock.patch.object(iscsi, 'clean_up', 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__uefi_bootloader_with_entry_removal( + self, mkdir_mock, mock_utils_efi_part, mock_partition, + mock_efi_bl, mock_iscsi_clean, mock_execute, mock_dispatch): + mock_dispatch.side_effect = [ + self.fake_dev, hardware.BootInfo(current_boot_mode='uefi') + ] + mock_partition.return_value = self.fake_dev + mock_utils_efi_part.return_value = '1' + mock_efi_bl.return_value = ['\\EFI\\BOOT\\BOOTX64.EFI'] + stdeer_msg = """ +efibootmgr: ** Warning ** : Boot0004 has same label ironic1\n +efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n +""" + mock_execute.side_effect = iter([('', ''), ('', ''), + ('', ''), ('', stdeer_msg), + ('', ''), ('', ''), + ('', ''), ('', '')]) + + expected = [mock.call('efibootmgr', '--version'), + mock.call('mount', self.fake_efi_system_part, + self.fake_dir + '/boot/efi'), + mock.call('efibootmgr'), + mock.call('efibootmgr', '-c', '-d', self.fake_dev, + '-p', '1', '-w', + '-L', 'ironic1', '-l', + '\\EFI\\BOOT\\BOOTX64.EFI'), + mock.call('efibootmgr', '-b', '0004', '-B'), + mock.call('efibootmgr', '-b', '0005', '-B'), + mock.call('umount', self.fake_dir + '/boot/efi', + attempts=3, delay_on_retry=True), + mock.call('sync')] + + self.agent_extension.install_bootloader( + root_uuid=self.fake_root_uuid, + efi_system_part_uuid=None) + + mock_dispatch.assert_any_call('get_os_install_device') + mock_dispatch.assert_any_call('get_boot_info') + 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_utils_efi_part.assert_called_once_with(self.fake_dev) + self.assertEqual(8, mock_execute.call_count) + + @mock.patch.object(os.path, 'exists', lambda *_: False) + @mock.patch.object(iscsi, 'clean_up', 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__add_multi_bootloaders( + self, mkdir_mock, mock_utils_efi_part, mock_partition, + mock_efi_bl, mock_iscsi_clean, mock_execute, mock_dispatch): + mock_dispatch.side_effect = [ + self.fake_dev, hardware.BootInfo(current_boot_mode='uefi') + ] + mock_partition.return_value = self.fake_dev + mock_utils_efi_part.return_value = '1' + mock_efi_bl.return_value = ['\\EFI\\BOOT\\BOOTX64.EFI', + '\\WINDOWS\\system32\\winload.efi'] + + mock_execute.side_effect = iter([('', ''), ('', ''), + ('', ''), ('', ''), + ('', ''), ('', ''), + ('', '')]) + + expected = [mock.call('efibootmgr', '--version'), + mock.call('mount', self.fake_efi_system_part, + self.fake_dir + '/boot/efi'), + mock.call('efibootmgr'), + mock.call('efibootmgr', '-c', '-d', self.fake_dev, + '-p', '1', '-w', + '-L', 'ironic1', '-l', + '\\EFI\\BOOT\\BOOTX64.EFI'), + mock.call('efibootmgr', '-c', '-d', self.fake_dev, + '-p', '1', '-w', + '-L', 'ironic2', '-l', + '\\WINDOWS\\system32\\winload.efi'), + mock.call('umount', self.fake_dir + '/boot/efi', + attempts=3, delay_on_retry=True), + mock.call('sync')] + + self.agent_extension.install_bootloader( + root_uuid=self.fake_root_uuid, + efi_system_part_uuid=None) + + mock_dispatch.assert_any_call('get_os_install_device') + mock_dispatch.assert_any_call('get_boot_info') + 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_utils_efi_part.assert_called_once_with(self.fake_dev) + self.assertEqual(7, mock_execute.call_count) + @mock.patch.object(iscsi, 'clean_up', autospec=True) @mock.patch.object(image, '_install_grub2', autospec=True) - def test_install_bootloader_prep(self, mock_grub2, mock_iscsi_clean, - mock_execute, mock_dispatch): - mock_dispatch.return_value = self.fake_dev + def test__install_bootloader_prep(self, mock_grub2, mock_iscsi_clean, + mock_execute, mock_dispatch): + mock_dispatch.side_effect = [ + self.fake_dev, hardware.BootInfo(current_boot_mode='bios') + ] self.agent_extension.install_bootloader( root_uuid=self.fake_root_uuid, efi_system_part_uuid=None, prep_boot_part_uuid=self.fake_prep_boot_part_uuid) - mock_dispatch.assert_called_once_with('get_os_install_device') + mock_dispatch.assert_any_call('get_os_install_device') + mock_dispatch.assert_any_call('get_boot_info') + self.assertEqual(2, mock_dispatch.call_count) mock_grub2.assert_called_once_with( self.fake_dev, root_uuid=self.fake_root_uuid, @@ -190,6 +392,7 @@ class TestImageExtension(base.IronicAgentTest): uuid=self.fake_prep_boot_part_uuid) self.assertFalse(mock_dispatch.called) + @mock.patch.object(os.path, 'exists', lambda *_: False) @mock.patch.object(image, '_is_bootloader_loaded', lambda *_: True) @mock.patch.object(hardware, 'is_md_device', autospec=True) @mock.patch.object(hardware, 'md_get_raid_devices', autospec=True) @@ -523,3 +726,160 @@ class TestImageExtension(base.IronicAgentTest): mock_execute.return_value = (parted_output, '') result = image._is_bootloader_loaded(self.fake_dev) self.assertFalse(result) + + @mock.patch.object(image, '_get_partition', autospec=True) + @mock.patch.object(utils, 'get_efi_part_on_device', autospec=True) + def test__manage_uefi_no_partition(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) + + @mock.patch.object(os.path, 'exists', lambda *_: False) + @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_utils_efi_part.return_value = '1' + mock_get_part_uuid.return_value = self.fake_dev + + mock_efi_bl.return_value = ['\\EFI\\BOOT\\BOOTX64.EFI'] + + mock_execute.side_effect = iter([('', ''), ('', ''), + ('', ''), ('', ''), + ('', '')]) + + expected = [mock.call('mount', self.fake_efi_system_part, + self.fake_dir + '/boot/efi'), + mock.call('efibootmgr'), + mock.call('efibootmgr', '-c', '-d', self.fake_dev, + '-p', '1', '-w', + '-L', 'ironic1', '-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, self.fake_root_uuid) + 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) + self.assertEqual(5, mock_execute.call_count) + + @mock.patch.object(os.path, 'exists', lambda *_: False) + @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_utils_efi_part.return_value = '1' + mock_get_part_uuid.side_effect = Exception + + mock_efi_bl.return_value = ['\\EFI\\BOOT\\BOOTX64.EFI'] + + mock_execute.side_effect = iter([('', ''), ('', ''), + ('', ''), ('', ''), + ('', '')]) + + expected = [mock.call('mount', self.fake_efi_system_part, + self.fake_dir + '/boot/efi'), + mock.call('efibootmgr'), + mock.call('efibootmgr', '-c', '-d', self.fake_dev, + '-p', '1', '-w', + '-L', 'ironic1', '-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) + self.assertEqual(5, mock_execute.call_count) + + @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, + mock_dispatch): + # No valid efi file. + mock_walk.return_value = [ + ('/boot/efi', ['EFI'], []), + ('/boot/efi/EFI', ['centos', 'BOOT'], []), + ('/boot/efi/EFI/centos', ['fw', 'fonts'], + ['shimx64-centos.efi', 'BOOT.CSV', 'BOOTX64.CSV', + 'MokManager.efi', 'mmx64.efi', 'shim.efi', 'fwupia32.efi', + 'fwupx64.efi', 'shimx64.efi', 'grubenv', 'grubx64.efi', + 'grub.cfg']), + ('/boot/efi/EFI/centos/fw', [], []), + ('/boot/efi/EFI/centos/fonts', [], ['unicode.pf2']), + ('/boot/efi/EFI/BOOT', [], []) + ] + + result = image._get_efi_bootloaders("/boot/efi") + self.assertEqual(result, []) + mock_access.assert_not_called() + + @mock.patch.object(os, 'walk', autospec=True) + @mock.patch.object(os, 'access', autospec=True) + def test__get_efi_bootloaders(self, mock_access, mock_walk, mock_execute, + mock_dispatch): + mock_walk.return_value = [ + ('/boot/efi', ['EFI'], []), + ('/boot/efi/EFI', ['centos', 'BOOT'], []), + ('/boot/efi/EFI/centos', ['fw', 'fonts'], + ['shimx64-centos.efi', 'BOOT.CSV', 'BOOTX64.CSV', + 'MokManager.efi', 'mmx64.efi', 'shim.efi', 'fwupia32.efi', + 'fwupx64.efi', 'shimx64.efi', 'grubenv', 'grubx64.efi', + 'grub.cfg']), + ('/boot/efi/EFI/centos/fw', [], []), + ('/boot/efi/EFI/centos/fonts', [], ['unicode.pf2']), + ('/boot/efi/EFI/BOOT', [], + ['BOOTX64.EFI', 'fallback.efi', 'fbx64.efi']) + ] + mock_access.return_value = True + result = image._get_efi_bootloaders("/boot/efi") + self.assertEqual(result[0], '\\EFI\\BOOT\\BOOTX64.EFI') + + @mock.patch.object(os, 'walk', autospec=True) + @mock.patch.object(os, 'access', autospec=True) + def test__get_windows_efi_bootloaders(self, mock_access, mock_walk, + mock_execute, mock_dispatch): + mock_walk.return_value = [ + ('/boot/efi', ['WINDOWS'], []), + ('/boot/efi/WINDOWS', ['system32'], []), + ('/boot/efi/WINDOWS/system32', [], + ['winload.efi']) + ] + mock_access.return_value = True + result = image._get_efi_bootloaders("/boot/efi") + self.assertEqual(result[0], '\\WINDOWS\\system32\\winload.efi') + + def test__run_efibootmgr_no_bootloaders(self, mock_execute, mock_dispatch): + result = image._run_efibootmgr([], self.fake_dev, + self.fake_efi_system_part) + expected = [] + self.assertIsNone(result) + mock_execute.assert_has_calls(expected) + + def test__run_efibootmgr(self, mock_execute, mock_dispatch): + result = image._run_efibootmgr(['\\EFI\\BOOT\\BOOTX64.EFI'], + self.fake_dev, + self.fake_efi_system_part) + expected = [mock.call('efibootmgr'), + mock.call('efibootmgr', '-c', '-d', self.fake_dev, + '-p', self.fake_efi_system_part, '-w', + '-L', 'ironic1', '-l', + '\\EFI\\BOOT\\BOOTX64.EFI')] + self.assertIsNone(result) + mock_execute.assert_has_calls(expected) diff --git a/ironic_python_agent/tests/unit/test_utils.py b/ironic_python_agent/tests/unit/test_utils.py index 94c3a31f3..8eb505efc 100644 --- a/ironic_python_agent/tests/unit/test_utils.py +++ b/ironic_python_agent/tests/unit/test_utils.py @@ -45,6 +45,18 @@ Number Start End Size File system Name Flags 1 116MB 2361MB 2245MB ext4 ''' +PARTED_OUTPUT_UNFORMATTED_NOFS = '''Model: whatever +Disk /dev/sda: 480GB +Sector size (logical/physical): 512B/512B +Partition Table: gpt +Disk Flags: + +Number Start End Size File system Name Flags +1 1049kB 9437kB 8389kB ESP boot, esp +2 9437kB 17.8MB 8389kB BSP bios_grub +3 17.8MB 40.0GB 40.0GB +4 479GB 480GB 68.1MB +''' PARTED_OUTPUT_NO_EFI = '''Model: whatever Disk /dev/sda: 450GB @@ -630,6 +642,18 @@ class TestUtils(testtools.TestCase): ) self.assertEqual('15', ret) + @mock.patch.object(utils, 'execute', autospec=True) + def test_get_efi_part_on_device_without_fs(self, mocked_execute): + parted_ret = PARTED_OUTPUT_UNFORMATTED_NOFS.format('gpt') + mocked_execute.side_effect = [ + (parted_ret, None) + ] + ret = utils.get_efi_part_on_device('/dev/sda') + mocked_execute.assert_has_calls( + [mock.call('parted', '-s', '/dev/sda', '--', 'print')] + ) + self.assertEqual('1', ret) + @mock.patch.object(utils, 'execute', autospec=True) def test_get_efi_part_on_device_not_found(self, mocked_execute): mocked_execute.side_effect = [ diff --git a/ironic_python_agent/utils.py b/ironic_python_agent/utils.py index d3eb08bd3..5f1c0dc2f 100644 --- a/ironic_python_agent/utils.py +++ b/ironic_python_agent/utils.py @@ -68,7 +68,7 @@ COLLECT_LOGS_COMMANDS = { DEVICE_EXTRACTOR = re.compile(r'^(?:(.*\d)p|(.*\D))(?:\d+)$') -PARTED_ESP_PATTERN = re.compile(r'^\s*(\d+)\s.*\sfat\d*\s.*esp(,|\s|$).*$') +PARTED_ESP_PATTERN = re.compile(r'^\s*(\d+)\s.*\s\s.*\s.*esp(,|\s|$).*$') def execute(*cmd, **kwargs): diff --git a/releasenotes/notes/avoid-grub2-using-efibootmgr-bd27c0978d1cf71b.yaml b/releasenotes/notes/avoid-grub2-using-efibootmgr-bd27c0978d1cf71b.yaml new file mode 100644 index 000000000..85193a741 --- /dev/null +++ b/releasenotes/notes/avoid-grub2-using-efibootmgr-bd27c0978d1cf71b.yaml @@ -0,0 +1,5 @@ +--- +fixes: + - | + Fixes the workflow for wholedisk images when using uefi boot mode, when + possible it will use efibootmgr instead of grub2 to update the nvram.