Description
salt.client.ssh.Single.run_wfunc() always calls self._run_wfunc_thin(), even when the deployment type is relenv:
def run_wfunc(self):
"""
...
"""
return self._run_wfunc_thin()
Single._run_wfunc_relenv() already exists as a complete implementation specifically intended for this case -- its own docstring says "Execute a function using salt-call from relenv deployment. Bypasses the wrapper system entirely since relenv includes a full salt-call binary." It is never called from anywhere in the codebase.
Why this isn't a trivial one-line fix
Wiring up _run_wfunc_relenv() naively (dispatch on self.opts.get("relenv")) was attempted while investigating #70186 and found to be unsafe as-is:
- No deploy-bootstrap fallback.
self.deploy() (which sends the relenv tarball and, via cmd_block()'s SCP logic, the minion config) is only ever triggered from inside cmd_block()'s "undefined SHIM state" error-detection/retry logic. _run_wfunc_relenv() has no equivalent -- it just runs {thin_dir}/salt-call and assumes the target is already deployed. Routing state.apply/etc. straight to it would break the very first command against a fresh target.
- Config-dir mismatch.
_run_wfunc_relenv() passes --config-dir={thin_dir}/conf to salt-call, but the actual shim (SSH_SH_SHIM_RELENV) writes the minion config to {thin_dir}/minion and invokes salt-call -c "{THIN_DIR}" (i.e. config lives at {thin_dir}, not {thin_dir}/conf). _run_wfunc_relenv()'s own config path appears to have never been exercised/tested.
- Some wrapper functions may be master-side-only.
salt/client/ssh/wrapper/state.py (and others) are heavily __context__["fileclient"]-dependent -- pillar/state compilation happens on the master using the master's fileserver access, which a bare salt-call --local on the target cannot replicate. It's not obvious which wrapper functions are safe to bypass this way without wrapper-by-wrapper review.
Suggested next step
Either: (a) fix _run_wfunc_relenv()'s config-dir path and add deploy-bootstrap/retry logic so it can safely replace _run_wfunc_thin() for the functions where that's valid (likely just state.*), or (b) remove _run_wfunc_relenv() entirely as unused/superseded dead code if no one intends to finish wiring it up.
Related
Description
salt.client.ssh.Single.run_wfunc()always callsself._run_wfunc_thin(), even when the deployment type isrelenv:Single._run_wfunc_relenv()already exists as a complete implementation specifically intended for this case -- its own docstring says "Execute a function using salt-call from relenv deployment. Bypasses the wrapper system entirely since relenv includes a full salt-call binary." It is never called from anywhere in the codebase.Why this isn't a trivial one-line fix
Wiring up
_run_wfunc_relenv()naively (dispatch onself.opts.get("relenv")) was attempted while investigating #70186 and found to be unsafe as-is:self.deploy()(which sends the relenv tarball and, viacmd_block()'s SCP logic, the minion config) is only ever triggered from insidecmd_block()'s "undefined SHIM state" error-detection/retry logic._run_wfunc_relenv()has no equivalent -- it just runs{thin_dir}/salt-calland assumes the target is already deployed. Routingstate.apply/etc. straight to it would break the very first command against a fresh target._run_wfunc_relenv()passes--config-dir={thin_dir}/conftosalt-call, but the actual shim (SSH_SH_SHIM_RELENV) writes the minion config to{thin_dir}/minionand invokessalt-call -c "{THIN_DIR}"(i.e. config lives at{thin_dir}, not{thin_dir}/conf)._run_wfunc_relenv()'s own config path appears to have never been exercised/tested.salt/client/ssh/wrapper/state.py(and others) are heavily__context__["fileclient"]-dependent -- pillar/state compilation happens on the master using the master's fileserver access, which a baresalt-call --localon the target cannot replicate. It's not obvious which wrapper functions are safe to bypass this way without wrapper-by-wrapper review.Suggested next step
Either: (a) fix
_run_wfunc_relenv()'s config-dir path and add deploy-bootstrap/retry logic so it can safely replace_run_wfunc_thin()for the functions where that's valid (likely juststate.*), or (b) remove_run_wfunc_relenv()entirely as unused/superseded dead code if no one intends to finish wiring it up.Related
__master_opts__embedding issue, fixed in Stop embedding master opts in relenv minion config to fix ARG_MAX #70194. This dispatch gap is real but was not the cause of that bug.