Proxmox module refactoring

[Lithimlin]

SUMMARY

This pull request refactors the historically grown proxmox module.
The control flow was very opaque with some redundant checks and - most recently - features that were only introduced in parts of the module when they intended to target the whole module (see #9065).
This refactor aims to make the module more readable and more easily extendable.

Additionally, this request contains various bug fixes:

• From proxmox: Handling of disk_volume variable is broken #9065: The disk_volume key is now properly parsed and passed on to the API
• From not possible to add lxc mount point as bind #8982: It is now possible to add bind-only mount points again.
• From proxmox: state=template is not idempotent (for LXCs) #8811: Converting an LXC to a template is now idempotent.
• From Update of LXC Containers does not work (Mounts/Memory/etc.) #9055: The default value of update=False is now deprecated and will be changed to update=True in version 11.0.0
• In the proxmox utils module: The parameter choose_first_if_multiple of get_vmid is now used correctly, causing the function to return the first of multiple matches instead of failing.

ISSUE TYPE

• Bugfix Pull Request
• Refactoring Pull Request

COMPONENT NAME

proxmox

ADDITIONAL INFORMATION

The following playbook should run correctly on both the old and refactored versions of this module, but the template creation will not be idempotent on the old version:

---
- name: Check proxmox LXC module
  hosts: pve
  become: false
  gather_facts: false
  tasks:
    - name: Create LXC
      community.general.proxmox:
        api_host: "{{ proxmox_api_host }}"
        api_user: "{{ proxmox_api_user }}"
        api_token_id: "{{ proxmox_api_token_id }}"
        api_token_secret: "{{ proxmox_api_token_secret }}"
        node: node01
        hostname: test-node
        ostemplate: local:vztmpl/debian-12-standard_12.2-1_amd64.tar.zst
        disk: local-lvm:8
        tags: test1,test3,debian
      register: create_res

    - name: Sleep for 5 seconds and continue with play
      ansible.builtin.wait_for:
        timeout: 5
      delegate_to: localhost

    - name: Check Creation Idempotency
      community.general.proxmox:
        api_host: "{{ proxmox_api_host }}"
        api_user: "{{ proxmox_api_user }}"
        api_token_id: "{{ proxmox_api_token_id }}"
        api_token_secret: "{{ proxmox_api_token_secret }}"
        node: node01
        hostname: test-node
        ostemplate: local:vztmpl/debian-12-standard_12.2-1_amd64.tar.zst
        disk: local-lvm:8
        tags: test1,test3,debian
      register: check_res
      failed_when: check_res.changed

    - name: Update LXC config [new - create volumes]
      community.general.proxmox:
        api_host: "{{ proxmox_api_host }}"
        api_user: "{{ proxmox_api_user }}"
        api_token_id: "{{ proxmox_api_token_id }}"
        api_token_secret: "{{ proxmox_api_token_secret }}"
        node: node01
        hostname: test-node
        cores: 1
        vmid: "{{ create_res.vmid | int }}"
        disk_volume:
          storage: local-lvm
          size: 14
        mount_volumes:
          - id: mp0
            storage: local-lvm
            size: 3
            mountpoint: /mnt/test
        update: true

    - name: Update LXC config [new - explicit volumes]
      community.general.proxmox:
        api_host: "{{ proxmox_api_host }}"
        api_user: "{{ proxmox_api_user }}"
        api_token_id: "{{ proxmox_api_token_id }}"
        api_token_secret: "{{ proxmox_api_token_secret }}"
        node: node01
        hostname: test-node
        cores: 1
        vmid: "{{ create_res.vmid }}"
        disk_volume:
          storage: local-lvm
          volume: "vm-{{ create_res.vmid }}-disk-0"
          size: 14
        mount_volumes:
          - id: mp0
            storage: local-lvm
            volume: "vm-{{ create_res.vmid }}-disk-1"
            size: 3
            mountpoint: /mnt/test
        update: true

    - name: Update LXC config [old - implicit volumes]
      community.general.proxmox:
        api_host: "{{ proxmox_api_host }}"
        api_user: "{{ proxmox_api_user }}"
        api_token_id: "{{ proxmox_api_token_id }}"
        api_token_secret: "{{ proxmox_api_token_secret }}"
        node: node01
        hostname: test-node
        cores: 1
        vmid: "{{ create_res.vmid }}"
        disk: local-lvm:14
        mounts: '{"mp0":"local-lvm:3,mp=/mnt/test"}'
        update: true

    - name: Update LXC config [old - explicit volumes]
      community.general.proxmox:
        api_host: "{{ proxmox_api_host }}"
        api_user: "{{ proxmox_api_user }}"
        api_token_id: "{{ proxmox_api_token_id }}"
        api_token_secret: "{{ proxmox_api_token_secret }}"
        node: node01
        hostname: test-node
        cores: 1
        vmid: "{{ create_res.vmid }}"
        disk: "local-lvm:vm-{{ create_res.vmid }}-disk-0,size=14G"
        mounts: '{"mp0":"local-lvm:vm-{{ create_res.vmid }}-disk-1,size=3G,mp=/mnt/test"}'
        update: true

    - name: Update LXC config [tags]
      community.general.proxmox:
        api_host: "{{ proxmox_api_host }}"
        api_user: "{{ proxmox_api_user }}"
        api_token_id: "{{ proxmox_api_token_id }}"
        api_token_secret: "{{ proxmox_api_token_secret }}"
        node: node01
        hostname: test-node
        cores: 1
        vmid: "{{ create_res.vmid }}"
        disk: "local-lvm:vm-{{ create_res.vmid }}-disk-0,size=14G"
        mounts: '{"mp0":"local-lvm:vm-{{ create_res.vmid }}-disk-1,size=3G,mp=/mnt/test"}'
        tags: test1,test2,debian
        update: true

    - name: Start LXC
      community.general.proxmox:
        api_host: "{{ proxmox_api_host }}"
        api_user: "{{ proxmox_api_user }}"
        api_token_id: "{{ proxmox_api_token_id }}"
        api_token_secret: "{{ proxmox_api_token_secret }}"
        vmid: "{{ create_res.vmid }}"
        state: started

    - name: Check Starting Idempotency
      community.general.proxmox:
        api_host: "{{ proxmox_api_host }}"
        api_user: "{{ proxmox_api_user }}"
        api_token_id: "{{ proxmox_api_token_id }}"
        api_token_secret: "{{ proxmox_api_token_secret }}"
        vmid: "{{ create_res.vmid }}"
        state: started
      retries: 1
      delay: 10
      register: check_res
      failed_when: check_res.changed

    - name: Restart LXC
      community.general.proxmox:
        api_host: "{{ proxmox_api_host }}"
        api_user: "{{ proxmox_api_user }}"
        api_token_id: "{{ proxmox_api_token_id }}"
        api_token_secret: "{{ proxmox_api_token_secret }}"
        vmid: "{{ create_res.vmid }}"
        state: restarted

    - name: Stop LXC
      community.general.proxmox:
        api_host: "{{ proxmox_api_host }}"
        api_user: "{{ proxmox_api_user }}"
        api_token_id: "{{ proxmox_api_token_id }}"
        api_token_secret: "{{ proxmox_api_token_secret }}"
        vmid: "{{ create_res.vmid }}"
        state: stopped

    - name: Check Stopping Idempotency
      community.general.proxmox:
        api_host: "{{ proxmox_api_host }}"
        api_user: "{{ proxmox_api_user }}"
        api_token_id: "{{ proxmox_api_token_id }}"
        api_token_secret: "{{ proxmox_api_token_secret }}"
        vmid: "{{ create_res.vmid }}"
        state: stopped
      retries: 1
      delay: 10
      register: check_res
      failed_when: check_res.changed

    - name: Convert LXC to Template
      community.general.proxmox:
        api_host: "{{ proxmox_api_host }}"
        api_user: "{{ proxmox_api_user }}"
        api_token_id: "{{ proxmox_api_token_id }}"
        api_token_secret: "{{ proxmox_api_token_secret }}"
        vmid: "{{ create_res.vmid }}"
        state: template

    - name: Check Template Idempotency
      community.general.proxmox:
        api_host: "{{ proxmox_api_host }}"
        api_user: "{{ proxmox_api_user }}"
        api_token_id: "{{ proxmox_api_token_id }}"
        api_token_secret: "{{ proxmox_api_token_secret }}"
        vmid: "{{ create_res.vmid }}"
        state: template
      retries: 1
      delay: 10
      register: check_res
      failed_when: check_res.changed

    - name: Remove LXC
      community.general.proxmox:
        api_host: "{{ proxmox_api_host }}"
        api_user: "{{ proxmox_api_user }}"
        api_token_id: "{{ proxmox_api_token_id }}"
        api_token_secret: "{{ proxmox_api_token_secret }}"
        vmid: "{{ create_res.vmid }}"
        state: absent

    - name: Check Removal Idempotency
      community.general.proxmox:
        api_host: "{{ proxmox_api_host }}"
        api_user: "{{ proxmox_api_user }}"
        api_token_id: "{{ proxmox_api_token_id }}"
        api_token_secret: "{{ proxmox_api_token_secret }}"
        vmid: "{{ create_res.vmid }}"
        state: absent
      retries: 1
      delay: 10
      register: check_res
      failed_when: check_res.changed

    - name: Create new LXC [With storage + disk] (should fail)
      community.general.proxmox:
        api_host: "{{ proxmox_api_host }}"
        api_user: "{{ proxmox_api_user }}"
        api_token_id: "{{ proxmox_api_token_id }}"
        api_token_secret: "{{ proxmox_api_token_secret }}"
        node: node01
        hostname: test-node-2
        ostemplate: local:vztmpl/debian-12-standard_12.2-1_amd64.tar.zst
        cores: 1
        disk: 6
        storage: local-lvm
      ignore_errors: true

[ansibullbot]

@Lithimlin this PR contains the following merge commits:

• 1b2476b

Please rebase your branch to remove these commits.

click here for bot help

[ansibullbot]

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

[russoz]

Hi @Lithimlin thanks for your contribution!

I would recommend breaking this PR into smaller pieces, as it seems to contain both bugfixes as well as new features. Additionally, we must not use type hints, as Ansible still provides support for Python 2.7 - it's quite a nuisance, but that should be the case until that support is dropped, I want to say end of next year but I am not sure right now.

[russoz]

Suggested change

	- proxmox - refactors the proxmox module
	- proxmox - refactors the proxmox module (https://github.com/ansible-collections/community.general/pull/9225).

[russoz]

This was not solved (missing the punctuation by the end of the line).

[russoz]

You might want to consider using ModuleHelper. See more in https://docs.ansible.com/ansible/latest/collections/community/general/docsite/guide_modulehelper.html

Particularly for this module, I would recommend using StateModuleHelper, as it is already tailored for modules with different behaviour for each state.

There is a number of modules already using these helpers, you can spot them using

git grep -E '\(\S*ModuleHelper\)'

[Lithimlin]

see this comment for a reply

[bcoca]

Ansible still provides support for Python 2.7 - it's quite a nuisance, but that should be the case until that support is dropped, I want to say end of next year but I am not sure right now.

The last 2 versions of core already dropped support for 2.7 and 2.16 will be EOL in May 2025
https://docs.ansible.com/ansible/latest/reference_appendices/release_and_maintenance.html#ansible-core-support-matrix

[russoz]

Ansible still provides support for Python 2.7 - it's quite a nuisance, but that should be the case until that support is dropped, I want to say end of next year but I am not sure right now.

The last 2 versions of core already dropped support for 2.7 and 2.16 will be EOL in May 2025 docs.ansible.com/ansible/latest/reference_appendices/release_and_maintenance.html#ansible-core-support-matrix

Yes, but community.general still supports ansible-core 2.15. Felix mentioned the deadline more than once but it will be late next year - far enough into the future to be forgotten. Until then, in the collection, we must support Python 2.7.

[russoz]

Hi, I had approved it before, but will mark it "Request changes" to prevent other maintainers from accidentally merging it as-is now. Also, on second thought, this is quite a big revamp, so it would benefit from a more thorough examination before we merge it. I approved it before (and I should not, because the changelog needed fixing) and, days later, I cannot recall what I have seen here or not. I'm writing this to manage the expectations - this PR might take a tad longer to be merged - but please bear with us.

[Lithimlin]

this is quite a big revamp, so it would benefit from a more thorough examination before we merge it.

Yes, that would be great. While I have tested quite a few cases, I can't test all corner cases that might occur in my current setup. Having more people take a look at this and run some tests would be great.

I will also take a look at the [State]ModuleHelper, but I find the approach of "documentation by example" a bit hard to work with.

[russoz]

this is quite a big revamp, so it would benefit from a more thorough examination before we merge it.

Yes, that would be great. While I have tested quite a few cases, I can't test all corner cases that might occur in my current setup. Having more people take a look at this and run some tests would be great.

Sweet. Thanks for your patience.

I will also take a look at the [State]ModuleHelper, but I find the approach of "documentation by example" a bit hard to work with.

There is proper documentation at: https://docs.ansible.com/ansible/latest/collections/community/general/docsite/guide_modulehelper.html

(unless that's what you are referring to, in which case I would gladly take suggestions on how to improve those docs)

[felixfontein]

Ansible still provides support for Python 2.7 - it's quite a nuisance, but that should be the case until that support is dropped, I want to say end of next year but I am not sure right now.

The last 2 versions of core already dropped support for 2.7 and 2.16 will be EOL in May 2025 docs.ansible.com/ansible/latest/reference_appendices/release_and_maintenance.html#ansible-core-support-matrix

Yes, but community.general still supports ansible-core 2.15. Felix mentioned the deadline more than once but it will be late next year - far enough into the future to be forgotten. Until then, in the collection, we must support Python 2.7.

We drop support for one ansible-core version per major release (unless we forget to do it, like for 9.0.0, and then we dropped two in 10.0.0 ;) ). We support all ansible-core versions that have not been EOL for more than a short time when we release a new major version. Since ansible-core 2.16 becomes EOL in May 2025, around when we have a new major release (11.0.0), we'll only drop support for it in the next major release (12.0.0, in ~November 2025). Only from that point on Python 2.7 is no longer supported by community.general.

[Lithimlin]

I've looked at a re-design using the StateModuleHelper, and while that would be very convenient indeed, I think it would require a rework of the module_utils.proxmox if I understood correctly since ProxmoxAnsible uses the base AnsibleModule in its self.module quite a lot. This, in turn, would require a re-design of all other proxmox_* modules, which is probably a good idea but for which I just do not have the time and resources.

I would like to change the implementation to one using the StateModuleHelper and take the first step towards refactoring all the modules, but with the additional overhead from above, it is simply not feasible for me to do so right now.