From 02e80c610b667b7e38de6508f4ccfb4869575303 Mon Sep 17 00:00:00 2001 From: Tristan Le Guern Date: Wed, 23 Sep 2020 15:39:46 +0200 Subject: [PATCH] proxmox_kvm: code cleanup (#934) * 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 --- .../943-proxmox-kvm-code-cleanup.yml | 3 ++ plugins/modules/cloud/misc/proxmox_kvm.py | 50 +++++++++---------- 2 files changed, 28 insertions(+), 25 deletions(-) create mode 100644 changelogs/fragments/943-proxmox-kvm-code-cleanup.yml diff --git a/changelogs/fragments/943-proxmox-kvm-code-cleanup.yml b/changelogs/fragments/943-proxmox-kvm-code-cleanup.yml new file mode 100644 index 0000000000..eab6f3a3e9 --- /dev/null +++ b/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). diff --git a/plugins/modules/cloud/misc/proxmox_kvm.py b/plugins/modules/cloud/misc/proxmox_kvm.py index 00e8f84b5f..7a7070715d 100644 --- a/plugins/modules/cloud/misc/proxmox_kvm.py +++ b/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): - taskid = getattr(proxmox.nodes(vm[0]['node']), VZ_TYPE)(vmid).status.start.post() +def start_vm(module, proxmox, vm): + 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): - taskid = proxmox.nodes(vm[0]['node']).qemu(vmid).status.shutdown.post(forceStop=(1 if force else 0)) +def stop_vm(module, proxmox, vm, force): + 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,12 +1152,12 @@ 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': if module.params['force']: - stop_vm(module, proxmox, vm, vmid, True) + stop_vm(module, proxmox, vm, True) else: module.exit_json(changed=False, msg="VM %s is running. Stop it before deletion or use force=yes." % 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]) @@ -1173,7 +1173,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)