Skip to content

Improve macOS service naming support #57646

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

Merged
merged 9 commits into from
Sep 30, 2020

Conversation

weswhet
Copy link
Contributor

@weswhet weswhet commented Jun 12, 2020

What does this PR do?

This PR makes it so if you specify a service on macOS by the naming convention similar to that of other platforms like salt-minion it will convert it over to its proper name com.saltstack.salt.minion. It will do the same for salt-master, salt-api, salt-syndic.

What issues does this PR fix or reference?

Fixes:

I've seen multi times where people mention that documentation mentions restarting a service by running salt * service.restart salt-minion they do this on macOS not knowing that the naming convention is different. This will automatically handle it for them.

Helps support the Formula PR here.

Previous Behavior

macOS would fail with Service not found: salt-master

New Behavior

macOS will now know which service to look for and perform the correct action.

Merge requirements satisfied?

[NOTICE] Bug fixes or features added to Salt require tests.

weswhet added 2 commits June 11, 2020 17:11
…r the module converts over to the macOS naming convention of com.saltstack.salt.master
@weswhet weswhet requested a review from a team as a code owner June 12, 2020 00:27
@ghost ghost requested review from DmitryKuzmenko and removed request for a team June 12, 2020 00:27
@weswhet
Copy link
Contributor Author

weswhet commented Jun 12, 2020

Test failure isn't related to this PR.

@weswhet
Copy link
Contributor Author

weswhet commented Jun 12, 2020

re-run ci/py3/ubuntu1604

@weswhet
Copy link
Contributor Author

weswhet commented Jun 12, 2020

re-run ubuntu1604

noelmcloughlin
noelmcloughlin previously approved these changes Jun 13, 2020
Copy link
Contributor

@noelmcloughlin noelmcloughlin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. We need to get MacOS added to CI sometime soon.

DmitryKuzmenko
DmitryKuzmenko previously approved these changes Jun 17, 2020
@DmitryKuzmenko
Copy link
Contributor

@weswhet thank you for contribution! I'm good with this but since I'm not very familiar to MacOS specifics I want also @garethgreenaway to review this.

@weswhet
Copy link
Contributor Author

weswhet commented Jun 17, 2020

Thanks @DmitryKuzmenko @garethgreenaway!

@garethgreenaway
Copy link
Contributor

@weswhet No worries. Can we get a changelog file with the changes documented?

@weswhet
Copy link
Contributor Author

weswhet commented Jul 6, 2020

@garethgreenaway thanks! Added the changelog for this fix.

@sagetherage sagetherage linked an issue Jul 6, 2020 that may be closed by this pull request
@krionbsd krionbsd added the MacOS pertains to the OS of fruit label Jul 7, 2020
s0undt3ch
s0undt3ch previously approved these changes Jul 7, 2020
@sagetherage sagetherage added Magnesium Mg release after Na prior to Al Bug broken, incorrect, or confusing behavior severity-high 2nd top severity, seen by most users, causes major problems labels Jul 22, 2020
@weswhet
Copy link
Contributor Author

weswhet commented Sep 29, 2020

another all green one for ya @garethgreenaway 💚

@dwoz dwoz merged commit a8a7eec into saltstack:master Sep 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug broken, incorrect, or confusing behavior has-failing-test MacOS pertains to the OS of fruit Magnesium Mg release after Na prior to Al severity-high 2nd top severity, seen by most users, causes major problems
Projects
None yet
Development

Successfully merging this pull request may close these issues.

macOS salt-minion service not found
8 participants