diff --git a/ironic/conductor/utils.py b/ironic/conductor/utils.py index b30fdee5e1..413cb0ce4a 100644 --- a/ironic/conductor/utils.py +++ b/ironic/conductor/utils.py @@ -284,14 +284,13 @@ def node_power_action(task, new_state, timeout=None): driver_internal_info = node.driver_internal_info driver_internal_info['last_power_state_change'] = str( timeutils.utcnow().isoformat()) + node.driver_internal_info = driver_internal_info # NOTE(dtantsur): wipe token on shutting down, otherwise a reboot in # fast-track (or an accidentally booted agent) will cause subsequent # actions to fail. if target_state in (states.POWER_OFF, states.SOFT_POWER_OFF, states.REBOOT, states.SOFT_REBOOT): - if not is_agent_token_pregenerated(node): - driver_internal_info.pop('agent_secret_token', False) - node.driver_internal_info = driver_internal_info + wipe_internal_info_on_power_off(node) node.save() # take power action @@ -451,6 +450,19 @@ def cleaning_error_handler(task, msg, tear_down_cleaning=True, task.process_event('fail', target_state=target_state) +def wipe_internal_info_on_power_off(node): + """Wipe information that should not survive reboot/power off.""" + driver_internal_info = node.driver_internal_info + if not is_agent_token_pregenerated(node): + # Wipe the token if it's not pre-generated, otherwise we'll refuse to + # generate it again for the newly booted agent. + driver_internal_info.pop('agent_secret_token', False) + # Wipe cached steps since they may change after reboot. + driver_internal_info.pop('agent_cached_deploy_steps', None) + driver_internal_info.pop('agent_cached_clean_steps', None) + node.driver_internal_info = driver_internal_info + + def wipe_token_and_url(task): """Remove agent URL and token from the task.""" info = task.node.driver_internal_info diff --git a/ironic/drivers/modules/agent.py b/ironic/drivers/modules/agent.py index 1466cb7650..8418c533ce 100644 --- a/ironic/drivers/modules/agent.py +++ b/ironic/drivers/modules/agent.py @@ -489,6 +489,9 @@ class AgentDeploy(AgentDeployMixin, agent_base.AgentBaseMixin, # immediately to the next deploy step. LOG.debug('Performing a fast track deployment for %(node)s.', {'node': task.node.uuid}) + # NOTE(dtantsur): while the node is up and heartbeating, we don't + # necessary have the deploy steps cached. Force a refresh here. + self.refresh_steps(task, 'deploy') elif task.driver.storage.should_write_image(task): # Check if the driver has already performed a reboot in a previous # deploy step. diff --git a/ironic/drivers/modules/iscsi_deploy.py b/ironic/drivers/modules/iscsi_deploy.py index 8a48431363..c1f15d990b 100644 --- a/ironic/drivers/modules/iscsi_deploy.py +++ b/ironic/drivers/modules/iscsi_deploy.py @@ -655,6 +655,9 @@ class ISCSIDeploy(agent_base.AgentDeployMixin, agent_base.AgentBaseMixin, {'node': task.node.uuid}) deploy_utils.cache_instance_image(task.context, node) check_image_size(task) + # NOTE(dtantsur): while the node is up and heartbeating, we don't + # necessary have the deploy steps cached. Force a refresh here. + self.refresh_steps(task, 'deploy') elif task.driver.storage.should_write_image(task): # Standard deploy process deploy_utils.cache_instance_image(task.context, node) diff --git a/ironic/tests/unit/conductor/test_utils.py b/ironic/tests/unit/conductor/test_utils.py index 0dea519e21..776a072179 100644 --- a/ironic/tests/unit/conductor/test_utils.py +++ b/ironic/tests/unit/conductor/test_utils.py @@ -225,7 +225,8 @@ class NodePowerActionTestCase(db_base.DbTestCase): @mock.patch.object(fake.FakePower, 'get_power_state', autospec=True) def test_node_power_action_power_off(self, get_power_mock): """Test node_power_action to turn node power off.""" - dii = {'agent_secret_token': 'token'} + dii = {'agent_secret_token': 'token', + 'agent_cached_deploy_steps': ['steps']} node = obj_utils.create_test_node(self.context, uuid=uuidutils.generate_uuid(), driver='fake-hardware', @@ -243,6 +244,8 @@ class NodePowerActionTestCase(db_base.DbTestCase): self.assertIsNone(node['target_power_state']) self.assertIsNone(node['last_error']) self.assertNotIn('agent_secret_token', node['driver_internal_info']) + self.assertNotIn('agent_cached_deploy_steps', + node['driver_internal_info']) @mock.patch.object(fake.FakePower, 'get_power_state', autospec=True) def test_node_power_action_power_off_pregenerated_token(self, diff --git a/ironic/tests/unit/drivers/modules/test_agent.py b/ironic/tests/unit/drivers/modules/test_agent.py index 7db79b066b..926f665240 100644 --- a/ironic/tests/unit/drivers/modules/test_agent.py +++ b/ironic/tests/unit/drivers/modules/test_agent.py @@ -467,13 +467,15 @@ class TestAgentDeploy(db_base.DbTestCase): self.assertIsNone(driver_return) self.assertFalse(mock_power.called) + @mock.patch.object(agent.AgentDeploy, 'refresh_steps', autospec=True) @mock.patch.object(agent_client.AgentClient, 'prepare_image', autospec=True) @mock.patch('ironic.conductor.utils.is_fast_track', autospec=True) @mock.patch.object(pxe.PXEBoot, 'prepare_instance', autospec=True) @mock.patch('ironic.conductor.utils.node_power_action', autospec=True) def test_deploy_fast_track(self, power_mock, mock_pxe_instance, - mock_is_fast_track, prepare_image_mock): + mock_is_fast_track, prepare_image_mock, + refresh_mock): mock_is_fast_track.return_value = True self.node.target_provision_state = states.ACTIVE self.node.provision_state = states.DEPLOYING @@ -488,6 +490,7 @@ class TestAgentDeploy(db_base.DbTestCase): self.assertEqual(states.DEPLOYING, task.node.provision_state) self.assertEqual(states.ACTIVE, task.node.target_provision_state) + refresh_mock.assert_called_once_with(self.driver, task, 'deploy') @mock.patch.object(noop_storage.NoopStorage, 'detach_volumes', autospec=True) diff --git a/ironic/tests/unit/drivers/modules/test_iscsi_deploy.py b/ironic/tests/unit/drivers/modules/test_iscsi_deploy.py index b6eb127717..1cd12feb00 100644 --- a/ironic/tests/unit/drivers/modules/test_iscsi_deploy.py +++ b/ironic/tests/unit/drivers/modules/test_iscsi_deploy.py @@ -855,6 +855,8 @@ class ISCSIDeployTestCase(db_base.DbTestCase): self.assertIsNone(ret) self.assertFalse(mock_node_power_action.called) + @mock.patch.object(iscsi_deploy.ISCSIDeploy, 'refresh_steps', + autospec=True) @mock.patch.object(iscsi_deploy, 'check_image_size', autospec=True) @mock.patch.object(deploy_utils, 'cache_instance_image', autospec=True) @mock.patch.object(iscsi_deploy.ISCSIDeploy, 'write_image', @@ -864,7 +866,8 @@ class ISCSIDeployTestCase(db_base.DbTestCase): @mock.patch('ironic.conductor.utils.node_power_action', autospec=True) def test_deploy_fast_track(self, power_mock, mock_pxe_instance, mock_is_fast_track, write_image_mock, - cache_image_mock, check_image_size_mock): + cache_image_mock, check_image_size_mock, + refresh_mock): mock_is_fast_track.return_value = True self.node.target_provision_state = states.ACTIVE self.node.provision_state = states.DEPLOYING @@ -885,6 +888,8 @@ class ISCSIDeployTestCase(db_base.DbTestCase): cache_image_mock.assert_called_with(mock.ANY, task.node) check_image_size_mock.assert_called_with(task) self.assertFalse(write_image_mock.called) + refresh_mock.assert_called_once_with(task.driver.deploy, + task, 'deploy') @mock.patch.object(noop_storage.NoopStorage, 'detach_volumes', autospec=True) diff --git a/releasenotes/notes/fast-track-steps-81bd79a2a91e1b30.yaml b/releasenotes/notes/fast-track-steps-81bd79a2a91e1b30.yaml new file mode 100644 index 0000000000..68e2ecce1c --- /dev/null +++ b/releasenotes/notes/fast-track-steps-81bd79a2a91e1b30.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - | + Fixes an issue that caused in-band deploy steps inserted before + ``write_image`` to be skipped when fast-track is used. + - | + Makes sure in-band deploy and clean steps are not cached across reboots.