Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[PR #9225/d71ba0fa backport][stable-10] Proxmox module refactoring #9634

Conversation

patchback[bot]
Copy link

@patchback patchback bot commented Jan 26, 2025

This is a backport of PR #9225 as merged into main (d71ba0f).

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:

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

* proxmox: Refactor

This is a squash of the following commits for easier rebasing:

proxmox module_utils: make use of choose_first_if_multiple in get_vm

proxmox: refactor module

proxmox: add changelog

proxmox: fix deprecation message

proxmox: remove type hints

proxmox: remove spaces for keywords

proxmox: run formatter

proxmox: make compabtible with old python versions

proxmox: remove f-strings

proxmox: fix string formatting in build_volume

proxmox: revert disk size parameter to simple integer

proxmox: update changelog fragment

proxmox: fix argument spec

proxmox: fix size handling in build_volume

proxmox: fix formatting

proxmox: update changelog fragment

* proxmox: Fix changelog fragment, doc, and deprecation string formatting.

* proxmox: Fix formatting in imports

* proxmox: require one of `vmid` or `hostname`, simplify checks

* proxmox: apply check for supported features to entire module

* proxmox: move parameter conversions inside create and update functions

(cherry picked from commit d71ba0f)
@patchback patchback bot mentioned this pull request Jan 26, 2025
@ansibullbot
Copy link
Collaborator

@ansibullbot ansibullbot added backport bug This issue/PR relates to a bug module module module_utils module_utils new_contributor Help guide this first time contributor plugins plugin (any type) labels Jan 26, 2025
@felixfontein felixfontein merged commit f79aa6f into stable-10 Jan 26, 2025
138 checks passed
@felixfontein felixfontein deleted the patchback/backports/stable-10/d71ba0fae80c4b47074c2472c96a086fa39f595a/pr-9225 branch January 26, 2025 15:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport bug This issue/PR relates to a bug module_utils module_utils module module new_contributor Help guide this first time contributor plugins plugin (any type)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants