diff --git a/ironic/conductor/deployments.py b/ironic/conductor/deployments.py index 54e05a62cd..9fdfe84cc8 100644 --- a/ironic/conductor/deployments.py +++ b/ironic/conductor/deployments.py @@ -201,13 +201,19 @@ def do_node_deploy(task, conductor_id=None, configdrive=None): _("No deploy steps returned by the driver")) raise exception.InstanceDeployFailure(msg) - do_next_deploy_step(task, 0, conductor_id) + if conductor_id is not None: + # Update conductor_affinity to reference this conductor's ID + # since there may be local persistent state + node.conductor_affinity = conductor_id + node.save() + + do_next_deploy_step(task, 0) @utils.fail_on_error(utils.deploying_error_handler, _("Unexpected error when processing next deploy step")) @task_manager.require_exclusive_lock -def do_next_deploy_step(task, step_index, conductor_id): +def do_next_deploy_step(task, step_index): """Do deployment, starting from the specified deploy step. :param task: a TaskManager instance with an exclusive lock @@ -277,13 +283,6 @@ def do_next_deploy_step(task, step_index, conductor_id): 'the remaining deploy steps', task.node) return - if ind == 0: - # We've done the very first deploy step. - # Update conductor_affinity to reference this conductor's ID - # since there may be local persistent state - node.conductor_affinity = conductor_id - node.save() - # Check if the step is done or not. The step should return # states.DEPLOYWAIT if the step is still being executed, or # None if the step is done. diff --git a/ironic/conductor/manager.py b/ironic/conductor/manager.py index 4d5b0f608c..406f6945fb 100644 --- a/ironic/conductor/manager.py +++ b/ironic/conductor/manager.py @@ -928,7 +928,7 @@ class ConductorManager(base_manager.BaseConductorManager): task.spawn_after( self._spawn_worker, deployments.do_next_deploy_step, - task, next_step_index, self.conductor.id) + task, next_step_index) @METRICS.timer('ConductorManager.do_node_tear_down') @messaging.expected_exceptions(exception.NoFreeConductorWorker, diff --git a/ironic/tests/unit/conductor/test_deployments.py b/ironic/tests/unit/conductor/test_deployments.py index 395c643a1e..bafe3363f1 100644 --- a/ironic/tests/unit/conductor/test_deployments.py +++ b/ironic/tests/unit/conductor/test_deployments.py @@ -161,6 +161,8 @@ class DoNodeDeployTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase): self.assertEqual( fast_track, bool(task.node.driver_internal_info.get('agent_secret_token'))) + self.assertEqual(self.service.conductor.id, + task.node.conductor_affinity) def test__do_node_deploy_ok(self): self._test__do_node_deploy_ok() @@ -434,7 +436,7 @@ class DoNextDeployStepTestCase(mgr_utils.ServiceSetUpMixin, task = task_manager.TaskManager(self.context, node.uuid) task.process_event('deploy') - deployments.do_next_deploy_step(task, None, self.service.conductor.id) + deployments.do_next_deploy_step(task, None) node.refresh() self.assertEqual(states.ACTIVE, node.provision_state) @@ -455,14 +457,13 @@ class DoNextDeployStepTestCase(mgr_utils.ServiceSetUpMixin, task = task_manager.TaskManager(self.context, node.uuid) task.process_event('deploy') - deployments.do_next_deploy_step(task, 0, self.service.conductor.id) + deployments.do_next_deploy_step(task, 0) node.refresh() self.assertEqual(states.DEPLOYWAIT, node.provision_state) self.assertEqual(states.ACTIVE, node.target_provision_state) self.assertEqual(expected_first_step, node.deploy_step) self.assertEqual(0, node.driver_internal_info['deploy_step_index']) - self.assertEqual(self.service.conductor.id, node.conductor_affinity) mock_execute.assert_called_once_with(mock.ANY, task, self.deploy_steps[0]) @@ -487,7 +488,7 @@ class DoNextDeployStepTestCase(mgr_utils.ServiceSetUpMixin, task = task_manager.TaskManager(self.context, node.uuid) task.process_event('deploy') - deployments.do_next_deploy_step(task, 0, self.service.conductor.id) + deployments.do_next_deploy_step(task, 0) node.refresh() self.assertIsNone(node.last_error) @@ -495,7 +496,6 @@ class DoNextDeployStepTestCase(mgr_utils.ServiceSetUpMixin, self.assertEqual(states.ACTIVE, node.target_provision_state) self.assertEqual(expected_first_step, node.deploy_step) self.assertEqual(0, node.driver_internal_info['deploy_step_index']) - self.assertEqual(self.service.conductor.id, node.conductor_affinity) mock_execute.assert_called_once_with(mock.ANY, task, self.deploy_steps[0]) @@ -517,7 +517,7 @@ class DoNextDeployStepTestCase(mgr_utils.ServiceSetUpMixin, task = task_manager.TaskManager(self.context, node.uuid) task.process_event('resume') - deployments.do_next_deploy_step(task, 1, self.service.conductor.id) + deployments.do_next_deploy_step(task, 1) node.refresh() self.assertEqual(states.DEPLOYWAIT, node.provision_state) @@ -553,7 +553,7 @@ class DoNextDeployStepTestCase(mgr_utils.ServiceSetUpMixin, task = task_manager.TaskManager(self.context, node.uuid) task.process_event('resume') - deployments.do_next_deploy_step(task, None, self.service.conductor.id) + deployments.do_next_deploy_step(task, None) node.refresh() # Deploying should be complete without calling additional steps self.assertEqual(states.ACTIVE, node.provision_state) @@ -595,7 +595,7 @@ class DoNextDeployStepTestCase(mgr_utils.ServiceSetUpMixin, task = task_manager.TaskManager(self.context, node.uuid) task.process_event('deploy') - deployments.do_next_deploy_step(task, 0, self.service.conductor.id) + deployments.do_next_deploy_step(task, 0) # Deploying should be complete node.refresh() @@ -629,7 +629,7 @@ class DoNextDeployStepTestCase(mgr_utils.ServiceSetUpMixin, task = task_manager.TaskManager(self.context, node.uuid) task.process_event('deploy') - deployments.do_next_deploy_step(task, 0, self.service.conductor.id) + deployments.do_next_deploy_step(task, 0) # Deploying should be complete node.refresh() @@ -663,7 +663,7 @@ class DoNextDeployStepTestCase(mgr_utils.ServiceSetUpMixin, task = task_manager.TaskManager(self.context, node.uuid) task.process_event('deploy') - deployments.do_next_deploy_step(task, 0, self.service.conductor.id) + deployments.do_next_deploy_step(task, 0) # Make sure we go to DEPLOYFAIL, clear deploy_steps node.refresh() @@ -702,8 +702,7 @@ class DoNextDeployStepTestCase(mgr_utils.ServiceSetUpMixin, task = task_manager.TaskManager(self.context, node.uuid) task.process_event('deploy') - deployments.do_next_deploy_step(task, None, - self.service.conductor.id) + deployments.do_next_deploy_step(task, None) # Deploying should be complete without calling additional steps node.refresh() @@ -729,7 +728,7 @@ class DoNextDeployStepTestCase(mgr_utils.ServiceSetUpMixin, task = task_manager.TaskManager(self.context, node.uuid) task.process_event('deploy') - deployments.do_next_deploy_step(task, 0, self.service.conductor.id) + deployments.do_next_deploy_step(task, 0) # Make sure we go to DEPLOYFAIL, clear deploy_steps node.refresh() @@ -763,7 +762,7 @@ class DoNextDeployStepTestCase(mgr_utils.ServiceSetUpMixin, with task_manager.acquire( self.context, node.uuid, shared=False) as task: - deployments.do_next_deploy_step(task, 0, mock.ANY) + deployments.do_next_deploy_step(task, 0) self._stop_service() node.refresh() @@ -797,7 +796,7 @@ class DoNextDeployStepTestCase(mgr_utils.ServiceSetUpMixin, with task_manager.acquire( self.context, node.uuid, shared=False) as task: - deployments.do_next_deploy_step(task, 0, mock.ANY) + deployments.do_next_deploy_step(task, 0) self._stop_service() node.refresh() diff --git a/ironic/tests/unit/conductor/test_manager.py b/ironic/tests/unit/conductor/test_manager.py index 63a44b0a4d..485e73665b 100644 --- a/ironic/tests/unit/conductor/test_manager.py +++ b/ironic/tests/unit/conductor/test_manager.py @@ -1788,7 +1788,7 @@ class ContinueNodeDeployTestCase(mgr_utils.ServiceSetUpMixin, self.assertEqual(tgt_prv_state, node.target_provision_state) mock_spawn.assert_called_with(mock.ANY, deployments.do_next_deploy_step, - mock.ANY, 1, mock.ANY) + mock.ANY, 1) @mock.patch('ironic.drivers.modules.fake.FakeDeploy.get_deploy_steps', autospec=True) @@ -1818,7 +1818,7 @@ class ContinueNodeDeployTestCase(mgr_utils.ServiceSetUpMixin, self.assertEqual(new_steps, node.driver_internal_info['deploy_steps']) mock_spawn.assert_called_with(mock.ANY, deployments.do_next_deploy_step, - mock.ANY, 1, mock.ANY) + mock.ANY, 1) @mock.patch.object(task_manager.TaskManager, 'process_event', autospec=True) @@ -1848,7 +1848,7 @@ class ContinueNodeDeployTestCase(mgr_utils.ServiceSetUpMixin, self.assertEqual(tgt_prv_state, node.target_provision_state) mock_spawn.assert_called_with(mock.ANY, deployments.do_next_deploy_step, - mock.ANY, 1, mock.ANY) + mock.ANY, 1) self.assertFalse(mock_event.called) @mock.patch('ironic.conductor.manager.ConductorManager._spawn_worker', @@ -1877,7 +1877,7 @@ class ContinueNodeDeployTestCase(mgr_utils.ServiceSetUpMixin, expected_step_index = 0 mock_spawn.assert_called_with(mock.ANY, deployments.do_next_deploy_step, - mock.ANY, expected_step_index, mock.ANY) + mock.ANY, expected_step_index) def test_continue_node_deploy_skip_step(self): self._continue_node_deploy_skip_step() @@ -1905,7 +1905,7 @@ class ContinueNodeDeployTestCase(mgr_utils.ServiceSetUpMixin, self.assertNotIn('deployment_polling', node.driver_internal_info) mock_spawn.assert_called_with(mock.ANY, deployments.do_next_deploy_step, - mock.ANY, 1, mock.ANY) + mock.ANY, 1) @mock.patch.object(conductor_steps, 'validate_deploy_templates', autospec=True)