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

Update of LXC Containers does not work (Mounts/Memory/etc.) #9055

Open
1 task done
Exzellius opened this issue Oct 23, 2024 · 16 comments · May be fixed by #9073
Open
1 task done

Update of LXC Containers does not work (Mounts/Memory/etc.) #9055

Exzellius opened this issue Oct 23, 2024 · 16 comments · May be fixed by #9073
Labels
bug This issue/PR relates to a bug has_pr module module plugins plugin (any type)

Comments

@Exzellius
Copy link

Summary

When trying ot update a running LXC on proxmox, I get return for ansible and nothing about the configuration is changed, even tho I clearly indicate so in the playbook.

Issue Type

Bug Report

Component Name

community.general.proxmox

Ansible Version

$ ansible --version
ansible:/ansible # ansible --version
ansible [core 2.16.5]
  config file = /etc/ansible/ansible.cfg
  configured module search path = ['/root/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/lib/python3.11/site-packages/ansible
  ansible collection location = /root/.ansible/collections:/usr/share/ansible/collections
  executable location = /usr/bin/ansible
  python version = 3.11.10 (main, Sep 18 2024, 22:14:32) [GCC] (/usr/bin/python3.11)
  jinja version = 3.1.2
  libyaml = True
ansible:/ansible #

Community.general Version

$ ansible-galaxy collection list community.general
ansible:/ansible # ansible-galaxy collection list community.general

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

# /usr/lib/python3.11/site-packages/ansible_collections
Collection        Version
----------------- -------
community.general 8.5.0
ansible:/ansible #

Configuration

$ ansible-config dump --only-changed
ansible:/ansible # ansible-config dump --only-changed
CONFIG_FILE() = /etc/ansible/ansible.cfg
DEFAULT_HOST_LIST(/etc/ansible/ansible.cfg) = ['/ansible/inventory.yaml']
DEFAULT_LOG_PATH(/etc/ansible/ansible.cfg) = /ansible/ansible.log
DEFAULT_VAULT_PASSWORD_FILE(/etc/ansible/ansible.cfg) = /ansible/vault_passwd.txt
INTERPRETER_PYTHON(/etc/ansible/ansible.cfg) = auto_legacy_silent
PAGER(env: PAGER) = less
ansible:/ansible #

OS / Environment

OpenSuse Leap 15.6

Steps to Reproduce

- name: Proxmox Create Container
  hosts: nextcloud
  gather_facts: false
  connection: local

  module_defaults:
    group/community.general.proxmox:
      api_host: pve01.exzellius.local
      api_user: ansible@pve
      api_token_id: ansible_token
      api_token_secret: "{{ pve_api_token_password }}"
      validate_certs: false

  tasks:
    - name: Update Container-Configs
      community.general.proxmox:
        state: started
        update: true
        cores: 4
        memory: 4096
        swap: 512
        mounts:
          mp0: "lvm:155,mp=/nextcloud_data,backup=1"
        vmid: 204

Expected Results

New config parameters should be applied and the status of the task should be changed.

Actual Results

ansible:/ansible # ansible-playbook pve_test.yaml -vvvv
ansible-playbook [core 2.16.5]
  config file = /etc/ansible/ansible.cfg
  configured module search path = ['/root/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/lib/python3.11/site-packages/ansible
  ansible collection location = /root/.ansible/collections:/usr/share/ansible/collections
  executable location = /usr/bin/ansible-playbook
  python version = 3.11.10 (main, Sep 18 2024, 22:14:32) [GCC] (/usr/bin/python3.11)
  jinja version = 3.1.2
  libyaml = True
Using /etc/ansible/ansible.cfg as config file
setting up inventory plugins
Loading collection ansible.builtin from
host_list declined parsing /ansible/inventory.yaml as it did not pass its verify_file() method
script declined parsing /ansible/inventory.yaml as it did not pass its verify_file() method
Parsed /ansible/inventory.yaml inventory source with yaml plugin
Loading collection community.general from /root/.ansible/collections/ansible_collections/community/general
Loading callback plugin default of type stdout, v2.0 from /usr/lib/python3.11/site-packages/ansible/plugins/callback/default.py
Skipping callback 'default', as we already have a stdout callback.
Skipping callback 'minimal', as we already have a stdout callback.
Skipping callback 'oneline', as we already have a stdout callback.

PLAYBOOK: pve_test.yaml **************************************************************************************************************************************
Positional arguments: pve_test.yaml
verbosity: 4
connection: ssh
become_method: sudo
tags: ('all',)
inventory: ('/ansible/inventory.yaml',)
forks: 5
1 plays in pve_test.yaml

PLAY [Proxmox Create Container] ******************************************************************************************************************************
Trying secret FileVaultSecret(filename='/ansible/vault_passwd.txt') for vault_id=default

TASK [Update Container-Configs] ******************************************************************************************************************************
task path: /ansible/pve_test.yaml:15
<{{inventory_hostname}}.{{ dns_domain }}> ESTABLISH LOCAL CONNECTION FOR USER: root
<{{inventory_hostname}}.{{ dns_domain }}> EXEC /bin/sh -c 'echo ~root && sleep 0'
<{{inventory_hostname}}.{{ dns_domain }}> EXEC /bin/sh -c '( umask 77 && mkdir -p "` echo /root/.ansible/tmp `"&& mkdir "` echo /root/.ansible/tmp/ansible-tmp-1729713251.1884532-845233-140897991318956 `" && echo ansible-tmp-1729713251.1884532-845233-140897991318956="` echo /root/.ansible/tmp/ansible-tmp-1729713251.1884532-845233-140897991318956 `" ) && sleep 0'
<nextcloud> Attempting python interpreter discovery
<{{inventory_hostname}}.{{ dns_domain }}> EXEC /bin/sh -c 'echo PLATFORM; uname; echo FOUND; command -v '"'"'python3.12'"'"'; command -v '"'"'python3.11'"'"'; command -v '"'"'python3.10'"'"'; command -v '"'"'python3.9'"'"'; command -v '"'"'python3.8'"'"'; command -v '"'"'python3.7'"'"'; command -v '"'"'python3.6'"'"'; command -v '"'"'/usr/bin/python3'"'"'; command -v '"'"'/usr/libexec/platform-python'"'"'; command -v '"'"'python2.7'"'"'; command -v '"'"'/usr/bin/python'"'"'; command -v '"'"'python'"'"'; echo ENDFOUND && sleep 0'
<{{inventory_hostname}}.{{ dns_domain }}> EXEC /bin/sh -c '/usr/bin/python3.11 && sleep 0'
<nextcloud> Python interpreter discovery fallback (unsupported Linux distribution: opensuse-leap)
Using module file /root/.ansible/collections/ansible_collections/community/general/plugins/modules/proxmox.py
<{{inventory_hostname}}.{{ dns_domain }}> PUT /root/.ansible/tmp/ansible-local-845230tufr97yi/tmpomylde6y TO /root/.ansible/tmp/ansible-tmp-1729713251.1884532-845233-140897991318956/AnsiballZ_proxmox.py
<{{inventory_hostname}}.{{ dns_domain }}> EXEC /bin/sh -c 'chmod u+x /root/.ansible/tmp/ansible-tmp-1729713251.1884532-845233-140897991318956/ /root/.ansible/tmp/ansible-tmp-1729713251.1884532-845233-140897991318956/AnsiballZ_proxmox.py && sleep 0'
<{{inventory_hostname}}.{{ dns_domain }}> EXEC /bin/sh -c '/usr/bin/python3.11 /root/.ansible/tmp/ansible-tmp-1729713251.1884532-845233-140897991318956/AnsiballZ_proxmox.py && sleep 0'
<{{inventory_hostname}}.{{ dns_domain }}> EXEC /bin/sh -c 'rm -f -r /root/.ansible/tmp/ansible-tmp-1729713251.1884532-845233-140897991318956/ > /dev/null 2>&1 && sleep 0'
ok: [nextcloud] => {
    "ansible_facts": {
        "discovered_interpreter_python": "/usr/bin/python3.11"
    },
    "changed": false,
    "invocation": {
        "module_args": {
            "api_host": "pve01.exzellius.local",
            "api_password": null,
            "api_port": null,
            "api_token_id": "ansible_token",
            "api_token_secret": "VALUE_SPECIFIED_IN_NO_LOG_PARAMETER",
            "api_user": "ansible@pve",
            "clone": null,
            "clone_type": "opportunistic",
            "cores": 4,
            "cpus": null,
            "cpuunits": null,
            "description": null,
            "disk": null,
            "disk_volume": null,
            "features": null,
            "force": false,
            "hookscript": null,
            "hostname": null,
            "ip_address": null,
            "memory": 4096,
            "mount_volumes": null,
            "mounts": {
                "mp0": "lvm:155,mp=/nextcloud_data,backup=1"
            },
            "nameserver": null,
            "netif": null,
            "node": null,
            "onboot": null,
            "ostemplate": null,
            "ostype": "auto",
            "password": null,
            "pool": null,
            "pubkey": null,
            "purge": false,
            "searchdomain": null,
            "startup": null,
            "state": "started",
            "storage": "local",
            "swap": 512,
            "tags": null,
            "timeout": 30,
            "timezone": null,
            "unprivileged": true,
            "update": true,
            "validate_certs": false,
            "vmid": 204
        }
    },
    "msg": "VM 204 is already running",
    "vmid": 204
}

PLAY RECAP ***************************************************************************************************************************************************
nextcloud                  : ok=1    changed=0    unreachable=0    failed=0    skipped=0    rescued=0    ignored=0

ansible:/ansible #

Code of Conduct

  • I agree to follow the Ansible Code of Conduct
@ansibullbot
Copy link
Collaborator

Files identified in the description:

  • plugins/modules/proxmox

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
Copy link
Collaborator

@ansibullbot ansibullbot added bug This issue/PR relates to a bug module module plugins plugin (any type) labels Oct 23, 2024
@Exzellius
Copy link
Author

Well seems like the "update" parameter only works for the state "present". Could you please update the documentation to reflect that?

@felixfontein
Copy link
Collaborator

Currently noone seems to actively maintain the proxmox modules. You could create a PR to update the documentation, if it looks good I'll merge it.

@Exzellius Exzellius linked a pull request Oct 29, 2024 that will close this issue
@Lithimlin
Copy link
Contributor

I'm currently working a bit on the module.
Why does it even have the update parameter? Shouldn't state=present be enough of an indicator? It seems to go against Ansible's usual philosophy.

Couldn't we deprecate it and remove it in future versions such that state=present is enough to keep a container updated? If necessary, we could add a parameter to indicate that no changes should be made if the container is present, but in such cases it might be better to use the proxmox_info module beforehand.

@felixfontein
Copy link
Collaborator

Why does it even have the update parameter? Shouldn't state=present be enough of an indicator? It seems to go against Ansible's usual philosophy.

I don't remember how the update parameter came to be, but I guess it's one of two possibilities:

  1. Updates are destructive, so let's let the user opt-in.
  2. Originally the module didn't update, and to avoid adding a breaking change, the update parameter was introduced.

@Lithimlin
Copy link
Contributor

Looking at the code, it seems to be the latter. Apparently, before PVE4.0 you couldn't update a container config (via the API?).

What about deprecating it?

@felixfontein
Copy link
Collaborator

I don't think it makes sense to deprecate that option. The behavior of update: false might be useful for some users, so I don't see why that should be removed.

What could be done is deprecating the current default (which is false) and changing it to true eventually, so that by default the module will update, but you can still explicitly disable that if you wish.

@Lithimlin
Copy link
Contributor

What could be done is deprecating the current default (which is false) and changing it to true eventually, so that by default the module will update, but you can still explicitly disable that if you wish.

That sounds like a good idea. How does one deprecate the default value of a variable though?

@felixfontein
Copy link
Collaborator

The general procedure is:

  1. Remove the default;
  2. Document the current default value in description, mention that it is deprecated, and will change to the new default in community.general x.0.0.
  3. Adjust the code to check whether the parameter is None; if it is, set it to the old value, and call module.deprecate() to inform the user that the current default is deprecated and will change. It makes sense to hide the module.deprecate() call behind some ifs that check whether the value is actually used by the module and has any effect, so that the deprecation is only shown to users for which it makes a difference. (For example if state=absent, it doens't make sense to show the deprecation for update's default since there it doesn't make a difference.)

This informs users what will happen and allows them to silence the user by explicitly setting the parameter to what they want it to be. So if your task should not update, explicitly set update=false to keep the old behavior, and if you notice that you'd rather want it to update, explicitly set update=true (and test whether it works as expected in your playbook/role).

Then in community.general x.0.0, we'll set the new value as default and remove the module.deprecate() call and its conditions.

@russoz
Copy link
Collaborator

russoz commented Dec 6, 2024

@Lithimlin I wrote this doc primarily for myself: https://github.com/russoz-ansible/ansible-contrib-unofficial/blob/main/deprecations.md to remember how to do deprecations in different situations. Felix has already explained this case but it might be good to keep the reference. ;-)

@felixfontein
Copy link
Collaborator

@russoz have you thought on adding this to the official Ansible documentation? I think it would be useful there.

@russoz
Copy link
Collaborator

russoz commented Dec 12, 2024

Err... nope. But now it seems not only a good idea, but an obvious one. :-) Will add this to my queue.

@samccann
Copy link
Contributor

How about at the end of https://docs.ansible.com/ansible/latest/dev_guide/module_lifecycle.html ?

@Exzellius Exzellius closed this as not planned Won't fix, can't repro, duplicate, stale Dec 28, 2024
@felixfontein
Copy link
Collaborator

I don't think this is resolved yet.

@felixfontein felixfontein reopened this Dec 28, 2024
@Exzellius Exzellius closed this as not planned Won't fix, can't repro, duplicate, stale Dec 28, 2024
@felixfontein
Copy link
Collaborator

@Exzellius please stop closing this issue. Thanks.

@felixfontein felixfontein reopened this Dec 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue/PR relates to a bug has_pr module module plugins plugin (any type)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants