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

Detect when system needs reboot after domain join #49684

Merged
merged 4 commits into from Sep 20, 2018

Conversation

Projects
None yet
3 participants
@twangboy
Copy link
Contributor

commented Sep 17, 2018

What does this PR do?

Properly detects if the system requires reboot after Joining a domain.
Adds value_exists function
Uses key_exists instead of reading and failing

What issues does this PR fix or reference?

#49675

Tests written?

No

Commits signed with GPG?

Yes

Detect when system needs reboot after domain join
Adds value_exists function
Uses key_exists instead of reading and failing

@salt-jenkins salt-jenkins requested a review from saltstack/team-windows Sep 17, 2018

@twangboy twangboy added the 2018.3.4 label Sep 17, 2018

@twangboy

This comment has been minimized.

Copy link
Contributor Author

commented Sep 17, 2018

@rallytime Probably need to backport to 2017.7, but I'll need to create separate PR.

@ghost

This comment has been minimized.

Copy link

commented Sep 17, 2018

@twangboy , thanks for the fix.
There are still two more instances of the same error in the module. You also need to fix the get_pending_update and get_pending_component_servicing functions. You may as well fix everything at once.

dwoz and others added some commits Sep 18, 2018

Fix other functions in the same manner
- get_pending_update
- get_pending_component_servicing
@ghost

This comment has been minimized.

Copy link

commented Sep 18, 2018

@twangboy I am now happy to see this committed.

@rallytime

This comment has been minimized.

Copy link
Contributor

commented Sep 19, 2018

@twangboy The following windows test failures look related to me:

  • integration.modules.test_system.WinSystemModuleTest.test_get_computer_name
  • integration.modules.test_service.ServiceModuleTest.test_service_disable_doesnot_exist
  • integration.modules.test_service.ServiceModuleTest.test_service_enable

https://jenkinsci.saltstack.com/job/pr-windows2016-py3/job/PR-49684/3/

Can you fix those?

@ghost

This comment has been minimized.

Copy link

commented Sep 19, 2018

@rallytime, This is none of my business I suppose. I don't know how the testing works. Certainly, the get_computer_name function works for me (on Windows 10). (I haven't looked at the other two functions). How can one know the particulars of the test system in order to duplicate the environment? Can it be ascertained that the test system has no permissions issues and so forth?

The test issues may or may not be related to each other but I don't see how they're related to the bugs @twangboy just fixed

@twangboy

This comment has been minimized.

Copy link
Contributor Author

commented Sep 19, 2018

@rallytime Yeah, I just relaunched the tests, but I don't think the failures are related.

@rallytime

This comment has been minimized.

Copy link
Contributor

commented Sep 19, 2018

Ok, the PR tests for windows still need some ❤️ so I was trying to make an educated guess so we don't cause more failures. Let's see how the next run goes. Thanks for chiming in!

@rallytime rallytime merged commit d434e36 into saltstack:2018.3 Sep 20, 2018

5 of 10 checks passed

continuous-integration/jenkins/pr-merge This commit cannot be built
Details
jenkins/pr/py2-centos-7 The py2-centos-7 job has failed
Details
jenkins/pr/py2-windows-2016 The py2-windows-2016 job has failed
Details
jenkins/pr/py3-centos-7 The py3-centos-7 job has failed
Details
jenkins/pr/py3-windows-2016 The py3-windows-2016 job has failed
Details
WIP ready for review
Details
jenkins/pr/docs The docs job has passed
Details
jenkins/pr/lint The lint job has passed
Details
jenkins/pr/py2-ubuntu-1604 The py2-ubuntu-1604 job has passed
Details
jenkins/pr/py3-ubuntu-1604 The py3-ubuntu-1604 job has passed
Details

@twangboy twangboy deleted the twangboy:fix_49675 branch Sep 20, 2018

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.