diff --git a/changelogs/fragments/9256-proxmox_disk-fix-async-method-of-resize_disk.yml b/changelogs/fragments/9256-proxmox_disk-fix-async-method-of-resize_disk.yml new file mode 100644 index 0000000000..0b0a826a0d --- /dev/null +++ b/changelogs/fragments/9256-proxmox_disk-fix-async-method-of-resize_disk.yml @@ -0,0 +1,4 @@ +bugfixes: + - proxmox_disk - fix async method and make ``resize_disk`` method handle errors correctly (https://github.com/ansible-collections/community.general/pull/9256). +minor_changes: + - proxmox module utils - add method ``api_task_complete`` that can wait for task completion and return error message (https://github.com/ansible-collections/community.general/pull/9256). diff --git a/plugins/module_utils/proxmox.py b/plugins/module_utils/proxmox.py index 05bf1874b3..b0037dacb3 100644 --- a/plugins/module_utils/proxmox.py +++ b/plugins/module_utils/proxmox.py @@ -8,6 +8,7 @@ from __future__ import absolute_import, division, print_function __metaclass__ = type import traceback +from time import sleep PROXMOXER_IMP_ERR = None try: @@ -70,6 +71,8 @@ def ansible_to_proxmox_bool(value): class ProxmoxAnsible(object): """Base class for Proxmox modules""" + TASK_TIMED_OUT = 'timeout expired' + def __init__(self, module): if not HAS_PROXMOXER: module.fail_json(msg=missing_required_lib('proxmoxer'), exception=PROXMOXER_IMP_ERR) @@ -167,6 +170,32 @@ class ProxmoxAnsible(object): except Exception as e: self.module.fail_json(msg='Unable to retrieve API task ID from node %s: %s' % (node, e)) + def api_task_complete(self, node_name, task_id, timeout): + """Wait until the task stops or times out. + + :param node_name: Proxmox node name where the task is running. + :param task_id: ID of the running task. + :param timeout: Timeout in seconds to wait for the task to complete. + :return: Task completion status (True/False) and ``exitstatus`` message when status=False. + """ + status = {} + while timeout: + try: + status = self.proxmox_api.nodes(node_name).tasks(task_id).status.get() + except Exception as e: + self.module.fail_json(msg='Unable to retrieve API task ID from node %s: %s' % (node_name, e)) + + if status['status'] == 'stopped': + if status['exitstatus'] == 'OK': + return True, None + else: + return False, status['exitstatus'] + else: + timeout -= 1 + if timeout <= 0: + return False, ProxmoxAnsible.TASK_TIMED_OUT + sleep(1) + def get_pool(self, poolid): """Retrieve pool information diff --git a/plugins/modules/proxmox_disk.py b/plugins/modules/proxmox_disk.py index a4a9dd8791..ed67445b30 100644 --- a/plugins/modules/proxmox_disk.py +++ b/plugins/modules/proxmox_disk.py @@ -453,10 +453,11 @@ msg: ''' from ansible.module_utils.basic import AnsibleModule + +from ansible_collections.community.general.plugins.module_utils.version import LooseVersion from ansible_collections.community.general.plugins.module_utils.proxmox import (proxmox_auth_argument_spec, ProxmoxAnsible) from re import compile, match, sub -from time import sleep def disk_conf_str_to_dict(config_string): @@ -531,17 +532,18 @@ class ProxmoxDiskAnsible(ProxmoxAnsible): } return params - def wait_till_complete_or_timeout(self, node_name, task_id): - timeout = self.module.params['timeout'] - while timeout: - if self.api_task_ok(node_name, task_id): - return True - timeout -= 1 - if timeout <= 0: - return False - sleep(1) - def create_disk(self, disk, vmid, vm, vm_config): + """Create a disk in the specified virtual machine. Check if creation is required, + and if so, compile the disk configuration and create it by updating the virtual + machine configuration. After calling the API function, wait for the result. + + :param disk: ID of the disk in format "". + :param vmid: ID of the virtual machine where the disk will be created. + :param vm: Name of the virtual machine where the disk will be created. + :param vm_config: Configuration of the virtual machine. + :return: (bool, string) Whether the task was successful or not + and the message to return to Ansible. + """ create = self.module.params['create'] if create == 'disabled' and disk not in vm_config: # NOOP @@ -610,15 +612,31 @@ class ProxmoxDiskAnsible(ProxmoxAnsible): disk_config_to_apply = {self.module.params["disk"]: config_str} current_task_id = self.proxmox_api.nodes(vm['node']).qemu(vmid).config.post(**disk_config_to_apply) - task_success = self.wait_till_complete_or_timeout(vm['node'], current_task_id) + task_success, fail_reason = self.api_task_complete(vm['node'], current_task_id, self.module.params['timeout']) + if task_success: return True, ok_str % (disk, vmid) else: - self.module.fail_json( - msg=timeout_str % self.proxmox_api.nodes(vm['node']).tasks(current_task_id).log.get()[:1] - ) + if fail_reason == ProxmoxAnsible.TASK_TIMED_OUT: + self.module.fail_json( + msg=timeout_str % self.proxmox_api.nodes(vm['node']).tasks(current_task_id).log.get()[:1] + ) + else: + self.module.fail_json(msg="Error occurred on task execution: %s" % fail_reason) def move_disk(self, disk, vmid, vm, vm_config): + """Call the `move_disk` API function that moves the disk to another storage and wait for the result. + + :param disk: ID of disk in format "". + :param vmid: ID of virtual machine which disk will be moved. + :param vm: Name of virtual machine which disk will be moved. + :param vm_config: Virtual machine configuration. + :return: (bool, string) Whether the task was successful or not + and the message to return to Ansible. + """ + disk_config = disk_conf_str_to_dict(vm_config[disk]) + disk_storage = disk_config["storage_name"] + params = dict() params['disk'] = disk params['vmid'] = vmid @@ -632,19 +650,62 @@ class ProxmoxDiskAnsible(ProxmoxAnsible): params = {k: v for k, v in params.items() if v is not None} if params.get('storage', False): + # Check if the disk is already in the target storage. disk_config = disk_conf_str_to_dict(vm_config[disk]) if params['storage'] == disk_config['storage_name']: - return False + return False, "Disk %s already at %s storage" % (disk, disk_storage) + + current_task_id = self.proxmox_api.nodes(vm['node']).qemu(vmid).move_disk.post(**params) + task_success, fail_reason = self.api_task_complete(vm['node'], current_task_id, self.module.params['timeout']) - task_id = self.proxmox_api.nodes(vm['node']).qemu(vmid).move_disk.post(**params) - task_success = self.wait_till_complete_or_timeout(vm['node'], task_id) if task_success: - return True + return True, "Disk %s moved from VM %s storage %s" % (disk, vmid, disk_storage) else: - self.module.fail_json( - msg='Reached timeout while waiting for moving VM disk. Last line in task before timeout: %s' % - self.proxmox_api.nodes(vm['node']).tasks(task_id).log.get()[:1] - ) + if fail_reason == ProxmoxAnsible.TASK_TIMED_OUT: + self.module.fail_json( + msg='Reached timeout while waiting for moving VM disk. Last line in task before timeout: %s' % + self.proxmox_api.nodes(vm['node']).tasks(current_task_id).log.get()[:1] + ) + else: + self.module.fail_json(msg="Error occurred on task execution: %s" % fail_reason) + + def resize_disk(self, disk, vmid, vm, vm_config): + """Call the `resize` API function to change the disk size and wait for the result. + + :param disk: ID of disk in format "". + :param vmid: ID of virtual machine which disk will be resized. + :param vm: Name of virtual machine which disk will be resized. + :param vm_config: Virtual machine configuration. + :return: (Bool, string) Whether the task was successful or not + and the message to return to Ansible. + """ + size = self.module.params['size'] + if not match(r'^\+?\d+(\.\d+)?[KMGT]?$', size): + self.module.fail_json(msg="Unrecognized size pattern for disk %s: %s" % (disk, size)) + disk_config = disk_conf_str_to_dict(vm_config[disk]) + actual_size = disk_config['size'] + if size == actual_size: + return False, "Disk %s is already %s size" % (disk, size) + + # Resize disk API endpoint has changed at v8.0: PUT method become async. + version = self.version() + pve_major_version = 3 if version < LooseVersion('4.0') else version.version[0] + if pve_major_version >= 8: + current_task_id = self.proxmox_api.nodes(vm['node']).qemu(vmid).resize.set(disk=disk, size=size) + task_success, fail_reason = self.api_task_complete(vm['node'], current_task_id, self.module.params['timeout']) + if task_success: + return True, "Disk %s resized in VM %s" % (disk, vmid) + else: + if fail_reason == ProxmoxAnsible.TASK_TIMED_OUT: + self.module.fail_json( + msg="Reached timeout while resizing disk. Last line in task before timeout: %s" % + self.proxmox_api.nodes(vm['node']).tasks(current_task_id).log.get()[:1] + ) + else: + self.module.fail_json(msg="Error occurred on task execution: %s" % fail_reason) + else: + self.proxmox_api.nodes(vm['node']).qemu(vmid).resize.set(disk=disk, size=size) + return True, "Disk %s resized in VM %s" % (disk, vmid) def main(): @@ -783,11 +844,8 @@ def main(): if state == 'present': try: - success, message = proxmox.create_disk(disk, vmid, vm, vm_config) - if success: - module.exit_json(changed=True, vmid=vmid, msg=message) - else: - module.exit_json(changed=False, vmid=vmid, msg=message) + changed, message = proxmox.create_disk(disk, vmid, vm, vm_config) + module.exit_json(changed=changed, vmid=vmid, msg=message) except Exception as e: module.fail_json(vmid=vmid, msg='Unable to create/update disk %s in VM %s: %s' % (disk, vmid, str(e))) @@ -804,27 +862,15 @@ def main(): elif state == 'moved': try: - disk_config = disk_conf_str_to_dict(vm_config[disk]) - disk_storage = disk_config["storage_name"] - if proxmox.move_disk(disk, vmid, vm, vm_config): - module.exit_json(changed=True, vmid=vmid, - msg="Disk %s moved from VM %s storage %s" % (disk, vmid, disk_storage)) - else: - module.exit_json(changed=False, vmid=vmid, msg="Disk %s already at %s storage" % (disk, disk_storage)) + changed, message = proxmox.move_disk(disk, vmid, vm, vm_config) + module.exit_json(changed=changed, vmid=vmid, msg=message) except Exception as e: module.fail_json(msg="Failed to move disk %s in VM %s with exception: %s" % (disk, vmid, str(e))) elif state == 'resized': try: - size = module.params['size'] - if not match(r'^\+?\d+(\.\d+)?[KMGT]?$', size): - module.fail_json(msg="Unrecognized size pattern for disk %s: %s" % (disk, size)) - disk_config = disk_conf_str_to_dict(vm_config[disk]) - actual_size = disk_config['size'] - if size == actual_size: - module.exit_json(changed=False, vmid=vmid, msg="Disk %s is already %s size" % (disk, size)) - proxmox.proxmox_api.nodes(vm['node']).qemu(vmid).resize.set(disk=disk, size=size) - module.exit_json(changed=True, vmid=vmid, msg="Disk %s resized in VM %s" % (disk, vmid)) + changed, message = proxmox.resize_disk(disk, vmid, vm, vm_config) + module.exit_json(changed=changed, vmid=vmid, msg=message) except Exception as e: module.fail_json(msg="Failed to resize disk %s in VM %s with exception: %s" % (disk, vmid, str(e)))