proxmox_kvm: code cleanup (#934) (#956)

* proxmox_kvm: remove redundant parameters

The functions start_vm() and stop_vm() receive four common parameters:
module, proxmox, vm and vmid.
The last too are redundant so keep only vm.

I also took the opportunity to remove extra API calls to proxmox.nodes()
by assigning its return value to a variable.

* proxmox_kvm: remove extra calls to status.current

The get_vm() function already returns an array of properties containing
the status so remove extra API calls to retrieve this information.

Example:

    [{''netin'': 177232, ''name'': ''test-instance'', ''maxcpu'': 1, ''node'': ''prx-01'', ''disk'': 0, ''template'': 0, ''uptime'': 267, ''cpu'': 0.0410680030805531, ''diskread'': 165294744, ''maxdisk'': 10737418240, ''vmid'': 42, ''status'': ''running'', ''id'': ''qemu/42'', ''maxmem'': 536870912, ''diskwrite'': 18528256, ''netout'': 2918, ''type'': ''qemu'', ''mem'': 160284950}]

* proxmox_kvm: kill VZ_TYPE global variable

It reduces readability without providing much values nowadays.

* proxmox_kvm: simplify vmid generation

Forgotten suggestion from Felix Fontein in PR#811.

* proxmox_kvm: add changelog fragment for PR#934

(cherry picked from commit 02e80c610b)

changelogs/fragments/943-proxmox-kvm-code-cleanup.yml

@@ -0,0 +1,3 @@
+---
+minor_changes:
+  - proxmox_kvm - improve code readability (https://github.com/ansible-collections/community.general/pull/934).

plugins/modules/cloud/misc/proxmox_kvm.py

@@ -645,9 +645,6 @@ from ansible.module_utils.basic import AnsibleModule
 from ansible.module_utils._text import to_native
 
 
-VZ_TYPE = 'qemu'
-
-
 def get_nextvmid(module, proxmox):
     try:
         vmid = proxmox.cluster.nextid.get()
@@ -715,7 +712,7 @@ def settings(module, proxmox, vmid, node, name, **kwargs):
     # Sanitize kwargs. Remove not defined args and ensure True and False converted to int.
     kwargs = dict((k, v) for k, v in kwargs.items() if v is not None)
 
-    if getattr(proxmox_node, VZ_TYPE)(vmid).config.set(**kwargs) is None:
+    if proxmox_node.qemu(vmid).config.set(**kwargs) is None:
         return True
     else:
         return False
@@ -798,7 +795,7 @@ def create_vm(module, proxmox, vmid, newid, node, name, memory, cpu, cores, sock
         module.fail_json(msg='skiplock parameter require root@pam user. ')
 
     if update:
-        if getattr(proxmox_node, VZ_TYPE)(vmid).config.set(name=name, memory=memory, cpu=cpu, cores=cores, sockets=sockets, **kwargs) is None:
+        if proxmox_node.qemu(vmid).config.set(name=name, memory=memory, cpu=cpu, cores=cores, sockets=sockets, **kwargs) is None:
             return True
         else:
             return False
@@ -809,7 +806,7 @@ def create_vm(module, proxmox, vmid, newid, node, name, memory, cpu, cores, sock
         clone_params.update(dict([k, int(v)] for k, v in clone_params.items() if isinstance(v, bool)))
         taskid = proxmox_node.qemu(vmid).clone.post(newid=newid, name=name, **clone_params)
     else:
-        taskid = getattr(proxmox_node, VZ_TYPE).create(vmid=vmid, name=name, memory=memory, cpu=cpu, cores=cores, sockets=sockets, **kwargs)
+        taskid = proxmox_node.qemu.create(vmid=vmid, name=name, memory=memory, cpu=cpu, cores=cores, sockets=sockets, **kwargs)
 
     if not wait_for_task(module, proxmox, node, taskid):
         module.fail_json(msg='Reached timeout while waiting for creating VM. Last line in task before timeout: %s' %
@@ -818,20 +815,24 @@ def create_vm(module, proxmox, vmid, newid, node, name, memory, cpu, cores, sock
     return True
 
 
-def start_vm(module, proxmox, vm, vmid):
+def start_vm(module, proxmox, vm):
-    taskid = getattr(proxmox.nodes(vm[0]['node']), VZ_TYPE)(vmid).status.start.post()
+    vmid = vm[0]['vmid']
+    proxmox_node = proxmox.nodes(vm[0]['node'])
+    taskid = proxmox_node.qemu(vmid).status.start.post()
     if not wait_for_task(module, proxmox, vm[0]['node'], taskid):
         module.fail_json(msg='Reached timeout while waiting for starting VM. Last line in task before timeout: %s' %
-                         proxmox.nodes(vm[0]['node']).tasks(taskid).log.get()[:1])
+                         proxmox_node.tasks(taskid).log.get()[:1])
         return False
     return True
 
 
-def stop_vm(module, proxmox, vm, vmid, force):
+def stop_vm(module, proxmox, vm, force):
-    taskid = proxmox.nodes(vm[0]['node']).qemu(vmid).status.shutdown.post(forceStop=(1 if force else 0))
+    vmid = vm[0]['vmid']
+    proxmox_node = proxmox.nodes(vm[0]['node'])
+    taskid = proxmox_node.qemu(vmid).status.shutdown.post(forceStop=(1 if force else 0))
     if not wait_for_task(module, proxmox, vm[0]['node'], taskid):
         module.fail_json(msg='Reached timeout while waiting for stopping VM. Last line in task before timeout: %s' %
-                         proxmox.nodes(vm[0]['node']).tasks(taskid).log.get()[:1])
+                         proxmox_node.tasks(taskid).log.get()[:1])
         return False
     return True
 
@@ -953,7 +954,6 @@ def main():
 
     try:
         proxmox = ProxmoxAPI(api_host, user=api_user, password=api_password, verify_ssl=validate_certs)
-        global VZ_TYPE
         global PVE_MAJOR_VERSION
         PVE_MAJOR_VERSION = 3 if proxmox_version(proxmox) < LooseVersion('4.0') else 4
     except Exception as e:
@@ -968,7 +968,7 @@ def main():
             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
+            clone_target = clone or name
             try:
                 vmid = get_vmid(proxmox, clone_target)[0]
             except Exception as e:
@@ -1095,7 +1095,7 @@ def main():
             elif clone is not None:
                 module.fail_json(msg="Unable to clone vm {0} from vmid {1}=".format(name, vmid) + str(e))
             else:
-                module.fail_json(msg="creation of %s VM %s with vmid %s failed with exception=%s" % (VZ_TYPE, name, vmid, e))
+                module.fail_json(msg="creation of qemu VM %s with vmid %s failed with exception=%s" % (name, vmid, e))
 
     elif state == 'started':
         try:
@@ -1104,10 +1104,10 @@ def main():
             vm = get_vm(proxmox, vmid)
             if not vm:
                 module.fail_json(msg='VM with vmid <%s> does not exist in cluster' % vmid)
-            if getattr(proxmox.nodes(vm[0]['node']), VZ_TYPE)(vmid).status.current.get()['status'] == 'running':
+            if vm[0]['status'] == 'running':
                 module.exit_json(changed=False, msg="VM %s is already running" % vmid)
 
-            if start_vm(module, proxmox, vm, vmid):
+            if start_vm(module, proxmox, vm):
                 module.exit_json(changed=True, msg="VM %s started" % vmid)
         except Exception as e:
             module.fail_json(msg="starting of VM %s failed with exception: %s" % (vmid, e))
@@ -1121,10 +1121,10 @@ def main():
             if not vm:
                 module.fail_json(msg='VM with vmid = %s does not exist in cluster' % vmid)
 
-            if getattr(proxmox.nodes(vm[0]['node']), VZ_TYPE)(vmid).status.current.get()['status'] == 'stopped':
+            if vm[0]['status'] == 'stopped':
                 module.exit_json(changed=False, msg="VM %s is already stopped" % vmid)
 
-            if stop_vm(module, proxmox, vm, vmid, force=module.params['force']):
+            if stop_vm(module, proxmox, vm, force=module.params['force']):
                 module.exit_json(changed=True, msg="VM %s is shutting down" % vmid)
         except Exception as e:
             module.fail_json(msg="stopping of VM %s failed with exception: %s" % (vmid, e))
@@ -1137,10 +1137,10 @@ def main():
             vm = get_vm(proxmox, vmid)
             if not vm:
                 module.fail_json(msg='VM with vmid = %s does not exist in cluster' % vmid)
-            if getattr(proxmox.nodes(vm[0]['node']), VZ_TYPE)(vmid).status.current.get()['status'] == 'stopped':
+            if vm[0]['status'] == 'stopped':
                 module.exit_json(changed=False, msg="VM %s is not running" % vmid)
 
-            if stop_vm(module, proxmox, vm, vmid, force=module.params['force']) and start_vm(module, proxmox, vm, vmid):
+            if stop_vm(module, proxmox, vm, force=module.params['force']) and start_vm(module, proxmox, vm):
                 module.exit_json(changed=True, msg="VM %s is restarted" % vmid)
         except Exception as e:
             module.fail_json(msg="restarting of VM %s failed with exception: %s" % (vmid, e))
@@ -1152,9 +1152,9 @@ def main():
                 module.exit_json(changed=False)
 
             proxmox_node = proxmox.nodes(vm[0]['node'])
-            if getattr(proxmox_node, VZ_TYPE)(vmid).status.current.get()['status'] == 'running':
+            if vm[0]['status'] == 'running':
                 module.exit_json(changed=False, msg="VM %s is running. Stop it before deletion." % vmid)
-            taskid = getattr(proxmox_node, VZ_TYPE).delete(vmid)
+            taskid = proxmox_node.qemu.delete(vmid)
             if not wait_for_task(module, proxmox, vm[0]['node'], taskid):
                 module.fail_json(msg='Reached timeout while waiting for removing VM. Last line in task before timeout: %s' %
                                  proxmox_node.tasks(taskid).log.get()[:1])
@@ -1170,7 +1170,7 @@ def main():
         vm = get_vm(proxmox, vmid)
         if not vm:
             module.fail_json(msg='VM with vmid = %s does not exist in cluster' % vmid)
-        current = getattr(proxmox.nodes(vm[0]['node']), VZ_TYPE)(vmid).status.current.get()['status']
+        current = proxmox.nodes(vm[0]['node']).qemu(vmid).status.current.get()['status']
         status['status'] = current
         if status:
             module.exit_json(changed=False, msg="VM %s with vmid = %s is %s" % (name, vmid, current), **status)