Remove direct DB calls from glusterfs_native driver
- Remove direct DB calls from methods create_snapshot() and delete_snapshot(). - Move code from _update_gluster_vols_dict() method to ensure_share() method. Change-Id: I6c95f6d9361093d832a536971a460c3cdda44dcb Partial-Bug: #1444914
This commit is contained in:
Igor Malinovskiy
parent
7cc4fc08f7
commit
4eb57f53c1
|
|
@ -223,24 +223,6 @@ class GlusterfsNativeShareDriver(driver.ExecuteMixin, driver.ShareDriver):
|
|||
LOG.error(msg)
|
||||
raise
|
||||
|
||||
# Update gluster_used_vols_dict by walking through the DB.
|
||||
self._update_gluster_vols_dict(context)
|
||||
unused_vols = gluster_volumes_initial - set(
|
||||
self.gluster_used_vols_dict)
|
||||
if not unused_vols:
|
||||
# No volumes available for use as share. Warn user.
|
||||
msg = (_("No unused gluster volumes available for use as share! "
|
||||
"Create share won't be supported unless existing shares "
|
||||
"are deleted or some gluster volumes are created with "
|
||||
"names matching 'glusterfs_volume_pattern'."))
|
||||
LOG.warn(msg)
|
||||
else:
|
||||
LOG.info(_LI("Number of gluster volumes in use: "
|
||||
"%(inuse-numvols)s. Number of gluster volumes "
|
||||
"available for use as share: %(unused-numvols)s"),
|
||||
{'inuse-numvols': len(self.gluster_used_vols_dict),
|
||||
'unused-numvols': len(unused_vols)})
|
||||
|
||||
def _glustermanager(self, gluster_address, has_volume=True):
|
||||
"""Create GlusterManager object for gluster_address."""
|
||||
|
||||
|
|
@ -288,18 +270,6 @@ class GlusterfsNativeShareDriver(driver.ExecuteMixin, driver.ShareDriver):
|
|||
volumes_dict[gsrv + ':/' + volname] = pattern_dict
|
||||
return volumes_dict
|
||||
|
||||
@utils.synchronized("glusterfs_native", external=False)
|
||||
def _update_gluster_vols_dict(self, context):
|
||||
"""Update dict of gluster vols that are used/unused."""
|
||||
|
||||
shares = self.db.share_get_all(context)
|
||||
|
||||
for s in shares:
|
||||
if (s['status'].lower() == 'available'):
|
||||
vol = s['export_location']
|
||||
gluster_mgr = self._glustermanager(vol)
|
||||
self.gluster_used_vols_dict[vol] = gluster_mgr
|
||||
|
||||
def _setup_gluster_vol(self, vol):
|
||||
# Enable gluster volumes for SSL access only.
|
||||
|
||||
|
|
@ -390,6 +360,21 @@ class GlusterfsNativeShareDriver(driver.ExecuteMixin, driver.ShareDriver):
|
|||
set(d) for d in (voldict, self.gluster_used_vols_dict)
|
||||
)
|
||||
unused_vols = set1 - set2
|
||||
|
||||
if not unused_vols:
|
||||
# No volumes available for use as share. Warn user.
|
||||
msg = (_("No unused gluster volumes available for use as share! "
|
||||
"Create share won't be supported unless existing shares "
|
||||
"are deleted or some gluster volumes are created with "
|
||||
"names matching 'glusterfs_volume_pattern'."))
|
||||
LOG.warn(msg)
|
||||
else:
|
||||
LOG.info(_LI("Number of gluster volumes in use: "
|
||||
"%(inuse-numvols)s. Number of gluster volumes "
|
||||
"available for use as share: %(unused-numvols)s"),
|
||||
{'inuse-numvols': len(self.gluster_used_vols_dict),
|
||||
'unused-numvols': len(unused_vols)})
|
||||
|
||||
# volmap is the data structure used to categorize and sort
|
||||
# the unused volumes. It's a nested dictionary of structure
|
||||
# {<size>: <hostmap>}
|
||||
|
|
@ -622,9 +607,8 @@ class GlusterfsNativeShareDriver(driver.ExecuteMixin, driver.ShareDriver):
|
|||
|
||||
def create_snapshot(self, context, snapshot, share_server=None):
|
||||
"""Creates a snapshot."""
|
||||
# FIXME: need to access db to retrieve share data
|
||||
vol = self.db.share_get(context,
|
||||
snapshot['share_id'])['export_location']
|
||||
|
||||
vol = snapshot['share']['export_location']
|
||||
if vol in self.gluster_nosnap_vols_dict:
|
||||
opret, operrno = -1, 0
|
||||
operrstr = self.gluster_nosnap_vols_dict[vol]
|
||||
|
|
@ -670,9 +654,8 @@ class GlusterfsNativeShareDriver(driver.ExecuteMixin, driver.ShareDriver):
|
|||
|
||||
def delete_snapshot(self, context, snapshot, share_server=None):
|
||||
"""Deletes a snapshot."""
|
||||
# FIXME: need to access db to retrieve share data
|
||||
vol = self.db.share_get(context,
|
||||
snapshot['share_id'])['export_location']
|
||||
|
||||
vol = snapshot['share']['export_location']
|
||||
gluster_mgr = self.gluster_used_vols_dict[vol]
|
||||
args = ('--xml', 'snapshot', 'delete', snapshot['id'], '--mode=script')
|
||||
try:
|
||||
|
|
@ -818,4 +801,6 @@ class GlusterfsNativeShareDriver(driver.ExecuteMixin, driver.ShareDriver):
|
|||
|
||||
def ensure_share(self, context, share, share_server=None):
|
||||
"""Invoked to ensure that share is exported."""
|
||||
pass
|
||||
vol = share['export_location']
|
||||
gluster_mgr = self._glustermanager(vol)
|
||||
self.gluster_used_vols_dict[vol] = gluster_mgr
|
||||
|
|
|
|||
|
|
@ -179,7 +179,6 @@ class GlusterfsNativeShareDriverTestCase(test.TestCase):
|
|||
mock.Mock(return_value=('3', '6')))
|
||||
self.mock_object(self._driver, '_fetch_gluster_volumes',
|
||||
mock.Mock(return_value=self.glusterfs_volumes_dict))
|
||||
self.mock_object(self._driver, '_update_gluster_vols_dict')
|
||||
self._driver.gluster_used_vols_dict = self.glusterfs_volumes_dict
|
||||
self.mock_object(glusterfs_native.LOG, 'warn')
|
||||
|
||||
|
|
@ -190,9 +189,6 @@ class GlusterfsNativeShareDriverTestCase(test.TestCase):
|
|||
self._driver._fetch_gluster_volumes.assert_called_once_with()
|
||||
self.assertEqual(expected_exec, fake_utils.fake_execute_get_log())
|
||||
self.gmgr1.get_gluster_version.assert_once_called_with()
|
||||
self._driver._update_gluster_vols_dict.assert_called_once_with(
|
||||
self._context)
|
||||
glusterfs_native.LOG.warn.assert_called_once_with(mock.ANY)
|
||||
|
||||
def test_do_setup_unsupported_glusterfs_version(self):
|
||||
self._driver.glusterfs_servers = {self.glusterfs_server1: self.gmgr1}
|
||||
|
|
@ -225,21 +221,16 @@ class GlusterfsNativeShareDriverTestCase(test.TestCase):
|
|||
self._driver.do_setup, self._context)
|
||||
self._driver._fetch_gluster_volumes.assert_called_once_with()
|
||||
|
||||
def test_update_gluster_vols_dict(self):
|
||||
share_in_error = new_share(status="error")
|
||||
self._db.share_get_all = mock.Mock(
|
||||
return_value=[self.share1, share_in_error])
|
||||
def test_ensure_share(self):
|
||||
share = self.share1
|
||||
self.mock_object(self._driver, '_glustermanager')
|
||||
|
||||
self._driver._update_gluster_vols_dict(self._context)
|
||||
self._driver.ensure_share(self._context, share)
|
||||
|
||||
self.assertEqual(1, len(self._driver.gluster_used_vols_dict))
|
||||
self._db.share_get_all.assert_called_once_with(self._context)
|
||||
|
||||
share_in_use = self.share1
|
||||
|
||||
self.assertTrue(
|
||||
share_in_use['export_location'] in
|
||||
self._driver.gluster_used_vols_dict)
|
||||
self.assertIsNotNone(
|
||||
self._driver.gluster_used_vols_dict[share['export_location']]
|
||||
)
|
||||
self.assertTrue(self._driver._glustermanager.called)
|
||||
|
||||
def test_setup_gluster_vol(self):
|
||||
test_args = [
|
||||
|
|
@ -775,7 +766,6 @@ class GlusterfsNativeShareDriverTestCase(test.TestCase):
|
|||
self.assertFalse(self._driver._push_gluster_vol.called)
|
||||
|
||||
def test_create_snapshot(self):
|
||||
self._db.share_get = mock.Mock(return_value=self.share1)
|
||||
self._driver.gluster_nosnap_vols_dict = {}
|
||||
self._driver.glusterfs_versions = {self.glusterfs_target1: ('3', '6')}
|
||||
|
||||
|
|
@ -783,7 +773,11 @@ class GlusterfsNativeShareDriverTestCase(test.TestCase):
|
|||
gmgr1 = gmgr(self.glusterfs_target1, self._execute, None, None)
|
||||
|
||||
self._driver.gluster_used_vols_dict = {self.glusterfs_target1: gmgr1}
|
||||
snapshot = {'id': 'fake_snap_id', 'share_id': self.share1['id']}
|
||||
snapshot = {
|
||||
'id': 'fake_snap_id',
|
||||
'share_id': self.share1['id'],
|
||||
'share': self.share1
|
||||
}
|
||||
|
||||
args = ('--xml', 'snapshot', 'create', 'fake_snap_id',
|
||||
gmgr1.volume)
|
||||
|
|
@ -794,7 +788,6 @@ class GlusterfsNativeShareDriverTestCase(test.TestCase):
|
|||
gmgr1.gluster_call.assert_called_once_with(*args)
|
||||
|
||||
def test_create_snapshot_error(self):
|
||||
self._db.share_get = mock.Mock(return_value=self.share1)
|
||||
self._driver.gluster_nosnap_vols_dict = {}
|
||||
self._driver.glusterfs_versions = {self.glusterfs_target1: ('3', '6')}
|
||||
|
||||
|
|
@ -802,7 +795,11 @@ class GlusterfsNativeShareDriverTestCase(test.TestCase):
|
|||
gmgr1 = gmgr(self.glusterfs_target1, self._execute, None, None)
|
||||
|
||||
self._driver.gluster_used_vols_dict = {self.glusterfs_target1: gmgr1}
|
||||
snapshot = {'id': 'fake_snap_id', 'share_id': self.share1['id']}
|
||||
snapshot = {
|
||||
'id': 'fake_snap_id',
|
||||
'share_id': self.share1['id'],
|
||||
'share': self.share1
|
||||
}
|
||||
|
||||
args = ('--xml', 'snapshot', 'create', 'fake_snap_id',
|
||||
gmgr1.volume)
|
||||
|
|
@ -818,7 +815,6 @@ class GlusterfsNativeShareDriverTestCase(test.TestCase):
|
|||
"exctype": exception.ShareSnapshotNotSupported})
|
||||
@ddt.unpack
|
||||
def test_create_snapshot_no_snap(self, vers_minor, exctype):
|
||||
self._db.share_get = mock.Mock(return_value=self.share1)
|
||||
self._driver.gluster_nosnap_vols_dict = {}
|
||||
self._driver.glusterfs_versions = {
|
||||
self.glusterfs_target1: ('3', vers_minor)}
|
||||
|
|
@ -827,7 +823,11 @@ class GlusterfsNativeShareDriverTestCase(test.TestCase):
|
|||
gmgr1 = gmgr(self.glusterfs_target1, self._execute, None, None)
|
||||
|
||||
self._driver.gluster_used_vols_dict = {self.glusterfs_target1: gmgr1}
|
||||
snapshot = {'id': 'fake_snap_id', 'share_id': self.share1['id']}
|
||||
snapshot = {
|
||||
'id': 'fake_snap_id',
|
||||
'share_id': self.share1['id'],
|
||||
'share': self.share1
|
||||
}
|
||||
|
||||
args = ('--xml', 'snapshot', 'create', 'fake_snap_id',
|
||||
gmgr1.volume)
|
||||
|
|
@ -842,26 +842,32 @@ class GlusterfsNativeShareDriverTestCase(test.TestCase):
|
|||
"exctype": exception.ShareSnapshotNotSupported})
|
||||
@ddt.unpack
|
||||
def test_create_snapshot_no_snap_cached(self, vers_minor, exctype):
|
||||
self._db.share_get = mock.Mock(return_value=self.share1)
|
||||
self._driver.gluster_nosnap_vols_dict = {
|
||||
self.share1['export_location']: 'fake error'}
|
||||
self._driver.glusterfs_versions = {
|
||||
self.glusterfs_target1: ('3', vers_minor)}
|
||||
|
||||
snapshot = {'id': 'fake_snap_id', 'share_id': self.share1['id']}
|
||||
snapshot = {
|
||||
'id': 'fake_snap_id',
|
||||
'share_id': self.share1['id'],
|
||||
'share': self.share1
|
||||
}
|
||||
|
||||
self.assertRaises(exctype, self._driver.create_snapshot, self._context,
|
||||
snapshot)
|
||||
|
||||
def test_delete_snapshot(self):
|
||||
self._db.share_get = mock.Mock(return_value=self.share1)
|
||||
self._driver.gluster_nosnap_vols_dict = {}
|
||||
|
||||
gmgr = glusterfs.GlusterManager
|
||||
gmgr1 = gmgr(self.share1['export_location'], self._execute, None, None)
|
||||
|
||||
self._driver.gluster_used_vols_dict = {self.glusterfs_target1: gmgr1}
|
||||
snapshot = {'id': 'fake_snap_id', 'share_id': self.share1['id']}
|
||||
snapshot = {
|
||||
'id': 'fake_snap_id',
|
||||
'share_id': self.share1['id'],
|
||||
'share': self.share1
|
||||
}
|
||||
|
||||
args = ('--xml', 'snapshot', 'delete', 'fake_snap_id',
|
||||
'--mode=script')
|
||||
|
|
@ -872,14 +878,17 @@ class GlusterfsNativeShareDriverTestCase(test.TestCase):
|
|||
gmgr1.gluster_call.assert_called_once_with(*args)
|
||||
|
||||
def test_delete_snapshot_error(self):
|
||||
self._db.share_get = mock.Mock(return_value=self.share1)
|
||||
self._driver.gluster_nosnap_vols_dict = {}
|
||||
|
||||
gmgr = glusterfs.GlusterManager
|
||||
gmgr1 = gmgr(self.share1['export_location'], self._execute, None, None)
|
||||
|
||||
self._driver.gluster_used_vols_dict = {self.glusterfs_target1: gmgr1}
|
||||
snapshot = {'id': 'fake_snap_id', 'share_id': self.share1['id']}
|
||||
snapshot = {
|
||||
'id': 'fake_snap_id',
|
||||
'share_id': self.share1['id'],
|
||||
'share': self.share1
|
||||
}
|
||||
|
||||
args = ('--xml', 'snapshot', 'delete', 'fake_snap_id', '--mode=script')
|
||||
self.mock_object(gmgr1, 'gluster_call',
|
||||
|
|
|
|||
Loading…
Reference in New Issue