diff --git a/ironic/conductor/periodics.py b/ironic/conductor/periodics.py index 105158faee..3c9db59072 100644 --- a/ironic/conductor/periodics.py +++ b/ironic/conductor/periodics.py @@ -143,7 +143,19 @@ def wrapper(self, *args, **kwargs): shared=shared_task) as task: if interface_type is not None: impl = getattr(task.driver, interface_type) - if not isinstance(impl, self.__class__): + # Match the node's interface by exact type + # rather than isinstance(). When a subclass + # (e.g. DracRedfishBIOS) inherits a periodic + # from its base (RedfishBIOS) and both are + # enabled, each interface is collected as its + # own task. An isinstance() check would let + # the base task also match subclass nodes, so + # the same node would be processed by two + # tasks (duplicate resume RPCs). Exact-type + # matching keeps the node sets disjoint and + # ensures each node is handled by its own + # implementation. + if type(impl) is not type(self): continue result = func(self, task, *args, **kwargs) diff --git a/ironic/tests/unit/conductor/test_periodics.py b/ironic/tests/unit/conductor/test_periodics.py index a960083305..218859d97f 100644 --- a/ironic/tests/unit/conductor/test_periodics.py +++ b/ironic/tests/unit/conductor/test_periodics.py @@ -75,6 +75,10 @@ def simple(self, task, manager, context): self.nodes.append(task.node.uuid) +class PeriodicTestSubInterface(PeriodicTestInterface): + """A subclass that inherits its parent's periodic unchanged.""" + + @mock.patch.object(PeriodicTestService, 'iter_nodes', autospec=True) class NodePeriodicTestCase(db_base.DbTestCase): @@ -182,3 +186,34 @@ def test_interface_check(self, mock_acquire, mock_iter_nodes): mock_iter_nodes.assert_called_once_with(self.service, filters=None, fields=()) self.assertEqual([self.uuid], iface.nodes) + + @mock.patch.object(task_manager, 'acquire', autospec=True) + def test_interface_check_exact_type(self, mock_acquire, mock_iter_nodes): + """Exact-type matching keeps base and subclass node sets disjoint. + + When both a base interface and a subclass that inherits its + periodic are enabled (e.g. RedfishBIOS and DracRedfishBIOS), + each is collected as its own task. The base-bound task must + not also process the subclass's nodes, otherwise the same node + is handled twice (duplicate resume RPCs). The subclass-bound + task must process them so subclass overrides apply. + """ + mock_iter_nodes.side_effect = [ + iter([(self.uuid, 'driver2', 'group')]), + iter([(self.uuid, 'driver2', 'group')]), + ] + base_iface = PeriodicTestInterface(self) + sub_iface = PeriodicTestSubInterface(self) + # The node's power interface is the subclass instance. + task = mock.Mock(spec=task_manager.TaskManager, + node=self.node, + driver=mock.Mock(power=sub_iface)) + mock_acquire.return_value.__enter__.return_value = task + + # The base-bound periodic must skip the subclass node... + base_iface.simple(self.service, self.context) + self.assertEqual([], base_iface.nodes) + + # ...while the subclass-bound periodic processes it. + sub_iface.simple(self.service, self.context) + self.assertEqual([self.uuid], sub_iface.nodes) diff --git a/releasenotes/notes/fix-inherited-periodic-task-dedup-26acfb9aa1424ff3.yaml b/releasenotes/notes/fix-inherited-periodic-task-dedup-26acfb9aa1424ff3.yaml new file mode 100644 index 0000000000..3b3cad9820 --- /dev/null +++ b/releasenotes/notes/fix-inherited-periodic-task-dedup-26acfb9aa1424ff3.yaml @@ -0,0 +1,24 @@ +--- +fixes: + - | + Fixes a bug where a clean, service, or deploy step immediately + following a BIOS settings change or RAID configuration could be + silently skipped. The problem occurs when both a base driver + interface and a subclass that inherits its periodic task are + enabled on a conductor, for example ``redfish`` and + ``idrac-redfish`` BIOS or RAID interfaces. Operators who observed + firmware update or RAID steps not executing after a BIOS apply, + factory reset, or RAID configuration were likely hitting this + issue. The root cause was that a periodic task's node match used + ``isinstance()``, so the base interface's task also matched nodes + whose interface was a subclass. With both interfaces enabled the + same node was processed by two tasks, issuing duplicate + ``resume`` RPCs that advanced past the next step. Periodic tasks + now match a node's interface by exact type. +upgrade: + - | + Periodic tasks defined on a driver interface now match a node's + interface by exact type rather than ``isinstance()``. Nodes using + a subclass interface (for example ``idrac-redfish``) are now + handled by their own interface instance instead of possibly the + parent interface's, so any subclass-specific overrides are applied.