openstack/ironic
Code Issues Proposed changes

node_periodics: encapsulate the interface class check

Browse Source 
Change-Id: I887d4fe4836bc58b5605e950a4287f0d27a590cb
This commit is contained in:
Dmitry Tantsur 2021-10-11 18:30:20 +02:00
parent cf1b42ea3d
commit 567aa056a2
11 changed files with 75 additions and 62 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
doc/source/contributor/deploy-steps.rst
View File

@ -251,10 +251,6 @@ periodic task that operates on relevant nodes:
predicate=lambda n: n.driver_internal_info.get('in_my_action'),
)
def check_my_action(self, task, manager, context):
# Double-check that the node is managed by this interface
if not isinstance(task.driver.management, MyManagement):
return
if not needs_actions(): # insert your checks here
return

13
ironic/conductor/periodics.py
View File

@ -23,6 +23,7 @@ from oslo_log import log
from ironic.common import exception
from ironic.conductor import base_manager
from ironic.conductor import task_manager
from ironic.drivers import base as driver_base
LOG = log.getLogger(__name__)
@ -57,6 +58,9 @@ def node_periodic(purpose, spacing, enabled=True, filters=None,
* for conductor manager: ``(self, task, context)``
* for hardware interfaces: ``(self, task, manager, context)``.
When the periodic is running on a hardware interface, only tasks
using this interface are considered.
``NodeNotFound`` and ``NodeLocked`` exceptions are ignored. Raise ``Stop``
to abort the current iteration of the task and reschedule it.
@ -103,6 +107,10 @@ def node_periodic(purpose, spacing, enabled=True, filters=None,
manager = args[0]
context = args[1]
interface_type = (getattr(self, 'interface_type', None)
if isinstance(self, driver_base.BaseInterface)
else None)
if callable(limit):
local_limit = limit()
else:
@ -125,6 +133,11 @@ def node_periodic(purpose, spacing, enabled=True, filters=None,
with task_manager.acquire(context, node_uuid,
purpose=purpose,
shared=shared_task) as task:
if interface_type is not None:
impl = getattr(task.driver, interface_type)
if not isinstance(impl, self.__class__):
continue
result = func(self, task, *args, **kwargs)
except exception.NodeNotFound:
LOG.info("During %(action)s, node %(node)s was not found "

4
ironic/drivers/modules/drac/bios.py
View File

@ -167,10 +167,6 @@ class DracWSManBIOS(base.BIOSInterface):
:param context: context of the request, needed when acquiring
a lock on a node. For access control.
"""
# skip a node not being managed by idrac driver
if not isinstance(task.driver.bios, DracWSManBIOS):
return
# check bios_config_job_id exist & checks job is completed
if task.node.driver_internal_info.get("bios_config_job_ids"):
self._check_node_bios_jobs(task)

3
ironic/drivers/modules/drac/management.py
View File

@ -496,9 +496,6 @@ class DracRedfishManagement(redfish_management.RedfishManagement):
)
def _query_import_configuration_status(self, task, manager, context):
"""Period job to check import configuration task."""
if not isinstance(task.driver.management, DracRedfishManagement):
return
self._check_import_configuration_task(
task, task.node.driver_internal_info.get(
'import_task_monitor_url'))

6
ironic/drivers/modules/drac/raid.py
View File

@ -1497,9 +1497,6 @@ class DracRedfishRAID(redfish_raid.RedfishRAID):
)
def _query_raid_tasks_status(self, task, manager, context):
"""Periodic task to check the progress of running RAID tasks"""
if not isinstance(task.driver.raid, DracRedfishRAID):
return
self._check_raid_tasks_status(
task, task.node.driver_internal_info.get('raid_task_monitor_uris'))
@ -1757,9 +1754,6 @@ class DracWSManRAID(base.RAIDInterface):
)
def _query_raid_config_job_status(self, task, manager, context):
"""Periodic task to check the progress of running RAID config jobs."""
if not isinstance(task.driver.raid, DracWSManRAID):
return
self._check_node_raid_jobs(task)
@METRICS.timer('DracRAID._check_node_raid_jobs')

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

@ -443,8 +443,6 @@ class IRMCRAID(base.RAIDInterface):
"""Periodic tasks to check the progress of running RAID config."""
node = task.node
node_uuid = task.node.uuid
if not isinstance(task.driver.raid, IRMCRAID):
return
if task.node.target_raid_config is None:
return
task.upgrade_lock()

3
ironic/drivers/modules/pxe_base.py
View File

@ -470,9 +470,6 @@ class PXEBaseMixin(object):
self._check_boot_status(task)
def _check_boot_status(self, task):
if not isinstance(task.driver.boot, PXEBaseMixin):
return
if not _should_retry_boot(task.node):
return

12
ironic/drivers/modules/redfish/management.py
View File

@ -863,21 +863,16 @@ class RedfishManagement(base.ManagementInterface):
)
def _query_firmware_update_failed(self, task, manager, context):
"""Periodic job to check for failed firmware updates."""
if not isinstance(task.driver.management, RedfishManagement):
return
node = task.node
# A firmware update failed. Discard any remaining firmware
# updates so when the user takes the node out of
# maintenance mode, pending firmware updates do not
# automatically continue.
LOG.warning('Firmware update failed for node %(node)s. '
'Discarding remaining firmware updates.',
{'node': node.uuid})
{'node': task.node.uuid})
task.upgrade_lock()
self._clear_firmware_updates(node)
self._clear_firmware_updates(task.node)
@METRICS.timer('RedfishManagement._query_firmware_update_status')
@periodics.node_periodic(
@ -889,9 +884,6 @@ class RedfishManagement(base.ManagementInterface):
)
def _query_firmware_update_status(self, task, manager, context):
"""Periodic job to check firmware update tasks."""
if not isinstance(task.driver.management, RedfishManagement):
return
self._check_node_firmware_update(task)
@METRICS.timer('RedfishManagement._check_node_firmware_update')

12
ironic/drivers/modules/redfish/raid.py
View File

@ -1023,21 +1023,16 @@ class RedfishRAID(base.RAIDInterface):
)
def _query_raid_config_failed(self, task, manager, context):
"""Periodic job to check for failed RAID configuration."""
if not isinstance(task.driver.raid, RedfishRAID):
return
node = task.node
# A RAID config failed. Discard any remaining RAID
# configs so when the user takes the node out of
# maintenance mode, pending RAID configs do not
# automatically continue.
LOG.warning('RAID configuration failed for node %(node)s. '
'Discarding remaining RAID configurations.',
{'node': node.uuid})
{'node': task.node.uuid})
task.upgrade_lock()
self._clear_raid_configs(node)
self._clear_raid_configs(task.node)
@METRICS.timer('RedfishRAID._query_raid_config_status')
@periodics.node_periodic(
@ -1049,9 +1044,6 @@ class RedfishRAID(base.RAIDInterface):
)
def _query_raid_config_status(self, task, manager, context):
"""Periodic job to check RAID config tasks."""
if not isinstance(task.driver.raid, RedfishRAID):
return
self._check_node_raid_config(task)
def _get_error_messages(self, response):

40
ironic/tests/unit/conductor/test_periodics.py
View File

@ -18,6 +18,7 @@ from ironic.common import context as ironic_context
from ironic.conductor import base_manager
from ironic.conductor import periodics
from ironic.conductor import task_manager
from ironic.drivers.modules import fake
from ironic.tests.unit.db import base as db_base
from ironic.tests.unit.objects import utils as obj_utils
@ -59,6 +60,19 @@ class PeriodicTestService(base_manager.BaseConductorManager):
raise periodics.Stop()
class PeriodicTestInterface(fake.FakePower):
def __init__(self, test):
self.test = test
self.nodes = []
@periodics.node_periodic(purpose="herding cats", spacing=42)
def simple(self, task, manager, context):
self.test.assertIsInstance(manager, PeriodicTestService)
self.test.assertIsInstance(context, ironic_context.RequestContext)
self.nodes.append(task.node.uuid)
@mock.patch.object(PeriodicTestService, 'iter_nodes', autospec=True)
class NodePeriodicTestCase(db_base.DbTestCase):
@ -133,3 +147,29 @@ class NodePeriodicTestCase(db_base.DbTestCase):
mock_iter_nodes.assert_called_once_with(self.service,
filters=None, fields=())
self.assertEqual(['stop'], self.service.nodes)
@mock.patch.object(task_manager, 'acquire', autospec=True)
def test_interface_check(self, mock_acquire, mock_iter_nodes):
mock_iter_nodes.return_value = iter([
(uuidutils.generate_uuid(), 'driver1', ''),
(self.uuid, 'driver2', 'group'),
])
iface = PeriodicTestInterface(self)
tasks = [
mock.Mock(spec=task_manager.TaskManager,
# This will not match the subclass
driver=mock.Mock(power=fake.FakePower())),
mock.Mock(spec=task_manager.TaskManager,
node=self.node,
driver=mock.Mock(power=iface)),
]
mock_acquire.side_effect = [
mock.MagicMock(**{'__enter__.return_value': task})
for task in tasks
]
iface.simple(self.service, self.context)
mock_iter_nodes.assert_called_once_with(self.service,
filters=None, fields=())
self.assertEqual([self.uuid], iface.nodes)

38
ironic/tests/unit/drivers/modules/test_pxe.py
View File

@ -36,7 +36,6 @@ from ironic.drivers import base as drivers_base
from ironic.drivers.modules import agent_base
from ironic.drivers.modules import boot_mode_utils
from ironic.drivers.modules import deploy_utils
from ironic.drivers.modules import fake
from ironic.drivers.modules import ipxe
from ironic.drivers.modules import pxe
from ironic.drivers.modules import pxe_base
@ -1047,6 +1046,7 @@ class PXEValidateRescueTestCase(db_base.DbTestCase):
class PXEBootRetryTestCase(db_base.DbTestCase):
boot_interface = 'pxe'
boot_interface_class = pxe.PXEBoot
def setUp(self):
super(PXEBootRetryTestCase, self).setUp()
@ -1058,31 +1058,28 @@ class PXEBootRetryTestCase(db_base.DbTestCase):
boot_interface=self.boot_interface,
provision_state=states.DEPLOYWAIT)
@mock.patch.object(pxe.PXEBoot, '_check_boot_status', autospec=True)
def test_check_boot_timeouts(self, mock_check_status, mock_power,
mock_boot_dev):
def test_check_boot_timeouts(self, mock_power, mock_boot_dev):
def _side_effect(iface, task):
self.assertEqual(self.node.uuid, task.node.uuid)
mock_check_status.side_effect = _side_effect
fake_node = obj_utils.create_test_node(
self.context,
uuid=uuidutils.generate_uuid(),
driver='fake-hardware',
boot_interface='fake',
provision_state=states.DEPLOYWAIT)
manager = mock.Mock(spec=['iter_nodes'])
manager.iter_nodes.return_value = [
(uuidutils.generate_uuid(), 'fake-hardware', ''),
(self.node.uuid, self.node.driver, self.node.conductor_group)
(fake_node.uuid, 'fake-hardware', ''),
(self.node.uuid, self.node.driver, self.node.conductor_group),
]
iface = pxe.PXEBoot()
iface._check_boot_timeouts(manager, self.context)
mock_check_status.assert_called_once_with(iface, mock.ANY)
def test_check_boot_status_another_boot_interface(self, mock_power,
mock_boot_dev):
with task_manager.acquire(self.context, self.node.uuid,
shared=True) as task:
task.driver.boot = fake.FakeBoot()
pxe.PXEBoot()._check_boot_status(task)
self.assertTrue(task.shared)
self.assertFalse(mock_power.called)
self.assertFalse(mock_boot_dev.called)
with mock.patch.object(self.boot_interface_class, '_check_boot_status',
autospec=True) as mock_check_status:
mock_check_status.side_effect = _side_effect
iface = self.boot_interface_class()
iface._check_boot_timeouts(manager, self.context)
mock_check_status.assert_called_once_with(iface, mock.ANY)
def test_check_boot_status_recent_power_change(self, mock_power,
mock_boot_dev):
@ -1133,3 +1130,4 @@ class PXEBootRetryTestCase(db_base.DbTestCase):
class iPXEBootRetryTestCase(PXEBootRetryTestCase):
boot_interface = 'ipxe'
boot_interface_class = ipxe.iPXEBoot