Skip to content

Commit fe0eef4

Browse files
committed
Align sshresource_state with upstream relenv improvements
The salt-ssh relenv work merged into master introduced three changes that required corresponding updates in our SSH resource module: 1. Single.cmd_block() now calls mod_data(self.fsclient) unconditionally to regenerate extension modules before every remote execution. Passing fsclient=None (the default) crashes immediately at fsclient.opts. _exec_state_pkg() now creates a fresh FSClient from the cached master opts and passes it via the fsclient= kwarg to Single. 2. Single.__init__ renames thin_dir from the default *_salt suffix to *_salt_relenv when relenv=True, then writes the updated path back into opts["thin_dir"]. The previous code built the state.pkg argv string before constructing Single, so the baked-in path diverged from where the state tar was actually sent. The argv is now set after __init__ so both the send path and the command string use the finalized thin_dir. 3. _relenv_path() hardcoded linux/x86_64, which would silently deploy the wrong tarball to arm64 hosts. It now probes x86_64 then arm64 and returns the first path that exists locally, or None. A None return tells Single.__init__ to call detect_os_arch() itself — one extra SSH round-trip as a fallback, but always correct. Also removes the leftover debug echo from SSH_SH_SHIM_RELENV that was writing SALT_CALL_CMD diagnostics to stderr on every remote execution. Tests added: TestRelenvPath (4 cases), fsclient mock added to all _exec_state_pkg test helpers, and test_single_receives_fsclient verifies the kwarg is threaded through.
1 parent fac401f commit fe0eef4

3 files changed

Lines changed: 114 additions & 12 deletions

File tree

salt/client/ssh/__init__.py

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -277,9 +277,6 @@
277277
echo "{RSTR}"
278278
echo "{RSTR}" >&2
279279
280-
# Debug: Show the actual command being executed
281-
echo "SALT_CALL_CMD: $SALT_CALL_BIN --retcode-passthrough --local --metadata --out=json -lquiet -c {THIN_DIR} -- {ARGS}" >&2
282-
283280
exec $SUDO "$SALT_CALL_BIN" --retcode-passthrough --local --metadata --out=json -lquiet -c "{THIN_DIR}" -- {ARGS}
284281
EOF
285282
""".split(

salt/modules/sshresource_state.py

Lines changed: 31 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -75,9 +75,21 @@ def _host_cfg():
7575

7676

7777
def _relenv_path():
78-
"""Return the local path of our pre-built custom relenv tarball."""
78+
"""
79+
Return the path to a pre-built relenv tarball if one exists locally, otherwise
80+
return ``None`` to let ``Single.__init__`` detect the remote arch and download
81+
the correct tarball itself.
82+
83+
The tarball is generated by ``salt.utils.relenv.gen_relenv``, which normalises
84+
the arch to ``x86_64`` or ``arm64`` before building the cache path, so the
85+
lookup uses those canonical names.
86+
"""
7987
cachedir = __opts__.get("cachedir", "") # pylint: disable=undefined-variable
80-
return os.path.join(cachedir, "relenv", "linux", "x86_64", "salt-relenv.tar.xz")
88+
for arch in ("x86_64", "arm64"):
89+
path = os.path.join(cachedir, "relenv", "linux", arch, "salt-relenv.tar.xz")
90+
if os.path.exists(path):
91+
return path
92+
return None
8193

8294

8395
def _target_opts():
@@ -429,23 +441,33 @@ def _exec_state_pkg(opts, trans_tar, test):
429441
430442
Returns the state result dict directly (what the minion dispatcher
431443
expects) rather than the full ``{"local": {"return": ...}}`` envelope.
444+
445+
A fresh file client is created here so that ``Single.cmd_block()`` can call
446+
``mod_data(fsclient)`` to scan for extension modules. (``cmd_block`` was
447+
updated in the relenv improvements merge to regenerate ext-mods before every
448+
remote execution.)
432449
"""
450+
fsclient = _file_client()
433451
try:
434452
trans_tar_sum = salt.utils.hashutils.get_hash(trans_tar, opts["hash_type"])
435-
cmd = "state.pkg {thin_dir}/salt_state.tgz test={test} pkg_sum={pkg_sum} hash_type={hash_type}".format(
436-
thin_dir=opts["thin_dir"],
437-
test=test,
438-
pkg_sum=trans_tar_sum,
439-
hash_type=opts["hash_type"],
440-
)
441453
single = salt.client.ssh.Single(
442454
opts,
443-
cmd,
455+
"state.pkg", # placeholder; argv is updated after __init__ rewrites thin_dir
444456
_resource_id(),
445457
thin=_relenv_path(),
446458
thin_dir=opts["thin_dir"],
459+
fsclient=fsclient,
447460
**_connection_kwargs(),
448461
)
462+
# Single.__init__ may rename thin_dir (e.g. _salt → _salt_relenv) and
463+
# writes the result back into opts["thin_dir"]. Build the real argv only now.
464+
cmd = "state.pkg {thin_dir}/salt_state.tgz test={test} pkg_sum={pkg_sum} hash_type={hash_type}".format(
465+
thin_dir=opts["thin_dir"],
466+
test=test,
467+
pkg_sum=trans_tar_sum,
468+
hash_type=opts["hash_type"],
469+
)
470+
single.argv = [cmd]
449471
single.shell.send(trans_tar, "{}/salt_state.tgz".format(opts["thin_dir"]))
450472
stdout, stderr, retcode = single.cmd_block()
451473
finally:

tests/pytests/unit/modules/test_sshresource_state.py

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,44 @@
4242
_BASE_RESOURCE = {"id": _RESOURCE_ID, "type": "ssh"}
4343

4444

45+
# ---------------------------------------------------------------------------
46+
# _relenv_path(): returns tarball or None
47+
# ---------------------------------------------------------------------------
48+
49+
50+
class TestRelenvPath:
51+
"""_relenv_path() returns the first existing tarball or None."""
52+
53+
def _run(self, existing_files=()):
54+
import salt.modules.sshresource_state as mod
55+
56+
opts = _BASE_OPTS.copy()
57+
with patch.object(mod, "__opts__", opts, create=True), patch.object(
58+
mod, "__resource__", dict(_BASE_RESOURCE), create=True
59+
), patch.object(mod, "__context__", {}, create=True), patch.object(
60+
mod, "__salt__", {}, create=True
61+
), patch(
62+
"os.path.exists", side_effect=lambda p: p in existing_files
63+
):
64+
return mod._relenv_path()
65+
66+
def test_returns_x86_64_when_present(self):
67+
path = "/tmp/relenv/linux/x86_64/salt-relenv.tar.xz"
68+
assert self._run(existing_files=(path,)) == path
69+
70+
def test_returns_arm64_when_present(self):
71+
path = "/tmp/relenv/linux/arm64/salt-relenv.tar.xz"
72+
assert self._run(existing_files=(path,)) == path
73+
74+
def test_returns_none_when_no_tarball(self):
75+
assert self._run(existing_files=()) is None
76+
77+
def test_prefers_x86_64_over_arm64(self):
78+
x86 = "/tmp/relenv/linux/x86_64/salt-relenv.tar.xz"
79+
arm = "/tmp/relenv/linux/arm64/salt-relenv.tar.xz"
80+
assert self._run(existing_files=(x86, arm)) == x86
81+
82+
4583
def _make_ssh_error(parsed):
4684
"""Build a fake SSHCommandExecutionError with .parsed attribute."""
4785
import salt.client.ssh.wrapper
@@ -139,6 +177,8 @@ def _run(self, exc_parsed):
139177
mod, "_resource_id", return_value=_RESOURCE_ID
140178
), patch.object(
141179
mod, "_relenv_path", return_value="/tmp/relenv.tar.xz"
180+
), patch.object(
181+
mod, "_file_client", return_value=MagicMock()
142182
), patch.object(
143183
mod, "_connection_kwargs", return_value={}
144184
), patch(
@@ -200,6 +240,8 @@ def test_reraises_when_parsed_is_none(self):
200240
mod, "_resource_id", return_value=_RESOURCE_ID
201241
), patch.object(
202242
mod, "_relenv_path", return_value="/tmp/relenv.tar.xz"
243+
), patch.object(
244+
mod, "_file_client", return_value=MagicMock()
203245
), patch.object(
204246
mod, "_connection_kwargs", return_value={}
205247
), patch(
@@ -242,6 +284,8 @@ def _run(self, envelope):
242284
mod, "_resource_id", return_value=_RESOURCE_ID
243285
), patch.object(
244286
mod, "_relenv_path", return_value="/tmp/relenv.tar.xz"
287+
), patch.object(
288+
mod, "_file_client", return_value=MagicMock()
245289
), patch.object(
246290
mod, "_connection_kwargs", return_value={}
247291
), patch(
@@ -276,3 +320,42 @@ def test_zero_retcode_does_not_set_context_retcode(self):
276320
envelope = {"return": _VALID_STATE_RETURN, "retcode": 0}
277321
_, context = self._run(envelope)
278322
assert context.get("retcode", 0) == 0
323+
324+
def test_single_receives_fsclient(self):
325+
"""Single must be constructed with a fsclient so cmd_block can call mod_data."""
326+
import salt.modules.sshresource_state as mod
327+
328+
opts = _BASE_OPTS.copy()
329+
mock_fsclient = MagicMock()
330+
331+
with patch.object(mod, "__opts__", opts, create=True), patch.object(
332+
mod, "__resource__", dict(_BASE_RESOURCE), create=True
333+
), patch.object(mod, "__context__", {}, create=True), patch.object(
334+
mod, "__salt__", {}, create=True
335+
), patch.object(
336+
mod, "_resource_id", return_value=_RESOURCE_ID
337+
), patch.object(
338+
mod, "_relenv_path", return_value="/tmp/relenv.tar.xz"
339+
), patch.object(
340+
mod, "_file_client", return_value=mock_fsclient
341+
), patch.object(
342+
mod, "_connection_kwargs", return_value={}
343+
), patch(
344+
"salt.utils.hashutils.get_hash", return_value="abc123"
345+
), patch(
346+
"os.remove"
347+
), patch(
348+
"salt.client.ssh.Single"
349+
) as mock_single_cls, patch(
350+
"salt.client.ssh.wrapper.parse_ret",
351+
return_value={"return": _VALID_STATE_RETURN, "retcode": 0},
352+
):
353+
mock_single = MagicMock()
354+
mock_single.cmd_block.return_value = ("", "", 0)
355+
mock_single.shell = MagicMock()
356+
mock_single_cls.return_value = mock_single
357+
358+
mod._exec_state_pkg(opts, "/tmp/fake.tgz", False)
359+
360+
_, kwargs = mock_single_cls.call_args
361+
assert kwargs.get("fsclient") is mock_fsclient

0 commit comments

Comments
 (0)