openstack/ironic
Code Issues Proposed changes

[Refactor] Make caching BIOS settings explicit

Browse Source 
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
This commit is contained in:
Dmitry Tantsur 2019-02-06 18:57:38 +01:00
parent 0a8c9a25ed
commit 699bd410c7
6 changed files with 31 additions and 25 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

30
ironic/drivers/base.py
View File

@ -1004,31 +1004,21 @@ class InspectInterface(BaseInterface):
driver=task.node.driver, extension='abort')
class BIOSInterface(BaseInterface):
interface_type = 'bios'
def cache_bios_settings(func):
"""A decorator to cache bios settings after running the function.
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):
:param func: Function or method to wrap.
"""
@six.wraps(func)
def wrapped(task, *args, **kwargs):
result = func(task, *args, **kwargs)
instance.cache_bios_settings(task)
def wrapped(self, task, *args, **kwargs):
result = func(self, task, *args, **kwargs)
self.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
class BIOSInterface(BaseInterface):
interface_type = 'bios'
@abc.abstractmethod
def apply_configuration(self, task, settings):

2
ironic/drivers/modules/ilo/bios.py
View File

@ -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.

2
ironic/drivers/modules/irmc/bios.py
View File

@ -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.

2
ironic/drivers/modules/redfish/bios.py
View File

@ -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.

12
ironic/tests/unit/drivers/modules/ilo/test_bios.py
View File

@ -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)

2
ironic/tests/unit/drivers/test_base.py
View File

@ -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"