Merge pull request #44726 from terminalmage/issue42188

Add __context__ to RunnerClient and WheelClient

doc/ref/states/writing.rst

@@ -111,6 +111,8 @@ This code will call the `managed` function in the :mod:`file
 <salt.states.file>` state module and pass the arguments ``name`` and ``source``
 to it.
 
+.. _state-return-data:
+
 Return Data
 ===========
 

doc/topics/orchestrate/orchestrate_runner.rst

@@ -5,10 +5,10 @@ Orchestrate Runner
 ==================
 
 Executing states or highstate on a minion is perfect when you want to ensure that
-minion configured and running the way you want. Sometimes however you want to 
+minion configured and running the way you want. Sometimes however you want to
 configure a set of minions all at once.
 
-For example, if you want to set up a load balancer in front of a cluster of web 
+For example, if you want to set up a load balancer in front of a cluster of web
 servers you can ensure the load balancer is set up first, and then the same
 matching configuration is applied consistently across the whole cluster.
 
@@ -222,6 +222,34 @@ To execute with pillar data.
     "master": "mymaster"}'
 
 
+Return Codes in Runner/Wheel Jobs
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+.. versionadded:: Oxygen
+
+State (``salt.state``) jobs are able to report failure via the :ref:`state
+return dictionary <state-return-data>`. Remote execution (``salt.function``)
+jobs are able to report failure by setting a ``retcode`` key in the
+``__context__`` dictionary. However, runner (``salt.runner``) and wheel
+(``salt.wheel``) jobs would only report a ``False`` result when the
+runner/wheel function raised an exception. As of the Oxygen release, it is now
+possible to set a retcode in runner and wheel functions just as you can do in
+remote execution functions. Here is some example pseudocode:
+
+.. code-block:: python
+
+    def myrunner():
+        ...
+        do stuff
+        ...
+        if some_error_condition:
+            __context__['retcode'] = 1
+        return result
+
+This allows a custom runner/wheel function to report its failure so that
+requisites can accurately tell that a job has failed.
+
+
 More Complex Orchestration
 ~~~~~~~~~~~~~~~~~~~~~~~~~~
 

doc/topics/releases/oxygen.rst

@@ -25,6 +25,25 @@ by any master tops matches that are not matched via a top file.
 To make master tops matches execute first, followed by top file matches, set
 the new :conf_minion:`master_tops_first` minion config option to ``True``.
 
+Return Codes for Runner/Wheel Functions
+---------------------------------------
+
+When using :ref:`orchestration <orchestrate-runner>`, runner and wheel
+functions used to report a ``True`` result if the function ran to completion
+without raising an exception. It is now possible to set a return code in the
+``__context__`` dictionary, allowing runner and wheel functions to report that
+they failed. Here's some example pseudocode:
+
+.. code-block:: python
+
+    def myrunner():
+        ...
+        do stuff
+        ...
+        if some_error_condition:
+            __context__['retcode'] = 1
+        return result
+
 LDAP via External Authentication Changes
 ----------------------------------------
 In this release of Salt, if LDAP Bind Credentials are supplied, then

salt/client/mixins.py

@@ -385,7 +385,11 @@ def _low(self, fun, low, print_event=True, full_return=False):
             # Initialize a context for executing the method.
             with tornado.stack_context.StackContext(self.functions.context_dict.clone):
                 data[u'return'] = self.functions[fun](*args, **kwargs)
-                data[u'success'] = True
+                try:
+                    data[u'success'] = self.context.get(u'retcode', 0) == 0
+                except AttributeError:
+                    # Assume a True result if no context attribute
+                    data[u'success'] = True
                 if isinstance(data[u'return'], dict) and u'data' in data[u'return']:
                     # some functions can return boolean values
                     data[u'success'] = salt.utils.state.check_result(data[u'return'][u'data'])

salt/loader.py

@@ -372,15 +372,18 @@ def tops(opts):
     return FilterDictWrapper(ret, u'.top')
 
 
-def wheels(opts, whitelist=None):
+def wheels(opts, whitelist=None, context=None):
     '''
     Returns the wheels modules
     '''
+    if context is None:
+        context = {}
     return LazyLoader(
         _module_dirs(opts, u'wheel'),
         opts,
         tag=u'wheel',
         whitelist=whitelist,
+        pack={u'__context__': context},
     )
 
 
@@ -836,17 +839,19 @@ def call(fun, **kwargs):
     return funcs[fun](*args)
 
 
-def runner(opts, utils=None):
+def runner(opts, utils=None, context=None):
     '''
     Directly call a function inside a loader directory
     '''
     if utils is None:
         utils = {}
+    if context is None:
+        context = {}
     ret = LazyLoader(
         _module_dirs(opts, u'runners', u'runner', ext_type_dirs=u'runner_dirs'),
         opts,
         tag=u'runners',
-        pack={u'__utils__': utils},
+        pack={u'__utils__': utils, u'__context__': context},
     )
     # TODO: change from __salt__ to something else, we overload __salt__ too much
     ret.pack[u'__salt__'] = ret

salt/runner.py

@@ -43,6 +43,7 @@ class RunnerClient(mixins.SyncClientMixin, mixins.AsyncClientMixin, object):
 
     def __init__(self, opts):
         self.opts = opts
+        self.context = {}
 
     @property
     def functions(self):
@@ -51,11 +52,13 @@ def functions(self):
                 self.utils = salt.loader.utils(self.opts)
             # Must be self.functions for mixin to work correctly :-/
             try:
-                self._functions = salt.loader.runner(self.opts, utils=self.utils)
+                self._functions = salt.loader.runner(
+                    self.opts, utils=self.utils, context=self.context)
             except AttributeError:
                 # Just in case self.utils is still not present (perhaps due to
                 # problems with the loader), load the runner funcs without them
-                self._functions = salt.loader.runner(self.opts)
+                self._functions = salt.loader.runner(
+                    self.opts, context=self.context)
 
         return self._functions
 

salt/states/saltmod.py

@@ -787,28 +787,15 @@ def runner(name, **kwargs):
     runner_return = out.get('return')
     if isinstance(runner_return, dict) and 'Error' in runner_return:
         out['success'] = False
-    if not out.get('success', True):
-        cmt = "Runner function '{0}' failed{1}.".format(
-            name,
-            ' with return {0}'.format(runner_return) if runner_return else '',
-        )
-        ret = {
-            'name': name,
-            'result': False,
-            'changes': {},
-            'comment': cmt,
-        }
-    else:
-        cmt = "Runner function '{0}' executed{1}.".format(
-            name,
-            ' with return {0}'.format(runner_return) if runner_return else '',
-        )
-        ret = {
-            'name': name,
-            'result': True,
-            'changes': {},
-            'comment': cmt,
-        }
+
+    success = out.get('success', True)
+    ret = {'name': name,
+           'changes': {'return': runner_return},
+           'result': success}
+    ret['comment'] = "Runner function '{0}' {1}.".format(
+        name,
+        'executed' if success else 'failed',
+    )
 
     ret['__orchestration__'] = True
     if 'jid' in out:
@@ -1039,15 +1026,21 @@ def wheel(name, **kwargs):
                                      __env__=__env__,
                                      **kwargs)
 
-    ret['result'] = True
-    ret['__orchestration__'] = True
-    if 'jid' in out:
-        ret['__jid__'] = out['jid']
+    wheel_return = out.get('return')
+    if isinstance(wheel_return, dict) and 'Error' in wheel_return:
+        out['success'] = False
 
-    runner_return = out.get('return')
-    ret['comment'] = "Wheel function '{0}' executed{1}.".format(
+    success = out.get('success', True)
+    ret = {'name': name,
+           'changes': {'return': wheel_return},
+           'result': success}
+    ret['comment'] = "Wheel function '{0}' {1}.".format(
         name,
-        ' with return {0}'.format(runner_return) if runner_return else '',
+        'executed' if success else 'failed',
     )
 
+    ret['__orchestration__'] = True
+    if 'jid' in out:
+        ret['__jid__'] = out['jid']
+
     return ret

salt/wheel/__init__.py

@@ -43,7 +43,8 @@ class WheelClient(salt.client.mixins.SyncClientMixin,
 
     def __init__(self, opts=None):
         self.opts = opts
-        self.functions = salt.loader.wheels(opts)
+        self.context = {}
+        self.functions = salt.loader.wheels(opts, context=self.context)
 
     # TODO: remove/deprecate
     def call_func(self, fun, **kwargs):

tests/integration/files/file/base/_runners/runtests_helpers.py

@@ -0,0 +1,16 @@
+# -*- coding: utf-8 -*-
+'''
+Runner functions for integration tests
+'''
+
+# Import python libs
+from __future__ import absolute_import
+
+
+def failure():
+    __context__['retcode'] = 1
+    return False
+
+
+def success():
+    return True

tests/integration/files/file/base/_wheel/runtests_helpers.py

@@ -0,0 +1,16 @@
+# -*- coding: utf-8 -*-
+'''
+Wheel functions for integration tests
+'''
+
+# Import python libs
+from __future__ import absolute_import
+
+
+def failure():
+    __context__['retcode'] = 1
+    return False
+
+
+def success():
+    return True

tests/integration/files/file/base/orch/retcode.sls

@@ -0,0 +1,15 @@
+test_runner_success:
+  salt.runner:
+    - name: runtests_helpers.success
+
+test_runner_failure:
+  salt.runner:
+    - name: runtests_helpers.failure
+
+test_wheel_success:
+  salt.wheel:
+    - name: runtests_helpers.success
+
+test_wheel_failure:
+  salt.wheel:
+    - name: runtests_helpers.failure

tests/integration/runners/test_state.py

@@ -106,6 +106,35 @@ def test_orchestrate_target_exists(self):
             for item in out:
                 self.assertIn(item, ret)
 
+    def test_orchestrate_retcode(self):
+        '''
+        Test orchestration with nonzero retcode set in __context__
+        '''
+        self.run_run('saltutil.sync_runners')
+        self.run_run('saltutil.sync_wheel')
+        ret = '\n'.join(self.run_run('state.orchestrate orch.retcode'))
+
+        for result in ('          ID: test_runner_success\n'
+                       '    Function: salt.runner\n'
+                       '        Name: runtests_helpers.success\n'
+                       '      Result: True',
+
+                       '          ID: test_runner_failure\n'
+                       '    Function: salt.runner\n'
+                       '        Name: runtests_helpers.failure\n'
+                       '      Result: False',
+
+                       '          ID: test_wheel_success\n'
+                       '    Function: salt.wheel\n'
+                       '        Name: runtests_helpers.success\n'
+                       '      Result: True',
+
+                       '          ID: test_wheel_failure\n'
+                       '    Function: salt.wheel\n'
+                       '        Name: runtests_helpers.failure\n'
+                       '      Result: False'):
+            self.assertIn(result, ret)
+
     def test_orchestrate_target_doesnt_exists(self):
         '''
         test orchestration when target doesnt exist

tests/unit/states/test_saltmod.py

@@ -258,8 +258,8 @@ def test_runner(self):
         '''
         name = 'state'
 
-        ret = {'changes': {}, 'name': 'state', 'result': True,
-               'comment': 'Runner function \'state\' executed with return True.',
+        ret = {'changes': {'return': True}, 'name': 'state', 'result': True,
+               'comment': 'Runner function \'state\' executed.',
                '__orchestration__': True}
         runner_mock = MagicMock(return_value={'return': True})
 
@@ -274,8 +274,8 @@ def test_wheel(self):
         '''
         name = 'state'
 
-        ret = {'changes': {}, 'name': 'state', 'result': True,
-               'comment': 'Wheel function \'state\' executed with return True.',
+        ret = {'changes': {'return': True}, 'name': 'state', 'result': True,
+               'comment': 'Wheel function \'state\' executed.',
                '__orchestration__': True}
         wheel_mock = MagicMock(return_value={'return': True})