Working subset of mount fixes (#4987)

* Fixing bind mount on Linux

* The latest update from jtyr doesn't pass integration tests.

Manually select the changes that are necessary to fix the bug with
unmounting

1 files changed, 244 insertions, 116 deletions

diff --git a/system/mount.py b/system/mount.py
index f14eb2a5..b80e6bfc 100644
--- a/system/mount.py
+++ b/system/mount.py
@@ -20,91 +20,137 @@
 # You should have received a copy of the GNU General Public License
 # along with Ansible.  If not, see <http://www.gnu.org/licenses/>.
 
+
+from ansible.module_utils.basic import AnsibleModule
+from ansible.module_utils.basic import get_platform
+from ansible.module_utils.ismount import ismount
+from ansible.module_utils.pycompat24 import get_exception
+from ansible.module_utils.six import iteritems
+import os
+import re
+
+
 DOCUMENTATION = '''
 ---
 module: mount
 short_description: Control active and configured mount points
 description:
-     - This module controls active and configured mount points in C(/etc/fstab).
+  - This module controls active and configured mount points in C(/etc/fstab).
+author:
+  - Ansible Core Team
+  - Seth Vidal
 version_added: "0.6"
 options:
   name:
     description:
-      - "path to the mount point, eg: C(/mnt/files)"
+      - Path to the mount point (e.g. C(/mnt/files))
     required: true
   src:
     description:
-      - device to be mounted on I(name).
-    required: true
+      - Device to be mounted on I(name). Required when I(state) set to
+        C(present) or C(mounted).
+    required: false
+    default: null
   fstype:
     description:
-      - file-system type
-    required: true
+      - Filesystem type. Required when I(state) is C(present) or C(mounted).
+    required: false
+    default: null
   opts:
     description:
-      - mount options (see fstab(5))
+      - Mount options (see fstab(5)).
     required: false
     default: null
   dump:
     description:
-      - "dump (see fstab(5)), Note that if nulled, C(state=present) will cease to work and duplicate entries will be made with subsequent runs."
+      - Dump (see fstab(5)). Note that if set to C(null) and I(state) set to
+        C(present), it will cease to work and duplicate entries will be made
+        with subsequent runs.
     required: false
     default: 0
   passno:
     description:
-      - "passno (see fstab(5)), Note that if nulled, C(state=present) will cease to work and duplicate entries will be made with subsequent runs."
+      - Passno (see fstab(5)). Note that if set to C(null) and I(state) set to
+        C(present), it will cease to work and duplicate entries will be made
+        with subsequent runs.
     required: false
     default: 0
   state:
     description:
-      - If C(mounted) or C(unmounted), the device will be actively mounted or unmounted as needed and appropriately configured in I(fstab).
-      - C(absent) and C(present) only deal with I(fstab) but will not affect current mounting.
-      - If specifying C(mounted) and the mount point is not present, the mount point will be created. Similarly.
+      - If C(mounted) or C(unmounted), the device will be actively mounted or
+        unmounted as needed and appropriately configured in I(fstab).
+      - C(absent) and C(present) only deal with I(fstab) but will not affect
+        current mounting.
+      - If specifying C(mounted) and the mount point is not present, the mount
+        point will be created. Similarly.
       - Specifying C(absent) will remove the mount point directory.
     required: true
-    choices: [ "present", "absent", "mounted", "unmounted" ]
+    choices: ["present", "absent", "mounted", "unmounted"]
   fstab:
     description:
-      - file to use instead of C(/etc/fstab). You shouldn't use that option
+      - File to use instead of C(/etc/fstab). You shouldn't use that option
         unless you really know what you are doing. This might be useful if
         you need to configure mountpoints in a chroot environment.
     required: false
     default: /etc/fstab
-
-author:
-    - Ansible Core Team
-    - Seth Vidal
 '''
-EXAMPLES = '''
-# Mount DVD read-only
-- mount: name=/mnt/dvd src=/dev/sr0 fstype=iso9660 opts=ro state=present
-
-# Mount up device by label
-- mount: name=/srv/disk src='LABEL=SOME_LABEL' fstype=ext4 state=present
 
-# Mount up device by UUID
-- mount: name=/home src='UUID=b3e48f45-f933-4c8e-a700-22a159ec9077' fstype=xfs opts=noatime state=present
+EXAMPLES = '''
+- name: Mount DVD read-only
+  mount:
+    name: /mnt/dvd
+    src: /dev/sr0
+    fstype: iso9660
+    opts: ro
+    state: present
+
+- name: Mount up device by label
+  mount:
+    name: /srv/disk
+    src: LABEL=SOME_LABEL
+    fstype: ext4
+    state: present
+
+- name: Mount up device by UUID
+  mount:
+    name: /home
+    src: UUID=b3e48f45-f933-4c8e-a700-22a159ec9077
+    fstype: xfs
+    opts: noatime
+    state: present
 '''
 
 
 def write_fstab(lines, dest):
-
     fs_w = open(dest, 'w')
+
     for l in lines:
         fs_w.write(l)
 
     fs_w.flush()
     fs_w.close()
 
+
 def _escape_fstab(v):
-    """ escape space (040), ampersand (046) and backslash (134) which are invalid in fstab fields """
+    """Escape invalid characters in fstab fields.
+
+    space (040)
+    ampersand (046)
+    backslash (134)
+    """
+
     if isinstance(v, int):
         return v
     else:
-        return v.replace('\\', '\\134').replace(' ', '\\040').replace('&', '\\046')
+        return(
+            v.
+            replace('\\', '\\134').
+            replace(' ', '\\040').
+            replace('&', '\\046'))
+
 
 def set_mount(module, **kwargs):
-    """ set/change a mount point location in fstab """
+    """Set/change a mount point location in fstab."""
 
     # kwargs: name, src, fstype, opts, dump, passno, state, fstab=/etc/fstab
     args = dict(
@@ -124,14 +170,19 @@ def set_mount(module, **kwargs):
     for line in open(args['fstab'], 'r').readlines():
         if not line.strip():
             to_write.append(line)
+
             continue
+
         if line.strip().startswith('#'):
             to_write.append(line)
+
             continue
+
         if len(line.split()) != 6:
             # not sure what this is or why it is here
             # but it is not our fault so leave it be
             to_write.append(line)
+
             continue
 
         ld = {}
@@ -139,9 +190,10 @@ def set_mount(module, **kwargs):
 
         if ld['name'] != escaped_args['name']:
             to_write.append(line)
+
             continue
 
-        # it exists - now see if what we have is different
+        # It exists - now see if what we have is different
         exists = True
         for t in ('src', 'fstype','opts', 'dump', 'passno'):
             if ld[t] != escaped_args[t]:
@@ -164,7 +216,7 @@ def set_mount(module, **kwargs):
 
 
 def unset_mount(module, **kwargs):
-    """ remove a mount point from fstab """
+    """Remove a mount point from fstab."""
 
     # kwargs: name, src, fstype, opts, dump, passno, state, fstab=/etc/fstab
     args = dict(
@@ -181,24 +233,31 @@ def unset_mount(module, **kwargs):
     for line in open(args['fstab'], 'r').readlines():
         if not line.strip():
             to_write.append(line)
+
             continue
+
         if line.strip().startswith('#'):
             to_write.append(line)
+
             continue
+
         if len(line.split()) != 6:
-            # not sure what this is or why it is here
-            # but it is not our fault so leave it be
+            # Not sure what this is or why it is here but it is not our fault
+            # so leave it be
             to_write.append(line)
+
             continue
 
         ld = {}
+
         ld['src'], ld['name'], ld['fstype'], ld['opts'], ld['dump'], ld['passno']  = line.split()
 
         if ld['name'] != escaped_name:
             to_write.append(line)
+
             continue
 
-        # if we got here we found a match - continue and mark changed
+        # If we got here we found a match - continue and mark changed
         changed = True
 
     if changed and not module.check_mode:
@@ -208,7 +267,7 @@ def unset_mount(module, **kwargs):
 
 
 def mount(module, **kwargs):
-    """ mount up a path or remount if needed """
+    """Mount up a path or remount if needed."""
 
     # kwargs: name, src, fstype, opts, dump, passno, state, fstab=/etc/fstab
     args = dict(
@@ -219,63 +278,127 @@ def mount(module, **kwargs):
     )
     args.update(kwargs)
 
-    mount_bin = module.get_bin_path('mount')
-
+    mount_bin = module.get_bin_path('mount', required=True)
     name = kwargs['name']
-
-    cmd = [ mount_bin, ]
+    cmd = [mount_bin]
 
     if ismount(name):
-        cmd += [ '-o', 'remount', ]
+        cmd += ['-o', 'remount']
 
     if get_platform().lower() == 'freebsd':
-        cmd += [ '-F', args['fstab'], ]
+        cmd += ['-F', args['fstab']]
+    elif get_platform().lower() == 'linux':
+        cmd += ['-T', args['fstab']]
 
-    cmd += [ name, ]
+    cmd += [name]
 
     rc, out, err = module.run_command(cmd)
+
     if rc == 0:
         return 0, ''
     else:
         return rc, out+err
 
+
 def umount(module, **kwargs):
-    """ unmount a path """
+    """Unmount a path."""
 
-    umount_bin = module.get_bin_path('umount')
+    umount_bin = module.get_bin_path('umount', required=True)
     name = kwargs['name']
     cmd = [umount_bin, name]
 
     rc, out, err = module.run_command(cmd)
+
     if rc == 0:
         return 0, ''
     else:
         return rc, out+err
 
-def main():
 
+# Note if we wanted to put this into module_utils we'd have to get permission
+# from @jupeter -- https://github.com/ansible/ansible-modules-core/pull/2923
+# @jtyr -- https://github.com/ansible/ansible-modules-core/issues/4439
+# and @abadger to relicense from GPLv3+
+def is_bind_mounted(module, dest, src=None, fstype=None):
+    """Return whether the dest is bind mounted
+
+    :arg module: The AnsibleModule (used for helper functions)
+    :arg dest: The directory to be mounted under. This is the primary means
+        of identifying whether the destination is mounted.
+    :kwarg src: The source directory. If specified, this is used to help
+        ensure that we are detecting that the correct source is mounted there.
+    :kwarg fstype: The filesystem type. If specified this is also used to
+        help ensure that we are detecting the right mount.
+    :returns: True if the dest is mounted with src otherwise False.
+    """
+
+    is_mounted = False
+    bin_path = module.get_bin_path('mount', required=True)
+    cmd = '%s -l' % bin_path
+
+    if get_platform().lower() == 'linux':
+        bin_path = module.get_bin_path('findmnt', required=True)
+        cmd = '%s -nr %s' % (bin_path, dest)
+
+    rc, out, err = module.run_command(cmd)
+    mounts = []
+
+    if len(out):
+        mounts = out.strip().split('\n')
+
+    mount_pattern = re.compile('\[(.*)\]')
+
+    for mnt in mounts:
+        arguments = mnt.split()
+
+        if get_platform().lower() == 'linux':
+            result = mount_pattern.search(arguments[1])
+
+            if len(result.groups()) == 1:
+                if arguments[0] == dest:
+                    is_mounted = True
+        elif (
+                (arguments[0] == src or src is None) and
+                arguments[2] == dest and
+                (arguments[4] == fstype or fstype is None)):
+            is_mounted = True
+
+        if is_mounted:
+            break
+
+    return is_mounted
+
+
+def main():
     module = AnsibleModule(
-        argument_spec = dict(
-            state  = dict(required=True, choices=['present', 'absent', 'mounted', 'unmounted']),
-            name   = dict(required=True),
-            opts   = dict(default=None),
-            passno = dict(default=None, type='str'),
-            dump   = dict(default=None),
-            src    = dict(required=True),
-            fstype = dict(required=True),
-            fstab  = dict(default='/etc/fstab')
+        argument_spec=dict(
+            dump=dict(),
+            fstab=dict(default='/etc/fstab'),
+            fstype=dict(),
+            name=dict(required=True, type='path'),
+            opts=dict(),
+            passno=dict(type='str'),
+            src=dict(type='path'),
+            state=dict(
+                required=True,
+                choices=['present', 'absent', 'mounted', 'unmounted']),
         ),
-        supports_check_mode=True
+        supports_check_mode=True,
+        required_if=(
+            ['state', 'mounted', ['src', 'fstype']],
+            ['state', 'present', ['src', 'fstype']]
+        )
     )
 
-
     changed = False
-    rc = 0
     args = {
-        'name': module.params['name'],
-        'src': module.params['src'],
-        'fstype': module.params['fstype']
+        'name': module.params['name']
     }
+
+    if module.params['src'] is not None:
+        args['src'] = module.params['src']
+    if module.params['fstype'] is not None:
+        args['fstype'] = module.params['fstype']
     if module.params['passno'] is not None:
         args['passno'] = module.params['passno']
     if module.params['opts'] is not None:
@@ -285,87 +408,92 @@ def main():
     if module.params['fstab'] is not None:
         args['fstab'] = module.params['fstab']
 
-    # if fstab file does not exist, we first need to create it. This mainly
-    # happens when fstab optin is passed to the module.
+    # If fstab file does not exist, we first need to create it. This mainly
+    # happens when fstab option is passed to the module.
     if not os.path.exists(args['fstab']):
         if not os.path.exists(os.path.dirname(args['fstab'])):
             os.makedirs(os.path.dirname(args['fstab']))
-        open(args['fstab'],'a').close()
 
-    # absent == remove from fstab and unmounted
-    # unmounted == do not change fstab state, but unmount
-    # present == add to fstab, do not change mount state
-    # mounted == add to fstab if not there and make sure it is mounted, if it has changed in fstab then remount it
+        open(args['fstab'], 'a').close()
+
+    # absent:
+    #   Remove from fstab and unmounted.
+    # unmounted:
+    #   Do not change fstab state, but unmount.
+    # present:
+    #   Add to fstab, do not change mount state.
+    # mounted:
+    #   Add to fstab if not there and make sure it is mounted. If it has
+    #   changed in fstab then remount it.
 
     state = module.params['state']
-    name  = module.params['name']
+    name = module.params['name']
+
     if state == 'absent':
         name, changed = unset_mount(module, **args)
+
         if changed and not module.check_mode:
-            if ismount(name):
-                res,msg  = umount(module, **args)
+            if ismount(name) or is_bind_mounted(module, name):
+                res, msg = umount(module, **args)
+
                 if res:
-                    module.fail_json(msg="Error unmounting %s: %s" % (name, msg))
+                    module.fail_json(
+                        msg="Error unmounting %s: %s" % (name, msg))
 
             if os.path.exists(name):
                 try:
                     os.rmdir(name)
                 except (OSError, IOError), e:
                     module.fail_json(msg="Error rmdir %s: %s" % (name, str(e)))
-
-        module.exit_json(changed=changed, **args)
-
-    if state == 'unmounted':
-        if ismount(name):
+    elif state == 'unmounted':
+        if ismount(name) or is_bind_mounted(module, name):
             if not module.check_mode:
-                res,msg  = umount(module, **args)
-                if res:
-                    module.fail_json(msg="Error unmounting %s: %s" % (name, msg))
-            changed = True
+                res, msg = umount(module, **args)
 
-        module.exit_json(changed=changed, **args)
+                if res:
+                    module.fail_json(
+                        msg="Error unmounting %s: %s" % (name, msg))
 
-    if state in ['mounted', 'present']:
-        if state == 'mounted':
-            if not os.path.exists(name) and not module.check_mode:
-                try:
-                    os.makedirs(name)
-                except (OSError, IOError), e:
-                    module.fail_json(msg="Error making dir %s: %s" % (name, str(e)))
+            changed = True
+    elif state == 'mounted':
+        if not os.path.exists(name) and not module.check_mode:
+            try:
+                os.makedirs(name)
+            except (OSError, IOError):
+                e = get_exception()
+                module.fail_json(
+                    msg="Error making dir %s: %s" % (name, str(e)))
 
         name, changed = set_mount(module, **args)
-        if state == 'mounted':
-            res = 0
-            if ismount(name):
-                if changed and not module.check_mode:
-                    res,msg = mount(module, **args)
-            elif 'bind' in args.get('opts', []):
-                changed = True
-                cmd = 'mount -l'
-                rc, out, err = module.run_command(cmd)
-                allmounts = out.split('\n')
-                for mounts in allmounts[:-1]:
-                    arguments = mounts.split()
-                    if arguments[0] == args['src'] and arguments[2] == args['name'] and arguments[4] == args['fstype']:
-                        changed = False
-                if changed:
-                    res,msg = mount(module, **args)
-            else:
+        res = 0
+
+        if ismount(name):
+            if not module.check_mode:
+                res, msg = mount(module, **args)
                 changed = True
-                if not module.check_mode:
-                    res,msg = mount(module, **args)
+        elif 'bind' in args.get('opts', []):
+            changed = True
 
+            if is_bind_mounted(module, name, args['src'], args['fstype']):
+                changed = False
 
-            if res:
-                module.fail_json(msg="Error mounting %s: %s" % (name, msg))
+            if changed and not module.check_mode:
+                res, msg = mount(module, **args)
+        else:
+            changed = True
 
+            if not module.check_mode:
+                res, msg = mount(module, **args)
 
-        module.exit_json(changed=changed, **args)
+        if res:
+            module.fail_json(msg="Error mounting %s: %s" % (name, msg))
+    elif state == 'present':
+        name, changed = set_mount(module, **args)
+    else:
+        module.fail_json(msg='Unexpected position reached')
 
-    module.fail_json(msg='Unexpected position reached')
+    module.exit_json(changed=changed, **args)
 
-# import module snippets
-from ansible.module_utils.basic import *
-from ansible.module_utils.ismount import *
 
-main()
+if __name__ == '__main__':
+    main()