Delay validating deploy templates until we get all steps
In-band steps are not known before the agent is running. Change-Id: Ib99017cdb8936714856716fcd612b49caa692459 Story: #2006963
This commit is contained in:
parent
4795c4a8b8
commit
2e276459b9
|
@ -189,7 +189,7 @@ def do_node_deploy(task, conductor_id=None, configdrive=None):
|
|||
# This gets the deploy steps (if any) and puts them in the node's
|
||||
# driver_internal_info['deploy_steps']. In-band steps are skipped since
|
||||
# we know that an agent is not running yet.
|
||||
conductor_steps.set_node_deployment_steps(task)
|
||||
conductor_steps.set_node_deployment_steps(task, skip_missing=True)
|
||||
except exception.InstanceDeployFailure as e:
|
||||
with excutils.save_and_reraise_exception():
|
||||
utils.deploying_error_handler(
|
||||
|
|
|
@ -873,7 +873,7 @@ class ConductorManager(base_manager.BaseConductorManager):
|
|||
save_required = False
|
||||
info = node.driver_internal_info
|
||||
|
||||
# Agent is running, we're ready to validate the remaining steps
|
||||
# Agent is now running, we're ready to validate the remaining steps
|
||||
if not info.get('steps_validated'):
|
||||
conductor_steps.validate_deploy_templates(task)
|
||||
conductor_steps.set_node_deployment_steps(
|
||||
|
@ -1944,7 +1944,11 @@ class ConductorManager(base_manager.BaseConductorManager):
|
|||
iface.validate(task)
|
||||
if iface_name == 'deploy':
|
||||
utils.validate_instance_info_traits(task.node)
|
||||
conductor_steps.validate_deploy_templates(task)
|
||||
# NOTE(dtantsur): without the agent running we cannot
|
||||
# have the complete list of steps, so skip ones that we
|
||||
# don't know.
|
||||
conductor_steps.validate_deploy_templates(
|
||||
task, skip_missing=True)
|
||||
result = True
|
||||
except (exception.InvalidParameterValue,
|
||||
exception.UnsupportedDriverExtension) as e:
|
||||
|
|
|
@ -238,7 +238,7 @@ def _get_steps_from_deployment_templates(task, templates):
|
|||
return steps
|
||||
|
||||
|
||||
def _get_validated_steps_from_templates(task):
|
||||
def _get_validated_steps_from_templates(task, skip_missing=False):
|
||||
"""Return a list of validated deploy steps from deploy templates.
|
||||
|
||||
Deployment template steps are those steps defined in deployment templates
|
||||
|
@ -269,10 +269,11 @@ def _get_validated_steps_from_templates(task):
|
|||
'deploy templates: %(templates)s. Errors: ') %
|
||||
{'templates': ','.join(t.name for t in templates)})
|
||||
return _validate_user_deploy_steps(task, user_steps,
|
||||
error_prefix=error_prefix)
|
||||
error_prefix=error_prefix,
|
||||
skip_missing=skip_missing)
|
||||
|
||||
|
||||
def _get_all_deployment_steps(task):
|
||||
def _get_all_deployment_steps(task, skip_missing=False):
|
||||
"""Get deployment steps for task.node.
|
||||
|
||||
Deployment steps from matching deployment templates are combined with those
|
||||
|
@ -287,7 +288,8 @@ def _get_all_deployment_steps(task):
|
|||
# NOTE(mgoddard): although we've probably just validated the templates in
|
||||
# do_node_deploy, they may have changed in the DB since we last checked, so
|
||||
# validate again.
|
||||
user_steps = _get_validated_steps_from_templates(task)
|
||||
user_steps = _get_validated_steps_from_templates(task,
|
||||
skip_missing=skip_missing)
|
||||
|
||||
# Gather enabled deploy steps from drivers.
|
||||
driver_steps = _get_deployment_steps(task, enabled=True, sort=False)
|
||||
|
@ -304,7 +306,7 @@ def _get_all_deployment_steps(task):
|
|||
return _sorted_steps(steps, _deploy_step_key)
|
||||
|
||||
|
||||
def set_node_deployment_steps(task, reset_current=True):
|
||||
def set_node_deployment_steps(task, reset_current=True, skip_missing=False):
|
||||
"""Set up the node with deployment step information for deploying.
|
||||
|
||||
Get the deploy steps from the driver.
|
||||
|
@ -315,7 +317,8 @@ def set_node_deployment_steps(task, reset_current=True):
|
|||
"""
|
||||
node = task.node
|
||||
driver_internal_info = node.driver_internal_info
|
||||
driver_internal_info['deploy_steps'] = _get_all_deployment_steps(task)
|
||||
driver_internal_info['deploy_steps'] = _get_all_deployment_steps(
|
||||
task, skip_missing=skip_missing)
|
||||
if reset_current:
|
||||
node.deploy_step = {}
|
||||
driver_internal_info['deploy_step_index'] = None
|
||||
|
@ -463,7 +466,7 @@ def _validate_user_step(task, user_step, driver_step, step_type):
|
|||
|
||||
|
||||
def _validate_user_steps(task, user_steps, driver_steps, step_type,
|
||||
error_prefix=None):
|
||||
error_prefix=None, skip_missing=False):
|
||||
"""Validate the user-specified steps.
|
||||
|
||||
:param task: A TaskManager object
|
||||
|
@ -519,12 +522,22 @@ def _validate_user_steps(task, user_steps, driver_steps, step_type,
|
|||
# Convert driver steps to a dict.
|
||||
driver_steps = {_step_id(s): s for s in driver_steps}
|
||||
|
||||
result = []
|
||||
|
||||
for user_step in user_steps:
|
||||
# Check if this user-specified step isn't supported by the driver
|
||||
try:
|
||||
driver_step = driver_steps[_step_id(user_step)]
|
||||
except KeyError:
|
||||
error = (_('node does not support this %(type)s step: %(step)s')
|
||||
if skip_missing:
|
||||
LOG.debug('%(type)s step %(step)s is not currently known for '
|
||||
'node %(node)s, delaying its validation until '
|
||||
'in-band steps are loaded',
|
||||
{'type': step_type.capitalize(),
|
||||
'step': user_step, 'node': task.node.uuid})
|
||||
else:
|
||||
error = (_('node does not support this %(type)s step: '
|
||||
'%(step)s')
|
||||
% {'type': step_type, 'step': user_step})
|
||||
errors.append(error)
|
||||
continue
|
||||
|
@ -532,10 +545,11 @@ def _validate_user_steps(task, user_steps, driver_steps, step_type,
|
|||
step_errors = _validate_user_step(task, user_step, driver_step,
|
||||
step_type)
|
||||
errors.extend(step_errors)
|
||||
result.append(user_step)
|
||||
|
||||
if step_type == 'deploy':
|
||||
# Deploy steps should be unique across all combined templates.
|
||||
dup_errors = _validate_deploy_steps_unique(user_steps)
|
||||
dup_errors = _validate_deploy_steps_unique(result)
|
||||
errors.extend(dup_errors)
|
||||
|
||||
if errors:
|
||||
|
@ -543,7 +557,7 @@ def _validate_user_steps(task, user_steps, driver_steps, step_type,
|
|||
err += '; '.join(errors)
|
||||
raise exception.InvalidParameterValue(err=err)
|
||||
|
||||
return user_steps
|
||||
return result
|
||||
|
||||
|
||||
def _validate_user_clean_steps(task, user_steps):
|
||||
|
@ -571,7 +585,8 @@ def _validate_user_clean_steps(task, user_steps):
|
|||
return _validate_user_steps(task, user_steps, driver_steps, 'clean')
|
||||
|
||||
|
||||
def _validate_user_deploy_steps(task, user_steps, error_prefix=None):
|
||||
def _validate_user_deploy_steps(task, user_steps, error_prefix=None,
|
||||
skip_missing=False):
|
||||
"""Validate the user-specified deploy steps.
|
||||
|
||||
:param task: A TaskManager object
|
||||
|
@ -598,10 +613,11 @@ def _validate_user_deploy_steps(task, user_steps, error_prefix=None):
|
|||
"""
|
||||
driver_steps = _get_deployment_steps(task, enabled=False, sort=False)
|
||||
return _validate_user_steps(task, user_steps, driver_steps, 'deploy',
|
||||
error_prefix=error_prefix)
|
||||
error_prefix=error_prefix,
|
||||
skip_missing=skip_missing)
|
||||
|
||||
|
||||
def validate_deploy_templates(task):
|
||||
def validate_deploy_templates(task, skip_missing=False):
|
||||
"""Validate the deploy templates for a node.
|
||||
|
||||
:param task: A TaskManager object
|
||||
|
@ -611,4 +627,4 @@ def validate_deploy_templates(task):
|
|||
steps from the driver.
|
||||
"""
|
||||
# Gather deploy steps from matching deploy templates and validate them.
|
||||
_get_validated_steps_from_templates(task)
|
||||
_get_validated_steps_from_templates(task, skip_missing=skip_missing)
|
||||
|
|
|
@ -323,7 +323,7 @@ class DoNodeDeployTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase):
|
|||
# these are not real steps...
|
||||
fake_deploy_steps = ['step1', 'step2']
|
||||
|
||||
def add_steps(task):
|
||||
def add_steps(task, **kwargs):
|
||||
info = task.node.driver_internal_info
|
||||
info['deploy_steps'] = fake_deploy_steps
|
||||
task.node.driver_internal_info = info
|
||||
|
@ -336,8 +336,7 @@ class DoNodeDeployTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase):
|
|||
task.process_event('deploy')
|
||||
|
||||
deployments.do_node_deploy(task, self.service.conductor.id)
|
||||
mock_set_steps.assert_called_once_with(task)
|
||||
mock_set_steps.assert_called_once_with(task)
|
||||
mock_set_steps.assert_called_once_with(task, skip_missing=True)
|
||||
self.assertEqual(fake_deploy_steps,
|
||||
task.node.driver_internal_info['deploy_steps'])
|
||||
|
||||
|
|
|
@ -193,7 +193,7 @@ class NodeDeployStepsTestCase(db_base.DbTestCase):
|
|||
self.context, self.node.uuid, shared=False) as task:
|
||||
steps = conductor_steps._get_all_deployment_steps(task)
|
||||
self.assertEqual(expected_steps, steps)
|
||||
mock_validated.assert_called_once_with(task)
|
||||
mock_validated.assert_called_once_with(task, skip_missing=False)
|
||||
mock_steps.assert_called_once_with(task, enabled=True, sort=False)
|
||||
|
||||
def test__get_all_deployment_steps_no_steps(self):
|
||||
|
@ -228,6 +228,25 @@ class NodeDeployStepsTestCase(db_base.DbTestCase):
|
|||
self._test__get_all_deployment_steps(user_steps, driver_steps,
|
||||
expected_steps)
|
||||
|
||||
@mock.patch.object(conductor_steps, '_get_validated_steps_from_templates',
|
||||
autospec=True)
|
||||
@mock.patch.object(conductor_steps, '_get_deployment_steps', autospec=True)
|
||||
def test__get_all_deployment_steps_skip_missing(self, mock_steps,
|
||||
mock_validated):
|
||||
user_steps = self.deploy_steps[:2]
|
||||
driver_steps = self.deploy_steps[2:]
|
||||
expected_steps = self.deploy_steps
|
||||
mock_validated.return_value = user_steps
|
||||
mock_steps.return_value = driver_steps
|
||||
|
||||
with task_manager.acquire(
|
||||
self.context, self.node.uuid, shared=False) as task:
|
||||
steps = conductor_steps._get_all_deployment_steps(
|
||||
task, skip_missing=True)
|
||||
self.assertEqual(expected_steps, steps)
|
||||
mock_validated.assert_called_once_with(task, skip_missing=True)
|
||||
mock_steps.assert_called_once_with(task, enabled=True, sort=False)
|
||||
|
||||
def test__get_all_deployment_steps_disable_core_steps(self):
|
||||
# User steps can disable core driver steps.
|
||||
user_steps = [self.deploy_core.copy()]
|
||||
|
@ -274,7 +293,7 @@ class NodeDeployStepsTestCase(db_base.DbTestCase):
|
|||
self.context, self.node.uuid, shared=False) as task:
|
||||
self.assertRaises(exception.InvalidParameterValue,
|
||||
conductor_steps._get_all_deployment_steps, task)
|
||||
mock_validated.assert_called_once_with(task)
|
||||
mock_validated.assert_called_once_with(task, skip_missing=False)
|
||||
self.assertFalse(mock_steps.called)
|
||||
|
||||
@mock.patch.object(conductor_steps, '_get_all_deployment_steps',
|
||||
|
@ -291,7 +310,23 @@ class NodeDeployStepsTestCase(db_base.DbTestCase):
|
|||
self.assertEqual({}, self.node.deploy_step)
|
||||
self.assertIsNone(
|
||||
self.node.driver_internal_info['deploy_step_index'])
|
||||
mock_steps.assert_called_once_with(task)
|
||||
mock_steps.assert_called_once_with(task, skip_missing=False)
|
||||
|
||||
@mock.patch.object(conductor_steps, '_get_all_deployment_steps',
|
||||
autospec=True)
|
||||
def test_set_node_deployment_steps_skip_missing(self, mock_steps):
|
||||
mock_steps.return_value = self.deploy_steps
|
||||
|
||||
with task_manager.acquire(
|
||||
self.context, self.node.uuid, shared=False) as task:
|
||||
conductor_steps.set_node_deployment_steps(task, skip_missing=True)
|
||||
self.node.refresh()
|
||||
self.assertEqual(self.deploy_steps,
|
||||
self.node.driver_internal_info['deploy_steps'])
|
||||
self.assertEqual({}, self.node.deploy_step)
|
||||
self.assertIsNone(
|
||||
self.node.driver_internal_info['deploy_step_index'])
|
||||
mock_steps.assert_called_once_with(task, skip_missing=True)
|
||||
|
||||
@mock.patch.object(conductor_steps, '_get_deployment_steps', autospec=True)
|
||||
def test__validate_user_deploy_steps(self, mock_steps):
|
||||
|
@ -342,6 +377,19 @@ class NodeDeployStepsTestCase(db_base.DbTestCase):
|
|||
task, user_steps)
|
||||
mock_steps.assert_called_once_with(task, enabled=False, sort=False)
|
||||
|
||||
@mock.patch.object(conductor_steps, '_get_deployment_steps', autospec=True)
|
||||
def test__validate_user_deploy_steps_skip_missing(self, mock_steps):
|
||||
mock_steps.return_value = self.deploy_steps
|
||||
user_steps = [{'step': 'power_one', 'interface': 'power',
|
||||
'priority': 200},
|
||||
{'step': 'bad_step', 'interface': 'deploy',
|
||||
'priority': 100}]
|
||||
|
||||
with task_manager.acquire(self.context, self.node.uuid) as task:
|
||||
result = conductor_steps._validate_user_deploy_steps(
|
||||
task, user_steps, skip_missing=True)
|
||||
self.assertEqual(user_steps[:1], result)
|
||||
|
||||
@mock.patch.object(conductor_steps, '_get_deployment_steps', autospec=True)
|
||||
def test__validate_user_deploy_steps_invalid_arg(self, mock_steps):
|
||||
mock_steps.return_value = self.deploy_steps
|
||||
|
@ -676,7 +724,23 @@ class GetValidatedStepsFromTemplatesTestCase(db_base.DbTestCase):
|
|||
self.assertEqual(steps, result)
|
||||
mock_templates.assert_called_once_with(task)
|
||||
mock_steps.assert_called_once_with(task, [self.template])
|
||||
mock_validate.assert_called_once_with(task, steps, mock.ANY)
|
||||
mock_validate.assert_called_once_with(task, steps, mock.ANY,
|
||||
skip_missing=False)
|
||||
|
||||
def test_skip_missing(self, mock_validate, mock_steps, mock_templates):
|
||||
mock_templates.return_value = [self.template]
|
||||
steps = [db_utils.get_test_deploy_template_step()]
|
||||
mock_steps.return_value = steps
|
||||
mock_validate.return_value = steps
|
||||
with task_manager.acquire(
|
||||
self.context, self.node.uuid, shared=False) as task:
|
||||
result = conductor_steps._get_validated_steps_from_templates(
|
||||
task, skip_missing=True)
|
||||
self.assertEqual(steps, result)
|
||||
mock_templates.assert_called_once_with(task)
|
||||
mock_steps.assert_called_once_with(task, [self.template])
|
||||
mock_validate.assert_called_once_with(task, steps, mock.ANY,
|
||||
skip_missing=True)
|
||||
|
||||
def test_invalid_parameter_value(self, mock_validate, mock_steps,
|
||||
mock_templates):
|
||||
|
@ -713,7 +777,15 @@ class ValidateDeployTemplatesTestCase(db_base.DbTestCase):
|
|||
self.context, self.node.uuid, shared=False) as task:
|
||||
result = conductor_steps.validate_deploy_templates(task)
|
||||
self.assertIsNone(result)
|
||||
mock_validated.assert_called_once_with(task)
|
||||
mock_validated.assert_called_once_with(task, skip_missing=False)
|
||||
|
||||
def test_skip_missing(self, mock_validated):
|
||||
with task_manager.acquire(
|
||||
self.context, self.node.uuid, shared=False) as task:
|
||||
result = conductor_steps.validate_deploy_templates(
|
||||
task, skip_missing=True)
|
||||
self.assertIsNone(result)
|
||||
mock_validated.assert_called_once_with(task, skip_missing=True)
|
||||
|
||||
def test_error(self, mock_validated):
|
||||
with task_manager.acquire(
|
||||
|
@ -721,4 +793,4 @@ class ValidateDeployTemplatesTestCase(db_base.DbTestCase):
|
|||
mock_validated.side_effect = exception.InvalidParameterValue('foo')
|
||||
self.assertRaises(exception.InvalidParameterValue,
|
||||
conductor_steps.validate_deploy_templates, task)
|
||||
mock_validated.assert_called_once_with(task)
|
||||
mock_validated.assert_called_once_with(task, skip_missing=False)
|
||||
|
|
Loading…
Reference in New Issue