11209: Replace "--unmount /path/..." with "--unmount-all /path".

diff --git a/services/fuse/arvados_fuse/command.py b/services/fuse/arvados_fuse/command.py
index da44df7ae3fbf67451008c1cd5cbb465c6cad68c..45e902f87c28cdeb9c7f52767bde22db4f573309 100644 (file)
--- a/services/fuse/arvados_fuse/command.py
+++ b/services/fuse/arvados_fuse/command.py
@@ -12,7 +12,7 @@ import time
 
 import arvados.commands._util as arv_cmd
 from arvados_fuse import *
-from arvados_fuse.unmount import unmount, unmount_all
+from arvados_fuse.unmount import unmount
 from arvados_fuse._version import __version__
 
 class ArgumentParser(argparse.ArgumentParser):
@@ -90,10 +90,13 @@ class ArgumentParser(argparse.ArgumentParser):
 
         self.add_argument('--crunchstat-interval', type=float, help="Write stats to stderr every N seconds (default disabled)", default=0)
 
-        self.add_argument('--unmount', action='store_true', default=False,
-                          help="Forcefully unmount the specified mountpoint (if it's a fuse mount) and exit. Use /path/... to unmount all fuse mounts below /path as well as /path itself.")
-        self.add_argument('--replace', action='store_true', default=False,
-                          help="If a fuse mount is already present at mountpoint, forcefully unmount it before mounting")
+        unmount = self.add_mutually_exclusive_group()
+        unmount.add_argument('--unmount', action='store_true', default=False,
+                             help="Forcefully unmount the specified mountpoint (if it's a fuse mount) and exit.")
+        unmount.add_argument('--unmount-all', action='store_true', default=False,
+                             help="Forcefully unmount every fuse mount at or below the specified mountpoint and exit.")
+        unmount.add_argument('--replace', action='store_true', default=False,
+                             help="If a fuse mount is already present at mountpoint, forcefully unmount it before mounting")
         self.add_argument('--unmount-timeout',
                           type=float, default=2.0,
                           help="Time to wait for graceful shutdown after --exec program exits and filesystem is unmounted")
@@ -124,7 +127,8 @@ class Mount(object):
 
     def __enter__(self):
         if self.args.replace:
-            unmount(self.args.mountpoint, timeout=self.args.unmount_timeout)
+            unmount(path=self.args.mountpoint,
+                    timeout=self.args.unmount_timeout)
         llfuse.init(self.operations, self.args.mountpoint, self._fuse_options())
         if self.daemon:
             daemon.DaemonContext(
@@ -152,8 +156,10 @@ class Mount(object):
                                 self.args.unmount_timeout)
 
     def run(self):
-        if self.args.unmount:
-            unmount_all(self.args.mountpoint, timeout=self.args.unmount_timeout)
+        if self.args.unmount or self.args.unmount_all:
+            unmount(path=self.args.mountpoint,
+                    timeout=self.args.unmount_timeout,
+                    recursive=self.args.unmount_all)
         elif self.args.exec_args:
             self._run_exec()
         else:

diff --git a/services/fuse/arvados_fuse/unmount.py b/services/fuse/arvados_fuse/unmount.py
index 7a7464d4da8a0fbe513eadf4ff8e26b9cd04dc07..db78ddc738311b40914087c7b07fd0e27d6700f1 100644 (file)
--- a/services/fuse/arvados_fuse/unmount.py
+++ b/services/fuse/arvados_fuse/unmount.py
@@ -26,25 +26,7 @@ def mountinfo():
     return mi
 
 
-def unmount_all(path, timeout=10):
-    if not path.endswith("/..."):
-        return unmount(path, timeout=timeout)
-    root = os.path.realpath(path[:-4])
-
-    paths = []
-    for m in mountinfo():
-        if m.path == root or m.path.startswith(root+"/"):
-            paths.append(m.path)
-            if not m.is_fuse:
-                raise Exception(
-                    "cannot unmount {}: non-fuse mountpoint {}".format(
-                        path, m))
-    for path in sorted(paths, key=len, reverse=True):
-        unmount(path, timeout=timeout)
-    return len(paths) > 0
-
-
-def unmount(path, timeout=10):
+def unmount(path, timeout=10, recursive=False):
     """Unmount the fuse mount at path.
 
     Unmounting is done by writing 1 to the "abort" control file in
@@ -61,6 +43,19 @@ def unmount(path, timeout=10):
 
     path = os.path.realpath(path)
 
+    if recursive:
+        paths = []
+        for m in mountinfo():
+            if m.path == path or m.path.startswith(path+"/"):
+                paths.append(m.path)
+                if not m.is_fuse:
+                    raise Exception(
+                        "cannot unmount {}: non-fuse mountpoint {}".format(
+                            path, m))
+        for path in sorted(paths, key=len, reverse=True):
+            unmount(path, timeout=timeout, recursive=False)
+        return len(paths) > 0
+
     was_mounted = False
     attempted = False
     if timeout is None:

diff --git a/services/fuse/tests/test_unmount.py b/services/fuse/tests/test_unmount.py
index 9e2ebd71282029fff0066ec08e57d0f93100fff7..0a2837e737cf626ebd44042e4a2148a18bdad3d6 100644 (file)
--- a/services/fuse/tests/test_unmount.py
+++ b/services/fuse/tests/test_unmount.py
@@ -1,5 +1,6 @@
 import os
 import subprocess
+import time
 
 from integration_test import IntegrationTest
 
@@ -30,15 +31,31 @@ class UnmountTest(IntegrationTest):
         for m in subprocess.check_output(['mount']).splitlines():
             self.assertNotIn(' '+self.mnt+' ', m)
 
+    def _mounted(self, mounts):
+        all_mounts = subprocess.check_output(['mount', '-t', 'fuse.test'])
+        return [m for m in mounts
+                if ' '+m+' ' in all_mounts]
+
     def test_unmount_children(self):
         for d in ['foo', 'foo/bar', 'bar']:
             mnt = self.tmp+'/'+d
             os.mkdir(mnt)
             self.to_delete.insert(0, mnt)
+        mounts = []
         for d in ['bar', 'foo/bar']:
             mnt = self.tmp+'/'+d
+            mounts.append(mnt)
             subprocess.check_call(
                 ['arv-mount', '--subtype', 'test', mnt])
-        subprocess.check_call(['arv-mount', '--unmount', self.tmp+'/...'])
-        for m in subprocess.check_output(['mount']).splitlines():
-            self.assertNotIn(' '+self.tmp+'/', m)
+
+        # Wait for mounts to attach
+        deadline = time.time() + 10
+        while self._mounted(mounts) != mounts:
+            time.sleep(0.1)
+            self.assertLess(time.time(), deadline)
+
+        self.assertEqual(mounts, self._mounted(mounts))
+        subprocess.check_call(['arv-mount', '--unmount', self.tmp])
+        self.assertEqual(mounts, self._mounted(mounts))
+        subprocess.check_call(['arv-mount', '--unmount-all', self.tmp])
+        self.assertEqual([], self._mounted(mounts))