proxmox_kvm: fix idempotency issue with state=absent (#811)

When the `vmid` parameter is not supplied and the module can only rely on
name look-up an early failure can happen if the targeted VM doesn't exist.
In this case a task execution with the parameter `state` set to `absent`
will actually fail instead of being considered ok.

This patch introduces a deferred error-checking for non-existent VMs
by assigning the value -1 to the `vmid` parameter, allowing the actual
verification to be performed in the right code paths.
Is also help to differentiate between a non-existent `vmid` or non-existent
VM `name`.

Previously:

    TASK [ansible-role-proxmox-instance : Remove instance-test]
    changed: [localhost]
    ...
    TASK [ansible-role-proxmox-instance : Remove instance-test]
    fatal: [localhost]: FAILED! => changed=false
      msg: VM instance-test does not exist in cluster.

Now:

    TASK [ansible-role-proxmox-instance : Remove instance-test]
    ok: [localhost]
    ...
    TASK [ansible-role-proxmox-instance : Remove instance-test]
    ok: [localhost]

Update changelogs/fragments/811-proxmox-kvm-state-absent.yml

With suggestions from Felix Fontein <felix@fontein.de>.

changelogs/fragments/811-proxmox-kvm-state-absent.yml

@@ -0,0 +1,3 @@
+---
+bugfixes:
+  - proxmox_kvm - defer error-checking for non-existent VMs in order to fix idempotency of tasks using ``state=absent`` and properly recognize a success (https://github.com/ansible-collections/community.general/pull/811).

plugins/modules/cloud/misc/proxmox_kvm.py

@@ -113,7 +113,8 @@ options:
   force:
     description:
       - Allow to force stop VM.
-      - Can be used only with states C(stopped), C(restarted).
+      - Can be used with states C(stopped) and C(restarted).
+    default: no
     type: bool
   format:
     description:
@@ -754,6 +755,8 @@ def create_vm(module, proxmox, vmid, newid, node, name, memory, cpu, cores, sock
             del kwargs['ide']
         if 'net' in kwargs:
             del kwargs['net']
+        if 'force' in kwargs:
+            del kwargs['force']
 
     # Convert all dict in kwargs to elements. For hostpci[n], ide[n], net[n], numa[n], parallel[n], sata[n], scsi[n], serial[n], virtio[n]
     for k in list(kwargs.keys()):
@@ -863,7 +866,7 @@ def main():
             delete=dict(type='str', default=None),
             description=dict(type='str'),
             digest=dict(type='str'),
-            force=dict(type='bool', default=None),
+            force=dict(type='bool', default=False),
             format=dict(type='str', default='qcow2', choices=['cloop', 'cow', 'qcow', 'qcow2', 'qed', 'raw', 'vmdk']),
             freeze=dict(type='bool'),
             full=dict(type='bool', default=True),
@@ -960,41 +963,43 @@ def main():
     except Exception as e:
         module.fail_json(msg='authorization on proxmox cluster failed with exception: %s' % e)
 
-    # If vmid not set get the Next VM id from ProxmoxAPI
+    # If vmid is not defined then retrieve its value from the vm name,
-    # If vm name is set get the VM id from ProxmoxAPI
+    # the cloned vm name or retrieve the next free VM id from ProxmoxAPI.
     if not vmid:
-        if state == 'present' and (not update and not clone) and (not delete and not revert):
+        if state == 'present' and not update and not clone and not delete and not revert:
             try:
                 vmid = get_nextvmid(module, proxmox)
             except Exception as e:
                 module.fail_json(msg="Can't get the next vmid for VM {0} automatically. Ensure your cluster state is good".format(name))
         else:
+            clone_target = name if not clone else clone
             try:
-                if not clone:
+                vmid = get_vmid(proxmox, clone_target)[0]
-                    vmid = get_vmid(proxmox, name)[0]
-                else:
-                    vmid = get_vmid(proxmox, clone)[0]
             except Exception as e:
-                if not clone:
+                vmid = -1
-                    module.fail_json(msg="VM {0} does not exist in cluster.".format(name))
-                else:
-                    module.fail_json(msg="VM {0} does not exist in cluster.".format(clone))
 
     if clone is not None:
-        if get_vmid(proxmox, name):
+        # If newid is not defined then retrieve the next free id from ProxmoxAPI
-            module.exit_json(changed=False, msg="VM with name <%s> already exists" % name)
-        if vmid is not None:
-            vm = get_vm(proxmox, vmid)
-            if not vm:
-                module.fail_json(msg='VM with vmid = %s does not exist in cluster' % vmid)
         if not newid:
             try:
                 newid = get_nextvmid(module, proxmox)
             except Exception as e:
                 module.fail_json(msg="Can't get the next vmid for VM {0} automatically. Ensure your cluster state is good".format(name))
-        else:
+
-            vm = get_vm(proxmox, newid)
+        # Ensure source VM name exists when cloning
-            if vm:
+        if -1 == vmid:
+            module.fail_json(msg='VM with name = %s does not exist in cluster' % clone)
+
+        # Ensure source VM id exists when cloning
+        if not get_vm(proxmox, vmid):
+            module.fail_json(msg='VM with vmid = %s does not exist in cluster' % vmid)
+
+        # Ensure the choosen VM name doesn't already exist when cloning
+        if get_vmid(proxmox, name):
+            module.exit_json(changed=False, msg="VM with name <%s> already exists" % name)
+
+        # Ensure the choosen VM id doesn't already exist when cloning
+        if get_vm(proxmox, newid):
             module.exit_json(changed=False, msg="vmid %s with VM name %s already exists" % (newid, name))
 
     if delete is not None:
@@ -1003,7 +1008,8 @@ def main():
             module.exit_json(changed=True, msg="Settings has deleted on VM {0} with vmid {1}".format(name, vmid))
         except Exception as e:
             module.fail_json(msg='Unable to delete settings on VM {0} with vmid {1}: '.format(name, vmid) + str(e))
-    elif revert is not None:
+
+    if revert is not None:
         try:
             settings(module, proxmox, vmid, node, name, timeout, revert=revert)
             module.exit_json(changed=True, msg="Settings has reverted on VM {0} with vmid {1}".format(name, vmid))
@@ -1097,6 +1103,8 @@ def main():
 
     elif state == 'started':
         try:
+            if -1 == vmid:
+                module.fail_json(msg='VM with name = %s does not exist in cluster' % name)
             vm = get_vm(proxmox, vmid)
             if not vm:
                 module.fail_json(msg='VM with vmid <%s> does not exist in cluster' % vmid)
@@ -1110,6 +1118,9 @@ def main():
 
     elif state == 'stopped':
         try:
+            if -1 == vmid:
+                module.fail_json(msg='VM with name = %s does not exist in cluster' % name)
+
             vm = get_vm(proxmox, vmid)
             if not vm:
                 module.fail_json(msg='VM with vmid = %s does not exist in cluster' % vmid)
@@ -1124,6 +1135,9 @@ def main():
 
     elif state == 'restarted':
         try:
+            if -1 == vmid:
+                module.fail_json(msg='VM with name = %s does not exist in cluster' % name)
+
             vm = get_vm(proxmox, vmid)
             if not vm:
                 module.fail_json(msg='VM with vmid = %s does not exist in cluster' % vmid)
@@ -1139,7 +1153,7 @@ def main():
         try:
             vm = get_vm(proxmox, vmid)
             if not vm:
-                module.exit_json(changed=False, msg="VM %s does not exist" % vmid)
+                module.exit_json(changed=False)
 
             if getattr(proxmox.nodes(vm[0]['node']), VZ_TYPE)(vmid).status.current.get()['status'] == 'running':
                 module.exit_json(changed=False, msg="VM %s is running. Stop it before deletion." % vmid)
@@ -1160,7 +1174,8 @@ def main():
 
     elif state == 'current':
         status = {}
-        try:
+        if -1 == vmid:
+            module.fail_json(msg='VM with name = %s does not exist in cluster' % name)
         vm = get_vm(proxmox, vmid)
         if not vm:
             module.fail_json(msg='VM with vmid = %s does not exist in cluster' % vmid)
@@ -1168,8 +1183,6 @@ def main():
         status['status'] = current
         if status:
             module.exit_json(changed=False, msg="VM %s with vmid = %s is %s" % (name, vmid, current), **status)
-        except Exception as e:
-            module.fail_json(msg="Unable to get vm {0} with vmid = {1} status: ".format(name, vmid) + str(e))
 
 
 if __name__ == '__main__':