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 exception handling to win_domain_controller #58234

Merged
merged 4 commits into from Aug 18, 2019
Merged

Conversation

agowa
Copy link
Contributor

@agowa agowa commented Jun 22, 2019

SUMMARY

Add error handling for two exceptions.

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

win_domain_controller

ADDITIONAL INFORMATION

If a user specifies a domain to join that cannot be resolved by dns, the Install-ADDSDomainController command throws an Microsoft.ADRoles.Deployment.Common.Tests.TestFailedException exception.

If the module is either invoked twice without a reboot, or a reboot is pending for another reason the Install-ADDSDomainController command throws an Microsoft.DirectoryServices.Deployment.DCPromoExecutionException exception with Errorcode 15.

I added error handling for both of them, so that a user can handle them from within the playbook.

@ansibot
Copy link
Contributor

ansibot commented Jun 22, 2019

@ansibot ansibot added affects_2.9 This issue/PR affects Ansible v2.9 bug This issue/PR relates to a bug. core_review In order to be merged, this PR must follow the core review workflow. module This issue/PR relates to a module. needs_triage Needs a first human triage before being processed. support:core This issue/PR relates to code supported by the Ansible Engineering Team. windows Windows community labels Jun 22, 2019
@samdoran samdoran removed the needs_triage Needs a first human triage before being processed. label Jun 25, 2019
@jborean93
Copy link
Contributor

Also it would be great if you could add a changelog similar to https://github.com/ansible/ansible/pull/53480/files#diff-0b9ea3a63f047de3086e081e4eb479f2R3 for the module.

@ansibot ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Jul 4, 2019
@agowa agowa closed this Jul 5, 2019
@agowa agowa reopened this Jul 5, 2019
@agowa
Copy link
Contributor Author

agowa commented Jul 5, 2019

closing and reopening to trigger the ci

@ansibot ansibot added support:community This issue/PR relates to code supported by the Ansible community. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. core_review In order to be merged, this PR must follow the core review workflow. labels Jul 5, 2019
@samdoran
Copy link
Contributor

This needs to be rebased in order for tests to run properly.

@samdoran samdoran added the ci_verified Changes made in this PR are causing tests to fail. label Jul 11, 2019
@ansibot ansibot added the needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html label Jul 11, 2019
@ansibot ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Jul 20, 2019
@ansibot
Copy link
Contributor

ansibot commented Jul 24, 2019

The test ansible-test sanity --test pslint [explain] failed with 1 error:

lib/ansible/modules/windows/win_domain_controller.ps1:217:21: PSUseDeclaredVarsMoreThanAssignments The variable 'install_result' is assigned but never used.

click here for bot help

@ansibot ansibot added has_issue and removed needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. ci_verified Changes made in this PR are causing tests to fail. labels Jul 24, 2019
@ansibot ansibot added core_review In order to be merged, this PR must follow the core review workflow. and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Jul 24, 2019
@agowa agowa force-pushed the patch-12 branch 2 times, most recently from b2ae13f to 304af42 Compare July 24, 2019 15:40
@ansibot ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Aug 8, 2019
@jborean93
Copy link
Contributor

rebuild_merge

@jborean93
Copy link
Contributor

Looks like the bot is having issues, manually rerunning tests to ensure it isn't stale before merging.

@ansibot ansibot added shipit This PR is ready to be merged by Core and removed core_review In order to be merged, this PR must follow the core review workflow. stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. labels Aug 18, 2019
@ansibot ansibot merged commit 811153a into ansible:devel Aug 18, 2019
@jborean93
Copy link
Contributor

Feel free to backport this to stable-2.8 if you wish https://docs.ansible.com/ansible/latest/community/development_process.html#backporting-merged-prs.

agowa pushed a commit to agowa/ansible-1 that referenced this pull request Aug 19, 2019
* Add exception handling to win_domain_controller

* Add changelog

* Fix PSUseDeclaredVarsMoreThanAssignments

* Remove dns domain cannot be resolved error message

As requested by PR review in ansible#58234 (comment)
@agowa agowa deleted the patch-12 branch August 19, 2019 08:36
agowa pushed a commit to agowa/ansible-1 that referenced this pull request Sep 4, 2019
* Add exception handling to win_domain_controller

* Add changelog

* Fix PSUseDeclaredVarsMoreThanAssignments

* Remove dns domain cannot be resolved error message

As requested by PR review in ansible#58234 (comment)
abadger pushed a commit that referenced this pull request Sep 4, 2019
* Add exception handling to win_domain_controller

* Add changelog

* Fix PSUseDeclaredVarsMoreThanAssignments

* Remove dns domain cannot be resolved error message

As requested by PR review in #58234 (comment)
@ansible ansible locked and limited conversation to collaborators Sep 16, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.9 This issue/PR affects Ansible v2.9 bug This issue/PR relates to a bug. has_issue module This issue/PR relates to a module. shipit This PR is ready to be merged by Core support:community This issue/PR relates to code supported by the Ansible community. support:core This issue/PR relates to code supported by the Ansible Engineering Team. windows Windows community
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants