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 <akasurde@redhat.com>

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']],
+                           )
 
+    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.

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

@@ -1,57 +1,54 @@
-- 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
@@ -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"