-
Notifications
You must be signed in to change notification settings - Fork 5.5k
46255 fix minion.restart #68225
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
Draft
barneysowood
wants to merge
4
commits into
saltstack:3006.x
Choose a base branch
from
barneysowood:46255-fix-minion-restart
base: 3006.x
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
46255 fix minion.restart #68225
barneysowood
wants to merge
4
commits into
saltstack:3006.x
from
barneysowood:46255-fix-minion-restart
Conversation
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
Updates minion.restart to use service.restart if system uses systemd or is a Windows system. Existing methods don't work if salt-minion is managed by the Windows service manager or systemd. This adds functionality to check for those and use service.restart if so.
Adds options to enable use of systemd or windows service to restart the salt minion. Both options default to True as we should prefer to use the service managers, but give the option to disable where users are doing something unusual like running another service supervisor.
Adds unit tests for the minion.restart function to test existing and new functionality. Also adds tests for helper functions.
175cf4f
to
093e95f
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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?
Adds support to
minion.restart
to detect if the minion is running under systemd or the Windows service manager. If so, it will useservice.restart
to restart the minion rather than killing and attempting to restart it.If
minion_restart_command
is set it will follow the old behaviour and use that command to handle the restart.If
minion_restart_command
isn't set and systemd/windows aren't detected, preserves the old behaviour of looking atsys.argv
and either just killing the minion process or killing it and attempting to restart the process with the same args if-d
(for daemon mode) is insys.argv
. Whilst I can see that the former option may be useful if running the minion under another process supervisor, I'm not really sure how the killing and starting the a new process could ever work... I've left it in for the moment, but it should maybe be removed in 3008.What issues does this PR fix or reference?
Fixes #46255
Previous Behavior
Calling
minion.restart
on a systemd based system would not work - it would just kill the minion processNew Behavior
Calling
minion.restart on a systemd based system will now use
service.restart` to restart the minion process.Merge requirements satisfied?
[NOTICE] Bug fixes or features added to Salt require tests.
Commits signed with SSH?
Yes