diff --git a/changelogs/fragments/6757-proxmox-template-fix-upload-error.yml b/changelogs/fragments/6757-proxmox-template-fix-upload-error.yml new file mode 100644 index 0000000000..74cc7de282 --- /dev/null +++ b/changelogs/fragments/6757-proxmox-template-fix-upload-error.yml @@ -0,0 +1,3 @@ +--- +bugfixes: + - proxmox_template - require ``requests_toolbelt`` module to fix issue with uploading large templates (https://github.com/ansible-collections/community.general/issues/5579, https://github.com/ansible-collections/community.general/pull/6757). diff --git a/plugins/module_utils/proxmox.py b/plugins/module_utils/proxmox.py index 47526e6b7b..09e3341780 100644 --- a/plugins/module_utils/proxmox.py +++ b/plugins/module_utils/proxmox.py @@ -18,6 +18,7 @@ import traceback PROXMOXER_IMP_ERR = None try: from proxmoxer import ProxmoxAPI + from proxmoxer import __version__ as proxmoxer_version HAS_PROXMOXER = True except ImportError: HAS_PROXMOXER = False @@ -80,6 +81,7 @@ class ProxmoxAnsible(object): self.module = module self.proxmox_api = self._connect() + self.proxmoxer_version = proxmoxer_version # Test token validity try: self.proxmox_api.version.get() diff --git a/plugins/modules/proxmox_template.py b/plugins/modules/proxmox_template.py index b851eab258..615bfc1823 100644 --- a/plugins/modules/proxmox_template.py +++ b/plugins/modules/proxmox_template.py @@ -65,7 +65,8 @@ options: choices: ['present', 'absent'] default: present notes: - - Requires C(proxmoxer) and C(requests) modules on host. This modules can be installed with M(ansible.builtin.pip). + - Requires C(proxmoxer) and C(requests) modules on host. Those modules can be installed with M(ansible.builtin.pip). + - C(proxmoxer) >= 1.2.0 requires C(requests_toolbelt) to upload files larger than 256 MB. author: Sergei Antipov (@UnderGreen) extends_documentation_fragment: - community.general.proxmox.documentation @@ -123,15 +124,29 @@ EXAMPLES = ''' import os import time +import traceback -from ansible.module_utils.basic import AnsibleModule +from ansible.module_utils.basic import AnsibleModule, missing_required_lib from ansible_collections.community.general.plugins.module_utils.proxmox import (proxmox_auth_argument_spec, ProxmoxAnsible) +from ansible_collections.community.general.plugins.module_utils.version import LooseVersion + +REQUESTS_TOOLBELT_ERR = None +try: + # requests_toolbelt is used internally by proxmoxer module + import requests_toolbelt # noqa: F401, pylint: disable=unused-import + HAS_REQUESTS_TOOLBELT = True +except ImportError: + HAS_REQUESTS_TOOLBELT = False + REQUESTS_TOOLBELT_ERR = traceback.format_exc() class ProxmoxTemplateAnsible(ProxmoxAnsible): def get_template(self, node, storage, content_type, template): - return [True for tmpl in self.proxmox_api.nodes(node).storage(storage).content.get() - if tmpl['volid'] == '%s:%s/%s' % (storage, content_type, template)] + try: + return [True for tmpl in self.proxmox_api.nodes(node).storage(storage).content.get() + if tmpl['volid'] == '%s:%s/%s' % (storage, content_type, template)] + except Exception as e: + self.module.fail_json(msg="Failed to retrieve template '%s:%s/%s': %s" % (storage, content_type, template, e)) def task_status(self, node, taskid, timeout): """ @@ -149,12 +164,24 @@ class ProxmoxTemplateAnsible(ProxmoxAnsible): return False def upload_template(self, node, storage, content_type, realpath, timeout): - taskid = self.proxmox_api.nodes(node).storage(storage).upload.post(content=content_type, filename=open(realpath, 'rb')) - return self.task_status(node, taskid, timeout) + stats = os.stat(realpath) + if (LooseVersion(self.proxmoxer_version) >= LooseVersion('1.2.0') and + stats.st_size > 268435456 and not HAS_REQUESTS_TOOLBELT): + self.module.fail_json(msg="'requests_toolbelt' module is required to upload files larger than 256MB", + exception=missing_required_lib('requests_toolbelt')) + + try: + taskid = self.proxmox_api.nodes(node).storage(storage).upload.post(content=content_type, filename=open(realpath, 'rb')) + return self.task_status(node, taskid, timeout) + except Exception as e: + self.module.fail_json(msg="Uploading template %s failed with error: %s" % (realpath, e)) def download_template(self, node, storage, template, timeout): - taskid = self.proxmox_api.nodes(node).aplinfo.post(storage=storage, template=template) - return self.task_status(node, taskid, timeout) + try: + taskid = self.proxmox_api.nodes(node).aplinfo.post(storage=storage, template=template) + return self.task_status(node, taskid, timeout) + except Exception as e: + self.module.fail_json(msg="Downloading template %s failed with error: %s" % (template, e)) def delete_template(self, node, storage, content_type, template, timeout): volid = '%s:%s/%s' % (storage, content_type, template) @@ -199,35 +226,32 @@ def main(): timeout = module.params['timeout'] if state == 'present': - try: - content_type = module.params['content_type'] - src = module.params['src'] + content_type = module.params['content_type'] + src = module.params['src'] - # download appliance template - if content_type == 'vztmpl' and not src: - template = module.params['template'] + # download appliance template + if content_type == 'vztmpl' and not src: + template = module.params['template'] - if not template: - module.fail_json(msg='template param for downloading appliance template is mandatory') + if not template: + module.fail_json(msg='template param for downloading appliance template is mandatory') - if proxmox.get_template(node, storage, content_type, template) and not module.params['force']: - module.exit_json(changed=False, msg='template with volid=%s:%s/%s already exists' % (storage, content_type, template)) - - if proxmox.download_template(node, storage, template, timeout): - module.exit_json(changed=True, msg='template with volid=%s:%s/%s downloaded' % (storage, content_type, template)) - - template = os.path.basename(src) if proxmox.get_template(node, storage, content_type, template) and not module.params['force']: - module.exit_json(changed=False, msg='template with volid=%s:%s/%s is already exists' % (storage, content_type, template)) - elif not src: - module.fail_json(msg='src param to uploading template file is mandatory') - elif not (os.path.exists(src) and os.path.isfile(src)): - module.fail_json(msg='template file on path %s not exists' % src) + module.exit_json(changed=False, msg='template with volid=%s:%s/%s already exists' % (storage, content_type, template)) - if proxmox.upload_template(node, storage, content_type, src, timeout): - module.exit_json(changed=True, msg='template with volid=%s:%s/%s uploaded' % (storage, content_type, template)) - except Exception as e: - module.fail_json(msg="uploading/downloading of template %s failed with exception: %s" % (template, e)) + if proxmox.download_template(node, storage, template, timeout): + module.exit_json(changed=True, msg='template with volid=%s:%s/%s downloaded' % (storage, content_type, template)) + + template = os.path.basename(src) + if proxmox.get_template(node, storage, content_type, template) and not module.params['force']: + module.exit_json(changed=False, msg='template with volid=%s:%s/%s is already exists' % (storage, content_type, template)) + elif not src: + module.fail_json(msg='src param to uploading template file is mandatory') + elif not (os.path.exists(src) and os.path.isfile(src)): + module.fail_json(msg='template file on path %s not exists' % src) + + if proxmox.upload_template(node, storage, content_type, src, timeout): + module.exit_json(changed=True, msg='template with volid=%s:%s/%s uploaded' % (storage, content_type, template)) elif state == 'absent': try: diff --git a/tests/integration/targets/proxmox_template/aliases b/tests/integration/targets/proxmox_template/aliases new file mode 100644 index 0000000000..5d9af81016 --- /dev/null +++ b/tests/integration/targets/proxmox_template/aliases @@ -0,0 +1,6 @@ +# Copyright (c) Ansible Project +# GNU General Public License v3.0+ (see LICENSES/GPL-3.0-or-later.txt or https://www.gnu.org/licenses/gpl-3.0.txt) +# SPDX-License-Identifier: GPL-3.0-or-later + +unsupported +proxmox_template diff --git a/tests/integration/targets/proxmox_template/tasks/main.yml b/tests/integration/targets/proxmox_template/tasks/main.yml new file mode 100644 index 0000000000..2d1187e890 --- /dev/null +++ b/tests/integration/targets/proxmox_template/tasks/main.yml @@ -0,0 +1,136 @@ +#################################################################### +# WARNING: These are designed specifically for Ansible tests # +# and should not be used as examples of how to write Ansible roles # +#################################################################### + +# Copyright (c) 2023, Sergei Antipov +# GNU General Public License v3.0+ (see LICENSES/GPL-3.0-or-later.txt or https://www.gnu.org/licenses/gpl-3.0.txt) +# SPDX-License-Identifier: GPL-3.0-or-later + +- name: Proxmox VE virtual machines templates management + tags: ['template'] + vars: + filename: /tmp/dummy.iso + block: + - name: Create dummy ISO file + ansible.builtin.command: + cmd: 'truncate -s 300M {{ filename }}' + + - name: Delete requests_toolbelt module if it is installed + ansible.builtin.pip: + name: requests_toolbelt + state: absent + + - name: Install latest proxmoxer + ansible.builtin.pip: + name: proxmoxer + state: latest + + - name: Upload ISO as template to Proxmox VE cluster should fail + proxmox_template: + api_host: '{{ api_host }}' + api_user: '{{ user }}@{{ domain }}' + api_password: '{{ api_password | default(omit) }}' + api_token_id: '{{ api_token_id | default(omit) }}' + api_token_secret: '{{ api_token_secret | default(omit) }}' + validate_certs: '{{ validate_certs }}' + node: '{{ node }}' + src: '{{ filename }}' + content_type: iso + force: true + register: result + ignore_errors: true + + - assert: + that: + - result is failed + - result.msg is match('\'requests_toolbelt\' module is required to upload files larger than 256MB') + + - name: Install old (1.1.2) version of proxmoxer + ansible.builtin.pip: + name: proxmoxer==1.1.1 + state: present + + - name: Upload ISO as template to Proxmox VE cluster should be successful + proxmox_template: + api_host: '{{ api_host }}' + api_user: '{{ user }}@{{ domain }}' + api_password: '{{ api_password | default(omit) }}' + api_token_id: '{{ api_token_id | default(omit) }}' + api_token_secret: '{{ api_token_secret | default(omit) }}' + validate_certs: '{{ validate_certs }}' + node: '{{ node }}' + src: '{{ filename }}' + content_type: iso + force: true + register: result + + - assert: + that: + - result is changed + - result is success + - result.msg is match('template with volid=local:iso/dummy.iso uploaded') + + - name: Install latest proxmoxer + ansible.builtin.pip: + name: proxmoxer + state: latest + + - name: Make smaller dummy file + ansible.builtin.command: + cmd: 'truncate -s 128M {{ filename }}' + + - name: Upload ISO as template to Proxmox VE cluster should be successful + proxmox_template: + api_host: '{{ api_host }}' + api_user: '{{ user }}@{{ domain }}' + api_password: '{{ api_password | default(omit) }}' + api_token_id: '{{ api_token_id | default(omit) }}' + api_token_secret: '{{ api_token_secret | default(omit) }}' + validate_certs: '{{ validate_certs }}' + node: '{{ node }}' + src: '{{ filename }}' + content_type: iso + force: true + register: result + + - assert: + that: + - result is changed + - result is success + - result.msg is match('template with volid=local:iso/dummy.iso uploaded') + + - name: Install requests_toolbelt + ansible.builtin.pip: + name: requests_toolbelt + state: present + + - name: Make big dummy file + ansible.builtin.command: + cmd: 'truncate -s 300M {{ filename }}' + + - name: Upload ISO as template to Proxmox VE cluster should be successful + proxmox_template: + api_host: '{{ api_host }}' + api_user: '{{ user }}@{{ domain }}' + api_password: '{{ api_password | default(omit) }}' + api_token_id: '{{ api_token_id | default(omit) }}' + api_token_secret: '{{ api_token_secret | default(omit) }}' + validate_certs: '{{ validate_certs }}' + node: '{{ node }}' + src: '{{ filename }}' + content_type: iso + force: true + register: result + + - assert: + that: + - result is changed + - result is success + - result.msg is match('template with volid=local:iso/dummy.iso uploaded') + + always: + - name: Delete ISO file from host + ansible.builtin.file: + path: '{{ filename }}' + state: absent diff --git a/tests/unit/plugins/modules/test_proxmox_kvm.py b/tests/unit/plugins/modules/test_proxmox_kvm.py index c86d804d71..c99cfa37a7 100644 --- a/tests/unit/plugins/modules/test_proxmox_kvm.py +++ b/tests/unit/plugins/modules/test_proxmox_kvm.py @@ -8,8 +8,16 @@ from __future__ import absolute_import, division, print_function __metaclass__ = type +import sys + import pytest +proxmoxer = pytest.importorskip('proxmoxer') +mandatory_py_version = pytest.mark.skipif( + sys.version_info < (2, 7), + reason='The proxmoxer dependency requires python2.7 or higher' +) + from ansible_collections.community.general.plugins.modules import proxmox_kvm from ansible_collections.community.general.tests.unit.compat.mock import patch from ansible_collections.community.general.tests.unit.plugins.modules.utils import ( diff --git a/tests/unit/plugins/modules/test_proxmox_snap.py b/tests/unit/plugins/modules/test_proxmox_snap.py index 4bdcaa8b77..545fcd1f59 100644 --- a/tests/unit/plugins/modules/test_proxmox_snap.py +++ b/tests/unit/plugins/modules/test_proxmox_snap.py @@ -7,8 +7,16 @@ from __future__ import (absolute_import, division, print_function) __metaclass__ = type import json +import sys + import pytest +proxmoxer = pytest.importorskip('proxmoxer') +mandatory_py_version = pytest.mark.skipif( + sys.version_info < (2, 7), + reason='The proxmoxer dependency requires python2.7 or higher' +) + from ansible_collections.community.general.tests.unit.compat.mock import MagicMock, patch from ansible_collections.community.general.plugins.modules import proxmox_snap import ansible_collections.community.general.plugins.module_utils.proxmox as proxmox_utils diff --git a/tests/unit/plugins/modules/test_proxmox_tasks_info.py b/tests/unit/plugins/modules/test_proxmox_tasks_info.py index 0d1b5a7bfb..5c228655be 100644 --- a/tests/unit/plugins/modules/test_proxmox_tasks_info.py +++ b/tests/unit/plugins/modules/test_proxmox_tasks_info.py @@ -10,8 +10,16 @@ from __future__ import (absolute_import, division, print_function) __metaclass__ = type -import pytest import json +import sys + +import pytest + +proxmoxer = pytest.importorskip('proxmoxer') +mandatory_py_version = pytest.mark.skipif( + sys.version_info < (2, 7), + reason='The proxmoxer dependency requires python2.7 or higher' +) from ansible_collections.community.general.plugins.modules import proxmox_tasks_info import ansible_collections.community.general.plugins.module_utils.proxmox as proxmox_utils diff --git a/tests/unit/plugins/modules/test_proxmox_template.py b/tests/unit/plugins/modules/test_proxmox_template.py new file mode 100644 index 0000000000..dc09a44b35 --- /dev/null +++ b/tests/unit/plugins/modules/test_proxmox_template.py @@ -0,0 +1,66 @@ +# -*- coding: utf-8 -*- +# +# Copyright (c) 2023, Sergei Antipov +# GNU General Public License v3.0+ (see LICENSES/GPL-3.0-or-later.txt or https://www.gnu.org/licenses/gpl-3.0.txt) +# SPDX-License-Identifier: GPL-3.0-or-later + +from __future__ import absolute_import, division, print_function + +__metaclass__ = type + +import os +import sys + +import pytest + +proxmoxer = pytest.importorskip('proxmoxer') +mandatory_py_version = pytest.mark.skipif( + sys.version_info < (2, 7), + reason='The proxmoxer dependency requires python2.7 or higher' +) + +from ansible_collections.community.general.plugins.modules import proxmox_template +from ansible_collections.community.general.tests.unit.compat.mock import patch, Mock +from ansible_collections.community.general.tests.unit.plugins.modules.utils import ( + AnsibleFailJson, + ModuleTestCase, + set_module_args, +) +import ansible_collections.community.general.plugins.module_utils.proxmox as proxmox_utils + + +class TestProxmoxTemplateModule(ModuleTestCase): + def setUp(self): + super(TestProxmoxTemplateModule, self).setUp() + proxmox_utils.HAS_PROXMOXER = True + self.module = proxmox_template + self.connect_mock = patch( + "ansible_collections.community.general.plugins.module_utils.proxmox.ProxmoxAnsible._connect" + ) + self.connect_mock.start() + + def tearDown(self): + self.connect_mock.stop() + super(TestProxmoxTemplateModule, self).tearDown() + + @patch("os.stat") + @patch.multiple(os.path, exists=Mock(return_value=True), isfile=Mock(return_value=True)) + def test_module_fail_when_toolbelt_not_installed_and_file_size_is_big(self, mock_stat): + self.module.HAS_REQUESTS_TOOLBELT = False + mock_stat.return_value.st_size = 268435460 + set_module_args( + { + "api_host": "host", + "api_user": "user", + "api_password": "password", + "node": "pve", + "src": "/tmp/mock.iso", + "content_type": "iso" + } + ) + with pytest.raises(AnsibleFailJson) as exc_info: + self.module.main() + + result = exc_info.value.args[0] + assert result["failed"] is True + assert result["msg"] == "'requests_toolbelt' module is required to upload files larger than 256MB" diff --git a/tests/unit/requirements.txt b/tests/unit/requirements.txt index 0aa7c1fc9f..be060a3dd3 100644 --- a/tests/unit/requirements.txt +++ b/tests/unit/requirements.txt @@ -43,4 +43,8 @@ dataclasses ; python_version == '3.6' elastic-apm ; python_version >= '3.6' # requirements for scaleway modules -passlib[argon2] \ No newline at end of file +passlib[argon2] + +# requirements for the proxmox modules +proxmoxer < 2.0.0 ; python_version >= '2.7' and python_version <= '3.6' +proxmoxer ; python_version > '3.6'