From 7a83773fbc9800217a0364c72b8ca9f823a60519 Mon Sep 17 00:00:00 2001 From: Julia Kreger Date: Tue, 24 Nov 2020 08:18:49 -0800 Subject: [PATCH] Option to enable bootloader config failure bypass Some hardware is very well intentioned. However this intention can result in the UEFI NVRAM table being full which prevents us from adding new records to the table. We can't be sure what to delete, so in this case some operators just need the ability to tell ironic "it is okay if this fails, it will still work." The added ``ignore_bootloader_failure`` option adds this capability which can be set per-node either in the agent configuation via the ramdisk image, or in the pxe_append_params configuration parameter for the node itself with a ``ipa-ignore-bootloader-failure`` option in order to prevent the failure from being raised. Change-Id: If3c83fb2ea2025fce092d495a64f32077c70d2d6 Story: 2008386 Task: 41309 --- ironic_python_agent/config.py | 12 ++ ironic_python_agent/extensions/image.py | 39 ++++-- .../tests/unit/extensions/test_image.py | 121 ++++++++++++++++++ ...ailure-to-be-ignored-b99667b13afa9759.yaml | 15 +++ 4 files changed, 178 insertions(+), 9 deletions(-) create mode 100644 releasenotes/notes/allows-bootloader-install-failure-to-be-ignored-b99667b13afa9759.yaml diff --git a/ironic_python_agent/config.py b/ironic_python_agent/config.py index bb28737d4..9c774624e 100644 --- a/ironic_python_agent/config.py +++ b/ironic_python_agent/config.py @@ -300,6 +300,18 @@ cli_opts = [ 'via lldp. If "all" is set then IPA should attempt ' 'to bring up all VLANs from lldp on all interfaces. ' 'By default, no VLANs will be brought up.'), + cfg.BoolOpt('ignore_bootloader_failure', + default=APARAMS.get('ipa-ignore-bootloader-failure'), + help='If the agent should ignore failures to install a ' + 'bootloader configuration into UEFI NVRAM. This ' + 'option should only be considered if the hardware ' + 'is automatically searching and adding UEFI ' + 'bootloaders from partitions. Use on a system ' + 'which is NOT doing this will likely cause the ' + 'deployment to fail. This setting should only be ' + 'used if you are absolutely sure of what you are ' + 'doing and that your hardware supports ' + 'such functionality. Hint: Most hardware does not.'), ] CONF.register_cli_opts(cli_opts) diff --git a/ironic_python_agent/extensions/image.py b/ironic_python_agent/extensions/image.py index 69ebcb912..de16e21e6 100644 --- a/ironic_python_agent/extensions/image.py +++ b/ironic_python_agent/extensions/image.py @@ -22,6 +22,7 @@ import tempfile from ironic_lib import utils as ilib_utils from oslo_concurrency import processutils +from oslo_config import cfg from oslo_log import log from ironic_python_agent import errors @@ -33,6 +34,7 @@ from ironic_python_agent import utils LOG = log.getLogger(__name__) +CONF = cfg.CONF BIND_MOUNTS = ('/dev', '/proc', '/run') @@ -712,7 +714,8 @@ class ImageExtension(base.BaseAgentExtension): @base.async_command('install_bootloader') def install_bootloader(self, root_uuid, efi_system_part_uuid=None, prep_boot_part_uuid=None, - target_boot_mode='bios'): + target_boot_mode='bios', + ignore_bootloader_failure=None): """Install the GRUB2 bootloader on the image. :param root_uuid: The UUID of the root partition. @@ -734,6 +737,13 @@ class ImageExtension(base.BaseAgentExtension): if self.agent.iscsi_started: iscsi.clean_up(device) + # Always allow the API client to be the final word on if this is + # overridden or not. + if ignore_bootloader_failure is None: + ignore_failure = CONF.ignore_bootloader_failure + else: + ignore_failure = ignore_bootloader_failure + boot = hardware.dispatch_to_managers('get_boot_info') if boot.current_boot_mode != target_boot_mode: LOG.warning('Boot mode mismatch: target boot mode is %(target)s, ' @@ -753,9 +763,15 @@ class ImageExtension(base.BaseAgentExtension): has_efibootmgr = False if has_efibootmgr: - if _manage_uefi(device, - efi_system_part_uuid=efi_system_part_uuid): - return + try: + if _manage_uefi( + device, + efi_system_part_uuid=efi_system_part_uuid): + return + except Exception as e: + LOG.error('Error setting up bootloader. Error %s', e) + if not ignore_failure: + raise # We don't have a working root UUID detection for whole disk images. # Until we can do it, avoid a confusing traceback. @@ -766,8 +782,13 @@ class ImageExtension(base.BaseAgentExtension): # 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, - prep_boot_part_uuid=prep_boot_part_uuid, - target_boot_mode=target_boot_mode) + try: + _install_grub2(device, + root_uuid=root_uuid, + efi_system_part_uuid=efi_system_part_uuid, + prep_boot_part_uuid=prep_boot_part_uuid, + target_boot_mode=target_boot_mode) + except Exception as e: + LOG.error('Error setting up bootloader. Error %s', e) + if not ignore_failure: + raise diff --git a/ironic_python_agent/tests/unit/extensions/test_image.py b/ironic_python_agent/tests/unit/extensions/test_image.py index 1f3b48dfc..605120df0 100644 --- a/ironic_python_agent/tests/unit/extensions/test_image.py +++ b/ironic_python_agent/tests/unit/extensions/test_image.py @@ -95,6 +95,127 @@ class TestImageExtension(base.IronicAgentTest): ) 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_ignores_manage_failure( + self, mock_grub2, mock_uefi, + mock_iscsi_clean, + mock_execute, mock_dispatch): + self.config(ignore_bootloader_failure=True) + mock_uefi.side_effect = OSError('meow') + 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, + target_boot_mode='uefi' + ).join() + 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=self.fake_efi_system_part_uuid, + prep_boot_part_uuid=None, + target_boot_mode='uefi' + ) + 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_ignores_grub_failure( + self, mock_grub2, mock_uefi, + mock_iscsi_clean, + mock_execute, mock_dispatch): + self.config(ignore_bootloader_failure=True) + mock_grub2.side_effect = OSError('meow') + 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, + target_boot_mode='uefi' + ).join() + 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=self.fake_efi_system_part_uuid, + prep_boot_part_uuid=None, + target_boot_mode='uefi' + ) + 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_ignores_grub_failure_api_override( + self, mock_grub2, mock_uefi, + mock_iscsi_clean, + mock_execute, mock_dispatch): + self.config(ignore_bootloader_failure=False) + mock_grub2.side_effect = OSError('meow') + 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, + target_boot_mode='uefi', ignore_bootloader_failure=True, + ).join() + 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=self.fake_efi_system_part_uuid, + prep_boot_part_uuid=None, + target_boot_mode='uefi' + ) + 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_grub_failure_api_override( + self, mock_grub2, mock_uefi, + mock_iscsi_clean, + mock_execute, mock_dispatch): + self.config(ignore_bootloader_failure=True) + mock_grub2.side_effect = OSError('meow') + mock_dispatch.side_effect = [ + self.fake_dev, hardware.BootInfo(current_boot_mode='uefi') + ] + mock_uefi.return_value = False + result = self.agent_extension.install_bootloader( + root_uuid=self.fake_root_uuid, + efi_system_part_uuid=self.fake_efi_system_part_uuid, + target_boot_mode='uefi', ignore_bootloader_failure=False, + ).join() + self.assertIsNotNone(result.command_error) + 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=self.fake_efi_system_part_uuid, + prep_boot_part_uuid=None, + target_boot_mode='uefi' + ) + mock_iscsi_clean.assert_called_once_with(self.fake_dev) + @mock.patch.object(iscsi, 'clean_up', autospec=True) @mock.patch.object(image, '_install_grub2', autospec=True) def test__install_bootloader_no_root(self, mock_grub2, mock_iscsi_clean, diff --git a/releasenotes/notes/allows-bootloader-install-failure-to-be-ignored-b99667b13afa9759.yaml b/releasenotes/notes/allows-bootloader-install-failure-to-be-ignored-b99667b13afa9759.yaml new file mode 100644 index 000000000..fe1590f46 --- /dev/null +++ b/releasenotes/notes/allows-bootloader-install-failure-to-be-ignored-b99667b13afa9759.yaml @@ -0,0 +1,15 @@ +--- +features: + - | + Adds an configuration option which can be encoded into the ramdisk itself + or the PXE parameters being provided to instruct the agent to ignore + bootloader installation or configuration failures. This functionality is + useful to work around well-intentioned hardware which is auto-populating + all possible device into the UEFI nvram firmware in order to try and help + ensure the machine boots. Except, this can also mean any + explict configuration attempt will fail. Operators needing this bypass + can use the ``ipa-ignore-bootloader-failure`` configuration option on the + PXE command line or utilize the ``ignore_bootloader_failure`` option + for the Ramdisk configuration. + In a future version of ironic, this setting may be able to be overriden + by ironic node level configuration.