openstack/cinder
Code Issues Proposed changes

Merge "Improve DB volume check in LVM manage volume"

Browse Source
This commit is contained in:
Jenkins 2016-06-23 17:39:47 +00:00 committed by Gerrit Code Review
commit 3d611461bf
6 changed files with 49 additions and 47 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
cinder/db/api.py
View File

@ -1204,6 +1204,10 @@ def message_destroy(context, message_id):
###################
def resource_exists(context, model, resource_id):
return IMPL.resource_exists(context, model, resource_id)
def get_model_for_versioned_object(versioned_object):
return IMPL.get_model_for_versioned_object(versioned_object)

5
cinder/objects/base.py
View File

@ -416,6 +416,11 @@ class CinderPersistentObject(object):
setattr(self, field, current_field)
self.obj_reset_changes()
@classmethod
def exists(cls, context, id_):
model = db.get_model_for_versioned_object(cls)
return db.resource_exists(context, model, id_)
class CinderComparableObject(base.ComparableVersionedObject):
def __eq__(self, obj):

5
cinder/tests/unit/test_lvm_driver.py
View File

@ -660,11 +660,10 @@ class LVMVolumeDriverTestCase(DriverTestCase):
model_update = self.volume.driver.manage_existing(vol, ref)
self.assertIsNone(model_update)
@mock.patch.object(db.sqlalchemy.api, 'volume_get')
def test_lvm_manage_existing_already_managed(self, mock_conf):
@mock.patch('cinder.db.sqlalchemy.api.resource_exists', return_value=True)
def test_lvm_manage_existing_already_managed(self, exists_mock):
self._setup_stubs_for_manage_existing()
mock_conf.volume_name_template = 'volume-%s'
vol_name = 'volume-d8cd1feb-2dcc-404d-9b15-b86fe3bec0a1'
ref = {'source-name': vol_name}
vol = {'name': 'test', 'id': 1, 'size': 0}

68
cinder/tests/unit/test_volume_utils.py
View File

@ -25,6 +25,7 @@ from oslo_concurrency import processutils
from oslo_config import cfg
from cinder import context
from cinder.db.sqlalchemy import models
from cinder import exception
from cinder.objects import fields
from cinder import test
@ -757,56 +758,51 @@ class VolumeUtilsTestCase(test.TestCase):
host_2 = 'fake_host2@backend1'
self.assertFalse(volume_utils.hosts_are_equivalent(host_1, host_2))
def test_check_managed_volume_already_managed(self):
mock_db = mock.Mock()
result = volume_utils.check_already_managed_volume(
mock_db, 'volume-d8cd1feb-2dcc-404d-9b15-b86fe3bec0a1')
@mock.patch('cinder.db.sqlalchemy.api.resource_exists', return_value=True)
def test_check_managed_volume_already_managed(self, exists_mock):
id_ = 'd8cd1feb-2dcc-404d-9b15-b86fe3bec0a1'
vol_id = 'volume-' + id_
result = volume_utils.check_already_managed_volume(vol_id)
self.assertTrue(result)
exists_mock.assert_called_once_with(mock.ANY, models.Volume, id_)
@mock.patch('cinder.volume.utils.CONF')
def test_check_already_managed_with_vol_id_vol_pattern(self, conf_mock):
mock_db = mock.Mock()
conf_mock.volume_name_template = 'volume-%s-volume'
@mock.patch('cinder.db.sqlalchemy.api.resource_exists', return_value=True)
def test_check_already_managed_with_vol_id_vol_pattern(self, exists_mock):
template = 'volume-%s-volume'
self.override_config('volume_name_template', template)
id_ = 'd8cd1feb-2dcc-404d-9b15-b86fe3bec0a1'
vol_id = template % id_
result = volume_utils.check_already_managed_volume(
mock_db, 'volume-d8cd1feb-2dcc-404d-9b15-b86fe3bec0a1-volume')
result = volume_utils.check_already_managed_volume(vol_id)
self.assertTrue(result)
exists_mock.assert_called_once_with(mock.ANY, models.Volume, id_)
@mock.patch('cinder.volume.utils.CONF')
def test_check_already_managed_with_id_vol_pattern(self, conf_mock):
mock_db = mock.Mock()
conf_mock.volume_name_template = '%s-volume'
@mock.patch('cinder.db.sqlalchemy.api.resource_exists', return_value=True)
def test_check_already_managed_with_id_vol_pattern(self, exists_mock):
template = '%s-volume'
self.override_config('volume_name_template', template)
id_ = 'd8cd1feb-2dcc-404d-9b15-b86fe3bec0a1'
vol_id = template % id_
result = volume_utils.check_already_managed_volume(
mock_db, 'd8cd1feb-2dcc-404d-9b15-b86fe3bec0a1-volume')
result = volume_utils.check_already_managed_volume(vol_id)
self.assertTrue(result)
exists_mock.assert_called_once_with(mock.ANY, models.Volume, id_)
def test_check_managed_volume_not_managed_cinder_like_name(self):
mock_db = mock.Mock()
mock_db.volume_get = mock.Mock(
side_effect=exception.VolumeNotFound(
'volume-d8cd1feb-2dcc-404d-9b15-b86fe3bec0a1'))
result = volume_utils.check_already_managed_volume(
mock_db, 'volume-d8cd1feb-2dcc-404d-9b15-b86fe3bec0a1')
@mock.patch('cinder.db.sqlalchemy.api.resource_exists', return_value=False)
def test_check_managed_volume_not_managed_cinder_like_name(self,
exists_mock):
id_ = 'd8cd1feb-2dcc-404d-9b15-b86fe3bec0a1'
vol_id = 'volume-' + id_
result = volume_utils.check_already_managed_volume(vol_id)
self.assertFalse(result)
exists_mock.assert_called_once_with(mock.ANY, models.Volume, id_)
def test_check_managed_volume_not_managed(self):
mock_db = mock.Mock()
result = volume_utils.check_already_managed_volume(
mock_db, 'test-volume')
result = volume_utils.check_already_managed_volume('test-volume')
self.assertFalse(result)
def test_check_managed_volume_not_managed_id_like_uuid(self):
mock_db = mock.Mock()
result = volume_utils.check_already_managed_volume(
mock_db, 'volume-d8cd1fe')
result = volume_utils.check_already_managed_volume('volume-d8cd1fe')
self.assertFalse(result)
def test_convert_config_string_to_dict(self):

2
cinder/volume/drivers/lvm.py
View File

@ -586,7 +586,7 @@ class LVMVolumeDriver(driver.VolumeDriver):
lv_name = existing_ref['source-name']
self.vg.get_volume(lv_name)
if volutils.check_already_managed_volume(self.db, lv_name):
if volutils.check_already_managed_volume(lv_name):
raise exception.ManageExistingAlreadyManaged(volume_ref=lv_name)
# Attempt to rename the LV to match the OpenStack internal name.

12
cinder/volume/utils.py
View File

@ -38,6 +38,7 @@ from cinder import context
from cinder import db
from cinder import exception
from cinder.i18n import _, _LI, _LW, _LE
from cinder import objects
from cinder import rpc
from cinder import utils
from cinder.volume import throttling
@ -670,22 +671,19 @@ def _extract_id(vol_name):
return match.group('uuid') if match else None
def check_already_managed_volume(db, vol_name):
def check_already_managed_volume(vol_name):
"""Check cinder db for already managed volume.
:param db: database api parameter
:param vol_name: volume name parameter
:returns: bool -- return True, if db entry with specified
volume name exist, otherwise return False
"""
vol_id = _extract_id(vol_name)
try:
if vol_id and uuid.UUID(vol_id, version=4):
db.volume_get(context.get_admin_context(), vol_id)
return True
except (exception.VolumeNotFound, ValueError):
return (vol_id and uuid.UUID(vol_id, version=4) and
objects.Volume.exists(context.get_admin_context(), vol_id))
except ValueError:
return False
return False
def convert_config_string_to_dict(config_string):