add template option to proxmox and proxmox_kvm #7143

etrombly · 2023-08-21T20:52:49Z

SUMMARY

Adds an option for converting LXC containers and KVM VMs to templates with the proxmox and proxmox_kvm modules.

ISSUE TYPE

Feature Pull Request

COMPONENT NAME

proxmox and proxmox_kvm

ADDITIONAL INFORMATION

ansibullbot · 2023-08-21T20:53:06Z

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

felixfontein · 2023-08-22T04:42:53Z

Thanks for your contribution. As a first comment: "template" does not sounds like a state name, but an operation. I'm not sure whether doing this with state is a good idea. Maybe it would make sense to have a new module (or two) for this operation? In any case, I'm interested in the module maintainers's opinions on this.

etrombly · 2023-08-22T10:46:21Z

My thought process was that the state of the VM would be a template, I wasn't thinking of the action of converting it to a template. I could see it going either way. The vcenter module uses an is_template parameter. So I'd be open to switching it to something like that, but then the state parameter would be useless for the task, since the VM needs to be stopped to be a template. Not sure if I should just ignore the state parameter if I use is_template.

felixfontein · 2023-08-22T19:25:40Z

I'm not sure what it means that a VM is a template. Is the VM no longer running or stopped, but in a completely new, different state? (I'm not using proxmox so I really don't know.) That's what state=template implies for me. Or does it mean that you create a template out of the current VM (and thus create a new object of another type - template - that is based on this VM, and the VM keeps being a VM)? That's why I hope one of the maintainers with proxmox knowledge will write something :)

etrombly · 2023-08-22T20:27:56Z

Yes, basically. When you convert a VM to a template it is in a new state. You can't start or stop it, just clone it.

felixfontein · 2023-08-23T20:53:20Z

So the existing VM is terminated and is now only available as a template?

etrombly · 2023-08-23T20:59:18Z

Yes, if you use force or the VM is already stopped. If it was running the task will fail.

felixfontein · 2023-08-29T05:14:34Z

I guess in that case adding it as a state makes sense. I don't think the design of the API is great, but that's nothing we can change :)

felixfontein

Sorry, I didn't got around to look at this again until now. (And unfortunately nobody else commented on it either :( )

plugins/modules/proxmox.py

felixfontein · 2023-10-08T10:06:48Z

plugins/modules/proxmox.py

+            if proxmox.convert_to_template(vm, vmid, timeout, force=module.params['force']):
+                module.exit_json(changed=True, msg="VM %s is converted to template" % vmid)


Since the function can only return True (if it doesn't raise an exception), I would get rid of the if since it implies to the reader that it is also possible that it returns False:

Suggested change

if proxmox.convert_to_template(vm, vmid, timeout, force=module.params['force']):

module.exit_json(changed=True, msg="VM %s is converted to template" % vmid)

proxmox.convert_to_template(vm, vmid, timeout, force=module.params['force'])

module.exit_json(changed=True, msg="VM %s is converted to template" % vmid)

plugins/modules/proxmox_kvm.py

etrombly · 2023-10-25T20:06:39Z

made the requested changes, fixed some lints, and rebased. Sorry for the delay on getting back to you.

felixfontein

Please also add a changelog fragment. Thanks!

plugins/modules/proxmox.py

plugins/modules/proxmox_kvm.py

etrombly · 2023-11-01T14:18:34Z

resolved the new comments and added a changelog fragment

changelogs/fragments/7143-proxmox-template.yml

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

felixfontein

If nobody objects, I'll merge this the upcoming weekend.

patchback · 2023-11-05T14:57:20Z

Backport to stable-8: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-8/07a47c047bc41e173b2218229c76ec121dbc26ca/pr-7143

Backported as #7488

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

* add template option to proxmox and proxmox_kvm * make recommended updates * fix tests * resolve comments on PR * save changes to changelog fragment * Update changelogs/fragments/7143-proxmox-template.yml Co-authored-by: Felix Fontein <[email protected]> --------- Co-authored-by: Eric Trombly <[email protected]> Co-authored-by: Felix Fontein <[email protected]> (cherry picked from commit 07a47c0)

felixfontein · 2023-11-05T14:57:33Z

@etrombly thanks for your contribution!

… and proxmox_kvm (#7488) add template option to proxmox and proxmox_kvm (#7143) * add template option to proxmox and proxmox_kvm * make recommended updates * fix tests * resolve comments on PR * save changes to changelog fragment * Update changelogs/fragments/7143-proxmox-template.yml Co-authored-by: Felix Fontein <[email protected]> --------- Co-authored-by: Eric Trombly <[email protected]> Co-authored-by: Felix Fontein <[email protected]> (cherry picked from commit 07a47c0) Co-authored-by: Eric Trombly <[email protected]>

v9.1.0 ansible.utils ~~~~~~~~~~~~~ - Fact_diff filter plugin - Add fact_diff filter plugin. (ansible-collections/ansible.utils#78). ansible.windows ~~~~~~~~~~~~~~~ - Set minimum supported Ansible version to 2.14 to align with the versions still supported by Ansible. - win_share - Added a new param called ``scope_name`` that allows file shares to be scoped for Windows Server failover cluster roles. cisco.ios ~~~~~~~~~ - Added ios_evpn_evi resource module. - Added ios_evpn_global resource module. - Added ios_vxlan_vtep resource module. - Fixed ios_evpn_evi resource module integration test failure - code to remove VLAN config. - ios_bgp_address_family - Fixed an issue with inherit peer-policy CLI - ios_bgp_address_family - added 'advertise' key - ios_vlans - added vlan config CLI feature. - ios_vrf - added MDT related keys cisco.ise ~~~~~~~~~ - Services included configuration, edda, dataconnect_services, subscriber. cisco.nxos ~~~~~~~~~~ - nxos_config - Relax restrictions on I(src) parameter so it can be used more like I(lines). (ansible-collections/cisco.nxos#89). community.general ~~~~~~~~~~~~~~~~~ - bitwarden lookup plugin - when looking for items using an item ID, the item is now accessed directly with ``bw get item`` instead of searching through all items. This doubles the lookup speed (ansible-collections/community.general#7468). - elastic callback plugin - close elastic client to not leak resources (ansible-collections/community.general#7517). - git_config - allow multiple git configs for the same name with the new ``add_mode`` option (ansible-collections/community.general#7260). - git_config - the ``after`` and ``before`` fields in the ``diff`` of the return value can be a list instead of a string in case more configs with the same key are affected (ansible-collections/community.general#7260). - git_config - when a value is unset, all configs with the same key are unset (ansible-collections/community.general#7260). - gitlab modules - add ``ca_path`` option (ansible-collections/community.general#7472). - gitlab modules - remove duplicate ``gitlab`` package check (ansible-collections/community.general#7486). - gitlab_runner - add support for new runner creation workflow (ansible-collections/community.general#7199). - ipa_config - adds ``passkey`` choice to ``ipauserauthtype`` parameter's choices (ansible-collections/community.general#7588). - ipa_sudorule - adds options to include denied commands or command groups (ansible-collections/community.general#7415). - ipa_user - adds ``idp`` and ``passkey`` choice to ``ipauserauthtype`` parameter's choices (ansible-collections/community.general#7589). - irc - add ``validate_certs`` option, and rename ``use_ssl`` to ``use_tls``, while keeping ``use_ssl`` as an alias. The default value for ``validate_certs`` is ``false`` for backwards compatibility. We recommend to every user of this module to explicitly set ``use_tls=true`` and `validate_certs=true`` whenever possible, especially when communicating to IRC servers over the internet (ansible-collections/community.general#7550). - keycloak module utils - expose error message from Keycloak server for HTTP errors in some specific situations (ansible-collections/community.general#7645). - keycloak_user_federation - add option for ``krbPrincipalAttribute`` (ansible-collections/community.general#7538). - lvol - change ``pvs`` argument type to list of strings (ansible-collections/community.general#7676, ansible-collections/community.general#7504). - lxd connection plugin - tighten the detection logic for lxd ``Instance not found`` errors, to avoid false detection on unrelated errors such as ``/usr/bin/python3: not found`` (ansible-collections/community.general#7521). - netcup_dns - adds support for record types ``OPENPGPKEY``, ``SMIMEA``, and ``SSHFP`` (ansible-collections/community.general#7489). - nmcli - add support for new connection type ``loopback`` (ansible-collections/community.general#6572). - nmcli - allow for ``infiniband`` slaves of ``bond`` interface types (ansible-collections/community.general#7569). - nmcli - allow for the setting of ``MTU`` for ``infiniband`` and ``bond`` interface types (ansible-collections/community.general#7499). - onepassword lookup plugin - support 1Password Connect with the opv2 client by setting the connect_host and connect_token parameters (ansible-collections/community.general#7116). - onepassword_raw lookup plugin - support 1Password Connect with the opv2 client by setting the connect_host and connect_token parameters (ansible-collections/community.general#7116) - passwordstore - adds ``timestamp`` and ``preserve`` parameters to modify the stored password format (ansible-collections/community.general#7426). - proxmox - adds ``template`` value to the ``state`` parameter, allowing conversion of container to a template (ansible-collections/community.general#7143). - proxmox - adds ``update`` parameter, allowing update of an already existing containers configuration (ansible-collections/community.general#7540). - proxmox inventory plugin - adds an option to exclude nodes from the dynamic inventory generation. The new setting is optional, not using this option will behave as usual (ansible-collections/community.general#6714, ansible-collections/community.general#7461). - proxmox_disk - add ability to manipulate CD-ROM drive (ansible-collections/community.general#7495). - proxmox_kvm - adds ``template`` value to the ``state`` parameter, allowing conversion of a VM to a template (ansible-collections/community.general#7143). - proxmox_kvm - support the ``hookscript`` parameter (ansible-collections/community.general#7600). - proxmox_ostype - it is now possible to specify the ``ostype`` when creating an LXC container (ansible-collections/community.general#7462). - proxmox_vm_info - add ability to retrieve configuration info (ansible-collections/community.general#7485). - redfish_info - adding the ``BootProgress`` property when getting ``Systems`` info (ansible-collections/community.general#7626). - ssh_config - adds ``controlmaster``, ``controlpath`` and ``controlpersist`` parameters (ansible-collections/community.general#7456). community.routeros ~~~~~~~~~~~~~~~~~~ - api_info, api_modify - add missing DoH parameters ``doh-max-concurrent-queries``, ``doh-max-server-connections``, and ``doh-timeout`` to the ``ip dns`` path (ansible-collections/community.routeros#230, ansible-collections/community.routeros#235) - api_info, api_modify - add missing parameters ``address-list``, ``address-list-timeout``, ``randomise-ports``, and ``realm`` to subpaths of the ``ip firewall`` path (ansible-collections/community.routeros#236, ansible-collections/community.routeros#237). - api_info, api_modify - mark the ``interface wireless`` parameter ``running`` as read-only (ansible-collections/community.routeros#233). - api_info, api_modify - set the default value to ``false`` for the ``disabled`` parameter in some more paths where it can be seen in the documentation (ansible-collections/community.routeros#237). - api_modify - add missing ``comment`` attribute to ``/routing id`` (ansible-collections/community.routeros#234). - api_modify - add missing attributes to the ``routing bgp connection`` path (ansible-collections/community.routeros#234). - api_modify - add versioning to the ``/tool e-mail`` path (RouterOS 7.12 release) (ansible-collections/community.routeros#234). - api_modify - make ``/ip traffic-flow target`` a multiple value attribute (ansible-collections/community.routeros#234). community.windows ~~~~~~~~~~~~~~~~~ - Set minimum supported Ansible version to 2.14 to align with the versions still supported by Ansible. community.zabbix ~~~~~~~~~~~~~~~~ - Added zabbix_group_events_info module - action module - Added notify_if_canceled property - agent and proxy roles - Set default `zabbix_api_server_port` to 80 or 443 based on `zabbix_api_use_ssl` - agent role - Removed duplicative Windows agent task - agent role - Standardized default yum priority to 99 - all roles - Re-added ability to override Debian repo source - all roles - Updated Debian repository format to 822 standard - various - updated testing modules - various - updated to fully qualified module names - zabbix agent - Added capability to add additional configuration includes - zabbix_api_info module added - zabbix_user module - add current_passwd optional parameter to enable password updating of the currently logged in user (https://www.zabbix.com/documentation/6.4/en/manual/api/reference/user/update) dellemc.powerflex ~~~~~~~~~~~~~~~~~ - Added support for PowerFlex Denver version(4.5.x) to TB and Config role. f5networks.f5_modules ~~~~~~~~~~~~~~~~~~~~~ - bigiq_device_discovery - Changes in documentation related to Provider block google.cloud ~~~~~~~~~~~~ - anisble-test - integration tests are now run against 2.14.0 and 2.15.0 - ansible - 2.14.0 is now the minimum version supported - ansible-lint - fixed over a thousand reported errors - ansible-lint - upgraded to 6.22 - ansible-test - add support for GCP application default credentials (ansible-collections/google.cloud#359). - gcp_serviceusage_service - added backoff when checking for operation completion. - gcp_serviceusage_service - use alloyb API for the integration test as spanner conflicts with other tests - gcp_sql_ssl_cert - made sha1_fingerprint optional, which enables resource creation - gcp_storage_default_object_acl - removed non-existent fields; the resource is not usable. hetzner.hcloud ~~~~~~~~~~~~~~ - Add the `hetzner.hcloud.all` group to configure all the modules using `module_defaults`. - Allow to set the `api_endpoint` module argument using the `HCLOUD_ENDPOINT` environment variable. - Removed the `hcloud_` prefix from all modules names, e.g. `hetzner.hcloud.hcloud_firewall` was renamed to `hetzner.hcloud.firewall`. Old module names will continue working. - Renamed the `endpoint` module argument to `api_endpoint`, backward compatibility is maintained using an alias. - hcloud inventory - Add the `api_endpoint` option. - hcloud inventory - Deprecate the `api_token_env` option, suggest using a lookup plugin (`{{ lookup('ansible.builtin.env', 'YOUR_ENV_VAR') }}`) or use the well-known `HCLOUD_TOKEN` environment variable name. - hcloud inventory - Rename the `token_env` option to `api_token_env`, use aliases for backward compatibility. - hcloud inventory - Rename the `token` option to `api_token`, use aliases for backward compatibility. inspur.ispim ~~~~~~~~~~~~ - Modify edit_smtp_com and add description information. microsoft.ad ~~~~~~~~~~~~ - Make ``name`` an optional parameter for the AD modules. Either ``name`` or ``identity`` needs to be set with their respective behaviours. If creating a new AD user and only ``identity`` is set, that will be the value used for the name of the object. - Set minimum supported Ansible version to 2.14 to align with the versions still supported by Ansible. - object_info - Add ActiveDirectory module import purestorage.flasharray ~~~~~~~~~~~~~~~~~~~~~~ - purefa_dns - Added facility to add a CA certifcate to management DNS and check peer. - purefa_info - Add NSID value for NVMe namespace in `hosts` response - purefa_info - Subset `pgroups` now also provides a new dict called `deleted_pgroups` - purefa_offload - Remove `nfs` as an option when Purity//FA 6.6.0 or higher is detected - purefa_snap - Add support for suffix on remote offload snapshots telekom_mms.icinga_director ~~~~~~~~~~~~~~~~~~~~~~~~~~~ - Extended docs and examples for multiple assign_filter conditions (telekom-mms/ansible-collection-icinga-director#227) theforeman.foreman ~~~~~~~~~~~~~~~~~~ - content_view_publish role - allow passing ``async`` and ``poll`` to the module (theforeman/foreman-ansible-modules#1676) - convert2rhel role - install ``convert2rhel`` from ``cdn-public.redhat.com``, dropping the requirement of a custom CA cert

This comment was marked as outdated.

Sign in to view

ansibullbot added ci_verified Push fixes to PR branch to re-run CI and removed needs_ci This PR requires CI testing to be performed. Please close and re-open this PR to trigger CI labels Aug 21, 2023

felixfontein added check-before-release PR will be looked at again shortly before release and merged if possible. backport-7 labels Aug 22, 2023

ansibullbot added the stale_ci CI is older than 7 days, rerun before merging label Sep 6, 2023

ansibullbot added the needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html label Sep 30, 2023

felixfontein reviewed Oct 8, 2023

View reviewed changes

felixfontein removed the backport-7 label Oct 9, 2023

etrombly force-pushed the proxmox_template branch from 8f4053c to cf987c3 Compare October 25, 2023 19:55

ansibullbot added needs_ci This PR requires CI testing to be performed. Please close and re-open this PR to trigger CI and removed ci_verified Push fixes to PR branch to re-run CI stale_ci CI is older than 7 days, rerun before merging labels Oct 25, 2023

Eric Trombly added 3 commits October 25, 2023 15:00

add template option to proxmox and proxmox_kvm

072faed

make recommended updates

f839d7f

fix tests

9e32e6a

etrombly force-pushed the proxmox_template branch from cf987c3 to 9e32e6a Compare October 25, 2023 20:04

felixfontein reviewed Oct 27, 2023

View reviewed changes

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

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

felixfontein added the backport-8 Automatically create a backport for the stable-8 branch label Nov 1, 2023

felixfontein reviewed Nov 1, 2023

View reviewed changes

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

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

resolve comments on PR

8261bcd

This comment was marked as outdated.

Sign in to view

ansibullbot added the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR label Nov 1, 2023

save changes to changelog fragment

ee86379

ansibullbot removed the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR label Nov 1, 2023

felixfontein reviewed Nov 1, 2023

View reviewed changes

changelogs/fragments/7143-proxmox-template.yml Outdated Show resolved Hide resolved

Update changelogs/fragments/7143-proxmox-template.yml

0b89d14

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

felixfontein approved these changes Nov 1, 2023

View reviewed changes

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

felixfontein merged commit 07a47c0 into ansible-collections:main Nov 5, 2023
118 checks passed

patchback bot mentioned this pull request Nov 5, 2023

[PR #7143/07a47c04 backport][stable-8] add template option to proxmox and proxmox_kvm #7488

Merged

etrombly deleted the proxmox_template branch November 10, 2023 12:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add template option to proxmox and proxmox_kvm #7143

add template option to proxmox and proxmox_kvm #7143

etrombly commented Aug 21, 2023

ansibullbot commented Aug 21, 2023

This comment was marked as outdated.

felixfontein commented Aug 22, 2023

etrombly commented Aug 22, 2023

felixfontein commented Aug 22, 2023

etrombly commented Aug 22, 2023

felixfontein commented Aug 23, 2023

etrombly commented Aug 23, 2023

felixfontein commented Aug 29, 2023

felixfontein left a comment

felixfontein Oct 8, 2023

etrombly commented Oct 25, 2023

felixfontein left a comment

etrombly commented Nov 1, 2023

This comment was marked as outdated.

felixfontein left a comment

patchback bot commented Nov 5, 2023 •

edited

Loading

felixfontein commented Nov 5, 2023

		if proxmox.convert_to_template(vm, vmid, timeout, force=module.params['force']):
		module.exit_json(changed=True, msg="VM %s is converted to template" % vmid)

add template option to proxmox and proxmox_kvm #7143

add template option to proxmox and proxmox_kvm #7143

Conversation

etrombly commented Aug 21, 2023

SUMMARY

ISSUE TYPE

COMPONENT NAME

ADDITIONAL INFORMATION

ansibullbot commented Aug 21, 2023

This comment was marked as outdated.

felixfontein commented Aug 22, 2023

etrombly commented Aug 22, 2023

felixfontein commented Aug 22, 2023

etrombly commented Aug 22, 2023

felixfontein commented Aug 23, 2023

etrombly commented Aug 23, 2023

felixfontein commented Aug 29, 2023

felixfontein left a comment

Choose a reason for hiding this comment

felixfontein Oct 8, 2023

Choose a reason for hiding this comment

etrombly commented Oct 25, 2023

felixfontein left a comment

Choose a reason for hiding this comment

etrombly commented Nov 1, 2023

This comment was marked as outdated.

felixfontein left a comment

Choose a reason for hiding this comment

patchback bot commented Nov 5, 2023 • edited Loading

Backport to stable-8: 💚 backport PR created

felixfontein commented Nov 5, 2023

patchback bot commented Nov 5, 2023 •

edited

Loading