[PR #6757/2d6e369d backport][stable-7] [proxmox_template] Fix error while uploading big ISO to Proxmox VE cluster (#6831)

[proxmox_template] Fix error while uploading big ISO to Proxmox VE cluster (#6757)

* [proxmox_template] Fix error while uploading big ISO to Proxmox VE cluster

* Fix pep8 test

* Add changelog fragment

* Add notes about requests_toolbelt

* Check versions and file size

* Fix typo in notes

* Add unit test. Move try inside of each function.

* Fix sanity tests

* Add proxmoxer in requirements file

* Update integration tests

* Add proxmoxer into constraints.txt

* Address review comments

* Don't run tests on 2.6 python

* Disable Python 2.6 tests for other proxmox modules

(cherry picked from commit 2d6e369d81)

Co-authored-by: Sergei Antipov <greendayonfire@gmail.com>

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

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()

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:

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

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 <greendayonfire at gmail.com>
+# 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

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 (

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

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

tests/unit/plugins/modules/test_proxmox_template.py

@@ -0,0 +1,66 @@
+# -*- coding: utf-8 -*-
+#
+# Copyright (c) 2023, Sergei Antipov <greendayonfire at gmail.com>
+# 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"

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]
+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'