moved to exceptions for basic skip/fails

better handling of checkmode and async
fix test to follow new flow control

(cherry picked from commit e29dc49a49eb042cb2f707eb8e9e030a718677ed)

12 files changed, 41 insertions, 41 deletions

diff --git a/lib/ansible/errors/__init__.py b/lib/ansible/errors/__init__.py
index 6bac864051..1b62a04160 100644
--- a/lib/ansible/errors/__init__.py
+++ b/lib/ansible/errors/__init__.py
@@ -201,3 +201,9 @@ class AnsibleModuleExit(Exception):
     ''' local module exit '''
     def __init__(self, result):
         self.result = result
+class AnsibleActionSkip(AnsibleRuntimeError):
+    ''' an action runtime skip'''
+    pass
+class AnsibleActionFail(AnsibleRuntimeError):
+    ''' an action runtime failure'''
+    pass
diff --git a/lib/ansible/executor/task_executor.py b/lib/ansible/executor/task_executor.py
index 6f9fb4d451..62dac58f64 100644
--- a/lib/ansible/executor/task_executor.py
+++ b/lib/ansible/executor/task_executor.py
@@ -27,7 +27,7 @@ import traceback
 from ansible.compat.six import iteritems, string_types, binary_type
 
 from ansible import constants as C
-from ansible.errors import AnsibleError, AnsibleParserError, AnsibleUndefinedVariable, AnsibleConnectionFailure
+from ansible.errors import AnsibleError, AnsibleParserError, AnsibleUndefinedVariable, AnsibleConnectionFailure, AnsibleActionFail, AnsibleActionSkip
 from ansible.executor.task_result import TaskResult
 from ansible.module_utils._text import to_text
 from ansible.playbook.conditional import Conditional
@@ -521,6 +521,10 @@ class TaskExecutor:
             display.debug("running the handler")
             try:
                 result = self._handler.run(task_vars=variables)
+            except AnsibleActionSkip as e:
+                return dict(skipped=True, msg=to_text(e))
+            except AnsibleActionFail as e:
+                return dict(failed=True, msg=to_text(e))
             except AnsibleConnectionFailure as e:
                 return dict(unreachable=True, msg=to_text(e))
             display.debug("handler run complete")
diff --git a/lib/ansible/plugins/action/__init__.py b/lib/ansible/plugins/action/__init__.py
index 4782b0bb7d..c530cdee43 100644
--- a/lib/ansible/plugins/action/__init__.py
+++ b/lib/ansible/plugins/action/__init__.py
@@ -32,7 +32,7 @@ from abc import ABCMeta, abstractmethod
 from ansible import constants as C
 from ansible.compat.six import binary_type, string_types, text_type, iteritems, with_metaclass
 from ansible.compat.six.moves import shlex_quote
-from ansible.errors import AnsibleError, AnsibleConnectionFailure
+from ansible.errors import AnsibleError, AnsibleConnectionFailure, AnsibleActionSkip, AnsibleActionFail
 from ansible.executor.module_common import modify_module, build_windows_module_payload
 from ansible.module_utils._text import to_bytes, to_native, to_text
 from ansible.module_utils.json_utils import _filter_non_json_lines
@@ -93,14 +93,11 @@ class ActionBase(with_metaclass(ABCMeta, object)):
         result = {}
 
         if self._task.async and not self._supports_async:
-            result['msg'] = 'async is not supported for this task.'
-            result['failed'] = True
+            raise AnsibleActionFail('async is not supported for this task.')
         elif self._play_context.check_mode and not self._supports_check_mode:
-            result['msg'] = 'check mode is not supported for this task.'
-            result['skipped'] = True
+            raise AnsibleActionSkip('check mode is not supported for this task.')
         elif self._task.async and self._play_context.check_mode:
-            result['msg'] = 'check mode and async cannot be used on same task.'
-            result['failed'] = True
+            raise AnsibleActionFail('check mode and async cannot be used on same task.')
 
         return result
 
diff --git a/lib/ansible/plugins/action/add_host.py b/lib/ansible/plugins/action/add_host.py
index 6531d5b320..67a2123d9b 100644
--- a/lib/ansible/plugins/action/add_host.py
+++ b/lib/ansible/plugins/action/add_host.py
@@ -46,9 +46,6 @@ class ActionModule(ActionBase):
 
         result = super(ActionModule, self).run(tmp, task_vars)
 
-        if result.get('skipped', False) or result.get('failed', False):
-            return result
-
         # Parse out any hostname:port patterns
         new_name = self._task.args.get('name', self._task.args.get('hostname', None))
         display.vv("creating host via 'add_host': hostname=%s" % new_name)
diff --git a/lib/ansible/plugins/action/assemble.py b/lib/ansible/plugins/action/assemble.py
index 9000db9f58..b76c78cf63 100644
--- a/lib/ansible/plugins/action/assemble.py
+++ b/lib/ansible/plugins/action/assemble.py
@@ -84,9 +84,6 @@ class ActionModule(ActionBase):
 
         result = super(ActionModule, self).run(tmp, task_vars)
 
-        if result.get('skipped', False) or result.get('failed', False):
-            return result
-
         if task_vars is None:
             task_vars = dict()
 
diff --git a/lib/ansible/plugins/action/copy.py b/lib/ansible/plugins/action/copy.py
index 02cd2d55cd..9133dcf0be 100644
--- a/lib/ansible/plugins/action/copy.py
+++ b/lib/ansible/plugins/action/copy.py
@@ -40,9 +40,6 @@ class ActionModule(ActionBase):
 
         result = super(ActionModule, self).run(tmp, task_vars)
 
-        if result.get('skipped', False) or result.get('failed', False):
-            return result
-
         source  = self._task.args.get('src', None)
         content = self._task.args.get('content', None)
         dest    = self._task.args.get('dest', None)
diff --git a/lib/ansible/plugins/action/package.py b/lib/ansible/plugins/action/package.py
index 6a69bbceb9..ec7cd213e8 100644
--- a/lib/ansible/plugins/action/package.py
+++ b/lib/ansible/plugins/action/package.py
@@ -38,9 +38,6 @@ class ActionModule(ActionBase):
 
         result = super(ActionModule, self).run(tmp, task_vars)
 
-        if result.get('skipped', False) or result.get('failed', False):
-            return result
-
         module = self._task.args.get('use', 'auto')
 
         if module == 'auto':
diff --git a/lib/ansible/plugins/action/script.py b/lib/ansible/plugins/action/script.py
index 19c094c78b..99ee30932e 100644
--- a/lib/ansible/plugins/action/script.py
+++ b/lib/ansible/plugins/action/script.py
@@ -34,9 +34,6 @@ class ActionModule(ActionBase):
 
         result = super(ActionModule, self).run(tmp, task_vars)
 
-        if result.get('skipped', False) or result.get('failed', False):
-            return result
-
         if not tmp:
             tmp = self._make_tmp_path()
 
diff --git a/lib/ansible/plugins/action/service.py b/lib/ansible/plugins/action/service.py
index 7e2921ff2a..36e93875b6 100644
--- a/lib/ansible/plugins/action/service.py
+++ b/lib/ansible/plugins/action/service.py
@@ -37,9 +37,6 @@ class ActionModule(ActionBase):
 
         result = super(ActionModule, self).run(tmp, task_vars)
 
-        if result.get('skipped', False) or result.get('failed', False):
-            return result
-
         module = self._task.args.get('use', 'auto').lower()
 
         if module == 'auto':
diff --git a/test/integration/targets/module_utils/module_utils_test.yml b/test/integration/targets/module_utils/module_utils_test.yml
index a131727084..ee737c6cdc 100644
--- a/test/integration/targets/module_utils/module_utils_test.yml
+++ b/test/integration/targets/module_utils/module_utils_test.yml
@@ -36,7 +36,7 @@
   - name: Make sure the we used the local facts.py, not the one shipped with ansible
     assert:
       that:
-        - 'result["data"] == "overridden facts.py"'
+        - result["data"] == "overridden facts.py"
 
   - name: Test that importing a module that only exists inside of a submodule does not work
     test_failure:
@@ -47,5 +47,5 @@
   - name: Make sure we failed in AnsiBallZ
     assert:
       that:
-        - 'result["failed"] == True'
-        - '"Could not find imported module support code for test_failure.  Looked for either foo.py or zebra.py" == result["msg"]'
+        - result|failed
+        - result['msg'] == "Could not find imported module support code for test_failure.  Looked for either foo.py or zebra.py"
diff --git a/test/integration/targets/script/tasks/main.yml b/test/integration/targets/script/tasks/main.yml
index 2c0f1f02ad..7b31444190 100644
--- a/test/integration/targets/script/tasks/main.yml
+++ b/test/integration/targets/script/tasks/main.yml
@@ -69,3 +69,17 @@
     that:
       - "script_result1|changed"
       - "script_result2.state == 'absent'"
+
+# async
+- name: test task failure with async param
+
+  script: /some/script.sh
+  async: 2
+  ignore_errors: true
+  register: script_result3
+
+- name: assert task with async param failed
+  assert:
+    that:
+      - script_result3|failed
+      - script_result3.msg == "async is not supported for this task."
diff --git a/test/units/plugins/action/test_raw.py b/test/units/plugins/action/test_raw.py
index c48d38b437..b6e12ee339 100644
--- a/test/units/plugins/action/test_raw.py
+++ b/test/units/plugins/action/test_raw.py
@@ -18,7 +18,7 @@
 from __future__ import (absolute_import, division, print_function)
 __metaclass__ = type
 
-
+from ansible.errors import AnsibleActionFail
 from ansible.compat.tests import unittest
 from ansible.compat.tests.mock import patch, MagicMock, Mock
 from ansible.plugins.action.raw import ActionModule
@@ -43,7 +43,7 @@ class TestCopyResultExclude(unittest.TestCase):
 
         play_context = Mock()
         task = MagicMock(Task)
-        task.async = MagicMock()
+        task.async = False
         connection = Mock()
 
         task.args = {'_raw_params': 'Args1'}
@@ -60,25 +60,22 @@ class TestCopyResultExclude(unittest.TestCase):
 
         play_context = Mock()
         task = MagicMock(Task)
-        task.async = MagicMock()
+        task.async = False
         connection = Mock()
 
         task.args = {'_raw_params': 'Args1'}
         play_context.check_mode = True
 
-        self.mock_am = ActionModule(task, connection, play_context, loader=None, templar=None, shared_loader_obj=None)
-        self.mock_am._low_level_execute_command = Mock(return_value = {})
-        self.mock_am.display = Mock()
-
-        skipped_result = self.mock_am.run()
-
-        self.assertEqual(skipped_result.get('skipped'), True)
+        try:
+            self.mock_am = ActionModule(task, connection, play_context, loader=None, templar=None, shared_loader_obj=None)
+        except AnsibleActionFail:
+            pass
 
     def test_raw_test_environment_is_None(self):
 
         play_context = Mock()
         task = MagicMock(Task)
-        task.async = MagicMock()
+        task.async = False
         connection = Mock()
 
         task.args = {'_raw_params': 'Args1'}
@@ -95,7 +92,7 @@ class TestCopyResultExclude(unittest.TestCase):
 
         play_context = Mock()
         task = MagicMock(Task)
-        task.async = MagicMock()
+        task.async = False
         connection = Mock()
 
         task.args = {'_raw_params': 'Args1'}