Merge "Fix mountpoint detection"

releasenotes/notes/mountpoint-detection-096734f0097eb75a.yaml

@@ -0,0 +1,4 @@
+---
+fixes:
+  - Improved mountpoint detection by running it as root. This prevents guests
+    that have undiscoverable mount points from failing to unmount.

trove/guestagent/common/operating_system.py

@@ -817,3 +817,23 @@ def _build_command_options(options):
     """
 
     return ['-' + item[0] for item in options if item[1]]
+
+
+def get_device(path, as_root=False):
+    """Get the device that a given path exists on."""
+    stdout = _execute_shell_cmd('df', [], path, as_root=as_root)
+    return stdout.splitlines()[1].split()[0]
+
+
+def is_mount(path):
+    """Check if the given directory path is a mountpoint. Try the standard
+    ismount first. This fails if the path is not accessible though, so resort
+    to checking as the root user (which is slower).
+    """
+    if os.access(path, os.R_OK):
+        return os.path.ismount(path)
+    if not exists(path, is_directory=True, as_root=True):
+        return False
+    directory_dev = get_device(path, as_root=True)
+    parent_dev = get_device(os.path.join(path, '..'), as_root=True)
+    return directory_dev != parent_dev

trove/guestagent/volume.py

@@ -135,7 +135,7 @@ class VolumeDevice(object):
                 "Error resizing the filesystem: %s") % self.device_path)
 
     def unmount(self, mount_point):
-        if os.path.exists(mount_point):
+        if operating_system.is_mount(mount_point):
             cmd = "sudo umount %s" % mount_point
             child = pexpect.spawn(cmd)
             child.expect(pexpect.EOF)
@@ -151,16 +151,10 @@ class VolumeDevice(object):
 
     def mount_points(self, device_path):
         """Returns a list of mount points on the specified device."""
-        try:
-            cmd = "grep %s /etc/mtab | awk '{print $2}'" % device_path
-            stdout, stderr = utils.execute(cmd, shell=True)
-            return stdout.strip().split('\n')
-
-        except ProcessExecutionError:
-            LOG.exception(_("Error retrieving mount points"))
-            raise GuestError(original_message=_(
-                "Could not obtain a list of mount points for device: %s") %
-                device_path)
+        stdout, stderr = utils.execute(
+            "grep %s /etc/mtab" % device_path,
+            shell=True, check_exit_code=[0, 1])
+        return [entry.strip().split()[1] for entry in stdout.splitlines()]
 
     def set_readahead_size(self, readahead_size,
                            execute_function=utils.execute):

trove/tests/unittests/guestagent/test_volume.py

@@ -19,6 +19,7 @@ import pexpect
 
 from trove.common.exception import GuestError, ProcessExecutionError
 from trove.common import utils
+from trove.guestagent.common import operating_system
 from trove.guestagent import volume
 from trove.tests.unittests import trove_testtools
 
@@ -160,8 +161,8 @@ class VolumeDeviceTest(trove_testtools.TestCase):
 
     @patch.object(pexpect, 'spawn', Mock())
     def _test_unmount(self, positive=True):
-        origin_ = os.path.exists
-        os.path.exists = MagicMock(return_value=positive)
+        origin_is_mount = operating_system.is_mount
+        operating_system.is_mount = MagicMock(return_value=positive)
         fake_spawn = _setUp_fake_spawn()
 
         self.volumeDevice.unmount('/mnt/volume')
@@ -169,19 +170,15 @@ class VolumeDeviceTest(trove_testtools.TestCase):
         if not positive:
             COUNT = 0
         self.assertEqual(COUNT, fake_spawn.expect.call_count)
-        os.path.exists = origin_
+        operating_system.is_mount = origin_is_mount
 
-    @patch.object(utils, 'execute', return_value=('/var/lib/mysql', ''))
+    @patch.object(utils, 'execute')
     def test_mount_points(self, mock_execute):
+        mock_execute.return_value = (
+            ("/dev/vdb /var/lib/mysql xfs rw 0 0", ""))
         mount_point = self.volumeDevice.mount_points('/dev/vdb')
         self.assertEqual(['/var/lib/mysql'], mount_point)
 
-    @patch.object(utils, 'execute', side_effect=ProcessExecutionError)
-    @patch('trove.guestagent.volume.LOG')
-    def test_fail_mount_points(self, mock_logging, mock_execute):
-        self.assertRaises(GuestError, self.volumeDevice.mount_points,
-                          '/mnt/volume')
-
     def test_set_readahead_size(self):
         origin_check_device_exists = self.volumeDevice._check_device_exists
         self.volumeDevice._check_device_exists = MagicMock()