-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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.
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
[3006.x] Fix parallel state execution with Salt-SSH #66517
base: 3006.x
Are you sure you want to change the base?
Conversation
if jid is not None: | ||
__invocation_id = jid |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if jid is not None: | |
__invocation_id = jid | |
__invocation_id = jid |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I quickly applied this, but then noticed this will likely break parallel execution on spawning platforms: Each subprocess then generates its own __invocation_jid
since jid
is always None
, but we need it to be the same to find the cached return.
The tests will not catch this though since SSH tests only run against forking platforms.
What does this PR do?
Fixes a TypeError that was raised when executing parallel states via Salt-SSH (jid was unset, but the cache dir used for returning the result of a parallel execution depended on it).
What issues does this PR fix or reference?
Fixes: #66514
Fixes: #53538
Previous Behavior
Exits with retcode=0, shows
sys.doc
ofstate.pkg
because of a TypeError (on <3007) / at least errors with retcode=1 and shows the stacktrace (>=3007).New Behavior
Executes the states in parallel as requested.
Merge requirements satisfied?
[NOTICE] Bug fixes or features added to Salt require tests.
Commits signed with GPG?
Yes