From c3955e68a43b513fd367887a30b419bc837b702b Mon Sep 17 00:00:00 2001
From: Julia Kreger <juliaashleykreger@gmail.com>
Date: Thu, 9 Jan 2020 08:12:25 -0800
Subject: [PATCH] Fix fast_track + agent_url update fix

In the case of fast_track, we do not power off agents
which allows for operators to perform a rapid transition,
such as those desired when performing a batch installation
of machines.

However, a security fix I1a5e3c2b34d45c06fb74e82d0f30735ce9041914
removed prior traces of an agent_url upon exiting cleaning.

So what was occuring:
- Agent was completing cleaning
- Conductor was removing 'agent_url' from driver internal info
- Agent was heartbeating, but no new agent_url was being recorded
  because how do we know if that is actually the agent.
- Deployment was started.
- Deployment fails upon next heartbeat as there is no 'agent_url'.

This patch places a conditional around the removal of the agent_url
value at the end of cleaning which allows nodes to transition as
through fast track as desired.

Change-Id: I2b5130a0845b327c2244d2d30a19842c46e1eed3
Story: 2007080
Task: 37991
---
 ironic/conductor/manager.py                    |  3 ++-
 ironic/tests/unit/conductor/test_manager.py    | 18 +++++++++++++++---
 ...ailure-with-fasttrack-f1fe05598fbdbe4a.yaml |  9 +++++++++
 3 files changed, 26 insertions(+), 4 deletions(-)
 create mode 100644 releasenotes/notes/fixes-deployment-failure-with-fasttrack-f1fe05598fbdbe4a.yaml

diff --git a/ironic/conductor/manager.py b/ironic/conductor/manager.py
index 8b4ab8deaa..8cc5a1c0b3 100644
--- a/ironic/conductor/manager.py
+++ b/ironic/conductor/manager.py
@@ -1480,7 +1480,8 @@ class ConductorManager(base_manager.BaseConductorManager):
         driver_internal_info.pop('cleaning_reboot', None)
         driver_internal_info.pop('cleaning_polling', None)
         # Remove agent_url
-        driver_internal_info.pop('agent_url', None)
+        if not utils.fast_track_able(task):
+            driver_internal_info.pop('agent_url', None)
         node.driver_internal_info = driver_internal_info
         node.save()
         try:
diff --git a/ironic/tests/unit/conductor/test_manager.py b/ironic/tests/unit/conductor/test_manager.py
index 301610b82c..f1ca254004 100644
--- a/ironic/tests/unit/conductor/test_manager.py
+++ b/ironic/tests/unit/conductor/test_manager.py
@@ -4490,10 +4490,14 @@ class DoNodeCleanTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase):
 
     @mock.patch('ironic.drivers.modules.fake.FakeDeploy.execute_clean_step',
                 autospec=True)
-    def _do_next_clean_step_no_steps(self, mock_execute, manual=False):
+    def _do_next_clean_step_no_steps(self, mock_execute, manual=False,
+                                     fast_track=False):
+        if fast_track:
+            self.config(fast_track=True, group='deploy')
 
-        for info in ({'clean_steps': None, 'clean_step_index': None},
-                     {'clean_steps': None}):
+        for info in ({'clean_steps': None, 'clean_step_index': None,
+                      'agent_url': 'test-url'},
+                     {'clean_steps': None, 'agent_url': 'test-url'}):
             # Resume where there are no steps, should be a noop
             tgt_prov_state = states.MANAGEABLE if manual else states.AVAILABLE
 
@@ -4520,6 +4524,11 @@ class DoNodeCleanTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase):
             self.assertEqual({}, node.clean_step)
             self.assertNotIn('clean_step_index', node.driver_internal_info)
             self.assertFalse(mock_execute.called)
+            if fast_track:
+                self.assertEqual('test-url',
+                                 node.driver_internal_info.get('agent_url'))
+            else:
+                self.assertNotIn('agent_url', node.driver_internal_info)
             mock_execute.reset_mock()
 
     def test__do_next_clean_step_automated_no_steps(self):
@@ -4528,6 +4537,9 @@ class DoNodeCleanTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase):
     def test__do_next_clean_step_manual_no_steps(self):
         self._do_next_clean_step_no_steps(manual=True)
 
+    def test__do_next_clean_step_fast_track(self):
+        self._do_next_clean_step_no_steps(fast_track=True)
+
     @mock.patch('ironic.drivers.modules.fake.FakePower.execute_clean_step',
                 autospec=True)
     @mock.patch('ironic.drivers.modules.fake.FakeDeploy.execute_clean_step',
diff --git a/releasenotes/notes/fixes-deployment-failure-with-fasttrack-f1fe05598fbdbe4a.yaml b/releasenotes/notes/fixes-deployment-failure-with-fasttrack-f1fe05598fbdbe4a.yaml
new file mode 100644
index 0000000000..b72b7abab1
--- /dev/null
+++ b/releasenotes/notes/fixes-deployment-failure-with-fasttrack-f1fe05598fbdbe4a.yaml
@@ -0,0 +1,9 @@
+---
+fixes:
+  - |
+    Fixes an issue with fasttrack where a recent security related change to
+    prevent the ``agent_url`` field from being updated in a node, to
+    functionally prevent fast_track from succeeding as the node would fail
+    with an exception indicating the ``agent_url`` could not be found.
+    The required ``agent_url`` value is now preserved when the fast track
+    feature is enabled as the running ramdisk is not shut down.