[proxmox] return vmid and taskid #7263

b- · 2023-09-15T02:42:35Z

SUMMARY

Proxmox uses an internal ID number (vmid) to refer to specific containers and virtual machines.

Ansible's community.general.proxmox module allows an LXC container to be created with an automatically-assigned vmid. However, presently (before this PR) it doesn't return the chosen ID. This means that the only way to programmatically delete a container that was programmatically created with an automatically-assigned ID involves string matching, string manipulation, and guesswork. (Some of the above logic already exists within the proxmox module, but I definitely don't trust the idea of deleting a VM based on an identifier that isn't even guaranteed unique!)

Additionally, long-lived tasks such as creating/starting/updating/deleting VMs and containers are given taskid numbers that show up in the audit log and can be used to reference the status of the task. The community.general.proxmox does use the taskid to keep track of dispatched tasks, but the module doesn't return it.

This PR adds vmid and taskid to the data returned by such actions, at least where it would make sense to do so. This way, for example, I can make a bunch of containers with automatically-assigned IDs and use Ansible's register function to reference them later in the play.

Being able to save the taskid may also help with other execution strategies, or for matching Ansible logs with Proxmox audit logs, or something. I'm not sure, really, but it seemed like a good idea to add it in while I was already there.

ISSUE TYPE

Feature Pull Request

COMPONENT NAME

proxmox

ADDITIONAL INFORMATION

This is my first contribution to Ansible, and probably my first time contributing Python code to any FOSS project! Please be gentle and be understanding if I missed anything.

if create succeeds, we want the vmid

ansibullbot · 2023-09-15T02:54:13Z

cc @Ajpantuso @Thulium-Drake @UnderGreen @joshainglis @karmab
click here for bot help

b- · 2023-09-15T03:11:00Z

Sorry for all the commits. I fixed the PEP8 errors that my code introduced.

b- · 2023-09-15T03:52:25Z

Sorry, somehow I accidentally reverted something and just fixed it

changelogs/fragments/7263-proxmox-return-vmid-and-taskid.yaml

Co-authored-by: Felix Fontein <[email protected]>

plugins/modules/proxmox.py

felixfontein · 2023-09-16T07:09:01Z

Please note that now another sanity test is failing.

b- · 2023-09-16T20:37:19Z

I should have been running the sanity tests locally (at least scoped to proxmox) this entire time instead of assuming that my editor's built-in linter was good enough... 😒

But anyway, I did just that -- opened a terminal pane in my editor and ran, until ansible-test sanity proxmox ; do inotifywait --event modify plugins/modules/proxmox.py ; done while I edited the file.

I'm pretty sure this should be the last commit for this PR, and again I appreciate your patience!

Okay, so maybe just waiting for CI to run the tests was a terrible idea. I installed `inotifywait`, set up a venv for the tests, and in my editor's terminal pane I ran the following, letting it run every save until it exited: until ansible-test sanity proxmox ; do inotifywait --event modify plugins/modules/proxmox.py done

felixfontein · 2023-09-17T11:30:19Z

At least the sanity tests seem to be happy ;-) If nobody objects, I'll merge this at the end of this upcoming week.

patchback · 2023-09-22T10:57:51Z

Backport to stable-7: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-7/28b3da88a9b05743811452f2fcb6271249dac8f7/pr-7263

Backported as #7307

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

* add vmid exit value if create succeeds, we want the vmid * fix syntax * add vmid to return codes * Add taskid to return, and only return vmid when it makes sense to * add changelog fragment with temporary filename * Add pr number to fragment * fix PEP8 E501: line too long * oops, I knew I still missed something... * Update 7263-proxmox-return-vmid-and-taskid.yaml Co-authored-by: Felix Fontein <[email protected]> * removed taskid from places it's not defined * finally fixed sanity test errors Okay, so maybe just waiting for CI to run the tests was a terrible idea. I installed `inotifywait`, set up a venv for the tests, and in my editor's terminal pane I ran the following, letting it run every save until it exited: until ansible-test sanity proxmox ; do inotifywait --event modify plugins/modules/proxmox.py done --------- Co-authored-by: Felix Fontein <[email protected]> (cherry picked from commit 28b3da8)

felixfontein · 2023-09-22T10:58:05Z

@b- thanks a lot for your contribution!

…id (#7307) [proxmox] return vmid and taskid (#7263) * add vmid exit value if create succeeds, we want the vmid * fix syntax * add vmid to return codes * Add taskid to return, and only return vmid when it makes sense to * add changelog fragment with temporary filename * Add pr number to fragment * fix PEP8 E501: line too long * oops, I knew I still missed something... * Update 7263-proxmox-return-vmid-and-taskid.yaml Co-authored-by: Felix Fontein <[email protected]> * removed taskid from places it's not defined * finally fixed sanity test errors Okay, so maybe just waiting for CI to run the tests was a terrible idea. I installed `inotifywait`, set up a venv for the tests, and in my editor's terminal pane I ran the following, letting it run every save until it exited: until ansible-test sanity proxmox ; do inotifywait --event modify plugins/modules/proxmox.py done --------- Co-authored-by: Felix Fontein <[email protected]> (cherry picked from commit 28b3da8) Co-authored-by: bri <[email protected]>

8.5.0 amazon.aws ~~~~~~~~~~ - ec2_ami - add support for ``org_arns`` and ``org_unit_arns`` in launch_permissions (ansible-collections/amazon.aws#1690). - elb_application_lb_info - drop redundant ``describe_load_balancers`` call fetching ``ip_address_type`` (ansible-collections/amazon.aws#1768). community.general ~~~~~~~~~~~~~~~~~ - cargo - add option ``executable``, which allows user to specify path to the cargo binary (ansible-collections/community.general#7352). - cargo - add option ``locked`` which allows user to specify install the locked version of dependency instead of latest compatible version (ansible-collections/community.general#6134). - dig lookup plugin - add TCP option to enable the use of TCP connection during DNS lookup (ansible-collections/community.general#7343). - gitlab_group - add option ``force_delete`` (default: false) which allows delete group even if projects exists in it (ansible-collections/community.general#7364). - ini_file - add ``ignore_spaces`` option (ansible-collections/community.general#7273). - newrelic_deployment - add option ``app_name_exact_match``, which filters results for the exact app_name provided (ansible-collections/community.general#7355). - onepassword lookup plugin - introduce ``account_id`` option which allows specifying which account to use (ansible-collections/community.general#7308). - onepassword_raw lookup plugin - introduce ``account_id`` option which allows specifying which account to use (ansible-collections/community.general#7308). - parted - on resize, use ``--fix`` option if available (ansible-collections/community.general#7304). - pnpm - set correct version when state is latest or version is not mentioned. Resolves previous idempotency problem (ansible-collections/community.general#7339). - proxmox - add ``vmid`` (and ``taskid`` when possible) to return values (ansible-collections/community.general#7263). - random_string - added new ``ignore_similar_chars`` and ``similar_chars`` option to ignore certain chars (ansible-collections/community.general#7242). - redfish_command - add new option ``update_oem_params`` for the ``MultipartHTTPPushUpdate`` command (ansible-collections/community.general#7331). - redfish_config - add ``CreateVolume`` command to allow creation of volumes on servers (ansible-collections/community.general#6813). - redfish_config - adding ``SetSecureBoot`` command (ansible-collections/community.general#7129). - redfish_info - add support for ``GetBiosRegistries`` command (ansible-collections/community.general#7144). - redfish_info - adds ``LinkStatus`` to NIC inventory (ansible-collections/community.general#7318). - redis_info - refactor the redis_info module to use the redis module_utils enabling to pass TLS parameters to the Redis client (ansible-collections/community.general#7267). - supervisorctl - allow to stop matching running processes before removing them with ``stop_before_removing=true`` (ansible-collections/community.general#7284). community.libvirt ~~~~~~~~~~~~~~~~~ - virt - add `mutate_flags` parameter to enable XML mutation (add UUID, MAC addresses from existing domain) (ansible-collections/community.libvirt#142). - virt - support ``--diff`` for ``define`` command (ansible-collections/community.libvirt#142). community.routeros ~~~~~~~~~~~~~~~~~~ - api_info - add new ``include_read_only`` option to select behavior for read-only values. By default these are not returned (ansible-collections/community.routeros#213). - api_info, api_modify - add support for ``address-list`` and ``match-subdomain`` introduced by RouterOS 7.7 in the ``ip dns static`` path (ansible-collections/community.routeros#197). - api_info, api_modify - add support for ``user``, ``time`` and ``gmt-offset`` under the ``system clock`` path (ansible-collections/community.routeros#210). - api_info, api_modify - add support for the ``interface ppp-client`` path (ansible-collections/community.routeros#199). - api_info, api_modify - add support for the ``interface wireless`` path (ansible-collections/community.routeros#195). - api_info, api_modify - add support for the ``iot modbus`` path (ansible-collections/community.routeros#205). - api_info, api_modify - add support for the ``ip dhcp-server option`` and ``ip dhcp-server option sets`` paths (ansible-collections/community.routeros#223). - api_info, api_modify - add support for the ``ip upnp interfaces``, ``tool graphing interface``, ``tool graphing resource`` paths (ansible-collections/community.routeros#227). - api_info, api_modify - add support for the ``ipv6 firewall nat`` path (ansible-collections/community.routeros#204). - api_info, api_modify - add support for the ``mode`` property in ``ip neighbor discovery-settings`` introduced in RouterOS 7.7 (ansible-collections/community.routeros#198). - api_info, api_modify - add support for the ``port remote-access`` path (ansible-collections/community.routeros#224). - api_info, api_modify - add support for the ``routing filter rule`` and ``routing filter select-rule`` paths (ansible-collections/community.routeros#200). - api_info, api_modify - add support for the ``routing table`` path in RouterOS 7 (ansible-collections/community.routeros#215). - api_info, api_modify - add support for the ``tool netwatch`` path in RouterOS 7 (ansible-collections/community.routeros#216). - api_info, api_modify - add support for the ``user settings`` path (ansible-collections/community.routeros#201). - api_info, api_modify - add support for the ``user`` path (ansible-collections/community.routeros#211). - api_info, api_modify - finalize fields for the ``interface wireless security-profiles`` path and enable it (ansible-collections/community.routeros#203). - api_info, api_modify - finalize fields for the ``ppp profile`` path and enable it (ansible-collections/community.routeros#217). - api_modify - add new ``handle_read_only`` and ``handle_write_only`` options to handle the module's behavior for read-only and write-only fields (ansible-collections/community.routeros#213). - api_modify, api_info - support API paths ``routing id``, ``routing bgp connection`` (ansible-collections/community.routeros#220). community.vmware ~~~~~~~~~~~~~~~~ - add moid property in the return value for the module(ansible-collections/community.vmware#1855). - add new snapshot_id option to the vmware_guest_snapshot module(ansible-collections/community.vmware#1847). dellemc.powerflex ~~~~~~~~~~~~~~~~~ - Added Ansible role to support installation and uninstallation of Gateway. - Added Ansible role to support installation and uninstallation of SDR. - Added Ansible role to support installation and uninstallation of Web UI. grafana.grafana ~~~~~~~~~~~~~~~ - Add check for Curl and failure step if Agent Version is not retrieved - Allow alert resource provisioning in Grafana Role - Bump cryptography from 39.0.2 to 41.0.3 - Bump cryptography from 41.0.3 to 41.0.4 - Bump semver from 5.7.1 to 5.7.2 - Bump word-wrap from 1.2.3 to 1.2.5 - Create local dashboard directory in check mode - Create missing notification directory in Grafana Role - Remove check_mode from create local directory task in Grafana Role - Remove dependency on local-fs.target from Grafana Agent role - Update CI Testing - Update Cloud Stack Module failures - Use 'ansible_system' env variable to detect os typ in Grafana Agent Role - hange grafana Agent Wal and Positions Directory in Grafana Agent Role ovirt.ovirt ~~~~~~~~~~~ - ovirt_vm - Add tpm_enabled (oVirt/ovirt-ansible-collection#722). - storage_error_resume_behaviour - Support VM storage error resume behaviour "auto_resume", "kill", "leave_paused". (oVirt/ovirt-ansible-collection#721) - vm_infra - Support boot disk renaming and resizing. (oVirt/ovirt-ansible-collection#705) purestorage.flashblade ~~~~~~~~~~~~~~~~~~~~~~ - purefb_bucket_replica - Added support for cascading replica links - purefb_info - New fields to display free space (remaining quota) for Accounts and Buckets. Space used by destroyed buckets is split out from virtual field to new destroyed_virtual field - purefb_info - Report encryption state in SMB client policy rules - purefb_info - Report more detailed space data from Purity//FB 4.3.0 - purefb_policy - Add deny effect for object store policy rules. Requires Purity//FB 4.3.0+ - purefb_policy - Added parameter to define object store policy description vultr.cloud ~~~~~~~~~~~ - inventory - Added VPC/VPC 2.0 support by adding ``internal_ip`` to the attributes (vultr/ansible-collection-vultr#86).

* add vmid exit value if create succeeds, we want the vmid * fix syntax * add vmid to return codes * Add taskid to return, and only return vmid when it makes sense to * add changelog fragment with temporary filename * Add pr number to fragment * fix PEP8 E501: line too long * oops, I knew I still missed something... * Update 7263-proxmox-return-vmid-and-taskid.yaml Co-authored-by: Felix Fontein <[email protected]> * removed taskid from places it's not defined * finally fixed sanity test errors Okay, so maybe just waiting for CI to run the tests was a terrible idea. I installed `inotifywait`, set up a venv for the tests, and in my editor's terminal pane I ran the following, letting it run every save until it exited: until ansible-test sanity proxmox ; do inotifywait --event modify plugins/modules/proxmox.py done --------- Co-authored-by: Felix Fontein <[email protected]>

b- and others added 6 commits September 14, 2023 21:12

add vmid exit value

db519e1

if create succeeds, we want the vmid

fix syntax

7028da2

add vmid to return codes

f559275

Add taskid to return, and only return vmid when it makes sense to

74e61e3

add changelog fragment with temporary filename

6f76b1e

Add pr number to fragment

86d32ef

This comment was marked as outdated.

Sign in to view

ansibullbot added ci_verified Push fixes to PR branch to re-run CI needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR and removed needs_ci This PR requires CI testing to be performed. Please close and re-open this PR to trigger CI labels Sep 15, 2023

fix PEP8 E501: line too long

24a1645

b- force-pushed the proxmox-return-vmid-and-taskid branch from 9cf2d92 to 24a1645 Compare September 15, 2023 03:10

ansibullbot removed the ci_verified Push fixes to PR branch to re-run CI label Sep 15, 2023

This comment was marked as outdated.

Sign in to view

ansibullbot added the ci_verified Push fixes to PR branch to re-run CI label Sep 15, 2023

oops, I knew I still missed something...

a1557aa

felixfontein added the check-before-release PR will be looked at again shortly before release and merged if possible. label Sep 15, 2023

felixfontein added the backport-7 label Sep 15, 2023

felixfontein reviewed Sep 15, 2023

View reviewed changes

changelogs/fragments/7263-proxmox-return-vmid-and-taskid.yaml Outdated Show resolved Hide resolved

Update 7263-proxmox-return-vmid-and-taskid.yaml

5ae879a

Co-authored-by: Felix Fontein <[email protected]>

felixfontein reviewed Sep 15, 2023

View reviewed changes

plugins/modules/proxmox.py Outdated Show resolved Hide resolved

removed taskid from places it's not defined

149992e

ansibullbot added ci_verified Push fixes to PR branch to re-run CI and removed ci_verified Push fixes to PR branch to re-run CI labels Sep 15, 2023

b- mentioned this pull request Sep 15, 2023

[proxmox] Refactor Proxmox LXC module #7272

Open

1 task

ansibullbot added the has_issue label Sep 16, 2023

ansibullbot removed ci_verified Push fixes to PR branch to re-run CI needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR labels Sep 16, 2023

felixfontein removed the check-before-release PR will be looked at again shortly before release and merged if possible. label Sep 22, 2023

felixfontein merged commit 28b3da8 into ansible-collections:main Sep 22, 2023
144 checks passed

patchback bot mentioned this pull request Sep 22, 2023

[PR #7263/28b3da88 backport][stable-7] [proxmox] return vmid and taskid #7307

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[proxmox] return vmid and taskid #7263

[proxmox] return vmid and taskid #7263

b- commented Sep 15, 2023

This comment was marked as outdated.

ansibullbot commented Sep 15, 2023

This comment was marked as outdated.

b- commented Sep 15, 2023

This comment was marked as outdated.

b- commented Sep 15, 2023

felixfontein commented Sep 16, 2023

b- commented Sep 16, 2023

felixfontein commented Sep 17, 2023

patchback bot commented Sep 22, 2023 •

edited

Loading

felixfontein commented Sep 22, 2023

[proxmox] return vmid and taskid #7263

[proxmox] return vmid and taskid #7263

Conversation

b- commented Sep 15, 2023

SUMMARY

ISSUE TYPE

COMPONENT NAME

ADDITIONAL INFORMATION

This comment was marked as outdated.

ansibullbot commented Sep 15, 2023

This comment was marked as outdated.

b- commented Sep 15, 2023

This comment was marked as outdated.

b- commented Sep 15, 2023

felixfontein commented Sep 16, 2023

b- commented Sep 16, 2023

felixfontein commented Sep 17, 2023

patchback bot commented Sep 22, 2023 • edited Loading

Backport to stable-7: 💚 backport PR created

felixfontein commented Sep 22, 2023

patchback bot commented Sep 22, 2023 •

edited

Loading