diff --git a/changelog/62580.fixed b/changelog/62580.fixed new file mode 100644 index 00000000000..f0c807c30cc --- /dev/null +++ b/changelog/62580.fixed @@ -0,0 +1 @@ +Fixed missing parameters when cloning a VM with the proxmox salt-cloud driver diff --git a/salt/cloud/clouds/proxmox.py b/salt/cloud/clouds/proxmox.py index 7469aedf863..e293ad4067f 100644 --- a/salt/cloud/clouds/proxmox.py +++ b/salt/cloud/clouds/proxmox.py @@ -31,6 +31,7 @@ import pprint import re import socket import time +import urllib import salt.config as config import salt.utils.cloud @@ -192,7 +193,12 @@ def query(conn_type, option, post_data=None): elif conn_type == "get": response = requests.get(full_url, verify=verify_ssl, cookies=ticket) - response.raise_for_status() + try: + response.raise_for_status() + except requests.exceptions.RequestException: + # Log the details of the response. + log.error("Error in %s query to %s:\n%s", conn_type, full_url, response.text) + raise try: returned_data = response.json() @@ -560,20 +566,18 @@ def _reconfigure_clone(vm_, vmid): log.warning("Reconfiguring clones is only available under `qemu`") return - # TODO: Support other settings here too as these are not the only ones that can be modified after a clone operation + # Determine which settings can be reconfigured. + query_path = "nodes/{}/qemu/{}/config" + valid_settings = set(_get_properties(query_path.format("{node}", "{vmid}"), "POST")) + log.info("Configuring cloned VM") # Modify the settings for the VM one at a time so we can see any problems with the values # as quickly as possible for setting in vm_: - if re.match(r"^(ide|sata|scsi)(\d+)$", setting): - postParams = {setting: vm_[setting]} - query( - "post", - "nodes/{}/qemu/{}/config".format(vm_["host"], vmid), - postParams, - ) - + postParams = None + if setting == "vmid": + pass # vmid gets passed in the URL and can't be reconfigured elif re.match(r"^net(\d+)$", setting): # net strings are a list of comma seperated settings. We need to merge the settings so that # the setting in the profile only changes the settings it touches and the other settings @@ -593,6 +597,13 @@ def _reconfigure_clone(vm_, vmid): # Convert the dictionary back into a string list postParams = {setting: _dictionary_to_stringlist(new_setting)} + + elif setting == "sshkeys": + postParams = {setting: urllib.parse.quote(vm_[setting], safe="")} + elif setting in valid_settings: + postParams = {setting: vm_[setting]} + + if postParams: query( "post", "nodes/{}/qemu/{}/config".format(vm_["host"], vmid), diff --git a/tests/unit/cloud/clouds/test_proxmox.py b/tests/unit/cloud/clouds/test_proxmox.py index cd6a83f412b..f479b71c06f 100644 --- a/tests/unit/cloud/clouds/test_proxmox.py +++ b/tests/unit/cloud/clouds/test_proxmox.py @@ -4,6 +4,7 @@ import io import textwrap +import urllib import requests @@ -11,7 +12,7 @@ from salt import config from salt.cloud.clouds import proxmox from tests.support.helpers import TstSuiteLoggingHandler from tests.support.mixins import LoaderModuleMockMixin -from tests.support.mock import ANY, MagicMock, patch +from tests.support.mock import ANY, MagicMock, call, patch from tests.support.unit import TestCase PROFILE = { @@ -98,9 +99,12 @@ class ProxmoxTest(TestCase, LoaderModuleMockMixin): result = proxmox._dictionary_to_stringlist({"a": "a", "b": "b"}) self.assertEqual(result, "a=a,b=b") - def test__reconfigure_clone(self): + def test__reconfigure_clone_net_hdd(self): # The return_value is for the net reconfigure assertions, it is irrelevant for the rest - with patch.object( + with patch( + "salt.cloud.clouds.proxmox._get_properties", + MagicMock(return_value=["net0", "ide0", "sata0", "scsi0"]), + ), patch.object( proxmox, "query", return_value={"net0": "c=overwritten,g=h"} ) as query: # Test a vm that lacks the required attributes @@ -127,6 +131,59 @@ class ProxmoxTest(TestCase, LoaderModuleMockMixin): "post", "nodes/127.0.0.1/qemu/0/config", {"scsi0": "data"} ) + def test__reconfigure_clone_params(self): + """ + Test cloning a VM with parameters to be reconfigured. + """ + vmid = 201 + properties = { + "ide2": "cdrom", + "sata1": "satatest", + "scsi0": "bootvol", + "net0": "model=virtio", + "agent": "1", + "args": "argsvalue", + "balloon": "128", + "ciuser": "root", + "cores": "2", + "description": "desc", + "memory": "256", + "name": "new2", + "onboot": "0", + "sshkeys": "ssh-rsa ABCDEF user@host\n", + } + query_calls = [call("get", "nodes/myhost/qemu/{}/config".format(vmid))] + for key, value in properties.items(): + if key == "sshkeys": + value = urllib.parse.quote(value, safe="") + query_calls.append( + call( + "post", + "nodes/myhost/qemu/{}/config".format(vmid), + {key: value}, + ) + ) + + mock_query = MagicMock(return_value="") + with patch( + "salt.cloud.clouds.proxmox._get_properties", + MagicMock(return_value=list(properties.keys())), + ), patch("salt.cloud.clouds.proxmox.query", mock_query): + vm_ = { + "profile": "my_proxmox", + "driver": "proxmox", + "technology": "qemu", + "name": "new2", + "host": "myhost", + "clone": True, + "clone_from": 123, + "ip_address": "10.10.10.10", + } + vm_.update(properties) + + proxmox._reconfigure_clone(vm_, vmid) + mock_query.assert_has_calls(query_calls, any_order=True) + def test_clone(self): """ Test that an integer value for clone_from