During cleaning, use current node.driver_internal_info

When iterating over the clean steps, the code in the conductor
keeps a copy of the node's driver_internal_info and uses that
to update the node's copy. However, it is possible that the
node's driver_internal_info is modified after the copy is made
(e.g. if a driver's clean step method modifies/saves it).
This patch changes it to use the current node's driver_internal_info.

Story: #2002688
Task: #22513
Change-Id: I92ad67b58343e5c6c1cca916269f134a5c78dcc6

ironic/conductor/manager.py

@@ -1250,11 +1250,10 @@ class ConductorManager(base_manager.BaseConductorManager):
         # whereas for automated cleaning, it is AVAILABLE.
         manual_clean = node.target_provision_state == states.MANAGEABLE
 
-        driver_internal_info = node.driver_internal_info
         if step_index is None:
             steps = []
         else:
-            steps = driver_internal_info['clean_steps'][step_index:]
+            steps = node.driver_internal_info['clean_steps'][step_index:]
 
         LOG.info('Executing %(state)s on node %(node)s, remaining steps: '
                  '%(steps)s', {'node': node.uuid, 'steps': steps,
@@ -1265,6 +1264,7 @@ class ConductorManager(base_manager.BaseConductorManager):
             # Save which step we're about to start so we can restart
             # if necessary
             node.clean_step = step
+            driver_internal_info = node.driver_internal_info
             driver_internal_info['clean_step_index'] = step_index + ind
             node.driver_internal_info = driver_internal_info
             node.save()
@@ -1305,6 +1305,7 @@ class ConductorManager(base_manager.BaseConductorManager):
 
         # Clear clean_step
         node.clean_step = None
+        driver_internal_info = node.driver_internal_info
         driver_internal_info['clean_steps'] = None
         driver_internal_info.pop('clean_step_index', None)
         node.driver_internal_info = driver_internal_info

ironic/tests/unit/conductor/test_manager.py

@@ -2739,7 +2739,14 @@ class DoNodeCleanTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase):
             driver_internal_info={'clean_steps': self.clean_steps,
                                   'clean_step_index': None},
             clean_step={})
-        mock_deploy_execute.return_value = None
+
+        def fake_deploy(conductor_obj, task, step):
+            driver_internal_info = task.node.driver_internal_info
+            driver_internal_info['goober'] = 'test'
+            task.node.driver_internal_info = driver_internal_info
+            task.node.save()
+
+        mock_deploy_execute.side_effect = fake_deploy
         mock_power_execute.return_value = None
 
         with task_manager.acquire(
@@ -2754,6 +2761,7 @@ class DoNodeCleanTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase):
         self.assertEqual(states.NOSTATE, node.target_provision_state)
         self.assertEqual({}, node.clean_step)
         self.assertNotIn('clean_step_index', node.driver_internal_info)
+        self.assertEqual('test', node.driver_internal_info['goober'])
         self.assertIsNone(node.driver_internal_info['clean_steps'])
         mock_power_execute.assert_called_once_with(mock.ANY, mock.ANY,
                                                    self.clean_steps[1])

releasenotes/notes/use-current-node-driver_internal_info-5c11de8f2c2b2e87.yaml

@@ -0,0 +1,8 @@
+---
+fixes:
+  - |
+    During node cleaning, the conductor was using a cached copy of the node's
+    driver_internal_info field. It is possible that the copy is outdated,
+    which would cause issues with the state of the node. This has been fixed.
+    For more information, see `bug 2002688
+    <https://storyboard.openstack.org/#!/story/2002688>`_.