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

Add ephemeral state to mount fs without altering fstab #264

Closed
wants to merge 5 commits into from
Closed

Add ephemeral state to mount fs without altering fstab #264

wants to merge 5 commits into from

Conversation

NeodymiumFerBore
Copy link
Contributor

@NeodymiumFerBore NeodymiumFerBore commented Sep 12, 2021

SUMMARY

Add ephemeral possible value for state parameter.

The ephemeral state allows end-users to mount a volume on a given path, without altering an fstab file or creating a dummy one.

There have been debates about splitting this module into an fstab module and a mount module, but nothing has been done in 5 years. This is why I'd like to propose this feature.

Downside: the way the posix.mount module handles the mount options prevent it to be able to check exactly if the given opts perfectly match the mount options of an already mounted volume. To achieve this, the module would have to be aware of every mount default options, for all platforms. This is why the ephemeral state always return changed=yes. In other terms, a remount will always be triggered if the volume is already mounted, even if the options look to be the same. However, using state unmounted on a volume previously mounted with ephemeral behaves correctly.

ISSUE TYPE
  • Feature Pull Request

Related issues:

COMPONENT NAME

mount

ADDITIONAL INFORMATION

Example use case

Sometimes it is handy to be able to temporarily mount a volume. I've seen this in some companies where they use Ansible to generate reports and put it on network shares. However, some admins don't look into mount options such as krb5 and multiuser for SMB shares. Being forced to use fstab-based mounts leads to clear text passwords being stored more or less temporarily on the host filesystem, requiring "manual" deletion (with the hassle of using blocks, rescues, always, etc.). This feature respond to this use case by providing a way to mount a volume without having to alter an fstab file.

Description of changes

  • Edit DOCUMENTATION section to add ephemeral state
  • Edit EXAMPLES section to add ephemeral state example
  • Add new function _set_ephemeral_args to use instead of _set_fstab_args when using ephemeral state
  • Add new function _is_same_mount_src to determine if the mounted volume on the destination path has the same source than the one supplied to the module
  • Add new function _get_mount_info to avoid redundant code between functions get_linux_mounts and _is_same_mount_src
  • Modify get_linux_mount to use the new function _get_mount_info. Behavior is preserved.
  • Integrate ephemeral parameter treatment into mounted treatment, and add if statements to avoid IO from/to fstab
  • Add ephemeral as a possible value for the state parameter in main()
  • Add required_if dependencies for ephemeral state

NeodymiumFerBore and others added 3 commits September 12, 2021 22:41
- Edit DOCUMENTATION section to add ephemeral state
- Edit EXAMPLES section to add ephemeral state example
- Add new function `_set_ephemeral_args` to replace `_set_fstab_args` when using ephemeral state
- Add new function `_is_same_mount_src` to determine if the mounted volume on the destination path has the same source than the one supplied to the module
- Add new function `_get_mount_info` to avoid redundant code between functions `get_linux_mounts` and `_is_same_mount_src`
- Modify `get_linux_mount` to use the new function `_get_mount_info`. Behavior is preserved.
- Integrate `ephemeral` parameter treatment into `mounted` treatment, and add `if` statements to avoid IO from/to fstab
- Add `ephemeral` as a possible value for the `state` parameter in `main()`
- Add `required_if` dependencies for `ephemeral` state
Copy link
Contributor Author

@NeodymiumFerBore NeodymiumFerBore left a comment

Choose a reason for hiding this comment

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

Added comments on major modifications in mount.py.

path: /mnt/smb_share
opts: "rw,vers=3,file_mode=0600,dir_mode=0700,dom={{ ad_domain }},username={{ ad_username }},password={{ ad_password }}"
fstype: cifs
state: ephemeral
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Examples: add an example for the use of state=ephemeral. This example has been tested on my side.

@@ -426,6 +443,23 @@ def _set_fstab_args(fstab_file):
return result


def _set_ephemeral_args(args):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

_set_ephemeral_args(args): When using ephemeral, we cannot rely on an fstab entry to set the mount options. This function sets the mount options accordingly to what was supplied to the module: fstype, opts and src.

"""Gather mount information"""

def _get_mount_info(module, mntinfo_file="/proc/self/mountinfo"):
"""Return raw mount information"""
Copy link
Contributor Author

Choose a reason for hiding this comment

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

New function _get_mount_info and minor change in function get_linux_mounts. As my new function _is_same_mount_src uses the mount info, I created this new function to avoid code redundancy. The function get_linux_mounts has been edited accordingly, and the original behavior is entirely preserved.

This function does exactly what get_linux_mounts used to do to retrieve mount info.

# Keep same behavior than before
if lines is None:
return

Copy link
Contributor Author

@NeodymiumFerBore NeodymiumFerBore Sep 12, 2021

Choose a reason for hiding this comment

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

The condition if lines is None: return preserve the function behavior from before the edit.

return lines


def get_linux_mounts(module, mntinfo_file="/proc/self/mountinfo"):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

As previously noticed, this function has been edited to use the new function _get_mount_info, instead of retrieving "manually" the mount info.

@@ -659,6 +709,24 @@ def get_linux_mounts(module, mntinfo_file="/proc/self/mountinfo"):
return mounts


def _is_same_mount_src(module, args, mntinfo_file="/proc/self/mountinfo"):
"""Return True if the mounted fs on mountpoint is the same source than src"""
Copy link
Contributor Author

Choose a reason for hiding this comment

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

New function _is_same_mount_src: This function is used for the ephemeral state: if this function returns False, then it means that the provided mount point has already a volume mounted on it, which is NOT the volume given in src. This leads to a module failure, to avoid an unwanted unmount.


for line in lines:
fields = line.split()
if fields[4] == mountpoint and fields[-2] == src:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

fields[4] contains a mountpoint and fields[-2] contains the associated source. See get_linux_mounts for more info about mountinfo fields. The logic behind picking these fields was already implemented, I only copy-pasted after test and review.

except Exception as e:
module.fail_json(msg="Failed to open %s due to %s" % (args['fstab'], to_native(e)))
# If state is 'ephemeral', we do not need fstab file
if module.params['state'] != 'ephemeral':
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Only this line changed (add if statement and fix block indent). This code block is in charge of creating the provided fstab file. For ephemeral state, the fstab file does not have to be created.

@@ -797,7 +870,7 @@ def main():
msg="Error unmounting %s: %s" % (name, msg))

changed = True
elif state == 'mounted':
elif state == 'mounted' or state == 'ephemeral':
Copy link
Contributor Author

Choose a reason for hiding this comment

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

ephemeral treatment is based on the mounted logic, bypassing the fstab read and write.

'unwanted unmount or override operation. Try replacing this command with '
'a "state: unmounted" followed by a "state: ephemeral", or use '
'a different destination path.'))

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When state is ephemeral or mounted, it tests if the provided mountpoint has already a volume mounted on it.

In the case of ephemeral, there is no fstab file to provide an authoritative answer to the question "What should be mounted here". To avoid an unwanted unmount and a possible service outage, the module fails if the volume already mounted on the mountpoint is not the same as the one provided in src (this is checked with the new function _is_same_mount_src).

@NeodymiumFerBore
Copy link
Contributor Author

Renaming the branch closed the pull request. I thought the PR would be relocated, but this only works the other way around... Sorry for that. New PR: #267

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant