openstack
/
manila
Code Issues Proposed changes

glusterfs vol layout: start volume cloned from snapshot 

When handling create_share_from_snapshot with glusterfs
volume layout, we do a snapshot clone gluster operation
that gives us a new volume (which will be used to back
the new share). 'snapshot clone' does not start the
resultant volume, we have explicitly start it from Manila.
So far the volume layout code did not bother about it,
rather the 'vol start' was called from glusterfs-native
driver. That however broke all other volume layout based
configs (ie. glusterfs driver with vol layout).

Fix this now by doing the 'vol start' call in the vol
layout code.

Change-Id: I63c13ce468a3227f09e381814f55e8c914fbef95
Closes-Bug: #1499347
(cherry picked from commit 4e4c8759a2)
This commit is contained in:
Csaba Henk 2015-09-29 09:49:19 +02:00
parent 552a80a5bf
commit be76d9e404
4 changed files with 93 additions and 25 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
manila/share/drivers/glusterfs/layout_volume.py
View File

@ -485,6 +485,17 @@ class GlusterfsVolumeMappedLayout(layout.GlusterfsShareLayoutBase):
export = self.driver._setup_via_manager(
{'share': share, 'manager': gmgr},
{'share': snapshot['share_instance'], 'manager': old_gmgr})
try:
gmgr.gluster_call(
'volume', 'start', gmgr.volume)
except exception.ProcessExecutionError as exc:
msg = (_("Error starting gluster volume. "
"Volume: %(volname)s, Error: %(error)s") %
{'volname': gmgr.volume, 'error': exc.stderr})
LOG.error(msg)
raise exception.GlusterfsException(msg)
self.gluster_used_vols.add(gmgr.qualified)
self.private_storage.update(share['id'], {'volume': gmgr.qualified})
return export

11
manila/share/drivers/glusterfs_native.py
View File

@ -126,12 +126,6 @@ class GlusterfsNativeShareDriver(driver.ExecuteMixin,
):
gluster_actions.append(('set', option, value))
if not gluster_mgr_parent:
# TODO(deepakcs) Remove this once ssl options can be
# set dynamically.
gluster_actions.append(('stop', '--mode=script'))
gluster_actions.append(('start',))
for action in gluster_actions:
try:
gluster_mgr.gluster_call(
@ -145,6 +139,11 @@ class GlusterfsNativeShareDriver(driver.ExecuteMixin,
LOG.error(msg)
raise exception.GlusterfsException(msg)
if not gluster_mgr_parent:
# TODO(deepakcs) Remove this once ssl options can be
# set dynamically.
common._restart_gluster_vol(gluster_mgr)
return gluster_mgr.export
@utils.synchronized("glusterfs_native_access", external=False)

77
manila/tests/share/drivers/glusterfs/test_layout_volume.py
View File

@ -715,7 +715,7 @@ class GlusterfsVolumeMappedLayoutTestCase(test.TestCase):
self.mock_object(old_gmgr, 'gluster_call',
mock.Mock(side_effect=[('', ''), ('', '')]))
self.mock_object(new_gmgr, 'gluster_call',
mock.Mock(side_effect=[('', ''), ('', '')]))
mock.Mock(side_effect=[('', '')]))
self.mock_object(new_gmgr, 'get_gluster_vol_option',
mock.Mock())
new_gmgr.get_gluster_vol_option.return_value = (
@ -738,6 +738,8 @@ class GlusterfsVolumeMappedLayoutTestCase(test.TestCase):
'force', '--mode=script'),
('snapshot', 'clone', volume, 'fake_snap_id_xyz'))
old_gmgr.gluster_call.assert_has_calls([mock.call(*a) for a in args])
args = (('volume', 'start', volume),)
new_gmgr.gluster_call.assert_has_calls([mock.call(*a) for a in args])
self._layout._share_manager.assert_called_once_with(
snapshot['share_instance'])
self._layout._glustermanager.assert_called_once_with(
@ -752,7 +754,69 @@ class GlusterfsVolumeMappedLayoutTestCase(test.TestCase):
self._layout.gluster_used_vols)
self.assertEqual('host1:/gv1', ret)
def test_create_share_from_snapshot_error_old_gmr_gluster_calls(self):
@ddt.data({'trouble': exception.ProcessExecutionError,
'_exception': exception.GlusterfsException},
{'trouble': RuntimeError, '_exception': RuntimeError})
@ddt.unpack
def test_create_share_from_snapshot_error_new_gmr_gluster_calls(
self, trouble, _exception):
glusterfs_target = 'root@host1:/gv1'
glusterfs_server = 'root@host1'
share = new_share()
volume = ''.join(['manila-', share['id']])
new_vol_addr = ':/'.join([glusterfs_server, volume])
gmgr = common.GlusterManager
old_gmgr = gmgr(glusterfs_target, self._execute, None, None)
new_gmgr = gmgr(new_vol_addr, self._execute, None, None)
self._layout.gluster_used_vols = set([glusterfs_target])
self._layout.glusterfs_versions = {glusterfs_server: ('3', '7')}
self.mock_object(old_gmgr, 'gluster_call',
mock.Mock(side_effect=[('', ''), ('', '')]))
self.mock_object(new_gmgr, 'gluster_call',
mock.Mock(side_effect=trouble))
self.mock_object(new_gmgr, 'get_gluster_vol_option',
mock.Mock())
new_gmgr.get_gluster_vol_option.return_value = (
'glusterfs-server-1,client')
self.mock_object(self._layout, '_find_actual_backend_snapshot_name',
mock.Mock(return_value='fake_snap_id_xyz'))
self.mock_object(self._layout, '_share_manager',
mock.Mock(return_value=old_gmgr))
self.mock_object(self._layout, '_glustermanager',
mock.Mock(return_value=new_gmgr))
self.mock_object(self.fake_driver, '_setup_via_manager',
mock.Mock(return_value='host1:/gv1'))
snapshot = {
'id': 'fake_snap_id',
'share_instance': new_share(export_location=glusterfs_target)
}
self.assertRaises(_exception,
self._layout.create_share_from_snapshot,
self._context, share, snapshot)
(self._layout._find_actual_backend_snapshot_name.
assert_called_once_with(old_gmgr, snapshot))
args = (('snapshot', 'activate', 'fake_snap_id_xyz',
'force', '--mode=script'),
('snapshot', 'clone', volume, 'fake_snap_id_xyz'))
old_gmgr.gluster_call.assert_has_calls([mock.call(*a) for a in args])
args = (('volume', 'start', volume),)
new_gmgr.gluster_call.assert_has_calls([mock.call(*a) for a in args])
self._layout._share_manager.assert_called_once_with(
snapshot['share_instance'])
self._layout._glustermanager.assert_called_once_with(
gmgr.parse(new_vol_addr))
self._layout.driver._setup_via_manager.assert_called_once_with(
{'manager': new_gmgr, 'share': share},
{'manager': old_gmgr, 'share': snapshot['share_instance']})
@ddt.data({'trouble': exception.ProcessExecutionError,
'_exception': exception.GlusterfsException},
{'trouble': RuntimeError, '_exception': RuntimeError})
@ddt.unpack
def test_create_share_from_snapshot_error_old_gmr_gluster_calls(
self, trouble, _exception):
glusterfs_target = 'root@host1:/gv1'
glusterfs_server = 'root@host1'
share = new_share()
@ -765,9 +829,7 @@ class GlusterfsVolumeMappedLayoutTestCase(test.TestCase):
self._layout.glusterfs_versions = {glusterfs_server: ('3', '7')}
self.mock_object(
old_gmgr, 'gluster_call',
mock.Mock(side_effect=[('', ''), exception.ProcessExecutionError]))
self.mock_object(new_gmgr, 'gluster_call',
mock.Mock(side_effect=[('', ''), ('', '')]))
mock.Mock(side_effect=[('', ''), trouble]))
self.mock_object(new_gmgr, 'get_gluster_vol_option',
mock.Mock())
new_gmgr.get_gluster_vol_option.return_value = (
@ -783,7 +845,7 @@ class GlusterfsVolumeMappedLayoutTestCase(test.TestCase):
'id': 'fake_snap_id',
'share_instance': new_share(export_location=glusterfs_target)
}
self.assertRaises(exception.GlusterfsException,
self.assertRaises(_exception,
self._layout.create_share_from_snapshot,
self._context, share, snapshot)
@ -815,9 +877,6 @@ class GlusterfsVolumeMappedLayoutTestCase(test.TestCase):
self.mock_object(
old_gmgr, 'gluster_call',
mock.Mock(side_effect=[('', ''), ('', '')]))
self.mock_object(
new_gmgr, 'gluster_call',
mock.Mock(side_effect=[('', ''), exception.ProcessExecutionError]))
self.mock_object(new_gmgr, 'get_gluster_vol_option',
mock.Mock())
new_gmgr.get_gluster_vol_option.return_value = (

19
manila/tests/share/drivers/test_glusterfs_native.py
View File

@ -143,8 +143,7 @@ class GlusterfsNativeShareDriverTestCase(test.TestCase):
'glusterfs-server-name,manila-host.com'),
('volume', 'set', 'fakevol', 'nfs.export-volumes', 'off'),
('volume', 'set', 'fakevol', 'client.ssl', 'on'),
('volume', 'set', 'fakevol', 'server.ssl', 'on'),
('volume', 'start', 'fakevol'))
('volume', 'set', 'fakevol', 'server.ssl', 'on'))
gmgr.gluster_call.assert_has_calls([mock.call(*a) for a in args])
self.assertEqual(ret, gmgr.export)
@ -187,7 +186,7 @@ class GlusterfsNativeShareDriverTestCase(test.TestCase):
self.assertTrue(self._driver.snapshots_are_supported)
def test_allow_access_via_manager(self):
common._restart_gluster_vol = mock.Mock()
self.mock_object(common, '_restart_gluster_vol', mock.Mock())
access = {'access_type': 'cert', 'access_to': 'client.example.com'}
gmgr1 = common.GlusterManager(self.glusterfs_target1, self._execute,
None, None)
@ -205,7 +204,7 @@ class GlusterfsNativeShareDriverTestCase(test.TestCase):
common._restart_gluster_vol.assert_called_once_with(gmgr1)
def test_allow_access_via_manager_with_share_having_access(self):
common._restart_gluster_vol = mock.Mock()
self.mock_object(common, '_restart_gluster_vol', mock.Mock())
access = {'access_type': 'cert', 'access_to': 'client.example.com'}
gmgr1 = common.GlusterManager(self.glusterfs_target1, self._execute,
None, None)
@ -222,7 +221,7 @@ class GlusterfsNativeShareDriverTestCase(test.TestCase):
self.assertFalse(common._restart_gluster_vol.called)
def test_allow_access_via_manager_invalid_access_type(self):
common._restart_gluster_vol = mock.Mock()
self.mock_object(common, '_restart_gluster_vol', mock.Mock())
access = {'access_type': 'invalid', 'access_to': 'client.example.com'}
expected_exec = []
@ -242,7 +241,7 @@ class GlusterfsNativeShareDriverTestCase(test.TestCase):
if (args == test_args):
raise exception.ProcessExecutionError()
common._restart_gluster_vol = mock.Mock()
self.mock_object(common, '_restart_gluster_vol', mock.Mock())
gmgr1 = common.GlusterManager(self.glusterfs_target1, self._execute,
None, None)
self.mock_object(gmgr1, 'get_gluster_vol_option',
@ -260,7 +259,7 @@ class GlusterfsNativeShareDriverTestCase(test.TestCase):
self.assertFalse(common._restart_gluster_vol.called)
def test_deny_access_via_manager(self):
common._restart_gluster_vol = mock.Mock()
self.mock_object(common, '_restart_gluster_vol', mock.Mock())
access = {'access_type': 'cert', 'access_to': 'client.example.com'}
gmgr1 = common.GlusterManager(self.glusterfs_target1, self._execute,
None, None)
@ -279,7 +278,7 @@ class GlusterfsNativeShareDriverTestCase(test.TestCase):
common._restart_gluster_vol.assert_called_once_with(gmgr1)
def test_deny_access_via_manager_with_share_having_no_access(self):
common._restart_gluster_vol = mock.Mock()
self.mock_object(common, '_restart_gluster_vol', mock.Mock())
access = {'access_type': 'cert', 'access_to': 'client.example.com'}
gmgr1 = common.GlusterManager(self.glusterfs_target1, self._execute,
None, None)
@ -295,7 +294,7 @@ class GlusterfsNativeShareDriverTestCase(test.TestCase):
self.assertFalse(common._restart_gluster_vol.called)
def test_deny_access_via_manager_invalid_access_type(self):
common._restart_gluster_vol = mock.Mock()
self.mock_object(common, '_restart_gluster_vol', mock.Mock())
access = {'access_type': 'invalid', 'access_to': 'NotApplicable'}
self.assertRaises(exception.InvalidShareAccess,
@ -313,7 +312,7 @@ class GlusterfsNativeShareDriverTestCase(test.TestCase):
if (args == test_args):
raise exception.ProcessExecutionError()
common._restart_gluster_vol = mock.Mock()
self.mock_object(common, '_restart_gluster_vol', mock.Mock())
gmgr1 = common.GlusterManager(self.glusterfs_target1, self._execute,
None, None)
self.mock_object(