Merge "Ansible module: fix clean error handling"

ironic/drivers/modules/ansible/deploy.py

@@ -526,17 +526,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):

ironic/tests/unit/drivers/modules/ansible/test_deploy.py

@@ -713,11 +713,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',
@@ -727,11 +726,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)

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.