diff --git a/releasenotes/notes/mountpoint-detection-096734f0097eb75a.yaml b/releasenotes/notes/mountpoint-detection-096734f0097eb75a.yaml new file mode 100644 index 0000000000..0971bbfbe8 --- /dev/null +++ b/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. diff --git a/trove/guestagent/common/operating_system.py b/trove/guestagent/common/operating_system.py index aacb1391a2..282d8857b9 100644 --- a/trove/guestagent/common/operating_system.py +++ b/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 diff --git a/trove/guestagent/volume.py b/trove/guestagent/volume.py index 5427a2fe28..2558b0bf60 100644 --- a/trove/guestagent/volume.py +++ b/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): diff --git a/trove/tests/unittests/guestagent/test_volume.py b/trove/tests/unittests/guestagent/test_volume.py index efbb96bb7c..6ef160659d 100644 --- a/trove/tests/unittests/guestagent/test_volume.py +++ b/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()