VMware: Refactor vmware_vswitch (#36091)

* Update documentation
* Update logic
* Added idempotency
* Added Error handling

Fixes: #36030

Signed-off-by: Abhijeet Kasurde <akasurde@redhat.com>

lib/ansible/module_utils/vmware.py

@@ -1100,7 +1100,7 @@ class PyVmomi(object):
         """
         Function to get datastore cluster managed object by name
         Args:
-            datastore_cluster: Name of datastore cluster
+            datastore_cluster_name: Name of datastore cluster
 
         Returns: Datastore cluster managed object if found else None
 

lib/ansible/modules/cloud/vmware/vmware_vswitch.py

@@ -1,28 +1,33 @@
 #!/usr/bin/python
 # -*- coding: utf-8 -*-
-
 # Copyright: (c) 2015, Joseph Callen <jcallen () csc.com>
+# Copyright: (c) 2018, Ansible Project
+# Copyright: (c) 2018, Abhijeet Kasurde <akasurde@redhat.com>
 # GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
 
 from __future__ import absolute_import, division, print_function
 __metaclass__ = type
 
-ANSIBLE_METADATA = {'metadata_version': '1.1',
+
+ANSIBLE_METADATA = {
+    'metadata_version': '1.1',
     'status': ['preview'],
-                    'supported_by': 'community'}
+    'supported_by': 'community'
+}
 
 DOCUMENTATION = '''
 ---
 module: vmware_vswitch
-short_description: Add or remove a VMware Standard Switch to an ESXi host
+short_description: Manage a VMware Standard Switch to an ESXi host.
 description:
-- Add or remove a VMware Standard Switch to an ESXi host.
+- This module can be used to add, remove and update a VMware Standard Switch to an ESXi host.
 version_added: 2.0
 author:
 - Joseph Callen (@jcpowermac)
 - Russell Teague (@mtnbikenc)
+- Abhijeet Kasurde (@akasurde) <akasurde@redhat.com>
 notes:
-- Tested on vSphere 5.5
+- Tested on vSphere 5.5 and 6.5
 requirements:
 - python >= 2.6
 - PyVmomi
@@ -38,6 +43,7 @@ options:
     - A list of vmnic names or vmnic name to attach to vSwitch.
     - Alias C(nics) is added in version 2.4.
     aliases: [ nic_name ]
+    default: []
   number_of_ports:
     description:
     - Number of port to configure on vSwitch.
@@ -53,7 +59,7 @@ options:
     choices: [ absent, present ]
   esxi_hostname:
     description:
-    - Manage the vSwitch using this ESXi host system
+    - Manage the vSwitch using this ESXi host system.
     version_added: "2.5"
     aliases: [ 'host' ]
 extends_documentation_fragment:
@@ -105,21 +111,22 @@ EXAMPLES = '''
   delegate_to: localhost
 '''
 
+RETURN = """
+result:
+    description: information about performed operation
+    returned: always
+    type: string
+    sample: "vSwitch 'vSwitch_1002' is created successfully"
+"""
 
-from ansible.module_utils.basic import AnsibleModule
-from ansible.module_utils.six import iteritems
-from ansible.module_utils.vmware import PyVmomi, vmware_argument_spec, get_all_objs
 try:
     from pyVmomi import vim, vmodl
 except ImportError:
     pass
 
-
+from ansible.module_utils.basic import AnsibleModule
-def find_vswitch_by_name(host, vswitch_name):
+from ansible.module_utils.vmware import PyVmomi, vmware_argument_spec
-    for vss in host.configManager.networkSystem.networkInfo.vswitch:
+from ansible.module_utils._text import to_native
-        if vss.name == vswitch_name:
-            return vss
-    return None
 
 
 class VMwareHostVirtualSwitch(PyVmomi):
@@ -132,96 +139,260 @@ class VMwareHostVirtualSwitch(PyVmomi):
         self.nics = module.params['nics']
         self.mtu = module.params['mtu']
         self.state = module.params['state']
-        self.esxi_hostname = module.params['esxi_hostname']
+        esxi_hostname = module.params['esxi_hostname']
+
+        hosts = self.get_all_host_objs(esxi_host_name=esxi_hostname)
+        if hosts:
+            self.host_system = hosts[0]
+        else:
+            self.module.fail_json(msg="Failed to get details of ESXi server."
+                                      " Please specify esxi_hostname.")
+
+        if self.params.get('state') == 'present':
+            # Gather information about all vSwitches and Physical NICs
+            network_manager = self.host_system.configManager.networkSystem
+            available_pnic = [pnic.device for pnic in network_manager.networkInfo.pnic]
+            self.available_vswitches = dict()
+            for available_vswitch in network_manager.networkInfo.vswitch:
+                used_pnic = []
+                for pnic in available_vswitch.pnic:
+                    # vSwitch contains all PNICs as string in format of 'key-vim.host.PhysicalNic-vmnic0'
+                    m_pnic = pnic.split("-", 3)[-1]
+                    used_pnic.append(m_pnic)
+                self.available_vswitches[available_vswitch.name] = dict(pnic=used_pnic,
+                                                                        mtu=available_vswitch.mtu,
+                                                                        num_ports=available_vswitch.spec.numPorts,
+                                                                        )
+            for desired_pnic in self.nics:
+                if desired_pnic not in available_pnic:
+                    # Check if pnic does not exists
+                    self.module.fail_json(msg="Specified Physical NIC '%s' does not"
+                                              " exists on given ESXi '%s'." % (desired_pnic,
+                                                                               self.host_system.name))
+                for vswitch in self.available_vswitches:
+                    if desired_pnic in self.available_vswitches[vswitch]['pnic'] and vswitch != self.switch:
+                        # Check if pnic is already part of some other vSwitch
+                        self.module.fail_json(msg="Specified Physical NIC '%s' is already used"
+                                                  " by vSwitch '%s'." % (desired_pnic, vswitch))
 
     def process_state(self):
-        try:
+        """
+        Function to manage internal state of vSwitch
+        """
         vswitch_states = {
             'absent': {
                 'present': self.state_destroy_vswitch,
                 'absent': self.state_exit_unchanged,
             },
             'present': {
-                    'update': self.state_update_vswitch,
+                'present': self.state_update_vswitch,
-                    'present': self.state_exit_unchanged,
                 'absent': self.state_create_vswitch,
             }
         }
 
+        try:
             vswitch_states[self.state][self.check_vswitch_configuration()]()
-
         except vmodl.RuntimeFault as runtime_fault:
-            self.module.fail_json(msg=runtime_fault.msg)
+            self.module.fail_json(msg=to_native(runtime_fault.msg))
         except vmodl.MethodFault as method_fault:
-            self.module.fail_json(msg=method_fault.msg)
+            self.module.fail_json(msg=to_native(method_fault.msg))
         except Exception as e:
-            self.module.fail_json(msg=str(e))
+            self.module.fail_json(msg=to_native(e))
-
-    # Source from
-    # https://github.com/rreubenur/pyvmomi-community-samples/blob/patch-1/samples/create_vswitch.py
 
     def state_create_vswitch(self):
+        """
+        Function to create a virtual switch
+
+        Source from
+        https://github.com/rreubenur/pyvmomi-community-samples/blob/patch-1/samples/create_vswitch.py
+
+        """
+
+        results = dict(changed=False, result="")
         vss_spec = vim.host.VirtualSwitch.Specification()
         vss_spec.numPorts = self.number_of_ports
         vss_spec.mtu = self.mtu
         if self.nics:
             vss_spec.bridge = vim.host.VirtualSwitch.BondBridge(nicDevice=self.nics)
-        self.host_system.configManager.networkSystem.AddVirtualSwitch(vswitchName=self.switch, spec=vss_spec)
+        try:
-        self.module.exit_json(changed=True)
+            network_mgr = self.host_system.configManager.networkSystem
+            if network_mgr:
+                network_mgr.AddVirtualSwitch(vswitchName=self.switch,
+                                             spec=vss_spec)
+                results['changed'] = True
+                results['result'] = "vSwitch '%s' is created successfully" % self.switch
+            else:
+                self.module.fail_json(msg="Failed to find network manager for ESXi system")
+        except vim.fault.AlreadyExists as already_exists:
+            results['result'] = "vSwitch with name %s already exists: %s" % (self.switch,
+                                                                             to_native(already_exists.msg))
+        except vim.fault.ResourceInUse as resource_used:
+            self.module.fail_json(msg="Failed to add vSwitch '%s' as physical network adapter"
+                                      " being bridged is already in use: %s" % (self.switch,
+                                                                                to_native(resource_used.msg)))
+        except vim.fault.HostConfigFault as host_config_fault:
+            self.module.fail_json(msg="Failed to add vSwitch '%s' due to host"
+                                      " configuration fault : %s" % (self.switch,
+                                                                     to_native(host_config_fault.msg)))
+        except vmodl.fault.InvalidArgument as invalid_argument:
+            self.module.fail_json(msg="Failed to add vSwitch '%s', this can be due to either of following :"
+                                      " 1. vSwitch Name exceeds the maximum allowed length,"
+                                      " 2. Number of ports specified falls out of valid range,"
+                                      " 3. Network policy is invalid,"
+                                      " 4. Beacon configuration is invalid : %s" % (self.switch,
+                                                                                    to_native(invalid_argument.msg)))
+        except vmodl.fault.SystemError as system_error:
+            self.module.fail_json(msg="Failed to add vSwitch '%s' due to : %s" % (self.switch,
+                                                                                  to_native(system_error.msg)))
+        except Exception as generic_exc:
+            self.module.fail_json(msg="Failed to add vSwitch '%s' due to"
+                                      " generic exception : %s" % (self.switch,
+                                                                   to_native(generic_exc)))
+        self.module.exit_json(**results)
 
     def state_exit_unchanged(self):
+        """
+        Function to declare exit without unchanged
+        """
         self.module.exit_json(changed=False)
 
     def state_destroy_vswitch(self):
-        config = vim.host.NetworkConfig()
+        """
+        Function to remove vSwitch from configuration
 
-        for portgroup in self.host_system.configManager.networkSystem.networkInfo.portgroup:
+        """
-            if portgroup.spec.vswitchName == self.vss.name:
+        results = dict(changed=False, result="")
-                portgroup_config = vim.host.PortGroup.Config()
-                portgroup_config.changeOperation = "remove"
-                portgroup_config.spec = vim.host.PortGroup.Specification()
-                portgroup_config.spec.name = portgroup.spec.name
-                portgroup_config.spec.name = portgroup.spec.name
-                portgroup_config.spec.vlanId = portgroup.spec.vlanId
-                portgroup_config.spec.vswitchName = portgroup.spec.vswitchName
-                portgroup_config.spec.policy = vim.host.NetworkPolicy()
-                config.portgroup.append(portgroup_config)
 
-        self.host_system.configManager.networkSystem.UpdateNetworkConfig(config, "modify")
+        try:
             self.host_system.configManager.networkSystem.RemoveVirtualSwitch(self.vss.name)
-        self.module.exit_json(changed=True)
+            results['changed'] = True
+            results['result'] = "vSwitch '%s' removed successfully." % self.vss.name
+        except vim.fault.NotFound as vswitch_not_found:
+            results['result'] = "vSwitch '%s' not available. %s" % (self.switch,
+                                                                    to_native(vswitch_not_found.msg))
+        except vim.fault.ResourceInUse as vswitch_in_use:
+            self.module.fail_json(msg="Failed to remove vSwitch '%s' as vSwitch"
+                                      " is used by several virtual"
+                                      " network adapters: %s" % (self.switch,
+                                                                 to_native(vswitch_in_use.msg)))
+        except vim.fault.HostConfigFault as host_config_fault:
+            self.module.fail_json(msg="Failed to remove vSwitch '%s' due to host"
+                                      " configuration fault : %s" % (self.switch,
+                                                                     to_native(host_config_fault.msg)))
+        except Exception as generic_exc:
+            self.module.fail_json(msg="Failed to remove vSwitch '%s' due to generic"
+                                      " exception : %s" % (self.switch,
+                                                           to_native(generic_exc)))
+
+        self.module.exit_json(**results)
 
     def state_update_vswitch(self):
-        self.module.exit_json(changed=False, msg="Currently not implemented.")
+        """
+        Function to update vSwitch
+
+        """
+        results = dict(changed=False, result="No change in vSwitch '%s'" % self.switch)
+        vswitch_pnic_info = self.available_vswitches[self.switch]
+        remain_pnic = []
+        for desired_pnic in self.nics:
+            if desired_pnic not in vswitch_pnic_info['pnic']:
+                remain_pnic.append(desired_pnic)
+
+        diff = False
+        # Update all nics
+        all_nics = vswitch_pnic_info['pnic']
+        if remain_pnic:
+            all_nics += remain_pnic
+            diff = True
+
+        # vSwitch needs every parameter again while updating,
+        # even if we are updating any one of them
+        vss_spec = vim.host.VirtualSwitch.Specification()
+        vss_spec.bridge = vim.host.VirtualSwitch.BondBridge(nicDevice=all_nics)
+        vss_spec.numPorts = self.number_of_ports
+        vss_spec.mtu = self.mtu
+
+        if vswitch_pnic_info['mtu'] != self.mtu or \
+                vswitch_pnic_info['num_ports'] != self.number_of_ports:
+            diff = True
+
+        try:
+            if diff:
+                network_mgr = self.host_system.configManager.networkSystem
+                if network_mgr:
+                    network_mgr.UpdateVirtualSwitch(vswitchName=self.switch,
+                                                    spec=vss_spec)
+                    results['changed'] = True
+                    results['result'] = "vSwitch '%s' is updated successfully" % self.switch
+                else:
+                    self.module.fail_json(msg="Failed to find network manager for ESXi system.")
+        except vim.fault.ResourceInUse as resource_used:
+            self.module.fail_json(msg="Failed to update vSwitch '%s' as physical network adapter"
+                                      " being bridged is already in use: %s" % (self.switch,
+                                                                                to_native(resource_used.msg)))
+        except vim.fault.NotFound as not_found:
+            self.module.fail_json(msg="Failed to update vSwitch with name '%s'"
+                                      " as it does not exists: %s" % (self.switch,
+                                                                      to_native(not_found.msg)))
+
+        except vim.fault.HostConfigFault as host_config_fault:
+            self.module.fail_json(msg="Failed to update vSwitch '%s' due to host"
+                                      " configuration fault : %s" % (self.switch,
+                                                                     to_native(host_config_fault.msg)))
+        except vmodl.fault.InvalidArgument as invalid_argument:
+            self.module.fail_json(msg="Failed to update vSwitch '%s', this can be due to either of following :"
+                                      " 1. vSwitch Name exceeds the maximum allowed length,"
+                                      " 2. Number of ports specified falls out of valid range,"
+                                      " 3. Network policy is invalid,"
+                                      " 4. Beacon configuration is invalid : %s" % (self.switch,
+                                                                                    to_native(invalid_argument.msg)))
+        except vmodl.fault.SystemError as system_error:
+            self.module.fail_json(msg="Failed to update vSwitch '%s' due to : %s" % (self.switch,
+                                                                                     to_native(system_error.msg)))
+        except vmodl.fault.NotSupported as not_supported:
+            self.module.fail_json(msg="Failed to update vSwitch '%s' as network adapter teaming policy"
+                                      " is set but is not supported : %s" % (self.switch,
+                                                                             to_native(not_supported.msg)))
+        except Exception as generic_exc:
+            self.module.fail_json(msg="Failed to update vSwitch '%s' due to"
+                                      " generic exception : %s" % (self.switch,
+                                                                   to_native(generic_exc)))
+        self.module.exit_json(**results)
 
     def check_vswitch_configuration(self):
-        hosts = get_all_objs(self.content, [vim.HostSystem])
+        """
-        if not hosts:
+        Function to check if vSwitch exists
-            self.module.fail_json(msg="Unable to find host")
+        Returns: 'present' if vSwitch exists or 'absent' if not
-
-        desired_host_system = None
-        if self.esxi_hostname:
-            for host_system_obj, host_system_name in iteritems(hosts):
-                if host_system_name == self.esxi_hostname:
-                    desired_host_system = host_system_obj
-
-        if desired_host_system:
-            self.host_system = desired_host_system
-        else:
-            self.host_system = list(hosts.keys())[0]
-        self.vss = find_vswitch_by_name(self.host_system, self.switch)
 
+        """
+        self.vss = self.find_vswitch_by_name(self.host_system, self.switch)
         if self.vss is None:
             return 'absent'
         else:
             return 'present'
 
+    @staticmethod
+    def find_vswitch_by_name(host, vswitch_name):
+        """
+        Function to find and return vSwitch managed object
+        Args:
+            host: Host system managed object
+            vswitch_name: Name of vSwitch to find
+
+        Returns: vSwitch managed object if found, else None
+
+        """
+        for vss in host.configManager.networkSystem.networkInfo.vswitch:
+            if vss.name == vswitch_name:
+                return vss
+        return None
+
 
 def main():
     argument_spec = vmware_argument_spec()
     argument_spec.update(dict(
         switch=dict(type='str', required=True, aliases=['switch_name']),
-        nics=dict(type='list', aliases=['nic_name']),
+        nics=dict(type='list', aliases=['nic_name'], default=[]),
         number_of_ports=dict(type='int', default=128),
         mtu=dict(type='int', default=1500),
         state=dict(type='str', default='present', choices=['absent', 'present'])),

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

@@ -2,68 +2,69 @@
 # Copyright: (c) 2017, Abhijeet Kasurde <akasurde@redhat.com>
 # GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
 
-- name: Make sure pyvmomi is installed
+# TODO: akasurde: VCSIM does not suport network manager system
-  pip:
+#- name: Make sure pyvmomi is installed
-    name: pyvmomi
+#  pip:
-    state: latest
+#    name: pyvmomi
-  when: "{{ ansible_user_id == 'root' }}"
+#    state: latest
+#  when: "{{ ansible_user_id == 'root' }}"
 
-- name: store the vcenter container ip
+#- name: store the vcenter container ip
-  set_fact:
+#  set_fact:
-    vcsim: '{{ lookup("env", "vcenter_host") }}'
+#    vcsim: '{{ lookup("env", "vcenter_host") }}'
 
-- debug:
+#- debug:
-    var: vcsim
+#    var: vcsim
 
-- name: Wait for Flask controller to come up online
+#- name: Wait for Flask controller to come up online
-  wait_for:
+#  wait_for:
-    host: '{{ vcsim }}'
+#    host: '{{ vcsim }}'
-    port: 5000
+#    port: 5000
-    state: started
+#    state: started
 
-- name: Kill vcsim
+#- name: Kill vcsim
-  uri:
+#  uri:
-    url: http://{{ vcsim }}:5000/killall
+#    url: http://{{ vcsim }}:5000/killall
 
-- name: Start vcsim
+#- name: Start vcsim
-  uri:
+#  uri:
-    url: http://{{ vcsim }}:5000/spawn?cluster=2
+#    url: http://{{ vcsim }}:5000/spawn?cluster=2
-  register: vcsim_instance
+#  register: vcsim_instance
 
-- name: Wait for Flask controller to come up online
+#- name: Wait for Flask controller to come up online
-  wait_for:
+#  wait_for:
-    host: '{{ vcsim }}'
+#    host: '{{ vcsim }}'
-    port: 443
+#    port: 443
-    state: started
+#    state: started
 
-- debug:
+#- debug:
-    var: vcsim_instance
+#    var: vcsim_instance
 
 # FIXME: Implement check-mode support
-- name: Add a nic to a switch (check-mode)
+#- name: Add a nic to a switch (check-mode)
-  vmware_vswitch: &add_nic
+#  vmware_vswitch: &add_nic
-    hostname: '{{ vcsim }}'
+#    hostname: '{{ vcsim }}'
-    username: '{{ vcsim_instance.json.username }}'
+#    username: '{{ vcsim_instance.json.username }}'
-    password: '{{ vcsim_instance.json.password }}'
+#    password: '{{ vcsim_instance.json.password }}'
-    validate_certs: no
+#    validate_certs: no
-    switch: vmswitch_0001
+#    switch: vmswitch_0001
-    nics: vnic_1
+#    nics: vnic_1
-    state: present
+#    state: present
-  check_mode: yes
+#  check_mode: yes
-  register: add_nic_check
+#  register: add_nic_check
 
-- assert:
+#- assert:
-    that:
+#    that:
 #    - add_nic_check.changed == true
-    - add_nic_check.skipped == true
+#    - add_nic_check.skipped == true
 
-- name: Add a nic to a switch
+#- name: Add a nic to a switch
-  vmware_vswitch: *add_nic
+#  vmware_vswitch: *add_nic
-  register: add_nic_run
+#  register: add_nic_run
 
-- assert:
+#- assert:
-    that:
+#    that:
-    - add_nic_run.changed == true
+#    - add_nic_run.changed == true
 
 ## FIXME: Implement check-mode support
 #- name: Add a nic to a switch again (check-mode)
@@ -127,30 +128,29 @@
 #    that:
 #    - remove_nic_again_run.changed == false
 
+#- name: get a list of Host Systems from vcsim
+#  uri:
+#    url: "{{ 'http://' + vcsim + ':5000/govc_find?filter=H' }}"
+#  register: host_systems
 
-- name: get a list of Host Systems from vcsim
+#- name: get a host system
-  uri:
+#  set_fact: hs1="{{ host_systems['json'][0] | basename }}"
-    url: "{{ 'http://' + vcsim + ':5000/govc_find?filter=H' }}"
-  register: host_systems
 
-- name: get a host system
+#- debug: var=hs1
-  set_fact: hs1="{{ host_systems['json'][0] | basename }}"
 
-- debug: var=hs1
+#- name: Add vswitch to a specific host system
+#  vmware_vswitch:
+#    validate_certs: False
+#    hostname: "{{ vcsim }}"
+#    username: "{{ vcsim_instance['json']['username'] }}"
+#    password: "{{ vcsim_instance['json']['password'] }}"
+#    switch: vmswitch_0002
+#    nics: vnic_1
+#    esxi_hostname: hs1
+#  register: add_vswitch_with_host_system
 
-- name: Add vswitch to a specific host system
+#- debug: var=add_vswitch_with_host_system
-  vmware_vswitch:
-    validate_certs: False
-    hostname: "{{ vcsim }}"
-    username: "{{ vcsim_instance['json']['username'] }}"
-    password: "{{ vcsim_instance['json']['password'] }}"
-    switch: vmswitch_0002
-    nics: vnic_1
-    esxi_hostname: hs1
-  register: add_vswitch_with_host_system
 
-- debug: var=add_vswitch_with_host_system
+#- assert:
-
+#    that:
-- assert:
+#    - add_vswitch_with_host_system.changed == true
-    that:
-    - add_vswitch_with_host_system.changed == true