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

@@ -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']

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

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=<something>)
+    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)

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

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
 
 #