[Refactor] Make caching BIOS settings explicit

Currently we cache BIOS setting after calling apply_configuration or
factory_reset implicitly via a wrapper added in BIOSInterface.__new__.
It's confusing, and we did forget about this aspect when writing
unit tests for the new iLO BIOS interface. This results in unsufficient
mocking that breaks unit tests with proliantutils installed.

This patch moves caching BIOS settings to an explicit decorator and
fixes the mocking problem.

Change-Id: I704eccea484b36cb5056fdb64d3702738c22c678
Story: #2004953
Task: #29375

ironic/drivers/base.py

@@ -1004,32 +1004,22 @@ class InspectInterface(BaseInterface):
             driver=task.node.driver, extension='abort')
 
 
+def cache_bios_settings(func):
+    """A decorator to cache bios settings after running the function.
+
+    :param func: Function or method to wrap.
+    """
+    @six.wraps(func)
+    def wrapped(self, task, *args, **kwargs):
+        result = func(self, task, *args, **kwargs)
+        self.cache_bios_settings(task)
+        return result
+    return wrapped
+
+
 class BIOSInterface(BaseInterface):
     interface_type = 'bios'
 
-    def __new__(cls, *args, **kwargs):
-        # Wrap the apply_configuration and factory_reset into a decorator
-        # which call cache_bios_settings() to update the node's BIOS setting
-        # table after apply_configuration and factory_reset have finished.
-
-        super_new = super(BIOSInterface, cls).__new__
-        instance = super_new(cls, *args, **kwargs)
-
-        def wrapper(func):
-            @six.wraps(func)
-            def wrapped(task, *args, **kwargs):
-                result = func(task, *args, **kwargs)
-                instance.cache_bios_settings(task)
-                return result
-            return wrapped
-
-        for n, method in inspect.getmembers(instance, inspect.ismethod):
-            if n == "apply_configuration":
-                instance.apply_configuration = wrapper(method)
-            elif n == "factory_reset":
-                instance.factory_reset = wrapper(method)
-        return instance
-
     @abc.abstractmethod
     def apply_configuration(self, task, settings):
         """Validate & apply BIOS settings on the given node.

ironic/drivers/modules/ilo/bios.py

@@ -154,6 +154,7 @@ class IloBIOS(base.BIOSInterface):
             'required': True
         }
     })
+    @base.cache_bios_settings
     def apply_configuration(self, task, settings):
         """Applies the provided configuration on the node.
 
@@ -177,6 +178,7 @@ class IloBIOS(base.BIOSInterface):
 
     @METRICS.timer('IloBIOS.factory_reset')
     @base.clean_step(priority=0, abortable=False)
+    @base.cache_bios_settings
     def factory_reset(self, task):
         """Reset the BIOS settings to factory configuration.
 

ironic/drivers/modules/irmc/bios.py

@@ -61,6 +61,7 @@ class IRMCBIOS(base.BIOSInterface):
             'required': True
         }
     })
+    @base.cache_bios_settings
     def apply_configuration(self, task, settings):
         """Applies BIOS configuration on the given node.
 
@@ -98,6 +99,7 @@ class IRMCBIOS(base.BIOSInterface):
                 operation='Apply BIOS configuration', error=e)
 
     @METRICS.timer('IRMCBIOS.factory_reset')
+    @base.cache_bios_settings
     def factory_reset(self, task):
         """Reset BIOS configuration to factory default on the given node.
 

ironic/drivers/modules/redfish/bios.py

@@ -79,6 +79,7 @@ class RedfishBIOS(base.BIOSInterface):
                 task.context, node_id, delete_names)
 
     @base.clean_step(priority=0)
+    @base.cache_bios_settings
     def factory_reset(self, task):
         """Reset the BIOS settings of the node to the factory default.
 
@@ -110,6 +111,7 @@ class RedfishBIOS(base.BIOSInterface):
             'required': True
         }
     })
+    @base.cache_bios_settings
     def apply_configuration(self, task, settings):
         """Apply the BIOS settings to the node.
 

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

@@ -68,12 +68,15 @@ class IloBiosTestCase(test_common.BaseIloTest):
             called_method["name"].assert_called_once_with(
                 *called_method["args"])
 
+    @mock.patch.object(ilo_bios.IloBIOS, 'cache_bios_settings',
+                       autospec=True)
     @mock.patch.object(ilo_bios.IloBIOS, '_execute_post_boot_bios_step',
                        autospec=True)
     @mock.patch.object(ilo_bios.IloBIOS, '_execute_pre_boot_bios_step',
                        autospec=True)
     def test_apply_configuration_pre_boot(self, exe_pre_boot_mock,
-                                          exe_post_boot_mock):
+                                          exe_post_boot_mock,
+                                          cache_settings_mock):
         settings = [
             {
                 "name": "SET_A", "value": "VAL_A",
@@ -101,13 +104,17 @@ class IloBiosTestCase(test_common.BaseIloTest):
             exe_pre_boot_mock.assert_called_once_with(
                 task.driver.bios, task, 'apply_configuration', actual_settings)
             self.assertFalse(exe_post_boot_mock.called)
+            cache_settings_mock.assert_called_once_with(task.driver.bios, task)
 
+    @mock.patch.object(ilo_bios.IloBIOS, 'cache_bios_settings',
+                       autospec=True)
     @mock.patch.object(ilo_bios.IloBIOS, '_execute_post_boot_bios_step',
                        autospec=True)
     @mock.patch.object(ilo_bios.IloBIOS, '_execute_pre_boot_bios_step',
                        autospec=True)
     def test_apply_configuration_post_boot(self, exe_pre_boot_mock,
-                                           exe_post_boot_mock):
+                                           exe_post_boot_mock,
+                                           cache_settings_mock):
         settings = [
             {
                 "name": "SET_A", "value": "VAL_A",
@@ -133,6 +140,7 @@ class IloBiosTestCase(test_common.BaseIloTest):
             exe_post_boot_mock.assert_called_once_with(
                 task.driver.bios, task, 'apply_configuration')
             self.assertFalse(exe_pre_boot_mock.called)
+            cache_settings_mock.assert_called_once_with(task.driver.bios, task)
 
     @mock.patch.object(ilo_boot.IloVirtualMediaBoot, 'prepare_ramdisk',
                        spec_set=True, autospec=True)

ironic/tests/unit/drivers/test_base.py

@@ -539,9 +539,11 @@ class MyBIOSInterface(driver_base.BIOSInterface):
     def validate(self, task):
         pass
 
+    @driver_base.cache_bios_settings
     def apply_configuration(self, task, settings):
         return "return_value_apply_configuration"
 
+    @driver_base.cache_bios_settings
     def factory_reset(self, task):
         return "return_value_factory_reset"