-
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
[master] Use salt-call from salt bundle with transactional_update #65204
base: master
Are you sure you want to change the base?
[master] Use salt-call from salt bundle with transactional_update #65204
Conversation
422ef4f
to
05be8e2
Compare
05be8e2
to
f6e54bc
Compare
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.
Looks good to me. Thanks
# Set default salt-call command | ||
salt_call_cmd = "salt-call" | ||
python_exec_dir = os.path.dirname(sys.executable) | ||
if "venv-salt-minion" in pathlib.Path(python_exec_dir).parts: | ||
# If the module is executed with the Salt Bundle, | ||
# use salt-call from the Salt Bundle | ||
salt_call_cmd = os.path.join(python_exec_dir, "salt-call") |
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.
Why is it needed to do the path juggling inside Python code, can't we prepend the "Salt Bundle" bin/ directory to $PATH
?
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.
Yes, we can do it with PATH outside, but actually I think to do it this way is more raliable as it's more strict and in case of handling transactional_update with salt-ssh it will push us to make the external solution more conditional
What does this PR do?
What issues does this PR fix or reference?
In some cases if both salt bundle and classic salt package installed on the same system, transactional_update module can wrongly execute
salt-call
of classic salt packge instead of salt bundle which was used to process the event.Previous Behavior
Wrong
salt-call
executed inside the transaction while usingtransactional_update
.New Behavior
In case if the event processed with salt from the bundle
transactional_update
will callsalt-call
from the bundle explicitly.Merge requirements satisfied?
[NOTICE] Bug fixes or features added to Salt require tests.
Commits signed with GPG?
Yes/No
Please review Salt's Contributing Guide for best practices.
See GitHub's page on GPG signing for more information about signing commits with GPG.