From 9649195c26812f1d543036f2888ed6d006593ac5 Mon Sep 17 00:00:00 2001 From: Abhijeet Kasurde Date: Mon, 24 Jul 2017 22:21:26 +0530 Subject: [PATCH] Refactor getvm method (#27188) Fix refactors getvm method and modules which are using it. Signed-off-by: Abhijeet Kasurde --- lib/ansible/module_utils/vmware.py | 21 ++++-- .../modules/cloud/vmware/vmware_guest.py | 58 +++++---------- .../cloud/vmware/vmware_guest_facts.py | 26 ++----- .../cloud/vmware/vmware_guest_snapshot.py | 64 +++++++--------- .../modules/cloud/vmware/vmware_vm_shell.py | 73 +++++++++++++------ test/sanity/pep8/legacy-files.txt | 1 - 6 files changed, 117 insertions(+), 126 deletions(-) diff --git a/lib/ansible/module_utils/vmware.py b/lib/ansible/module_utils/vmware.py index f0881c66f3..4975f0f58c 100644 --- a/lib/ansible/module_utils/vmware.py +++ b/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 diff --git a/lib/ansible/modules/cloud/vmware/vmware_guest.py b/lib/ansible/modules/cloud/vmware/vmware_guest.py index 474f1e000e..2758b3aa0a 100644 --- a/lib/ansible/modules/cloud/vmware/vmware_guest.py +++ b/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, vmware_argument_spec -from ansible.module_utils.vmware import serialize_spec +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 serialize_spec, find_vm_by_id, vmware_argument_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) - elif folder: - # 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 - + vm = find_vm_by_id(self.content, vm_id=uuid, vm_id_type="uuid") + elif folder and name: + if self.params['name_match'] == 'first': + 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: - e = get_exception() + except Exception as e: 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: - e = get_exception() + except Exception as e: 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: - e = get_exception() - self.module.fail_json(msg="TypeError was returned, please ensure to give correct inputs. %s" % e) + except TypeError as e: + self.module.fail_json(msg="TypeError was returned, please ensure to give correct inputs. %s" % to_text(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'), diff --git a/lib/ansible/modules/cloud/vmware/vmware_guest_facts.py b/lib/ansible/modules/cloud/vmware/vmware_guest_facts.py index 17e096b770..ec6daf6139 100644 --- a/lib/ansible/modules/cloud/vmware/vmware_guest_facts.py +++ b/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: - 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 - + elif folder and name: + if self.params['name_match'] == 'first': + 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), diff --git a/lib/ansible/modules/cloud/vmware/vmware_guest_snapshot.py b/lib/ansible/modules/cloud/vmware/vmware_guest_snapshot.py index 29b8668aa7..49163b2881 100644 --- a/lib/ansible/modules/cloud/vmware/vmware_guest_snapshot.py +++ b/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) - elif folder: - # 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 - + vm = find_vm_by_id(self.content, uuid, vm_id_type="uuid") + elif folder and name: + if self.params['name_match'] == 'first': + 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 - if not module.params['folder'].startswith('/vm') and module.params['folder'].startswith('/'): - module.params['folder'] = '/vm%(folder)s' % module.params + # FindByInventoryPath() does not require an absolute path + # so we should leave the input folder path unmodified module.params['folder'] = module.params['folder'].rstrip('/') pyv = PyVmomiHelper(module) diff --git a/lib/ansible/modules/cloud/vmware/vmware_vm_shell.py b/lib/ansible/modules/cloud/vmware/vmware_vm_shell.py index 50ad40428b..aae3004414 100644 --- a/lib/ansible/modules/cloud/vmware/vmware_vm_shell.py +++ b/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 @@ -97,22 +115,23 @@ extends_documentation_fragment: vmware.documentation ''' EXAMPLES = ''' - - name: shell execution - local_action: - module: vmware_vm_shell - hostname: myVSphere - username: myUsername - password: mySecret - datacenter: myDatacenter - vm_id: NameOfVM - vm_username: root - vm_password: superSecret - vm_shell: /bin/echo - vm_shell_args: " $var >> myFile " - vm_shell_env: - - "PATH=/bin" - - "VAR=test" - vm_shell_cwd: "/tmp" +- name: shell execution + local_action: + module: vmware_vm_shell + hostname: myVSphere + username: myUsername + password: mySecret + datacenter: myDatacenter + folder: /vm + vm_id: NameOfVM + vm_username: root + vm_password: superSecret + vm_shell: /bin/echo + vm_shell_args: " $var >> myFile " + vm_shell_env: + - "PATH=/bin" + - "VAR=test" + vm_shell_cwd: "/tmp" ''' @@ -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() diff --git a/test/sanity/pep8/legacy-files.txt b/test/sanity/pep8/legacy-files.txt index 9c82c4e196..9edf5aa7dd 100644 --- a/test/sanity/pep8/legacy-files.txt +++ b/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