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

btrfs_submodule examples use nonexistent `device` parameter #7098

Open MarcinWieczorek opened 1 year ago

MarcinWieczorek commented 1 year ago

Summary

The documentation for btrfs_submodule lists multiple examples like:

- name: Create a @home subvolume under the root subvolume
  community.general.btrfs_subvolume:
    name: /@home
    device: /dev/vda2

Unfortunately the device parameter does not exist. There only is filesystem_device and it is unclear if it is the same parameter. α-conversion does not work in my case, but possibly for other reasons. Before task listed below I only create the filesystem with community.general.filesystem, it's not mounted. It is unclear what the prerequisites are. root_device=/dev/vda2.

  - name: Setup default btrfs volume
    community.general.btrfs_subvolume:
      name: "/@"
      filesystem_device: "{{ root_device }}"
      default: yes
    when: fstype == "btrfs"

Result:

An exception occurred during task execution. To see the full traceback, use -vvv. The error was: AttributeError: 'NoneType' object has no attribute 'get_subvolume_by_name'
fatal: [libvirt_wyse]: FAILED! => {
    "changed": false,
    "rc": 1
}

MSG:

MODULE FAILURE
See stdout/stderr for the exact error

MODULE_STDOUT:

Traceback (most recent call last):
  File "/root/.local/share/ansible/tmp/ansible-tmp-1691827052.2975812-441057-66821381645042/AnsiballZ_btrfs_subvolume.py", line 107, in <module>
    _ansiballz_main()
  File "/root/.local/share/ansible/tmp/ansible-tmp-1691827052.2975812-441057-66821381645042/AnsiballZ_btrfs_subvolume.py", line 99, in _ansiballz_main
    invoke_module(zipped_mod, temp_path, ANSIBALLZ_PARAMS)
  File "/root/.local/share/ansible/tmp/ansible-tmp-1691827052.2975812-441057-66821381645042/AnsiballZ_btrfs_subvolume.py", line 47, in invoke_module
    runpy.run_module(mod_name='ansible_collections.community.general.plugins.modules.btrfs_subvolume', init_globals=dict(_module_fqn='ansible_collections.community.general.plugins.modules.btrfs_subvolume', _modlib_path=modlib_path),
  File "<frozen runpy>", line 226, in run_module
  File "<frozen runpy>", line 98, in _run_module_code
  File "<frozen runpy>", line 88, in _run_code
  File "/tmp/ansible_community.general.btrfs_subvolume_payload_2odjhzh6/ansible_community.general.btrfs_subvolume_payload.zip/ansible_collections/community/general/plugins/modules/btrfs_subvolume.py", line 682, in <module>
  File "/tmp/ansible_community.general.btrfs_subvolume_payload_2odjhzh6/ansible_community.general.btrfs_subvolume_payload.zip/ansible_collections/community/general/plugins/modules/btrfs_subvolume.py", line 678, in main
  File "/tmp/ansible_community.general.btrfs_subvolume_payload_2odjhzh6/ansible_community.general.btrfs_subvolume_payload.zip/ansible_collections/community/general/plugins/modules/btrfs_subvolume.py", line 670, in run_module
  File "/tmp/ansible_community.general.btrfs_subvolume_payload_2odjhzh6/ansible_community.general.btrfs_subvolume_payload.zip/ansible_collections/community/general/plugins/modules/btrfs_subvolume.py", line 278, in run
  File "/tmp/ansible_community.general.btrfs_subvolume_payload_2odjhzh6/ansible_community.general.btrfs_subvolume_payload.zip/ansible_collections/community/general/plugins/modules/btrfs_subvolume.py", line 596, in get_results
AttributeError: 'NoneType' object has no attribute 'get_subvolume_by_name'

Issue Type

Documentation Report

Component Name

btrfs_subvolume

Ansible Version

$ ansible --version
ansible [core 2.15.1]
  config file = /home/user/.config/ansible/ansible.cfg
  configured module search path = ['/home/user/.local/share/ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/lib/python3.11/site-packages/ansible
  ansible collection location = /home/user/.local/share/ansible/collections:/usr/share/ansible/collections
  executable location = /usr/bin/ansible
  python version = 3.11.3 (main, Jun  5 2023, 09:32:32) [GCC 13.1.1 20230429] (/usr/bin/python)
  jinja version = 3.1.2
  libyaml = True

Community.general Version

$ ansible-galaxy collection list community.general

# /usr/lib/python3.11/site-packages/ansible_collections
Collection        Version
----------------- -------
community.general 7.1.0

Configuration

$ ansible-config dump --only-changed
COLLECTIONS_PATHS(/home/user/.config/ansible/ansible.cfg) = ['/home/user/.local/share/ansible/collections', '/usr/share/ansible/collections']
CONFIG_FILE() = /home/user/.config/ansible/ansible.cfg
DEFAULT_HOST_LIST(/home/user/.config/ansible/ansible.cfg) = ['/home/user/.config/ansible/hosts']
DEFAULT_LOCAL_TMP(/home/user/.config/ansible/ansible.cfg) = /home/user/.local/share/ansible/tmp/ansible-local-447497k47791oj
DEFAULT_MODULE_PATH(/home/user/.config/ansible/ansible.cfg) = ['/home/user/.local/share/ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
DEFAULT_MODULE_UTILS_PATH(/home/user/.config/ansible/ansible.cfg) = ['/home/user/.local/share/ansible/plugins/module_utils', '/usr/share/ansible/plugins/module_utils']
DEFAULT_ROLES_PATH(/home/user/.config/ansible/ansible.cfg) = ['/home/user/.local/share/ansible/roles', '/usr/share/ansible/roles', '/etc/ansible/roles']
DEFAULT_STDOUT_CALLBACK(/home/user/.config/ansible/ansible.cfg) = debug
EDITOR(env: EDITOR) = vim
HOST_KEY_CHECKING(/home/user/.config/ansible/ansible.cfg) = False
PAGER(env: PAGER) = less
RETRY_FILES_SAVE_PATH(/home/user/.config/ansible/ansible.cfg) = /home/user/.local/share/ansible/ansible-retry

OS / Environment

Host is ArchLinux Linux hostname 6.4.1-arch2-1 #1 SMP PREEMPT_DYNAMIC Tue, 04 Jul 2023 08:39:40 +0000 x86_64 GNU/Linux So is the guest, but archiso (arch installer)

Additional Information

No response

Code of Conduct

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

cc @gnfzdz click here for bot help

MarcinWieczorek commented 1 year ago

I have tracked down the error. Turns out, there is an exception about btrfs not being mounted. The issue is that the very thing that caused that error is used in get_results and obviously is None. A simple if (see below) allows us to see the error. I don't know if returning an empty dict is valid, anyways I'll be happy to create a PR with a patch.

        if error:
            return (error, {})
        return (error, self.get_results())
{
  "failed": true,
  "msg": "Target filesystem uuid=97a970f2-3769-4d79-9d76-6da34abdf839 is not currently mounted and automount=False.Mount explicitly before module execution or pass automount=True",
  "invocation": {
    "module_args": {
      "name": "/@",
      "default": true,
      "automount": false,
      "recursive": false,
      "state": "present",
      "snapshot_conflict": "skip",
      "filesystem_device": null,
      "filesystem_label": null,
      "filesystem_uuid": null,
      "snapshot_source": null
    }
  }
}

Note, that the error is not related to the docs issue I originally posted this about.

MarcinWieczorek commented 1 year ago

Another detail: examples should use true instead of yes.

grishy commented 1 year ago

I encountered an issue while using:

- name: btrbk | Create a subvolume for snapshots
  community.general.btrfs_subvolume:
    name: /btrbk_snapshots

I think it should work as sudo btrfs subvolume create /btrbk_snapshots

MarcinWieczorek commented 1 year ago

And what do you observe instead?

grishy commented 1 year ago

I think similar issue

  File \"<stdin>\", line 107, in <module>
  File \"<stdin>\", line 99, in _ansiballz_main
  File \"<stdin>\", line 47, in invoke_module
  File \"<frozen runpy>\", line 226, in run_module
  File \"<frozen runpy>\", line 98, in _run_module_code
  File \"<frozen runpy>\", line 88, in _run_code
  File \"/tmp/ansible_community.general.btrfs_subvolume_payload_susfsgs6/ansible_community.general.btrfs_subvolume_payload.zip/ansible_collections/community/general/plugins/modules/btrfs_subvolume.py\", line 682, in <module>
  File \"/tmp/ansible_community.general.btrfs_subvolume_payload_susfsgs6/ansible_community.general.btrfs_subvolume_payload.zip/ansible_collections/community/general/plugins/modules/btrfs_subvolume.py\", line 678, in main
  File \"/tmp/ansible_community.general.btrfs_subvolume_payload_susfsgs6/ansible_community.general.btrfs_subvolume_payload.zip/ansible_collections/community/general/plugins/modules/btrfs_subvolume.py\", line 670, in run_module
  File \"/tmp/ansible_community.general.btrfs_subvolume_payload_susfsgs6/ansible_community.general.btrfs_subvolume_payload.zip/ansible_collections/community/general/plugins/modules/btrfs_subvolume.py\", line 278, in run
  File \"/tmp/ansible_community.general.btrfs_subvolume_payload_susfsgs6/ansible_community.general.btrfs_subvolume_payload.zip/ansible_collections/community/general/plugins/modules/btrfs_subvolume.py\", line 596, in get_results
AttributeError: 'NoneType' object has no attribute 'get_subvolume_by_name'
debug3: mux_client_read_packet: read header failed: Broken pipe

Just changed to this for now


- name: btrbk | Create a subvolume for snapshots
  become: true
  ansible.builtin.shell: btrfs subvolume create /btrbk_snapshots
  register: btrfs_subvolume_create
  changed_when: "btrfs_subvolume_create.rc == 0"
  failed_when: "btrfs_subvolume_create.rc != 0 and 'target path already exists' not in btrfs_subvolume_create.stderr"
MarcinWieczorek commented 1 year ago

Try this https://docs.ansible.com/ansible/latest/dev_guide/debugging.html#id3 and apply my patch, you'll see what the real exception is.

mks-h commented 1 year ago

I also get the same issue as the OP.

Just changed to this for now

Thanks for the workaround, I'll use it instead.

MarcinWieczorek commented 1 year ago

As I said I'll be happy to fix all issues if my questions get answered.

mrmeowski commented 12 months ago

I'm also running into this issue. Using filesystem_device causes the error mentioned in this comment https://github.com/ansible-collections/community.general/issues/7098#issue-1847849559: AttributeError: 'NoneType' object has no attribute 'get_subvolume_by_name'.

Usingfilesystem_uuid instead of filesystem_device seems to work, but it is not ideal..

hparfr commented 11 months ago

Without the patch:

    "msg": "MODULE FAILURE\nSee stdout/stderr for the exact error",
    "rc": 1

I have applied the patch from MarcinWieczorek

        if error:
            return (error, {})
        return (error, self.get_results())

Here is my output:

FAILED! => {
    "changed": false,
    "invocation": {
        "module_args": {
            "automount": true,
            "default": false,
            "filesystem_device": "/dev/sdb1",
            "filesystem_label": null,
            "filesystem_uuid": null,
            "name": "/builds",
            "recursive": false,
            "snapshot_conflict": "skip",
            "snapshot_source": null,
            "state": "present"
        }
    },
    "msg": "Found 0 filesystems matching criteria uuid=None label=None device=/dev/sdb1"
}

So it's better than without this patch.

When I run on the server python3 AnsiballZ_btrfs_subvolume.py execute I got nothing in error, so the patch only work when executed on localhost instead of on the server.

#playbook

- name: Create partition
  community.general.parted:
    device: "/dev/sdb"
    state: present
    fs_type: "btrfs"
    number: "1"
    part_type: "primary"
  become: true

- name: "Create btrfs fs"
  community.general.filesystem:
    fstype: btrfs
    dev: "/dev/sdb1"
  become: true

- name: "Create builds subvolume"
  community.general.btrfs_subvolume:
    name: "/builds"
    filesystem_device: "/dev/sdb1"
    automount: true
    state: "present"
hparfr commented 11 months ago

Ok I get it ! I forgot the become:

The issue is

btrfs filesystem show && echo $?
ERROR: cannot open /dev/sdb: Permission denied
0

with a retcode=0 it's not interpreted as an error.

This kind of patch should do the trick:

    def filesystem_show(self):
        command = "%s filesystem show -d" % (self.__btrfs)
        result = self.__module.run_command(command, check_rc=False)
        stdout = [x.strip() for x in result[1].splitlines()]
        filesystems = []
        current = None
+        if len(result[2]) > 0:
+            raise BtrfsModuleException("%s" % result[2])
        for line in stdout:
            if line.startswith('Label'):
                current = self.__parse_filesystem(line)
                filesystems.append(current)
            elif line.startswith('devid'):
                current['devices'].append(self.__parse_filesystem_device(line))
        return filesystems

(btrfs.py)

MarcinWieczorek commented 11 months ago

I wanted to recommend patching btrfs filesystem show to return correct exit code, but it seems that they're still using tabs and goto...

mks-h commented 11 months ago

they're still using tabs and goto...

Dude, are you trying to start a war here or what

MarcinWieczorek commented 11 months ago

No, I literally wanted to patch it, but adding hparfr's code will just be easier.

jarppiko commented 4 months ago

Hello,

I am running into an odd ansible_community.general.btrfs_subvolume bug that gives the same error:

AttributeError: 'NoneType' object has no attribute 'get_subvolume_by_name'

but maybe be a different bug (if so, I can make a new ticket).

Managed node OS: Ubuntu 22.04 LTS amd64

The very same playbook works fine on Ubuntu 23.10 managed node. The Controller node is the same 22.04 LTS on both the cases.

Playbook

- name: Debug btrfs_subvolume
  hosts: myhosts
  become: true
  tasks:
    - name: Create Btrfs subvolumes
      community.general.btrfs_subvolume:
        name: /root/debug-btrfs

Error message

The full traceback is:
Traceback (most recent call last):
  File "/root/.ansible/tmp/ansible-tmp-1715524349.8152192-715066-236550211353982/AnsiballZ_btrfs_subvolume.py", line 107, in <module>
    _ansiballz_main()
  File "/root/.ansible/tmp/ansible-tmp-1715524349.8152192-715066-236550211353982/AnsiballZ_btrfs_subvolume.py", line 99, in _ansiballz_main
    invoke_module(zipped_mod, temp_path, ANSIBALLZ_PARAMS)
  File "/root/.ansible/tmp/ansible-tmp-1715524349.8152192-715066-236550211353982/AnsiballZ_btrfs_subvolume.py", line 47, in invoke_module
    runpy.run_module(mod_name='ansible_collections.community.general.plugins.modules.btrfs_subvolume', init_globals=dict(_module_fqn='ansible_collections.community.general.plugins.modules.btrfs_subvolume', _modlib_path=modlib_path),
  File "/usr/lib/python3.10/runpy.py", line 224, in run_module
    return _run_module_code(code, init_globals, run_name, mod_spec)
  File "/usr/lib/python3.10/runpy.py", line 96, in _run_module_code
    _run_code(code, mod_globals, init_globals,
  File "/usr/lib/python3.10/runpy.py", line 86, in _run_code
    exec(code, run_globals)
  File "/tmp/ansible_community.general.btrfs_subvolume_payload_sf16lue1/ansible_community.general.btrfs_subvolume_payload.zip/ansible_collections/community/general/plugins/modules/btrfs_subvolume.py", line 682, in <module>
  File "/tmp/ansible_community.general.btrfs_subvolume_payload_sf16lue1/ansible_community.general.btrfs_subvolume_payload.zip/ansible_collections/community/general/plugins/modules/btrfs_subvolume.py", line 678, in main
  File "/tmp/ansible_community.general.btrfs_subvolume_payload_sf16lue1/ansible_community.general.btrfs_subvolume_payload.zip/ansible_collections/community/general/plugins/modules/btrfs_subvolume.py", line 670, in run_module
  File "/tmp/ansible_community.general.btrfs_subvolume_payload_sf16lue1/ansible_community.general.btrfs_subvolume_payload.zip/ansible_collections/community/general/plugins/modules/btrfs_subvolume.py", line 278, in run
  File "/tmp/ansible_community.general.btrfs_subvolume_payload_sf16lue1/ansible_community.general.btrfs_subvolume_payload.zip/ansible_collections/community/general/plugins/modules/btrfs_subvolume.py", line 596, in get_results
AttributeError: 'NoneType' object has no attribute 'get_subvolume_by_name'
fatal: [computer1]: FAILED! => {
    "changed": false,
    "module_stderr": "Shared connection to computer1.example.com closed.\r\n",
    "module_stdout": "Traceback (most recent call last):\r\n  File \"/root/.ansible/tmp/ansible-tmp-1715524349.8152192-715066-236550211353982/AnsiballZ_btrfs_subvolume.py\", line 107, in <module>\r\n    _ansiballz_main()\r\n  File \"/root/.ansible/tmp/ansible-tmp-1715524349.8152192-715066-236550211353982/AnsiballZ_btrfs_subvolume.py\", line 99, in _ansiballz_main\r\n    invoke_module(zipped_mod, temp_path, ANSIBALLZ_PARAMS)\r\n  File \"/root/.ansible/tmp/ansible-tmp-1715524349.8152192-715066-236550211353982/AnsiballZ_btrfs_subvolume.py\", line 47, in invoke_module\r\n    runpy.run_module(mod_name='ansible_collections.community.general.plugins.modules.btrfs_subvolume', init_globals=dict(_module_fqn='ansible_collections.community.general.plugins.modules.btrfs_subvolume', _modlib_path=modlib_path),\r\n  File \"/usr/lib/python3.10/runpy.py\", line 224, in run_module\r\n    return _run_module_code(code, init_globals, run_name, mod_spec)\r\n  File \"/usr/lib/python3.10/runpy.py\", line 96, in _run_module_code\r\n    _run_code(code, mod_globals, init_globals,\r\n  File \"/usr/lib/python3.10/runpy.py\", line 86, in _run_code\r\n    exec(code, run_globals)\r\n  File \"/tmp/ansible_community.general.btrfs_subvolume_payload_sf16lue1/ansible_community.general.btrfs_subvolume_payload.zip/ansible_collections/community/general/plugins/modules/btrfs_subvolume.py\", line 682, in <module>\r\n  File \"/tmp/ansible_community.general.btrfs_subvolume_payload_sf16lue1/ansible_community.general.btrfs_subvolume_payload.zip/ansible_collections/community/general/plugins/modules/btrfs_subvolume.py\", line 678, in main\r\n  File \"/tmp/ansible_community.general.btrfs_subvolume_payload_sf16lue1/ansible_community.general.btrfs_subvolume_payload.zip/ansible_collections/community/general/plugins/modules/btrfs_subvolume.py\", line 670, in run_module\r\n  File \"/tmp/ansible_community.general.btrfs_subvolume_payload_sf16lue1/ansible_community.general.btrfs_subvolume_payload.zip/ansible_collections/community/general/plugins/modules/btrfs_subvolume.py\", line 278, in run\r\n  File \"/tmp/ansible_community.general.btrfs_subvolume_payload_sf16lue1/ansible_community.general.btrfs_subvolume_payload.zip/ansible_collections/community/general/plugins/modules/btrfs_subvolume.py\", line 596, in get_results\r\nAttributeError: 'NoneType' object has no attribute 'get_subvolume_by_name'\r\n",
    "msg": "MODULE FAILURE\nSee stdout/stderr for the exact error",
    "rc": 1
}
MarcinWieczorek commented 4 months ago

@jarppiko most likely the same error. Thanks

jarppiko commented 4 months ago

I looked at the code and I think there are couple of issues:

  1. BtrfsSubvolumeModule.__load_filesystem(self) is not able to find a matching Btrfs filesystem for a name (path) if there is more than one Btrfs filesystem mounted.

    • __find_matching_filesystem() expects one of uuid, label or device has been defined
    • __find_default_filesystem() returns a filesystem only if there are only one Btrfs filesystem mounted
    • This can be fixed with a function that tries to find a matching Btrfs filesystem for a path (not just mountpoint).
  2. BtrfsFilesystem.get_nearest_subvolume() assumes the subvolume argument is a Btrfs subvolume path, but BtrfsSubvolumeModule.__prepare_subvolume_present() calling it gives it a filesystem path as an argument. It is common to mount a non-root subvolume as filesystem root. This used to be the standard in Ubuntu. Below a subvolume /@ has been mounted as filesystem root /. The get_nearest_subvolume() expects its argument to have that /@ prefix in this case.

% mount | grep /\ 
/dev/nvme0n1p4 on / type btrfs (rw,relatime,ssd,discard=async,space_cache,autodefrag,subvolid=266,subvol=/@)

I can try to fix and create a PR.

I am new to Ansible and I do not fully understand the design logic of the btrfs_subvolume module. There is lot of work being done to figure out various information when it could just call btrfs program and let it do the work. Maybe the author has built the infra to enable the module to serve a base for further development.

The issue might be rather a "feature" than a bug. The code seems to have been designed so that name is a intra-filesystem subvolume path as listed with btrfs subvolume list PATH, not a filesystem path one (e.g. me) could erroneously assume. But the current design makes it difficult just to create a subvolume at a (filesystem) path /path/in/some/mounted/btrfs/filesystem/but/not/its/root.

MarcinWieczorek commented 4 months ago

I'm afraid you won't get much support from the original dev here. This issue is originally a docs problem. I'll be happy to collaborate with you on the fix.

ckotte commented 1 month ago

What's the status of this? Are you guys willing to create a PR? Before I was using ansible-btrfs-subvolume, but it doesn't work anymore. Looks like it's only an issue with Python, but I would rather prefer a module in the community repo (which is maintained)..

jarppiko commented 1 month ago

Hello @ckotte, I started refactoring the module and its helper (plugins/module_utils/btrfs.py). I found the original code quite hard to work with so I implemented its functionality with classes and (Python2 compatible) type hints to make the code more readable. I added there functionality to find any subvolume based on its filesystem path, not just with a subvolume path. But I did not have time to refactor the module itself :-(

See branch dev-2 at jarppiko/ansible-collection-community.general.

I am now re-evaluating the use of Ansible for my purposes (in favor of Nixos) so I am not putting time on the module at the moment. If anyone is interested to continue/work on this, feel free to check if my module_utils/btrfs.py is of any help.