diff --git a/ironic/conductor/utils.py b/ironic/conductor/utils.py index fc374d5078..23dea27c76 100644 --- a/ironic/conductor/utils.py +++ b/ironic/conductor/utils.py @@ -413,7 +413,8 @@ def set_node_cleaning_steps(task): # Now that we know what the driver's available clean steps are, we can # do further checks to validate the user's clean steps. steps = node.driver_internal_info['clean_steps'] - _validate_user_clean_steps(task, steps) + driver_internal_info['clean_steps'] = ( + _validate_user_clean_steps(task, steps)) node.clean_step = {} driver_internal_info['clean_step_index'] = None @@ -440,6 +441,7 @@ def _validate_user_clean_steps(task, user_steps): :raises: InvalidParameterValue if validation of clean steps fails. :raises: NodeCleaningFailure if there was a problem getting the clean steps from the driver. + :return: validated clean steps update with information from the driver """ def step_id(step): @@ -461,6 +463,7 @@ def _validate_user_clean_steps(task, user_steps): for s in _get_cleaning_steps(task, enabled=False, sort=False): driver_steps[step_id(s)] = s + result = [] for user_step in user_steps: # Check if this user_specified clean step isn't supported by the driver try: @@ -495,5 +498,11 @@ def _validate_user_clean_steps(task, user_steps): 'miss': ', '.join(missing)} errors.append(error) + # Copy fields that should not be provided by a user + user_step['abortable'] = driver_step.get('abortable', False) + user_step['priority'] = driver_step.get('priority', 0) + result.append(user_step) + if errors: raise exception.InvalidParameterValue('; '.join(errors)) + return result diff --git a/ironic/tests/unit/conductor/test_utils.py b/ironic/tests/unit/conductor/test_utils.py index f1c393d068..2a16a4109b 100644 --- a/ironic/tests/unit/conductor/test_utils.py +++ b/ironic/tests/unit/conductor/test_utils.py @@ -640,7 +640,8 @@ class NodeCleaningStepsTestCase(base.DbTestCase): self.deploy_update = { 'step': 'update_firmware', 'priority': 10, 'interface': 'deploy'} self.deploy_erase = { - 'step': 'erase_disks', 'priority': 20, 'interface': 'deploy'} + 'step': 'erase_disks', 'priority': 20, 'interface': 'deploy', + 'abortable': True} # Automated cleaning should be executed in this order self.clean_steps = [self.deploy_erase, self.power_update, self.deploy_update] @@ -740,6 +741,7 @@ class NodeCleaningStepsTestCase(base.DbTestCase): mock_validate_user_steps): clean_steps = [self.deploy_raid] mock_steps.return_value = self.clean_steps + mock_validate_user_steps.return_value = clean_steps node = obj_utils.create_test_node( self.context, driver='fake', @@ -768,9 +770,16 @@ class NodeCleaningStepsTestCase(base.DbTestCase): {'step': 'erase_disks', 'interface': 'deploy'}] with task_manager.acquire(self.context, node.uuid) as task: - conductor_utils._validate_user_clean_steps(task, user_steps) + result = conductor_utils._validate_user_clean_steps(task, + user_steps) mock_steps.assert_called_once_with(task, enabled=False, sort=False) + expected = [{'step': 'update_firmware', 'interface': 'power', + 'priority': 10, 'abortable': False}, + {'step': 'erase_disks', 'interface': 'deploy', + 'priority': 20, 'abortable': True}] + self.assertEqual(expected, result) + @mock.patch.object(conductor_utils, '_get_cleaning_steps') def test__validate_user_clean_steps_no_steps(self, mock_steps): node = obj_utils.create_test_node(self.context) diff --git a/releasenotes/notes/manual-abort-d3d8985a5de7376a.yaml b/releasenotes/notes/manual-abort-d3d8985a5de7376a.yaml new file mode 100644 index 0000000000..7cc864b55a --- /dev/null +++ b/releasenotes/notes/manual-abort-d3d8985a5de7376a.yaml @@ -0,0 +1,5 @@ +--- +fixes: + - | + Fix bug in manual clean steps caching, which resulting in all clean steps + being not abortable. See https://bugs.launchpad.net/ironic/+bug/1658061.