openstack/ironic
Code Issues Proposed changes

Fix fast_track + agent_url update fix

Browse Source 
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
This commit is contained in:
Julia Kreger 2020-01-09 08:12:25 -08:00
parent 9669948732
commit c3955e68a4
3 changed files with 26 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
ironic/conductor/manager.py
View File

@ -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:

18
ironic/tests/unit/conductor/test_manager.py
View File

@ -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',

9
releasenotes/notes/fixes-deployment-failure-with-fasttrack-f1fe05598fbdbe4a.yaml Normal file
View File

@ -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.