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

Multi device support for btrfs type in filesystem module #2356

Open marbud0 opened 3 years ago

marbud0 commented 3 years ago

Summary

When creating filesystems we like to be able to use multiple devices for btrfs filesystems. Currently dev can only be a path, not a list of paths.

Issue Type

Feature Idea

Component Name

filesystem

Additional Information

We like to be able to write things like:

- name: Create btrfs mirror filesystem
  filesytem:
    dev:
      - /dev/nvme0n1
      - /dev/nvme1n1
    fstype: btrfs
    opt: -L label -mraid1 -draid1

or

$ ansible -b -m filesystem -a 'dev=["/dev/nvme0n1","/dev/nvme1n1"] fstype=btrfs opt="-L label -mraid1 -draid0"' -i hostname, hostname hostname | FAILED! => { "changed": false, "msg": "Device [\"/dev/nvme0n1\",\"/dev/nvme1n1\"] not found." }

This would allow us to treat any type of fs with the same code no matter if it's a xfs/ext4 or btrfs filesystem, even when there are multiple devices for the btrfs case.

Code of Conduct

ansibullbot commented 3 years ago

Files identified in the description:

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

click here for bot help

ansibullbot commented 3 years ago

cc @abulimov @pilou- @quidame click here for bot help

Ajpantuso commented 3 years ago

Is there a reason ansible's builtin iteration doesn't work for this? The difference being that the following executes as two tasks and your suggestion would run as a single task (which creates a dependency that both filesystems be in the exact same state)

- name: Create btrfs mirror filesystem
      community.general.filesystem:
        dev: "{{ item }}"
        fstype: btrfs
        opts: -L label -mraid1 -draid1
      loop:
        - /dev/nvme0n1
        - /dev/nvme1n1
felixfontein commented 3 years ago

@Ajpantuso btrfs filesystems can span multiple devices. The author wants to create a btrfs filesystem spanning two devices, not two btrfs filesystems on two devices with the same settings.

Ajpantuso commented 3 years ago

@Ajpantuso btrfs filesystems can span multiple devices. The author wants to create a btrfs filesystem spanning two devices, not two btrfs filesystems on two devices with the same settings.

Ah, gotcha. Definitely read that wrong. Converting the dev option to a list is difficult, but adding a btrfs specific option for additional devices wouldn't be too bad. It does complicate the idempotency checks however.

felixfontein commented 3 years ago

Actually we're kind of lucky: since device names seldomly contain commas, converting that string option to a list option is essentially risk-free. But yeah, there is definitely some work involved :)

quidame commented 3 years ago

not only btrfs can span multiple devices at creation time, but it can also be enlarged or reduced afterwards by adding or removing devices to/from its pool. Say we add support for multi-devices filesystem, and say we do it like this (implementation doesn't matter here, this comment is only about rationale):

- name: create btrfs on multiple devices
  filesystem:
    fstype: btrfs
    dev: /dev/sde1,/dev/sdf1

So, what if 3 months later we need this ?

    dev: /dev/sde1,/dev/sdf1,/dev/sdg1

and later again, this ?

    dev: /dev/sdf1,/dev/sdg1

From my point of view, it would be very nice to be able to add or remove devices to a btrfs filesystem this way. On the other hand, this promises non-trivial things :)

felixfontein commented 3 years ago

It's some years since I used btrfs (at all, but back then also spanning multiple devices). I think the hard part here will be deciding which filesystem we are talking about, and which of the devices in the list are not yet part of the btrfs filesystem we're interested in. I guess this falls in the non-trivial category of yours @quidame :)

Maybe it would also be better to force dev to only point to one device of the filesystem, and have a new option other_devices which allows to specify more devices. If other_devices is specified, the module would check which of the ones listed there are not yet part of the filesystem (and add them), and also check which devices are part of the filesystem but not in that list (and remove them). This obviously assumes that one identify a filesystem by one of the devices in it; I think that was the case, but I'm not 100% sure.

norg commented 3 years ago

At least we can use the opts as a workaround in some scenarios:

- name: create btrfs filesystem
  community.general.filesystem:
    fstype: btrfs
    dev: /dev/nvme0n1
    opts: /dev/nvme1n1 -m raid1 -d raid1 -L btrfsraid

But I agree that a builtin option would be better, the other_devices idea sounds like one valid way.

oivindoh commented 1 year ago

Thanks for this workaround @norg - I implemented it for a bigger array like this:

  community.general.filesystem:
    fstype: btrfs
    opts: "{{ data_filesystem_drives | reject('search', data_filesystem_drives | first) | join(' ') }} -U {{ data_filesystem_uuid }} -d {{ data_filesystem_level }}"
    dev: "{{ data_filesystem_drives | first }}"
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