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
(cherry picked from commit bcf6c12269168c5b4f0d9d4d3212e813f1827494)

3 files changed, 38 insertions, 0 deletions

diff --git a/ironic/conductor/utils.py b/ironic/conductor/utils.py
index cdf3a99ee..402ec2241 100644
--- a/ironic/conductor/utils.py
+++ b/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)
 
diff --git a/ironic/tests/unit/conductor/test_utils.py b/ironic/tests/unit/conductor/test_utils.py
index a29da21a7..563ac0b70 100644
--- a/ironic/tests/unit/conductor/test_utils.py
+++ b/ironic/tests/unit/conductor/test_utils.py
@@ -304,6 +304,31 @@ class NodePowerActionTestCase(db_base.DbTestCase):
                          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):
         dii = {'agent_secret_token': 'token',
diff --git a/releasenotes/notes/fix-power-off-token-wipe-e7d605997f00d39d.yaml b/releasenotes/notes/fix-power-off-token-wipe-e7d605997f00d39d.yaml
new file mode 100644
index 000000000..14a489b46
--- /dev/null
+++ b/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.