[release-4.21] Fix periodic task processing a node via the wrong interface

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)

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)

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.