Multiple fixes for Proxmox salt-cloud driver

changelog/62211.fixed

@@ -0,0 +1 @@
+Improved error handling and diagnostics in the proxmox salt-cloud driver

changelog/62521.fixed

@@ -0,0 +1 @@
+Fixed the omitted "pool" parameter when cloning a VM with the proxmox salt-cloud driver

changelog/62558.fixed

@@ -0,0 +1 @@
+Fixed salt-cloud cloning a proxmox VM with a specified new vmid.

changelog/62580.fixed

@@ -0,0 +1 @@
+Fixed missing parameters when cloning a VM with the proxmox salt-cloud driver

salt/cloud/clouds/proxmox.py

@@ -31,6 +31,7 @@
 import re
 import socket
 import time
+import urllib
 
 import salt.config as config
 import salt.utils.cloud
@@ -136,7 +137,9 @@ def _authenticate():
     connect_data = {"username": username, "password": passwd}
     full_url = "https://{}:{}/api2/json/access/ticket".format(url, port)
 
-    returned_data = requests.post(full_url, verify=verify_ssl, data=connect_data).json()
+    response = requests.post(full_url, verify=verify_ssl, data=connect_data)
+    response.raise_for_status()
+    returned_data = response.json()
 
     ticket = {"PVEAuthCookie": returned_data["data"]["ticket"]}
     csrf = str(returned_data["data"]["CSRFPreventionToken"])
@@ -190,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()
@@ -558,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
@@ -591,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),
@@ -655,23 +668,26 @@ def create(vm_):
         newid = _get_next_vmid()
         data = create_node(vm_, newid)
     except Exception as exc:  # pylint: disable=broad-except
+        msg = str(exc)
+        if (
+            isinstance(exc, requests.exceptions.RequestException)
+            and exc.response is not None
+        ):
+            msg = msg + "\n" + exc.response.text
         log.error(
             "Error creating %s on PROXMOX\n\n"
             "The following exception was thrown when trying to "
             "run the initial deployment: \n%s",
             vm_["name"],
-            exc,
+            msg,
             # Show the traceback if the debug logging level is enabled
             exc_info_on_loglevel=logging.DEBUG,
         )
         return False
 
     ret["creation_data"] = data
     name = vm_["name"]  # hostname which we know
-    if "clone" in vm_ and vm_["clone"] is True:
-        vmid = newid
-    else:
-        vmid = data["vmid"]  # vmid which we have received
+    vmid = data["vmid"]  # vmid which we have received
     host = data["node"]  # host which we have received
     nodeType = data["technology"]  # VM tech (Qemu / OpenVZ)
 
@@ -1007,6 +1023,7 @@ def create_node(vm_, newid):
         )
         for prop in _get_properties("/nodes/{node}/qemu", "POST", static_props):
             if prop in vm_:  # if the property is set, use it for the VM request
+                # If specified, vmid will override newid.
                 newnode[prop] = vm_[prop]
 
     # The node is ready. Lets request it to be added
@@ -1026,6 +1043,8 @@ def create_node(vm_, newid):
     if "clone" in vm_ and vm_["clone"] is True and vm_["technology"] == "qemu":
         postParams = {}
         postParams["newid"] = newnode["vmid"]
+        if "pool" in vm_:
+            postParams["pool"] = vm_["pool"]
 
         for prop in "description", "format", "full", "name":
             if (
@@ -1047,7 +1066,12 @@ def create_node(vm_, newid):
         )
     else:
         node = query("post", "nodes/{}/{}".format(vmhost, vm_["technology"]), newnode)
-    return _parse_proxmox_upid(node, vm_)
+    result = _parse_proxmox_upid(node, vm_)
+
+    # When cloning, the upid contains the clone_from vmid instead of the new vmid
+    result["vmid"] = newnode["vmid"]
+
+    return result
 
 
 def show_instance(name, call=None):