Clean out agent token even if power is already off

While investigating a very curious report, I discovered that
if somehow the power was *already* turned off to a node, say
through an incorrect BMC *or* human action, and Ironic were
to pick it up (as it does by default, because it checks before
applying the power state, then it would not wipe the token
information, preventing the agent from connecting on the next
action/attempt/operation.

We now remove the token on all calls to conductor
utilities node_power_action method when appropriate, even
if no other work is required.

Change-Id: Ie89e8be9ad2887467f277772445d4bef79fa5ea1

ironic/conductor/utils.py

@@ -297,6 +297,13 @@ def node_power_action(task, new_state, timeout=None):
     node = task.node
 
     if _can_skip_state_change(task, new_state):
+        # NOTE(TheJulia): Even if we are not changing the power state,
+        # we need to wipe the token out, just in case for some reason
+        # the power was turned off outside of our interaction/management.
+        if new_state in (states.POWER_OFF, states.SOFT_POWER_OFF,
+                         states.REBOOT, states.SOFT_REBOOT):
+            wipe_internal_info_on_power_off(node)
+            node.save()
         return
     target_state = _calculate_target_state(new_state)
 

ironic/tests/unit/conductor/test_utils.py

@@ -281,6 +281,31 @@ class NodePowerActionTestCase(db_base.DbTestCase):
         self.assertNotIn('agent_cached_deploy_steps',
                          node['driver_internal_info'])
 
+    @mock.patch.object(fake.FakePower, 'get_power_state', autospec=True)
+    def test_node_power_action_power_off_already(self, get_power_mock):
+        """Test node_power_action to turn node power off, but already off."""
+        dii = {'agent_secret_token': 'token',
+               'agent_cached_deploy_steps': ['steps']}
+        node = obj_utils.create_test_node(self.context,
+                                          uuid=uuidutils.generate_uuid(),
+                                          driver='fake-hardware',
+                                          power_state=states.POWER_ON,
+                                          driver_internal_info=dii)
+        task = task_manager.TaskManager(self.context, node.uuid)
+
+        get_power_mock.return_value = states.POWER_OFF
+
+        conductor_utils.node_power_action(task, states.POWER_OFF)
+
+        node.refresh()
+        get_power_mock.assert_called_once_with(mock.ANY, mock.ANY)
+        self.assertEqual(states.POWER_OFF, node['power_state'])
+        self.assertIsNone(node['target_power_state'])
+        self.assertIsNone(node['last_error'])
+        self.assertNotIn('agent_secret_token', node['driver_internal_info'])
+        self.assertNotIn('agent_cached_deploy_steps',
+                         node['driver_internal_info'])
+
     @mock.patch.object(fake.FakePower, 'get_power_state', autospec=True)
     def test_node_power_action_power_off_pregenerated_token(self,
                                                             get_power_mock):

releasenotes/notes/fix-power-off-token-wipe-e7d605997f00d39d.yaml

@@ -0,0 +1,6 @@
+---
+fixes:
+  - |
+    Fixes an issue where an agent token could be inadvertently orphaned
+    if a node is already in the target power state when we attempt to turn
+    the node off.