diff --git a/ironic/conductor/deployments.py b/ironic/conductor/deployments.py index 3c7eadb2bf..114be9b49f 100644 --- a/ironic/conductor/deployments.py +++ b/ironic/conductor/deployments.py @@ -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( diff --git a/ironic/conductor/manager.py b/ironic/conductor/manager.py index 7a44ca12b3..c662173365 100644 --- a/ironic/conductor/manager.py +++ b/ironic/conductor/manager.py @@ -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: diff --git a/ironic/conductor/steps.py b/ironic/conductor/steps.py index 09ecf5fa24..a6663db01d 100644 --- a/ironic/conductor/steps.py +++ b/ironic/conductor/steps.py @@ -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,23 +522,34 @@ 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') - % {'type': step_type, 'step': user_step}) - errors.append(error) + 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 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) diff --git a/ironic/tests/unit/conductor/test_deployments.py b/ironic/tests/unit/conductor/test_deployments.py index 13b35c26ca..bbac176528 100644 --- a/ironic/tests/unit/conductor/test_deployments.py +++ b/ironic/tests/unit/conductor/test_deployments.py @@ -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']) diff --git a/ironic/tests/unit/conductor/test_steps.py b/ironic/tests/unit/conductor/test_steps.py index f1abb0c970..cc7c8af474 100644 --- a/ironic/tests/unit/conductor/test_steps.py +++ b/ironic/tests/unit/conductor/test_steps.py @@ -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)