use proxmoxer and fix bugs

[I3urny]

What does this PR do?

• Change config structure by splitting token into token_name and token_value, and replacing url with host.
• Use the proxmoxer library to replace the self-built _query() function.
• Wait for tasks produced by VM creation (creating and cloning) to complete before continuing with the next step.
• Add a descriptive error message for when the technology parameter is missing in the VM profile.
• avail_images() now only shows actual images stored in proxmox (images, vztmpl and iso) instead of all datatypes.
• list_nodes_full() now returns all available data about VMs that can be gathered from proxmox.
• list_nodes() now uses list_nodes_full() and then reduces its output to only id, image, private_ips, public_ips, size and state.
• Add a retry logic in create() for starting the VM
• switch back from salt.cloud.utils.* to __utils__. reason being that e.g. salt.utils.cloud.filter_event() causes an error whereas __utils__["cloud.filter_event"]() does not

What issues does this PR fix or reference?

Fixes:

Previous Behavior

• used requests and the self-written _query() function to communicate with the proxmox instance.
• clone() and create() didn't wait for task completion which could result in errors in subsequent steps.
• creating a VM without specifying the technology parameter in a VM profile resulted in erroneous calls to the API.
• cloning a VM without specifying the vmid parameter resulted in erroneous calls to the API.
• avail_images() returned all data stored in proxmox's storage locations instead of only showing images.
• list_nodes_full() returned the data provided by the resource and config endpoint in a non-standard format.
• list_nodes() duplicated the calls to the resource and config endpoint done by list_nodes_full() and then returned the required format.
• create() sometimes could not start the newly created VM because proxmox takes a few seconds to list the VM via the API.

New Behavior

• use proxmoxer library for all proxmox communication.
• clone() and create() now also wait for task completion similar to start(), stop()orshutdown()`.
• creating a VM without specifying the technology parameter in a VM profile now results in a descriptive error.
• cloning a VM without specifying the vmid parameter now results in a descriptive error.
• avail_images() now only shows images stored in proxmox's storage locations.
• list_nodes_full() now returns all available data about the VMs in the format used by list_nodes() with the addition of the keys config and resource for the respective data.
• list_nodes() now uses list_nodes_full() to gather data and then only returns id, image, private_ips, public_ips, size and state.
• create() now implements a retry logic when trying to start the newly created VM to work around proxmox's delay.

Merge requirements satisfied?

[NOTICE] Bug fixes or features added to Salt require tests.

• Docs
• Changelog - https://docs.saltproject.io/en/master/topics/development/changelog.html
• Tests written/updated

Commits signed with GPG?

No

Please review Salt's Contributing Guide for best practices.

See GitHub's page on GPG signing for more information about
signing commits with GPG.

[lkubb]

Thanks a lot for your contribution and sorry it took so long for any reviewer to show up. I'm not overly familiar with the Proxmox modules, but they don't have a dedicated maintainer at the moment.

I remember diagnosing the __utils__ issue on Discord, which is unique to this extension. We should release another v1 v2 with the fixed old code before introducing your breaking changes in a v2 v3, so I took the liberty to patch it in a separate PR. Could you rebase on current main? There are some additional conflicts.

Additionally, I think this change needs more documentation:

• At least .. versionadded:: 3.0.0/.. versionchanged:: 3.0.0 on the module/function docstrings for the important changes (afaict dependency switch, required config and function return values)
• A changelog entry for each fixed issue and breaking change. If there are no submitted issues and you don't want to create each one, you can avoid having to specify an issue number by prefixing the changelog entry file name with a +.

Since it seems you are an active user of this extension and familiar with the code, you could consider becoming a maintainer for saltext-proxmox.

[lkubb]

note: Patching in the global context should be avoided. I know the current tests are written that way, but it would be nice if we could use this breaking change to improve the situation by defining fixtures:

@pytest.fixture
def wait_for_task_mock():
    with patch(_fqn(proxmox._wait_for_task), autospec=True) as wait:
        yield wait

# ...

@pytest.mark.usefixtures("wait_for_task_mock")
# ...

[lkubb]

nitpick: Afaik MagicMock() is redundant.

Suggested change

	with patch("salt.utils.cloud.fire_event", MagicMock()):
	with patch("salt.utils.cloud.fire_event"):

[lkubb]

Suggested change

	with pytest.raises(SaltCloudSystemExit), patch("salt.utils.cloud.fire_event", MagicMock()):
	with pytest.raises(SaltCloudSystemExit), patch("salt.utils.cloud.fire_event"):

[lkubb]

Suggested change

	with patch("salt.utils.cloud.fire_event", MagicMock()):
	with patch("salt.utils.cloud.fire_event"):

[lkubb]

Suggested change

	with patch("salt.utils.cloud.fire_event", MagicMock()):
	with patch("salt.utils.cloud.fire_event"):

[lkubb]

Suggested change

	with patch("salt.utils.cloud.fire_event", MagicMock()):
	with patch("salt.utils.cloud.fire_event"):