openstack
/
ironic
Code Issues Proposed changes

Handle conductor_affinity earlier in the deployment process 

Not sure why we do it only after the first deploy step, seems more
logical to it before running steps. Will simplify further refactoring.

Change-Id: Ia102c8697a9d23253c8b86fef71fc13ab9d68753
This commit is contained in:
Dmitry Tantsur 2020-09-25 14:31:22 +02:00
parent fb90ed41fe
commit 631fe9d8cc
4 changed files with 28 additions and 30 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
ironic/conductor/deployments.py
View File

@ -201,11 +201,17 @@ 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)
@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
@ -275,13 +281,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.

2
ironic/conductor/manager.py
View File

@ -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,

29
ironic/tests/unit/conductor/test_deployments.py
View File

@ -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()

10
ironic/tests/unit/conductor/test_manager.py
View File

@ -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)