Stop embedding master opts in relenv minion config to fix ARG_MAX - #70194
Open
twangboy wants to merge 1 commit into
Open
Stop embedding master opts in relenv minion config to fix ARG_MAX#70194twangboy wants to merge 1 commit into
twangboy wants to merge 1 commit into
Conversation
salt.client.ssh.Single.__init__ set self.minion_opts["__master_opts__"] = self.context["master_opts"] for relenv targets, embedding the master's entire own config (395 keys in a typical test master) into the minion config file shipped to and read by salt-call on the remote target. __master_opts__ is a master-side-only convention: every other reader of it (salt/client/ssh/wrapper/cmdmod.py, cp.py, publish.py, salt/client/ssh/state.py, salt/roster/__init__.py) pulls it from the Python wrapper opts dict while running on the master; nothing on the remote target ever reads it back out of its own minion config. self.context["master_opts"] is an alias for the master's own opts, which gets mutated as nested Single/wrapper calls restore/adjust the master cachedir (saltstack#69605, saltstack#68458). Embedding it in the relenv minion config meant that config grew, unbounded, with every nested Single created during a single state run, until it exceeded the kernel's ARG_MAX and the ssh invocation failed with "Argument list too long". Fixes saltstack#70186.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
salt.client.ssh.Single.__init__setself.minion_opts["__master_opts__"] = self.context["master_opts"]for relenv targets, embedding the master's entire own config (395 keys in a typical test master) into the minion config file shipped to and read bysalt-callon the remote target.__master_opts__is a master-side-only convention: every other reader of it (salt/client/ssh/wrapper/cmdmod.py,cp.py,publish.py,salt/client/ssh/state.py,salt/roster/__init__.py) pulls it from the Python wrapper opts dict while running on the master; nothing on the remote target ever reads it back out of its own minion config.self.context["master_opts"]is an alias for the master's own opts, which gets mutated as nestedSingle/wrapper calls restore/adjust the master cachedir (#69605, #68458). Embedding it in the relenv minion config meant that config grew, unbounded, with every nestedSinglecreated during a single state run, until it exceeded the kernel'sARG_MAXand the ssh invocation failed with "Argument list too long".Root-caused via a temporary debug patch tracing argv sizes across a full CI run — see #70186 for the full write-up and evidence (argv growth measured at ~20 KB → ~120 KB → ~207 KB within a single
state.applycall before failing).What issues does this PR fix or reference?
Fixes #70186
Merge requirements satisfied?
[NOTICE] Bug fixes or features added to Salt require tests.
Commits signed with GPG?
Yes