From f4a7002023ddbd41505342bbad43fb6e0f963ef4 Mon Sep 17 00:00:00 2001 From: Dmitry Tantsur Date: Thu, 22 Apr 2021 13:54:34 +0200 Subject: [PATCH] Remove temporary cleaning information on starting cleaning Currently we only remove the URL, which may leave a stale token. Change-Id: I9ff2d726cb75317fe09bd43342541db0e721f2b8 --- ironic/conductor/cleaning.py | 3 +- ironic/conductor/manager.py | 5 --- ironic/tests/unit/conductor/test_cleaning.py | 13 ++++++-- ironic/tests/unit/conductor/test_manager.py | 31 ------------------- .../cleaning-token-9755f96d1284f78a.yaml | 4 +++ 5 files changed, 16 insertions(+), 40 deletions(-) create mode 100644 releasenotes/notes/cleaning-token-9755f96d1284f78a.yaml diff --git a/ironic/conductor/cleaning.py b/ironic/conductor/cleaning.py index 20a864e56b..4325616ba3 100644 --- a/ironic/conductor/cleaning.py +++ b/ironic/conductor/cleaning.py @@ -73,12 +73,13 @@ def do_node_clean(task, clean_steps=None, disable_ramdisk=False): {'node': node.uuid, 'msg': e}) return utils.cleaning_error_handler(task, msg) + utils.wipe_cleaning_internal_info(task) if manual_clean: info = node.driver_internal_info info['clean_steps'] = clean_steps info['cleaning_disable_ramdisk'] = disable_ramdisk node.driver_internal_info = info - node.save() + task.node.save() # Retrieve BIOS config settings for this node utils.node_cache_bios_settings(task, node) diff --git a/ironic/conductor/manager.py b/ironic/conductor/manager.py index 2daa73749f..fbe4dfa386 100644 --- a/ironic/conductor/manager.py +++ b/ironic/conductor/manager.py @@ -1068,11 +1068,6 @@ class ConductorManager(base_manager.BaseConductorManager): with task_manager.acquire(context, node_id, shared=False, purpose='node manual cleaning') as task: node = task.node - # Record of any pre-existing agent_url should be removed. - if not utils.is_fast_track(task): - # If clean->clean with an online agent, we should honor - # the operating agent and not prevent the action. - utils.remove_agent_url(node) if node.maintenance: raise exception.NodeInMaintenance(op=_('cleaning'), node=node.uuid) diff --git a/ironic/tests/unit/conductor/test_cleaning.py b/ironic/tests/unit/conductor/test_cleaning.py index 18d99d0049..65261450af 100644 --- a/ironic/tests/unit/conductor/test_cleaning.py +++ b/ironic/tests/unit/conductor/test_cleaning.py @@ -443,10 +443,16 @@ class DoNodeCleanTestCase(db_base.DbTestCase): disable_ramdisk=False): if clean_steps: tgt_prov_state = states.MANAGEABLE - driver_info = {} else: tgt_prov_state = states.AVAILABLE - driver_info = {'clean_steps': self.clean_steps} + + def set_steps(task, disable_ramdisk=None): + dii = task.node.driver_internal_info + dii['clean_steps'] = self.clean_steps + task.node.driver_internal_info = dii + task.node.save() + + mock_steps.side_effect = set_steps node = obj_utils.create_test_node( self.context, driver='fake-hardware', @@ -454,7 +460,7 @@ class DoNodeCleanTestCase(db_base.DbTestCase): target_provision_state=tgt_prov_state, last_error=None, power_state=states.POWER_OFF, - driver_internal_info=driver_info) + driver_internal_info={'agent_secret_token': 'old'}) with task_manager.acquire( self.context, node.uuid, shared=False) as task: @@ -477,6 +483,7 @@ class DoNodeCleanTestCase(db_base.DbTestCase): self.assertEqual(clean_steps, node.driver_internal_info['clean_steps']) self.assertFalse(node.maintenance) + self.assertNotIn('agent_secret_token', node.driver_internal_info) # Check that state didn't change self.assertEqual(states.CLEANING, node.provision_state) diff --git a/ironic/tests/unit/conductor/test_manager.py b/ironic/tests/unit/conductor/test_manager.py index 6f520df383..48c62d6137 100644 --- a/ironic/tests/unit/conductor/test_manager.py +++ b/ironic/tests/unit/conductor/test_manager.py @@ -2424,37 +2424,6 @@ class DoNodeCleanTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase): self.assertNotIn('clean_steps', node.driver_internal_info) self.assertIsNone(node.last_error) - @mock.patch('ironic.conductor.utils.remove_agent_url', autospec=True) - @mock.patch('ironic.conductor.utils.is_fast_track', autospec=True) - @mock.patch('ironic.conductor.manager.ConductorManager._spawn_worker', - autospec=True) - @mock.patch('ironic.drivers.modules.network.flat.FlatNetwork.validate', - autospec=True) - @mock.patch('ironic.drivers.modules.fake.FakePower.validate', - autospec=True) - def test_do_node_clean_ok_fast_track( - self, mock_power_valid, mock_network_valid, mock_spawn, - mock_is_fast_track, mock_remove_agent_url): - node = obj_utils.create_test_node( - self.context, driver='fake-hardware', - provision_state=states.MANAGEABLE, - driver_internal_info={'agent_url': 'meow'}) - mock_is_fast_track.return_value = True - self._start_service() - clean_steps = [self.deploy_raid] - self.service.do_node_clean(self.context, node.uuid, clean_steps) - mock_power_valid.assert_called_once_with(mock.ANY, mock.ANY) - mock_network_valid.assert_called_once_with(mock.ANY, mock.ANY) - mock_spawn.assert_called_with( - self.service, cleaning.do_node_clean, mock.ANY, clean_steps, False) - node.refresh() - # Node will be moved to CLEANING - self.assertEqual(states.CLEANING, node.provision_state) - self.assertEqual(states.MANAGEABLE, node.target_provision_state) - self.assertNotIn('clean_steps', node.driver_internal_info) - mock_is_fast_track.assert_called_once_with(mock.ANY) - mock_remove_agent_url.assert_not_called() - @mock.patch('ironic.conductor.manager.ConductorManager._spawn_worker', autospec=True) @mock.patch('ironic.drivers.modules.network.flat.FlatNetwork.validate', diff --git a/releasenotes/notes/cleaning-token-9755f96d1284f78a.yaml b/releasenotes/notes/cleaning-token-9755f96d1284f78a.yaml new file mode 100644 index 0000000000..f0c935d83d --- /dev/null +++ b/releasenotes/notes/cleaning-token-9755f96d1284f78a.yaml @@ -0,0 +1,4 @@ +--- +fixes: + - | + Removes temporary cleaning information on starting or restarting cleaning.