summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorRaphael Glon <raphael.glon@ovh.net>2018-12-28 16:15:32 +0100
committerDmitry Tantsur <divius.inside@gmail.com>2019-04-15 11:55:28 +0000
commitdd9c3a795895157f73b7fbc74f4a16441d869458 (patch)
tree917b55232c2bfb55e64f40cae3828bcfd913dc19
parentaaca6a270ba5d117d10f55682d262e21734a0795 (diff)
downloadironic-dd9c3a795895157f73b7fbc74f4a16441d869458.tar.gz
Ansible module: fix clean error handling
It should not be up to the driver to handle the error. The error should reach the manager. Moreover, handling the error in the driver and returning nothing caused the manager to consider the step done and go to the next one instead of interrupting the cleaning workflow Change-Id: I3825838b5507bc735d983466aa3cac0edd4dfaca Story: #2005357 Task: #30315 (cherry picked from commit df5261bb349dd56ac88876e14779d244bdc0beb3)
-rw-r--r--ironic/drivers/modules/ansible/deploy.py15
-rw-r--r--ironic/tests/unit/drivers/modules/ansible/test_deploy.py11
-rw-r--r--releasenotes/notes/bug-30315-e46eafe5b575f3da.yaml8
3 files changed, 16 insertions, 18 deletions
diff --git a/ironic/drivers/modules/ansible/deploy.py b/ironic/drivers/modules/ansible/deploy.py
index 8a4adadba..a8645619f 100644
--- a/ironic/drivers/modules/ansible/deploy.py
+++ b/ironic/drivers/modules/ansible/deploy.py
@@ -528,17 +528,10 @@ class AnsibleDeploy(agent_base.HeartbeatMixin, base.DeployInterface):
LOG.debug('Starting cleaning step %(step)s on node %(node)s',
{'node': node.uuid, 'step': stepname})
step_tags = step['args'].get('tags', [])
- try:
- _run_playbook(node, playbook, extra_vars, key, tags=step_tags)
- except exception.InstanceDeployFailure as e:
- LOG.error("Ansible failed cleaning step %(step)s "
- "on node %(node)s.",
- {'node': node.uuid, 'step': stepname})
- manager_utils.cleaning_error_handler(task, six.text_type(e))
- else:
- LOG.info('Ansible completed cleaning step %(step)s '
- 'on node %(node)s.',
- {'node': node.uuid, 'step': stepname})
+ _run_playbook(node, playbook, extra_vars, key, tags=step_tags)
+ LOG.info('Ansible completed cleaning step %(step)s '
+ 'on node %(node)s.',
+ {'node': node.uuid, 'step': stepname})
@METRICS.timer('AnsibleDeploy.prepare_cleaning')
def prepare_cleaning(self, task):
diff --git a/ironic/tests/unit/drivers/modules/ansible/test_deploy.py b/ironic/tests/unit/drivers/modules/ansible/test_deploy.py
index 02dddd98c..711e24c52 100644
--- a/ironic/tests/unit/drivers/modules/ansible/test_deploy.py
+++ b/ironic/tests/unit/drivers/modules/ansible/test_deploy.py
@@ -674,11 +674,10 @@ class TestAnsibleDeploy(AnsibleDeployTestCaseBase):
@mock.patch.object(ansible_deploy, '_parse_ansible_driver_info',
return_value=('test_pl', 'test_u', 'test_k'),
autospec=True)
- @mock.patch.object(utils, 'cleaning_error_handler', autospec=True)
@mock.patch.object(ansible_deploy, '_run_playbook', autospec=True)
@mock.patch.object(ansible_deploy, 'LOG', autospec=True)
def test_execute_clean_step_no_success_log(
- self, log_mock, run_mock, utils_mock, parse_driver_info_mock):
+ self, log_mock, run_mock, parse_driver_info_mock):
run_mock.side_effect = exception.InstanceDeployFailure('Boom')
step = {'priority': 10, 'interface': 'deploy',
@@ -688,11 +687,9 @@ class TestAnsibleDeploy(AnsibleDeployTestCaseBase):
self.node.driver_internal_info = di_info
self.node.save()
with task_manager.acquire(self.context, self.node.uuid) as task:
- self.driver.execute_clean_step(task, step)
- log_mock.error.assert_called_once_with(
- mock.ANY, {'node': task.node['uuid'],
- 'step': 'erase_devices'})
- utils_mock.assert_called_once_with(task, 'Boom')
+ self.assertRaises(exception.InstanceDeployFailure,
+ self.driver.execute_clean_step,
+ task, step)
self.assertFalse(log_mock.info.called)
@mock.patch.object(ansible_deploy, '_run_playbook', autospec=True)
diff --git a/releasenotes/notes/bug-30315-e46eafe5b575f3da.yaml b/releasenotes/notes/bug-30315-e46eafe5b575f3da.yaml
new file mode 100644
index 000000000..52020517c
--- /dev/null
+++ b/releasenotes/notes/bug-30315-e46eafe5b575f3da.yaml
@@ -0,0 +1,8 @@
+---
+fixes:
+ - |
+ Fixes an issue regarding the ``ansible deployment interface`` cleaning
+ workflow.
+ Handling the error in the driver and returning nothing caused the manager
+ to consider the step done and go to the next one instead of interrupting
+ the cleaning workflow.