Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[3006.x] Fix salt-ssh state.* commands retcode for render fail #64515

Merged
merged 4 commits into from
Jun 21, 2023
Merged

[3006.x] Fix salt-ssh state.* commands retcode for render fail #64515

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7f13acd
Add tests for salt-ssh `state.*` exitcodes
lkubb Jun 21, 2023
1154b17
Fix salt-ssh state.* commands retcode for render fail
lkubb Jun 21, 2023
199c284
Add misc tests for retcode passthrough
lkubb Jun 21, 2023
7ee2340
run pre-commit upgrade code for Py3.8+
lkubb Jun 21, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog/64514.fixed.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fixed salt-ssh state.* commands returning retcode 0 when state/pillar rendering fails
90 changes: 44 additions & 46 deletions salt/client/ssh/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,7 @@ def __init__(self, opts):
}
if self.opts.get("rand_thin_dir"):
self.defaults["thin_dir"] = os.path.join(
"/var/tmp", ".{}".format(uuid.uuid4().hex[:6])
"/var/tmp", f".{uuid.uuid4().hex[:6]}"
)
self.opts["ssh_wipe"] = "True"
self.returners = salt.loader.returners(self.opts, {})
Expand Down Expand Up @@ -454,9 +454,9 @@ def get_pubkey(self):
priv = self.opts.get(
"ssh_priv", os.path.join(self.opts["pki_dir"], "ssh", "salt-ssh.rsa")
)
pub = "{}.pub".format(priv)
pub = f"{priv}.pub"
with salt.utils.files.fopen(pub, "r") as fp_:
return "{} rsa root@master".format(fp_.read().split()[1])
return f"{fp_.read().split()[1]} rsa root@master"

def key_deploy(self, host, ret):
"""
Expand Down Expand Up @@ -500,7 +500,7 @@ def _key_deploy_run(self, host, target, re_run=True):
mods=self.mods,
fsclient=self.fsclient,
thin=self.thin,
**target
**target,
)
if salt.utils.path.which("ssh-copy-id"):
# we have ssh-copy-id, use it!
Expand All @@ -516,7 +516,7 @@ def _key_deploy_run(self, host, target, re_run=True):
mods=self.mods,
fsclient=self.fsclient,
thin=self.thin,
**target
**target,
)
stdout, stderr, retcode = single.cmd_block()
try:
Expand All @@ -543,7 +543,7 @@ def handle_routine(self, que, opts, host, target, mine=False):
fsclient=self.fsclient,
thin=self.thin,
mine=mine,
**target
**target,
)
ret = {"id": single.id}
stdout, stderr, retcode = single.run()
Expand All @@ -552,6 +552,11 @@ def handle_routine(self, que, opts, host, target, mine=False):
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,
Expand All @@ -564,7 +569,7 @@ def handle_routine(self, que, opts, host, target, mine=False):
"stderr": stderr,
"retcode": retcode,
}
que.put(ret)
que.put((ret, retcode))

def handle_ssh(self, mine=False):
"""
Expand Down Expand Up @@ -608,7 +613,7 @@ def handle_ssh(self, mine=False):
"fun": "",
"id": host,
}
yield {host: no_ret}
yield {host: no_ret}, 1
continue
args = (
que,
Expand All @@ -622,11 +627,12 @@ def handle_ssh(self, mine=False):
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:
Expand All @@ -636,10 +642,10 @@ def handle_ssh(self, mine=False):
# 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

Expand All @@ -650,7 +656,7 @@ def handle_ssh(self, mine=False):
)
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:
Expand Down Expand Up @@ -705,8 +711,8 @@ def run_iter(self, mine=False, jid=None):
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()))
Expand Down Expand Up @@ -792,22 +798,16 @@ def run(self, jid=None):
)

if self.opts.get("verbose"):
msg = "Executing job with jid {}".format(jid)
msg = f"Executing job with jid {jid}"
print(msg)
print("-" * len(msg) + "\n")
print("")
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)
Expand Down Expand Up @@ -883,7 +883,7 @@ def __init__(
remote_port_forwards=None,
winrm=False,
ssh_options=None,
**kwargs
**kwargs,
):
# Get mine setting and mine_functions if defined in kwargs (from roster)
self.mine = mine
Expand Down Expand Up @@ -1017,9 +1017,7 @@ def check_thin_dir(self):
"""
check if the thindir exists on the remote machine
"""
stdout, stderr, retcode = self.shell.exec_cmd(
"test -d {}".format(self.thin_dir)
)
stdout, stderr, retcode = self.shell.exec_cmd(f"test -d {self.thin_dir}")
if retcode != 0:
return False
return True
Expand Down Expand Up @@ -1131,7 +1129,7 @@ def run_wfunc(self):
self.id,
fsclient=self.fsclient,
minion_opts=self.minion_opts,
**self.target
**self.target,
)

opts_pkg = pre_wrapper["test.opts_pkg"]() # pylint: disable=E1102
Expand Down Expand Up @@ -1217,7 +1215,7 @@ def run_wfunc(self):
self.id,
fsclient=self.fsclient,
minion_opts=self.minion_opts,
**self.target
**self.target,
)
wrapper.fsclient.opts["cachedir"] = opts["cachedir"]
self.wfuncs = salt.loader.ssh_wrapper(opts, wrapper, self.context)
Expand Down Expand Up @@ -1265,7 +1263,7 @@ def run_wfunc(self):
else:
result = self.wfuncs[self.fun](*self.args, **self.kwargs)
except TypeError as exc:
result = "TypeError encountered executing {}: {}".format(self.fun, exc)
result = f"TypeError encountered executing {self.fun}: {exc}"
log.error(result, exc_info_on_loglevel=logging.DEBUG)
retcode = 1
except Exception as exc: # pylint: disable=broad-except
Expand All @@ -1274,6 +1272,10 @@ def run_wfunc(self):
)
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:
Expand All @@ -1288,7 +1290,7 @@ def _cmd_str(self):
"""
if self.target.get("sudo"):
sudo = (
"sudo -p '{}'".format(salt.client.ssh.shell.SUDO_PROMPT)
f"sudo -p '{salt.client.ssh.shell.SUDO_PROMPT}'"
if self.target.get("passwd")
else "sudo"
)
Expand Down Expand Up @@ -1360,20 +1362,18 @@ def execute_script(self, script, extension="py", pre_dir="", script_args=None):
script_args = shlex.split(str(script_args))
args = " {}".format(" ".join([shlex.quote(str(el)) for el in script_args]))
if extension == "ps1":
ret = self.shell.exec_cmd('"powershell {}"'.format(script))
ret = self.shell.exec_cmd(f'"powershell {script}"')
else:
if not self.winrm:
ret = self.shell.exec_cmd(
"/bin/sh '{}{}'{}".format(pre_dir, script, args)
)
ret = self.shell.exec_cmd(f"/bin/sh '{pre_dir}{script}'{args}")
else:
ret = saltwinshell.call_python(self, script)

# Remove file from target system
if not self.winrm:
self.shell.exec_cmd("rm '{}{}'".format(pre_dir, script))
self.shell.exec_cmd(f"rm '{pre_dir}{script}'")
else:
self.shell.exec_cmd("del {}".format(script))
self.shell.exec_cmd(f"del {script}")

return ret

Expand Down Expand Up @@ -1461,7 +1461,7 @@ def cmd_block(self, is_retry=False):
while re.search(RSTR_RE, stderr):
stderr = re.split(RSTR_RE, stderr, 1)[1].strip()
else:
return "ERROR: {}".format(error), stderr, retcode
return f"ERROR: {error}", stderr, retcode

# FIXME: this discards output from ssh_shim if the shim succeeds. It should
# always save the shim output regardless of shim success or failure.
Expand Down Expand Up @@ -1521,7 +1521,7 @@ def cmd_block(self, is_retry=False):
# If RSTR is not seen in both stdout and stderr then there
# was a thin deployment problem.
return (
"ERROR: Failure deploying ext_mods: {}".format(stdout),
f"ERROR: Failure deploying ext_mods: {stdout}",
stderr,
retcode,
)
Expand Down Expand Up @@ -1689,7 +1689,7 @@ def mod_data(fsclient):
files = fsclient.file_list(env)
for ref in sync_refs:
mods_data = {}
pref = "_{}".format(ref)
pref = f"_{ref}"
for fn_ in sorted(files):
if fn_.startswith(pref):
if fn_.endswith((".py", ".so", ".pyx")):
Expand All @@ -1711,9 +1711,7 @@ def mod_data(fsclient):
ver_base = salt.utils.stringutils.to_bytes(ver_base)

ver = hashlib.sha1(ver_base).hexdigest()
ext_tar_path = os.path.join(
fsclient.opts["cachedir"], "ext_mods.{}.tgz".format(ver)
)
ext_tar_path = os.path.join(fsclient.opts["cachedir"], f"ext_mods.{ver}.tgz")
mods = {"version": ver, "file": ext_tar_path}
if os.path.isfile(ext_tar_path):
return mods
Expand Down Expand Up @@ -1762,7 +1760,7 @@ def _convert_args(args):
for key in list(arg.keys()):
if key == "__kwarg__":
continue
converted.append("{}={}".format(key, arg[key]))
converted.append(f"{key}={arg[key]}")
else:
converted.append(arg)
return converted
Loading