IPA does not boot up after cleaning reboot for 'redfish' bios interface

The clean steps of 'redfish' bios interface does not set up deploy ramdisk
before initiating the cleaning reboot of bare metal.

Change-Id: I979358ead1e19f9b24f23117d0cba3479e33707b
Story: 2006217
Task: 35810

ironic/drivers/modules/redfish/bios.py

@@ -22,6 +22,7 @@ from ironic.common import states
 from ironic.conductor import task_manager
 from ironic.conductor import utils as manager_utils
 from ironic.drivers import base
+from ironic.drivers.modules import deploy_utils
 from ironic.drivers.modules.redfish import utils as redfish_utils
 from ironic import objects
 
@@ -97,19 +98,39 @@ class RedfishBIOS(base.BIOSInterface):
         :raises: RedfishError on an error from the Sushy library
         """
         system = redfish_utils.get_system(task.node)
-        LOG.debug('Factory reset BIOS settings for node %(node_uuid)s',
-                  {'node_uuid': task.node.uuid})
         try:
-            system.bios.reset_bios()
-        except sushy.exceptions.SushyError as e:
+            bios = system.bios
+        except sushy.exceptions.MissingAttributeError:
             error_msg = (_('Redfish BIOS factory reset failed for node '
-                           '%(node)s. Error: %(error)s') %
-                         {'node': task.node.uuid, 'error': e})
+                           '%s, because BIOS settings are not supported.') %
+                         task.node.uuid)
             LOG.error(error_msg)
             raise exception.RedfishError(error=error_msg)
 
-        self.post_reset(task)
-        self._set_cleaning_reboot(task)
+        node = task.node
+        info = node.driver_internal_info
+        reboot_requested = info.get('post_factory_reset_reboot_requested')
+        if not reboot_requested:
+            LOG.debug('Factory reset BIOS configuration for node %(node)s',
+                      {'node': node.uuid})
+            try:
+                bios.reset_bios()
+            except sushy.exceptions.SushyError as e:
+                error_msg = (_('Redfish BIOS factory reset failed for node '
+                               '%(node)s. Error: %(error)s') %
+                             {'node': node.uuid, 'error': e})
+                LOG.error(error_msg)
+                raise exception.RedfishError(error=error_msg)
+
+            self.post_reset(task)
+            self._set_cleaning_reboot(task)
+            return states.CLEANWAIT
+        else:
+            current_attrs = bios.attributes
+            LOG.debug('Post factory reset, BIOS configuration for node '
+                      '%(node_uuid)s: %(attrs)r',
+                      {'node_uuid': node.uuid, 'attrs': current_attrs})
+            self._clear_reboot_requested(task)
 
     @base.clean_step(priority=0, argsinfo={
         'settings': {
@@ -182,6 +203,8 @@ class RedfishBIOS(base.BIOSInterface):
 
         :param task: a TaskManager instance containing the node to act on.
         """
+        deploy_opts = deploy_utils.build_agent_options(task.node)
+        task.driver.boot.prepare_ramdisk(task, deploy_opts)
         self._reboot(task)
 
     def post_configuration(self, task, settings):
@@ -195,6 +218,8 @@ class RedfishBIOS(base.BIOSInterface):
         :param task: a TaskManager instance containing the node to act on.
         :param settings: a list of BIOS settings to be updated.
         """
+        deploy_opts = deploy_utils.build_agent_options(task.node)
+        task.driver.boot.prepare_ramdisk(task, deploy_opts)
         self._reboot(task)
 
     def get_properties(self):
@@ -252,7 +277,9 @@ class RedfishBIOS(base.BIOSInterface):
         :param task: a TaskManager instance containing the node to act on.
         """
         info = task.node.driver_internal_info
+        info['post_factory_reset_reboot_requested'] = True
         info['cleaning_reboot'] = True
+        info['skip_current_clean_step'] = False
         task.node.driver_internal_info = info
         task.node.save()
 
@@ -276,10 +303,9 @@ class RedfishBIOS(base.BIOSInterface):
         :param task: a TaskManager instance containing the node to act on.
         """
         info = task.node.driver_internal_info
-        if 'post_config_reboot_requested' in info:
-            del info['post_config_reboot_requested']
-        if 'requested_bios_attrs' in info:
-            del info['requested_bios_attrs']
+        info.pop('post_config_reboot_requested', None)
+        info.pop('post_factory_reset_reboot_requested', None)
+        info.pop('requested_bios_attrs', None)
         task.node.driver_internal_info = info
         task.node.save()
 

ironic/tests/unit/drivers/modules/redfish/test_bios.py

@@ -19,6 +19,8 @@ from ironic.common import exception
 from ironic.common import states
 from ironic.conductor import task_manager
 from ironic.conductor import utils as manager_utils
+from ironic.drivers.modules import deploy_utils
+from ironic.drivers.modules import pxe as pxe_boot
 from ironic.drivers.modules.redfish import bios as redfish_bios
 from ironic.drivers.modules.redfish import utils as redfish_utils
 from ironic import objects
@@ -158,9 +160,14 @@ class RedfishBiosTestCase(db_base.DbTestCase):
             mock_setting_list.delete.assert_called_once_with(
                 task.context, task.node.id, delete_names)
 
+    @mock.patch.object(pxe_boot.PXEBoot, 'prepare_ramdisk',
+                       spec_set=True, autospec=True)
+    @mock.patch.object(deploy_utils, 'build_agent_options', autospec=True)
     @mock.patch.object(redfish_utils, 'get_system', autospec=True)
     @mock.patch.object(manager_utils, 'node_power_action', autospec=True)
-    def test_factory_reset(self, mock_power_action, mock_get_system):
+    def test_factory_reset_step1(self, mock_power_action, mock_get_system,
+                                 mock_build_agent_options, mock_prepare):
+        mock_build_agent_options.return_value = {'a': 'b'}
         with task_manager.acquire(self.context, self.node.uuid,
                                   shared=False) as task:
             task.driver.bios.factory_reset(task)
@@ -168,6 +175,26 @@ class RedfishBiosTestCase(db_base.DbTestCase):
             mock_power_action.assert_called_once_with(task, states.REBOOT)
             bios = mock_get_system(task.node).bios
             bios.reset_bios.assert_called_once()
+            mock_build_agent_options.assert_called_once_with(task.node)
+            mock_prepare.assert_called_once_with(mock.ANY, task, {'a': 'b'})
+            info = task.node.driver_internal_info
+            self.assertTrue(
+                all(x in info for x in (
+                    'post_factory_reset_reboot_requested', 'cleaning_reboot',
+                    'skip_current_clean_step')))
+
+    @mock.patch.object(redfish_utils, 'get_system', autospec=True)
+    def test_factory_reset_step2(self, mock_get_system):
+        with task_manager.acquire(self.context, self.node.uuid,
+                                  shared=False) as task:
+            driver_internal_info = task.node.driver_internal_info
+            driver_internal_info['post_factory_reset_reboot_requested'] = True
+            task.node.driver_internal_info = driver_internal_info
+            task.node.save()
+            task.driver.bios.factory_reset(task)
+            mock_get_system.assert_called_with(task.node)
+            info = task.node.driver_internal_info
+            self.assertNotIn('post_factory_reset_reboot_requested', info)
 
     @mock.patch.object(redfish_utils, 'get_system', autospec=True)
     def test_factory_reset_fail(self, mock_get_system):
@@ -188,13 +215,19 @@ class RedfishBiosTestCase(db_base.DbTestCase):
                 exception.RedfishError, 'BIOS factory reset failed',
                 task.driver.bios.factory_reset, task)
 
+    @mock.patch.object(pxe_boot.PXEBoot, 'prepare_ramdisk',
+                       spec_set=True, autospec=True)
+    @mock.patch.object(deploy_utils, 'build_agent_options', autospec=True)
     @mock.patch.object(redfish_utils, 'get_system', autospec=True)
     @mock.patch.object(manager_utils, 'node_power_action', autospec=True)
     def test_apply_configuration_step1(self, mock_power_action,
-                                       mock_get_system):
+                                       mock_get_system,
+                                       mock_build_agent_options,
+                                       mock_prepare):
         settings = [{'name': 'ProcTurboMode', 'value': 'Disabled'},
                     {'name': 'NicBoot1', 'value': 'NetworkBoot'}]
         attributes = {s['name']: s['value'] for s in settings}
+        mock_build_agent_options.return_value = {'a': 'b'}
         with task_manager.acquire(self.context, self.node.uuid,
                                   shared=False) as task:
             task.driver.bios.apply_configuration(task, settings)
@@ -202,6 +235,13 @@ class RedfishBiosTestCase(db_base.DbTestCase):
             mock_power_action.assert_called_once_with(task, states.REBOOT)
             bios = mock_get_system(task.node).bios
             bios.set_attributes.assert_called_once_with(attributes)
+            mock_build_agent_options.assert_called_once_with(task.node)
+            mock_prepare.assert_called_once_with(mock.ANY, task, {'a': 'b'})
+            info = task.node.driver_internal_info
+            self.assertTrue(
+                all(x in info for x in (
+                    'post_config_reboot_requested', 'cleaning_reboot',
+                    'skip_current_clean_step')))
 
     @mock.patch.object(redfish_utils, 'get_system', autospec=True)
     def test_apply_configuration_step2(self, mock_get_system):
@@ -210,15 +250,16 @@ class RedfishBiosTestCase(db_base.DbTestCase):
         requested_attrs = {'ProcTurboMode': 'Enabled'}
         with task_manager.acquire(self.context, self.node.uuid,
                                   shared=False) as task:
-            task.node.driver_internal_info[
-                'post_config_reboot_requested'] = True
-            task.node.driver_internal_info[
-                'requested_bios_attrs'] = requested_attrs
-            task.driver.bios._clear_reboot_requested = mock.MagicMock()
+            driver_internal_info = task.node.driver_internal_info
+            driver_internal_info['post_config_reboot_requested'] = True
+            driver_internal_info['requested_bios_attrs'] = requested_attrs
+            task.node.driver_internal_info = driver_internal_info
+            task.node.save()
             task.driver.bios.apply_configuration(task, settings)
             mock_get_system.assert_called_with(task.node)
-            task.driver.bios._clear_reboot_requested\
-                .assert_called_once_with(task)
+            info = task.node.driver_internal_info
+            self.assertNotIn('post_config_reboot_requested', info)
+            self.assertNotIn('requested_bios_attrs', info)
 
     @mock.patch.object(redfish_utils, 'get_system', autospec=True)
     def test_apply_configuration_not_supported(self, mock_get_system):

releasenotes/notes/story-2006217-redfish-bios-cleaning-fails-fee32f04dd97cbd2.yaml

@@ -0,0 +1,6 @@
+---
+fixes:
+  - |
+    Fixes an issue where clean steps of ``redfish`` BIOS interface do not
+    boot up the IPA ramdisk after cleaning reboot. See `story 2006217
+    <https://storyboard.openstack.org/#!/story/2006217>`__ for details.