Correctly cache "abortable" flag for manual clean steps

Currently we don't store it, thus all clean steps are considered not
abortable. Also cache "priority" in case something relies on it.

Change-Id: I9e30925113711456ff4d9e5f1c00d60af78a45ff
Closes-Bug: #1658061

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.