Refactoring: finish splitting do_node_deploy

This change splits ConductorManager.do_node_deploy into two new
functions: validate_node and start_deploy. The only small behavior
change is validating that an FSM action is possible before making
any changes (previously we did it in the very end, potentiall after
modifying a node).

Change-Id: I635a30264f25d758850fd57c0f07e7a68b332fe3
This commit is contained in:
Dmitry Tantsur 2020-02-12 15:15:30 +01:00
parent 0d0a8a6631
commit dc6f92c973
3 changed files with 92 additions and 66 deletions

View File

@ -21,7 +21,9 @@ from oslo_utils import excutils
from oslo_utils import versionutils
from ironic.common import exception
from ironic.common.glance_service import service_utils as glance_utils
from ironic.common.i18n import _
from ironic.common import images
from ironic.common import release_mappings as versions
from ironic.common import states
from ironic.common import swift
@ -41,6 +43,91 @@ METRICS = metrics_utils.get_metrics_logger(__name__)
_SEEN_NO_DEPLOY_STEP_DEPRECATIONS = set()
def validate_node(task, event='deploy'):
"""Validate that a node is suitable for deployment/rebuilding.
:param task: a TaskManager instance.
:param event: event to process: deploy or rebuild.
:raises: NodeInMaintenance, NodeProtected, InvalidStateRequested
"""
if task.node.maintenance:
raise exception.NodeInMaintenance(op=_('provisioning'),
node=task.node.uuid)
if event == 'rebuild' and task.node.protected:
raise exception.NodeProtected(node=task.node.uuid)
if not task.fsm.is_actionable_event(event):
raise exception.InvalidStateRequested(
action=event, node=task.node.uuid, state=task.node.provision_state)
@METRICS.timer('start_deploy')
@task_manager.require_exclusive_lock
def start_deploy(task, manager, configdrive=None, event='deploy'):
"""Start deployment or rebuilding on a node.
This function does not check the node suitability for deployment, it's left
up to the caller.
:param task: a TaskManager instance.
:param manager: a ConductorManager to run tasks on.
:param configdrive: a configdrive, if requested.
: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
# check whether they have been changed in glance
# NOTE(vdrok): If image_source is not from Glance we should
# not clear kernel and ramdisk as they're input manually
if glance_utils.is_glance_image(
node.instance_info.get('image_source')):
instance_info = node.instance_info
instance_info.pop('kernel', None)
instance_info.pop('ramdisk', None)
node.instance_info = instance_info
driver_internal_info = node.driver_internal_info
# Infer the image type to make sure the deploy driver
# validates only the necessary variables for different
# image types.
# NOTE(sirushtim): The iwdi variable can be None. It's up to
# the deploy driver to validate this.
iwdi = images.is_whole_disk_image(task.context, node.instance_info)
driver_internal_info['is_whole_disk_image'] = iwdi
node.driver_internal_info = driver_internal_info
node.save()
try:
task.driver.power.validate(task)
task.driver.deploy.validate(task)
utils.validate_instance_info_traits(task.node)
conductor_steps.validate_deploy_templates(task)
except exception.InvalidParameterValue as e:
raise exception.InstanceDeployFailure(
_("Failed to validate deploy or power info for node "
"%(node_uuid)s. Error: %(msg)s") %
{'node_uuid': node.uuid, 'msg': e}, code=e.code)
try:
task.process_event(
event,
callback=manager._spawn_worker,
call_args=(do_node_deploy, task,
manager.conductor.id, configdrive),
err_handler=utils.provisioning_error_handler)
except exception.InvalidState:
raise exception.InvalidStateRequested(
action=event, node=task.node.uuid,
state=task.node.provision_state)
@METRICS.timer('do_node_deploy')
@task_manager.require_exclusive_lock
def do_node_deploy(task, conductor_id=None, configdrive=None):

View File

@ -56,7 +56,6 @@ from oslo_utils import uuidutils
from ironic.common import driver_factory
from ironic.common import exception
from ironic.common import faults
from ironic.common.glance_service import service_utils as glance_utils
from ironic.common.i18n import _
from ironic.common import images
from ironic.common import network
@ -822,6 +821,7 @@ class ConductorManager(base_manager.BaseConductorManager):
:raises: NodeProtected if the node is protected.
"""
LOG.debug("RPC do_node_deploy called for node %s.", node_id)
event = 'rebuild' if rebuild else 'deploy'
# NOTE(comstud): If the _sync_power_states() periodic task happens
# to have locked this node, we'll fail to acquire the lock. The
@ -829,69 +829,8 @@ class ConductorManager(base_manager.BaseConductorManager):
# want to add retries or extra synchronization here.
with task_manager.acquire(context, node_id, shared=False,
purpose='node deployment') as task:
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 node.maintenance:
raise exception.NodeInMaintenance(op=_('provisioning'),
node=node.uuid)
if rebuild:
if node.protected:
raise exception.NodeProtected(node=node.uuid)
event = 'rebuild'
# Note(gilliard) Clear these to force the driver to
# check whether they have been changed in glance
# NOTE(vdrok): If image_source is not from Glance we should
# not clear kernel and ramdisk as they're input manually
if glance_utils.is_glance_image(
node.instance_info.get('image_source')):
instance_info = node.instance_info
instance_info.pop('kernel', None)
instance_info.pop('ramdisk', None)
node.instance_info = instance_info
else:
event = 'deploy'
driver_internal_info = node.driver_internal_info
# Infer the image type to make sure the deploy driver
# validates only the necessary variables for different
# image types.
# NOTE(sirushtim): The iwdi variable can be None. It's up to
# the deploy driver to validate this.
iwdi = images.is_whole_disk_image(context, node.instance_info)
driver_internal_info['is_whole_disk_image'] = iwdi
node.driver_internal_info = driver_internal_info
node.save()
try:
task.driver.power.validate(task)
task.driver.deploy.validate(task)
utils.validate_instance_info_traits(task.node)
conductor_steps.validate_deploy_templates(task)
except exception.InvalidParameterValue as e:
raise exception.InstanceDeployFailure(
_("Failed to validate deploy or power info for node "
"%(node_uuid)s. Error: %(msg)s") %
{'node_uuid': node.uuid, 'msg': e}, code=e.code)
LOG.debug("do_node_deploy Calling event: %(event)s for node: "
"%(node)s", {'event': event, 'node': node.uuid})
try:
task.process_event(
event,
callback=self._spawn_worker,
call_args=(deployments.do_node_deploy, task,
self.conductor.id, configdrive),
err_handler=utils.provisioning_error_handler)
except exception.InvalidState:
raise exception.InvalidStateRequested(
action=event, node=task.node.uuid,
state=task.node.provision_state)
deployments.validate_node(task, event=event)
deployments.start_deploy(task, self, configdrive, event=event)
@METRICS.timer('ConductorManager.continue_node_deploy')
def continue_node_deploy(self, context, node_id):

View File

@ -1341,7 +1341,7 @@ class ServiceDoNodeDeployTestCase(mgr_utils.ServiceSetUpMixin,
self.assertIsNone(node.last_error)
# Verify reservation has been cleared.
self.assertIsNone(node.reservation)
mock_iwdi.assert_called_once_with(self.context, node.instance_info)
self.assertFalse(mock_iwdi.called)
self.assertNotIn('is_whole_disk_image', node.driver_internal_info)
def test_do_node_deploy_maintenance(self, mock_iwdi):
@ -1761,7 +1761,7 @@ class ServiceDoNodeDeployTestCase(mgr_utils.ServiceSetUpMixin,
self.assertIsNone(node.last_error)
# Verify reservation has been cleared.
self.assertIsNone(node.reservation)
mock_iwdi.assert_called_once_with(self.context, node.instance_info)
self.assertFalse(mock_iwdi.called)
self.assertNotIn('is_whole_disk_image', node.driver_internal_info)
def test_do_node_deploy_rebuild_protected(self, mock_iwdi):