From f8f17c22bb4cdc229a97f015704235f2ace7dea3 Mon Sep 17 00:00:00 2001 From: Jan Pokorny Date: Wed, 3 May 2023 14:36:48 +0200 Subject: [PATCH] feat: Added support for new blivet fstab handling Blivet can now handle fstab, so handling it in the role became redundant. This adapts the role for the blivet change and removes no longer needed code. - requires blivet PRs #1119 (main change) and #1197 (bug fixes) Issue Tracker Tickets (Jira): #20182 --- library/blivet.py | 106 +++------------------------- tasks/main-blivet.yml | 56 --------------- tests/scripts/test_fstab_dev_ids.py | 61 ++++++++++++++++ tests/test-verify-volume-fstab.yml | 22 ++++-- tests/test-verify-volume-mount.yml | 2 + 5 files changed, 89 insertions(+), 158 deletions(-) create mode 100644 tests/scripts/test_fstab_dev_ids.py diff --git a/library/blivet.py b/library/blivet.py index 20389ea6..edf50b9c 100644 --- a/library/blivet.py +++ b/library/blivet.py @@ -1851,109 +1851,19 @@ def manage_pool(b, pool): volume['_mount_id'] = bvolume._volume.get('_mount_id', '') -class FSTab(object): - def __init__(self, blivet_obj): - self._blivet = blivet_obj - self._entries = list() - self.parse() - - def lookup(self, key, value): - return next((e for e in self._entries if e.get(key) == value), None) - - def reset(self): - self._entries = list() - - def parse(self): - if self._entries: - self.reset() - - with open('/etc/fstab') as f: - for line in f.readlines(): - if line.lstrip().startswith("#"): - continue - - fields = line.split() - if len(fields) < 6: - continue - - device = self._blivet.devicetree.resolve_device(fields[0]) - self._entries.append(dict(device_id=fields[0], - device_path=getattr(device, 'path', None), - fs_type=fields[2], - mount_point=fields[1], - mount_options=fields[3])) - - -def get_mount_info(pools, volumes, actions, fstab): - """ Return a list of argument dicts to pass to the mount module to manage mounts. - - The overall approach is to remove existing mounts associated with file systems - we are removing and those with changed mount points, re-adding them with the - new mount point later. - - Removed mounts go directly into the mount_info list, which is the return value, - while added/active mounts to a list that gets appended to the mount_info list - at the end to ensure that removals happen first. - """ - mount_info = list() - mount_vols = list() - - # account for mounts removed by removing or reformatting volumes - if actions: - for action in actions: - if action.is_destroy and action.is_format and action.format.type is not None: - mount = fstab.lookup('device_path', action.device.path) - if mount is not None: - mount_info.append({"src": mount['device_id'], "path": mount['mount_point'], - 'state': 'absent', 'fstype': mount['fs_type']}) - - def handle_new_mount(volume, fstab): - replace = None - mounted = False - - mount = fstab.lookup('device_path', volume['_device']) - if (volume['mount_point'] and volume['mount_point'].startswith('/')) \ - or volume['fs_type'] == 'swap': - mounted = True - - # handle removal of existing mounts of this volume - if mount and mount['fs_type'] != 'swap' and mount['mount_point'] != volume['mount_point']: - replace = dict(path=mount['mount_point'], state="absent") - - return mounted, replace - +def update_mounts(b, pools, volumes, actions): # account for mounts that we set up or are replacing in pools for pool in pools: for volume in pool['volumes']: if pool['state'] == 'present' and volume['state'] == 'present': - mounted, replace = handle_new_mount(volume, fstab) - if replace: - mount_info.append(replace) - if mounted: - mount_vols.append(volume) + dev = b.devicetree.resolve_device(volume['_device']) + dev.format.mount(mountpoint=volume['mount_point'], options=volume['mount_options']) # account for mounts that we set up or are replacing in standalone volumes for volume in volumes: if volume['state'] == 'present': - mounted, replace = handle_new_mount(volume, fstab) - if replace: - mount_info.append(replace) - if mounted: - mount_vols.append(volume) - - for volume in mount_vols: - mount_info.append({'src': volume['_mount_id'], - 'path': volume['mount_point'] if volume['fs_type'] != "swap" else "none", - 'fstype': volume['fs_type'], - 'opts': volume['mount_options'], - 'dump': volume['mount_check'], - 'passno': volume['mount_passno'], - 'state': 'mounted' if volume['fs_type'] != "swap" else "present", - 'owner': volume['mount_user'], - 'group': volume['mount_group'], - 'mode': volume['mount_mode']}) - - return mount_info + dev = b.devicetree.resolve_device(volume['_device']) + dev.format.mount(mountpoint=volume['mount_point'], options=volume['mount_options']) def get_crypt_info(actions): @@ -2210,8 +2120,8 @@ def run_module(): volume_defaults = module.params['volume_defaults'] b = Blivet() + b.fstab.dest_file = "/etc/fstab" b.reset() - fstab = FSTab(b) actions = list() if module.params['packages_only']: @@ -2277,7 +2187,7 @@ def action_dict(action): callbacks.action_executed.add(record_action) callbacks.action_executed.add(ensure_udev_update) try: - b.devicetree.actions.process(devices=b.devicetree.devices, dry_run=module.check_mode) + b.devicetree.actions.process(devices=b.devicetree.devices, fstab=b.fstab, dry_run=module.check_mode) except Exception as e: module.fail_json(msg="Failed to commit changes to disk: %s" % str(e), **result) finally: @@ -2286,8 +2196,8 @@ def action_dict(action): update_fstab_identifiers(b, module.params['pools'], module.params['volumes']) activate_swaps(b, module.params['pools'], module.params['volumes']) + update_mounts(b, module.params['pools'], module.params['volumes'], actions) - result['mounts'] = get_mount_info(module.params['pools'], module.params['volumes'], actions, fstab) result['crypts'] = get_crypt_info(actions) result['leaves'] = [d.path for d in b.devicetree.leaves] result['pools'] = module.params['pools'] diff --git a/tasks/main-blivet.yml b/tasks/main-blivet.yml index f73be000..d2532a87 100644 --- a/tasks/main-blivet.yml +++ b/tasks/main-blivet.yml @@ -130,62 +130,6 @@ set_fact: _storage_volumes_list: "{{ blivet_output.volumes }}" -# Mount Management -# -# remove obsolete mounts -# removed -# reformatted (just add w/ new settings?) -# changed options? (just add w/ new settings?) -# add new mounts -# -# XXX Apparently we have to do the removals, then tell systemd to -# update its view, then set up the new mounts. Otherwise, -# systemd will forcibly prevent mounting a new volume to an -# existing mount point. -- name: Remove obsolete mounts - mount: # noqa fqcn - src: "{{ mount_info['src'] | default(omit) }}" - path: "{{ mount_info['path'] }}" - fstype: "{{ mount_info['fstype'] | default(omit) }}" - opts: "{{ mount_info['opts'] | default(omit) }}" - state: "{{ mount_info['state'] }}" - loop: "{{ blivet_output.mounts | selectattr('state', 'defined') | - selectattr('state', 'match', '^absent$') | list }}" - loop_control: - loop_var: mount_info - -- name: Tell systemd to refresh its view of /etc/fstab - systemd: - daemon_reload: true - when: blivet_output['mounts'] - -- name: Set up new/current mounts - mount: # noqa fqcn - src: "{{ mount_info['src'] | default(omit) }}" - path: "{{ mount_info['path'] }}" - fstype: "{{ mount_info['fstype'] | default(omit) }}" - opts: "{{ mount_info['opts'] | default(omit) }}" - state: "{{ mount_info['state'] }}" - loop: "{{ blivet_output.mounts | selectattr('state', 'defined') | - rejectattr('state', 'match', '^absent$') | list }}" - loop_control: - loop_var: mount_info - -- name: Manage mount ownership/permissions - file: - path: "{{ mount_info['path'] }}" - owner: "{{ mount_info['owner'] if 'owner' in mount_info else omit }}" - group: "{{ mount_info['group'] if 'group' in mount_info else omit }}" - mode: "{{ mount_info['mode'] if 'mode' in mount_info else omit }}" - state: directory - when: mount_info['owner'] != none or - mount_info['group'] != none or - mount_info['mode'] != none - loop: "{{ blivet_output.mounts | selectattr('state', 'defined') | - rejectattr('state', 'match', '^absent$') | list }}" - loop_control: - loop_var: mount_info - - name: Tell systemd to refresh its view of /etc/fstab systemd: daemon_reload: true diff --git a/tests/scripts/test_fstab_dev_ids.py b/tests/scripts/test_fstab_dev_ids.py new file mode 100644 index 00000000..b7493cdf --- /dev/null +++ b/tests/scripts/test_fstab_dev_ids.py @@ -0,0 +1,61 @@ +import json +import subprocess +import sys + + +def find_dev_in_fstab(dev, fstab): + # check whether some device representation is present in the fstab + if not dev: + raise ValueError("No device given") + + # dev can be represented by string starting with description followed by '=' (e.g. UUID=) + pair = dev.split('=') + prefix = "/dev/disk/by-*/" + + if len(pair) == 1: + dev_id = pair[0] + if dev_id.startswith('/'): + prefix = "" + else: + dev_id = pair[1] + + cmd_output = subprocess.run("lsblk --list -Jno PATH,UUID,PARTUUID,LABEL,PARTLABEL %s%s" % (prefix, dev_id), + shell=True, + stdout=subprocess.PIPE, + text=True) + + if cmd_output.stderr: + raise ValueError("Lsblk command failed: %s" % cmd_output.stderr) + + blockdevices = json.loads(cmd_output.stdout)['blockdevices'] + + exact_matches = 0 + partial_matches = 0 + + for bdev in blockdevices: + for key in ['path', 'uuid', 'partuuid', 'label', 'partlabel']: + if bdev[key] is not None and bdev[key] in fstab: + if dev_id == bdev[key]: + exact_matches += 1 + else: + partial_matches += 1 + + return exact_matches, partial_matches + + +def main(args): + if len(args) < 2: + raise ValueError("Missing arguments") + + dev = args[1] + + # This is an ugly stuff but it will work for the testing purposes + fstab = ' '.join(args[2:]) + + exact_matches, partial_matches = find_dev_in_fstab(dev, fstab) + + print(exact_matches, partial_matches) + + +if __name__ == "__main__": + main(sys.argv) diff --git a/tests/test-verify-volume-fstab.yml b/tests/test-verify-volume-fstab.yml index f4414e2c..926612b0 100644 --- a/tests/test-verify-volume-fstab.yml +++ b/tests/test-verify-volume-fstab.yml @@ -36,13 +36,24 @@ __mount_options: "{{ storage_test_volume.mount_options }}" __pattern: ' {{ __mount_point }} .* {{ __mount_options }} +' +- name: Execute script to find if there is a device alias in fstab + script: scripts/test_fstab_dev_ids.py {{ storage_test_volume._mount_id }} {{ storage_test_fstab.stdout_lines }} + args: + executable: python3 + register: storage_test_dev_id_results + changed_when: false + +- name: Clarify the results by assigning them into variables + set_fact: + storage_test_exact_id_matches: "{{ storage_test_dev_id_results.stdout.split(' ')[0] }}" + storage_test_partial_id_matches: "{{ storage_test_dev_id_results.stdout.split(' ')[1] }}" + # device id - name: Verify that the device identifier appears in /etc/fstab assert: - that: storage_test_fstab_id_matches | length == - storage_test_fstab_expected_id_matches | int - msg: Expected device identifier not found in /etc/fstab. - when: _storage_test_volume_present | bool + that: "{{ storage_test_exact_id_matches|int + storage_test_partial_id_matches|int == storage_test_fstab_expected_id_matches|int }}" + msg: "Expected device identifier not found in /etc/fstab. (expected {{ storage_test_fstab_expected_id_matches }} matches for {{ storage_test_volume._mount_id }})" + when: _storage_test_volume_present # mount point - name: Verify the fstab mount point @@ -71,8 +82,11 @@ - name: Clean up variables set_fact: storage_test_fstab_id_matches: null + storage_test_fstab_uuid_matches: null storage_test_fstab_mount_point_matches: null storage_test_fstab_expected_id_matches: null + storage_test_fstab_exact_id_matches: null + storage_test_fstab_partial_id_matches: null storage_test_fstab_expected_mount_point_matches: null storage_test_fstab_mount_options_matches: null storage_test_fstab_expected_mount_options_matches: null diff --git a/tests/test-verify-volume-mount.yml b/tests/test-verify-volume-mount.yml index cf86b343..d6d484bb 100644 --- a/tests/test-verify-volume-mount.yml +++ b/tests/test-verify-volume-mount.yml @@ -55,6 +55,8 @@ msg: >- Found unexpected mount state for volume '{{ storage_test_volume.name }}' device + (expected matches '{{ storage_test_mount_expected_match_count | int }}'; + found matches '{{ storage_test_mount_device_matches|length }}') when: _storage_test_volume_present and storage_test_volume.mount_point #