Add support for managing pool members

[vojtechtrefny]

For LVM pools this adds support for adding and removing members
(PVs) from the pool (VG).

Fixes: #166

---

Keeping this as a draft for now, I found some issues in blivet that needs to be fixed first. There is a workaround in the code for storaged-project/blivet#1034 but storaged-project/blivet#1036 needs to be fixed before releasing this.

[vojtechtrefny]

Question: Should we consider removing the PV unsafe? Currently the code will remove the PV from the VG even with the safe mode on. This isn't a destructive operation, but I am also removing the PV from the disk after that and that might be potentially dangerous. And it also might not be user friendly to remove the PV just because user specified incomplete disks by accident.

[briansmith0]

In my opinion, it makes sense to require storage_safe_mode = false to remove PV's from a VG. The storage_safe_mode description in the README states that "When true (the default), an error will occur instead of automatically removing existing devices and/or formatting.", and it seems that this operation would fall under "removing existing devices".

Does the role fail if the user tries to remove a PV from a VG and the PV has data on it?

[vojtechtrefny]

The storage_safe_mode description in the README states that "When true (the default), an error will occur instead of automatically removing existing devices and/or formatting.", and it seems that this operation would fall under "removing existing devices".

Thanks, I'll change the code to require safe mode to be turned off for this operation.

Does the role fail if the user tries to remove a PV from a VG and the PV has data on it?

If there is enough free space in the VG it will run pvmove to move the LV to an empty PV but it will return error if there isn't enough free space.

[richm]

example error:

TASK [linux-system-roles.storage : manage the pools and volumes to match the specified state] ***
task path: /tmp/tmpngnprtsi/tests/roles/linux-system-roles.storage/tasks/main-blivet.yml:77
Thursday 28 April 2022  00:49:58 +0000 (0:00:00.020)       0:00:36.477 ******** 
fatal: [/cache/centos-7.qcow2c.snap]: FAILED! => {
    "actions": [],
    "changed": false,
    "crypts": [],
    "leaves": [],
    "mounts": [],
    "packages": [],
    "pools": [],
    "volumes": []
}

MSG:

failed to remove disk 'sda1' from pool 'foo': 'unicode' object has no attribute 'path'

TASK [linux-system-roles.storage : failed message] *****************************
task path: /tmp/tmpngnprtsi/tests/roles/linux-system-roles.storage/tasks/main-blivet.yml:99
Thursday 28 April 2022  00:50:01 +0000 (0:00:02.424)       0:00:38.902 ******** 
fatal: [/cache/centos-7.qcow2c.snap]: FAILED! => {
    "changed": false
}

MSG:

{'crypts': [], 'mounts': [], 'leaves': [], 'changed': False, 'actions': [], 'failed': True, 'volumes': [], 'invocation': {'module_args': {'packages_only': False, 'disklabel_type': None, 'diskvolume_mkfs_option_map': {'ext4': '-F', 'ext3': '-F', 'ext2': '-F'}, 'safe_mode': False, 'pools': [{'name': 'foo', 'encryption_password': None, 'state': 'present', 'raid_metadata_version': None, 'encryption': False, 'encryption_key_size': None, 'disks': ['nvme0n1'], 'raid_level': None, 'encryption_luks_version': None, 'raid_device_count': None, 'encryption_key': None, 'volumes': [], 'raid_chunk_size': None, 'type': 'lvm', 'encryption_cipher': None, 'raid_spare_count': None}], 'volumes': [], 'pool_defaults': {'encryption_password': None, 'raid_metadata_version': None, 'encryption': False, 'encryption_cipher': None, 'disks': [], 'encryption_key': None, 'encryption_key_size': None, 'encryption_luks_version': None, 'raid_device_count': None, 'state': 'present', 'volumes': [], 'raid_chunk_size': None, 'type': 'lvm', 'raid_level': None, 'raid_spare_count': None}, 'volume_defaults': {'raid_metadata_version': None, 'mount_device_identifier': 'uuid', 'fs_type': 'xfs', 'mount_options': 'defaults', 'size': 0, 'mount_point': '', 'compression': None, 'encryption_password': None, 'encryption': False, 'raid_level': None, 'raid_device_count': None, 'state': 'present', 'vdo_pool_size': None, 'fs_overwrite_existing': True, 'encryption_cipher': None, 'encryption_key_size': None, 'encryption_key': None, 'fs_label': '', 'encryption_luks_version': None, 'mount_passno': 0, 'raid_spare_count': None, 'cache_mode': None, 'deduplication': None, 'cached': False, 'type': 'lvm', 'disks': [], 'mount_check': 0, 'cache_size': 0, 'raid_chunk_size': None, 'cache_devices': [], 'fs_create_options': ''}, 'use_partitions': True}}, 'pools': [], 'packages': [], 'msg': "failed to remove disk 'sda1' from pool 'foo': 'unicode' object has no attribute 'path'", '_ansible_no_log': False}

looks like trying to dereference a string instead of an object

[vojtechtrefny]

looks like trying to dereference a string instead of an object

That's another bug in blivet, already fixed in upstream and tracked here https://bugzilla.redhat.com/show_bug.cgi?id=2079220 for RHEL. Fedora 35 has already the fixed version but the tests are failing on F35 due to lack of space.

[richm]

fedora-35 tests are failing due to https://pagure.io/cloud-sig/issue/379

[richm]

[citest bad]

[japokorn]

Looks good to me.

[richm]

Except for the question about safe_mode global, lgtm

[vojtechtrefny]

Except for the question about safe_mode global, lgtm

Sorry, I missed the comment. You are right the global safe_mode wasn't needed, I removed the line, thanks.