diff options
| -rw-r--r-- | ironic/conductor/cleaning.py | 25 | ||||
| -rw-r--r-- | ironic/conductor/deployments.py | 9 | ||||
| -rw-r--r-- | ironic/conductor/manager.py | 4 | ||||
| -rw-r--r-- | ironic/conductor/utils.py | 49 | ||||
| -rw-r--r-- | ironic/tests/unit/conductor/test_cleaning.py | 26 | ||||
| -rw-r--r-- | ironic/tests/unit/conductor/test_deployments.py | 55 | ||||
| -rw-r--r-- | ironic/tests/unit/conductor/test_manager.py | 5 | ||||
| -rw-r--r-- | ironic/tests/unit/conductor/test_utils.py | 4 | ||||
| -rw-r--r-- | releasenotes/notes/agent-token-817a03776bd46d5b.yaml | 6 |
9 files changed, 118 insertions, 65 deletions
diff --git a/ironic/conductor/cleaning.py b/ironic/conductor/cleaning.py index e02abdb36..9a923abf7 100644 --- a/ironic/conductor/cleaning.py +++ b/ironic/conductor/cleaning.py @@ -213,19 +213,7 @@ def do_next_clean_step(task, step_index): # Clear clean_step node.clean_step = None - driver_internal_info = node.driver_internal_info - driver_internal_info['clean_steps'] = None - driver_internal_info.pop('clean_step_index', None) - driver_internal_info.pop('cleaning_reboot', None) - driver_internal_info.pop('cleaning_polling', None) - - # Remove agent_url - if not utils.fast_track_able(task): - driver_internal_info.pop('agent_url', None) - driver_internal_info.pop('agent_secret_token', None) - driver_internal_info.pop('agent_secret_token_pregenerated', None) - - node.driver_internal_info = driver_internal_info + utils.wipe_cleaning_internal_info(task) node.save() try: task.driver.deploy.tear_down_cleaning(task) @@ -273,15 +261,6 @@ def do_node_clean_abort(task, step_name=None): node.last_error = last_error node.clean_step = None - info = node.driver_internal_info - # Clear any leftover metadata about cleaning - info.pop('clean_step_index', None) - info.pop('cleaning_reboot', None) - info.pop('cleaning_polling', None) - info.pop('skip_current_clean_step', None) - info.pop('agent_url', None) - info.pop('agent_secret_token', None) - info.pop('agent_secret_token_pregenerated', None) - node.driver_internal_info = info + utils.wipe_cleaning_internal_info(task) node.save() LOG.info(info_message) diff --git a/ironic/conductor/deployments.py b/ironic/conductor/deployments.py index 3bda75b23..75308482d 100644 --- a/ironic/conductor/deployments.py +++ b/ironic/conductor/deployments.py @@ -70,10 +70,6 @@ def start_deploy(task, manager, configdrive=None, event='deploy'): :param event: event to process: deploy or rebuild. """ node = task.node - # Record of any pre-existing agent_url should be removed - # except when we are in fast track conditions. - if not utils.is_fast_track(task): - utils.remove_agent_url(node) if event == 'rebuild': # Note(gilliard) Clear these to force the driver to @@ -127,8 +123,7 @@ def start_deploy(task, manager, configdrive=None, event='deploy'): def do_node_deploy(task, conductor_id=None, configdrive=None): """Prepare the environment and deploy a node.""" node = task.node - utils.wipe_deploy_internal_info(node) - utils.del_secret_token(node) + utils.wipe_deploy_internal_info(task) try: if configdrive: if isinstance(configdrive, dict): @@ -308,7 +303,7 @@ def do_next_deploy_step(task, step_index, conductor_id): # Finished executing the steps. Clear deploy_step. node.deploy_step = None - utils.wipe_deploy_internal_info(node) + utils.wipe_deploy_internal_info(task) node.save() _start_console_in_deploy(task) diff --git a/ironic/conductor/manager.py b/ironic/conductor/manager.py index b63d5412e..9cdf25558 100644 --- a/ironic/conductor/manager.py +++ b/ironic/conductor/manager.py @@ -1025,11 +1025,9 @@ class ConductorManager(base_manager.BaseConductorManager): # But we do need to clear the instance-related fields. node.instance_info = {} node.instance_uuid = None + utils.wipe_deploy_internal_info(task) driver_internal_info = node.driver_internal_info - driver_internal_info.pop('agent_secret_token', None) - driver_internal_info.pop('agent_secret_token_pregenerated', None) driver_internal_info.pop('instance', None) - driver_internal_info.pop('clean_steps', None) driver_internal_info.pop('root_uuid_or_disk_id', None) driver_internal_info.pop('is_whole_disk_image', None) driver_internal_info.pop('deploy_boot_mode', None) diff --git a/ironic/conductor/utils.py b/ironic/conductor/utils.py index 35e2c94fc..4b994db86 100644 --- a/ironic/conductor/utils.py +++ b/ironic/conductor/utils.py @@ -444,11 +444,15 @@ def cleaning_error_handler(task, msg, tear_down_cleaning=True, task.process_event('fail', target_state=target_state) -def wipe_deploy_internal_info(node): +def wipe_deploy_internal_info(task): """Remove temporary deployment fields from driver_internal_info.""" - info = node.driver_internal_info - info.pop('agent_secret_token', None) - info.pop('agent_secret_token_pregenerated', None) + 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) # Clear any leftover metadata about deployment. info['deploy_steps'] = None info.pop('agent_cached_deploy_steps', None) @@ -457,10 +461,24 @@ def wipe_deploy_internal_info(node): info.pop('deployment_polling', None) info.pop('skip_current_deploy_step', None) info.pop('steps_validated', None) - # Remove agent_url since it will be re-asserted - # upon the next deployment attempt. - info.pop('agent_url', None) - node.driver_internal_info = info + task.node.driver_internal_info = info + + +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) + info['clean_steps'] = None + info.pop('agent_cached_clean_steps', None) + info.pop('clean_step_index', None) + info.pop('cleaning_reboot', None) + info.pop('cleaning_polling', None) + info.pop('skip_current_clean_step', None) + info.pop('steps_validated', None) + task.node.driver_internal_info = info def deploying_error_handler(task, logmsg, errmsg=None, traceback=False, @@ -503,7 +521,7 @@ def deploying_error_handler(task, logmsg, errmsg=None, traceback=False, # Clear deploy step; we leave the list of deploy steps # in node.driver_internal_info for debugging purposes. node.deploy_step = {} - wipe_deploy_internal_info(node) + wipe_deploy_internal_info(task) if cleanup_err: node.last_error = cleanup_err @@ -1047,19 +1065,6 @@ def add_secret_token(node, pregenerated=False): node.driver_internal_info = i_info -def del_secret_token(node): - """Deletes the IPA agent secret token. - - Removes the agent token secret from the driver_internal_info field - from the Node object. - - :param node: Node object - """ - i_info = node.driver_internal_info - i_info.pop('agent_secret_token', None) - node.driver_internal_info = i_info - - def is_agent_token_present(node): """Determines if an agent token is present upon a node. diff --git a/ironic/tests/unit/conductor/test_cleaning.py b/ironic/tests/unit/conductor/test_cleaning.py index b6f67df92..6ed4bd270 100644 --- a/ironic/tests/unit/conductor/test_cleaning.py +++ b/ironic/tests/unit/conductor/test_cleaning.py @@ -531,11 +531,15 @@ class DoNodeCleanTestCase(db_base.DbTestCase): @mock.patch('ironic.drivers.modules.fake.FakeDeploy.execute_clean_step', autospec=True) def _do_next_clean_step_last_step_noop(self, mock_execute, manual=False, - retired=False): + retired=False, fast_track=False): + if fast_track: + self.config(fast_track=True, group='deploy') # Resume where last_step is the last cleaning step, should be noop tgt_prov_state = states.MANAGEABLE if manual else states.AVAILABLE info = {'clean_steps': self.clean_steps, - 'clean_step_index': len(self.clean_steps) - 1} + 'clean_step_index': len(self.clean_steps) - 1, + 'agent_url': 'test-url', + 'agent_secret_token': 'token'} node = obj_utils.create_test_node( self.context, driver='fake-hardware', @@ -563,6 +567,15 @@ class DoNodeCleanTestCase(db_base.DbTestCase): self.assertNotIn('clean_step_index', node.driver_internal_info) self.assertIsNone(node.driver_internal_info['clean_steps']) self.assertFalse(mock_execute.called) + if fast_track: + self.assertEqual('test-url', + node.driver_internal_info.get('agent_url')) + self.assertIsNotNone( + node.driver_internal_info.get('agent_secret_token')) + else: + self.assertNotIn('agent_url', node.driver_internal_info) + self.assertNotIn('agent_secret_token', + node.driver_internal_info) def test__do_next_clean_step_automated_last_step_noop(self): self._do_next_clean_step_last_step_noop() @@ -573,6 +586,9 @@ class DoNodeCleanTestCase(db_base.DbTestCase): def test__do_next_clean_step_retired_last_step_change_tgt_state(self): self._do_next_clean_step_last_step_noop(retired=True) + def test__do_next_clean_step_last_step_noop_fast_track(self): + self._do_next_clean_step_last_step_noop(fast_track=True) + @mock.patch('ironic.drivers.utils.collect_ramdisk_logs', autospec=True) @mock.patch('ironic.drivers.modules.fake.FakePower.execute_clean_step', autospec=True) @@ -989,6 +1005,8 @@ class DoNodeCleanAbortTestCase(db_base.DbTestCase): target_provision_state=states.AVAILABLE, clean_step={'step': 'foo', 'abortable': True}, driver_internal_info={ + 'agent_url': 'some url', + 'agent_secret_token': 'token', 'clean_step_index': 2, 'cleaning_reboot': True, 'cleaning_polling': True, @@ -1010,6 +1028,10 @@ class DoNodeCleanAbortTestCase(db_base.DbTestCase): task.node.driver_internal_info) self.assertNotIn('skip_current_clean_step', task.node.driver_internal_info) + self.assertNotIn('agent_url', + task.node.driver_internal_info) + self.assertNotIn('agent_secret_token', + task.node.driver_internal_info) def test__do_node_clean_abort(self): self._test__do_node_clean_abort(None) diff --git a/ironic/tests/unit/conductor/test_deployments.py b/ironic/tests/unit/conductor/test_deployments.py index 985cdcebc..8e8ccdea6 100644 --- a/ironic/tests/unit/conductor/test_deployments.py +++ b/ironic/tests/unit/conductor/test_deployments.py @@ -124,7 +124,9 @@ class DoNodeDeployTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase): @mock.patch.object(deployments, '_store_configdrive', autospec=True) def _test__do_node_deploy_ok(self, mock_store, configdrive=None, - expected_configdrive=None): + expected_configdrive=None, fast_track=False): + if fast_track: + self.config(fast_track=True, group='deploy') expected_configdrive = expected_configdrive or configdrive self._start_service() with mock.patch.object(fake.FakeDeploy, @@ -133,7 +135,9 @@ class DoNodeDeployTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase): self.node = obj_utils.create_test_node( self.context, driver='fake-hardware', name=None, provision_state=states.DEPLOYING, - target_provision_state=states.ACTIVE) + target_provision_state=states.ACTIVE, + driver_internal_info={'agent_url': 'url', + 'agent_secret_token': 'token'}) task = task_manager.TaskManager(self.context, self.node.uuid) deployments.do_node_deploy(task, self.service.conductor.id, @@ -148,6 +152,12 @@ class DoNodeDeployTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase): expected_configdrive) else: self.assertFalse(mock_store.called) + self.assertEqual( + fast_track, + bool(task.node.driver_internal_info.get('agent_url'))) + self.assertEqual( + fast_track, + bool(task.node.driver_internal_info.get('agent_secret_token'))) def test__do_node_deploy_ok(self): self._test__do_node_deploy_ok() @@ -156,6 +166,9 @@ class DoNodeDeployTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase): configdrive = 'foo' self._test__do_node_deploy_ok(configdrive=configdrive) + def test__do_node_deploy_fast_track(self): + self._test__do_node_deploy_ok(fast_track=True) + @mock.patch('openstack.baremetal.configdrive.build') def test__do_node_deploy_configdrive_as_dict(self, mock_cd): mock_cd.return_value = 'foo' @@ -495,7 +508,8 @@ class DoNextDeployStepTestCase(mgr_utils.ServiceSetUpMixin, # Run all steps from start to finish (all synchronous) driver_internal_info = {'deploy_step_index': None, 'deploy_steps': self.deploy_steps, - 'agent_url': 'url'} + 'agent_url': 'url', + 'agent_secret_token': 'token'} self._start_service() node = obj_utils.create_test_node( self.context, driver='fake-hardware', @@ -518,6 +532,41 @@ class DoNextDeployStepTestCase(mgr_utils.ServiceSetUpMixin, mock_execute.assert_has_calls = [mock.call(self.deploy_steps[0]), mock.call(self.deploy_steps[1])] self.assertNotIn('agent_url', node.driver_internal_info) + self.assertNotIn('agent_secret_token', node.driver_internal_info) + + @mock.patch('ironic.drivers.modules.fake.FakeDeploy.execute_deploy_step', + autospec=True) + def test__do_next_deploy_step_fast_track(self, mock_execute): + self.config(fast_track=True, group='deploy') + # Run all steps from start to finish (all synchronous) + driver_internal_info = {'deploy_step_index': None, + 'deploy_steps': self.deploy_steps, + 'agent_url': 'url', + 'agent_secret_token': 'token'} + self._start_service() + node = obj_utils.create_test_node( + self.context, driver='fake-hardware', + driver_internal_info=driver_internal_info, + deploy_step={}) + mock_execute.return_value = None + + task = task_manager.TaskManager(self.context, node.uuid) + task.process_event('deploy') + + deployments.do_next_deploy_step(task, 1, self.service.conductor.id) + + # Deploying should be complete + node.refresh() + self.assertEqual(states.ACTIVE, node.provision_state) + self.assertEqual(states.NOSTATE, node.target_provision_state) + self.assertEqual({}, node.deploy_step) + self.assertNotIn('deploy_step_index', node.driver_internal_info) + self.assertIsNone(node.driver_internal_info['deploy_steps']) + mock_execute.assert_has_calls = [mock.call(self.deploy_steps[0]), + mock.call(self.deploy_steps[1])] + self.assertEqual('url', node.driver_internal_info['agent_url']) + self.assertEqual('token', + node.driver_internal_info['agent_secret_token']) @mock.patch.object(conductor_utils, 'LOG', autospec=True) @mock.patch('ironic.drivers.modules.fake.FakeDeploy.execute_deploy_step', diff --git a/ironic/tests/unit/conductor/test_manager.py b/ironic/tests/unit/conductor/test_manager.py index 28a250254..6e142481d 100644 --- a/ironic/tests/unit/conductor/test_manager.py +++ b/ironic/tests/unit/conductor/test_manager.py @@ -1431,7 +1431,6 @@ class ServiceDoNodeDeployTestCase(mgr_utils.ServiceSetUpMixin, mock.ANY, None) mock_iwdi.assert_called_once_with(self.context, node.instance_info) self.assertFalse(node.driver_internal_info['is_whole_disk_image']) - self.assertNotIn('agent_url', node.driver_internal_info) def test_do_node_deploy_rebuild_active_state_error(self, mock_iwdi): # Tests manager.do_node_deploy() & deployments.do_next_deploy_step(), @@ -2042,7 +2041,7 @@ class DoNodeTearDownTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase): instance_info={'foo': 'bar'}, console_enabled=enabled_console, driver_internal_info={'is_whole_disk_image': False, - 'clean_steps': {}, + 'deploy_steps': {}, 'root_uuid_or_disk_id': 'foo', 'instance': {'ephemeral_gb': 10}}) port = obj_utils.create_test_port( @@ -2068,7 +2067,7 @@ class DoNodeTearDownTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase): self.assertIsNone(node.allocation_id) self.assertEqual({}, node.instance_info) self.assertNotIn('instance', node.driver_internal_info) - self.assertNotIn('clean_steps', node.driver_internal_info) + self.assertIsNone(node.driver_internal_info['deploy_steps']) self.assertNotIn('root_uuid_or_disk_id', node.driver_internal_info) self.assertNotIn('is_whole_disk_image', node.driver_internal_info) mock_tear_down.assert_called_once_with(task) diff --git a/ironic/tests/unit/conductor/test_utils.py b/ironic/tests/unit/conductor/test_utils.py index 7e3b8ce4a..e8368aaa9 100644 --- a/ironic/tests/unit/conductor/test_utils.py +++ b/ironic/tests/unit/conductor/test_utils.py @@ -2032,10 +2032,10 @@ class AgentTokenUtilsTestCase(tests_base.TestCase): conductor_utils.add_secret_token(self.node) self.assertIn('agent_secret_token', self.node.driver_internal_info) - def test_del_secret_token(self): + def test_wipe_deploy_internal_info(self): conductor_utils.add_secret_token(self.node) self.assertIn('agent_secret_token', self.node.driver_internal_info) - conductor_utils.del_secret_token(self.node) + conductor_utils.wipe_deploy_internal_info(mock.Mock(node=self.node)) self.assertNotIn('agent_secret_token', self.node.driver_internal_info) def test_is_agent_token_present(self): diff --git a/releasenotes/notes/agent-token-817a03776bd46d5b.yaml b/releasenotes/notes/agent-token-817a03776bd46d5b.yaml new file mode 100644 index 000000000..88071d67c --- /dev/null +++ b/releasenotes/notes/agent-token-817a03776bd46d5b.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - | + Fixes deployment in fast-track mode by keeping the required internal fields + (``agent_url`` and ``agent_secret_token``) intact when starting and + finishing deployment and cleaning. |