Fix salt-ssh state.* commands retcode for render fail

changelog/64514.fixed.md

@@ -0,0 +1 @@
+Fixed salt-ssh state.* commands returning retcode 0 when state/pillar rendering fails

salt/client/ssh/__init__.py

@@ -552,6 +552,11 @@ class SSH(MultiprocessingStateMixin):
             data = salt.utils.json.find_json(stdout)
             if len(data) < 2 and "local" in data:
                 ret["ret"] = data["local"]
+                try:
+                    # Ensure a reported local retcode is kept
+                    retcode = data["local"]["retcode"]
+                except (KeyError, TypeError):
+                    pass
             else:
                 ret["ret"] = {
                     "stdout": stdout,
@@ -564,7 +569,7 @@ class SSH(MultiprocessingStateMixin):
                 "stderr": stderr,
                 "retcode": retcode,
             }
-        que.put(ret)
+        que.put((ret, retcode))
 
     def handle_ssh(self, mine=False):
         """
@@ -608,7 +613,7 @@ class SSH(MultiprocessingStateMixin):
                         "fun": "",
                         "id": host,
                     }
-                    yield {host: no_ret}
+                    yield {host: no_ret}, 1
                     continue
                 args = (
                     que,
@@ -622,11 +627,12 @@ class SSH(MultiprocessingStateMixin):
                 running[host] = {"thread": routine}
                 continue
             ret = {}
+            retcode = 0
             try:
-                ret = que.get(False)
+                ret, retcode = que.get(False)
                 if "id" in ret:
                     returned.add(ret["id"])
-                    yield {ret["id"]: ret["ret"]}
+                    yield {ret["id"]: ret["ret"]}, retcode
             except queue.Empty:
                 pass
             for host in running:
@@ -636,10 +642,10 @@ class SSH(MultiprocessingStateMixin):
                         # last checked
                         try:
                             while True:
-                                ret = que.get(False)
+                                ret, retcode = que.get(False)
                                 if "id" in ret:
                                     returned.add(ret["id"])
-                                    yield {ret["id"]: ret["ret"]}
+                                    yield {ret["id"]: ret["ret"]}, retcode
                         except queue.Empty:
                             pass
 
@@ -650,7 +656,7 @@ class SSH(MultiprocessingStateMixin):
                             )
                             ret = {"id": host, "ret": error}
                             log.error(error)
-                            yield {ret["id"]: ret["ret"]}
+                            yield {ret["id"]: ret["ret"]}, 1
                     running[host]["thread"].join()
                     rets.add(host)
             for host in rets:
@@ -705,8 +711,8 @@ class SSH(MultiprocessingStateMixin):
                 jid, job_load
             )
 
-        for ret in self.handle_ssh(mine=mine):
-            host = next(iter(ret.keys()))
+        for ret, _ in self.handle_ssh(mine=mine):
+            host = next(iter(ret))
             self.cache_job(jid, host, ret[host], fun)
             if self.event:
                 id_, data = next(iter(ret.items()))
@@ -799,15 +805,9 @@ class SSH(MultiprocessingStateMixin):
         sret = {}
         outputter = self.opts.get("output", "nested")
         final_exit = 0
-        for ret in self.handle_ssh():
-            host = next(iter(ret.keys()))
-            if isinstance(ret[host], dict):
-                host_ret = ret[host].get("retcode", 0)
-                if host_ret != 0:
-                    final_exit = 1
-            else:
-                # Error on host
-                final_exit = 1
+        for ret, retcode in self.handle_ssh():
+            host = next(iter(ret))
+            final_exit = max(final_exit, retcode)
 
             self.cache_job(jid, host, ret[host], fun)
             ret = self.key_deploy(host, ret)
@@ -1274,6 +1274,10 @@ class Single:
             )
             log.error(result, exc_info_on_loglevel=logging.DEBUG)
             retcode = 1
+
+        # Ensure retcode from wrappers is respected, especially state render exceptions
+        retcode = max(retcode, self.context.get("retcode", 0))
+
         # Mimic the json data-structure that "salt-call --local" will
         # emit (as seen in ssh_py_shim.py)
         if isinstance(result, dict) and "local" in result:

salt/client/ssh/wrapper/state.py

@@ -8,6 +8,7 @@ import time
 
 import salt.client.ssh.shell
 import salt.client.ssh.state
+import salt.defaults.exitcodes
 import salt.loader
 import salt.minion
 import salt.roster
@@ -84,14 +85,14 @@ def _set_retcode(ret, highstate=None):
     """
 
     # Set default retcode to 0
-    __context__["retcode"] = 0
+    __context__["retcode"] = salt.defaults.exitcodes.EX_OK
 
     if isinstance(ret, list):
-        __context__["retcode"] = 1
+        __context__["retcode"] = salt.defaults.exitcodes.EX_STATE_COMPILER_ERROR
         return
     if not salt.utils.state.check_result(ret, highstate=highstate):
 
-        __context__["retcode"] = 2
+        __context__["retcode"] = salt.defaults.exitcodes.EX_STATE_FAILURE
 
 
 def _check_pillar(kwargs, pillar=None):
@@ -182,6 +183,11 @@ def sls(mods, saltenv="base", test=None, exclude=None, **kwargs):
         __context__["fileclient"],
         context=__context__.value(),
     ) as st_:
+        if not _check_pillar(kwargs, st_.opts["pillar"]):
+            __context__["retcode"] = salt.defaults.exitcodes.EX_PILLAR_FAILURE
+            err = ["Pillar failed to render with the following messages:"]
+            err += st_.opts["pillar"]["_errors"]
+            return err
         st_.push_active()
         mods = _parse_mods(mods)
         high_data, errors = st_.render_highstate(
@@ -198,12 +204,14 @@ def sls(mods, saltenv="base", test=None, exclude=None, **kwargs):
         errors += ext_errors
         errors += st_.state.verify_high(high_data)
         if errors:
+            __context__["retcode"] = salt.defaults.exitcodes.EX_STATE_COMPILER_ERROR
             return errors
         high_data, req_in_errors = st_.state.requisite_in(high_data)
         errors += req_in_errors
         high_data = st_.state.apply_exclude(high_data)
         # Verify that the high data is structurally sound
         if errors:
+            __context__["retcode"] = salt.defaults.exitcodes.EX_STATE_COMPILER_ERROR
             return errors
         # Compile and verify the raw chunks
         chunks = st_.state.compile_high_data(high_data)
@@ -316,7 +324,7 @@ def _check_queue(queue, kwargs):
     else:
         conflict = running(concurrent=kwargs.get("concurrent", False))
         if conflict:
-            __context__["retcode"] = 1
+            __context__["retcode"] = salt.defaults.exitcodes.EX_STATE_COMPILER_ERROR
             return conflict
 
 
@@ -681,6 +689,11 @@ def highstate(test=None, **kwargs):
         __context__["fileclient"],
         context=__context__.value(),
     ) as st_:
+        if not _check_pillar(kwargs, st_.opts["pillar"]):
+            __context__["retcode"] = salt.defaults.exitcodes.EX_PILLAR_FAILURE
+            err = ["Pillar failed to render with the following messages:"]
+            err += st_.opts["pillar"]["_errors"]
+            return err
         st_.push_active()
         chunks = st_.compile_low_chunks(context=__context__.value())
         file_refs = salt.client.ssh.state.lowstate_file_refs(
@@ -692,7 +705,7 @@ def highstate(test=None, **kwargs):
         # Check for errors
         for chunk in chunks:
             if not isinstance(chunk, dict):
-                __context__["retcode"] = 1
+                __context__["retcode"] = salt.defaults.exitcodes.EX_STATE_COMPILER_ERROR
                 return chunks
 
         roster = salt.roster.Roster(opts, opts.get("roster", "flat"))
@@ -766,9 +779,19 @@ def top(topfn, test=None, **kwargs):
         __context__["fileclient"],
         context=__context__.value(),
     ) as st_:
+        if not _check_pillar(kwargs, st_.opts["pillar"]):
+            __context__["retcode"] = salt.defaults.exitcodes.EX_PILLAR_FAILURE
+            err = ["Pillar failed to render with the following messages:"]
+            err += st_.opts["pillar"]["_errors"]
+            return err
         st_.opts["state_top"] = os.path.join("salt://", topfn)
         st_.push_active()
         chunks = st_.compile_low_chunks(context=__context__.value())
+        # Check for errors
+        for chunk in chunks:
+            if not isinstance(chunk, dict):
+                __context__["retcode"] = salt.defaults.exitcodes.EX_STATE_COMPILER_ERROR
+                return chunks
         file_refs = salt.client.ssh.state.lowstate_file_refs(
             chunks,
             _merge_extra_filerefs(
@@ -839,8 +862,17 @@ def show_highstate(**kwargs):
         __context__["fileclient"],
         context=__context__.value(),
     ) as st_:
+        if not _check_pillar(kwargs, st_.opts["pillar"]):
+            __context__["retcode"] = salt.defaults.exitcodes.EX_PILLAR_FAILURE
+            err = ["Pillar failed to render with the following messages:"]
+            err += st_.opts["pillar"]["_errors"]
+            return err
         st_.push_active()
         chunks = st_.compile_highstate(context=__context__.value())
+        # Check for errors
+        if not isinstance(chunks, dict):
+            __context__["retcode"] = salt.defaults.exitcodes.EX_STATE_COMPILER_ERROR
+            return chunks
         _cleanup_slsmod_high_data(chunks)
         return chunks
 
@@ -864,6 +896,11 @@ def show_lowstate(**kwargs):
         __context__["fileclient"],
         context=__context__.value(),
     ) as st_:
+        if not _check_pillar(kwargs, st_.opts["pillar"]):
+            __context__["retcode"] = salt.defaults.exitcodes.EX_PILLAR_FAILURE
+            err = ["Pillar failed to render with the following messages:"]
+            err += st_.opts["pillar"]["_errors"]
+            return err
         st_.push_active()
         chunks = st_.compile_low_chunks(context=__context__.value())
         _cleanup_slsmod_low_data(chunks)
@@ -925,7 +962,7 @@ def sls_id(id_, mods, test=None, queue=False, **kwargs):
     ) as st_:
 
         if not _check_pillar(kwargs, st_.opts["pillar"]):
-            __context__["retcode"] = 5
+            __context__["retcode"] = salt.defaults.exitcodes.EX_PILLAR_FAILURE
             err = ["Pillar failed to render with the following messages:"]
             err += __pillar__["_errors"]
             return err
@@ -943,7 +980,7 @@ def sls_id(id_, mods, test=None, queue=False, **kwargs):
             # but it is required to get the unit tests to pass.
             errors.extend(req_in_errors)
         if errors:
-            __context__["retcode"] = 1
+            __context__["retcode"] = salt.defaults.exitcodes.EX_STATE_COMPILER_ERROR
             return errors
         chunks = st_.state.compile_high_data(high_)
         chunk = [x for x in chunks if x.get("__id__", "") == id_]
@@ -988,6 +1025,11 @@ def show_sls(mods, saltenv="base", test=None, **kwargs):
         __context__["fileclient"],
         context=__context__.value(),
     ) as st_:
+        if not _check_pillar(kwargs, st_.opts["pillar"]):
+            __context__["retcode"] = salt.defaults.exitcodes.EX_PILLAR_FAILURE
+            err = ["Pillar failed to render with the following messages:"]
+            err += st_.opts["pillar"]["_errors"]
+            return err
         st_.push_active()
         mods = _parse_mods(mods)
         high_data, errors = st_.render_highstate(
@@ -997,12 +1039,14 @@ def show_sls(mods, saltenv="base", test=None, **kwargs):
         errors += ext_errors
         errors += st_.state.verify_high(high_data)
         if errors:
+            __context__["retcode"] = salt.defaults.exitcodes.EX_STATE_COMPILER_ERROR
             return errors
         high_data, req_in_errors = st_.state.requisite_in(high_data)
         errors += req_in_errors
         high_data = st_.state.apply_exclude(high_data)
         # Verify that the high data is structurally sound
         if errors:
+            __context__["retcode"] = salt.defaults.exitcodes.EX_STATE_COMPILER_ERROR
             return errors
         _cleanup_slsmod_high_data(high_data)
         return high_data
@@ -1036,6 +1080,11 @@ def show_low_sls(mods, saltenv="base", test=None, **kwargs):
         __context__["fileclient"],
         context=__context__.value(),
     ) as st_:
+        if not _check_pillar(kwargs, st_.opts["pillar"]):
+            __context__["retcode"] = salt.defaults.exitcodes.EX_PILLAR_FAILURE
+            err = ["Pillar failed to render with the following messages:"]
+            err += st_.opts["pillar"]["_errors"]
+            return err
         st_.push_active()
         mods = _parse_mods(mods)
         high_data, errors = st_.render_highstate(
@@ -1045,12 +1094,14 @@ def show_low_sls(mods, saltenv="base", test=None, **kwargs):
         errors += ext_errors
         errors += st_.state.verify_high(high_data)
         if errors:
+            __context__["retcode"] = salt.defaults.exitcodes.EX_STATE_COMPILER_ERROR
             return errors
         high_data, req_in_errors = st_.state.requisite_in(high_data)
         errors += req_in_errors
         high_data = st_.state.apply_exclude(high_data)
         # Verify that the high data is structurally sound
         if errors:
+            __context__["retcode"] = salt.defaults.exitcodes.EX_STATE_COMPILER_ERROR
             return errors
         ret = st_.state.compile_high_data(high_data)
         _cleanup_slsmod_low_data(ret)
@@ -1080,6 +1131,7 @@ def show_top(**kwargs):
         errors = []
         errors += st_.verify_tops(top_data)
         if errors:
+            __context__["retcode"] = salt.defaults.exitcodes.EX_STATE_COMPILER_ERROR
             return errors
         matches = st_.top_matches(top_data)
         return matches
@@ -1110,7 +1162,7 @@ def single(fun, name, test=None, **kwargs):
     # state.fun -> [state, fun]
     comps = fun.split(".")
     if len(comps) < 2:
-        __context__["retcode"] = 1
+        __context__["retcode"] = salt.defaults.exitcodes.EX_STATE_COMPILER_ERROR
         return "Invalid function passed"
 
     # Create the low chunk, using kwargs as a base
@@ -1133,7 +1185,7 @@ def single(fun, name, test=None, **kwargs):
     # Verify the low chunk
     err = st_.verify_data(kwargs)
     if err:
-        __context__["retcode"] = 1
+        __context__["retcode"] = salt.defaults.exitcodes.EX_STATE_COMPILER_ERROR
         return err
 
     # Must be a list of low-chunks