openstack
/
ironic
Code Issues Proposed changes

Merge "node_periodics: encapsulate the interface class check"

This commit is contained in:
Zuul 2021-10-14 12:21:27 +00:00 committed by Gerrit Code Review
parent b93dfd54c2 567aa056a2
commit 98896c515d
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
@ -1175,6 +1174,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()
@ -1186,31 +1186,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):
@ -1261,3 +1258,4 @@ class PXEBootRetryTestCase(db_base.DbTestCase):
class iPXEBootRetryTestCase(PXEBootRetryTestCase):
boot_interface = 'ipxe'
boot_interface_class = ipxe.iPXEBoot