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 support for configuring host networks with vmware_guest #38647

Closed
wants to merge 1 commit into from
Closed

add support for configuring host networks with vmware_guest #38647

wants to merge 1 commit into from

Conversation

phemmer
Copy link
Contributor

@phemmer phemmer commented Apr 12, 2018

SUMMARY

previously the vmware_guest module would only allow use of distributed networks, and not host-specific networks. This change makes it support both.

ISSUE TYPE
  • Bugfix Pull Request

Classifying this as a bug because this isn't really new functionality. The functionality is there, it's just the validation that was incorrectly classifying the network as invalid.

COMPONENT NAME

vmware_guest

ANSIBLE VERSION
ansible 2.5.0
  config file = /Users/phemmer/git/c14n/core/ansible/ansible.cfg
  configured module search path = [u'/Users/phemmer/git/c14n/core/ansible/library']
  ansible python module location = /Users/phemmer/ansible-2.5/lib/python2.7/site-packages/ansible
  executable location = /Users/phemmer/ansible-2.5/bin/ansible
  python version = 2.7.10 (default, Feb  7 2017, 00:08:15) [GCC 4.2.1 Compatible Apple LLVM 8.0.0 (clang-800.0.34)]
ADDITIONAL INFORMATION

Before:

TASK [add interface to node] *************************************************************************************
fatal: [localhost]: FAILED! => {"changed": false, "msg": "Network 'MyNetwork' does not exist."}

After:

TASK [add interface to node] *************************************************************************************
changed: [localhost]

previously the vmware_guest module would only allow use of distributed networks, and not host-specific networks. This change makes it support both.
@ansibot
Copy link
Contributor

ansibot commented Apr 12, 2018

@ansibot ansibot added cloud community_review In order to be merged, this PR must follow the community review workflow. feature This issue/PR relates to a feature request. module This issue/PR relates to a module. needs_triage Needs a first human triage before being processed. new_contributor This PR is the first contribution by a new community member. support:community This issue/PR relates to code supported by the Ansible community. vmware VMware community bug This issue/PR relates to a bug. labels Apr 12, 2018
@Akasurde Akasurde removed needs_triage Needs a first human triage before being processed. feature This issue/PR relates to a feature request. labels Apr 12, 2018
host = vm.summary.runtime.host
elif self.params['esxi_hostname']:
host = self.select_host()

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about adding this -

diff --git a/lib/ansible/modules/cloud/vmware/vmware_guest.py b/lib/ansible/modules/cloud/vmware/vmware_guest.py
index 6985ca5497..f4fec7eec5 100644
--- a/lib/ansible/modules/cloud/vmware/vmware_guest.py
+++ b/lib/ansible/modules/cloud/vmware/vmware_guest.py
@@ -992,8 +992,24 @@ class PyVmomiHelper(PyVmomi):
                 self.module.fail_json(msg="Please specify at least a network name or"
                                           " a VLAN name under VM network list.")

-            if 'name' in network and find_obj(self.content, [vim.Network], network['name']) is None:
-                self.module.fail_json(msg="Network '%(name)s' does not exist." % network)
+            if 'name' in network:
+                if find_obj(self.content, [vim.Network], network['name']) is None:
+                    self.module.fail_json(msg="Network '%(name)s' does not exist." % network)
+                else:
+                    if self.params.get('esxi_hostname', None):
+                        esxi_hostname = self.select_host()
+                        if network['name'] not in [nw.name for nw in esxi_hostname.network]:
+                            self.module.fail_json(msg="Specified network '%s' is not associated "
+                                                      "with the given ESXi hostname '%s'" % (network['name'],
+                                                                                             self.params['esxi_hostname']))
+                    elif self.params.get('cluster', None):
+                        cluster = self.cache.get_cluster(self.params['cluster'])
+                        if network['name'] not in [nw.name for nw in cluster.network]:
+                            self.module.fail_json(msg="Specified network '%s' is not associated"
+                                                      " with the given cluster '%s'" % (network['name'],
+                                                                                        self.params['cluster']))

Copy link
Contributor Author

@phemmer phemmer Apr 12, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+                if find_obj(self.content, [vim.Network], network['name']) is None:
+                    self.module.fail_json(msg="Network '%(name)s' does not exist." % network)
+                else:

^ The else will get evaluated when the network is found, which is not what we're after. If the network is not found, we want to try searching the host for it instead.

+                    if self.params.get('esxi_hostname', None):
+                        esxi_hostname = self.select_host()

This will only handle the scenario where the user specifies an esxi_hostname. It won't handle the scenario where the user didn't specify one, but was automatically assigned one.

+                        cluster = self.cache.get_cluster(self.params['cluster'])
+                        if network['name'] not in [nw.name for nw in cluster.network]:

I don't follow this one. get_cluster looks like it will only ever return vim.ClusterComputeResource objects. This object does not have a network attribute.


if host is not None:
for hostnet in host.network:
if hostnet.name == network['name']:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure if this will work because - if find_obj is not able find this object that means it does not exists. If it finds then you will not hit this condition. Let me know if I am wrong.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

find_obj apparently only finds things that are cluster-wide. It does not find networks present on only a single host.

@ansibot ansibot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed community_review In order to be merged, this PR must follow the community review workflow. labels Apr 12, 2018
@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 Apr 20, 2018
@ansibot
Copy link
Contributor

ansibot commented May 6, 2018

Copy link
Contributor

@pdellaert pdellaert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assuming @phemmer is right with host networks never being picked up (bit suspicious, can't imagine no-one ever tried with host networks itself, but no to time to test currently). This looks good.

@ansibot ansibot added the affects_2.6 This issue/PR affects Ansible v2.6 label May 22, 2018
@phemmer phemmer closed this Aug 6, 2018
@ansible ansible locked and limited conversation to collaborators Jul 22, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.6 This issue/PR affects Ansible v2.6 bug This issue/PR relates to a bug. cloud module This issue/PR relates to a module. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. new_contributor This PR is the first contribution by a new community member. stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. support:community This issue/PR relates to code supported by the Ansible community. vmware VMware community
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants