Fix mountpoint detection

During guestagent volume operations the calls to unmount check if the
volume is mounted first. The python function os.path.ismount returns
False if the directory if not readable by the current user, breaking
this functionality for some datastores.

The symptom of this failure is that the device ends up mounted twice
during prepare and then fails to unmount fully during resize.

The fix is to create a custom is_mount function that runs as the root user.

Change-Id: I151402717386230371bafcedc170d70b3588e912
Closes-Bug: #1645773

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:
+        stdout, stderr = utils.execute(
-            cmd = "grep %s /etc/mtab | awk '{print $2}'" % device_path
+            "grep %s /etc/mtab" % device_path,
-            stdout, stderr = utils.execute(cmd, shell=True)
+            shell=True, check_exit_code=[0, 1])
-            return stdout.strip().split('\n')
+        return [entry.strip().split()[1] for entry in stdout.splitlines()]
-
-        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)
 
     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
+        origin_is_mount = operating_system.is_mount
-        os.path.exists = MagicMock(return_value=positive)
+        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()