openstack
/
manila
Code Issues Proposed changes

Make block devices mounts permanent within service instances 

Restart of service VM does not save mount state of block device for share.
So, if we do not restart m-shr we have export to dir that links not to
block device, but to root filesystem of service VM.

Changes:
- added sync method of temp and permanent fs mount files;
- changed lock value for mount operations from share id to service instance id,
  because any action changes common config file;
- updated related unittests;

Change-Id: I874d84f5342208906379aba5572540a984f79f03
Closes-Bug: #1367731
This commit is contained in:
Your Name 2014-09-11 03:28:36 -04:00 committed by Valeriy Ponomaryov
parent 7d79a8cc5b
commit 9d4059d5ea
3 changed files with 95 additions and 23 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
manila/common/constants.py
View File

@ -28,6 +28,9 @@ SECURITY_SERVICES_ALLOWED_TYPES = ['active_directory', 'ldap', 'kerberos']
NFS_EXPORTS_FILE = '/etc/exports'
NFS_EXPORTS_FILE_TEMP = '/var/lib/nfs/etab'
MOUNT_FILE = '/etc/fstab'
MOUNT_FILE_TEMP = '/etc/mtab'
# Below represented ports are ranges (from, to)
CIFS_PORTS = (
("tcp", (445, 445)),

30
manila/share/drivers/generic.py
View File

@ -212,6 +212,25 @@ class GenericShareDriver(driver.ExecuteMixin, driver.ShareDriver):
return True
return False
def _sync_mount_temp_and_perm_files(self, server_details):
"""Sync temporary and permanent files for mounted filesystems."""
try:
self._ssh_exec(
server_details,
['sudo', 'cp', const.MOUNT_FILE_TEMP, const.MOUNT_FILE],
)
except exception.ProcessExecutionError as e:
LOG.error(_("Failed to sync mount files on server '%s'."),
server_details['instance_id'])
raise exception.ShareBackendException(msg=six.text_type(e))
try:
# Remount it to avoid postponed point of failure
self._ssh_exec(server_details, ['sudo', 'mount', '-a'])
except exception.ProcessExecutionError as e:
LOG.error(_("Failed to mount all shares on server '%s'."),
server_details['instance_id'])
raise exception.ShareBackendException(msg=six.text_type(e))
def _mount_device(self, share, server_details, volume):
"""Mounts block device to the directory on service vm.
@ -219,7 +238,8 @@ class GenericShareDriver(driver.ExecuteMixin, driver.ShareDriver):
mounted yet.
"""
@utils.synchronized('generic_driver_mounts_%s' % share['id'])
@utils.synchronized('generic_driver_mounts_'
'%s' % server_details['instance_id'])
def _mount_device_with_lock():
mount_path = self._get_mount_path(share)
log_data = {
@ -236,6 +256,9 @@ class GenericShareDriver(driver.ExecuteMixin, driver.ShareDriver):
mount_path])
mount_cmd.extend(['&& sudo chmod 777', mount_path])
self._ssh_exec(server_details, mount_cmd)
# Add mount permanently
self._sync_mount_temp_and_perm_files(server_details)
else:
LOG.warning(_("Mount point '%(path)s' already exists on "
"server '%(server)s'."), log_data)
@ -246,7 +269,8 @@ class GenericShareDriver(driver.ExecuteMixin, driver.ShareDriver):
def _unmount_device(self, share, server_details):
"""Unmounts block device from directory on service vm."""
@utils.synchronized('generic_driver_mounts_%s' % share['id'])
@utils.synchronized('generic_driver_mounts_'
'%s' % server_details['instance_id'])
def _unmount_device_with_lock():
mount_path = self._get_mount_path(share)
log_data = {
@ -259,6 +283,8 @@ class GenericShareDriver(driver.ExecuteMixin, driver.ShareDriver):
unmount_cmd = ['sudo umount', mount_path, '&& sudo rmdir',
mount_path]
self._ssh_exec(server_details, unmount_cmd)
# Remove mount permanently
self._sync_mount_temp_and_perm_files(server_details)
else:
LOG.warning(_("Mount point '%(path)s' does not exist on "
"server '%(server)s'."), log_data)

85
manila/tests/share/drivers/test_generic.py
View File

@ -20,6 +20,7 @@ import os
import mock
from oslo.config import cfg
from manila.common import constants as const
from manila import compute
from manila import context
from manila import exception
@ -197,6 +198,8 @@ class GenericShareDriverTestCase(test.TestCase):
volume = {'mountpoint': 'fake_mount_point'}
self.stubs.Set(self._driver, '_is_device_mounted',
mock.Mock(return_value=False))
self.stubs.Set(self._driver, '_sync_mount_temp_and_perm_files',
mock.Mock())
self.stubs.Set(self._driver, '_get_mount_path',
mock.Mock(return_value=mount_path))
self.stubs.Set(self._driver, '_ssh_exec',
@ -207,6 +210,8 @@ class GenericShareDriverTestCase(test.TestCase):
self._driver._get_mount_path.assert_called_once_with(self.share)
self._driver._is_device_mounted.assert_called_once_with(
self.share, server, volume)
self._driver._sync_mount_temp_and_perm_files.assert_called_once_with(
server)
self._driver._ssh_exec.assert_called_once_with(
server,
['sudo mkdir -p', mount_path,
@ -215,7 +220,6 @@ class GenericShareDriverTestCase(test.TestCase):
)
def test_mount_device_present(self):
server = {'instance_id': 'fake_server_id'}
mount_path = '/fake/mount/path'
volume = {'mountpoint': 'fake_mount_point'}
self.stubs.Set(self._driver, '_is_device_mounted',
@ -224,15 +228,14 @@ class GenericShareDriverTestCase(test.TestCase):
mock.Mock(return_value=mount_path))
self.stubs.Set(generic.LOG, 'warning', mock.Mock())
self._driver._mount_device(self.share, server, volume)
self._driver._mount_device(self.share, self.server, volume)
self._driver._get_mount_path.assert_called_once_with(self.share)
self._driver._is_device_mounted.assert_called_once_with(
self.share, server, volume)
self.share, self.server, volume)
generic.LOG.warning.assert_called_once_with(mock.ANY, mock.ANY)
def test_mount_device_exception_raised(self):
server = {'instance_id': 'fake_server_id'}
volume = {'mountpoint': 'fake_mount_point'}
self.stubs.Set(self._driver, '_get_mount_path',
mock.Mock(return_value='fake'))
@ -243,17 +246,19 @@ class GenericShareDriverTestCase(test.TestCase):
exception.ShareBackendException,
self._driver._mount_device,
self.share,
server,
self.server,
volume,
)
self._driver._get_mount_path.assert_called_once_with(self.share)
self._driver._is_device_mounted.assert_called_once_with(
self.share, server, volume)
self.share, self.server, volume)
def test_unmount_device_present(self):
mount_path = '/fake/mount/path'
self.stubs.Set(self._driver, '_is_device_mounted',
mock.Mock(return_value=True))
self.stubs.Set(self._driver, '_sync_mount_temp_and_perm_files',
mock.Mock())
self.stubs.Set(self._driver, '_get_mount_path',
mock.Mock(return_value=mount_path))
self.stubs.Set(self._driver, '_ssh_exec',
@ -264,13 +269,14 @@ class GenericShareDriverTestCase(test.TestCase):
self._driver._get_mount_path.assert_called_once_with(self.share)
self._driver._is_device_mounted.assert_called_once_with(
self.share, self.server)
self._driver._sync_mount_temp_and_perm_files.assert_called_once_with(
self.server)
self._driver._ssh_exec.assert_called_once_with(
self.server,
['sudo umount', mount_path, '&& sudo rmdir', mount_path],
)
def test_unmount_device_not_present(self):
server = {'instance_id': 'fake_server_id'}
mount_path = '/fake/mount/path'
self.stubs.Set(self._driver, '_is_device_mounted',
mock.Mock(return_value=False))
@ -278,16 +284,15 @@ class GenericShareDriverTestCase(test.TestCase):
mock.Mock(return_value=mount_path))
self.stubs.Set(generic.LOG, 'warning', mock.Mock())
self._driver._unmount_device(self.share, server)
self._driver._unmount_device(self.share, self.server)
self._driver._get_mount_path.assert_called_once_with(self.share)
self._driver._is_device_mounted.assert_called_once_with(
self.share, server)
self.share, self.server)
generic.LOG.warning.assert_called_once_with(mock.ANY, mock.ANY)
def test_is_device_mounted_true(self):
volume = {'mountpoint': 'fake_mount_point', 'id': 'fake_id'}
server = {'instance_id': 'fake_server_id'}
mount_path = '/fake/mount/path'
mounts = "%(dev)s on %(path)s" % {'dev': volume['mountpoint'],
'path': mount_path}
@ -296,15 +301,15 @@ class GenericShareDriverTestCase(test.TestCase):
self.stubs.Set(self._driver, '_get_mount_path',
mock.Mock(return_value=mount_path))
result = self._driver._is_device_mounted(self.share, server, volume)
result = self._driver._is_device_mounted(
self.share, self.server, volume)
self._driver._get_mount_path.assert_called_once_with(self.share)
self._driver._ssh_exec.assert_called_once_with(
server, ['sudo', 'mount'])
self.server, ['sudo', 'mount'])
self.assertEqual(result, True)
def test_is_device_mounted_true_no_volume_provided(self):
server = {'instance_id': 'fake_server_id'}
mount_path = '/fake/mount/path'
mounts = "/fake/dev/path on %(path)s type fake" % {'path': mount_path}
self.stubs.Set(self._driver, '_ssh_exec',
@ -312,15 +317,14 @@ class GenericShareDriverTestCase(test.TestCase):
self.stubs.Set(self._driver, '_get_mount_path',
mock.Mock(return_value=mount_path))
result = self._driver._is_device_mounted(self.share, server)
result = self._driver._is_device_mounted(self.share, self.server)
self._driver._get_mount_path.assert_called_once_with(self.share)
self._driver._ssh_exec.assert_called_once_with(
server, ['sudo', 'mount'])
self.server, ['sudo', 'mount'])
self.assertEqual(result, True)
def test_is_device_mounted_false(self):
server = {'instance_id': 'fake_server_id'}
mount_path = '/fake/mount/path'
volume = {'mountpoint': 'fake_mount_point', 'id': 'fake_id'}
mounts = "%(dev)s on %(path)s" % {'dev': '/fake',
@ -330,15 +334,15 @@ class GenericShareDriverTestCase(test.TestCase):
self.stubs.Set(self._driver, '_get_mount_path',
mock.Mock(return_value=mount_path))
result = self._driver._is_device_mounted(self.share, server, volume)
result = self._driver._is_device_mounted(
self.share, self.server, volume)
self._driver._get_mount_path.assert_called_once_with(self.share)
self._driver._ssh_exec.assert_called_once_with(
server, ['sudo', 'mount'])
self.server, ['sudo', 'mount'])
self.assertEqual(result, False)
def test_is_device_mounted_false_no_volume_provided(self):
server = {'instance_id': 'fake_server_id'}
mount_path = '/fake/mount/path'
mounts = "%(path)s" % {'path': 'fake'}
self.stubs.Set(self._driver, '_ssh_exec',
@ -346,13 +350,52 @@ class GenericShareDriverTestCase(test.TestCase):
self.stubs.Set(self._driver, '_get_mount_path',
mock.Mock(return_value=mount_path))
result = self._driver._is_device_mounted(self.share, server)
result = self._driver._is_device_mounted(self.share, self.server)
self._driver._get_mount_path.assert_called_once_with(self.share)
self._driver._ssh_exec.assert_called_once_with(
server, ['sudo', 'mount'])
self.server, ['sudo', 'mount'])
self.assertEqual(result, False)
def test_sync_mount_temp_and_perm_files(self):
self.stubs.Set(self._driver, '_ssh_exec', mock.Mock())
self._driver._sync_mount_temp_and_perm_files(self.server)
self._driver._ssh_exec.has_calls(
mock.call(
self.server,
['sudo', 'cp', const.MOUNT_FILE_TEMP, const.MOUNT_FILE]),
mock.call(self.server, ['sudo', 'mount', '-a']))
def test_sync_mount_temp_and_perm_files_raise_error_on_copy(self):
self.stubs.Set(self._driver, '_ssh_exec',
mock.Mock(side_effect=exception.ProcessExecutionError))
self.assertRaises(
exception.ShareBackendException,
self._driver._sync_mount_temp_and_perm_files,
self.server
)
self._driver._ssh_exec.assert_called_once_with(
self.server,
['sudo', 'cp', const.MOUNT_FILE_TEMP, const.MOUNT_FILE])
def test_sync_mount_temp_and_perm_files_raise_error_on_mount(self):
def raise_error_on_mount(*args, **kwargs):
if args[1][1] == 'cp':
raise exception.ProcessExecutionError()
self.stubs.Set(self._driver, '_ssh_exec',
mock.Mock(side_effect=raise_error_on_mount))
self.assertRaises(
exception.ShareBackendException,
self._driver._sync_mount_temp_and_perm_files,
self.server
)
self._driver._ssh_exec.has_calls(
mock.call(
self.server,
['sudo', 'cp', const.MOUNT_FILE_TEMP, const.MOUNT_FILE]),
mock.call(self.server, ['sudo', 'mount', '-a']))
def test_get_mount_path(self):
result = self._driver._get_mount_path(self.share)
self.assertEqual(result, os.path.join(CONF.share_mount_path,