filesystem: extend support for FreeBSD (#2902) (#2983)

* extend support for FreeBSD

* Check if FS exists with `fstyp` if `blkid` fails to find FS signature
  (fix a potential data loss)
* Add support for FreeBSD special devices (character devices).
* Add support for FreeBSD native fstype (UFS).
* Update DOCUMENTATION accordingly.

* add/update integration tests

* Add tests for `fstype=ufs` on FreeBSD.
* Run `remove_fs` tests (`state=absent`) on FreeBSD.
* Run `overwrite_another_fs` tests on FreeBSD.

* add a changelog fragment

* fix indentation

* restrict new tests to regular files

* fix typo

* fix searching of providersize (block count)

* add '-y' option to growfs command

* remove references to versions older than the collection itself

* bump version adding new feats to 3.4.0

* reformat *collection* and *version added* for better DOCUMENTATION parsing

* skip tests for FreeBSD < 12.2

* run tests for FreeBSD >= 12.2

* re-enable tests for FreeBSD < 12.2 and give it a try with group1

* util-linux not available on FreeBSD < 12.2

(cherry picked from commit 9023d4dba1)

Co-authored-by: quidame <quidame@poivron.org>

changelogs/fragments/2902-filesystem_extend_freebsd_support.yml

@@ -0,0 +1,6 @@
+---
+minor_changes:
+  - filesystem - extend support for FreeBSD. Avoid potential data loss by checking
+    existence of a filesystem with ``fstyp`` (native command) if ``blkid`` (foreign
+    command) doesn't find one. Add support for character devices and ``ufs`` filesystem
+    type (https://github.com/ansible-collections/community.general/pull/2902).

plugins/modules/system/filesystem.py

@@ -1,6 +1,7 @@
 #!/usr/bin/python
 # -*- coding: utf-8 -*-
 
+# Copyright: (c) 2021, quidame <quidame@poivron.org>
 # Copyright: (c) 2013, Alexander Bulimov <lazywolf0@gmail.com>
 # GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
 
@@ -12,6 +13,7 @@ DOCUMENTATION = '''
 ---
 author:
   - Alexander Bulimov (@abulimov)
+  - quidame (@quidame)
 module: filesystem
 short_description: Makes a filesystem
 description:
@@ -30,25 +32,22 @@ options:
     default: present
     version_added: 1.3.0
   fstype:
-    choices: [ btrfs, ext2, ext3, ext4, ext4dev, f2fs, lvm, ocfs2, reiserfs, xfs, vfat, swap ]
+    choices: [ btrfs, ext2, ext3, ext4, ext4dev, f2fs, lvm, ocfs2, reiserfs, xfs, vfat, swap, ufs ]
     description:
       - Filesystem type to be created. This option is required with
         C(state=present) (or if I(state) is omitted).
-      - reiserfs support was added in 2.2.
-      - lvm support was added in 2.5.
-      - since 2.5, I(dev) can be an image file.
-      - vfat support was added in 2.5
-      - ocfs2 support was added in 2.6
-      - f2fs support was added in 2.7
-      - swap support was added in 2.8
+      - ufs support has been added in community.general 3.4.0.
     type: str
     aliases: [type]
   dev:
     description:
-      - Target path to block device or regular file.
-      - On systems not using block devices but character devices instead (as
-        FreeBSD), this module only works when applying to regular files, aka
-        disk images.
+      - Target path to block device (Linux) or character device (FreeBSD) or
+        regular file (both).
+      - When setting Linux-specific filesystem types on FreeBSD, this module
+        only works when applying to regular files, aka disk images.
+      - Currently C(lvm) (Linux-only) and C(ufs) (FreeBSD-only) don't support
+        a regular file as their target I(dev).
+      - Support for character devices on FreeBSD has been added in community.general 3.4.0.
     type: path
     required: yes
     aliases: [device]
@@ -60,7 +59,7 @@ options:
   resizefs:
     description:
       - If C(yes), if the block device and filesystem size differ, grow the filesystem into the space.
-      - Supported for C(ext2), C(ext3), C(ext4), C(ext4dev), C(f2fs), C(lvm), C(xfs) and C(vfat) filesystems.
+      - Supported for C(ext2), C(ext3), C(ext4), C(ext4dev), C(f2fs), C(lvm), C(xfs), C(ufs) and C(vfat) filesystems.
         Attempts to resize other filesystem types will fail.
       - XFS Will only grow if mounted. Currently, the module is based on commands
         from C(util-linux) package to perform operations, so resizing of XFS is
@@ -73,16 +72,24 @@ options:
       - List of options to be passed to mkfs command.
     type: str
 requirements:
-  - Uses tools related to the I(fstype) (C(mkfs)) and the C(blkid) command.
-  - When I(resizefs) is enabled, C(blockdev) command is required too.
+  - Uses specific tools related to the I(fstype) for creating or resizing a
+    filesystem (from packages e2fsprogs, xfsprogs, dosfstools, and so on).
+  - Uses generic tools mostly related to the Operating System (Linux or
+    FreeBSD) or available on both, as C(blkid).
+  - On FreeBSD, either C(util-linux) or C(e2fsprogs) package is required.
 notes:
-  - Potential filesystem on I(dev) are checked using C(blkid). In case C(blkid)
-    isn't able to detect an existing filesystem, this filesystem is overwritten
-    even if I(force) is C(no).
-  - On FreeBSD systems, either C(e2fsprogs) or C(util-linux) packages provide
-    a C(blkid) command that is compatible with this module, when applied to
-    regular files.
+  - Potential filesystems on I(dev) are checked using C(blkid). In case C(blkid)
+    is unable to detect a filesystem (and in case C(fstyp) on FreeBSD is also
+    unable to detect a filesystem), this filesystem is overwritten even if
+    I(force) is C(no).
+  - On FreeBSD systems, both C(e2fsprogs) and C(util-linux) packages provide
+    a C(blkid) command that is compatible with this module. However, these
+    packages conflict with each other, and only the C(util-linux) package
+    provides the command required to not fail when I(state=absent).
   - This module supports I(check_mode).
+seealso:
+  - module: community.general.filesize
+  - module: ansible.posix.mount
 '''
 
 EXAMPLES = '''
@@ -101,6 +108,11 @@ EXAMPLES = '''
   community.general.filesystem:
     dev: /dev/sdb1
     state: absent
+
+- name: Create a filesystem on top of a regular file
+  community.general.filesystem:
+    dev: /path/to/disk.img
+    fstype: vfat
 '''
 
 from distutils.version import LooseVersion
@@ -125,6 +137,10 @@ class Device(object):
             blockdev_cmd = self.module.get_bin_path("blockdev", required=True)
             dummy, out, dummy = self.module.run_command([blockdev_cmd, "--getsize64", self.path], check_rc=True)
             devsize_in_bytes = int(out)
+        elif stat.S_ISCHR(statinfo.st_mode) and platform.system() == 'FreeBSD':
+            diskinfo_cmd = self.module.get_bin_path("diskinfo", required=True)
+            dummy, out, dummy = self.module.run_command([diskinfo_cmd, self.path], check_rc=True)
+            devsize_in_bytes = int(out.split()[2])
         elif os.path.isfile(self.path):
             devsize_in_bytes = os.path.getsize(self.path)
         else:
@@ -423,6 +439,31 @@ class Swap(Filesystem):
     MKFS_FORCE_FLAGS = ['-f']
 
 
+class UFS(Filesystem):
+    MKFS = 'newfs'
+    INFO = 'dumpfs'
+    GROW = 'growfs'
+    GROW_MAX_SPACE_FLAGS = ['-y']
+
+    def get_fs_size(self, dev):
+        """Get providersize and fragment size and return their product."""
+        cmd = self.module.get_bin_path(self.INFO, required=True)
+        dummy, out, dummy = self.module.run_command([cmd, str(dev)], check_rc=True, environ_update=self.LANG_ENV)
+
+        fragmentsize = providersize = None
+        for line in out.splitlines():
+            if line.startswith('fsize'):
+                fragmentsize = int(line.split()[1])
+            elif 'providersize' in line:
+                providersize = int(line.split()[-1])
+            if None not in (fragmentsize, providersize):
+                break
+        else:
+            raise ValueError(out)
+
+        return fragmentsize * providersize
+
+
 FILESYSTEMS = {
     'ext2': Ext2,
     'ext3': Ext3,
@@ -436,6 +477,7 @@ FILESYSTEMS = {
     'ocfs2': Ocfs2,
     'LVM2_member': LVM,
     'swap': Swap,
+    'ufs': UFS,
 }
 
 
@@ -484,11 +526,16 @@ def main():
 
     dev = Device(module, dev)
 
+    # In case blkid/fstyp isn't able to identify an existing filesystem, device
+    # is considered as empty, then this existing filesystem would be overwritten
+    # even if force isn't enabled.
     cmd = module.get_bin_path('blkid', required=True)
     rc, raw_fs, err = module.run_command([cmd, '-c', os.devnull, '-o', 'value', '-s', 'TYPE', str(dev)])
-    # In case blkid isn't able to identify an existing filesystem, device is considered as empty,
-    # then this existing filesystem would be overwritten even if force isn't enabled.
     fs = raw_fs.strip()
+    if not fs and platform.system() == 'FreeBSD':
+        cmd = module.get_bin_path('fstyp', required=True)
+        rc, raw_fs, err = module.run_command([cmd, str(dev)])
+        fs = raw_fs.strip()
 
     if state == "present":
         if fstype in friendly_names:

tests/integration/targets/filesystem/aliases

@@ -1,5 +1,5 @@
 destructive
-shippable/posix/group3
+shippable/posix/group1
 skip/aix
 skip/osx
 skip/macos

tests/integration/targets/filesystem/defaults/main.yml

@@ -23,3 +23,9 @@ tested_filesystems:
   f2fs: {fssize: '{{ f2fs_fssize|default(60) }}', grow: 'f2fs_version is version("1.10.0", ">=")'}
   lvm: {fssize: 20, grow: True}
   swap: {fssize: 10, grow: False}  # grow not implemented
+  ufs: {fssize: 10, grow: True}
+
+
+get_uuid_any: "blkid -c /dev/null -o value -s UUID {{ dev }}"
+get_uuid_ufs: "dumpfs {{ dev }} | awk -v sb=superblock -v id=id '$1 == sb && $4 == id {print $6$7}'"
+get_uuid_cmd: "{{ get_uuid_ufs if fstype == 'ufs' else get_uuid_any }}"

tests/integration/targets/filesystem/tasks/create_device.yml

@@ -19,6 +19,17 @@
           ansible.builtin.set_fact:
             dev: "{{ loop_device_cmd.stdout }}"
 
+    - when: fstype == 'ufs'
+      block:
+        - name: 'Create a memory disk for UFS'
+          ansible.builtin.command:
+            cmd: 'mdconfig -a -f {{ dev }}'
+          register: memory_disk_cmd
+
+        - name: 'Switch to memory disk target for further tasks'
+          ansible.builtin.set_fact:
+            dev: "/dev/{{ memory_disk_cmd.stdout }}"
+
     - include_tasks: '{{ action }}.yml'
 
   always:
@@ -28,10 +39,16 @@
         removes: '{{ dev }}'
       when: fstype == 'lvm'
 
-    - name: 'Clean correct device for LVM'
+    - name: 'Detach memory disk used for UFS'
+      ansible.builtin.command:
+        cmd: 'mdconfig -d -u {{ dev }}'
+        removes: '{{ dev }}'
+      when: fstype == 'ufs'
+
+    - name: 'Clean correct device for LVM and UFS'
       ansible.builtin.set_fact:
         dev: '{{ image_file }}'
-      when: fstype == 'lvm'
+      when: fstype in ['lvm', 'ufs']
 
     - name: 'Remove disk image file'
       ansible.builtin.file:

tests/integration/targets/filesystem/tasks/create_fs.yml

@@ -12,8 +12,8 @@
       - 'fs_result is success'
 
 - name: "Get UUID of created filesystem"
-  ansible.builtin.command:
-    cmd: 'blkid -c /dev/null -o value -s UUID {{ dev }}'
+  ansible.builtin.shell:
+    cmd: "{{ get_uuid_cmd }}"
   changed_when: false
   register: uuid
 
@@ -24,8 +24,8 @@
   register: fs2_result
 
 - name: "Get UUID of the filesystem"
-  ansible.builtin.command:
-    cmd: 'blkid -c /dev/null -o value -s UUID {{ dev }}'
+  ansible.builtin.shell:
+    cmd: "{{ get_uuid_cmd }}"
   changed_when: false
   register: uuid2
 
@@ -44,8 +44,8 @@
   register: fs3_result
 
 - name: "Get UUID of the new filesystem"
-  ansible.builtin.command:
-    cmd: 'blkid -c /dev/null -o value -s UUID {{ dev }}'
+  ansible.builtin.shell:
+    cmd: "{{ get_uuid_cmd }}"
   changed_when: false
   register: uuid3
 
@@ -71,6 +71,11 @@
         cmd: 'losetup -c {{ dev }}'
       when: fstype == 'lvm'
 
+    - name: "Resize memory disk for UFS"
+      ansible.builtin.command:
+        cmd: 'mdconfig -r -u {{ dev }} -s {{ fssize | int + 1 }}M'
+      when: fstype == 'ufs'
+
     - name: "Expand filesystem"
       community.general.filesystem:
         dev: '{{ dev }}'
@@ -79,8 +84,8 @@
       register: fs4_result
 
     - name: "Get UUID of the filesystem"
-      ansible.builtin.command:
-        cmd: 'blkid -c /dev/null -o value -s UUID {{ dev }}'
+      ansible.builtin.shell:
+        cmd: "{{ get_uuid_cmd }}"
       changed_when: false
       register: uuid4
 

tests/integration/targets/filesystem/tasks/freebsd_setup.yml

@@ -0,0 +1,10 @@
+---
+- name: "Uninstall e2fsprogs"
+  ansible.builtin.package:
+    name: e2fsprogs
+    state: absent
+
+- name: "Install util-linux"
+  ansible.builtin.package:
+    name: util-linux
+    state: present

tests/integration/targets/filesystem/tasks/main.yml

@@ -35,6 +35,10 @@
     # Available on FreeBSD but not on testbed (util-linux conflicts with e2fsprogs): wipefs, mkfs.minix
     - 'not (ansible_system == "FreeBSD" and item.1 in ["overwrite_another_fs", "remove_fs"])'
 
+    # Linux limited support
+    # Not available: ufs (this is FreeBSD's native fs)
+    - 'not (ansible_system == "Linux" and item.0.key == "ufs")'
+
     # Other limitations and corner cases
 
     # f2fs-tools and reiserfs-utils packages not available with RHEL/CentOS on CI
@@ -59,3 +63,24 @@
             item.0.key == "xfs" and ansible_python.version.major == 2)'
 
   loop: "{{ query('dict', tested_filesystems)|product(['create_fs', 'overwrite_another_fs', 'remove_fs'])|list }}"
+
+
+# With FreeBSD extended support (util-linux is not available before 12.2)
+
+- include_tasks: freebsd_setup.yml
+  when:
+    - 'ansible_system == "FreeBSD"'
+    - 'ansible_distribution_version is version("12.2", ">=")'
+
+- include_tasks: create_device.yml
+  vars:
+    image_file: '{{ remote_tmp_dir }}/img'
+    fstype: '{{ item.0.key }}'
+    fssize: '{{ item.0.value.fssize }}'
+    grow: '{{ item.0.value.grow }}'
+    action: '{{ item.1 }}'
+  when:
+    - 'ansible_system == "FreeBSD"'
+    - 'ansible_distribution_version is version("12.2", ">=")'
+    - 'item.0.key in ["xfs", "vfat"]'
+  loop: "{{ query('dict', tested_filesystems)|product(['create_fs', 'overwrite_another_fs', 'remove_fs'])|list }}"

tests/integration/targets/filesystem/tasks/overwrite_another_fs.yml

@@ -10,8 +10,8 @@
     cmd: 'mkfs.minix {{ dev }}'
 
 - name: 'Get UUID of the new filesystem'
-  ansible.builtin.command:
-    cmd: 'blkid -c /dev/null -o value -s UUID {{ dev }}'
+  ansible.builtin.shell:
+    cmd: "{{ get_uuid_cmd }}"
   changed_when: false
   register: uuid
 
@@ -23,8 +23,8 @@
   ignore_errors: True
 
 - name: 'Get UUID of the filesystem'
-  ansible.builtin.command:
-    cmd: 'blkid -c /dev/null -o value -s UUID {{ dev }}'
+  ansible.builtin.shell:
+    cmd: "{{ get_uuid_cmd }}"
   changed_when: false
   register: uuid2
 
@@ -42,8 +42,8 @@
   register: fs_result2
 
 - name: 'Get UUID of the new filesystem'
-  ansible.builtin.command:
-    cmd: 'blkid -c /dev/null -o value -s UUID {{ dev }}'
+  ansible.builtin.shell:
+    cmd: "{{ get_uuid_cmd }}"
   changed_when: false
   register: uuid3
 

tests/integration/targets/filesystem/tasks/remove_fs.yml

@@ -7,8 +7,8 @@
     fstype: '{{ fstype }}'
 
 - name: "Get filesystem UUID with 'blkid'"
-  ansible.builtin.command:
-    cmd: 'blkid -c /dev/null -o value -s UUID {{ dev }}'
+  ansible.builtin.shell:
+    cmd: "{{ get_uuid_cmd }}"
   changed_when: false
   register: blkid_ref
 
@@ -27,8 +27,8 @@
   check_mode: yes
 
 - name: "Get filesystem UUID with 'blkid' (should remain the same)"
-  ansible.builtin.command:
-    cmd: 'blkid -c /dev/null -o value -s UUID {{ dev }}'
+  ansible.builtin.shell:
+    cmd: "{{ get_uuid_cmd }}"
   changed_when: false
   register: blkid
 
@@ -46,8 +46,8 @@
   register: wipefs
 
 - name: "Get filesystem UUID with 'blkid' (should be empty)"
-  ansible.builtin.command:
-    cmd: 'blkid -c /dev/null -o value -s UUID {{ dev }}'
+  ansible.builtin.shell:
+    cmd: "{{ get_uuid_cmd }}"
   changed_when: false
   failed_when: false
   register: blkid