From addb882138d64243dd5ef4237c256ec55136e18e Mon Sep 17 00:00:00 2001 From: Abhijeet Kasurde Date: Fri, 1 Dec 2017 21:21:45 +0530 Subject: [PATCH] VMware: make name and folder required parameter (#33135) This fix makes name and folder as required together parameters in vmware_guest_snapshot. Update integration tests for vmware_guest_snapshot. Fixes: #22644 Signed-off-by: Abhijeet Kasurde --- .../cloud/vmware/vmware_guest_snapshot.py | 46 ++--- .../vmware_guest_snapshot/tasks/main.yml | 170 +++++++++++------- 2 files changed, 119 insertions(+), 97 deletions(-) diff --git a/lib/ansible/modules/cloud/vmware/vmware_guest_snapshot.py b/lib/ansible/modules/cloud/vmware/vmware_guest_snapshot.py index 6bab4bf1046..13b9e08b2a9 100644 --- a/lib/ansible/modules/cloud/vmware/vmware_guest_snapshot.py +++ b/lib/ansible/modules/cloud/vmware/vmware_guest_snapshot.py @@ -16,7 +16,7 @@ ANSIBLE_METADATA = {'metadata_version': '1.1', DOCUMENTATION = ''' --- module: vmware_guest_snapshot -short_description: Manages virtual machines snapshots in vcenter +short_description: Manages virtual machines snapshots in vCenter description: - Create virtual machines snapshots version_added: 2.3 @@ -64,7 +64,6 @@ options: - ' folder: /folder1/datacenter1/vm/folder2' - ' folder: vm/folder2' - ' folder: folder2' - default: /vm datacenter: description: - Destination datacenter for the deploy operation @@ -206,7 +205,6 @@ instance: """ import time - try: import pyVmomi from pyVmomi import vim @@ -215,28 +213,12 @@ except ImportError: from ansible.module_utils.basic import AnsibleModule from ansible.module_utils._text import to_native -from ansible.module_utils.vmware import connect_to_api, find_vm_by_id, HAS_PYVMOMI, list_snapshots, vmware_argument_spec +from ansible.module_utils.vmware import PyVmomi, list_snapshots, vmware_argument_spec -class PyVmomiHelper(object): +class PyVmomiHelper(PyVmomi): def __init__(self, module): - if not HAS_PYVMOMI: - module.fail_json(msg='pyvmomi module required') - - self.module = module - self.params = module.params - self.content = connect_to_api(self.module) - - def getvm(self, name=None, uuid=None, folder=None): - vm = None - match_first = False - if uuid: - 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 + super(PyVmomiHelper, self).__init__(module) @staticmethod def wait_for_task(task): @@ -358,10 +340,10 @@ def main(): argument_spec = vmware_argument_spec() argument_spec.update( state=dict(default='present', choices=['present', 'absent', 'revert', 'remove_all']), - name=dict(required=True, type='str'), + name=dict(type='str'), name_match=dict(type='str', choices=['first', 'last'], default='first'), uuid=dict(type='str'), - folder=dict(type='str', default='/vm'), + folder=dict(type='str'), datacenter=dict(required=True, type='str'), snapshot_name=dict(type='str'), description=dict(type='str', default=''), @@ -371,17 +353,19 @@ def main(): new_snapshot_name=dict(type='str'), new_description=dict(type='str'), ) - module = AnsibleModule(argument_spec=argument_spec, required_one_of=[['name', 'uuid']]) + module = AnsibleModule(argument_spec=argument_spec, + required_together=[['name', 'folder']], + required_one_of=[['name', 'uuid']], + ) - # FindByInventoryPath() does not require an absolute path - # so we should leave the input folder path unmodified - module.params['folder'] = module.params['folder'].rstrip('/') + if module.params['folder']: + # 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) # Check if the VM exists before continuing - vm = pyv.getvm(name=module.params['name'], - folder=module.params['folder'], - uuid=module.params['uuid']) + vm = pyv.get_vm() if not vm: # If UUID is set, getvm select UUID, show error message accordingly. diff --git a/test/integration/targets/vmware_guest_snapshot/tasks/main.yml b/test/integration/targets/vmware_guest_snapshot/tasks/main.yml index a510be29b85..8cfb84e135c 100644 --- a/test/integration/targets/vmware_guest_snapshot/tasks/main.yml +++ b/test/integration/targets/vmware_guest_snapshot/tasks/main.yml @@ -1,58 +1,55 @@ -- name: make sure pyvmomi is installed - pip: - name: pyvmomi - state: latest - when: ansible_user_id == 'root' - -- name: store the vcenter container ip - set_fact: - vcsim: "{{ lookup('env', 'vcenter_host') }}" -- debug: var=vcsim - -- name: Wait for Flask controller to come up online - wait_for: - host: "{{ vcsim }}" - port: 5000 - state: started - -- name: kill vcsim - uri: - url: "{{ 'http://' + vcsim + ':5000/killall' }}" -- name: start vcsim with no folders - uri: - url: "{{ 'http://' + vcsim + ':5000/spawn?datacenter=1&cluster=1&folder=1' }}" - register: vcsim_instance - -- name: Wait for Flask controller to come up online - wait_for: - host: "{{ vcsim }}" - port: 443 - state: started - -- name: get a list of VMS from vcsim - uri: - url: "{{ 'http://' + vcsim + ':5000/govc_find?filter=VM' }}" - register: vmlist - -- set_fact: - vm1: "{{ vmlist['json'][0] }}" - -- name: get a list of datacenters from vcsim - uri: - url: "{{ 'http://' + vcsim + ':5000/govc_find?filter=DC' }}" - register: datacenters - -- set_fact: - dc1: "{{ datacenters['json'][0] }}" - -- debug: var=vcsim_instance -- debug: var=vmlist -- debug: var=vm1 -- debug: var=dc1 - -# FIXME: VCSIM does not currently implement snapshots -# Awaiting: https://github.com/vmware/govmomi/pull/861/files -# +# - name: make sure pyvmomi is installed +# pip: +# name: pyvmomi +# state: latest +# when: ansible_user_id == 'root' +# +# - name: store the vcenter container ip +# set_fact: +# vcsim: "{{ lookup('env', 'vcenter_host') }}" +# - debug: var=vcsim +# +# - name: Wait for Flask controller to come up online +# wait_for: +# host: "{{ vcsim }}" +# port: 5000 +# state: started +# +# - name: kill vcsim +# uri: +# url: "{{ 'http://' + vcsim + ':5000/killall' }}" +# - name: start vcsim with no folders +# uri: +# url: "{{ 'http://' + vcsim + ':5000/spawn?datacenter=1&cluster=1&folder=1' }}" +# register: vcsim_instance +# +# - name: Wait for Flask controller to come up online +# wait_for: +# host: "{{ vcsim }}" +# port: 443 +# state: started +# +# - name: get a list of VMS from vcsim +# uri: +# url: "{{ 'http://' + vcsim + ':5000/govc_find?filter=VM' }}" +# register: vmlist +# +# - set_fact: +# vm1: "{{ vmlist['json'][0] }}" +# +# - name: get a list of datacenters from vcsim +# uri: +# url: "{{ 'http://' + vcsim + ':5000/govc_find?filter=DC' }}" +# register: datacenters +# +# - set_fact: +# dc1: "{{ datacenters['json'][0] }}" +# +# - debug: var=vcsim_instance +# - debug: var=vmlist +# - debug: var=vm1 +# - debug: var=dc1 +# # # Test0001: Try to delete the non-existent snapshot # - name: 0001 - Delete non-existent snapshot # vmware_guest_snapshot: @@ -65,7 +62,7 @@ # name: "{{ vm1 | basename }}" # state: absent # snapshot_name: snap_a - +# # # Test0002: Create two snapshots # - name: 0002 - Create snapshot # vmware_guest_snapshot: @@ -82,7 +79,7 @@ # with_items: # - a # - b - +# # # Test0003: Reanme a to c # - name: 0003 - Rename snapshot # vmware_guest_snapshot: @@ -96,7 +93,7 @@ # state: present # snapshot_name: snap_a # new_snapshot_name: snap_c - +# # # Test0004: Create snap_a again # - name: 0004 - Re-create snapshot a # vmware_guest_snapshot: @@ -110,7 +107,7 @@ # state: present # snapshot_name: snap_a # description: "snap named a" - +# # # Test0005: Change description of snap_c # - name: 0005 - Change description of snap_c # vmware_guest_snapshot: @@ -124,7 +121,7 @@ # state: present # snapshot_name: snap_c # new_description: "renamed to snap_c from snap_a" - +# # # Test0006: Delete snap_b with child remove # - name: 0006 - Delete snap_b with child remove # vmware_guest_snapshot: @@ -138,7 +135,7 @@ # state: absent # snapshot_name: snap_b # remove_children: True - +# # # Test0007: Delete all snapshots # - name: 0007 - Delete all snapshots # vmware_guest_snapshot: @@ -150,7 +147,7 @@ # folder: "{{ vm1 | dirname }}" # name: "{{ vm1 | basename }}" # state: remove_all - +# # # Test0008: Create snap_a again and revert to it # - name: 0008 - Re-create snapshot a # vmware_guest_snapshot: @@ -164,7 +161,7 @@ # state: present # snapshot_name: snap_a # description: "snap named a" - +# # - name: 0008 - Revert to snap_a # vmware_guest_snapshot: # validate_certs: False @@ -176,7 +173,7 @@ # name: "{{ vm1 | basename }}" # state: revert # snapshot_name: snap_a - +# # # Test0009: Create snap_a and check in result # - name: 0009 - create snapshot a # vmware_guest_snapshot: @@ -191,8 +188,49 @@ # snapshot_name: snap_a # description: "snap named a" # register: snapshot_details - +# # - name: Check if snapshot details available or not # assert: # that: -# - "snapshot_details.results['current_snapshot']['name'] == 'snap_a' +# - "snapshot_details.results['current_snapshot']['name'] == 'snap_a'" +# +# # Test0010: Failure sceanrios +# - name: 0010 - Folder is missing +# vmware_guest_snapshot: +# validate_certs: False +# hostname: "{{ vcsim }}" +# username: "{{ vcsim_instance['json']['username'] }}" +# password: "{{ vcsim_instance['json']['password'] }}" +# datacenter: "{{ dc1 | basename }}" +# name: "{{ vm1 | basename }}" +# state: present +# snapshot_name: snap_a +# description: "snap named a" +# register: snapshot_failure_details +# ignore_errors: yes +# +# - name: Check if error is shown +# assert: +# that: +# - "'parameters are required together: name, folder' in snapshot_failure_details['msg']" +# - "snapshot_failure_details.changed == false" +# +# # Test0011: Failure sceanrios - when name and UUID is not specified +# - name: 0011 - name and UUID is missing +# vmware_guest_snapshot: +# validate_certs: False +# hostname: "{{ vcsim }}" +# username: "{{ vcsim_instance['json']['username'] }}" +# password: "{{ vcsim_instance['json']['password'] }}" +# datacenter: "{{ dc1 | basename }}" +# state: present +# snapshot_name: snap_a +# description: "snap named a" +# register: snapshot_failure_details +# ignore_errors: yes +# +# - name: Check if error is shown +# assert: +# that: +# - "'one of the following is required: name, uuid' in snapshot_failure_details['msg']" +# - "snapshot_failure_details.changed == false"