Retrieve BIOS configuration when moving node to ``manageable``

When moving the node to ``manageable``, in addition to
``cleaning``, retrieve the BIOS configuration settings.  In the
case of ``manageable``, this may allow the settings to be used
when choosing which node to deploy.

Change-Id: Ic2b162f31d4a1465fcb61671e7f48b3d31de788c
Story: 2008326
Task: 41224

6 files changed, 68 insertions, 16 deletions

diff --git a/ironic/conductor/cleaning.py b/ironic/conductor/cleaning.py
index 269d63004..ed684ce6e 100644
--- a/ironic/conductor/cleaning.py
+++ b/ironic/conductor/cleaning.py
@@ -77,19 +77,8 @@ def do_node_clean(task, clean_steps=None):
         node.driver_internal_info = info
         node.save()
 
-    # Do caching of bios settings if supported by driver,
-    # this will be called for both manual and automated cleaning.
-    try:
-        task.driver.bios.cache_bios_settings(task)
-    except exception.UnsupportedDriverExtension:
-        LOG.warning('BIOS settings are not supported for node %s, '
-                    'skipping', task.node.uuid)
-    # TODO(zshi) remove this check when classic drivers are removed
-    except Exception:
-        msg = (_('Caching of bios settings failed on node %(node)s. '
-                 'Continuing with node cleaning.')
-               % {'node': node.uuid})
-        LOG.exception(msg)
+    # Retrieve BIOS config settings for this node
+    utils.node_cache_bios_settings(task, node)
 
     # Allow the deploy driver to set up the ramdisk again (necessary for
     # IPA cleaning)
diff --git a/ironic/conductor/manager.py b/ironic/conductor/manager.py
index 7fcccec04..35eee97b1 100644
--- a/ironic/conductor/manager.py
+++ b/ironic/conductor/manager.py
@@ -1169,6 +1169,9 @@ class ConductorManager(base_manager.BaseConductorManager):
                 LOG.error(error, exc_info=log_traceback)
 
         if error is None:
+            # Retrieve BIOS config settings for this node
+            utils.node_cache_bios_settings(task, node)
+
             if power_state != node.power_state:
                 old_power_state = node.power_state
                 node.power_state = power_state
@@ -1694,6 +1697,7 @@ class ConductorManager(base_manager.BaseConductorManager):
             # is called as part of the transition from ENROLL to MANAGEABLE
             # states. As such it is redundant to call here.
             self._do_takeover(task)
+
             LOG.info("Successfully adopted node %(node)s",
                      {'node': node.uuid})
             task.process_event('done')
diff --git a/ironic/conductor/utils.py b/ironic/conductor/utils.py
index 5830011fa..209092c1b 100644
--- a/ironic/conductor/utils.py
+++ b/ironic/conductor/utils.py
@@ -1313,3 +1313,19 @@ def store_agent_certificate(node, agent_verify_ca):
         LOG.debug('Saved the custom certificate for node %(node)s to %(file)s',
                   {'node': node.uuid, 'file': fname})
         return fname
+
+
+def node_cache_bios_settings(task, node):
+    """Do caching of bios settings if supported by driver"""
+    try:
+        LOG.debug('BF getting BIOS info for node %s',
+                  node.uuid)
+        task.driver.bios.cache_bios_settings(task)
+    except exception.UnsupportedDriverExtension:
+        LOG.warning('BIOS settings are not supported for node %s, '
+                    'skipping', node.uuid)
+    # TODO(zshi) remove this check when classic drivers are removed
+    except Exception:
+        msg = (_('Caching of bios settings failed on node %(node)s.')
+               % {'node': node.uuid})
+        LOG.exception(msg)
diff --git a/ironic/tests/unit/conductor/test_cleaning.py b/ironic/tests/unit/conductor/test_cleaning.py
index 8398b38e9..a079af408 100644
--- a/ironic/tests/unit/conductor/test_cleaning.py
+++ b/ironic/tests/unit/conductor/test_cleaning.py
@@ -22,6 +22,7 @@ from ironic.common import states
 from ironic.conductor import cleaning
 from ironic.conductor import steps as conductor_steps
 from ironic.conductor import task_manager
+from ironic.conductor import utils as conductor_utils
 from ironic.drivers.modules import fake
 from ironic.drivers.modules.network import flat as n_flat
 from ironic.tests.unit.db import base as db_base
@@ -85,7 +86,7 @@ class DoNodeCleanTestCase(db_base.DbTestCase):
     def test__do_node_clean_manual_network_validate_fail(self, mock_validate):
         self.__do_node_clean_validate_fail(mock_validate, clean_steps=[])
 
-    @mock.patch.object(cleaning, 'LOG', autospec=True)
+    @mock.patch.object(conductor_utils, 'LOG', autospec=True)
     @mock.patch.object(conductor_steps, 'set_node_cleaning_steps',
                        autospec=True)
     @mock.patch.object(cleaning, 'do_next_clean_step', autospec=True)
@@ -124,8 +125,7 @@ class DoNodeCleanTestCase(db_base.DbTestCase):
             mock_validate.assert_called_once_with(mock.ANY, task)
             if enable_exception:
                 mock_log.exception.assert_called_once_with(
-                    'Caching of bios settings failed on node {}. '
-                    'Continuing with node cleaning.'
+                    'Caching of bios settings failed on node {}.'
                     .format(node.uuid))
 
     def test__do_node_clean_manual_cache_bios(self):
diff --git a/ironic/tests/unit/conductor/test_manager.py b/ironic/tests/unit/conductor/test_manager.py
index 648e86c98..c1379c531 100644
--- a/ironic/tests/unit/conductor/test_manager.py
+++ b/ironic/tests/unit/conductor/test_manager.py
@@ -3033,6 +3033,42 @@ class DoNodeVerifyTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase):
         self.assertIsNone(node.target_provision_state)
         self.assertTrue(node.last_error)
 
+    @mock.patch.object(conductor_utils, 'LOG', autospec=True)
+    @mock.patch('ironic.drivers.modules.fake.FakePower.validate',
+                autospec=True)
+    @mock.patch('ironic.drivers.modules.fake.FakeBIOS.cache_bios_settings',
+                autospec=True)
+    def _test__do_node_cache_bios(self, mock_bios, mock_validate,
+                                  mock_log,
+                                  enable_unsupported=False,
+                                  enable_exception=False):
+        if enable_unsupported:
+            mock_bios.side_effect = exception.UnsupportedDriverExtension('')
+        elif enable_exception:
+            mock_bios.side_effect = exception.IronicException('test')
+        node = obj_utils.create_test_node(
+            self.context, driver='fake-hardware',
+            provision_state=states.VERIFYING,
+            target_provision_state=states.MANAGEABLE)
+        with task_manager.acquire(
+                self.context, node.uuid, shared=False) as task:
+            self.service._do_node_verify(task)
+            mock_bios.assert_called_once_with(mock.ANY, task)
+            mock_validate.assert_called_once_with(mock.ANY, task)
+            if enable_exception:
+                mock_log.exception.assert_called_once_with(
+                    'Caching of bios settings failed on node {}.'
+                    .format(node.uuid))
+
+    def test__do_node_cache_bios(self):
+        self._test__do_node_cache_bios()
+
+    def test__do_node_cache_bios_exception(self):
+        self._test__do_node_cache_bios(enable_exception=True)
+
+    def test__do_node_cache_bios_unsupported(self):
+        self._test__do_node_cache_bios(enable_unsupported=True)
+
 
 @mgr_utils.mock_record_keepalive
 class MiscTestCase(mgr_utils.ServiceSetUpMixin, mgr_utils.CommonMixIn,
diff --git a/releasenotes/notes/bios-config-manage-ccefd24054cc73ee.yaml b/releasenotes/notes/bios-config-manage-ccefd24054cc73ee.yaml
new file mode 100644
index 000000000..487a19799
--- /dev/null
+++ b/releasenotes/notes/bios-config-manage-ccefd24054cc73ee.yaml
@@ -0,0 +1,7 @@
+---
+features:
+  - |
+    Retrieves BIOS configuration settings when moving a node to ``manageable``.
+    This allows the settings to be used when choosing which node to deploy.
+    For more details, see
+    `story 2008326 <https://storyboard.openstack.org/#!/story/2008326>`_.