Remove fstab features from mount, call new module mount2 #27174

Jmainguy · 2017-07-21T18:52:45Z

SUMMARY

ISSUE TYPE

Feature Pull Request
New Module Pull Request

COMPONENT NAME

lib/ansible/modules/system/mount2.py

ANSIBLE VERSION

Latest Devel

ADDITIONAL INFORMATION

See issue #19820 where discussion on splitting module started

ansibot · 2017-07-21T18:54:36Z

@Jmainguy this PR contains more than one new module.

Please submit only one new module per pullrequest. For further explanation, please read grouped module documentation

click here for bot help

Jmainguy · 2017-07-21T18:57:54Z

mount2 just deals with the mount command and will not check fstab.
fstab module just deals with fstab and will not check what is and is not already mounted.

Limitations present in this solution.
For someone to have a mount persist and mount at playbook runtime, they would need to run 2 tasks at a minimum, either fstab module, followed by mount2, or fstab module, followed by shell module calling mount -a.

bcoca · 2017-07-21T19:03:11Z

I would consider it the same way we 'install/configure' services and then call handler, in this case, fstab is configuration, mount2 the 'handler' (only if you want to, it seems like a good way but this is not a mandate).

bcoca · 2017-07-21T19:04:26Z

@Jmainguy could you split into 2 PRs, we normally prefer PR per new module to be able to review in parallel.

ansibot · 2017-07-21T19:06:05Z

The test ansible-test sanity --test pep8 failed with the following errors:

lib/ansible/modules/system/mount2.py:126:23: E226 missing whitespace around arithmetic operator
lib/ansible/modules/system/mount2.py:140:23: E226 missing whitespace around arithmetic operator

The test ansible-test sanity --test validate-modules failed with the following errors:

lib/ansible/modules/system/fstab.py:0:0: E312 No RETURN provided
lib/ansible/modules/system/mount2.py:0:0: E312 No RETURN provided

click here for bot help

Jmainguy · 2017-07-21T19:17:02Z

I will split this into two PR's to fit the model

ansibot · 2017-07-21T19:31:06Z

The test ansible-test sanity --test pep8 failed with the following errors:

lib/ansible/modules/system/mount2.py:126:23: E226 missing whitespace around arithmetic operator
lib/ansible/modules/system/mount2.py:140:23: E226 missing whitespace around arithmetic operator

The test ansible-test sanity --test validate-modules failed with the following errors:

lib/ansible/modules/system/fstab.py:0:0: E312 No RETURN provided
lib/ansible/modules/system/mount2.py:0:0: E312 No RETURN provided

click here for bot help

Jmainguy · 2017-07-24T15:54:57Z

Split it out, fstab is now in #27245

ansibot · 2017-07-24T16:43:31Z

@AugustusKling @ColOfAbRiX @DavidWittman @EvanK @LinusU @abulimov @adejoux @agaffney @ahtik @Akasurde @azaghal @dankeder @davixx @dougluce @dsummersl @goozbach @groks @jasperla @jhoekx @jsumners @kevensen @lberruti @matze @maxamillion @mcv21 @molekuul @mpdehaan @mulby @natefoo @nibalizer @ovcharenko @pmarkham @pyykkis @risaacson @rosmo @saito-hideki @sfromm @srvg @tdtrask @tmshn @xen0l

As a maintainer of a module in the same namespace this new module has been submitted to, your vote counts for shipits. Please review this module and add shipit if you would like to see it merged.

click here for bot help

mscherer · 2017-07-27T13:55:34Z

Do we really need to call it 'mount2' ? I suspect that we would maybe need a better way to deprecate 'mount', and I suspect this would create a lot of confusion to people over using 'mount' or 'mount2'

mscherer · 2017-07-27T13:56:07Z

lib/ansible/modules/system/mount2.py

+#!/usr/bin/python
+# -*- coding: utf-8 -*-
+
+# (c) 2012, Red Hat, inc


Copyright should be updated (that's a detail)

mscherer · 2017-07-27T13:57:28Z

lib/ansible/modules/system/mount2.py

+    description:
+      - Filesystem type. Required when I(state) is C(mounted).
+    required: false
+    default: null


Would using 'auto' here permit to not specifiy it ?

I would default to 'auto' and not require it, let the command fail if it is not present in fstab (like mount does)

mscherer · 2017-07-27T14:01:58Z

lib/ansible/modules/system/mount2.py

+            try:
+                os.makedirs(path)
+            except (OSError, IOError):
+                e = get_exception()


As we no longer care about 2.4, I think this can be cleaned

bcoca · 2017-07-27T14:08:52Z

lib/ansible/modules/system/mount2.py

+      - If specifying C(mounted) and the mount point is not present, the mount
+        point will be created.
+    required: true
+    choices: ["mounted", "unmounted"]


I would add 'fstab' option to let user specify a specific fstab file, using the system default otherwise

bcoca · 2017-07-27T14:09:29Z

lib/ansible/modules/system/mount2.py

+      - If specifying C(mounted) and the mount point is not present, the mount
+        point will be created.
+    required: true
+    choices: ["mounted", "unmounted"]


present/absent could be used here, just to keep it in line with other 'state' fields

RainerW · 2017-07-27T18:43:13Z

As said in #19820 separating mount and fstab makes ansible too verbose :

- name: Mount for Access now
  mount2:
    path: /home
    src: UUID=b3e48f45-f933-4c8e-a700-22a159ec9077
    fstype: xfs
    opts: noatime
    state: present

- name: make Mount persistent
  fstab:
    path: /home
    src: UUID=b3e48f45-f933-4c8e-a700-22a159ec9077
    fstype: xfs
    opts: noatime
    state: present

vs

- name: Mount shared filesystem
  mount:
    path: /home
    src: UUID=b3e48f45-f933-4c8e-a700-22a159ec9077
    fstype: xfs
    opts: noatime
    state: present
    fstab: present

bcoca · 2017-07-28T00:32:13Z

@RainerW only if you force module to always define all parameters, it should be able use existing entries in fstab to avoid this, but it should work both ways.

- name: make Mount persistent
  fstab:
    path: /home
    src: UUID=b3e48f45-f933-4c8e-a700-22a159ec9077
    fstype: xfs
    opts: noatime
    state: present

- name: Mount for Access now
  mount2:  path=/home  state=present

ltalirz · 2017-11-08T20:11:22Z

+1
To me, the fact that the mount module would better be split into two is also reflected in the current way of "undoing" a state: mounted.
If I understand the manual correctly, I have to

call mount with state: unmounted to unmount the file system
call mount again with state: absent to remove the fstab entry

There are simply two "states" at play here

darkdragon-001 · 2020-05-18T17:06:44Z

Any progress on this?

ansibot · 2020-08-17T01:29:28Z

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.

lib/ansible/modules/system/mount2.py -> https://galaxy.ansible.com/community/general

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

Jmainguy mentioned this pull request Jul 21, 2017

mount module feature request: allow to mount without adding to fstab #19820

Closed

bcoca removed the needs_triage Needs a first human triage before being processed. label Jul 21, 2017

ansibot added ci_verified Changes made in this PR are causing tests to fail. and removed ci_verified Changes made in this PR are causing tests to fail. labels Jul 21, 2017

ansibot added the ci_verified Changes made in this PR are causing tests to fail. label Jul 21, 2017

Split mount into mount2 and fstab modules

56ecf61

Jmainguy changed the title ~~Split mount into mount2 and fstab modules~~ Remove fstab features from mount, call new module mount2 Jul 24, 2017

ansibot removed the ci_verified Changes made in this PR are causing tests to fail. label Jul 24, 2017

ansibot added community_review In order to be merged, this PR must follow the community review workflow. and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Jul 24, 2017

mscherer reviewed Jul 27, 2017

View reviewed changes

bcoca reviewed Jul 27, 2017

View reviewed changes

ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Aug 2, 2017

ansible deleted a comment from ansibot Nov 16, 2017

ghost mentioned this pull request Dec 7, 2017

Regression: receiving notifications for namespace I contributed modules to ansible/ansibullbot#832

Closed

ansibot added feature This issue/PR relates to a feature request. and removed feature_pull_request labels Mar 2, 2018

ansibot added the new_plugin This PR includes a new plugin. label May 22, 2018

ansibot added the system System category label Feb 17, 2019

ansibot added the has_issue label Jul 28, 2019

ansibot added collection Related to Ansible Collections work collection:community.general needs_collection_redirect https://github.com/ansible/ansibullbot/blob/master/docs/collection_migration.md labels Apr 29, 2020

ansibot added the bot_closed label Aug 17, 2020

ansibot closed this Aug 17, 2020

ansible locked and limited conversation to collaborators Sep 14, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove fstab features from mount, call new module mount2 #27174

Remove fstab features from mount, call new module mount2 #27174

Jmainguy commented Jul 21, 2017 •

edited

Loading

ansibot commented Jul 21, 2017

Jmainguy commented Jul 21, 2017

bcoca commented Jul 21, 2017

bcoca commented Jul 21, 2017

ansibot commented Jul 21, 2017

Jmainguy commented Jul 21, 2017

ansibot commented Jul 21, 2017

Jmainguy commented Jul 24, 2017

ansibot commented Jul 24, 2017

mscherer commented Jul 27, 2017

mscherer Jul 27, 2017

mscherer Jul 27, 2017

bcoca Jul 27, 2017

mscherer Jul 27, 2017

bcoca Jul 27, 2017

bcoca Jul 27, 2017

RainerW commented Jul 27, 2017

bcoca commented Jul 28, 2017 •

edited

Loading

ltalirz commented Nov 8, 2017

darkdragon-001 commented May 18, 2020

ansibot commented Aug 17, 2020

Remove fstab features from mount, call new module mount2 #27174

Remove fstab features from mount, call new module mount2 #27174

Conversation

Jmainguy commented Jul 21, 2017 • edited Loading

SUMMARY

ISSUE TYPE

COMPONENT NAME

ANSIBLE VERSION

ADDITIONAL INFORMATION

ansibot commented Jul 21, 2017

Jmainguy commented Jul 21, 2017

bcoca commented Jul 21, 2017

bcoca commented Jul 21, 2017

ansibot commented Jul 21, 2017

Jmainguy commented Jul 21, 2017

ansibot commented Jul 21, 2017

Jmainguy commented Jul 24, 2017

ansibot commented Jul 24, 2017

mscherer commented Jul 27, 2017

mscherer Jul 27, 2017

Choose a reason for hiding this comment

mscherer Jul 27, 2017

Choose a reason for hiding this comment

bcoca Jul 27, 2017

Choose a reason for hiding this comment

mscherer Jul 27, 2017

Choose a reason for hiding this comment

bcoca Jul 27, 2017

Choose a reason for hiding this comment

bcoca Jul 27, 2017

Choose a reason for hiding this comment

RainerW commented Jul 27, 2017

bcoca commented Jul 28, 2017 • edited Loading

ltalirz commented Nov 8, 2017

darkdragon-001 commented May 18, 2020

ansibot commented Aug 17, 2020

Jmainguy commented Jul 21, 2017 •

edited

Loading

bcoca commented Jul 28, 2017 •

edited

Loading