Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mount.py no rmdir when state=='absent' #569

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
16 changes: 14 additions & 2 deletions plugins/modules/mount.py
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,8 @@
real source. V(absent) does not unmount recursively, and the module will
fail if multiple devices are mounted on the same mount point. Using
V(absent) with a mount point that is not registered in the I(fstab) has
no effect, use V(unmounted) instead.
no effect, use V(unmounted) instead. You can set O(keep_mountpoint) to
True to keep the mountpoint.
- V(remounted) specifies that the device will be remounted for when you
want to force a refresh on the mount itself (added in 2.9). This will
always return RV(ignore:changed=true). If O(opts) is set, the options will be
Expand Down Expand Up @@ -132,6 +133,16 @@
the original file back if you somehow clobbered it incorrectly.
type: bool
default: false
keep_mountpoint:
description:
- Change the default behaviour of state=absent by keeping the mountpoint
- With keep_mountpoint=true, state=absent is like unmounted plus the
fstab update.
- Use it if you care about finding original mountpoint content without failing
and want to remove the entry in fstab. If you have no entry to clean in
fstab you can use state=unmounted
type: bool
default: false
notes:
- As of Ansible 2.3, the O(name) option has been changed to O(path) as
default, but O(name) still works as well.
Expand Down Expand Up @@ -779,6 +790,7 @@ def main():
src=dict(type='path'),
backup=dict(type='bool', default=False),
state=dict(type='str', required=True, choices=['absent', 'absent_from_fstab', 'mounted', 'present', 'unmounted', 'remounted', 'ephemeral']),
keep_mountpoint=dict(type='bool', default=False),
),
supports_check_mode=True,
required_if=(
Expand Down Expand Up @@ -899,7 +911,7 @@ def main():
module.fail_json(
msg="Error unmounting %s: %s" % (name, msg))

if os.path.exists(name):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will change the behavior of state: absent.
But I don't think the rmdir logic should be skipped. Because it breaks backward compatibility.
If you want to skip it, how about adding keep_mountpoint or preserve_mountpoint to keep the mountpoint to avoid Directory not empty error instead of removing this section.

If this is set as true, you can skip rmdir section. Also, if the default value is set as false, to keep backward compatibility, you can set it to false by default.

if os.path.exists(name) and module.params['keep_mountpoint'] is False:
try:
os.rmdir(name)
except (OSError, IOError) as e:
Expand Down