Merge "[Refactor] Make caching BIOS settings explicit"

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"