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

Add Win WUSA management, to install Windows Update files (.msu). #50397

Merged
merged 6 commits into from Nov 16, 2018

Conversation

Projects
None yet
5 participants
@tlemarchand
Copy link
Contributor

commented Nov 6, 2018

What does this PR do?

This PR adds an execution module and a state module to manage .msu files with WUSA.
With this state we can ensure that a specific KB is installed, even if Windows Update is disabled.
I've chosen to add a win_wusa module and state, and not to extend win_wua, because when Windows Update is disabled centrally, win_wua module is not functioning correctly.

What issues does this PR fix or reference?

#46292

Tests written?

No

Commits signed with GPG?

No

@salt-jenkins salt-jenkins requested a review from saltstack/team-windows Nov 6, 2018

@twangboy
Copy link
Contributor

left a comment

Just a few minor changes here... Looks awesome!

Additionally, we need to get tests in here if possible.

Show resolved Hide resolved salt/states/win_wusa.py Outdated
Show resolved Hide resolved salt/states/win_wusa.py Outdated
Show resolved Hide resolved salt/modules/win_wusa.py Outdated
Show resolved Hide resolved salt/modules/win_wusa.py
Show resolved Hide resolved salt/modules/win_wusa.py
pchanges to changes
named parameters
@tlemarchand

This comment has been minimized.

Copy link
Contributor Author

commented Nov 7, 2018

@twangboy : you are right about tests. I've never developed tests for Salt though, not sure how to do that.

@damon-atkins
Copy link
Member

left a comment

Pls also add list_patches or list_kb

Show resolved Hide resolved salt/modules/win_wusa.py Outdated
Disable powershell modules list
Add list_kbs function
@twangboy

This comment has been minimized.

Copy link
Contributor

commented Nov 7, 2018

@tlemarchand There are two types of tests we do. Unit tests and integration tests.

In unit tests most things are mocked and logic is tested. They're very fast but more difficult to write, especially on Windows if you're mocking API calls.

Integration tests are a little easier to write as they basically call your function an then verify what is returned or set through another means. They're much slower since they spin up an entire salt environment (master, sub-master, minion, sub-minion, syndic).

You can look at examples in salt/tests. You'll find directories for unit and integration tests. Under those you'll find folders for modules and states. You should be able to find some examples in there to get you started. They all start with test_. Probably look at some of the test_win_* tests that work similarly, ie: shelling out to powershell.

@twangboy

This comment has been minimized.

Copy link
Contributor

commented Nov 15, 2018

@rallytime Can we go ahead with this. I can work on tests for it.

@damon-atkins
Copy link
Member

left a comment

👍

@rallytime
Copy link
Contributor

left a comment

I have some small requests here.

Show resolved Hide resolved salt/modules/win_wusa.py Outdated
Show resolved Hide resolved salt/modules/win_wusa.py
Show resolved Hide resolved salt/states/win_wusa.py
Show resolved Hide resolved salt/states/win_wusa.py Outdated
@damon-atkins

This comment has been minimized.

Copy link
Member

commented Nov 15, 2018

Surprised unicode_literals is not in the pylint checks.

@tlemarchand

This comment has been minimized.

Copy link
Contributor Author

commented Nov 15, 2018

@rallytime Can we go ahead with this. I can work on tests for it.

@twangboy That would be great, I won't have time to develop tests before 2 or 3 weeks, at least, unfortunately.

I'll implement changes asked by @rallytime today however.

@rallytime

This comment has been minimized.

Copy link
Contributor

commented Nov 15, 2018

@rallytime

This comment has been minimized.

Copy link
Contributor

commented Nov 15, 2018

I've added the back-port label here as we need this in fluorine. @twangboy is going to take care of that once this original PR is merged.

@tlemarchand

This comment has been minimized.

Copy link
Contributor Author

commented Nov 15, 2018

Oh, hey @tlemarchand - this is failing a test because you haven't added some required documentation. Can you fix that up?

https://jenkinsci.saltstack.com/job/pr-kitchen-windows2016-py2/job/PR-50397/7/testReport/junit/integration.modules.test_sysmod/SysModuleTest/test_valid_docs/

No problem. I'll do that tomorrow or monday.

@rallytime rallytime merged commit 03d91a0 into saltstack:develop Nov 16, 2018

11 checks passed

WIP Ready for review
Details
codeclimate Approved by Mike Place.
Details
continuous-integration/jenkins/pr-merge This commit looks good
Details
jenkins/pr/docs The docs job has passed
Details
jenkins/pr/lint The lint job has passed
Details
jenkins/pr/py2-centos-7 The py2-centos-7 job has passed
Details
jenkins/pr/py2-ubuntu-1604 The py2-ubuntu-1604 job has passed
Details
jenkins/pr/py2-windows-2016 The py2-windows-2016 job has passed
Details
jenkins/pr/py3-centos-7 The py3-centos-7 job has passed
Details
jenkins/pr/py3-ubuntu-1604 The py3-ubuntu-1604 job has passed
Details
jenkins/pr/py3-windows-2016 The py3-windows-2016 job has passed
Details
@rallytime

This comment has been minimized.

Copy link
Contributor

commented Nov 16, 2018

@twangboy this is good to go now with the back-port and tests.

cachedout pushed a commit that referenced this pull request Nov 19, 2018

Mike Place
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.