From 88df78ef24b6eb0513e8fa66f5cdf91bc67a477a Mon Sep 17 00:00:00 2001 From: Jacob Anders Date: Thu, 13 Aug 2026 11:44:13 +1000 Subject: [PATCH] Fix periodic task processing a node via the wrong interface node_periodic matched a node's interface with isinstance(), so a periodic defined on a base interface also matched nodes whose interface is a subclass. When both a base and a subclass that inherits the periodic are enabled (e.g. redfish and idrac-redfish BIOS or RAID), each interface is collected as its own task and the base task also matched the subclass's nodes. The same node was then processed by two tasks, issuing duplicate resume RPCs that advanced past the next clean, service, or deploy step (silently skipping it). Fix: match the node's interface by exact type instead of isinstance. The two tasks now cover disjoint node sets -- the redfish task only matches RedfishBIOS nodes and the idrac-redfish task only matches DracRedfishBIOS nodes -- so no node is processed twice, and each node is handled by its own interface instance so subclass overrides apply. This keeps the existing one-task-per-interface-instance collection model untouched; the whole functional change is the match predicate. Assisted-By: Claude Opus 4.8 Change-Id: I26e66fce55cb013e66b6da3174d4ba6bb64a675e Signed-off-by: Jacob Anders (cherry picked from commit 5fe5a9b54227aed5c2210582d90967b04cf85595) (cherry picked from commit de95d9866e610cf7cb7a034e2518a108c52406dc) (cherry picked from commit f785d6485fa9da8f51060846b9da20ebe133275d) --- ironic/conductor/periodics.py | 14 +++++++- ironic/tests/unit/conductor/test_periodics.py | 35 +++++++++++++++++++ ...-periodic-task-dedup-26acfb9aa1424ff3.yaml | 24 +++++++++++++ 3 files changed, 72 insertions(+), 1 deletion(-) create mode 100644 releasenotes/notes/fix-inherited-periodic-task-dedup-26acfb9aa1424ff3.yaml 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.