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

Ability to temporarily mount a filesystem without altering /etc/fstab #48134

Closed
ahuffman opened this issue Nov 5, 2018 · 47 comments
Closed

Ability to temporarily mount a filesystem without altering /etc/fstab #48134

ahuffman opened this issue Nov 5, 2018 · 47 comments
Labels
affects_2.8 This issue/PR affects Ansible v2.8 bot_closed collection:ansible.posix collection:ibm.power_aix collection Related to Ansible Collections work feature This issue/PR relates to a feature request. has_pr This issue has an associated PR. module This issue/PR relates to a module. needs_collection_redirect https://github.com/ansible/ansibullbot/blob/master/docs/collection_migration.md support:community This issue/PR relates to code supported by the Ansible community. system System category

Comments

@ahuffman
Copy link
Contributor

ahuffman commented Nov 5, 2018

SUMMARY

Ability to perform temporary mounts without altering /etc/fstab. Currently anything done with the mount module needs to manipulate /etc/fstab.

Additional parameter is needed to perform temporary mounts, such as with an ISO image that would be unmounted later in a play. Currently this can't be done with the mount module, as altering the /etc/fstab is required. Maybe call it "modify_fstab" and have it be a boolean. Currently the only way to achieve this is via the command module using direct mount/unmount commands.

ISSUE TYPE
  • Feature Idea
COMPONENT NAME

mount, fstab

ADDITIONAL INFORMATION

To mount an ISO image temporarily for pulling in data, but not make an entry in /etc/fstab.

- name: Mount an ISO Temporarily
  mount:
    src: /path/to/my/iso.iso
    path: /path/to/mount/my/iso
    fstype: iso9660
    opts: ro
    modify_fstab: no
    state: mounted

- name: Unmount an ISO
  mount:
    path: /path/to/unmount/my/iso
    modify_fstab: no
    state: unmounted
@ansibot

This comment has been minimized.

@ansibot

This comment has been minimized.

@ansibot

This comment has been minimized.

@ansibot
Copy link
Contributor

ansibot commented Nov 5, 2018

cc @jtyr
click here for bot help

@ansibot ansibot added affects_2.8 This issue/PR affects Ansible v2.8 module This issue/PR relates to a module. needs_info This issue requires further information. Please answer any outstanding questions. needs_template This issue/PR has an incomplete description. Please fill in the proposed template correctly. needs_triage Needs a first human triage before being processed. support:core This issue/PR relates to code supported by the Ansible Engineering Team. labels Nov 5, 2018
@webknjaz
Copy link
Member

webknjaz commented Nov 6, 2018

Hi @ahuffman,

I think it might be a better idea to have a separate module for that. Normally, when there's a ton of options module accepts it's a sign that it should be broken into parts honoring the single responsibility principle. Also it's hard to use and test when the matrix of argument combos becomes huge.

Do you want to try drafting such module?

@webknjaz webknjaz removed the needs_triage Needs a first human triage before being processed. label Nov 6, 2018
@bbyhuy
Copy link
Contributor

bbyhuy commented Nov 7, 2018

Seems like someone already tried to implement something along these lines, but never completed due to massive number of commit changes.
#35294

@ahuffman
Copy link
Contributor Author

ahuffman commented Nov 7, 2018

@bbyhuy Thanks for filling me in on the other PR. That looks very similar to the functionality I was looking for.

@webknjaz I could certainly take a crack at it, but the PR @bbyhuy mentioned is quite close. I may add a commit to the open PR if I can get some time this week. If the PR fails to be merged, I will definitely take a crack at breaking it out into it's own module, but I probably won't get to it until 2019. If I take that route, are there any preferences on module naming? I would think "temp_mount" or "mount_temp" would be decent names, but naming it that way could be confusing because it leads you to think of "/tmp".

@webknjaz
Copy link
Member

webknjaz commented Nov 7, 2018

@ahuffman the new module name is up to you if you go for it, it'll be community-supported by default. I'd suggest maybe smth like live_mount/mount_live.

@BryanJeterAL
Copy link

BryanJeterAL commented Nov 7, 2018

Possible alternative to splitting this into two modules, add two new states, temp and reset. temp would implement all of the settings from fstab, overridden by any specified when calling the module. reset would remount according to fstab and would only need src or path. The code from mounted could be reused to implement these, just don't use the parts that write to fstab. This also aligns with the usage of the CLI mount command behavior which reads from fstab when not given full information.

@nikaro
Copy link

nikaro commented Nov 8, 2018

Duplicate of #19820. But seems to be a better approach to what have been done for now.

If we go for a separate module, ideally this module should be called "mount" and the actual "mount" should be renamed to "fstab". But for backward compatibility reason i do not think it is possible. So… +1 for "mount_live".

@webknjaz
Copy link
Member

webknjaz commented Nov 8, 2018

I like @BryanJeterCTR's idea

@nikaro right, when renaming modules we still leave old name symlinked and it would not be possible to just replace it. Also, users will complain about "their old playbooks doing a different thing" in four versions from now...

@nikaro
Copy link

nikaro commented Nov 8, 2018

@BryanJeterCTR @webknjaz i have been told that the state should always be in paste tense: #35294 (comment)

@webknjaz
Copy link
Member

webknjaz commented Nov 8, 2018

Ah.. Right. Past tense is better for sure :)

@bbyhuy
Copy link
Contributor

bbyhuy commented Nov 8, 2018

So is there a consensus on how we want to go about this? Add a state or split it out as a new module? I can take a stab at it if @ahuffman isn't able to get to it til next year.

Seems like making a mount_live module makes the most sense in terms of adding the requested functionality while keeping the original mount module safe from future "why is this not behaving the same" issues popping up.

@ahuffman
Copy link
Contributor Author

ahuffman commented Nov 8, 2018

@bbyhuy something like mount_live would work for me, as it could just avoid touching any of the other cases and logic. I was initially thinking a boolean of modify_fstab: False would be good, but this is probably simplest.

How about state: "mounted_non_persistent" instead of mount_live? This seems like a better state name to me.

Also it looks like state: "unmounted" should still work with this implementation without requiring yet another state:

elif state == 'unmounted':
if ismount(name) or is_bind_mounted(module, linux_mounts, name):
if not module.check_mode:
res, msg = umount(module, name)
if res:
module.fail_json(
msg="Error unmounting %s: %s" % (name, msg))

@webknjaz
Copy link
Member

webknjaz commented Nov 9, 2018

state: mounted_live maybe?

@bbyhuy
Copy link
Contributor

bbyhuy commented Nov 9, 2018

@webknjaz you read my mind. I literally started implementing "mounted_live" just this morning.

@bbyhuy
Copy link
Contributor

bbyhuy commented Nov 9, 2018

Hm, so I've implemented a mount_live function along with modifying the umount function a bit to handle openbsd specific live mounting.
However, I'm not sure how I should handle the remount function for this. I'm tempted to just call umount and mount in the case if "mounted_live" is called twice. The remount involves a lot of unnecessary code in terms of the mounted_live state is concerned. Or to clean it up I could just do a simple remount_live function...but it'd be a bit redundant.

@webknjaz
Copy link
Member

webknjaz commented Nov 9, 2018

@bbyhuy I think it'll be easier for everyone to provide a feedback within some WIP PR as one can attach comments exactly to lines they refer to, which is better illustrative...

@bbyhuy
Copy link
Contributor

bbyhuy commented Nov 9, 2018

This is the snippet I added based on mounted.
https://github.com/bbyhuy/ansible/blob/a5a9a5282f5a5a98b2c7e77e17aae3ab21ebcecc/lib/ansible/modules/system/mount.py#L783-L803

The function call remount at L797 is what I'm referring to. This references to this function.
https://github.com/bbyhuy/ansible/blob/a5a9a5282f5a5a98b2c7e77e17aae3ab21ebcecc/lib/ansible/modules/system/mount.py#L462-L509

Should i replace the remount call with just a umount / mounted_live combo? Or...

@webknjaz
Copy link
Member

@bbyhuy please send it as a PR, so that we could have the discussion there.

@smurfix
Copy link

smurfix commented Jan 6, 2019

If we do need to split that, mountpoint would work.

However, I do have a conceptual problem with splitting mount because this leads to code duplication in everybody's tasks: now I can write a mount rule which sets up the system's state (and ensures that it stays that way after rebooting), but after mount is deprecated I need two – with almost-but-not-quite the same arguments, and the concomitant danger of mistakenly editing one but not the other.

@dagwieers
Copy link
Contributor

We have module_utils to avoid code-duplication. It's widely used.

@jtyr
Copy link
Contributor

jtyr commented Jan 6, 2019

@smurfix I'm absolutely with you. It makes sense to have the mount module to manage the mounting as well as fstab file as you need the same parameters in both.

@dagwieers there is no problem to split the module. It's more about the convenience it brings if you use single module for both, the mounting and managing the fstab file.

@ansibot ansibot added the system System category label Feb 15, 2019
@ushuz
Copy link
Contributor

ushuz commented Feb 25, 2019

I suppose splitting into two modules means that if I want to do both mount and fstab modification in a row, then I have to write two tasks, with almost identical parameters? Also it would be a breaking change, which costs users time to migrate in the future. I don't think it worth the trouble just to save a few options in mount module.

We have module_utils to avoid code-duplication. It's widely used.

@dagwieers module_utils can avoid dup in ansible's own code base, but it can't avoid dup tasks in users' playbooks.

@dagwieers
Copy link
Contributor

dagwieers commented Feb 25, 2019

@jtyr @ushuz I am not in favor of splitting the module myself. I think you're misreading my comment.

@bluikko
Copy link
Contributor

bluikko commented May 6, 2019

There has been some back-and-forth between the design of "ephemeral mounts".

Has consensus been reached? Is someone working on this?

To summarize it looks like:

  • @smurfix, @dagwieers, @ushuz would support modifying the existing mount module
  • @webknjaz would prefer to split the fstab/mount functionalities to separate modules
  • @sivel comments based on IRC discussion - 6 months ago - to leave the old mount module as is and implement two new modules fstab and mountpoint

Edit: it is extremely surprising that such basic functionality is still missing. My use case is chrooted/read-only file systems requiring temporary mounts. And I totally agree that the warning about using the mount module (which can be disabled) is wrong since the module cannot actually do what I want. But then again, several modules have this problem: for example the yum module implements a very small subset of yum command features.

@jflorian
Copy link

Later arrival here. My use case is a very simple mount -o remount,rw /boot/efi to deal with the horribly fragile FAT filesystem on a Raspberry Pi. Embedded devices are hard(er) to service and frequently are powered off w/o any formal shutdown. So I prefer to keep them RO, but would like to Ansible to remount RW for doing things like updates. This issue looks the nearest to my need.

My 2 cents: one module to do all things mount related, but I could potentially see a separate remount module. I'm not as deep into Ansible as Puppet, but I'd think the fstab part would be akin to a Puppet provider because there should also (someday) be support for systemd mount units and the like. Maybe remount would be another provider ... or whatever Ansible's equivalent is. If another module is needed, I too am disliking any live type of naming for confusion with live boot media which naturally has strong ties with mounting.

@webknjaz
Copy link
Member

Just to clarify: the position of Ansible Core team is that one module should do one thing. That's why we want a separate module with a distinct purpose.

@jflorian
Copy link

Just to clarify: the position of Ansible Core team is that one module should do one thing. That's why we want a separate module with a distinct purpose.

Yes, I read that several times throughout the thread. However, there seems to be at least two ways of seeing this "one thing". For some, myself included, that one thing is "managing mounts". Others, and apparently you, see that one thing as how that management is actually achieved. I don't buy a calculator expecting it to do spell checking, but I do expect it do multiple math operations.

@webknjaz
Copy link
Member

We aim to keep the number of module arguments. If it's big it's a sign that it's already doing too much. This leads to a combinatorial explosion of arg combos which is both hard to support/maintain and keep well-tested.

@darkdragon-001
Copy link

We aim to keep the number of module arguments. If it's big it's a sign that it's already doing too much. This leads to a combinatorial explosion of arg combos which is both hard to support/maintain and keep well-tested.

When I understand it correctly, the suggested changes would add more options to the state argument and thus not even introduce a new one. While the implementation would probably be completely different (change /etc/fstab and calling mount -a vs. calling mount SRC DST), from a user point of view it doesn't add much more complexity.

While I would prefer to have everything in one module with additional state options over having a separate module, in the end I don't care which one you choose as long as the issue gets resolved! I stumble over this problem regularly and use the command module for now. Please agree on a direction so that people can start implementing it so that this issue can get finally resolved after more than a year!

@ansibot ansibot added collection Related to Ansible Collections work collection:ansible.posix needs_collection_redirect https://github.com/ansible/ansibullbot/blob/master/docs/collection_migration.md support:community This issue/PR relates to code supported by the Ansible community. and removed support:core This issue/PR relates to code supported by the Ansible Engineering Team. labels Apr 29, 2020
@darkdragon-001
Copy link

Just stumbled over this again. It has been almost 3.5 years since the issue was first reported. How can we proceed to push this forward?

@relrod relrod removed the P3 Priority 3 - Approved, No Time Limitation label Jul 7, 2020
@ansibot
Copy link
Contributor

ansibot commented Aug 16, 2020

Thank you very much for your interest in Ansible. Ansible has migrated much of the content into separate repositories to allow for more rapid, independent development. We are closing this issue/PR because this content has been moved to one or more collection repositories.

For further information, please see:
https://github.com/ansible/ansibullbot/blob/master/docs/collection_migration.md

@ansibot ansibot closed this as completed Aug 16, 2020
@ansible ansible locked and limited conversation to collaborators Sep 13, 2020
@sivel sivel removed the waiting_on_contributor This would be accepted but there are no plans to actively work on it. label Dec 3, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.8 This issue/PR affects Ansible v2.8 bot_closed collection:ansible.posix collection:ibm.power_aix collection Related to Ansible Collections work feature This issue/PR relates to a feature request. has_pr This issue has an associated PR. module This issue/PR relates to a module. needs_collection_redirect https://github.com/ansible/ansibullbot/blob/master/docs/collection_migration.md support:community This issue/PR relates to code supported by the Ansible community. system System category
Projects
None yet
Development

Successfully merging a pull request may close this issue.