azure_rm_networkinterface: Feature/dns servers #43588

spmp · 2018-08-02T04:51:24Z

SUMMARY

Add dns_servers option to azure_rm_networkinterface

ISSUE TYPE

New Module Pull Request

COMPONENT NAME

azure_rm_networkinterface

ANSIBLE VERSION

2.7

ansibot · 2018-08-02T04:57:03Z

cc @haroldwongms @nitzmahone @trstringer @xscript @yuwzho @zikalino
click here for bot help

ansibot · 2018-08-02T04:57:03Z

Hi @spmp,

Thank you for the pullrequest, just so you are aware we have a dedicated Working Group for azure.
You can find other people interested in this in #ansible-azure on Freenode IRC
For more information about communities, meetings and agendas see https://github.com/ansible/community

click here for bot help

ansibot · 2018-08-02T05:08:50Z

The test ansible-test sanity --test validate-modules [explain] failed with 2 errors:

lib/ansible/modules/cloud/azure/azure_rm_networkinterface.py:534:0: E403 Type comparison using type() found. Use isinstance() instead
lib/ansible/modules/cloud/azure/azure_rm_networkinterface.py:535:0: E403 Type comparison using type() found. Use isinstance() instead

click here for bot help

ansibot · 2018-08-03T00:37:55Z

The test ansible-test sanity --test validate-modules [explain] failed with 2 errors:

lib/ansible/modules/cloud/azure/azure_rm_networkinterface.py:534:0: E403 Type comparison using type() found. Use isinstance() instead
lib/ansible/modules/cloud/azure/azure_rm_networkinterface.py:535:0: E403 Type comparison using type() found. Use isinstance() instead

click here for bot help

ansibot · 2018-08-03T02:21:58Z

The test ansible-test sanity --test pylint [explain] failed with 2 errors:

lib/ansible/modules/cloud/azure/azure_rm_networkinterface.py:534:63: undefined-variable Undefined variable 'istype'
lib/ansible/modules/cloud/azure/azure_rm_networkinterface.py:535:62: undefined-variable Undefined variable 'istype'

The test ansible-test sanity --test pep8 [explain] failed with 2 errors:

lib/ansible/modules/cloud/azure/azure_rm_networkinterface.py:534:64: E721 do not compare types, use 'isinstance()'
lib/ansible/modules/cloud/azure/azure_rm_networkinterface.py:535:63: E721 do not compare types, use 'isinstance()'

The test ansible-test sanity --test validate-modules [explain] failed with 2 errors:

lib/ansible/modules/cloud/azure/azure_rm_networkinterface.py:534:0: E403 Type comparison using type() found. Use isinstance() instead
lib/ansible/modules/cloud/azure/azure_rm_networkinterface.py:535:0: E403 Type comparison using type() found. Use isinstance() instead

click here for bot help

yuwzho · 2018-08-06T02:42:45Z

test/integration/targets/azure_rm_networkinterface/tasks/main.yml

@@ -1,29 +1,34 @@
 - name: Prepare random number
  set_fact:
    rpfx: "{{ resource_group | hash('md5') | truncate(7, True, '') }}{{ 1000 | random }}"
+    auth_source: auto


no need to add auth_source

yuwzho · 2018-08-09T06:29:17Z

test/integration/targets/azure_rm_networkinterface/tasks/main.yml

-      resource_group: "{{ resource_group_secondary }}"
-      name: "tn{{ rpfx }}"
-      address_prefixes: "10.10.0.0/16"
+    resource_group: "{{ resource_group_secondary }}"


Hi @spmp As we talked this morning, please make the test with 4 spaces

yuwzho · 2018-08-09T06:30:31Z

lib/ansible/modules/cloud/azure/azure_rm_networkinterface.py

@@ -512,6 +529,16 @@ def exec_module(self, **kwargs):
                        results.get('enable_accelerated_networking')))
                    changed = True

+                # We need to ensure that dns_servers are list like
+                dns_servers_res = results.get('dns_settings').get('dns_servers')


Null check for dns_settings here?

dns_settings should never be null as it comes from network_client.network_interfaces.get()
Certainly can test, but we could miss breaking API changes from Azure SDK if we do this...

Just for fun: The DNS settings does not conform to published API from a setting perspective BTW. I had an interesting conversation with the Azure Architect (or an architect) about this.... turns out you cannot set anything except dns_servers, hence only this option. Well, you can set them, they are simply ignored.

Fred-sun · 2018-08-14T08:41:09Z

@spmp Thanks for your contribution, Could you please resolve conflicting files?

spmp · 2018-08-14T22:54:27Z

@Fred-sun rebased

Fred-sun · 2018-08-15T05:28:23Z

@spmp Thanks for your contribution, Does this PR ready for review or not?

spmp · 2018-08-15T23:11:51Z

@Fred-sun yes this is ready for review

Fred-sun · 2018-08-16T01:40:20Z

@spmp Thanks for your update, I will push review. Thanks!

Fred-sun · 2018-08-16T01:47:59Z

@yuwzho Could you help approve those change? Thanks!

yuwzho · 2018-08-16T02:28:36Z

@jborean93 for a further review

Fred-sun · 2018-08-17T08:15:22Z

@jborean93 Could you help take a look this PR? Thanks!

Fred-sun · 2018-08-21T10:02:12Z

@jborean93 Could you help take a look this PR? Thanks!

Fred-sun · 2018-08-24T06:49:40Z

@jborean93 Could you help take a look this PR? Thanks!

Fred-sun · 2018-08-28T07:59:40Z

@jborean93 Could you help take a look this PR? we need push this PR to next step . Thanks!

spmp · 2018-08-29T20:36:03Z

Cheers @nitzmahone

ansibot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed committer_review In order to be merged, this PR must follow the certified review workflow. labels Aug 2, 2018

jborean93 removed the needs_triage Needs a first human triage before being processed. label Aug 2, 2018

gundalow changed the title ~~Feature/dns servers~~ azure_rm_networkinterface: Feature/dns servers Aug 2, 2018

spmp force-pushed the feature/dns_servers branch from 305f892 to 2e75ec4 Compare August 3, 2018 00:17

ansibot added the ci_verified Changes made in this PR are causing tests to fail. label Aug 3, 2018

ansibleazurebot mentioned this pull request Aug 3, 2018

azure_rm_networkinterface: Feature/dns servers VSChina/ansible-issue-tracker#368

Closed

ansibot removed the ci_verified Changes made in this PR are causing tests to fail. label Aug 3, 2018

ansibot added committer_review In order to be merged, this PR must follow the certified review workflow. and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Aug 5, 2018

yuwzho reviewed Aug 6, 2018

View reviewed changes

yuwzho suggested changes Aug 9, 2018

View reviewed changes

ansibot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed committer_review In order to be merged, this PR must follow the certified review workflow. labels Aug 9, 2018

yuwzho approved these changes Aug 10, 2018

View reviewed changes

ansibot removed the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. label Aug 10, 2018

ansibot added the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. label Aug 13, 2018

Jasper Mackenzie added 5 commits August 15, 2018 10:05

Working through tests and fixing dumb mistakes

fd85ee2

Fix sanity issues and tests. All G

51858ea

Correct type test for azure_rm_networkinterface

182cb5e

azure_rm_networkinterface correct type comparison

34fc3ce

Post rebase cleanup

ec6a8ac

spmp force-pushed the feature/dns_servers branch from cbb7749 to ec6a8ac Compare August 14, 2018 22:53

ansibot removed the needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html label Aug 14, 2018

Remove erroneous reference to dns_servers

39ddd89

ansibot added committer_review In order to be merged, this PR must follow the certified review workflow. and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Aug 15, 2018

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 24, 2018

nitzmahone approved these changes Aug 28, 2018

View reviewed changes

nitzmahone merged commit 56ea56f into ansible:devel Aug 28, 2018

spmp deleted the feature/dns_servers branch August 29, 2018 20:33

ansible locked and limited conversation to collaborators Jul 22, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

azure_rm_networkinterface: Feature/dns servers #43588

azure_rm_networkinterface: Feature/dns servers #43588

spmp commented Aug 2, 2018

ansibot commented Aug 2, 2018

ansibot commented Aug 2, 2018

ansibot commented Aug 2, 2018

ansibot commented Aug 3, 2018

ansibot commented Aug 3, 2018

yuwzho Aug 6, 2018

yuwzho Aug 9, 2018

yuwzho Aug 9, 2018

spmp Aug 9, 2018

Fred-sun commented Aug 14, 2018

spmp commented Aug 14, 2018

Fred-sun commented Aug 15, 2018

spmp commented Aug 15, 2018

Fred-sun commented Aug 16, 2018

Fred-sun commented Aug 16, 2018

yuwzho commented Aug 16, 2018

Fred-sun commented Aug 17, 2018

Fred-sun commented Aug 21, 2018

Fred-sun commented Aug 24, 2018

Fred-sun commented Aug 28, 2018

spmp commented Aug 29, 2018

azure_rm_networkinterface: Feature/dns servers #43588

azure_rm_networkinterface: Feature/dns servers #43588

Conversation

spmp commented Aug 2, 2018

SUMMARY

ISSUE TYPE

COMPONENT NAME

ANSIBLE VERSION

ansibot commented Aug 2, 2018

ansibot commented Aug 2, 2018

ansibot commented Aug 2, 2018

ansibot commented Aug 3, 2018

ansibot commented Aug 3, 2018

yuwzho Aug 6, 2018

Choose a reason for hiding this comment

yuwzho Aug 9, 2018

Choose a reason for hiding this comment

yuwzho Aug 9, 2018

Choose a reason for hiding this comment

spmp Aug 9, 2018

Choose a reason for hiding this comment

Fred-sun commented Aug 14, 2018

spmp commented Aug 14, 2018

Fred-sun commented Aug 15, 2018

spmp commented Aug 15, 2018

Fred-sun commented Aug 16, 2018

Fred-sun commented Aug 16, 2018

yuwzho commented Aug 16, 2018

Fred-sun commented Aug 17, 2018

Fred-sun commented Aug 21, 2018

Fred-sun commented Aug 24, 2018

Fred-sun commented Aug 28, 2018

spmp commented Aug 29, 2018