diff --git a/changelogs/fragments/2472_filesystem_module_revamp.yml b/changelogs/fragments/2472_filesystem_module_revamp.yml new file mode 100644 index 0000000000..691c861078 --- /dev/null +++ b/changelogs/fragments/2472_filesystem_module_revamp.yml @@ -0,0 +1,9 @@ +--- +minor_changes: + - "filesystem - cleanup and revamp module, tests and doc. Pass all commands to + ``module.run_command()`` as lists. Move the device-vs-mountpoint logic to + ``grow()`` method. Give to all ``get_fs_size()`` the same logic and error + handling. (https://github.com/ansible-collections/community.general/pull/2472)." +bugfixes: + - "filesystem - repair ``reiserfs`` fstype support after adding it to integration + tests (https://github.com/ansible-collections/community.general/pull/2472)." diff --git a/plugins/modules/system/filesystem.py b/plugins/modules/system/filesystem.py index 6944178da1..97fe2dc1ab 100644 --- a/plugins/modules/system/filesystem.py +++ b/plugins/modules/system/filesystem.py @@ -7,10 +7,11 @@ from __future__ import absolute_import, division, print_function __metaclass__ = type + DOCUMENTATION = ''' --- author: -- Alexander Bulimov (@abulimov) + - Alexander Bulimov (@abulimov) module: filesystem short_description: Makes a filesystem description: @@ -18,13 +19,12 @@ description: options: state: description: - - If C(state=present), the filesystem is created if it doesn't already - exist, that is the default behaviour if I(state) is omitted. - - If C(state=absent), filesystem signatures on I(dev) are wiped if it - contains a filesystem (as known by C(blkid)). - - When C(state=absent), all other options but I(dev) are ignored, and the - module doesn't fail if the device I(dev) doesn't actually exist. - - C(state=absent) is not supported and will fail on FreeBSD systems. + - If C(state=present), the filesystem is created if it doesn't already + exist, that is the default behaviour if I(state) is omitted. + - If C(state=absent), filesystem signatures on I(dev) are wiped if it + contains a filesystem (as known by C(blkid)). + - When C(state=absent), all other options but I(dev) are ignored, and the + module doesn't fail if the device I(dev) doesn't actually exist. type: str choices: [ present, absent ] default: present @@ -32,48 +32,56 @@ options: fstype: choices: [ btrfs, ext2, ext3, ext4, ext4dev, f2fs, lvm, ocfs2, reiserfs, xfs, vfat, swap ] 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 + - 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 type: str aliases: [type] dev: description: - - Target path to device or image file. + - 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. type: path required: yes aliases: [device] force: description: - - If C(yes), allows to create new filesystem on devices that already has filesystem. + - If C(yes), allows to create new filesystem on devices that already has filesystem. type: bool default: 'no' 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. - 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 - not supported on FreeBSD systems. - - vFAT will likely fail if fatresize < 1.04. + - 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. + 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 + not supported on FreeBSD systems. + - vFAT will likely fail if fatresize < 1.04. type: bool default: 'no' opts: description: - - List of options to be passed to mkfs command. + - List of options to be passed to mkfs command. type: str requirements: - - Uses tools related to the I(fstype) (C(mkfs)) and C(blkid) command. When I(resizefs) is enabled, C(blockdev) command is required too. + - 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. 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). + - 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. - This module supports I(check_mode). ''' @@ -102,6 +110,7 @@ import re import stat from ansible.module_utils.basic import AnsibleModule +from ansible.module_utils._text import to_native class Device(object): @@ -114,13 +123,15 @@ class Device(object): statinfo = os.stat(self.path) if stat.S_ISBLK(statinfo.st_mode): blockdev_cmd = self.module.get_bin_path("blockdev", required=True) - dummy, devsize_in_bytes, dummy = self.module.run_command([blockdev_cmd, "--getsize64", self.path], check_rc=True) - return int(devsize_in_bytes) + dummy, out, dummy = self.module.run_command([blockdev_cmd, "--getsize64", self.path], check_rc=True) + devsize_in_bytes = int(out) elif os.path.isfile(self.path): - return os.path.getsize(self.path) + devsize_in_bytes = os.path.getsize(self.path) else: self.module.fail_json(changed=False, msg="Target device not supported: %s" % self) + return devsize_in_bytes + def get_mountpoint(self): """Return (first) mountpoint of device. Returns None when not mounted.""" cmd_findmnt = self.module.get_bin_path("findmnt", required=True) @@ -141,9 +152,12 @@ class Device(object): class Filesystem(object): - GROW = None MKFS = None - MKFS_FORCE_FLAGS = '' + MKFS_FORCE_FLAGS = [] + INFO = None + GROW = None + GROW_MAX_SPACE_FLAGS = [] + GROW_MOUNTPOINT_ONLY = False LANG_ENV = {'LANG': 'C', 'LC_ALL': 'C', 'LC_MESSAGES': 'C'} @@ -155,7 +169,11 @@ class Filesystem(object): return type(self).__name__ def get_fs_size(self, dev): - """ Return size in bytes of filesystem on device. Returns int """ + """Return size in bytes of filesystem on device (integer). + Should query the info with a per-fstype command that can access the + device whenever it is mounted or not, and parse the command output. + Parser must ensure to return an integer, or raise a ValueError. + """ raise NotImplementedError() def create(self, opts, dev): @@ -163,31 +181,27 @@ class Filesystem(object): return mkfs = self.module.get_bin_path(self.MKFS, required=True) - if opts is None: - cmd = "%s %s '%s'" % (mkfs, self.MKFS_FORCE_FLAGS, dev) - else: - cmd = "%s %s %s '%s'" % (mkfs, self.MKFS_FORCE_FLAGS, opts, dev) + cmd = [mkfs] + self.MKFS_FORCE_FLAGS + opts + [str(dev)] self.module.run_command(cmd, check_rc=True) def wipefs(self, dev): - if platform.system() == 'FreeBSD': - msg = "module param state=absent is currently not supported on this OS (FreeBSD)." - self.module.fail_json(msg=msg) - if self.module.check_mode: return # wipefs comes with util-linux package (as 'blockdev' & 'findmnt' above) - # so it is not supported on FreeBSD. Even the use of dd as a fallback is + # that is ported to FreeBSD. The use of dd as a portable fallback is # not doable here if it needs get_mountpoint() (to prevent corruption of - # a mounted filesystem), since 'findmnt' is not available on FreeBSD. + # a mounted filesystem), since 'findmnt' is not available on FreeBSD, + # even in util-linux port for this OS. wipefs = self.module.get_bin_path('wipefs', required=True) - cmd = [wipefs, "--all", dev.__str__()] + cmd = [wipefs, "--all", str(dev)] self.module.run_command(cmd, check_rc=True) - def grow_cmd(self, dev): - cmd = self.module.get_bin_path(self.GROW, required=True) - return [cmd, str(dev)] + def grow_cmd(self, target): + """Build and return the resizefs commandline as list.""" + cmdline = [self.module.get_bin_path(self.GROW, required=True)] + cmdline += self.GROW_MAX_SPACE_FLAGS + [target] + return cmdline def grow(self, dev): """Get dev and fs size and compare. Returns stdout of used command.""" @@ -196,31 +210,50 @@ class Filesystem(object): try: fssize_in_bytes = self.get_fs_size(dev) except NotImplementedError: - self.module.fail_json(changed=False, msg="module does not support resizing %s filesystem yet." % self.fstype) + self.module.fail_json(msg="module does not support resizing %s filesystem yet" % self.fstype) + except ValueError as err: + self.module.warn("unable to process %s output '%s'" % (self.INFO, to_native(err))) + self.module.fail_json(msg="unable to process %s output for %s" % (self.INFO, dev)) if not fssize_in_bytes < devsize_in_bytes: self.module.exit_json(changed=False, msg="%s filesystem is using the whole device %s" % (self.fstype, dev)) elif self.module.check_mode: - self.module.exit_json(changed=True, msg="Resizing filesystem %s on device %s" % (self.fstype, dev)) + self.module.exit_json(changed=True, msg="resizing filesystem %s on device %s" % (self.fstype, dev)) + + if self.GROW_MOUNTPOINT_ONLY: + mountpoint = dev.get_mountpoint() + if not mountpoint: + self.module.fail_json(msg="%s needs to be mounted for %s operations" % (dev, self.fstype)) + grow_target = mountpoint else: - dummy, out, dummy = self.module.run_command(self.grow_cmd(dev), check_rc=True) - return out + grow_target = str(dev) + + dummy, out, dummy = self.module.run_command(self.grow_cmd(grow_target), check_rc=True) + return out class Ext(Filesystem): - MKFS_FORCE_FLAGS = '-F' + MKFS_FORCE_FLAGS = ['-F'] + INFO = 'tune2fs' GROW = 'resize2fs' def get_fs_size(self, dev): - cmd = self.module.get_bin_path('tune2fs', required=True) - # Get Block count and Block size - dummy, size, dummy = self.module.run_command([cmd, '-l', str(dev)], check_rc=True, environ_update=self.LANG_ENV) - for line in size.splitlines(): + """Get Block count and Block size and return their product.""" + cmd = self.module.get_bin_path(self.INFO, required=True) + dummy, out, dummy = self.module.run_command([cmd, '-l', str(dev)], check_rc=True, environ_update=self.LANG_ENV) + + block_count = block_size = None + for line in out.splitlines(): if 'Block count:' in line: block_count = int(line.split(':')[1].strip()) elif 'Block size:' in line: block_size = int(line.split(':')[1].strip()) - return block_size * block_count + if None not in (block_size, block_count): + break + else: + raise ValueError(out) + + return block_size * block_count class Ext2(Ext): @@ -237,52 +270,46 @@ class Ext4(Ext): class XFS(Filesystem): MKFS = 'mkfs.xfs' - MKFS_FORCE_FLAGS = '-f' + MKFS_FORCE_FLAGS = ['-f'] + INFO = 'xfs_info' GROW = 'xfs_growfs' + GROW_MOUNTPOINT_ONLY = True def get_fs_size(self, dev): - cmd = self.module.get_bin_path('xfs_info', required=True) + """Get bsize and blocks and return their product.""" + cmdline = [self.module.get_bin_path(self.INFO, required=True)] + # Depending on the versions, xfs_info is able to get info from the + # device, whenever it is mounted or not, or only if unmounted, or + # only if mounted, or not at all. For any version until now, it is + # able to query info from the mountpoint. So try it first, and use + # device as the last resort: it may or may not work. mountpoint = dev.get_mountpoint() if mountpoint: - rc, out, err = self.module.run_command([cmd, str(mountpoint)], environ_update=self.LANG_ENV) + cmdline += [mountpoint] else: - # Recent GNU/Linux distros support access to unmounted XFS filesystems - rc, out, err = self.module.run_command([cmd, str(dev)], environ_update=self.LANG_ENV) - if rc != 0: - self.module.fail_json(msg="Error while attempting to query size of XFS filesystem: %s" % err) + cmdline += [str(dev)] + dummy, out, dummy = self.module.run_command(cmdline, check_rc=True, environ_update=self.LANG_ENV) + block_size = block_count = None for line in out.splitlines(): col = line.split('=') if col[0].strip() == 'data': - if col[1].strip() != 'bsize': - self.module.fail_json(msg='Unexpected output format from xfs_info (could not locate "bsize")') - if col[2].split()[1] != 'blocks': - self.module.fail_json(msg='Unexpected output format from xfs_info (could not locate "blocks")') - block_size = int(col[2].split()[0]) - block_count = int(col[3].split(',')[0]) - return block_size * block_count + if col[1].strip() == 'bsize': + block_size = int(col[2].split()[0]) + if col[2].split()[1] == 'blocks': + block_count = int(col[3].split(',')[0]) + if None not in (block_size, block_count): + break + else: + raise ValueError(out) - def grow_cmd(self, dev): - # Check first if growing is needed, and then if it is doable or not. - devsize_in_bytes = dev.size() - fssize_in_bytes = self.get_fs_size(dev) - if not fssize_in_bytes < devsize_in_bytes: - self.module.exit_json(changed=False, msg="%s filesystem is using the whole device %s" % (self.fstype, dev)) - - mountpoint = dev.get_mountpoint() - if not mountpoint: - # xfs filesystem needs to be mounted - self.module.fail_json(msg="%s needs to be mounted for xfs operations" % dev) - - cmd = self.module.get_bin_path(self.GROW, required=True) - - return [cmd, str(mountpoint)] + return block_size * block_count class Reiserfs(Filesystem): MKFS = 'mkfs.reiserfs' - MKFS_FORCE_FLAGS = '-f' + MKFS_FORCE_FLAGS = ['-q'] class Btrfs(Filesystem): @@ -290,7 +317,8 @@ class Btrfs(Filesystem): def __init__(self, module): super(Btrfs, self).__init__(module) - dummy, stdout, stderr = self.module.run_command('%s --version' % self.MKFS, check_rc=True) + mkfs = self.module.get_bin_path(self.MKFS, required=True) + dummy, stdout, stderr = self.module.run_command([mkfs, '--version'], check_rc=True) match = re.search(r" v([0-9.]+)", stdout) if not match: # v0.20-rc1 use stderr @@ -298,29 +326,27 @@ class Btrfs(Filesystem): if match: # v0.20-rc1 doesn't have --force parameter added in following version v3.12 if LooseVersion(match.group(1)) >= LooseVersion('3.12'): - self.MKFS_FORCE_FLAGS = '-f' - else: - self.MKFS_FORCE_FLAGS = '' + self.MKFS_FORCE_FLAGS = ['-f'] else: # assume version is greater or equal to 3.12 - self.MKFS_FORCE_FLAGS = '-f' + self.MKFS_FORCE_FLAGS = ['-f'] self.module.warn('Unable to identify mkfs.btrfs version (%r, %r)' % (stdout, stderr)) class Ocfs2(Filesystem): MKFS = 'mkfs.ocfs2' - MKFS_FORCE_FLAGS = '-Fx' + MKFS_FORCE_FLAGS = ['-Fx'] class F2fs(Filesystem): MKFS = 'mkfs.f2fs' + INFO = 'dump.f2fs' GROW = 'resize.f2fs' - @property - def MKFS_FORCE_FLAGS(self): + def __init__(self, module): + super(F2fs, self).__init__(module) mkfs = self.module.get_bin_path(self.MKFS, required=True) - cmd = "%s %s" % (mkfs, os.devnull) - dummy, out, dummy = self.module.run_command(cmd, check_rc=False, environ_update=self.LANG_ENV) + dummy, out, dummy = self.module.run_command([mkfs, os.devnull], check_rc=False, environ_update=self.LANG_ENV) # Looking for " F2FS-tools: mkfs.f2fs Ver: 1.10.0 (2018-01-30)" # mkfs.f2fs displays version since v1.2.0 match = re.search(r"F2FS-tools: mkfs.f2fs Ver: ([0-9.]+) \(", out) @@ -328,69 +354,73 @@ class F2fs(Filesystem): # Since 1.9.0, mkfs.f2fs check overwrite before make filesystem # before that version -f switch wasn't used if LooseVersion(match.group(1)) >= LooseVersion('1.9.0'): - return '-f' - - return '' + self.MKFS_FORCE_FLAGS = ['-f'] def get_fs_size(self, dev): - cmd = self.module.get_bin_path('dump.f2fs', required=True) - # Get sector count and sector size - dummy, dump, dummy = self.module.run_command([cmd, str(dev)], check_rc=True, environ_update=self.LANG_ENV) - sector_size = None - sector_count = None - for line in dump.splitlines(): + """Get sector size and total FS sectors 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) + sector_size = sector_count = None + for line in out.splitlines(): if 'Info: sector size = ' in line: # expected: 'Info: sector size = 512' sector_size = int(line.split()[4]) elif 'Info: total FS sectors = ' in line: # expected: 'Info: total FS sectors = 102400 (50 MB)' sector_count = int(line.split()[5]) - if None not in (sector_size, sector_count): break else: - self.module.warn("Unable to process dump.f2fs output '%s'", '\n'.join(dump)) - self.module.fail_json(msg="Unable to process dump.f2fs output for %s" % dev) + raise ValueError(out) return sector_size * sector_count class VFAT(Filesystem): - if platform.system() == 'FreeBSD': - MKFS = "newfs_msdos" - else: - MKFS = 'mkfs.vfat' + INFO = 'fatresize' GROW = 'fatresize' + GROW_MAX_SPACE_FLAGS = ['-s', 'max'] + + def __init__(self, module): + super(VFAT, self).__init__(module) + if platform.system() == 'FreeBSD': + self.MKFS = 'newfs_msdos' + else: + self.MKFS = 'mkfs.vfat' def get_fs_size(self, dev): - cmd = self.module.get_bin_path(self.GROW, required=True) - dummy, output, dummy = self.module.run_command([cmd, '--info', str(dev)], check_rc=True, environ_update=self.LANG_ENV) - for line in output.splitlines()[1:]: + """Get and return size of filesystem, in bytes.""" + cmd = self.module.get_bin_path(self.INFO, required=True) + dummy, out, dummy = self.module.run_command([cmd, '--info', str(dev)], check_rc=True, environ_update=self.LANG_ENV) + fssize = None + for line in out.splitlines()[1:]: param, value = line.split(':', 1) if param.strip() == 'Size': - return int(value.strip()) - self.module.fail_json(msg="fatresize failed to provide filesystem size for %s" % dev) + fssize = int(value.strip()) + break + else: + raise ValueError(out) - def grow_cmd(self, dev): - cmd = self.module.get_bin_path(self.GROW) - return [cmd, "-s", str(dev.size()), str(dev.path)] + return fssize class LVM(Filesystem): MKFS = 'pvcreate' - MKFS_FORCE_FLAGS = '-f' + MKFS_FORCE_FLAGS = ['-f'] + INFO = 'pvs' GROW = 'pvresize' def get_fs_size(self, dev): - cmd = self.module.get_bin_path('pvs', required=True) + """Get and return PV size, in bytes.""" + cmd = self.module.get_bin_path(self.INFO, required=True) dummy, size, dummy = self.module.run_command([cmd, '--noheadings', '-o', 'pv_size', '--units', 'b', '--nosuffix', str(dev)], check_rc=True) - block_count = int(size) - return block_count + pv_size = int(size) + return pv_size class Swap(Filesystem): MKFS = 'mkswap' - MKFS_FORCE_FLAGS = '-f' + MKFS_FORCE_FLAGS = ['-f'] FILESYSTEMS = { @@ -439,6 +469,10 @@ def main(): force = module.params['force'] resizefs = module.params['resizefs'] + mkfs_opts = [] + if opts is not None: + mkfs_opts = opts.split() + changed = False if not os.path.exists(dev): @@ -451,7 +485,7 @@ def main(): dev = Device(module, dev) cmd = module.get_bin_path('blkid', required=True) - rc, raw_fs, err = module.run_command("%s -c /dev/null -o value -s TYPE %s" % (cmd, dev)) + 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() @@ -481,7 +515,7 @@ def main(): module.fail_json(msg="'%s' is already used as %s, use force=yes to overwrite" % (dev, fs), rc=rc, err=err) # create fs - filesystem.create(opts, dev) + filesystem.create(mkfs_opts, dev) changed = True elif fs: diff --git a/tests/integration/targets/filesystem/defaults/main.yml b/tests/integration/targets/filesystem/defaults/main.yml index 764b98b6ba..15ef85aa0e 100644 --- a/tests/integration/targets/filesystem/defaults/main.yml +++ b/tests/integration/targets/filesystem/defaults/main.yml @@ -17,7 +17,9 @@ tested_filesystems: ext2: {fssize: 10, grow: True} xfs: {fssize: 20, grow: False} # grow requires a mounted filesystem btrfs: {fssize: 150, grow: False} # grow not implemented + reiserfs: {fssize: 33, grow: False} # grow not implemented vfat: {fssize: 20, grow: True} ocfs2: {fssize: '{{ ocfs2_fssize }}', grow: False} # grow not implemented 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 diff --git a/tests/integration/targets/filesystem/tasks/create_device.yml b/tests/integration/targets/filesystem/tasks/create_device.yml index e49861e7ca..30fd62e33a 100644 --- a/tests/integration/targets/filesystem/tasks/create_device.yml +++ b/tests/integration/targets/filesystem/tasks/create_device.yml @@ -1,6 +1,9 @@ --- - name: 'Create a "disk" file' - command: 'dd if=/dev/zero of={{ image_file }} bs=1M count={{ fssize }}' + community.general.filesize: + path: '{{ image_file }}' + size: '{{ fssize }}M' + force: true - vars: dev: '{{ image_file }}' @@ -8,26 +11,29 @@ - when: fstype == 'lvm' block: - name: 'Create a loop device for LVM' - command: 'losetup --show -f {{ dev }}' + ansible.builtin.command: + cmd: 'losetup --show -f {{ dev }}' register: loop_device_cmd - - set_fact: + - name: 'Switch to loop device target for further tasks' + ansible.builtin.set_fact: dev: "{{ loop_device_cmd.stdout }}" - include_tasks: '{{ action }}.yml' always: - name: 'Detach loop device used for LVM' - command: 'losetup -d {{ dev }}' - args: + ansible.builtin.command: + cmd: 'losetup -d {{ dev }}' removes: '{{ dev }}' when: fstype == 'lvm' - name: 'Clean correct device for LVM' - set_fact: + ansible.builtin.set_fact: dev: '{{ image_file }}' when: fstype == 'lvm' - - file: + - name: 'Remove disk image file' + ansible.builtin.file: name: '{{ image_file }}' state: absent diff --git a/tests/integration/targets/filesystem/tasks/create_fs.yml b/tests/integration/targets/filesystem/tasks/create_fs.yml index 688a4462db..de1a9f18a0 100644 --- a/tests/integration/targets/filesystem/tasks/create_fs.yml +++ b/tests/integration/targets/filesystem/tasks/create_fs.yml @@ -1,43 +1,58 @@ -- name: filesystem creation - filesystem: +--- +- name: "Create filesystem" + community.general.filesystem: dev: '{{ dev }}' fstype: '{{ fstype }}' register: fs_result -- assert: +- name: "Assert that results are as expected" + ansible.builtin.assert: that: - 'fs_result is changed' - 'fs_result is success' -- command: 'blkid -c /dev/null -o value -s UUID {{ dev }}' +- name: "Get UUID of created filesystem" + ansible.builtin.command: + cmd: 'blkid -c /dev/null -o value -s UUID {{ dev }}' + changed_when: false register: uuid - name: "Check that filesystem isn't created if force isn't used" - filesystem: + community.general.filesystem: dev: '{{ dev }}' fstype: '{{ fstype }}' register: fs2_result -- command: 'blkid -c /dev/null -o value -s UUID {{ dev }}' +- name: "Get UUID of the filesystem" + ansible.builtin.command: + cmd: 'blkid -c /dev/null -o value -s UUID {{ dev }}' + changed_when: false register: uuid2 -- assert: +- name: "Assert that filesystem UUID is not changed" + ansible.builtin.assert: that: - - 'not (fs2_result is changed)' + - 'fs2_result is not changed' - 'fs2_result is success' - 'uuid.stdout == uuid2.stdout' -- name: Check that filesystem is recreated if force is used - filesystem: +- name: "Check that filesystem is recreated if force is used" + community.general.filesystem: dev: '{{ dev }}' fstype: '{{ fstype }}' force: yes register: fs3_result -- command: 'blkid -c /dev/null -o value -s UUID {{ dev }}' +- name: "Get UUID of the new filesystem" + ansible.builtin.command: + cmd: 'blkid -c /dev/null -o value -s UUID {{ dev }}' + changed_when: false register: uuid3 -- assert: +- name: "Assert that filesystem UUID is changed" + # libblkid gets no UUID at all for this fstype on FreeBSD + when: not (ansible_system == 'FreeBSD' and fstype == 'reiserfs') + ansible.builtin.assert: that: - 'fs3_result is changed' - 'fs3_result is success' @@ -46,24 +61,31 @@ - when: 'grow|bool and (fstype != "vfat" or resize_vfat)' block: - - name: increase fake device - shell: 'dd if=/dev/zero bs=1M count=1 >> {{ image_file }}' + - name: "Increase fake device" + community.general.filesize: + path: '{{ image_file }}' + size: '{{ fssize | int + 1 }}M' - - name: Resize loop device for LVM - command: losetup -c {{ dev }} + - name: "Resize loop device for LVM" + ansible.builtin.command: + cmd: 'losetup -c {{ dev }}' when: fstype == 'lvm' - - name: Expand filesystem - filesystem: + - name: "Expand filesystem" + community.general.filesystem: dev: '{{ dev }}' fstype: '{{ fstype }}' resizefs: yes register: fs4_result - - command: 'blkid -c /dev/null -o value -s UUID {{ dev }}' + - name: "Get UUID of the filesystem" + ansible.builtin.command: + cmd: 'blkid -c /dev/null -o value -s UUID {{ dev }}' + changed_when: false register: uuid4 - - assert: + - name: "Assert that filesystem UUID is not changed" + ansible.builtin.assert: that: - 'fs4_result is changed' - 'fs4_result is success' @@ -74,14 +96,15 @@ (fstype == "xfs" and ansible_system == "Linux" and ansible_distribution not in ["CentOS", "Ubuntu"]) block: - - name: Check that resizefs does nothing if device size is not changed - filesystem: + - name: "Check that resizefs does nothing if device size is not changed" + community.general.filesystem: dev: '{{ dev }}' fstype: '{{ fstype }}' resizefs: yes register: fs5_result - - assert: + - name: "Assert that the state did not change" + ansible.builtin.assert: that: - 'fs5_result is not changed' - 'fs5_result is succeeded' diff --git a/tests/integration/targets/filesystem/tasks/main.yml b/tests/integration/targets/filesystem/tasks/main.yml index 44e8c49f61..d836c8a15d 100644 --- a/tests/integration/targets/filesystem/tasks/main.yml +++ b/tests/integration/targets/filesystem/tasks/main.yml @@ -4,9 +4,9 @@ # and should not be used as examples of how to write Ansible roles # #################################################################### -- debug: +- ansible.builtin.debug: msg: '{{ role_name }}' -- debug: +- ansible.builtin.debug: msg: '{{ role_path|basename }}' - import_tasks: setup.yml @@ -27,29 +27,35 @@ grow: '{{ item.0.value.grow }}' action: '{{ item.1 }}' when: - - 'not (item.0.key == "btrfs" and ansible_system == "FreeBSD")' # btrfs not available on FreeBSD - # On Ubuntu trusty, blkid is unable to identify filesystem smaller than 256Mo, see - # https://www.kernel.org/pub/linux/utils/util-linux/v2.21/v2.21-ChangeLog - # https://anonscm.debian.org/cgit/collab-maint/pkg-util-linux.git/commit/?id=04f7020eadf31efc731558df92daa0a1c336c46c - - 'not (item.0.key == "btrfs" and (ansible_distribution == "Ubuntu" and ansible_distribution_release == "trusty"))' - - 'not (item.0.key == "btrfs" and (ansible_facts.os_family == "RedHat" and ansible_facts.distribution_major_version is version("8", ">=")))' - - 'not (item.0.key == "lvm" and ansible_system == "FreeBSD")' # LVM not available on FreeBSD - - 'not (item.0.key == "lvm" and ansible_virtualization_type in ["docker", "container", "containerd"])' # Tests use losetup which can not be used inside unprivileged container - - 'not (item.0.key == "ocfs2" and ansible_os_family != "Debian")' # ocfs2 only available on Debian based distributions - - 'not (item.0.key == "f2fs" and ansible_system == "FreeBSD")' - # f2fs-tools package not available with RHEL/CentOS - - 'not (item.0.key == "f2fs" and ansible_distribution in ["CentOS", "RedHat"])' - # On Ubuntu trusty, blkid (2.20.1) is unable to identify F2FS filesystem. blkid handles F2FS since v2.23, see: - # https://mirrors.edge.kernel.org/pub/linux/utils/util-linux/v2.23/v2.23-ReleaseNotes - - 'not (item.0.key == "f2fs" and ansible_distribution == "Ubuntu" and ansible_distribution_version is version("14.04", "<="))' - - 'not (item.1 == "overwrite_another_fs" and ansible_system == "FreeBSD")' + # FreeBSD limited support + # Not available: btrfs, lvm, f2fs, ocfs2 + # All BSD systems use swap fs, but only Linux needs mkswap + # Supported: ext2/3/4 (e2fsprogs), xfs (xfsprogs), reiserfs (progsreiserfs), vfat + - 'not (ansible_system == "FreeBSD" and item.0.key in ["btrfs", "f2fs", "swap", "lvm", "ocfs2"])' + # 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"])' + + # Other limitations and corner cases + + # f2fs-tools and reiserfs-utils packages not available with RHEL/CentOS on CI + - 'not (ansible_distribution in ["CentOS", "RedHat"] and item.0.key in ["f2fs", "reiserfs"])' + - 'not (ansible_os_family == "RedHat" and ansible_distribution_major_version is version("8", ">=") and + item.0.key == "btrfs")' + # ocfs2 only available on Debian based distributions + - 'not (item.0.key == "ocfs2" and ansible_os_family != "Debian")' + # Tests use losetup which can not be used inside unprivileged container + - 'not (item.0.key == "lvm" and ansible_virtualization_type in ["docker", "container", "containerd"])' - - 'not (item.1 == "remove_fs" and ansible_system == "FreeBSD")' # util-linux not available on FreeBSD # On CentOS 6 shippable containers, wipefs seems unable to remove vfat signatures - - 'not (item.1 == "remove_fs" and item.0.key == "vfat" and ansible_distribution == "CentOS" and - ansible_distribution_version is version("7.0", "<"))' + - 'not (ansible_distribution == "CentOS" and ansible_distribution_version is version("7.0", "<") and + item.1 == "remove_fs" and item.0.key == "vfat")' + # On same systems, mkfs.minix (unhandled by the module) can't find the device/file + - 'not (ansible_distribution == "CentOS" and ansible_distribution_version is version("7.0", "<") and + item.1 == "overwrite_another_fs")' # The xfsprogs package on newer versions of OpenSUSE (15+) require Python 3, we skip this on our Python 2 container # OpenSUSE 42.3 Python2 and the other py3 containers are not affected so we will continue to run that - - 'not (item.0.key == "xfs" and ansible_os_family == "Suse" and ansible_python.version.major == 2 and ansible_distribution_major_version|int != 42)' + - 'not (ansible_os_family == "Suse" and ansible_distribution_major_version|int != 42 and + item.0.key == "xfs" and ansible_python.version.major == 2)' + 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 671d9b0bea..4bf92836bb 100644 --- a/tests/integration/targets/filesystem/tasks/overwrite_another_fs.yml +++ b/tests/integration/targets/filesystem/tasks/overwrite_another_fs.yml @@ -1,40 +1,55 @@ --- - name: 'Recreate "disk" file' - command: 'dd if=/dev/zero of={{ image_file }} bs=1M count={{ fssize }}' + community.general.filesize: + path: '{{ image_file }}' + size: '{{ fssize }}M' + force: true -- name: 'Create a swap filesystem' - command: 'mkswap {{ dev }}' +- name: 'Create a minix filesystem' + ansible.builtin.command: + cmd: 'mkfs.minix {{ dev }}' -- command: 'blkid -c /dev/null -o value -s UUID {{ dev }}' +- name: 'Get UUID of the new filesystem' + ansible.builtin.command: + cmd: 'blkid -c /dev/null -o value -s UUID {{ dev }}' + changed_when: false register: uuid - name: "Check that an existing filesystem (not handled by this module) isn't overwritten when force isn't used" - filesystem: + community.general.filesystem: dev: '{{ dev }}' fstype: '{{ fstype }}' register: fs_result ignore_errors: True -- command: 'blkid -c /dev/null -o value -s UUID {{ dev }}' +- name: 'Get UUID of the filesystem' + ansible.builtin.command: + cmd: 'blkid -c /dev/null -o value -s UUID {{ dev }}' + changed_when: false register: uuid2 -- assert: +- name: 'Assert that module failed and filesystem UUID is not changed' + ansible.builtin.assert: that: - 'fs_result is failed' - 'uuid.stdout == uuid2.stdout' - name: "Check that an existing filesystem (not handled by this module) is overwritten when force is used" - filesystem: + community.general.filesystem: dev: '{{ dev }}' fstype: '{{ fstype }}' force: yes register: fs_result2 -- command: 'blkid -c /dev/null -o value -s UUID {{ dev }}' +- name: 'Get UUID of the new filesystem' + ansible.builtin.command: + cmd: 'blkid -c /dev/null -o value -s UUID {{ dev }}' + changed_when: false register: uuid3 -- assert: +- name: 'Assert that module succeeded and filesystem UUID is changed' + ansible.builtin.assert: that: - - 'fs_result2 is successful' + - 'fs_result2 is success' - 'fs_result2 is changed' - 'uuid2.stdout != uuid3.stdout' diff --git a/tests/integration/targets/filesystem/tasks/remove_fs.yml b/tests/integration/targets/filesystem/tasks/remove_fs.yml index 7d1ca2a19c..338d439d60 100644 --- a/tests/integration/targets/filesystem/tasks/remove_fs.yml +++ b/tests/integration/targets/filesystem/tasks/remove_fs.yml @@ -1,98 +1,98 @@ --- # We assume 'create_fs' tests have passed. -- name: filesystem creation - filesystem: +- name: "Create filesystem" + community.general.filesystem: dev: '{{ dev }}' fstype: '{{ fstype }}' -- name: get filesystem UUID with 'blkid' - command: +- name: "Get filesystem UUID with 'blkid'" + ansible.builtin.command: cmd: 'blkid -c /dev/null -o value -s UUID {{ dev }}' changed_when: false register: blkid_ref -- name: Assert that a filesystem exists on top of the device - assert: +- name: "Assert that a filesystem exists on top of the device" + ansible.builtin.assert: that: - blkid_ref.stdout | length > 0 # Test check_mode first -- name: filesystem removal (check mode) - filesystem: +- name: "Remove filesystem (check mode)" + community.general.filesystem: dev: '{{ dev }}' state: absent register: wipefs check_mode: yes -- name: get filesystem UUID with 'blkid' (should remain the same) - command: +- name: "Get filesystem UUID with 'blkid' (should remain the same)" + ansible.builtin.command: cmd: 'blkid -c /dev/null -o value -s UUID {{ dev }}' changed_when: false register: blkid -- name: Assert that the state changed but the filesystem still exists - assert: +- name: "Assert that the state changed but the filesystem still exists" + ansible.builtin.assert: that: - wipefs is changed - blkid.stdout == blkid_ref.stdout # Do it -- name: filesystem removal - filesystem: +- name: "Remove filesystem" + community.general.filesystem: dev: '{{ dev }}' state: absent register: wipefs -- name: get filesystem UUID with 'blkid' (should be empty) - command: +- name: "Get filesystem UUID with 'blkid' (should be empty)" + ansible.builtin.command: cmd: 'blkid -c /dev/null -o value -s UUID {{ dev }}' changed_when: false failed_when: false register: blkid -- name: Assert that the state changed and the device has no filesystem - assert: +- name: "Assert that the state changed and the device has no filesystem" + ansible.builtin.assert: that: - wipefs is changed - blkid.stdout | length == 0 - blkid.rc == 2 # Do it again -- name: filesystem removal (idempotency) - filesystem: +- name: "Remove filesystem (idempotency)" + community.general.filesystem: dev: '{{ dev }}' state: absent register: wipefs -- name: Assert that the state did not change - assert: +- name: "Assert that the state did not change" + ansible.builtin.assert: that: - wipefs is not changed # and again -- name: filesystem removal (idempotency, check mode) - filesystem: +- name: "Remove filesystem (idempotency, check mode)" + community.general.filesystem: dev: '{{ dev }}' state: absent register: wipefs check_mode: yes -- name: Assert that the state did not change - assert: +- name: "Assert that the state did not change" + ansible.builtin.assert: that: - wipefs is not changed # By the way, test removal of a filesystem on unexistent device -- name: filesystem removal (unexistent device) - filesystem: +- name: "Remove filesystem (unexistent device)" + community.general.filesystem: dev: '/dev/unexistent_device' state: absent register: wipefs -- name: Assert that the state did not change - assert: +- name: "Assert that the state did not change" + ansible.builtin.assert: that: - wipefs is not changed diff --git a/tests/integration/targets/filesystem/tasks/setup.yml b/tests/integration/targets/filesystem/tasks/setup.yml index 82fe7c54e6..9ca4b983d0 100644 --- a/tests/integration/targets/filesystem/tasks/setup.yml +++ b/tests/integration/targets/filesystem/tasks/setup.yml @@ -1,6 +1,9 @@ --- -- name: install filesystem tools - package: +# By installing e2fsprogs on FreeBSD, we get a usable blkid command, but this +# package conflicts with util-linux, that provides blkid too, but also wipefs +# (required for filesystem state=absent). +- name: "Install filesystem tools" + ansible.builtin.package: name: '{{ item }}' state: present # xfsprogs on OpenSUSE requires Python 3, skip this for our newer Py2 OpenSUSE builds @@ -9,86 +12,134 @@ - e2fsprogs - xfsprogs -- block: - - name: install btrfs progs - package: - name: btrfs-progs - state: present - when: - - ansible_os_family != 'Suse' - - not (ansible_distribution == 'Ubuntu' and ansible_distribution_version is version('16.04', '<=')) - - ansible_system != "FreeBSD" - - not (ansible_facts.os_family == "RedHat" and ansible_facts.distribution_major_version is version('8', '>=')) +- name: "Install btrfs progs" + ansible.builtin.package: + name: btrfs-progs + state: present + when: + - ansible_os_family != 'Suse' + - not (ansible_distribution == 'Ubuntu' and ansible_distribution_version is version('16.04', '<=')) + - ansible_system != "FreeBSD" + - not (ansible_facts.os_family == "RedHat" and ansible_facts.distribution_major_version is version('8', '>=')) - - name: install btrfs progs (Ubuntu <= 16.04) - package: - name: btrfs-tools - state: present - when: ansible_distribution == 'Ubuntu' and ansible_distribution_version is version('16.04', '<=') +- name: "Install btrfs tools (Ubuntu <= 16.04)" + ansible.builtin.package: + name: btrfs-tools + state: present + when: + - ansible_distribution == 'Ubuntu' + - ansible_distribution_version is version('16.04', '<=') - - name: install btrfs progs (OpenSuse) - package: - name: '{{ item }}' - state: present - when: ansible_os_family == 'Suse' - with_items: - - python{{ ansible_python.version.major }}-xml - - btrfsprogs +- name: "Install btrfs progs (OpenSuse)" + ansible.builtin.package: + name: '{{ item }}' + state: present + when: ansible_os_family == 'Suse' + with_items: + - python{{ ansible_python.version.major }}-xml + - btrfsprogs - - name: install ocfs2 (Debian) - package: - name: ocfs2-tools - state: present - when: ansible_os_family == 'Debian' +- name: "Install reiserfs utils (Fedora)" + ansible.builtin.package: + name: reiserfs-utils + state: present + when: + - ansible_distribution == 'Fedora' - - when: - - ansible_os_family != 'RedHat' or ansible_distribution == 'Fedora' - - ansible_distribution != 'Ubuntu' or ansible_distribution_version is version('16.04', '>=') - - ansible_system != "FreeBSD" - block: - - name: install f2fs - package: - name: f2fs-tools - state: present +- name: "Install reiserfs (OpenSuse)" + ansible.builtin.package: + name: reiserfs + state: present + when: + - ansible_os_family == 'Suse' - - name: fetch f2fs version - command: mkfs.f2fs /dev/null - ignore_errors: yes - register: mkfs_f2fs +- name: "Install reiserfs progs (Debian and more)" + ansible.builtin.package: + name: reiserfsprogs + state: present + when: + - ansible_system == 'Linux' + - ansible_os_family not in ['Suse', 'RedHat'] - - set_fact: - f2fs_version: '{{ mkfs_f2fs.stdout | regex_search("F2FS-tools: mkfs.f2fs Ver:.*") | regex_replace("F2FS-tools: mkfs.f2fs Ver: ([0-9.]+) .*", "\1") }}' +- name: "Install reiserfs progs (FreeBSD)" + ansible.builtin.package: + name: progsreiserfs + state: present + when: + - ansible_system == 'FreeBSD' - - name: install dosfstools and lvm2 (Linux) - package: - name: '{{ item }}' - with_items: - - dosfstools - - lvm2 - when: ansible_system == 'Linux' +- name: "Install ocfs2 (Debian)" + ansible.builtin.package: + name: ocfs2-tools + state: present + when: ansible_os_family == 'Debian' -- block: - - name: install fatresize - package: - name: fatresize - state: present - - command: fatresize --help - register: fatresize - - set_fact: - fatresize_version: '{{ fatresize.stdout_lines[0] | regex_search("[0-9]+\.[0-9]+\.[0-9]+") }}' +- name: "Install f2fs tools and get version" + when: + - ansible_os_family != 'RedHat' or ansible_distribution == 'Fedora' + - ansible_distribution != 'Ubuntu' or ansible_distribution_version is version('16.04', '>=') + - ansible_system != "FreeBSD" + block: + - name: "Install f2fs tools" + ansible.builtin.package: + name: f2fs-tools + state: present + + - name: "Fetch f2fs version" + ansible.builtin.command: + cmd: mkfs.f2fs /dev/null + changed_when: false + ignore_errors: true + register: mkfs_f2fs + + - name: "Record f2fs_version" + ansible.builtin.set_fact: + f2fs_version: '{{ mkfs_f2fs.stdout + | regex_search("F2FS-tools: mkfs.f2fs Ver:.*") + | regex_replace("F2FS-tools: mkfs.f2fs Ver: ([0-9.]+) .*", "\1") }}' + +- name: "Install dosfstools and lvm2 (Linux)" + ansible.builtin.package: + name: '{{ item }}' + with_items: + - dosfstools + - lvm2 + when: ansible_system == 'Linux' + +- name: "Install fatresize and get version" when: - ansible_system == 'Linux' - ansible_os_family != 'Suse' - ansible_os_family != 'RedHat' or (ansible_distribution == 'CentOS' and ansible_distribution_version is version('7.0', '==')) + block: + - name: "Install fatresize" + ansible.builtin.package: + name: fatresize + state: present -- command: mke2fs -V + - name: "Fetch fatresize version" + ansible.builtin.command: + cmd: fatresize --help + changed_when: false + register: fatresize + + - name: "Record fatresize_version" + ansible.builtin.set_fact: + fatresize_version: '{{ fatresize.stdout_lines[0] | regex_search("[0-9]+\.[0-9]+\.[0-9]+") }}' + +- name: "Fetch e2fsprogs version" + ansible.builtin.command: + cmd: mke2fs -V + changed_when: false register: mke2fs -- set_fact: +- name: "Record e2fsprogs_version" + ansible.builtin.set_fact: # mke2fs 1.43.6 (29-Aug-2017) e2fsprogs_version: '{{ mke2fs.stderr_lines[0] | regex_search("[0-9]{1,2}\.[0-9]{1,2}(\.[0-9]{1,2})?") }}' -- set_fact: +- name: "Set version-related facts to skip further tasks" + ansible.builtin.set_fact: # http://e2fsprogs.sourceforge.net/e2fsprogs-release.html#1.43 # Mke2fs no longer complains if the user tries to create a file system # using the entire block device.