diff options
author | Dmitry Tantsur <dtantsur@protonmail.com> | 2020-08-04 09:39:37 +0200 |
---|---|---|
committer | Dmitry Tantsur <dtantsur@protonmail.com> | 2020-08-06 07:45:16 +0000 |
commit | a61bd37418a2834770bef9b8f0155a2b6ab02e33 (patch) | |
tree | c501f42adfbf7e69c95565039df7a2270e354756 | |
parent | a53bba694b49fbfd5e17f0aea8e889368ba02d43 (diff) | |
download | ironic-a61bd37418a2834770bef9b8f0155a2b6ab02e33.tar.gz |
Wipe agent token and URL on rescue and unrescue
Yet another place where we missed it :(
Change-Id: Iaa56e5965806e975ed0f97f2d6a0d15e13351c22
(cherry picked from commit 0b0ab9eb167c5a568141518e5a4698fc7f601d2b)
-rw-r--r-- | ironic/conductor/manager.py | 8 | ||||
-rw-r--r-- | ironic/conductor/utils.py | 25 | ||||
-rw-r--r-- | ironic/tests/unit/conductor/test_manager.py | 16 | ||||
-rw-r--r-- | releasenotes/notes/unrescue-token-ae664a17343e0610.yaml | 5 |
4 files changed, 39 insertions, 15 deletions
diff --git a/ironic/conductor/manager.py b/ironic/conductor/manager.py index ea8e3daea..6eb0cb03e 100644 --- a/ironic/conductor/manager.py +++ b/ironic/conductor/manager.py @@ -602,12 +602,13 @@ class ConductorManager(base_manager.BaseConductorManager): node_id, purpose='node rescue') as task: node = task.node - # Record of any pre-existing agent_url should be removed. - utils.remove_agent_url(node) if node.maintenance: raise exception.NodeInMaintenance(op=_('rescuing'), node=node.uuid) + # Record of any pre-existing agent_url should be removed. + utils.wipe_token_and_url(task) + # driver validation may check rescue_password, so save it on the # node early i_info = node.instance_info @@ -754,6 +755,9 @@ class ConductorManager(base_manager.BaseConductorManager): handle_failure(e, _('Failed to unrescue. Exception: %s'), log_func=LOG.exception) + + utils.wipe_token_and_url(task) + if next_state == states.ACTIVE: task.process_event('done') else: diff --git a/ironic/conductor/utils.py b/ironic/conductor/utils.py index 09fe3ef78..093c06115 100644 --- a/ironic/conductor/utils.py +++ b/ironic/conductor/utils.py @@ -444,16 +444,23 @@ def cleaning_error_handler(task, msg, tear_down_cleaning=True, task.process_event('fail', target_state=target_state) +def wipe_token_and_url(task): + """Remove agent URL and token from the task.""" + info = task.node.driver_internal_info + info.pop('agent_secret_token', None) + info.pop('agent_secret_token_pregenerated', None) + # Remove agent_url since it will be re-asserted + # upon the next deployment attempt. + info.pop('agent_url', None) + task.node.driver_internal_info = info + + def wipe_deploy_internal_info(task): """Remove temporary deployment fields from driver_internal_info.""" - info = task.node.driver_internal_info if not fast_track_able(task): - info.pop('agent_secret_token', None) - info.pop('agent_secret_token_pregenerated', None) - # Remove agent_url since it will be re-asserted - # upon the next deployment attempt. - info.pop('agent_url', None) + wipe_token_and_url(task) # Clear any leftover metadata about deployment. + info = task.node.driver_internal_info info['deploy_steps'] = None info.pop('agent_cached_deploy_steps', None) info.pop('deploy_step_index', None) @@ -466,11 +473,9 @@ def wipe_deploy_internal_info(task): def wipe_cleaning_internal_info(task): """Remove temporary cleaning fields from driver_internal_info.""" - info = task.node.driver_internal_info if not fast_track_able(task): - info.pop('agent_url', None) - info.pop('agent_secret_token', None) - info.pop('agent_secret_token_pregenerated', None) + wipe_token_and_url(task) + info = task.node.driver_internal_info info['clean_steps'] = None info.pop('agent_cached_clean_steps', None) info.pop('clean_step_index', None) diff --git a/ironic/tests/unit/conductor/test_manager.py b/ironic/tests/unit/conductor/test_manager.py index 06cf9bd02..090cd2851 100644 --- a/ironic/tests/unit/conductor/test_manager.py +++ b/ironic/tests/unit/conductor/test_manager.py @@ -2673,11 +2673,14 @@ class DoNodeRescueTestCase(mgr_utils.CommonMixIn, mgr_utils.ServiceSetUpMixin, @mock.patch('ironic.conductor.task_manager.acquire', autospec=True) def test_do_node_rescue(self, mock_acquire): self._start_service() + dii = {'agent_secret_token': 'token', + 'agent_url': 'http://url', + 'other field': 'value'} task = self._create_task( node_attrs=dict(driver='fake-hardware', provision_state=states.ACTIVE, instance_info={}, - driver_internal_info={'agent_url': 'url'})) + driver_internal_info=dii)) mock_acquire.side_effect = self._get_acquire_side_effect(task) self.service.do_node_rescue(self.context, task.node.uuid, "password") @@ -2688,7 +2691,8 @@ class DoNodeRescueTestCase(mgr_utils.CommonMixIn, mgr_utils.ServiceSetUpMixin, err_handler=conductor_utils.spawn_rescue_error_handler) self.assertIn('rescue_password', task.node.instance_info) self.assertIn('hashed_rescue_password', task.node.instance_info) - self.assertNotIn('agent_url', task.node.driver_internal_info) + self.assertEqual({'other field': 'value'}, + task.node.driver_internal_info) def test_do_node_rescue_invalid_state(self): self._start_service() @@ -2886,16 +2890,22 @@ class DoNodeRescueTestCase(mgr_utils.CommonMixIn, mgr_utils.ServiceSetUpMixin, @mock.patch('ironic.drivers.modules.fake.FakeRescue.unrescue') def test__do_node_unrescue(self, mock_unrescue): self._start_service() + dii = {'agent_url': 'http://url', + 'agent_secret_token': 'token', + 'other field': 'value'} node = obj_utils.create_test_node(self.context, driver='fake-hardware', provision_state=states.UNRESCUING, target_provision_state=states.ACTIVE, - instance_info={}) + instance_info={}, + driver_internal_info=dii) with task_manager.TaskManager(self.context, node.uuid) as task: mock_unrescue.return_value = states.ACTIVE self.service._do_node_unrescue(task) node.refresh() self.assertEqual(states.ACTIVE, node.provision_state) self.assertEqual(states.NOSTATE, node.target_provision_state) + self.assertEqual({'other field': 'value'}, + node.driver_internal_info) @mock.patch.object(manager, 'LOG') @mock.patch('ironic.drivers.modules.fake.FakeRescue.unrescue') diff --git a/releasenotes/notes/unrescue-token-ae664a17343e0610.yaml b/releasenotes/notes/unrescue-token-ae664a17343e0610.yaml new file mode 100644 index 000000000..7ce3273e7 --- /dev/null +++ b/releasenotes/notes/unrescue-token-ae664a17343e0610.yaml @@ -0,0 +1,5 @@ +--- +fixes: + - | + Removes stale agent token on rescue and unrescue operations. Previously it + would cause subsequent rescue operations to fail. |