ansible/community.general
1
0
Fork 0
mirror of https://github.com/ansible-collections/community.general.git synced 2024-09-14 20:13:21 +02:00
Code Releases Activity

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

Browse source 
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>.

(cherry picked from commit 73f8338980)

Co-authored-by: Tristan Le Guern <tleguern@bouledef.eu>
This commit is contained in:
patchback[bot] 2020-09-17 19:41:19 +02:00 committed by GitHub
parent 6e2fee77a7
commit 33cf4877f5
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 51 additions and 35 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

3
changelogs/fragments/811-proxmox-kvm-state-absent.yml Normal file
View file

@ -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).

83
plugins/modules/cloud/misc/proxmox_kvm.py
View file

@ -97,7 +97,8 @@ options:
force: force:
description: description:
- Allow to force stop VM. - 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 type: bool
format: format:
description: description:
@ -698,6 +699,8 @@ def create_vm(module, proxmox, vmid, newid, node, name, memory, cpu, cores, sock
del kwargs['ide'] del kwargs['ide']
if 'net' in kwargs: if 'net' in kwargs:
del kwargs['net'] 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] # 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()): for k in list(kwargs.keys()):
@ -807,7 +810,7 @@ def main():
delete=dict(type='str', default=None), delete=dict(type='str', default=None),
description=dict(type='str'), description=dict(type='str'),
digest=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']), format=dict(type='str', default='qcow2', choices=['cloop', 'cow', 'qcow', 'qcow2', 'qed', 'raw', 'vmdk']),
freeze=dict(type='bool'), freeze=dict(type='bool'),
full=dict(type='bool', default=True), full=dict(type='bool', default=True),
@ -904,42 +907,44 @@ def main():
except Exception as e: except Exception as e:
module.fail_json(msg='authorization on proxmox cluster failed with exception: %s' % 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 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: try:
vmid = get_nextvmid(module, proxmox) vmid = get_nextvmid(module, proxmox)
except Exception as e: 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)) module.fail_json(msg="Can't get the next vmid for VM {0} automatically. Ensure your cluster state is good".format(name))
else: else:
clone_target = name if not clone else clone
try: 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: 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 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: if not newid:
try: try:
newid = get_nextvmid(module, proxmox) newid = get_nextvmid(module, proxmox)
except Exception as e: 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)) 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.exit_json(changed=False, msg="vmid %s with VM name %s already exists" % (newid, name)) 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: if delete is not None:
try: try:
@ -947,7 +952,8 @@ def main():
module.exit_json(changed=True, msg="Settings has deleted on VM {0} with vmid {1}".format(name, vmid)) module.exit_json(changed=True, msg="Settings has deleted on VM {0} with vmid {1}".format(name, vmid))
except Exception as e: except Exception as e:
module.fail_json(msg='Unable to delete settings on VM {0} with vmid {1}: '.format(name, vmid) + str(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: try:
settings(module, proxmox, vmid, node, name, timeout, revert=revert) 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)) module.exit_json(changed=True, msg="Settings has reverted on VM {0} with vmid {1}".format(name, vmid))
@ -1041,6 +1047,8 @@ def main():
elif state == 'started': elif state == 'started':
try: try:
if -1 == vmid:
module.fail_json(msg='VM with name = %s does not exist in cluster' % name)
vm = get_vm(proxmox, vmid) vm = get_vm(proxmox, vmid)
if not vm: if not vm:
module.fail_json(msg='VM with vmid <%s> does not exist in cluster' % vmid) module.fail_json(msg='VM with vmid <%s> does not exist in cluster' % vmid)
@ -1054,6 +1062,9 @@ def main():
elif state == 'stopped': elif state == 'stopped':
try: try:
if -1 == vmid:
module.fail_json(msg='VM with name = %s does not exist in cluster' % name)
vm = get_vm(proxmox, vmid) vm = get_vm(proxmox, vmid)
if not vm: if not vm:
module.fail_json(msg='VM with vmid = %s does not exist in cluster' % vmid) module.fail_json(msg='VM with vmid = %s does not exist in cluster' % vmid)
@ -1068,6 +1079,9 @@ def main():
elif state == 'restarted': elif state == 'restarted':
try: try:
if -1 == vmid:
module.fail_json(msg='VM with name = %s does not exist in cluster' % name)
vm = get_vm(proxmox, vmid) vm = get_vm(proxmox, vmid)
if not vm: if not vm:
module.fail_json(msg='VM with vmid = %s does not exist in cluster' % vmid) module.fail_json(msg='VM with vmid = %s does not exist in cluster' % vmid)
@ -1083,7 +1097,7 @@ def main():
try: try:
vm = get_vm(proxmox, vmid) vm = get_vm(proxmox, vmid)
if not vm: 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': 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) module.exit_json(changed=False, msg="VM %s is running. Stop it before deletion." % vmid)
@ -1104,16 +1118,15 @@ def main():
elif state == 'current': elif state == 'current':
status = {} status = {}
try: if -1 == vmid:
vm = get_vm(proxmox, vmid) module.fail_json(msg='VM with name = %s does not exist in cluster' % name)
if not vm: vm = get_vm(proxmox, vmid)
module.fail_json(msg='VM with vmid = %s does not exist in cluster' % vmid) if not vm:
current = getattr(proxmox.nodes(vm[0]['node']), VZ_TYPE)(vmid).status.current.get()['status'] module.fail_json(msg='VM with vmid = %s does not exist in cluster' % vmid)
status['status'] = current current = getattr(proxmox.nodes(vm[0]['node']), VZ_TYPE)(vmid).status.current.get()['status']
if status: status['status'] = current
module.exit_json(changed=False, msg="VM %s with vmid = %s is %s" % (name, vmid, current), **status) if status:
except Exception as e: module.exit_json(changed=False, msg="VM %s with vmid = %s is %s" % (name, vmid, current), **status)
module.fail_json(msg="Unable to get vm {0} with vmid = {1} status: ".format(name, vmid) + str(e))
if __name__ == '__main__': if __name__ == '__main__':