ansible-collections / community.general

Ansible Community General Collection
https://galaxy.ansible.com/ui/repo/published/community/general/
GNU General Public License v3.0
814 stars 1.49k forks source link

Proxmox LXC - Mounts configuration causes repeated creation of volumes #8407

Closed Lithimlin closed 2 months ago

Lithimlin commented 3 months ago

Summary

Currently, it seems the creation of mount points with the documented syntax is not idempotent but instead creates a new mount disk each time the update task is run.

In the documentation's example, the syntax is: <storage>:<size-in-GB>,mp=<mountpoint>

Issue Type

Bug Report

Component Name

proxmox

Ansible Version

$ ansible --version
ansible [core 2.16.7]
  config file = <my-repo>/ansible.cfg
  configured module search path = ['<my-home>/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = <my-repo>/.venv/lib/python3.12/site-packages/ansible
  ansible collection location = <my-home>/.ansible/collections:/usr/share/ansible/collections
  executable location = <my-repo>/.venv/bin/ansible
  python version = 3.12.3 (main, Apr 23 2024, 09:16:07) [GCC 13.2.1 20240417] (<my-repo>/.venv/bin/python)
  jinja version = 3.1.4
  libyaml = True

Community.general Version

$ ansible-galaxy collection list community.general
Collection        Version
----------------- -------
community.general 8.4.0

Configuration

$ ansible-config dump --only-changed
CONFIG_FILE() = <my-repo>/ansible.cfg
DEFAULT_ROLES_PATH(<my-repo>/ansible.cfg) = ['<my-repo>/roles']
DEFAULT_TIMEOUT(<my-repo>/ansible.cfg) = 30
EDITOR(env: EDITOR) = vim
PAGER(env: PAGER) = less

OS / Environment

proxmoxer==2.0.1
requests==2.32.2

Steps to Reproduce

- name: Create LXC
  community.general.proxmox:
    api_host: "{{ api_host }}"
    api_user: "{{ api_user }}"
    api_token_id: "{{ api_token_id }}"
    api_token_secret: "{{ api_token_secret }}"
    hostname: "{{ inventory_hostname }}"
    node: my-node
    ostemplate: images:vztmpl/debian-12-standard_12.2-1_amd64.tar.zst
    storage: machines01
    timeout: 30
  register: create_result

- name: Update LXC config
  community.general.proxmox:
    api_host: "{{ api_host }}"
    api_user: "{{ api_user }}"
    api_token_id: "{{ api_token_id }}"
    api_token_secret: "{{ api_token_secret }}"
    node: my-node
    vmid: "{{ create_result.vmid }}"
    cores: 2
    memory: 2048
    swap: 2048
    disk: machines01:vm-{{ create_result.vmid }}-disk-0,size=12G"
    mounts: 
      mp0: machines01:4,mp=/mnt/data
    update: true

Expected Results

The LXC is created and configured with a single mountpoint idempotently, even when the playbook is run again. More importantly, no extra volumes are created on subsequent runs unless specified.

Actual Results

The LXC is updated and a new mount volume is created. On a container restart, the previous volume is unmounted and the newly-created on is mounted. In the machines01 storage, new volumes vm-<id>-disk-<n> are created each time the playbook is run. In the LXC resources tab, the actually mounted disk is machines01:vm-<id>-disk-<n>,mp=/mnt/data,size=4G with a change to machines01:4,mp=/mnt/data is queued.

Code of Conduct

ansibullbot commented 3 months ago

Files identified in the description:

If these files are incorrect, please update the component name section of the description or use the !component bot command.

click here for bot help

ansibullbot commented 3 months ago

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

Lithimlin commented 3 months ago

Upon further investigation it seems that this is a problem with the Proxmox API: The syntax for creating and mounting a new volume is <storage>:<size-in-GB>,mp=<mountpoint>. However, to not overwrite this, the syntax <storage>:vm-<vmid>-disk-<index>,mp=<mountpoint>,size=<size>G[,...] would be required, otherwise a new volume is created each time.

I'd prefer to have a solution inside the proxmox module though rather than, e.g., check inside a playbook each time whether a volume already exists or it needs to be created.

Lithimlin commented 3 months ago

Looking at the module, it seems the best way to go about this may be to slightly rework the way mounts and disks are defined: Instead of taking the plain string, the module would take dicts to define each mount and the rootfs.

felixfontein commented 3 months ago

Sounds like a good idea to me - but you have to make this backwards compatible, so it's probably best to add a new option for that, mutually exclusive with the old one, and in case the old is specified parse the mount config and convert it to the new form.

(I have no idea how much work that would be though...)

Lithimlin commented 3 months ago

I'm not sure how long this would take either and I'm also not sure how much time I'll be able to allocate to this. I also have no clue yet what overhead goes into contributing to this. The biggest question I think is how much we'd want to future-proof the implementation against possible future options.

Lithimlin commented 3 months ago

Due to the existence of Bind Mount Points, things seem more complicated than I first expected.