proxmox_disk: fix async method of resize_disk (#9256)

* proxmox_disk: fix async method of resize_disk

Rewritten resizing of disk into separated function and used async method to retrieve task result. Additionally, rewritten function to detect failed tasks early, without waiting for timeout.

* proxmox_disk: add changelog fragment

* proxmox_disk: fix sanity errors

* Apply suggestions from code review

Co-authored-by: Alexei Znamensky <103110+russoz@users.noreply.github.com>

* proxmox_disk: workaround for legacy Proxmox

* Apply suggestions from code review

Co-authored-by: Felix Fontein <felix@fontein.de>

* Apply suggestions from the review

---------

Co-authored-by: Alexei Znamensky <103110+russoz@users.noreply.github.com>
Co-authored-by: Felix Fontein <felix@fontein.de>
pull/9443/head
castorsky 2024-12-28 15:37:05 +03:00 committed by GitHub
parent 6748ec3993
commit d8b38073c1
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 123 additions and 44 deletions

View File

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

View File

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

View File

@ -439,10 +439,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):
@ -517,17 +518,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 "<bus><number>".
: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
@ -596,15 +598,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 "<bus><number>".
: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
@ -618,19 +636,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 "<bus><number>".
: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():
@ -769,11 +830,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)))
@ -790,27 +848,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)))