Merge "Option to enable bootloader config failure bypass"

4 files changed, 178 insertions, 9 deletions

diff --git a/ironic_python_agent/config.py b/ironic_python_agent/config.py
index bb28737d..9c774624 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 69ebcb91..de16e21e 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 1f3b48df..605120df 100644
--- a/ironic_python_agent/tests/unit/extensions/test_image.py
+++ b/ironic_python_agent/tests/unit/extensions/test_image.py
@@ -96,6 +96,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,
                                          mock_execute, mock_dispatch):
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 00000000..fe1590f4
--- /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.