From 9023d4dba1c635e9839448e975a8c0c0fdf1fdff Mon Sep 17 00:00:00 2001 From: quidame Date: Sat, 10 Jul 2021 16:37:31 +0200 Subject: [PATCH] filesystem: extend support for FreeBSD (#2902) * 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 --- ...2902-filesystem_extend_freebsd_support.yml | 6 ++ plugins/modules/system/filesystem.py | 93 ++++++++++++++----- tests/integration/targets/filesystem/aliases | 2 +- .../targets/filesystem/defaults/main.yml | 6 ++ .../filesystem/tasks/create_device.yml | 21 ++++- .../targets/filesystem/tasks/create_fs.yml | 21 +++-- .../filesystem/tasks/freebsd_setup.yml | 10 ++ .../targets/filesystem/tasks/main.yml | 25 +++++ .../filesystem/tasks/overwrite_another_fs.yml | 12 +-- .../targets/filesystem/tasks/remove_fs.yml | 12 +-- 10 files changed, 162 insertions(+), 46 deletions(-) create mode 100644 changelogs/fragments/2902-filesystem_extend_freebsd_support.yml create mode 100644 tests/integration/targets/filesystem/tasks/freebsd_setup.yml diff --git a/changelogs/fragments/2902-filesystem_extend_freebsd_support.yml b/changelogs/fragments/2902-filesystem_extend_freebsd_support.yml new file mode 100644 index 0000000000..1518d0190f --- /dev/null +++ b/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). diff --git a/plugins/modules/system/filesystem.py b/plugins/modules/system/filesystem.py index cbb0e5e95e..4f1d6ee0d1 100644 --- a/plugins/modules/system/filesystem.py +++ b/plugins/modules/system/filesystem.py @@ -1,6 +1,7 @@ #!/usr/bin/python # -*- coding: utf-8 -*- +# Copyright: (c) 2021, quidame # Copyright: (c) 2013, Alexander Bulimov # 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: diff --git a/tests/integration/targets/filesystem/aliases b/tests/integration/targets/filesystem/aliases index 1c80472f94..1ef4c3619a 100644 --- a/tests/integration/targets/filesystem/aliases +++ b/tests/integration/targets/filesystem/aliases @@ -1,5 +1,5 @@ destructive -shippable/posix/group3 +shippable/posix/group1 skip/aix skip/osx skip/macos diff --git a/tests/integration/targets/filesystem/defaults/main.yml b/tests/integration/targets/filesystem/defaults/main.yml index 15ef85aa0e..27672bbea6 100644 --- a/tests/integration/targets/filesystem/defaults/main.yml +++ b/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 }}" diff --git a/tests/integration/targets/filesystem/tasks/create_device.yml b/tests/integration/targets/filesystem/tasks/create_device.yml index 30fd62e33a..ae314221a5 100644 --- a/tests/integration/targets/filesystem/tasks/create_device.yml +++ b/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: diff --git a/tests/integration/targets/filesystem/tasks/create_fs.yml b/tests/integration/targets/filesystem/tasks/create_fs.yml index de1a9f18a0..3c92197c0a 100644 --- a/tests/integration/targets/filesystem/tasks/create_fs.yml +++ b/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 diff --git a/tests/integration/targets/filesystem/tasks/freebsd_setup.yml b/tests/integration/targets/filesystem/tasks/freebsd_setup.yml new file mode 100644 index 0000000000..e08beca4a8 --- /dev/null +++ b/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 diff --git a/tests/integration/targets/filesystem/tasks/main.yml b/tests/integration/targets/filesystem/tasks/main.yml index d836c8a15d..4b2c5bdc2a 100644 --- a/tests/integration/targets/filesystem/tasks/main.yml +++ b/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 }}" diff --git a/tests/integration/targets/filesystem/tasks/overwrite_another_fs.yml b/tests/integration/targets/filesystem/tasks/overwrite_another_fs.yml index 4bf92836bb..83a623fa75 100644 --- a/tests/integration/targets/filesystem/tasks/overwrite_another_fs.yml +++ b/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 diff --git a/tests/integration/targets/filesystem/tasks/remove_fs.yml b/tests/integration/targets/filesystem/tasks/remove_fs.yml index 338d439d60..3127dce559 100644 --- a/tests/integration/targets/filesystem/tasks/remove_fs.yml +++ b/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