Skip to content
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

Fix mac softwareupdate for catalina #56191

Merged

Conversation

sheagcraig
Copy link
Contributor

What does this PR do?

This PR fixes the mac_softwareupdate.list_available func to address changes in the output of the macOS /usr/sbin/softwareupdate tool. It handles two issues:

  1. macOS Catalina changes the structure of the output.
  2. At some point Apple added a "shut down" action to some updates (as opposed to "restart").

This PR switches on OS version to determine which regular expression to use, while adding the ability to filter updates by the new "shut down" action.

This PR was already accepted as #54281, but I was asked to rebase against master. So here we are!

What issues does this PR fix or reference?

#54220

Previous Behavior

mac_softwareupdate.list_available would not find any available updates on Catalina.
mac_softwareupdate.list_available was not able to filter for shut down type updates.

New Behavior

mac_softwareupdate.list_available has functional parity for the newer output format on Catalina.
mac_softwareupdate.list_available now has an arg to filter for shut down type updates.

Tests written?

Yes

Commits signed with GPG?

No

@sheagcraig sheagcraig requested a review from a team as a code owner February 18, 2020 15:06
@ghost ghost requested a review from waynew February 18, 2020 15:07
@sheagcraig
Copy link
Contributor Author

@weswhet please to be checking for correctness in this 2020 we find ourselves in.

@weswhet
Copy link
Contributor

weswhet commented Feb 18, 2020

LGTM. ✅

@sheagcraig
Copy link
Contributor Author

🎉

Copy link
Contributor

@Ch3LL Ch3LL left a comment

Choose a reason for hiding this comment

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

looks like there is a test and lint failure

@sheagcraig
Copy link
Contributor Author

@Ch3LL I don't know what to do here; the lint error is from a file that isn't touched by this PR.

@Ch3LL
Copy link
Contributor

Ch3LL commented Mar 2, 2020

you are totally right, i'll take a look at that

@waynew
Copy link
Contributor

waynew commented Mar 10, 2020

@sheagcraig FWIW I think we've figured out the lint failure. I don't believe the tests are in any way related to your changes though :suspect:

@sheagcraig
Copy link
Contributor Author

Just looking over a handful of the ci/py2/macosoxmojave and ci/py2/amazon2 test failures, I agree; I don't think my changes have anything to do with this. I'll let someone else of course be the decider on that, as that's a lot of tests that I'm not familiar with to have to wade through. Please let me know what you find out!

@waynew waynew self-assigned this Mar 18, 2020
waynew
waynew previously approved these changes Mar 18, 2020
Copy link
Contributor

@waynew waynew left a comment

Choose a reason for hiding this comment

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

@sheagcraig looks like tests are all passing!

@Ch3LL do you have any other issues with this PR?

Ch3LL
Ch3LL previously approved these changes Mar 26, 2020
@sheagcraig sheagcraig dismissed stale reviews from Ch3LL and waynew via 55cd6bb May 19, 2020 17:23
@sheagcraig
Copy link
Contributor Author

@Ch3LL I just fixed the merge conflict that appeared; I guess I need a new review! Thanks!

@dwoz
Copy link
Contributor

dwoz commented May 22, 2020

@sheagcraig The pre-commit needs to be addressed an the tests need to pass for this to make it into Sodium

@weswhet
Copy link
Contributor

weswhet commented May 22, 2020

@dwoz should be good to go :)

@weswhet
Copy link
Contributor

weswhet commented May 22, 2020

I needed to squash all the commits into one as they weren't signed. @garethgreenaway mind having a look again?

@dwoz dwoz merged commit 5340f3c into saltstack:master May 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ZRelease-Sodium retired label
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants