1
0
Fork 0
mirror of https://github.com/ansible-collections/community.general.git synced 2024-09-14 20:13:21 +02:00

filesystem: fix 355 state absent (#1149)

* add support for filesystem removal (fix #355)

- Add 'state' option, defaults to 'present'.
- When state=absent, ignore other options (even 'dev' if the device
  doesn't exist)

* test filesystem state=absent (+ check_mode + idempotency)

* fix doc-required-mismatch

* add changelog fragment

* fix blkid return code

* ext4dev may be deprecated

* base checks on UUID instead

* Update changelogs/fragments/1149-filesystem-fix-355-state-absent.yml

Co-authored-by: Felix Fontein <felix@fontein.de>

* Update plugins/modules/system/filesystem.py (version_added)

Co-authored-by: Felix Fontein <felix@fontein.de>

* use array for new run_command() calls; do not wipefs if no fs found

* use dd as a fallback

* do not use bare 'except' (pep8)

* force string type

* use dd anyway (wipefs not supported everywhere, possibly buggy with vfat, etc.)

* do not truncate regular files; update changelog fragment

* doc: update state description and an example; notice check_mode support

* do not wipe mounted fs, fail instead

* back to wipefs implementation

* update test's main conditions

* update changelog fragment

* explicit types

* fail state=absent on freebsd

* remove doc-missing-type exceptions (2.9, 2.10, 2.11)

Co-authored-by: Felix Fontein <felix@fontein.de>
This commit is contained in:
quidame 2020-10-26 18:43:01 +00:00 committed by GitHub
parent c776387daa
commit a5ca990857
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
7 changed files with 194 additions and 34 deletions

View file

@ -0,0 +1,4 @@
---
bugfixes:
- filesystem - add option ``state`` with default ``present``. When set to ``absent``, filesystem signatures are removed
(https://github.com/ansible-collections/community.general/issues/355).

View file

@ -16,10 +16,24 @@ short_description: Makes a filesystem
description: description:
- This module creates a filesystem. - This module creates a filesystem.
options: 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.
type: str
choices: [ present, absent ]
default: present
version_added: 1.3.0
fstype: 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 ]
description: description:
- Filesystem type to be created. - 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. - reiserfs support was added in 2.2.
- lvm support was added in 2.5. - lvm support was added in 2.5.
- since 2.5, I(dev) can be an image file. - since 2.5, I(dev) can be an image file.
@ -27,11 +41,12 @@ options:
- ocfs2 support was added in 2.6 - ocfs2 support was added in 2.6
- f2fs support was added in 2.7 - f2fs support was added in 2.7
- swap support was added in 2.8 - swap support was added in 2.8
required: yes type: str
aliases: [type] aliases: [type]
dev: dev:
description: description:
- Target path to device or image file. - Target path to device or image file.
type: path
required: yes required: yes
aliases: [device] aliases: [device]
force: force:
@ -43,18 +58,22 @@ options:
description: description:
- If C(yes), if the block device and filesystem size differ, grow the filesystem into the space. - 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), C(vfat), C(swap) filesystems. - Supported for C(ext2), C(ext3), C(ext4), C(ext4dev), C(f2fs), C(lvm), C(xfs), C(vfat), C(swap) filesystems.
- XFS Will only grow if mounted. - 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. - vFAT will likely fail if fatresize < 1.04.
type: bool type: bool
default: 'no' default: 'no'
opts: opts:
description: description:
- List of options to be passed to mkfs command. - List of options to be passed to mkfs command.
type: str
requirements: 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 C(blkid) command. When I(resizefs) is enabled, C(blockdev) command is required too.
notes: notes:
- Potential filesystem on I(dev) are checked using C(blkid), in case C(blkid) isn't able to detect an existing filesystem, - 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). this filesystem is overwritten even if I(force) is C(no).
- This module supports I(check_mode).
''' '''
EXAMPLES = ''' EXAMPLES = '''
@ -68,6 +87,11 @@ EXAMPLES = '''
fstype: ext4 fstype: ext4
dev: /dev/sdb1 dev: /dev/sdb1
opts: -cc opts: -cc
- name: Blank filesystem signature on /dev/sdb1
community.general.filesystem:
dev: /dev/sdb1
state: absent
''' '''
from distutils.version import LooseVersion from distutils.version import LooseVersion
@ -144,6 +168,22 @@ class Filesystem(object):
cmd = "%s %s %s '%s'" % (mkfs, self.MKFS_FORCE_FLAGS, opts, dev) cmd = "%s %s %s '%s'" % (mkfs, self.MKFS_FORCE_FLAGS, opts, dev)
self.module.run_command(cmd, check_rc=True) 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
# not doable here if it needs get_mountpoint() (to prevent corruption of
# a mounted filesystem), since 'findmnt' is not available on FreeBSD.
wipefs = self.module.get_bin_path('wipefs', required=True)
cmd = [wipefs, "--all", dev.__str__()]
self.module.run_command(cmd, check_rc=True)
def grow_cmd(self, dev): def grow_cmd(self, dev):
cmd = self.module.get_bin_path(self.GROW, required=True) cmd = self.module.get_bin_path(self.GROW, required=True)
return [cmd, str(dev)] return [cmd, str(dev)]
@ -369,34 +409,35 @@ def main():
# There is no "single command" to manipulate filesystems, so we map them all out and their options # There is no "single command" to manipulate filesystems, so we map them all out and their options
module = AnsibleModule( module = AnsibleModule(
argument_spec=dict( argument_spec=dict(
fstype=dict(required=True, aliases=['type'], state=dict(type='str', default='present', choices=['present', 'absent']),
choices=list(fstypes)), fstype=dict(type='str', aliases=['type'], choices=list(fstypes)),
dev=dict(required=True, aliases=['device']), dev=dict(type='path', required=True, aliases=['device']),
opts=dict(), opts=dict(type='str'),
force=dict(type='bool', default=False), force=dict(type='bool', default=False),
resizefs=dict(type='bool', default=False), resizefs=dict(type='bool', default=False),
), ),
required_if=[
('state', 'present', ['fstype'])
],
supports_check_mode=True, supports_check_mode=True,
) )
state = module.params['state']
dev = module.params['dev'] dev = module.params['dev']
fstype = module.params['fstype'] fstype = module.params['fstype']
opts = module.params['opts'] opts = module.params['opts']
force = module.params['force'] force = module.params['force']
resizefs = module.params['resizefs'] resizefs = module.params['resizefs']
if fstype in friendly_names:
fstype = friendly_names[fstype]
changed = False changed = False
try:
klass = FILESYSTEMS[fstype]
except KeyError:
module.fail_json(changed=False, msg="module does not support this filesystem (%s) yet." % fstype)
if not os.path.exists(dev): if not os.path.exists(dev):
module.fail_json(msg="Device %s not found." % dev) msg = "Device %s not found." % dev
if state == "present":
module.fail_json(msg=msg)
else:
module.exit_json(msg=msg)
dev = Device(module, dev) dev = Device(module, dev)
cmd = module.get_bin_path('blkid', required=True) cmd = module.get_bin_path('blkid', required=True)
@ -405,6 +446,15 @@ def main():
# then this existing filesystem would be overwritten even if force isn't enabled. # then this existing filesystem would be overwritten even if force isn't enabled.
fs = raw_fs.strip() fs = raw_fs.strip()
if state == "present":
if fstype in friendly_names:
fstype = friendly_names[fstype]
try:
klass = FILESYSTEMS[fstype]
except KeyError:
module.fail_json(changed=False, msg="module does not support this filesystem (%s) yet." % fstype)
filesystem = klass(module) filesystem = klass(module)
same_fs = fs and FILESYSTEMS.get(fs) == FILESYSTEMS[fstype] same_fs = fs and FILESYSTEMS.get(fs) == FILESYSTEMS[fstype]
@ -424,6 +474,12 @@ def main():
filesystem.create(opts, dev) filesystem.create(opts, dev)
changed = True changed = True
elif fs:
# wipe fs signatures
filesystem = Filesystem(module)
filesystem.wipefs(dev)
changed = True
module.exit_json(changed=changed) module.exit_json(changed=changed)

View file

@ -43,7 +43,12 @@
- 'not (item.0.key == "f2fs" and ansible_distribution == "Ubuntu" and ansible_distribution_version is version("14.04", "<="))' - '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")' - 'not (item.1 == "overwrite_another_fs" and ansible_system == "FreeBSD")'
- '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", "<"))'
# The xfsprogs package on newer versions of OpenSUSE (15+) require Python 3, we skip this on our Python 2 container # 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 # 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 (item.0.key == "xfs" and ansible_os_family == "Suse" and ansible_python.version.major == 2 and ansible_distribution_major_version|int != 42)'
loop: "{{ query('dict', tested_filesystems)|product(['create_fs', 'overwrite_another_fs'])|list }}" loop: "{{ query('dict', tested_filesystems)|product(['create_fs', 'overwrite_another_fs', 'remove_fs'])|list }}"

View file

@ -0,0 +1,98 @@
---
# We assume 'create_fs' tests have passed.
- name: filesystem creation
filesystem:
dev: '{{ dev }}'
fstype: '{{ fstype }}'
- name: get filesystem UUID with 'blkid'
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:
that:
- blkid_ref.stdout | length > 0
# Test check_mode first
- name: filesystem removal (check mode)
filesystem:
dev: '{{ dev }}'
state: absent
register: wipefs
check_mode: yes
- name: get filesystem UUID with 'blkid' (should remain the same)
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:
that:
- wipefs is changed
- blkid.stdout == blkid_ref.stdout
# Do it
- name: filesystem removal
filesystem:
dev: '{{ dev }}'
state: absent
register: wipefs
- name: get filesystem UUID with 'blkid' (should be empty)
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:
that:
- wipefs is changed
- blkid.stdout | length == 0
- blkid.rc == 2
# Do it again
- name: filesystem removal (idempotency)
filesystem:
dev: '{{ dev }}'
state: absent
register: wipefs
- name: Assert that the state did not change
assert:
that:
- wipefs is not changed
# and again
- name: filesystem removal (idempotency, check mode)
filesystem:
dev: '{{ dev }}'
state: absent
register: wipefs
check_mode: yes
- name: Assert that the state did not change
assert:
that:
- wipefs is not changed
# By the way, test removal of a filesystem on unexistent device
- name: filesystem removal (unexistent device)
filesystem:
dev: '/dev/unexistent_device'
state: absent
register: wipefs
- name: Assert that the state did not change
assert:
that:
- wipefs is not changed

View file

@ -1169,7 +1169,6 @@ plugins/modules/system/dconf.py pylint:blacklisted-name
plugins/modules/system/dconf.py validate-modules:doc-missing-type plugins/modules/system/dconf.py validate-modules:doc-missing-type
plugins/modules/system/dconf.py validate-modules:parameter-type-not-in-doc plugins/modules/system/dconf.py validate-modules:parameter-type-not-in-doc
plugins/modules/system/filesystem.py pylint:blacklisted-name plugins/modules/system/filesystem.py pylint:blacklisted-name
plugins/modules/system/filesystem.py validate-modules:doc-missing-type
plugins/modules/system/gconftool2.py pylint:blacklisted-name plugins/modules/system/gconftool2.py pylint:blacklisted-name
plugins/modules/system/gconftool2.py validate-modules:parameter-state-invalid-choice plugins/modules/system/gconftool2.py validate-modules:parameter-state-invalid-choice
plugins/modules/system/gconftool2.py validate-modules:parameter-type-not-in-doc plugins/modules/system/gconftool2.py validate-modules:parameter-type-not-in-doc

View file

@ -1169,7 +1169,6 @@ plugins/modules/system/dconf.py pylint:blacklisted-name
plugins/modules/system/dconf.py validate-modules:doc-missing-type plugins/modules/system/dconf.py validate-modules:doc-missing-type
plugins/modules/system/dconf.py validate-modules:parameter-type-not-in-doc plugins/modules/system/dconf.py validate-modules:parameter-type-not-in-doc
plugins/modules/system/filesystem.py pylint:blacklisted-name plugins/modules/system/filesystem.py pylint:blacklisted-name
plugins/modules/system/filesystem.py validate-modules:doc-missing-type
plugins/modules/system/gconftool2.py pylint:blacklisted-name plugins/modules/system/gconftool2.py pylint:blacklisted-name
plugins/modules/system/gconftool2.py validate-modules:parameter-state-invalid-choice plugins/modules/system/gconftool2.py validate-modules:parameter-state-invalid-choice
plugins/modules/system/gconftool2.py validate-modules:parameter-type-not-in-doc plugins/modules/system/gconftool2.py validate-modules:parameter-type-not-in-doc

View file

@ -921,7 +921,6 @@ plugins/modules/system/dconf.py pylint:blacklisted-name
plugins/modules/system/dconf.py validate-modules:doc-missing-type plugins/modules/system/dconf.py validate-modules:doc-missing-type
plugins/modules/system/dconf.py validate-modules:parameter-type-not-in-doc plugins/modules/system/dconf.py validate-modules:parameter-type-not-in-doc
plugins/modules/system/filesystem.py pylint:blacklisted-name plugins/modules/system/filesystem.py pylint:blacklisted-name
plugins/modules/system/filesystem.py validate-modules:doc-missing-type
plugins/modules/system/gconftool2.py pylint:blacklisted-name plugins/modules/system/gconftool2.py pylint:blacklisted-name
plugins/modules/system/gconftool2.py validate-modules:parameter-type-not-in-doc plugins/modules/system/gconftool2.py validate-modules:parameter-type-not-in-doc
plugins/modules/system/interfaces_file.py pylint:blacklisted-name plugins/modules/system/interfaces_file.py pylint:blacklisted-name