diff --git a/cinder/db/api.py b/cinder/db/api.py index 605b2e8f067..7ee9b905a31 100644 --- a/cinder/db/api.py +++ b/cinder/db/api.py @@ -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) diff --git a/cinder/objects/base.py b/cinder/objects/base.py index a4929eed954..76a5de2b2db 100644 --- a/cinder/objects/base.py +++ b/cinder/objects/base.py @@ -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): diff --git a/cinder/tests/unit/test_lvm_driver.py b/cinder/tests/unit/test_lvm_driver.py index 8aafb2f88ef..9d93059a602 100644 --- a/cinder/tests/unit/test_lvm_driver.py +++ b/cinder/tests/unit/test_lvm_driver.py @@ -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} diff --git a/cinder/tests/unit/test_volume_utils.py b/cinder/tests/unit/test_volume_utils.py index a5a667522db..30e9a7df694 100644 --- a/cinder/tests/unit/test_volume_utils.py +++ b/cinder/tests/unit/test_volume_utils.py @@ -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): diff --git a/cinder/volume/drivers/lvm.py b/cinder/volume/drivers/lvm.py index be7534d8890..c7141646ada 100644 --- a/cinder/volume/drivers/lvm.py +++ b/cinder/volume/drivers/lvm.py @@ -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. diff --git a/cinder/volume/utils.py b/cinder/volume/utils.py index 4e17e635dab..53fd2bdb7cb 100644 --- a/cinder/volume/utils.py +++ b/cinder/volume/utils.py @@ -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):