Merge pull request #47843 from garethgreenaway/30367_orch_return_false_when_function_fails

[2018.3] orchestration results False when function is False

salt/states/saltmod.py

@@ -581,6 +581,9 @@ def function(
             m_ret = mdata['ret']
             m_func = (not fail_function and True) or __salt__[fail_function](m_ret)
 
+            if m_ret is False:
+                m_func = False
+
         if not m_func:
             if minion not in fail_minions:
                 fail.add(minion)

tests/integration/files/file/base/orch/issue30367/init.sls

@@ -0,0 +1,4 @@
+deploy_check:
+  salt.function:
+    - name: test.false
+    - tgt: minion

tests/integration/runners/test_state.py

@@ -257,6 +257,33 @@ class StateRunnerTest(ShellCase):
         self.assertEqual(count('Succeeded: 1', ret), 1)
         self.assertEqual(count('Failed:    0', ret), 1)
 
+    def test_orchestrate_salt_function_return_false_failure(self):
+        '''
+        Ensure that functions that only return False in the return
+        are flagged as failed when run as orchestrations.
+
+        See https://github.com/saltstack/salt/issues/30367
+        '''
+        self.run_run('saltutil.sync_modules')
+        ret = salt.utils.json.loads(
+            '\n'.join(
+                self.run_run('state.orchestrate orch.issue30367 --out=json')
+            )
+        )
+        # Drill down to the changes dict
+        state_result = ret['data']['master']['salt_|-deploy_check_|-test.false_|-function']['result']
+        func_ret = ret['data']['master']['salt_|-deploy_check_|-test.false_|-function']['changes']
+
+        self.assertEqual(
+            state_result,
+            False,
+        )
+
+        self.assertEqual(
+            func_ret,
+            {'out': 'highstate', 'ret': {'minion': False}}
+        )
+
 
 @skipIf(salt.utils.platform.is_windows(), '*NIX-only test')
 class OrchEventTest(ShellCase):