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 ability to clear DNS entries on static DNS #49763

Merged
merged 7 commits into from Sep 30, 2018

Conversation

Projects
None yet
4 participants
@twangboy
Copy link
Contributor

commented Sep 24, 2018

What does this PR do?

Adds the ability to clear the list of DNS entries in Windows by passing an empty list.

Execution Module:

  • Passing no DNS servers will clear the list because it is using *args.
  • Update documentation
  • Added a note about clearing DNS entries

State Module

  • An empty list in dns_servers will clear the entires
  • None in dns_servers will do nothing (default behavior)
  • Update Documentation

What issues does this PR fix or reference?

#49730

Tests written?

No

Commits signed with GPG?

Yes

Add ability to clear DNS entries on static DNS
Execution Module:
- Passing no DNS servers will clear the list.
- Update documentation
- Added a note about clearing DNS entries

State Module
- An empty list in dns_servers will clear the entires
- None in dns_servers will do nothing (default behavior)
- Update Documentation

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

addrs (*):
One or more DNS servers to be added
.. note::

This comment has been minimized.

Copy link
@cachedout

cachedout Sep 25, 2018

Collaborator

This does not seem like the right behavior to me. These sorts of side-effects are implicit and, in my opinion, should be explicit. IMHO, nothing passed should mean no change is applied and a list should only be cleared if, and only if, an empty list is explicitly passed in.

twangboy added some commits Sep 25, 2018

Only clear dns entries if you pass []
Add test=True support when clearing
Add/fix tests
Use named parameters where possible
@twangboy

This comment has been minimized.

Copy link
Contributor Author

commented Sep 25, 2018

@cachedout Only clears the list if you specifically pass []
@dwoz Added some tests

@dwoz

dwoz approved these changes Sep 25, 2018

@twangboy

This comment has been minimized.

Copy link
Contributor Author

commented Sep 26, 2018

I don't think all the tests actually ran... 4 seems like a small number

@rallytime rallytime closed this Sep 26, 2018

@rallytime rallytime reopened this Sep 26, 2018

@rallytime

This comment has been minimized.

Copy link
Contributor

commented Sep 26, 2018

@twangboy Yes, 4 is not the right number. The rest of the tests are waiting in the jenkins queue for an executor slot. Once they start, this will update with the remaining jobs.

@rallytime

This comment has been minimized.

Copy link
Contributor

commented Sep 26, 2018

twangboy and others added some commits Sep 28, 2018

Mike Place

@rallytime rallytime merged commit 98c9372 into saltstack:2017.7 Sep 30, 2018

10 checks passed

WIP ready for review
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

@twangboy twangboy deleted the twangboy:fix_49730 branch Oct 2, 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.