Merge "Correctly cache "abortable" flag for manual clean steps"

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

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)

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.