From ee6bfb3ac921c292ac4e60af732f8ef4bf45496c Mon Sep 17 00:00:00 2001
From: Dmitry Tantsur <divius.inside@gmail.com>
Date: Fri, 20 Jan 2017 13:11:12 +0100
Subject: [PATCH] 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 | 11 ++++++++++-
ironic/tests/unit/conductor/test_utils.py | 13 +++++++++++--
.../notes/manual-abort-d3d8985a5de7376a.yaml | 5 +++++
3 files changed, 26 insertions(+), 3 deletions(-)
create mode 100644 releasenotes/notes/manual-abort-d3d8985a5de7376a.yaml
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.