ansible-collections / community.general

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

not possible to add lxc mount point as bind #8982

Open mrrudy opened 1 month ago

mrrudy commented 1 month ago

Summary

When I try to add a mount point that is only host source and guest destination ("/mnt/nas,mp=/mnt/nas") in the community.general.proxmox I get FAILD.

Issue Type

Bug Report

Component Name

proxmox

Ansible Version

ansible [core 2.14.16]
  config file = /opt/semphore/ansible.cfg
  configured module search path = ['/root/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/lib/python3/dist-packages/ansible
  ansible collection location = /root/.ansible/collections:/usr/share/ansible/collections
  executable location = /usr/bin/ansible
  python version = 3.11.2 (main, Aug 26 2024, 07:20:54) [GCC 12.2.0] (/usr/bin/python3)
  jinja version = 3.1.2
  libyaml = True

Community.general Version


# /usr/lib/python3/dist-packages/ansible_collections
Collection        Version
----------------- -------
community.general 6.6.2  

# /root/.ansible/collections/ansible_collections
Collection        Version
----------------- -------
community.general 9.4.0 

Configuration

CONFIG_FILE() = /opt/semphore/ansible.cfg
DEFAULT_CONNECTION_PLUGIN_PATH(/opt/semphore/ansible.cfg) = ['/opt/semphore/connection_plugin']
DEFAULT_REMOTE_USER(/opt/semphore/ansible.cfg) = semaphore
DEFAULT_VAULT_PASSWORD_FILE(/opt/semphore/ansible.cfg) = /root/.vault_pass.txt
HOST_KEY_CHECKING(/opt/semphore/ansible.cfg) = False
INTERPRETER_PYTHON(/opt/semphore/ansible.cfg) = auto_silent

OS / Environment

Debian 12

Steps to Reproduce

- hosts: group_nas_mount:&proxmox_all_lxc
  become: true
  gather_facts: true
  vars_files:
    - ../vault/secrets.yml
    - ../vars/pve_conf.yml

  tasks:
    - name: Add mountpoint
      community.general.proxmox:
        api_user: "{{ api_user  }}"
        api_token_id: "{{ api_token_id  }}"
        api_token_secret: "{{ api_token_secret }}"
        api_host: "{{ proxmox_node }}"
        vmid: "{{ ct_id }}"
        hostname: "{{ proxmox_hostname }}"
        node: "{{ proxmox_node }}"
        mounts:
          mp0: "/mnt/nas,mp=/mnt/nas" # this generates error
#          mp0: "volume=/mnt/nas,mp=/mnt/nas" # this generates error as well
#          mp0: "local-zfs:2,mp=/mnt/test/"   # this works fine
        update: true
      delegate_to: "{{ proxmox_node }}"

Expected Results

pct set 104 -mp0 /mnt/nas,mp=/mnt/nas

Actual Results

TASK [Add mountpoint] ******************************************************************************************************************************************************************
fatal: [tailscaletest -> nas-06]: FAILED! => {"changed": false, "msg": "Configuration of lxc VM 104 failed with exception: Internal error", "vmid": 104}

Code of Conduct

ansibullbot commented 1 month 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 1 month ago

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

mrrudy commented 1 month ago

I did some investigation and was able to notice that
parse_disk_string("/mnt/nas,mp=/mnt/nas") does ok and gives: {'mp': '/mnt/nas', 'host_path': '/mnt/nas'} but the build_volume throws the error as there is nor storage neither volume defined.

markusj commented 1 month ago

The proxmox module is currently in a completely unusable state:

I can not invest further time into tracking down the remaining bugs and will revert to an older working version. If anyone is interested, my fixes so far are in this changeset: https://github.com/markusj/community.general/commit/a7d378c787aaaedf9e7c5dda8166982a8e116504

felixfontein commented 1 month ago

The changes introduced in https://github.com/ansible-collections/community.general/pull/8720 break the logic of the modified method

I'm not sure this broke anything that wasn't broken before. Since vol_string was undefined in that code path, and is used a few lines later, it would simply have crashed with a different error message before. The problem was introduced in #8646, a follow-up to #8542.

@Lithimlin please take a look, since you created #8542 and #8646.

Lithimlin commented 3 weeks ago

I'll take a look ASAP. I was pretty sure I took care of all variants the Proxmox API allowed, but evidently, I must've overlooked some.

Lithimlin commented 3 weeks ago

If anyone is interested, my fixes so far are in this changeset: markusj@a7d378c

Thanks a lot! This will speed up things a lot. The right thing to do with the size is probably to allow for strings (32M, 0T) and not just assume GiB as a unit. This was a misinterpretation of the Proxmox API docs on my end. I also wasn't aware that the (old) storage parameter could cause such problems. I'll see how those can be fixed.

To be completely honest though, it might be easiest to just completely re-write the module at this point. As @markusj said, it's in a very unstable state, and trying to "improve" things as I have done can have unforeseen side-effects. I'll see what I can do in the coming days but unfortunately, I cannot promise I'll have time to look at the issue in-depth.