Refactor getvm method (#27188)

Fix refactors getvm method and modules which are using it.

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

lib/ansible/module_utils/vmware.py

@@ -170,17 +170,13 @@ def find_hostsystem_by_name(content, hostname):
     return None
 
 
-def find_vm_by_id(content, vm_id, vm_id_type="vm_name", datacenter=None, cluster=None):
+def find_vm_by_id(content, vm_id, vm_id_type="vm_name", datacenter=None, cluster=None, folder=None, match_first=False):
     """ UUID is unique to a VM, every other id returns the first match. """
     si = content.searchIndex
     vm = None
 
     if vm_id_type == 'dns_name':
         vm = si.FindByDnsName(datacenter=datacenter, dnsName=vm_id, vmSearch=True)
-    elif vm_id_type == 'inventory_path':
-        vm = si.FindByInventoryPath(inventoryPath=vm_id)
-        if isinstance(vm, vim.VirtualMachine):
-            vm = None
     elif vm_id_type == 'uuid':
         # Search By BIOS UUID rather than instance UUID
         vm = si.FindByUuid(datacenter=datacenter, instanceUuid=False, uuid=vm_id, vmSearch=True)
@@ -193,7 +189,20 @@ def find_vm_by_id(content, vm_id, vm_id_type="vm_name", datacenter=None, cluster
         elif datacenter:
             folder = datacenter.hostFolder
         vm = find_vm_by_name(content, vm_id, folder)
-
+    elif vm_id_type == 'inventory_path':
+        searchpath = folder
+        # get all objects for this path
+        f_obj = si.FindByInventoryPath(searchpath)
+        if f_obj:
+            if isinstance(f_obj, vim.Datacenter):
+                f_obj = f_obj.vmFolder
+            for c_obj in f_obj.childEntity:
+                if not isinstance(c_obj, vim.VirtualMachine):
+                    continue
+                if c_obj.name == vm_id:
+                    vm = c_obj
+                    if match_first:
+                        break
     return vm
 
 

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

@@ -313,10 +313,11 @@ instance:
 
 
 import time
+from ansible.module_utils._text import to_text
 from ansible.module_utils.basic import AnsibleModule
-from ansible.module_utils.pycompat24 import get_exception
+from ansible.module_utils.vmware import connect_to_api, find_obj, gather_vm_facts, get_all_objs, compile_folder_path_for_object
-from ansible.module_utils.vmware import connect_to_api, find_obj, gather_vm_facts, get_all_objs, compile_folder_path_for_object, vmware_argument_spec
+from ansible.module_utils.vmware import serialize_spec, find_vm_by_id, vmware_argument_spec
-from ansible.module_utils.vmware import serialize_spec
+
 
 try:
     import json
@@ -524,36 +525,14 @@ class PyVmomiHelper(object):
         self.cache = PyVmomiCache(self.content, dc_name=self.params['datacenter'])
 
     def getvm(self, name=None, uuid=None, folder=None):
-
-        # https://www.vmware.com/support/developer/vc-sdk/visdk2xpubs/ReferenceGuide/vim.SearchIndex.html
-        # self.si.content.searchIndex.FindByInventoryPath('DC1/vm/test_folder')
-
         vm = None
-        searchpath = None
+        match_first = False
-
         if uuid:
-            vm = self.content.searchIndex.FindByUuid(uuid=uuid, vmSearch=True)
+            vm = find_vm_by_id(self.content, vm_id=uuid, vm_id_type="uuid")
-        elif folder:
+        elif folder and name:
-            # searchpaths do not need to be absolute
-            searchpath = self.params['folder']
-
-            # get all objects for this path ...
-            f_obj = self.content.searchIndex.FindByInventoryPath(searchpath)
-
-            if f_obj:
-                if isinstance(f_obj, vim.Datacenter):
-                    f_obj = f_obj.vmFolder
-
-                for c_obj in f_obj.childEntity:
-
-                    if not isinstance(c_obj, vim.VirtualMachine):
-                        continue
-
-                    if c_obj.name == name:
-                        vm = c_obj
             if self.params['name_match'] == 'first':
-                            break
+                match_first = True
-
+            vm = find_vm_by_id(self.content, vm_id=name, vm_id_type="inventory_path", folder=folder, match_first=match_first)
         if vm:
             self.current_vm_obj = vm
 
@@ -612,10 +591,9 @@ class PyVmomiHelper(object):
                         result['failed'] = True
                         result['msg'] = "VM %s must be in poweredon state & tools should be installed for guest shutdown/reboot" % vm.name
 
-            except Exception:
+            except Exception as e:
-                e = get_exception()
                 result['failed'] = True
-                result['msg'] = str(e)
+                result['msg'] = to_text(e)
 
             if task:
                 self.wait_for_task(task)
@@ -801,10 +779,9 @@ class PyVmomiHelper(object):
                 try:
                     vm_obj.setCustomValue(key=kv['key'], value=kv['value'])
                     self.change_detected = True
-                except Exception:
+                except Exception as e:
-                    e = get_exception()
                     self.module.fail_json(msg="Failed to set custom value for key='%s' and value='%s'. Error was: %s"
-                                          % (kv['key'], kv['value'], e))
+                                          % (kv['key'], kv['value'], to_text(e)))
 
     def customize_vm(self, vm_obj):
         # Network settings
@@ -1329,9 +1306,8 @@ class PyVmomiHelper(object):
                 task = destfolder.CreateVM_Task(config=self.configspec, pool=resource_pool)
                 self.change_detected = True
             self.wait_for_task(task)
-        except TypeError:
+        except TypeError as e:
-            e = get_exception()
+            self.module.fail_json(msg="TypeError was returned, please ensure to give correct inputs. %s" % to_text(e))
-            self.module.fail_json(msg="TypeError was returned, please ensure to give correct inputs. %s" % e)
 
         if task.info.state == 'error':
             # https://kb.vmware.com/selfservice/microsites/search.do?language=en_US&cmd=displayKC&externalId=2021361
@@ -1469,7 +1445,7 @@ def main():
         annotation=dict(type='str', aliases=['notes']),
         customvalues=dict(type='list', default=[]),
         name=dict(type='str', required=True),
-        name_match=dict(type='str', default='first'),
+        name_match=dict(type='str', choices=['first', 'last'], default='first'),
         uuid=dict(type='str'),
         folder=dict(type='str', default='/vm'),
         guest_id=dict(type='str'),

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

@@ -67,7 +67,7 @@ options:
             - '   folder: folder1/datacenter1/vm'
             - '   folder: /folder1/datacenter1/vm/folder2'
             - '   folder: vm/folder2'
-            - '   fodler: folder2'
+            - '   folder: folder2'
         default: /vm
    datacenter:
         description:
@@ -126,25 +126,13 @@ class PyVmomiHelper(object):
 
     def getvm(self, name=None, uuid=None, folder=None):
         vm = None
-
+        match_first = False
         if uuid:
             vm = find_vm_by_id(self.content, vm_id=uuid, vm_id_type="uuid")
-        elif folder:
+        elif folder and name:
-            searchpath = self.params['folder']
-
-            # get all objects for this path ...
-            f_obj = self.content.searchIndex.FindByInventoryPath(searchpath)
-            if f_obj:
-                if isinstance(f_obj, vim.Datacenter):
-                    f_obj = f_obj.vmFolder
-                for c_obj in f_obj.childEntity:
-                    if not isinstance(c_obj, vim.VirtualMachine):
-                        continue
-                    if c_obj.name == name:
-                        vm = c_obj
             if self.params['name_match'] == 'first':
-                            break
+                match_first = True
-
+            vm = find_vm_by_id(self.content, vm_id=name, vm_id_type="inventory_path", folder=folder, match_first=match_first)
         return vm
 
     def gather_facts(self, vm):
@@ -155,7 +143,7 @@ def main():
     argument_spec = vmware_argument_spec()
     argument_spec.update(
         name=dict(type='str'),
-        name_match=dict(type='str', default='first'),
+        name_match=dict(type='str', choices=['first', 'last'], default='first'),
         uuid=dict(type='str'),
         folder=dict(type='str', default='/vm'),
         datacenter=dict(type='str', required=True),

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

@@ -47,7 +47,7 @@ options:
    name:
         description:
             - Name of the VM to work with
-        required: True
+            - This is required if uuid is not supplied.
    name_match:
         description:
             - If multiple VMs matching the name, use the first or last found
@@ -59,7 +59,22 @@ options:
             - This is required if name is not supplied.
    folder:
         description:
-            - Define instance folder location.
+            - Destination folder, absolute or relative path to find an existing guest.
+            - This is required if name is supplied.
+            - The folder should include the datacenter. ESX's datacenter is ha-datacenter
+            - 'Examples:'
+            - '   folder: /ha-datacenter/vm'
+            - '   folder: ha-datacenter/vm'
+            - '   folder: /datacenter1/vm'
+            - '   folder: datacenter1/vm'
+            - '   folder: /datacenter1/vm/folder1'
+            - '   folder: datacenter1/vm/folder1'
+            - '   folder: /folder1/datacenter1/vm'
+            - '   folder: folder1/datacenter1/vm'
+            - '   folder: /folder1/datacenter1/vm/folder2'
+            - '   folder: vm/folder2'
+            - '   folder: folder2'
+        default: /vm
    datacenter:
         description:
             - Destination datacenter for the deploy operation
@@ -172,11 +187,10 @@ instance:
     sample: None
 """
 
-
 import time
 from ansible.module_utils._text import to_native
 from ansible.module_utils.basic import AnsibleModule
-from ansible.module_utils.vmware import connect_to_api, vmware_argument_spec
+from ansible.module_utils.vmware import connect_to_api, vmware_argument_spec, find_vm_by_id
 
 try:
     import json
@@ -200,38 +214,17 @@ class PyVmomiHelper(object):
 
         self.module = module
         self.params = module.params
-        self.si = None
         self.content = connect_to_api(self.module)
-        self.change_detected = False
 
     def getvm(self, name=None, uuid=None, folder=None):
-
-        # https://www.vmware.com/support/developer/vc-sdk/visdk2xpubs/ReferenceGuide/vim.SearchIndex.html
-        # self.si.content.searchIndex.FindByInventoryPath('DC1/vm/test_folder')
-
         vm = None
-
+        match_first = False
         if uuid:
-            vm = self.content.searchIndex.FindByUuid(uuid=uuid, vmSearch=True)
+            vm = find_vm_by_id(self.content, uuid, vm_id_type="uuid")
-        elif folder:
+        elif folder and name:
-            # Build the absolute folder path to pass into the search method
-            if not self.params['folder'].startswith('/'):
-                self.module.fail_json(msg="Folder %(folder)s needs to be an absolute path, starting with '/'." % self.params)
-            searchpath = '%(datacenter)s%(folder)s' % self.params
-
-            # get all objects for this path ...
-            f_obj = self.content.searchIndex.FindByInventoryPath(searchpath)
-            if f_obj:
-                if isinstance(f_obj, vim.Datacenter):
-                    f_obj = f_obj.vmFolder
-                for c_obj in f_obj.childEntity:
-                    if not isinstance(c_obj, vim.VirtualMachine):
-                        continue
-                    if c_obj.name == name:
-                        vm = c_obj
             if self.params['name_match'] == 'first':
-                            break
+                match_first = True
-
+            vm = find_vm_by_id(self.content, vm_id=name, vm_id_type="inventory_path", folder=folder, match_first=match_first)
         return vm
 
     @staticmethod
@@ -323,7 +316,7 @@ def main():
     argument_spec.update(
         state=dict(default='present', choices=['present', 'absent', 'revert', 'remove_all']),
         name=dict(required=True, type='str'),
-        name_match=dict(type='str', default='first'),
+        name_match=dict(type='str', choices=['first', 'last'], default='first'),
         uuid=dict(type='str'),
         folder=dict(type='str', default='/vm'),
         datacenter=dict(required=True, type='str'),
@@ -333,11 +326,10 @@ def main():
         memory_dump=dict(type='bool', default=False),
         remove_children=dict(type='bool', default=False),
     )
-    module = AnsibleModule(argument_spec=argument_spec)
+    module = AnsibleModule(argument_spec=argument_spec, required_one_of=[['name', 'uuid']])
 
-    # Prepend /vm if it was missing from the folder path, also strip trailing slashes
+    # FindByInventoryPath() does not require an absolute path
-    if not module.params['folder'].startswith('/vm') and module.params['folder'].startswith('/'):
+    # so we should leave the input folder path unmodified
-        module.params['folder'] = '/vm%(folder)s' % module.params
     module.params['folder'] = module.params['folder'].rstrip('/')
 
     pyv = PyVmomiHelper(module)

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

@@ -50,6 +50,24 @@ options:
             - Will help speed up search
         required: False
         default: None
+    folder:
+        description:
+            - Destination folder, absolute or relative path to find an existing guest or create the new guest.
+            - The folder should include the datacenter. ESX's datacenter is ha-datacenter
+            - 'Examples:'
+            - '   folder: /ha-datacenter/vm'
+            - '   folder: ha-datacenter/vm'
+            - '   folder: /datacenter1/vm'
+            - '   folder: datacenter1/vm'
+            - '   folder: /datacenter1/vm/folder1'
+            - '   folder: datacenter1/vm/folder1'
+            - '   folder: /folder1/datacenter1/vm'
+            - '   folder: folder1/datacenter1/vm'
+            - '   folder: /folder1/datacenter1/vm/folder2'
+            - '   folder: vm/folder2'
+            - '   folder: folder2'
+        default: /vm
+        version_added: "2.4"
     vm_id:
         description:
             - The identification for the VM
@@ -104,6 +122,7 @@ EXAMPLES = '''
     username: myUsername
     password: mySecret
     datacenter: myDatacenter
+    folder: /vm
     vm_id: NameOfVM
     vm_username: root
     vm_password: superSecret
@@ -121,6 +140,9 @@ try:
     HAS_PYVMOMI = True
 except ImportError:
     HAS_PYVMOMI = False
+from ansible.module_utils.vmware import connect_to_api, find_cluster_by_name, find_datacenter_by_name, find_vm_by_id, vmware_argument_spec
+from ansible.module_utils.basic import AnsibleModule
+
 
 # https://github.com/vmware/pyvmomi-community-samples/blob/master/samples/execute_program_in_vm.py
 def execute_command(content, vm, vm_username, vm_password, program_path, args="", env=None, cwd=None):
@@ -131,11 +153,12 @@ def execute_command(content, vm, vm_username, vm_password, program_path, args=""
 
     return cmdpid
 
-def main():
 
+def main():
     argument_spec = vmware_argument_spec()
     argument_spec.update(dict(datacenter=dict(default=None, type='str'),
                               cluster=dict(default=None, type='str'),
+                              folder=dict(type='str', default='/vm'),
                               vm_id=dict(required=True, type='str'),
                               vm_id_type=dict(default='vm_name', type='str', choices=['inventory_path', 'uuid', 'dns_name', 'vm_name']),
                               vm_username=dict(required=False, type='str'),
@@ -145,16 +168,19 @@ def main():
                               vm_shell_env=dict(default=None, type='list'),
                               vm_shell_cwd=dict(default=None, type='str')))
 
-    module = AnsibleModule(argument_spec=argument_spec, supports_check_mode=False)
+    module = AnsibleModule(argument_spec=argument_spec,
+                           supports_check_mode=False,
+                           required_if=[['vm_id_type', 'inventory_path', ['folder']]],
+                           )
 
     if not HAS_PYVMOMI:
         module.fail_json(changed=False, msg='pyvmomi is required for this module')
 
-
     try:
         p = module.params
         datacenter_name = p['datacenter']
         cluster_name = p['cluster']
+        folder = p['folder']
         content = connect_to_api(module)
 
         datacenter = None
@@ -169,7 +195,11 @@ def main():
             if not cluster:
                 module.fail_json(changed=False, msg="cluster not found")
 
-        vm = find_vm_by_id(content, p['vm_id'], p['vm_id_type'], datacenter, cluster)
+        if p['vm_id_type'] == 'inventory_path':
+            vm = find_vm_by_id(content, vm_id=p['vm_id'], vm_id_type="inventory_path", folder=folder)
+        else:
+            vm = find_vm_by_id(content, vm_id=p['vm_id'], vm_id_type=p['vm_id_type'], datacenter=datacenter, cluster=cluster)
+
         if not vm:
             module.fail_json(msg='VM not found')
 
@@ -184,8 +214,5 @@ def main():
     except Exception as e:
         module.fail_json(changed=False, msg=str(e))
 
-from ansible.module_utils.vmware import *
-from ansible.module_utils.basic import *
-
 if __name__ == '__main__':
     main()

test/sanity/pep8/legacy-files.txt

@@ -192,7 +192,6 @@ lib/ansible/modules/cloud/vmware/vmware_dvswitch.py
 lib/ansible/modules/cloud/vmware/vmware_local_user_manager.py
 lib/ansible/modules/cloud/vmware/vmware_migrate_vmk.py
 lib/ansible/modules/cloud/vmware/vmware_target_canonical_facts.py
-lib/ansible/modules/cloud/vmware/vmware_vm_shell.py
 lib/ansible/modules/cloud/vmware/vmware_vmotion.py
 lib/ansible/modules/cloud/vmware/vsphere_copy.py
 lib/ansible/modules/cloud/vmware/vsphere_guest.py