diff --git a/cinder/api/contrib/volume_manage.py b/cinder/api/contrib/volume_manage.py index 3c4167b6e..819e5fad1 100644 --- a/cinder/api/contrib/volume_manage.py +++ b/cinder/api/contrib/volume_manage.py @@ -143,7 +143,6 @@ class VolumeManageController(wsgi.Controller): msg = _("Service not found.") raise exc.HTTPNotFound(explanation=msg) - new_volume = dict(new_volume) utils.add_visible_admin_metadata(new_volume) return self._view_builder.detail(req, new_volume) diff --git a/cinder/api/v1/volumes.py b/cinder/api/v1/volumes.py index ce594284b..9a9de03d9 100644 --- a/cinder/api/v1/volumes.py +++ b/cinder/api/v1/volumes.py @@ -49,7 +49,7 @@ def _translate_attachment_detail_view(_context, vol): def _translate_attachment_summary_view(_context, vol): """Maps keys for attachment summary view.""" d = [] - attachments = vol.get('volume_attachment', []) + attachments = vol.volume_attachment for attachment in attachments: if attachment.get('attach_status') == 'attached': a = {'id': attachment.get('volume_id'), @@ -118,12 +118,8 @@ def _translate_volume_summary_view(context, vol, image_id=None): LOG.info(_LI("vol=%s"), vol, context=context) - if vol.get('volume_metadata'): - metadata = vol.get('volume_metadata') - d['metadata'] = {item['key']: item['value'] for item in metadata} - # avoid circular ref when vol is a Volume instance - elif vol.get('metadata') and isinstance(vol.get('metadata'), dict): - d['metadata'] = vol['metadata'] + if vol.metadata: + d['metadata'] = vol.metadata else: d['metadata'] = {} @@ -292,12 +288,10 @@ class VolumeController(wsgi.Controller): filters=search_opts, viewable_admin_meta=True) - volumes = [dict(vol) for vol in volumes] - for volume in volumes: utils.add_visible_admin_metadata(volume) - limited_list = common.limited(volumes, req) + limited_list = common.limited(volumes.objects, req) req.cache_db_volumes(limited_list) res = [entity_maker(context, vol) for vol in limited_list] diff --git a/cinder/api/v2/views/volumes.py b/cinder/api/v2/views/volumes.py index cd9cd8913..ff9e64342 100644 --- a/cinder/api/v2/views/volumes.py +++ b/cinder/api/v2/views/volumes.py @@ -14,6 +14,7 @@ # under the License. from oslo_log import log as logging +import six from cinder.api import common @@ -71,7 +72,7 @@ class ViewBuilder(common.ViewBuilder): 'metadata': self._get_volume_metadata(volume), 'links': self._get_links(request, volume['id']), 'user_id': volume.get('user_id'), - 'bootable': str(volume.get('bootable')).lower(), + 'bootable': six.text_type(volume.get('bootable')).lower(), 'encrypted': self._is_volume_encrypted(volume), 'replication_status': volume.get('replication_status'), 'consistencygroup_id': volume.get('consistencygroup_id'), @@ -92,7 +93,7 @@ class ViewBuilder(common.ViewBuilder): attachments = [] if volume['attach_status'] == 'attached': - attaches = volume.get('volume_attachment', []) + attaches = volume.volume_attachment for attachment in attaches: if attachment.get('attach_status') == 'attached': a = {'id': attachment.get('volume_id'), @@ -109,14 +110,7 @@ class ViewBuilder(common.ViewBuilder): def _get_volume_metadata(self, volume): """Retrieve the metadata of the volume object.""" - if volume.get('volume_metadata'): - metadata = volume.get('volume_metadata') - return {item['key']: item['value'] for item in metadata} - # avoid circular ref when vol is a Volume instance - elif volume.get('metadata') and isinstance(volume.get('metadata'), - dict): - return volume['metadata'] - return {} + return volume.metadata def _get_volume_type(self, volume): """Retrieve the type the volume object.""" diff --git a/cinder/api/v2/volumes.py b/cinder/api/v2/volumes.py index 9f23290c0..24e4b0d94 100644 --- a/cinder/api/v2/volumes.py +++ b/cinder/api/v2/volumes.py @@ -249,12 +249,10 @@ class VolumeController(wsgi.Controller): viewable_admin_meta=True, offset=offset) - volumes = [dict(vol) for vol in volumes] - for volume in volumes: utils.add_visible_admin_metadata(volume) - req.cache_db_volumes(volumes) + req.cache_db_volumes(volumes.objects) if is_detail: volumes = self._view_builder.detail_list(req, volumes) diff --git a/cinder/cmd/manage.py b/cinder/cmd/manage.py index f787db589..116e436fc 100644 --- a/cinder/cmd/manage.py +++ b/cinder/cmd/manage.py @@ -63,7 +63,6 @@ from oslo_db.sqlalchemy import migration from oslo_log import log as logging import oslo_messaging as messaging from oslo_utils import timeutils -from oslo_utils import uuidutils from cinder import i18n i18n.enable_lazy() @@ -94,23 +93,6 @@ def args(*args, **kwargs): return _decorator -def param2id(object_id): - """Helper function to convert various id types to internal id. - - :param object_id: e.g. 'vol-0000000a' or 'volume-0000000a' or '10' - """ - if uuidutils.is_uuid_like(object_id): - return object_id - elif '-' in object_id: - # FIXME(ja): mapping occurs in nova? - pass - else: - try: - return int(object_id) - except ValueError: - return object_id - - class ShellCommands(object): def bpython(self): """Runs a bpython shell. @@ -283,22 +265,22 @@ class VolumeCommands(object): def delete(self, volume_id): """Delete a volume, bypassing the check that it must be available.""" ctxt = context.get_admin_context() - volume = db.volume_get(ctxt, param2id(volume_id)) - host = vutils.extract_host(volume['host']) if volume['host'] else None + volume = objects.Volume.get_by_id(ctxt, volume_id) + host = vutils.extract_host(volume.host) if volume.host else None if not host: print(_("Volume not yet assigned to host.")) print(_("Deleting volume from database and skipping rpc.")) - db.volume_destroy(ctxt, param2id(volume_id)) + volume.destroy() return - if volume['status'] == 'in-use': + if volume.status == 'in-use': print(_("Volume is in-use.")) print(_("Detach volume from instance and then try again.")) return cctxt = self._rpc_client().prepare(server=host) - cctxt.cast(ctxt, "delete_volume", volume_id=volume['id']) + cctxt.cast(ctxt, "delete_volume", volume_id=volume.id, volume=volume) @args('--currenthost', required=True, help='Existing volume host name') @args('--newhost', required=True, help='New volume host name') diff --git a/cinder/tests/unit/api/contrib/test_admin_actions.py b/cinder/tests/unit/api/contrib/test_admin_actions.py index 5128cacea..a3d47fda7 100644 --- a/cinder/tests/unit/api/contrib/test_admin_actions.py +++ b/cinder/tests/unit/api/contrib/test_admin_actions.py @@ -99,6 +99,18 @@ class AdminActionsTest(test.TestCase): resp = req.get_response(app()) return resp + def _create_volume(self, context, updates=None): + db_volume = {'status': 'available', + 'host': 'test', + 'availability_zone': 'fake_zone', + 'attach_status': 'detached'} + if updates: + db_volume.update(updates) + + volume = objects.Volume(context=context, **db_volume) + volume.create() + return volume + def test_valid_updates(self): vac = admin_actions.VolumeAdminController() @@ -375,7 +387,7 @@ class AdminActionsTest(test.TestCase): # admin context ctx = context.RequestContext('admin', 'fake', True) # current status is creating - volume = db.volume_create(ctx, {'size': 1}) + volume = self._create_volume(ctx, {'size': 1, 'host': None}) req = webob.Request.blank('/v2/fake/volumes/%s/action' % volume['id']) req.method = 'POST' req.headers['content-type'] = 'application/json' @@ -386,7 +398,8 @@ class AdminActionsTest(test.TestCase): # request is accepted self.assertEqual(202, resp.status_int) # volume is deleted - self.assertRaises(exception.NotFound, db.volume_get, ctx, volume['id']) + self.assertRaises(exception.NotFound, objects.Volume.get_by_id, ctx, + volume.id) @mock.patch.object(volume_api.API, 'delete_snapshot', return_value=True) @mock.patch('cinder.objects.Snapshot.get_by_id') @@ -416,8 +429,8 @@ class AdminActionsTest(test.TestCase): # admin context ctx = context.RequestContext('admin', 'fake', True) # current status is available - volume = db.volume_create(ctx, {'status': 'available', 'host': 'test', - 'provider_location': '', 'size': 1}) + volume = self._create_volume(ctx, {'provider_location': '', + 'size': 1}) connector = {'initiator': 'iqn.2012-07.org.fake:01'} # start service to handle rpc messages for attach requests svc = self.start_service('volume', host='test') @@ -473,8 +486,8 @@ class AdminActionsTest(test.TestCase): # admin context ctx = context.RequestContext('admin', 'fake', True) # current status is available - volume = db.volume_create(ctx, {'status': 'available', 'host': 'test', - 'provider_location': '', 'size': 1}) + volume = self._create_volume(ctx, {'provider_location': '', + 'size': 1}) connector = {'initiator': 'iqn.2012-07.org.fake:01'} # start service to handle rpc messages for attach requests svc = self.start_service('volume', host='test') @@ -530,8 +543,8 @@ class AdminActionsTest(test.TestCase): # admin context ctx = context.RequestContext('admin', 'fake', True) # current status is available - volume = db.volume_create(ctx, {'status': 'available', 'host': 'test', - 'provider_location': '', 'size': 1}) + volume = self._create_volume(ctx, {'provider_location': '', + 'size': 1}) connector = {'initiator': 'iqn.2012-07.org.fake:01'} # start service to handle rpc messages for attach requests svc = self.start_service('volume', host='test') @@ -617,8 +630,8 @@ class AdminActionsTest(test.TestCase): # admin context ctx = context.RequestContext('admin', 'fake', True) # current status is available - volume = db.volume_create(ctx, {'status': 'available', 'host': 'test', - 'provider_location': '', 'size': 1}) + volume = self._create_volume(ctx, {'provider_location': '', + 'size': 1}) connector = {'initiator': 'iqn.2012-07.org.fake:01'} # start service to handle rpc messages for attach requests svc = self.start_service('volume', host='test') @@ -668,8 +681,8 @@ class AdminActionsTest(test.TestCase): # admin context ctx = context.RequestContext('admin', 'fake', True) # current status is available - volume = db.volume_create(ctx, {'status': 'available', 'host': 'test', - 'provider_location': '', 'size': 1}) + volume = self._create_volume(ctx, {'provider_location': '', + 'size': 1}) connector = {'initiator': 'iqn.2012-07.org.fake:01'} # start service to handle rpc messages for attach requests svc = self.start_service('volume', host='test') @@ -695,8 +708,8 @@ class AdminActionsTest(test.TestCase): # admin context ctx = context.RequestContext('admin', 'fake', True) # current status is available - volume = db.volume_create(ctx, {'status': 'available', 'host': 'test', - 'provider_location': '', 'size': 1}) + volume = self._create_volume(ctx, {'provider_location': '', + 'size': 1}) connector = {'initiator': 'iqn.2012-07.org.fake:01'} # start service to handle rpc messages for attach requests svc = self.start_service('volume', host='test') @@ -723,8 +736,8 @@ class AdminActionsTest(test.TestCase): # admin context ctx = context.RequestContext('admin', 'fake', True) # current status is available - volume = db.volume_create(ctx, {'status': 'available', 'host': 'test', - 'provider_location': '', 'size': 1}) + volume = self._create_volume(ctx, {'provider_location': '', + 'size': 1}) connector = {} # start service to handle rpc messages for attach requests svc = self.start_service('volume', host='test') @@ -738,8 +751,8 @@ class AdminActionsTest(test.TestCase): """Test that attaching volume reserved for another instance fails.""" ctx = context.RequestContext('admin', 'fake', True) # current status is available - volume = db.volume_create(ctx, {'status': 'available', 'host': 'test', - 'provider_location': '', 'size': 1}) + volume = self._create_volume(ctx, {'provider_location': '', + 'size': 1}) # start service to handle rpc messages for attach requests svc = self.start_service('volume', host='test') self.volume_api.reserve_volume(ctx, volume) @@ -766,8 +779,8 @@ class AdminActionsTest(test.TestCase): # admin context ctx = context.RequestContext('admin', 'fake', True) # current status is available - volume = db.volume_create(ctx, {'status': 'available', 'host': 'test', - 'provider_location': '', 'size': 1}) + volume = self._create_volume(ctx, {'provider_location': '', + 'size': 1}) # start service to handle rpc messages for attach requests svc = self.start_service('volume', host='test') values = {'status': 'attaching', @@ -799,11 +812,7 @@ class AdminActionsTest(test.TestCase): 'topic': CONF.volume_topic, 'created_at': timeutils.utcnow()}) # current status is available - volume = db.volume_create(admin_ctx, - {'status': 'available', - 'host': 'test', - 'provider_location': '', - 'attach_status': ''}) + volume = self._create_volume(admin_ctx) return volume def _migrate_volume_exec(self, ctx, volume, host, expected_status, @@ -837,12 +846,9 @@ class AdminActionsTest(test.TestCase): ctx = context.RequestContext('admin', 'fake', True) volume = self._migrate_volume_prep() # current status is available - volume = db.volume_create(ctx, - {'status': 'available', - 'host': 'test', - 'provider_location': '', - 'attach_status': '', - 'replication_status': 'active'}) + volume = self._create_volume(ctx, {'provider_location': '', + 'attach_status': '', + 'replication_status': 'active'}) volume = self._migrate_volume_exec(ctx, volume, host, expected_status) def test_migrate_volume_as_non_admin(self): @@ -943,10 +949,9 @@ class AdminActionsTest(test.TestCase): def test_migrate_volume_comp_no_mig_status(self): admin_ctx = context.get_admin_context() - volume1 = db.volume_create(admin_ctx, {'id': 'fake1', - 'migration_status': 'foo'}) - volume2 = db.volume_create(admin_ctx, {'id': 'fake2', - 'migration_status': None}) + volume1 = self._create_volume(admin_ctx, {'migration_status': 'foo'}) + volume2 = self._create_volume(admin_ctx, {'migration_status': None}) + expected_status = 400 expected_id = None ctx = context.RequestContext('admin', 'fake', True) @@ -957,12 +962,10 @@ class AdminActionsTest(test.TestCase): def test_migrate_volume_comp_bad_mig_status(self): admin_ctx = context.get_admin_context() - volume1 = db.volume_create(admin_ctx, - {'id': 'fake1', - 'migration_status': 'migrating'}) - volume2 = db.volume_create(admin_ctx, - {'id': 'fake2', - 'migration_status': 'target:foo'}) + volume1 = self._create_volume(admin_ctx, + {'migration_status': 'migrating'}) + volume2 = self._create_volume(admin_ctx, + {'migration_status': 'target:foo'}) expected_status = 400 expected_id = None ctx = context.RequestContext('admin', 'fake', True) @@ -981,20 +984,14 @@ class AdminActionsTest(test.TestCase): def test_migrate_volume_comp_from_nova(self): admin_ctx = context.get_admin_context() - volume = db.volume_create(admin_ctx, - {'id': 'fake1', - 'status': 'in-use', - 'host': 'test', - 'migration_status': None, - 'attach_status': 'attached'}) - new_volume = db.volume_create(admin_ctx, - {'id': 'fake2', - 'status': 'available', - 'host': 'test', - 'migration_status': None, - 'attach_status': 'detached'}) + volume = self._create_volume(admin_ctx, {'status': 'in-use', + 'migration_status': None, + 'attach_status': 'attached'}) + new_volume = self._create_volume(admin_ctx, + {'migration_status': None, + 'attach_status': 'detached'}) expected_status = 200 - expected_id = 'fake2' + expected_id = new_volume.id ctx = context.RequestContext('admin', 'fake', True) self._migrate_volume_comp_exec(ctx, volume, new_volume, False, expected_status, expected_id) diff --git a/cinder/tests/unit/api/contrib/test_volume_actions.py b/cinder/tests/unit/api/contrib/test_volume_actions.py index ea5f76c54..c2078d597 100644 --- a/cinder/tests/unit/api/contrib/test_volume_actions.py +++ b/cinder/tests/unit/api/contrib/test_volume_actions.py @@ -13,6 +13,7 @@ # under the License. import datetime +import iso8601 import json import uuid @@ -23,11 +24,13 @@ from oslo_serialization import jsonutils import webob from cinder.api.contrib import volume_actions +from cinder import context from cinder import exception from cinder.image import glance from cinder import test from cinder.tests.unit.api import fakes from cinder.tests.unit.api.v2 import stubs +from cinder.tests.unit import fake_volume from cinder import volume from cinder.volume import api as volume_api from cinder.volume import rpcapi as volume_rpcapi @@ -43,6 +46,7 @@ class VolumeActionsTest(test.TestCase): def setUp(self): super(VolumeActionsTest, self).setUp() + self.context = context.RequestContext('fake', 'fake', is_admin=False) self.UUID = uuid.uuid4() self.controller = volume_actions.VolumeActionsController() self.api_patchers = {} @@ -52,9 +56,10 @@ class VolumeActionsTest(test.TestCase): self.addCleanup(self.api_patchers[_meth].stop) self.api_patchers[_meth].return_value = True - vol = {'id': 'fake', 'host': 'fake', 'status': 'available', 'size': 1, - 'migration_status': None, 'volume_type_id': 'fake', - 'project_id': 'project_id'} + db_vol = {'id': 'fake', 'host': 'fake', 'status': 'available', + 'size': 1, 'migration_status': None, + 'volume_type_id': 'fake', 'project_id': 'project_id'} + vol = fake_volume.fake_volume_obj(self.context, **db_vol) self.get_patcher = mock.patch('cinder.volume.API.get') self.mock_volume_get = self.get_patcher.start() self.addCleanup(self.get_patcher.stop) @@ -789,8 +794,9 @@ class VolumeImageActionsTest(test.TestCase): expected_res = { 'os-volume_upload_image': { 'id': id, - 'updated_at': datetime.datetime(1900, 1, 1, - 1, 1, 1), + 'updated_at': datetime.datetime( + 1900, 1, 1, 1, 1, 1, + tzinfo=iso8601.iso8601.Utc()), 'status': 'uploading', 'display_description': 'displaydesc', 'size': 1, @@ -845,8 +851,9 @@ class VolumeImageActionsTest(test.TestCase): expected_res = { 'os-volume_upload_image': { 'id': id, - 'updated_at': datetime.datetime(1900, 1, 1, - 1, 1, 1), + 'updated_at': datetime.datetime( + 1900, 1, 1, 1, 1, 1, + tzinfo=iso8601.iso8601.Utc()), 'status': 'uploading', 'display_description': 'displaydesc', 'size': 1, @@ -898,8 +905,9 @@ class VolumeImageActionsTest(test.TestCase): expected_res = { 'os-volume_upload_image': { 'id': id, - 'updated_at': datetime.datetime(1900, 1, 1, - 1, 1, 1), + 'updated_at': datetime.datetime( + 1900, 1, 1, 1, 1, 1, + tzinfo=iso8601.iso8601.Utc()), 'status': 'uploading', 'display_description': 'displaydesc', 'size': 1, @@ -944,8 +952,9 @@ class VolumeImageActionsTest(test.TestCase): expected_res = { 'os-volume_upload_image': { 'id': id, - 'updated_at': datetime.datetime(1900, 1, 1, - 1, 1, 1), + 'updated_at': datetime.datetime( + 1900, 1, 1, 1, 1, 1, + tzinfo=iso8601.iso8601.Utc()), 'status': 'uploading', 'display_description': 'displaydesc', 'size': 1, diff --git a/cinder/tests/unit/api/contrib/test_volume_host_attribute.py b/cinder/tests/unit/api/contrib/test_volume_host_attribute.py index 8660bd199..f6fe4919c 100644 --- a/cinder/tests/unit/api/contrib/test_volume_host_attribute.py +++ b/cinder/tests/unit/api/contrib/test_volume_host_attribute.py @@ -21,12 +21,14 @@ import webob from cinder import context from cinder import db +from cinder import objects from cinder import test from cinder.tests.unit.api import fakes +from cinder.tests.unit import fake_volume from cinder import volume -def fake_volume_get(*args, **kwargs): +def fake_db_volume_get(*args, **kwargs): return { 'id': 'fake', 'host': 'host001', @@ -42,11 +44,18 @@ def fake_volume_get(*args, **kwargs): 'project_id': 'fake', 'migration_status': None, '_name_id': 'fake2', + 'attach_status': 'detached', } +def fake_volume_api_get(*args, **kwargs): + ctx = context.RequestContext('admin', 'fake', True) + db_volume = fake_db_volume_get() + return fake_volume.fake_volume_obj(ctx, **db_volume) + + def fake_volume_get_all(*args, **kwargs): - return [fake_volume_get()] + return objects.VolumeList(objects=[fake_volume_api_get()]) def app(): @@ -61,9 +70,9 @@ class VolumeHostAttributeTest(test.TestCase): def setUp(self): super(VolumeHostAttributeTest, self).setUp() - self.stubs.Set(volume.API, 'get', fake_volume_get) + self.stubs.Set(volume.API, 'get', fake_volume_api_get) self.stubs.Set(volume.API, 'get_all', fake_volume_get_all) - self.stubs.Set(db, 'volume_get', fake_volume_get) + self.stubs.Set(db, 'volume_get', fake_db_volume_get) self.UUID = uuid.uuid4() diff --git a/cinder/tests/unit/api/contrib/test_volume_image_metadata.py b/cinder/tests/unit/api/contrib/test_volume_image_metadata.py index 937d4ca2e..eaa5f5fd1 100644 --- a/cinder/tests/unit/api/contrib/test_volume_image_metadata.py +++ b/cinder/tests/unit/api/contrib/test_volume_image_metadata.py @@ -26,12 +26,14 @@ from cinder.api.openstack import wsgi from cinder import context from cinder import db from cinder import exception +from cinder import objects from cinder import test from cinder.tests.unit.api import fakes +from cinder.tests.unit import fake_volume from cinder import volume -def fake_volume_get(*args, **kwargs): +def fake_db_volume_get(*args, **kwargs): return { 'id': 'fake', 'host': 'host001', @@ -45,11 +47,20 @@ def fake_volume_get(*args, **kwargs): 'volume_type_id': None, 'snapshot_id': None, 'project_id': 'fake', + 'migration_status': None, + '_name_id': 'fake2', + 'attach_status': 'detached', } +def fake_volume_api_get(*args, **kwargs): + ctx = context.RequestContext('admin', 'fake', True) + db_volume = fake_db_volume_get() + return fake_volume.fake_volume_obj(ctx, **db_volume) + + def fake_volume_get_all(*args, **kwargs): - return [fake_volume_get()] + return objects.VolumeList(objects=[fake_volume_api_get()]) fake_image_metadata = { @@ -90,13 +101,12 @@ class VolumeImageMetadataTest(test.TestCase): def setUp(self): super(VolumeImageMetadataTest, self).setUp() - self.stubs.Set(volume.API, 'get', fake_volume_get) + self.stubs.Set(volume.API, 'get', fake_volume_api_get) self.stubs.Set(volume.API, 'get_all', fake_volume_get_all) self.stubs.Set(volume.API, 'get_volume_image_metadata', fake_get_volume_image_metadata) self.stubs.Set(volume.API, 'get_volumes_image_metadata', fake_get_volumes_image_metadata) - self.stubs.Set(db, 'volume_get', fake_volume_get) self.UUID = uuid.uuid4() self.controller = (volume_image_metadata. VolumeImageMetadataController()) diff --git a/cinder/tests/unit/api/contrib/test_volume_manage.py b/cinder/tests/unit/api/contrib/test_volume_manage.py index 2419af1a0..d6de78a3e 100644 --- a/cinder/tests/unit/api/contrib/test_volume_manage.py +++ b/cinder/tests/unit/api/contrib/test_volume_manage.py @@ -20,6 +20,7 @@ from cinder import context from cinder import exception from cinder import test from cinder.tests.unit.api import fakes +from cinder.tests.unit import fake_volume def app(): @@ -82,21 +83,20 @@ def api_manage(*args, **kwargs): Note that we don't try to replicate any passed-in information (e.g. name, volume type) in the returned structure. """ + ctx = context.RequestContext('admin', 'fake', True) vol = { 'status': 'creating', 'display_name': 'fake_name', 'availability_zone': 'nova', 'tenant_id': 'fake', - 'created_at': 'DONTCARE', 'id': 'ffffffff-0000-ffff-0000-ffffffffffff', 'volume_type': None, 'snapshot_id': None, 'user_id': 'fake', - 'launched_at': 'DONTCARE', 'size': 0, 'attach_status': 'detached', 'volume_type_id': None} - return vol + return fake_volume.fake_volume_obj(ctx, **vol) @mock.patch('cinder.db.service_get_by_host_and_topic', diff --git a/cinder/tests/unit/api/contrib/test_volume_migration_status_attribute.py b/cinder/tests/unit/api/contrib/test_volume_migration_status_attribute.py index 07817450e..a3cc246d3 100644 --- a/cinder/tests/unit/api/contrib/test_volume_migration_status_attribute.py +++ b/cinder/tests/unit/api/contrib/test_volume_migration_status_attribute.py @@ -20,12 +20,14 @@ from oslo_utils import timeutils import webob from cinder import context +from cinder import objects from cinder import test from cinder.tests.unit.api import fakes +from cinder.tests.unit import fake_volume from cinder import volume -def fake_volume_get(*args, **kwargs): +def fake_db_volume_get(*args, **kwargs): return { 'id': 'fake', 'host': 'host001', @@ -33,7 +35,7 @@ def fake_volume_get(*args, **kwargs): 'size': 5, 'availability_zone': 'somewhere', 'created_at': timeutils.utcnow(), - 'attach_status': None, + 'attach_status': 'detached', 'display_name': 'anothervolume', 'display_description': 'Just another volume!', 'volume_type_id': None, @@ -44,8 +46,14 @@ def fake_volume_get(*args, **kwargs): } +def fake_volume_api_get(*args, **kwargs): + ctx = context.RequestContext('admin', 'fake', True) + db_volume = fake_db_volume_get() + return fake_volume.fake_volume_obj(ctx, **db_volume) + + def fake_volume_get_all(*args, **kwargs): - return [fake_volume_get()] + return objects.VolumeList(objects=[fake_volume_api_get()]) def app(): @@ -60,7 +68,7 @@ class VolumeMigStatusAttributeTest(test.TestCase): def setUp(self): super(VolumeMigStatusAttributeTest, self).setUp() - self.stubs.Set(volume.API, 'get', fake_volume_get) + self.stubs.Set(volume.API, 'get', fake_volume_api_get) self.stubs.Set(volume.API, 'get_all', fake_volume_get_all) self.UUID = uuid.uuid4() diff --git a/cinder/tests/unit/api/contrib/test_volume_tenant_attribute.py b/cinder/tests/unit/api/contrib/test_volume_tenant_attribute.py index 702634e74..54e255454 100644 --- a/cinder/tests/unit/api/contrib/test_volume_tenant_attribute.py +++ b/cinder/tests/unit/api/contrib/test_volume_tenant_attribute.py @@ -16,12 +16,13 @@ import json import uuid from lxml import etree -from oslo_utils import timeutils import webob from cinder import context +from cinder import objects from cinder import test from cinder.tests.unit.api import fakes +from cinder.tests.unit import fake_volume from cinder import volume @@ -29,26 +30,16 @@ PROJECT_ID = '88fd1da4-f464-4a87-9ce5-26f2f40743b9' def fake_volume_get(*args, **kwargs): - return { + ctx = context.RequestContext('non-admin', 'fake', False) + vol = { 'id': 'fake', - 'host': 'host001', - 'status': 'available', - 'size': 5, - 'availability_zone': 'somewhere', - 'created_at': timeutils.utcnow(), - 'attach_status': None, - 'display_name': 'anothervolume', - 'display_description': 'Just another volume!', - 'volume_type_id': None, - 'snapshot_id': None, 'project_id': PROJECT_ID, - 'migration_status': None, - '_name_id': 'fake2', } + return fake_volume.fake_volume_obj(ctx, **vol) def fake_volume_get_all(*args, **kwargs): - return [fake_volume_get()] + return objects.VolumeList(objects=[fake_volume_get()]) def app(): diff --git a/cinder/tests/unit/api/contrib/test_volume_transfer.py b/cinder/tests/unit/api/contrib/test_volume_transfer.py index c3e40bbd7..999c157be 100644 --- a/cinder/tests/unit/api/contrib/test_volume_transfer.py +++ b/cinder/tests/unit/api/contrib/test_volume_transfer.py @@ -63,6 +63,7 @@ class VolumeTransferAPITestCase(test.TestCase): vol['display_name'] = display_name vol['display_description'] = display_description vol['attach_status'] = status + vol['availability_zone'] = 'fake_zone' return db.volume_create(context.get_admin_context(), vol)['id'] def test_show_transfer(self): diff --git a/cinder/tests/unit/api/contrib/test_volume_unmanage.py b/cinder/tests/unit/api/contrib/test_volume_unmanage.py index 102d786c9..e94d8ad09 100644 --- a/cinder/tests/unit/api/contrib/test_volume_unmanage.py +++ b/cinder/tests/unit/api/contrib/test_volume_unmanage.py @@ -21,6 +21,7 @@ from cinder import exception from cinder import test from cinder.tests.unit.api import fakes from cinder.tests.unit import fake_snapshot +from cinder.tests.unit import fake_volume # This list of fake volumes is used by our tests. Each is configured in a @@ -78,7 +79,7 @@ def api_get(self, context, volume_id): if not vol: raise exception.VolumeNotFound(volume_id) - return vol + return fake_volume.fake_volume_obj(context, **vol) def db_snapshot_get_all_for_volume(context, volume_id): diff --git a/cinder/tests/unit/api/v1/test_snapshot_metadata.py b/cinder/tests/unit/api/v1/test_snapshot_metadata.py index 9d83798db..3fab2c339 100644 --- a/cinder/tests/unit/api/v1/test_snapshot_metadata.py +++ b/cinder/tests/unit/api/v1/test_snapshot_metadata.py @@ -30,6 +30,7 @@ from cinder import test from cinder.tests.unit.api import fakes from cinder.tests.unit import fake_snapshot from cinder.tests.unit import fake_volume +from cinder import volume CONF = cfg.CONF @@ -87,17 +88,18 @@ def return_snapshot(context, snapshot_id): 'metadata': {}} -def return_volume(context, volume_id): - return {'id': 'fake-vol-id', - 'size': 100, - 'name': 'fake', - 'host': 'fake-host', - 'status': 'available', - 'encryption_key_id': None, - 'volume_type_id': None, - 'migration_status': None, - 'metadata': {}, - 'project_id': context.project_id} +def stub_get(context, volume_id, *args, **kwargs): + vol = {'id': volume_id, + 'size': 100, + 'name': 'fake', + 'host': 'fake-host', + 'status': 'available', + 'encryption_key_id': None, + 'volume_type_id': None, + 'migration_status': None, + 'availability_zone': 'zone1:host1', + 'attach_status': 'detached'} + return fake_volume.fake_volume_obj(context, **vol) def return_snapshot_nonexistent(context, snapshot_id): @@ -113,7 +115,7 @@ class SnapshotMetaDataTest(test.TestCase): def setUp(self): super(SnapshotMetaDataTest, self).setUp() self.volume_api = cinder.volume.api.API() - self.stubs.Set(cinder.db, 'volume_get', return_volume) + self.stubs.Set(volume.API, 'get', stub_get) self.stubs.Set(cinder.db, 'snapshot_get', return_snapshot) self.stubs.Set(self.volume_api, 'update_snapshot_metadata', diff --git a/cinder/tests/unit/api/v1/test_volume_metadata.py b/cinder/tests/unit/api/v1/test_volume_metadata.py index 9c6a0d483..6ef0d6f7d 100644 --- a/cinder/tests/unit/api/v1/test_volume_metadata.py +++ b/cinder/tests/unit/api/v1/test_volume_metadata.py @@ -28,6 +28,8 @@ from cinder import exception from cinder import test from cinder.tests.unit.api import fakes from cinder.tests.unit.api.v1 import stubs +from cinder.tests.unit import fake_volume +from cinder import volume CONF = cfg.CONF @@ -54,9 +56,6 @@ def return_create_volume_metadata_insensitive(context, snapshot_id, def return_volume_metadata(context, volume_id): - if not isinstance(volume_id, str) or not len(volume_id) == 36: - msg = 'id %s must be a uuid in return volume metadata' % volume_id - raise Exception(msg) return stub_volume_metadata() @@ -108,11 +107,18 @@ def stub_max_volume_metadata(): return metadata -def return_volume(context, volume_id): - return {'id': '0cc3346e-9fef-4445-abe6-5d2b2690ec64', - 'name': 'fake', - 'metadata': {}, - 'project_id': context.project_id} +def get_volume(*args, **kwargs): + vol = {'id': args[1], + 'size': 100, + 'name': 'fake', + 'host': 'fake-host', + 'status': 'available', + 'encryption_key_id': None, + 'volume_type_id': None, + 'migration_status': None, + 'availability_zone': 'zone1:host1', + 'attach_status': 'detached'} + return fake_volume.fake_volume_obj(args[0], **vol) def return_volume_nonexistent(*args, **kwargs): @@ -128,7 +134,7 @@ class volumeMetaDataTest(test.TestCase): def setUp(self): super(volumeMetaDataTest, self).setUp() self.volume_api = cinder.volume.api.API() - self.stubs.Set(cinder.db, 'volume_get', return_volume) + self.stubs.Set(volume.API, 'get', get_volume) self.stubs.Set(cinder.db, 'volume_metadata_get', return_volume_metadata) self.stubs.Set(cinder.db, 'service_get_all_by_topic', @@ -337,8 +343,7 @@ class volumeMetaDataTest(test.TestCase): req, self.req_id, body) def test_create_nonexistent_volume(self): - self.stubs.Set(cinder.db, 'volume_get', - return_volume_nonexistent) + self.stubs.Set(volume.API, 'get', return_volume_nonexistent) self.stubs.Set(cinder.db, 'volume_metadata_get', return_volume_metadata) self.stubs.Set(cinder.db, 'volume_metadata_update', diff --git a/cinder/tests/unit/api/v1/test_volumes.py b/cinder/tests/unit/api/v1/test_volumes.py index f4371c2f9..df2ec828a 100644 --- a/cinder/tests/unit/api/v1/test_volumes.py +++ b/cinder/tests/unit/api/v1/test_volumes.py @@ -14,6 +14,7 @@ # under the License. import datetime +import iso8601 from lxml import etree import mock @@ -30,6 +31,7 @@ from cinder import test from cinder.tests.unit.api import fakes from cinder.tests.unit.api.v2 import stubs from cinder.tests.unit import fake_notifier +from cinder.tests.unit import fake_volume from cinder.tests.unit.image import fake as fake_image from cinder.volume import api as volume_api @@ -70,8 +72,8 @@ class VolumeApiTest(test.TestCase): self.stubs.Set(volume_api.API, 'delete', stubs.stub_volume_delete) def test_volume_create(self): - self.stubs.Set(volume_api.API, 'get', stubs.stub_volume_get) - self.stubs.Set(volume_api.API, "create", stubs.stub_volume_create) + self.stubs.Set(volume_api.API, "create", stubs.stub_volume_api_create) + self.stubs.Set(db, 'volume_type_get', stubs.stub_volume_type_get) vol = {"size": 100, "display_name": "Volume Test Name", @@ -92,8 +94,9 @@ class VolumeApiTest(test.TestCase): 'source_volid': None, 'metadata': {}, 'id': '1', - 'created_at': datetime.datetime(1900, 1, 1, - 1, 1, 1), + 'created_at': datetime.datetime( + 1900, 1, 1, 1, 1, 1, + tzinfo=iso8601.iso8601.Utc()), 'size': 100, 'encrypted': False}} self.assertEqual(expected, res_dict) @@ -157,9 +160,9 @@ class VolumeApiTest(test.TestCase): req, body) def test_volume_create_with_image_id(self): - self.stubs.Set(db, 'volume_get', stubs.stub_volume_get_db) + self.stubs.Set(volume_api.API, "create", stubs.stub_volume_api_create) + self.stubs.Set(db, 'volume_type_get', stubs.stub_volume_type_get) - self.stubs.Set(volume_api.API, "create", stubs.stub_volume_create) self.ext_mgr.extensions = {'os-image-create': 'fake'} test_id = "c905cedb-7281-47e4-8a62-f26bc5fc4c77" vol = {"size": '1', @@ -181,9 +184,10 @@ class VolumeApiTest(test.TestCase): 'source_volid': None, 'metadata': {}, 'id': '1', - 'created_at': datetime.datetime(1900, 1, 1, - 1, 1, 1), - 'size': '1'}} + 'created_at': datetime.datetime( + 1900, 1, 1, 1, 1, 1, + tzinfo=iso8601.iso8601.Utc()), + 'size': 1}} body = {"volume": vol} req = fakes.HTTPRequest.blank('/v1/volumes') res_dict = self.controller.create(req, body) @@ -237,9 +241,12 @@ class VolumeApiTest(test.TestCase): @mock.patch.object(db, 'volume_admin_metadata_get', return_value={'attached_mode': 'rw', 'readonly': 'False'}) - @mock.patch.object(db, 'volume_get', side_effect=stubs.stub_volume_get_db) + @mock.patch.object(db, 'volume_type_get', + side_effect=stubs.stub_volume_type_get) + @mock.patch.object(volume_api.API, 'get', + side_effect=stubs.stub_volume_api_get, autospec=True) @mock.patch.object(volume_api.API, 'update', - side_effect=stubs.stub_volume_update) + side_effect=stubs.stub_volume_update, autospec=True) def test_volume_update(self, *args): updates = { "display_name": "Updated Test Name", @@ -263,7 +270,8 @@ class VolumeApiTest(test.TestCase): 'metadata': {'attached_mode': 'rw', 'readonly': 'False'}, 'id': '1', - 'created_at': datetime.datetime(1900, 1, 1, 1, 1, 1), + 'created_at': datetime.datetime(1900, 1, 1, 1, 1, 1, + tzinfo=iso8601.iso8601.Utc()), 'size': 1}} self.assertEqual(expected, res_dict) self.assertEqual(2, len(self.notifier.notifications)) @@ -272,9 +280,12 @@ class VolumeApiTest(test.TestCase): return_value={"qos_max_iops": 2000, "readonly": "False", "attached_mode": "rw"}) - @mock.patch.object(db, 'volume_get', side_effect=stubs.stub_volume_get_db) + @mock.patch.object(db, 'volume_type_get', + side_effect=stubs.stub_volume_type_get) + @mock.patch.object(volume_api.API, 'get', + side_effect=stubs.stub_volume_api_get, autospec=True) @mock.patch.object(volume_api.API, 'update', - side_effect=stubs.stub_volume_update) + side_effect=stubs.stub_volume_update, autospec=True) def test_volume_update_metadata(self, *args): updates = { "metadata": {"qos_max_iops": 2000} @@ -295,11 +306,12 @@ class VolumeApiTest(test.TestCase): 'volume_type': 'vol_type_name', 'snapshot_id': None, 'source_volid': None, - 'metadata': {"qos_max_iops": 2000, + 'metadata': {"qos_max_iops": '2000', "readonly": "False", "attached_mode": "rw"}, 'id': '1', - 'created_at': datetime.datetime(1900, 1, 1, 1, 1, 1), + 'created_at': datetime.datetime(1900, 1, 1, 1, 1, 1, + tzinfo=iso8601.iso8601.Utc()), 'size': 1 }} self.assertEqual(expected, res_dict) @@ -359,7 +371,8 @@ class VolumeApiTest(test.TestCase): 'metadata': {'key': 'value', 'readonly': 'True'}, 'id': '1', - 'created_at': datetime.datetime(1900, 1, 1, 1, 1, 1), + 'created_at': datetime.datetime(1900, 1, 1, 1, 1, 1, + tzinfo=iso8601.iso8601.Utc()), 'size': 1}} self.assertEqual(expected, res_dict) self.assertEqual(2, len(self.notifier.notifications)) @@ -397,7 +410,8 @@ class VolumeApiTest(test.TestCase): stubs_volume_admin_metadata_get) self.stubs.Set(db, 'volume_get', stubs.stub_volume_get_db) self.stubs.Set(volume_api.API, 'get_all', - stubs.stub_volume_get_all_by_project) + stubs.stub_volume_api_get_all_by_project) + self.stubs.Set(db, 'volume_type_get', stubs.stub_volume_type_get) req = fakes.HTTPRequest.blank('/v1/volumes') res_dict = self.controller.index(req) @@ -415,8 +429,9 @@ class VolumeApiTest(test.TestCase): 'metadata': {'attached_mode': 'rw', 'readonly': 'False'}, 'id': '1', - 'created_at': datetime.datetime(1900, 1, 1, - 1, 1, 1), + 'created_at': datetime.datetime( + 1900, 1, 1, 1, 1, 1, + tzinfo=iso8601.iso8601.Utc()), 'size': 1}]} self.assertEqual(expected, res_dict) # Finally test that we cached the returned volumes @@ -462,15 +477,19 @@ class VolumeApiTest(test.TestCase): 'metadata': {'key': 'value', 'readonly': 'True'}, 'id': '1', - 'created_at': datetime.datetime(1900, 1, 1, - 1, 1, 1), + 'created_at': datetime.datetime( + 1900, 1, 1, 1, 1, 1, + tzinfo=iso8601.iso8601.Utc()), 'size': 1}]} self.assertEqual(expected, res_dict) - def test_volume_list_detail(self): - self.stubs.Set(db, 'volume_get', stubs.stub_volume_get_db) + @mock.patch.object(db, 'volume_admin_metadata_get', + return_value={'attached_mode': 'rw', + 'readonly': 'False'}) + def test_volume_list_detail(self, *args): self.stubs.Set(volume_api.API, 'get_all', - stubs.stub_volume_get_all_by_project) + stubs.stub_volume_api_get_all_by_project) + self.stubs.Set(db, 'volume_type_get', stubs.stub_volume_type_get) req = fakes.HTTPRequest.blank('/v1/volumes/detail') res_dict = self.controller.index(req) @@ -488,8 +507,9 @@ class VolumeApiTest(test.TestCase): 'metadata': {'attached_mode': 'rw', 'readonly': 'False'}, 'id': '1', - 'created_at': datetime.datetime(1900, 1, 1, - 1, 1, 1), + 'created_at': datetime.datetime( + 1900, 1, 1, 1, 1, 1, + tzinfo=iso8601.iso8601.Utc()), 'size': 1}]} self.assertEqual(expected, res_dict) # Finally test that we cached the returned volumes @@ -535,15 +555,19 @@ class VolumeApiTest(test.TestCase): 'metadata': {'key': 'value', 'readonly': 'True'}, 'id': '1', - 'created_at': datetime.datetime(1900, 1, 1, - 1, 1, 1), + 'created_at': datetime.datetime( + 1900, 1, 1, 1, 1, 1, + tzinfo=iso8601.iso8601.Utc()), 'size': 1}]} self.assertEqual(expected, res_dict) @mock.patch.object(db, 'volume_admin_metadata_get', return_value={'attached_mode': 'rw', 'readonly': 'False'}) - @mock.patch.object(db, 'volume_get', side_effect=stubs.stub_volume_get_db) + @mock.patch.object(volume_api.API, 'get', + side_effect=stubs.stub_volume_api_get, autospec=True) + @mock.patch.object(db, 'volume_type_get', + side_effect=stubs.stub_volume_type_get, autospec=True) def test_volume_show(self, *args): req = fakes.HTTPRequest.blank('/v1/volumes/1') res_dict = self.controller.show(req, '1') @@ -561,8 +585,9 @@ class VolumeApiTest(test.TestCase): 'metadata': {'attached_mode': 'rw', 'readonly': 'False'}, 'id': '1', - 'created_at': datetime.datetime(1900, 1, 1, - 1, 1, 1), + 'created_at': datetime.datetime( + 1900, 1, 1, 1, 1, 1, + tzinfo=iso8601.iso8601.Utc()), 'size': 1}} self.assertEqual(expected, res_dict) # Finally test that we cached the returned volume @@ -570,9 +595,11 @@ class VolumeApiTest(test.TestCase): def test_volume_show_no_attachments(self): def stub_volume_get(self, context, volume_id, **kwargs): - return stubs.stub_volume(volume_id, attach_status='detached') + vol = stubs.stub_volume(volume_id, attach_status='detached') + return fake_volume.fake_volume_obj(context, **vol) self.stubs.Set(volume_api.API, 'get', stub_volume_get) + self.stubs.Set(db, 'volume_type_get', stubs.stub_volume_type_get) req = fakes.HTTPRequest.blank('/v1/volumes/1') res_dict = self.controller.show(req, '1') @@ -589,17 +616,20 @@ class VolumeApiTest(test.TestCase): 'source_volid': None, 'metadata': {'readonly': 'False'}, 'id': '1', - 'created_at': datetime.datetime(1900, 1, 1, - 1, 1, 1), + 'created_at': datetime.datetime( + 1900, 1, 1, 1, 1, 1, + tzinfo=iso8601.iso8601.Utc()), 'size': 1}} self.assertEqual(expected, res_dict) def test_volume_show_bootable(self): def stub_volume_get(self, context, volume_id, **kwargs): - return (stubs.stub_volume(volume_id, - volume_glance_metadata=dict(foo='bar'))) + vol = (stubs.stub_volume(volume_id, + volume_glance_metadata=dict(foo='bar'))) + return fake_volume.fake_volume_obj(context, **vol) self.stubs.Set(volume_api.API, 'get', stub_volume_get) + self.stubs.Set(db, 'volume_type_get', stubs.stub_volume_type_get) req = fakes.HTTPRequest.blank('/v1/volumes/1') res_dict = self.controller.show(req, '1') @@ -617,8 +647,9 @@ class VolumeApiTest(test.TestCase): 'metadata': {'attached_mode': 'rw', 'readonly': 'False'}, 'id': '1', - 'created_at': datetime.datetime(1900, 1, 1, - 1, 1, 1), + 'created_at': datetime.datetime( + 1900, 1, 1, 1, 1, 1, + tzinfo=iso8601.iso8601.Utc()), 'size': 1}} self.assertEqual(expected, res_dict) @@ -648,6 +679,7 @@ class VolumeApiTest(test.TestCase): self.stubs.Set(db, 'volume_get_all_by_project', stub_volume_get_all_by_project) self.stubs.Set(db, 'volume_get', stubs.stub_volume_get_db) + self.stubs.Set(db, 'volume_type_get', stubs.stub_volume_type_get) req = fakes.HTTPRequest.blank('/v1/volumes/detail?limit=2\ &offset=1', @@ -655,7 +687,7 @@ class VolumeApiTest(test.TestCase): res_dict = self.controller.index(req) volumes = res_dict['volumes'] self.assertEqual(1, len(volumes)) - self.assertEqual(2, volumes[0]['id']) + self.assertEqual('2', volumes[0]['id']) # admin case volume_detail_limit_offset(is_admin=True) @@ -702,26 +734,27 @@ class VolumeApiTest(test.TestCase): 'metadata': {'key': 'value', 'readonly': 'True'}, 'id': '1', - 'created_at': datetime.datetime(1900, 1, 1, - 1, 1, 1), + 'created_at': datetime.datetime( + 1900, 1, 1, 1, 1, 1, + tzinfo=iso8601.iso8601.Utc()), 'size': 1}} self.assertEqual(expected, res_dict) def test_volume_show_with_encrypted_volume(self): def stub_volume_get(self, context, volume_id, **kwargs): - return stubs.stub_volume(volume_id, encryption_key_id='fake_id') + vol = stubs.stub_volume(volume_id, encryption_key_id='fake_id') + return fake_volume.fake_volume_obj(context, **vol) self.stubs.Set(volume_api.API, 'get', stub_volume_get) + self.stubs.Set(db, 'volume_type_get', stubs.stub_volume_type_get) req = fakes.HTTPRequest.blank('/v1/volumes/1') res_dict = self.controller.show(req, 1) self.assertEqual(True, res_dict['volume']['encrypted']) def test_volume_show_with_unencrypted_volume(self): - def stub_volume_get(self, context, volume_id, **kwargs): - return stubs.stub_volume(volume_id, encryption_key_id=None) - - self.stubs.Set(volume_api.API, 'get', stub_volume_get) + self.stubs.Set(volume_api.API, 'get', stubs.stub_volume_api_get) + self.stubs.Set(db, 'volume_type_get', stubs.stub_volume_type_get) req = fakes.HTTPRequest.blank('/v1/volumes/1') res_dict = self.controller.show(req, 1) @@ -746,6 +779,7 @@ class VolumeApiTest(test.TestCase): def test_admin_list_volumes_limited_to_project(self): self.stubs.Set(db, 'volume_get_all_by_project', stubs.stub_volume_get_all_by_project) + self.stubs.Set(db, 'volume_type_get', stubs.stub_volume_type_get) req = fakes.HTTPRequest.blank('/v1/fake/volumes', use_admin_context=True) @@ -755,6 +789,7 @@ class VolumeApiTest(test.TestCase): self.assertEqual(1, len(res['volumes'])) def test_admin_list_volumes_all_tenants(self): + self.stubs.Set(db, 'volume_type_get', stubs.stub_volume_type_get) req = fakes.HTTPRequest.blank('/v1/fake/volumes?all_tenants=1', use_admin_context=True) res = self.controller.index(req) @@ -765,6 +800,7 @@ class VolumeApiTest(test.TestCase): self.stubs.Set(db, 'volume_get_all_by_project', stubs.stub_volume_get_all_by_project) self.stubs.Set(db, 'volume_get', stubs.stub_volume_get_db) + self.stubs.Set(db, 'volume_type_get', stubs.stub_volume_type_get) req = fakes.HTTPRequest.blank('/v1/fake/volumes?all_tenants=1') res = self.controller.index(req) @@ -775,6 +811,7 @@ class VolumeApiTest(test.TestCase): self.stubs.Set(db, 'volume_get_all_by_project', stubs.stub_volume_get_all_by_project) self.stubs.Set(db, 'volume_get', stubs.stub_volume_get_db) + self.stubs.Set(db, 'volume_type_get', stubs.stub_volume_type_get) req = fakes.HTTPRequest.blank('/v1/fake/volumes') res = self.controller.index(req) diff --git a/cinder/tests/unit/api/v2/stubs.py b/cinder/tests/unit/api/v2/stubs.py index e42ff046d..e478658c0 100644 --- a/cinder/tests/unit/api/v2/stubs.py +++ b/cinder/tests/unit/api/v2/stubs.py @@ -14,8 +14,11 @@ # under the License. import datetime +import iso8601 from cinder import exception as exc +from cinder import objects +from cinder.tests.unit import fake_volume FAKE_UUID = 'aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa' @@ -49,8 +52,10 @@ def stub_volume(id, **kwargs): 'name': 'vol name', 'display_name': DEFAULT_VOL_NAME, 'display_description': DEFAULT_VOL_DESCRIPTION, - 'updated_at': datetime.datetime(1900, 1, 1, 1, 1, 1), - 'created_at': datetime.datetime(1900, 1, 1, 1, 1, 1), + 'updated_at': datetime.datetime(1900, 1, 1, 1, 1, 1, + tzinfo=iso8601.iso8601.Utc()), + 'created_at': datetime.datetime(1900, 1, 1, 1, 1, 1, + tzinfo=iso8601.iso8601.Utc()), 'snapshot_id': None, 'source_volid': None, 'volume_type_id': '3e196c20-3c06-11e2-81c1-0800200c9a66', @@ -58,7 +63,8 @@ def stub_volume(id, **kwargs): 'volume_admin_metadata': [{'key': 'attached_mode', 'value': 'rw'}, {'key': 'readonly', 'value': 'False'}], 'bootable': False, - 'launched_at': datetime.datetime(1900, 1, 1, 1, 1, 1), + 'launched_at': datetime.datetime(1900, 1, 1, 1, 1, 1, + tzinfo=iso8601.iso8601.Utc()), 'volume_type': {'name': DEFAULT_VOL_TYPE}, 'replication_status': 'disabled', 'replication_extended_status': None, @@ -84,6 +90,7 @@ def stub_volume_create(self, context, size, name, description, snapshot=None, source_volume = param.get('source_volume') or {} vol['source_volid'] = source_volume.get('id') vol['bootable'] = False + vol['volume_attachment'] = [] try: vol['snapshot_id'] = snapshot['id'] except (KeyError, TypeError): @@ -92,6 +99,11 @@ def stub_volume_create(self, context, size, name, description, snapshot=None, return vol +def stub_volume_api_create(self, context, *args, **kwargs): + vol = stub_volume_create(self, context, *args, **kwargs) + return fake_volume.fake_volume_obj(context, **vol) + + def stub_image_service_detail(self, context, **kwargs): filters = kwargs.get('filters', {'name': ''}) if filters['name'] == "Fedora-x86_64-20-20140618-sda": @@ -146,6 +158,11 @@ def stub_volume_get_db(context, volume_id): return volume +def stub_volume_api_get(self, context, volume_id, viewable_admin_meta=False): + vol = stub_volume(volume_id) + return fake_volume.fake_volume_obj(context, **vol) + + def stub_volume_get_all(context, search_opts=None, marker=None, limit=None, sort_keys=None, sort_dirs=None, filters=None, viewable_admin_meta=False, offset=None): @@ -162,6 +179,18 @@ def stub_volume_get_all_by_project(self, context, marker, limit, return [stub_volume_get(self, context, '1', viewable_admin_meta=True)] +def stub_volume_api_get_all_by_project(self, context, marker, limit, + sort_keys=None, sort_dirs=None, + filters=None, + viewable_admin_meta=False, + offset=None): + filters = filters or {} + vol = stub_volume_get(self, context, '1', + viewable_admin_meta=viewable_admin_meta) + vol_obj = fake_volume.fake_volume_obj(context, **vol) + return objects.VolumeList(objects=[vol_obj]) + + def stub_snapshot(id, **kwargs): snapshot = {'id': id, 'volume_id': 12, @@ -207,3 +236,24 @@ def stub_snapshot_get(self, context, snapshot_id): def stub_consistencygroup_get_notfound(self, context, cg_id): raise exc.ConsistencyGroupNotFound(consistencygroup_id=cg_id) + + +def stub_volume_type_get(context, id, *args, **kwargs): + return {'id': id, + 'name': 'vol_type_name', + 'description': 'A fake volume type', + 'is_public': True, + 'projects': [], + 'extra_specs': {}, + 'created_at': None, + 'deleted_at': None, + 'updated_at': None, + 'deleted': False} + + +def stub_volume_admin_metadata_get(context, volume_id, **kwargs): + admin_meta = {'attached_mode': 'rw', 'readonly': 'False'} + if kwargs.get('attach_status') == 'detached': + del admin_meta['attached_mode'] + + return admin_meta diff --git a/cinder/tests/unit/api/v2/test_snapshot_metadata.py b/cinder/tests/unit/api/v2/test_snapshot_metadata.py index ce5eb5d29..a5ac43b49 100644 --- a/cinder/tests/unit/api/v2/test_snapshot_metadata.py +++ b/cinder/tests/unit/api/v2/test_snapshot_metadata.py @@ -30,6 +30,7 @@ from cinder import test from cinder.tests.unit.api import fakes from cinder.tests.unit import fake_snapshot from cinder.tests.unit import fake_volume +from cinder import volume CONF = cfg.CONF @@ -87,17 +88,19 @@ def return_snapshot(context, snapshot_id): 'metadata': {}} -def return_volume(context, volume_id): - return {'id': 'fake-vol-id', - 'size': 100, - 'name': 'fake', - 'host': 'fake-host', - 'status': 'available', - 'encryption_key_id': None, - 'volume_type_id': None, - 'migration_status': None, - 'metadata': {}, - 'project_id': context.project_id} +def stub_get(context, *args, **kwargs): + vol = {'id': 'fake-vol-id', + 'size': 100, + 'name': 'fake', + 'host': 'fake-host', + 'status': 'available', + 'encryption_key_id': None, + 'volume_type_id': None, + 'migration_status': None, + 'availability_zone': 'fake-zone', + 'attach_status': 'detached', + 'metadata': {}} + return fake_volume.fake_volume_obj(context, **vol) def return_snapshot_nonexistent(context, snapshot_id): @@ -113,7 +116,7 @@ class SnapshotMetaDataTest(test.TestCase): def setUp(self): super(SnapshotMetaDataTest, self).setUp() self.volume_api = cinder.volume.api.API() - self.stubs.Set(cinder.db, 'volume_get', return_volume) + self.stubs.Set(volume.API, 'get', stub_get) self.stubs.Set(cinder.db, 'snapshot_get', return_snapshot) self.stubs.Set(self.volume_api, 'update_snapshot_metadata', diff --git a/cinder/tests/unit/api/v2/test_volume_metadata.py b/cinder/tests/unit/api/v2/test_volume_metadata.py index de6fa15c6..27d3a3530 100644 --- a/cinder/tests/unit/api/v2/test_volume_metadata.py +++ b/cinder/tests/unit/api/v2/test_volume_metadata.py @@ -28,6 +28,8 @@ from cinder import exception from cinder import test from cinder.tests.unit.api import fakes from cinder.tests.unit.api.v2 import stubs +from cinder.tests.unit import fake_volume +from cinder import volume from cinder.volume import api as volume_api @@ -55,9 +57,6 @@ def return_create_volume_metadata_insensitive(context, snapshot_id, def return_volume_metadata(context, volume_id): - if not isinstance(volume_id, str) or not len(volume_id) == 36: - msg = 'id %s must be a uuid in return volume metadata' % volume_id - raise Exception(msg) return stub_volume_metadata() @@ -109,11 +108,10 @@ def stub_max_volume_metadata(): return metadata -def return_volume(context, volume_id): - return {'id': '0cc3346e-9fef-4445-abe6-5d2b2690ec64', - 'name': 'fake', - 'metadata': {}, - 'project_id': context.project_id} +def get_volume(*args, **kwargs): + vol = {'name': 'fake', + 'metadata': {}} + return fake_volume.fake_volume_obj(args[0], **vol) def return_volume_nonexistent(*args, **kwargs): @@ -129,7 +127,7 @@ class volumeMetaDataTest(test.TestCase): def setUp(self): super(volumeMetaDataTest, self).setUp() self.volume_api = volume_api.API() - self.stubs.Set(db, 'volume_get', return_volume) + self.stubs.Set(volume.API, 'get', get_volume) self.stubs.Set(db, 'volume_metadata_get', return_volume_metadata) self.stubs.Set(db, 'service_get_all_by_topic', @@ -380,8 +378,7 @@ class volumeMetaDataTest(test.TestCase): req, self.req_id, body) def test_create_nonexistent_volume(self): - self.stubs.Set(db, 'volume_get', - return_volume_nonexistent) + self.stubs.Set(volume.API, 'get', return_volume_nonexistent) self.stubs.Set(db, 'volume_metadata_get', return_volume_metadata) self.stubs.Set(db, 'volume_metadata_update', diff --git a/cinder/tests/unit/api/v2/test_volumes.py b/cinder/tests/unit/api/v2/test_volumes.py index 659c4295e..0d1e2be48 100644 --- a/cinder/tests/unit/api/v2/test_volumes.py +++ b/cinder/tests/unit/api/v2/test_volumes.py @@ -15,7 +15,7 @@ import datetime -import functools +import iso8601 from lxml import etree import mock @@ -33,10 +33,12 @@ from cinder import consistencygroup as consistencygroupAPI from cinder import context from cinder import db from cinder import exception +from cinder import objects from cinder import test from cinder.tests.unit.api import fakes from cinder.tests.unit.api.v2 import stubs from cinder.tests.unit import fake_notifier +from cinder.tests.unit import fake_volume from cinder.tests.unit.image import fake as fake_image from cinder.tests.unit import utils from cinder.volume import api as volume_api @@ -69,7 +71,8 @@ class VolumeApiTest(test.TestCase): 'cinder.api.openstack.wsgi.Controller.validate_name_and_description') def test_volume_create(self, mock_validate): self.stubs.Set(volume_api.API, 'get', stubs.stub_volume_get) - self.stubs.Set(volume_api.API, "create", stubs.stub_volume_create) + self.stubs.Set(volume_api.API, "create", stubs.stub_volume_api_create) + self.stubs.Set(db, 'volume_type_get', stubs.stub_volume_type_get) vol = self._vol_in_request_body() body = {"volume": vol} @@ -111,10 +114,13 @@ class VolumeApiTest(test.TestCase): volume_id = res_dict['volume']['id'] self.assertEqual(1, len(res_dict)) + vol_db = stubs.stub_volume(volume_id, volume_type={'name': vol_type}) + vol_obj = fake_volume.fake_volume_obj(context.get_admin_context(), + **vol_db) self.stubs.Set(volume_api.API, 'get_all', lambda *args, **kwargs: - [stubs.stub_volume(volume_id, - volume_type={'name': vol_type})]) + objects.VolumeList(objects=[vol_obj])) + self.stubs.Set(db, 'volume_type_get', stubs.stub_volume_type_get) req = fakes.HTTPRequest.blank('/v2/volumes/detail') res_dict = self.controller.detail(req) self.assertTrue(mock_validate.called) @@ -170,8 +176,10 @@ class VolumeApiTest(test.TestCase): 'availability_zone': availability_zone, 'bootable': 'false', 'consistencygroup_id': consistencygroup_id, - 'created_at': datetime.datetime(1900, 1, 1, 1, 1, 1), - 'updated_at': datetime.datetime(1900, 1, 1, 1, 1, 1), + 'created_at': datetime.datetime( + 1900, 1, 1, 1, 1, 1, tzinfo=iso8601.iso8601.Utc()), + 'updated_at': datetime.datetime( + 1900, 1, 1, 1, 1, 1, tzinfo=iso8601.iso8601.Utc()), 'description': description, 'id': stubs.DEFAULT_VOL_ID, 'links': @@ -209,12 +217,14 @@ class VolumeApiTest(test.TestCase): 'multiattach': False, } + @mock.patch.object(db, 'volume_type_get', autospec=True) @mock.patch.object(volume_api.API, 'get_snapshot', autospec=True) @mock.patch.object(volume_api.API, 'create', autospec=True) - def test_volume_creation_from_snapshot(self, create, get_snapshot): - - create.side_effect = stubs.stub_volume_create + def test_volume_creation_from_snapshot(self, create, get_snapshot, + volume_type_get): + create.side_effect = stubs.stub_volume_api_create get_snapshot.side_effect = stubs.stub_snapshot_get + volume_type_get.side_effect = stubs.stub_volume_type_get snapshot_id = stubs.TEST_SNAPSHOT_UUID vol = self._vol_in_request_body(snapshot_id=stubs.TEST_SNAPSHOT_UUID) @@ -252,13 +262,14 @@ class VolumeApiTest(test.TestCase): get_snapshot.assert_called_once_with(self.controller.volume_api, context, snapshot_id) + @mock.patch.object(db, 'volume_type_get', autospec=True) @mock.patch.object(volume_api.API, 'get_volume', autospec=True) @mock.patch.object(volume_api.API, 'create', autospec=True) - def test_volume_creation_from_source_volume(self, create, get_volume): - - get_volume.side_effect = functools.partial(stubs.stub_volume_get, - viewable_admin_meta=True) - create.side_effect = stubs.stub_volume_create + def test_volume_creation_from_source_volume(self, create, get_volume, + volume_type_get): + get_volume.side_effect = stubs.stub_volume_api_get + create.side_effect = stubs.stub_volume_api_create + volume_type_get.side_effect = stubs.stub_volume_type_get source_volid = '2f49aa3a-6aae-488d-8b99-a43271605af6' vol = self._vol_in_request_body(source_volid=source_volid) @@ -273,8 +284,10 @@ class VolumeApiTest(test.TestCase): get_volume.assert_called_once_with(self.controller.volume_api, context, source_volid) + db_vol = stubs.stub_volume(source_volid) + vol_obj = fake_volume.fake_volume_obj(context, **db_vol) kwargs = self._expected_volume_api_create_kwargs( - source_volume=stubs.stub_volume(source_volid)) + source_volume=vol_obj) create.assert_called_once_with(self.controller.volume_api, context, vol['size'], stubs.DEFAULT_VOL_NAME, stubs.DEFAULT_VOL_DESCRIPTION, **kwargs) @@ -372,8 +385,8 @@ class VolumeApiTest(test.TestCase): @mock.patch( 'cinder.api.openstack.wsgi.Controller.validate_name_and_description') def test_volume_create_with_image_ref(self, mock_validate): - self.stubs.Set(volume_api.API, 'get', stubs.stub_volume_get) - self.stubs.Set(volume_api.API, "create", stubs.stub_volume_create) + self.stubs.Set(volume_api.API, "create", stubs.stub_volume_api_create) + self.stubs.Set(db, 'volume_type_get', stubs.stub_volume_type_get) self.ext_mgr.extensions = {'os-image-create': 'fake'} vol = self._vol_in_request_body( @@ -425,8 +438,8 @@ class VolumeApiTest(test.TestCase): @mock.patch( 'cinder.api.openstack.wsgi.Controller.validate_name_and_description') def test_volume_create_with_image_id(self, mock_validate): - self.stubs.Set(volume_api.API, 'get', stubs.stub_volume_get) - self.stubs.Set(volume_api.API, "create", stubs.stub_volume_create) + self.stubs.Set(volume_api.API, "create", stubs.stub_volume_api_create) + self.stubs.Set(db, 'volume_type_get', stubs.stub_volume_type_get) self.ext_mgr.extensions = {'os-image-create': 'fake'} vol = self._vol_in_request_body( @@ -478,8 +491,8 @@ class VolumeApiTest(test.TestCase): @mock.patch( 'cinder.api.openstack.wsgi.Controller.validate_name_and_description') def test_volume_create_with_image_name(self, mock_validate): - self.stubs.Set(db, 'volume_get', stubs.stub_volume_get_db) - self.stubs.Set(volume_api.API, "create", stubs.stub_volume_create) + self.stubs.Set(volume_api.API, "create", stubs.stub_volume_api_create) + self.stubs.Set(db, 'volume_type_get', stubs.stub_volume_type_get) self.stubs.Set(fake_image._FakeImageService, "detail", stubs.stub_image_service_detail) @@ -534,8 +547,9 @@ class VolumeApiTest(test.TestCase): @mock.patch( 'cinder.api.openstack.wsgi.Controller.validate_name_and_description') def test_volume_update(self, mock_validate): - self.stubs.Set(volume_api.API, 'get', stubs.stub_volume_get) + self.stubs.Set(volume_api.API, 'get', stubs.stub_volume_api_get) self.stubs.Set(volume_api.API, "update", stubs.stub_volume_update) + self.stubs.Set(db, 'volume_type_get', stubs.stub_volume_type_get) updates = { "name": "Updated Test Name", @@ -554,8 +568,9 @@ class VolumeApiTest(test.TestCase): @mock.patch( 'cinder.api.openstack.wsgi.Controller.validate_name_and_description') def test_volume_update_deprecation(self, mock_validate): - self.stubs.Set(volume_api.API, 'get', stubs.stub_volume_get) + self.stubs.Set(volume_api.API, 'get', stubs.stub_volume_api_get) self.stubs.Set(volume_api.API, "update", stubs.stub_volume_update) + self.stubs.Set(db, 'volume_type_get', stubs.stub_volume_type_get) updates = { "display_name": "Updated Test Name", @@ -577,8 +592,9 @@ class VolumeApiTest(test.TestCase): 'cinder.api.openstack.wsgi.Controller.validate_name_and_description') def test_volume_update_deprecation_key_priority(self, mock_validate): """Test current update keys have priority over deprecated keys.""" - self.stubs.Set(volume_api.API, 'get', stubs.stub_volume_get) + self.stubs.Set(volume_api.API, 'get', stubs.stub_volume_api_get) self.stubs.Set(volume_api.API, "update", stubs.stub_volume_update) + self.stubs.Set(db, 'volume_type_get', stubs.stub_volume_type_get) updates = { "name": "New Name", @@ -601,8 +617,9 @@ class VolumeApiTest(test.TestCase): @mock.patch( 'cinder.api.openstack.wsgi.Controller.validate_name_and_description') def test_volume_update_metadata(self, mock_validate): - self.stubs.Set(volume_api.API, 'get', stubs.stub_volume_get) + self.stubs.Set(volume_api.API, 'get', stubs.stub_volume_api_get) self.stubs.Set(volume_api.API, "update", stubs.stub_volume_update) + self.stubs.Set(db, 'volume_type_get', stubs.stub_volume_type_get) updates = { "metadata": {"qos_max_iops": 2000} @@ -614,7 +631,7 @@ class VolumeApiTest(test.TestCase): expected = self._expected_vol_from_controller( availability_zone=stubs.DEFAULT_AZ, metadata={'attached_mode': 'rw', 'readonly': 'False', - 'qos_max_iops': 2000}) + 'qos_max_iops': '2000'}) self.assertEqual(expected, res_dict) self.assertEqual(2, len(self.notifier.notifications)) self.assertTrue(mock_validate.called) @@ -659,11 +676,13 @@ class VolumeApiTest(test.TestCase): 'server_id': stubs.FAKE_UUID, 'host_name': None, 'device': '/', - 'attached_at': attach_tmp['attach_time'], + 'attached_at': attach_tmp['attach_time'].replace( + tzinfo=iso8601.iso8601.Utc()), }], metadata={'key': 'value', 'readonly': 'True'}, with_migration_status=True) - expected['volume']['updated_at'] = volume_tmp['updated_at'] + expected['volume']['updated_at'] = volume_tmp['updated_at'].replace( + tzinfo=iso8601.iso8601.Utc()) self.assertEqual(expected, res_dict) self.assertEqual(2, len(self.notifier.notifications)) self.assertTrue(mock_validate.called) @@ -697,8 +716,8 @@ class VolumeApiTest(test.TestCase): def test_volume_list_summary(self): self.stubs.Set(volume_api.API, 'get_all', - stubs.stub_volume_get_all_by_project) - self.stubs.Set(volume_api.API, 'get', stubs.stub_volume_get) + stubs.stub_volume_api_get_all_by_project) + self.stubs.Set(db, 'volume_type_get', stubs.stub_volume_type_get) req = fakes.HTTPRequest.blank('/v2/volumes') res_dict = self.controller.index(req) @@ -727,8 +746,8 @@ class VolumeApiTest(test.TestCase): def test_volume_list_detail(self): self.stubs.Set(volume_api.API, 'get_all', - stubs.stub_volume_get_all_by_project) - self.stubs.Set(volume_api.API, 'get', stubs.stub_volume_get) + stubs.stub_volume_api_get_all_by_project) + self.stubs.Set(db, 'volume_type_get', stubs.stub_volume_type_get) req = fakes.HTTPRequest.blank('/v2/volumes/detail') res_dict = self.controller.detail(req) @@ -772,11 +791,13 @@ class VolumeApiTest(test.TestCase): 'host_name': None, 'id': '1', 'volume_id': stubs.DEFAULT_VOL_ID, - 'attached_at': attach_tmp['attach_time'], + 'attached_at': attach_tmp['attach_time'].replace( + tzinfo=iso8601.iso8601.Utc()), }], metadata={'key': 'value', 'readonly': 'True'}, with_migration_status=True) - exp_vol['volume']['updated_at'] = volume_tmp['updated_at'] + exp_vol['volume']['updated_at'] = volume_tmp['updated_at'].replace( + tzinfo=iso8601.iso8601.Utc()) expected = {'volumes': [exp_vol['volume']]} self.assertEqual(expected, res_dict) @@ -798,8 +819,8 @@ class VolumeApiTest(test.TestCase): res_dict = self.controller.index(req) volumes = res_dict['volumes'] self.assertEqual(2, len(volumes)) - self.assertEqual(1, volumes[0]['id']) - self.assertEqual(2, volumes[1]['id']) + self.assertEqual('1', volumes[0]['id']) + self.assertEqual('2', volumes[1]['id']) def test_volume_index_limit(self): self.stubs.Set(db, 'volume_get_all_by_project', @@ -889,19 +910,19 @@ class VolumeApiTest(test.TestCase): ] self.stubs.Set(db, 'volume_get_all_by_project', stub_volume_get_all_by_project) - self.stubs.Set(volume_api.API, 'get', stubs.stub_volume_get) + self.stubs.Set(db, 'volume_type_get', stubs.stub_volume_type_get) req = fakes.HTTPRequest.blank('/v2/volumes/detail?marker=1') res_dict = self.controller.detail(req) volumes = res_dict['volumes'] self.assertEqual(2, len(volumes)) - self.assertEqual(1, volumes[0]['id']) - self.assertEqual(2, volumes[1]['id']) + self.assertEqual('1', volumes[0]['id']) + self.assertEqual('2', volumes[1]['id']) def test_volume_detail_limit(self): self.stubs.Set(db, 'volume_get_all_by_project', stubs.stub_volume_get_all_by_project) - self.stubs.Set(volume_api.API, 'get', stubs.stub_volume_get) + self.stubs.Set(db, 'volume_type_get', stubs.stub_volume_type_get) req = fakes.HTTPRequest.blank('/v2/volumes/detail?limit=1') res_dict = self.controller.detail(req) @@ -932,7 +953,7 @@ class VolumeApiTest(test.TestCase): def test_volume_detail_limit_marker(self): self.stubs.Set(db, 'volume_get_all_by_project', stubs.stub_volume_get_all_by_project) - self.stubs.Set(volume_api.API, 'get', stubs.stub_volume_get) + self.stubs.Set(db, 'volume_type_get', stubs.stub_volume_type_get) req = fakes.HTTPRequest.blank('/v2/volumes/detail?marker=1&limit=1') res_dict = self.controller.detail(req) @@ -1128,7 +1149,8 @@ class VolumeApiTest(test.TestCase): self.assertEqual('vol3', resp['volumes'][0]['name']) def test_volume_show(self): - self.stubs.Set(volume_api.API, 'get', stubs.stub_volume_get) + self.stubs.Set(volume_api.API, 'get', stubs.stub_volume_api_get) + self.stubs.Set(db, 'volume_type_get', stubs.stub_volume_type_get) req = fakes.HTTPRequest.blank('/v2/volumes/1') res_dict = self.controller.show(req, '1') @@ -1141,9 +1163,17 @@ class VolumeApiTest(test.TestCase): def test_volume_show_no_attachments(self): def stub_volume_get(self, context, volume_id, **kwargs): - return stubs.stub_volume(volume_id, attach_status='detached') + vol = stubs.stub_volume(volume_id, attach_status='detached') + return fake_volume.fake_volume_obj(context, **vol) + + def stub_volume_admin_metadata_get(context, volume_id, **kwargs): + return stubs.stub_volume_admin_metadata_get( + context, volume_id, attach_status='detached') self.stubs.Set(volume_api.API, 'get', stub_volume_get) + self.stubs.Set(db, 'volume_admin_metadata_get', + stub_volume_admin_metadata_get) + self.stubs.Set(db, 'volume_type_get', stubs.stub_volume_type_get) req = fakes.HTTPRequest.blank('/v2/volumes/1') res_dict = self.controller.show(req, '1') @@ -1193,28 +1223,30 @@ class VolumeApiTest(test.TestCase): 'server_id': stubs.FAKE_UUID, 'host_name': None, 'device': '/', - 'attached_at': attach_tmp['attach_time'], + 'attached_at': attach_tmp['attach_time'].replace( + tzinfo=iso8601.iso8601.Utc()), }], metadata={'key': 'value', 'readonly': 'True'}, with_migration_status=True) - expected['volume']['updated_at'] = volume_tmp['updated_at'] + expected['volume']['updated_at'] = volume_tmp['updated_at'].replace( + tzinfo=iso8601.iso8601.Utc()) self.assertEqual(expected, res_dict) def test_volume_show_with_encrypted_volume(self): def stub_volume_get(self, context, volume_id, **kwargs): - return stubs.stub_volume(volume_id, encryption_key_id='fake_id') + vol = stubs.stub_volume(volume_id, encryption_key_id='fake_id') + return fake_volume.fake_volume_obj(context, **vol) self.stubs.Set(volume_api.API, 'get', stub_volume_get) + self.stubs.Set(db, 'volume_type_get', stubs.stub_volume_type_get) req = fakes.HTTPRequest.blank('/v2/volumes/1') res_dict = self.controller.show(req, 1) self.assertEqual(True, res_dict['volume']['encrypted']) def test_volume_show_with_unencrypted_volume(self): - def stub_volume_get(self, context, volume_id, **kwargs): - return stubs.stub_volume(volume_id, encryption_key_id=None) - - self.stubs.Set(volume_api.API, 'get', stub_volume_get) + self.stubs.Set(volume_api.API, 'get', stubs.stub_volume_api_get) + self.stubs.Set(db, 'volume_type_get', stubs.stub_volume_type_get) req = fakes.HTTPRequest.blank('/v2/volumes/1') res_dict = self.controller.show(req, 1) diff --git a/cinder/tests/unit/test_cmd.py b/cinder/tests/unit/test_cmd.py index 7499e3fc7..704827bc8 100644 --- a/cinder/tests/unit/test_cmd.py +++ b/cinder/tests/unit/test_cmd.py @@ -36,6 +36,7 @@ from cinder.cmd import volume as cinder_volume from cinder.cmd import volume_usage_audit from cinder import context from cinder import test +from cinder.tests.unit import fake_volume from cinder import version CONF = cfg.CONF @@ -369,24 +370,6 @@ class TestCinderManageCmd(test.TestCase): def tearDown(self): super(TestCinderManageCmd, self).tearDown() - @mock.patch('oslo_utils.uuidutils.is_uuid_like') - def test_param2id(self, is_uuid_like): - mock_object_id = mock.MagicMock() - is_uuid_like.return_value = True - - object_id = cinder_manage.param2id(mock_object_id) - self.assertEqual(mock_object_id, object_id) - is_uuid_like.assert_called_once_with(mock_object_id) - - @mock.patch('oslo_utils.uuidutils.is_uuid_like') - def test_param2id_int_string(self, is_uuid_like): - object_id_str = '10' - is_uuid_like.return_value = False - - object_id = cinder_manage.param2id(object_id_str) - self.assertEqual(10, object_id) - is_uuid_like.assert_called_once_with(object_id_str) - @mock.patch('cinder.db.migration.db_sync') def test_db_commands_sync(self, db_sync): version = mock.MagicMock() @@ -485,28 +468,28 @@ class TestCinderManageCmd(test.TestCase): @mock.patch('cinder.rpc.init') def test_volume_commands_delete(self, rpc_init, get_client, get_admin_context, volume_get): - ctxt = context.RequestContext('fake-user', 'fake-project') + ctxt = context.RequestContext('admin', 'fake', True) get_admin_context.return_value = ctxt mock_client = mock.MagicMock() cctxt = mock.MagicMock() mock_client.prepare.return_value = cctxt get_client.return_value = mock_client - volume_id = '123' host = 'fake@host' - volume = {'id': volume_id, - 'host': host + '#pool1', - 'status': 'available'} + db_volume = {'host': host + '#pool1'} + volume = fake_volume.fake_db_volume(**db_volume) + volume_obj = fake_volume.fake_volume_obj(ctxt, **volume) + volume_id = volume['id'] volume_get.return_value = volume volume_cmds = cinder_manage.VolumeCommands() volume_cmds._client = mock_client volume_cmds.delete(volume_id) - volume_get.assert_called_once_with(ctxt, 123) - # NOTE prepare called w/o pool part in host + volume_get.assert_called_once_with(ctxt, volume_id) mock_client.prepare.assert_called_once_with(server=host) cctxt.cast.assert_called_once_with(ctxt, 'delete_volume', - volume_id=volume['id']) + volume_id=volume['id'], + volume=volume_obj) @mock.patch('cinder.db.volume_destroy') @mock.patch('cinder.db.volume_get') @@ -514,10 +497,11 @@ class TestCinderManageCmd(test.TestCase): @mock.patch('cinder.rpc.init') def test_volume_commands_delete_no_host(self, rpc_init, get_admin_context, volume_get, volume_destroy): - ctxt = context.RequestContext('fake-user', 'fake-project') + ctxt = context.RequestContext('fake-user', 'fake-project', + is_admin=True) get_admin_context.return_value = ctxt - volume_id = '123' - volume = {'id': volume_id, 'host': None, 'status': 'available'} + volume = fake_volume.fake_db_volume() + volume_id = volume['id'] volume_get.return_value = volume with mock.patch('sys.stdout', new=six.StringIO()) as fake_out: @@ -528,8 +512,10 @@ class TestCinderManageCmd(test.TestCase): volume_cmds.delete(volume_id) get_admin_context.assert_called_once_with() - volume_get.assert_called_once_with(ctxt, 123) - volume_destroy.assert_called_once_with(ctxt, 123) + volume_get.assert_called_once_with(ctxt, volume_id) + self.assertTrue(volume_destroy.called) + admin_context = volume_destroy.call_args[0][0] + self.assertTrue(admin_context.is_admin) self.assertEqual(expected_out, fake_out.getvalue()) @mock.patch('cinder.db.volume_destroy') @@ -541,8 +527,9 @@ class TestCinderManageCmd(test.TestCase): volume_get, volume_destroy): ctxt = context.RequestContext('fake-user', 'fake-project') get_admin_context.return_value = ctxt - volume_id = '123' - volume = {'id': volume_id, 'host': 'fake-host', 'status': 'in-use'} + db_volume = {'status': 'in-use', 'host': 'fake-host'} + volume = fake_volume.fake_db_volume(**db_volume) + volume_id = volume['id'] volume_get.return_value = volume with mock.patch('sys.stdout', new=six.StringIO()) as fake_out: @@ -552,7 +539,7 @@ class TestCinderManageCmd(test.TestCase): volume_cmds = cinder_manage.VolumeCommands() volume_cmds.delete(volume_id) - volume_get.assert_called_once_with(ctxt, 123) + volume_get.assert_called_once_with(ctxt, volume_id) self.assertEqual(expected_out, fake_out.getvalue()) def test_config_commands_list(self): diff --git a/cinder/tests/unit/test_quota.py b/cinder/tests/unit/test_quota.py index e35c518d0..ca6532df8 100644 --- a/cinder/tests/unit/test_quota.py +++ b/cinder/tests/unit/test_quota.py @@ -79,7 +79,11 @@ class QuotaIntegrationTestCase(test.TestCase): vol['status'] = 'available' vol['volume_type_id'] = self.volume_type['id'] vol['host'] = 'fake_host' - return db.volume_create(self.context, vol) + vol['availability_zone'] = 'fake_zone' + vol['attach_status'] = 'detached' + volume = objects.Volume(context=self.context, **vol) + volume.create() + return volume def _create_snapshot(self, volume): snapshot = objects.Snapshot(self.context) @@ -145,7 +149,7 @@ class QuotaIntegrationTestCase(test.TestCase): msg = ("Maximum number of volumes allowed (1) exceeded for" " quota '%s'." % resource) self.assertEqual(msg, six.text_type(ex)) - db.volume_destroy(self.context, vol_ref['id']) + vol_ref.destroy() def test_too_many_snapshots_of_type(self): resource = 'snapshots_%s' % self.volume_type_name @@ -161,7 +165,7 @@ class QuotaIntegrationTestCase(test.TestCase): volume.API().create_snapshot, self.context, vol_ref, '', '') snap_ref.destroy() - db.volume_destroy(self.context, vol_ref['id']) + vol_ref.destroy() def test_too_many_backups(self): resource = 'backups' @@ -211,7 +215,7 @@ class QuotaIntegrationTestCase(test.TestCase): self.project_id) self.assertEqual(20, usages['gigabytes']['in_use']) snap_ref.destroy() - db.volume_destroy(self.context, vol_ref['id']) + vol_ref.destroy() def test_too_many_combined_backup_gigabytes(self): vol_ref = self._create_volume(size=10000) @@ -229,7 +233,7 @@ class QuotaIntegrationTestCase(test.TestCase): container='container', incremental=False) db.backup_destroy(self.context, backup_ref['id']) - db.volume_destroy(self.context, vol_ref['id']) + vol_ref.destroy() def test_no_snapshot_gb_quota_flag(self): self.flags(quota_volumes=2, @@ -250,8 +254,8 @@ class QuotaIntegrationTestCase(test.TestCase): snap_ref.destroy() snap_ref2.destroy() - db.volume_destroy(self.context, vol_ref['id']) - db.volume_destroy(self.context, vol_ref2['id']) + vol_ref.destroy() + vol_ref2.destroy() def test_backup_gb_quota_flag(self): self.flags(quota_volumes=2, @@ -281,8 +285,8 @@ class QuotaIntegrationTestCase(test.TestCase): db.backup_destroy(self.context, backup_ref['id']) db.backup_destroy(self.context, backup_ref2['id']) - db.volume_destroy(self.context, vol_ref['id']) - db.volume_destroy(self.context, vol_ref2['id']) + vol_ref.destroy() + vol_ref2.destroy() def test_too_many_gigabytes_of_type(self): resource = 'gigabytes_%s' % self.volume_type_name @@ -299,7 +303,7 @@ class QuotaIntegrationTestCase(test.TestCase): expected = exception.VolumeSizeExceedsAvailableQuota( requested=1, quota=10, consumed=10, name=resource) self.assertEqual(str(expected), str(raised_exc)) - db.volume_destroy(self.context, vol_ref['id']) + vol_ref.destroy() class FakeContext(object): diff --git a/cinder/tests/unit/test_volume.py b/cinder/tests/unit/test_volume.py index e06ae27d8..8b6193906 100644 --- a/cinder/tests/unit/test_volume.py +++ b/cinder/tests/unit/test_volume.py @@ -580,7 +580,6 @@ class VolumeTestCase(BaseVolumeTestCase): self.assertEqual(4, len(self.notifier.notifications), self.notifier.notifications) msg = self.notifier.notifications[2] - expected['metadata'] = [] self.assertEqual('volume.delete.start', msg['event_type']) self.assertDictMatch(expected, msg['payload']) msg = self.notifier.notifications[3] @@ -1086,15 +1085,14 @@ class VolumeTestCase(BaseVolumeTestCase): 'volume_get_all_by_project') as by_project: with mock.patch.object(volume_api.db, 'volume_get_all') as get_all: - fake_volume = {'volume_type_id': 'fake_type_id', - 'name': 'fake_name', - 'host': 'fake_host', - 'id': 'fake_volume_id'} + db_volume = {'volume_type_id': 'fake_type_id', + 'name': 'fake_name', + 'host': 'fake_host', + 'id': 'fake_volume_id'} - fake_volume_list = [] - fake_volume_list.append([fake_volume]) - by_project.return_value = fake_volume_list - get_all.return_value = fake_volume_list + volume = fake_volume.fake_db_volume(**db_volume) + by_project.return_value = [volume] + get_all.return_value = [volume] volume_api.get_all(self.context, filters={'all_tenants': '0'}) self.assertTrue(by_project.called) @@ -3205,7 +3203,6 @@ class VolumeTestCase(BaseVolumeTestCase): volume = tests_utils.create_volume(self.context, **self.volume_params) self.volume.create_volume(self.context, volume['id']) volume['status'] = 'error_deleting' - volume['host'] = 'fakehost' volume_api = cinder.volume.api.API() @@ -3219,11 +3216,12 @@ class VolumeTestCase(BaseVolumeTestCase): volume_api.delete(self.context, volume, force=True) # status is deleting - volume = db.volume_get(context.get_admin_context(), volume['id']) - self.assertEqual('deleting', volume['status']) + volume = objects.Volume.get_by_id(context.get_admin_context(), + volume.id) + self.assertEqual('deleting', volume.status) # clean up - self.volume.delete_volume(self.context, volume['id']) + self.volume.delete_volume(self.context, volume.id) def test_cannot_force_delete_attached_volume(self): """Test volume can't be force delete in attached state.""" @@ -7664,8 +7662,8 @@ class ImageVolumeCacheTestCase(BaseVolumeTestCase): } volume_api = cinder.volume.api.API() volume = tests_utils.create_volume(self.context, **volume_params) - volume = db.volume_update(self.context, volume['id'], - {'status': 'available'}) + volume.status = 'available' + volume.save() image_id = '70a599e0-31e7-49b7-b260-868f441e862b' db.image_volume_cache_create(self.context, volume['host'], diff --git a/cinder/tests/unit/test_volume_rpcapi.py b/cinder/tests/unit/test_volume_rpcapi.py index 72af1a9c9..8e5bf1dc4 100644 --- a/cinder/tests/unit/test_volume_rpcapi.py +++ b/cinder/tests/unit/test_volume_rpcapi.py @@ -271,12 +271,25 @@ class VolumeRpcAPITestCase(test.TestCase): version='1.32') can_send_version.assert_called_once_with('1.32') - def test_delete_volume(self): + @mock.patch('oslo_messaging.RPCClient.can_send_version', + return_value=True) + def test_delete_volume(self, can_send_version): self._test_volume_api('delete_volume', rpc_method='cast', - volume=self.fake_volume, + volume=self.fake_volume_obj, + unmanage_only=False, + version='1.33') + can_send_version.assert_called_once_with('1.33') + + @mock.patch('oslo_messaging.RPCClient.can_send_version', + return_value=False) + def test_delete_volume_old(self, can_send_version): + self._test_volume_api('delete_volume', + rpc_method='cast', + volume=self.fake_volume_obj, unmanage_only=False, version='1.15') + can_send_version.assert_called_once_with('1.33') def test_create_snapshot(self): self._test_volume_api('create_snapshot', diff --git a/cinder/volume/api.py b/cinder/volume/api.py index 1c84175c8..68af86522 100644 --- a/cinder/volume/api.py +++ b/cinder/volume/api.py @@ -372,7 +372,7 @@ class API(base.Base): reservations = None LOG.exception(_LE("Failed to update quota while " "deleting volume.")) - self.db.volume_destroy(context.elevated(), volume_id) + volume.destroy() if reservations: QUOTAS.commit(context, reservations, project_id=project_id) @@ -440,15 +440,13 @@ class API(base.Base): msg = _("Unable to delete encrypted volume: %s.") % e.msg raise exception.InvalidVolume(reason=msg) - now = timeutils.utcnow() - vref = self.db.volume_update(context, - volume_id, - {'status': 'deleting', - 'terminated_at': now}) + volume.status = 'deleting' + volume.terminated_at = timeutils.utcnow() + volume.save() self.volume_rpcapi.delete_volume(context, volume, unmanage_only) LOG.info(_LI("Delete volume request issued successfully."), - resource=vref) + resource=volume) @wrap_check_policy def update(self, context, volume, fields): @@ -462,15 +460,14 @@ class API(base.Base): LOG.info(_LI("Volume updated successfully."), resource=vref) def get(self, context, volume_id, viewable_admin_meta=False): - rv = self.db.volume_get(context, volume_id) - - volume = dict(rv) + volume = objects.Volume.get_by_id(context, volume_id) if viewable_admin_meta: ctxt = context.elevated() admin_metadata = self.db.volume_admin_metadata_get(ctxt, volume_id) - volume['volume_admin_metadata'] = admin_metadata + volume.admin_metadata = admin_metadata + volume.obj_reset_changes() try: check_policy(context, 'get', volume) @@ -478,7 +475,7 @@ class API(base.Base): # raise VolumeNotFound instead to make sure Cinder behaves # as it used to raise exception.VolumeNotFound(volume_id=volume_id) - LOG.info(_LI("Volume info retrieved successfully."), resource=rv) + LOG.info(_LI("Volume info retrieved successfully."), resource=volume) return volume def get_all(self, context, marker=None, limit=None, sort_keys=None, @@ -514,21 +511,18 @@ class API(base.Base): if context.is_admin and allTenants: # Need to remove all_tenants to pass the filtering below. del filters['all_tenants'] - volumes = self.db.volume_get_all(context, marker, limit, - sort_keys=sort_keys, - sort_dirs=sort_dirs, - filters=filters, - offset=offset) + volumes = objects.VolumeList.get_all(context, marker, limit, + sort_keys=sort_keys, + sort_dirs=sort_dirs, + filters=filters, + offset=offset) else: if viewable_admin_meta: context = context.elevated() - volumes = self.db.volume_get_all_by_project(context, - context.project_id, - marker, limit, - sort_keys=sort_keys, - sort_dirs=sort_dirs, - filters=filters, - offset=offset) + volumes = objects.VolumeList.get_all_by_project( + context, context.project_id, marker, limit, + sort_keys=sort_keys, sort_dirs=sort_dirs, filters=filters, + offset=offset) LOG.info(_LI("Get all volumes completed successfully.")) return volumes @@ -545,9 +539,9 @@ class API(base.Base): def get_volume(self, context, volume_id): check_policy(context, 'get_volume') - vref = self.db.volume_get(context, volume_id) - LOG.info(_LI("Volume retrieved successfully."), resource=vref) - return dict(vref) + volume = objects.Volume.get_by_id(context, volume_id) + LOG.info(_LI("Volume retrieved successfully."), resource=volume) + return volume def get_all_snapshots(self, context, search_opts=None, marker=None, limit=None, sort_keys=None, sort_dirs=None, diff --git a/cinder/volume/manager.py b/cinder/volume/manager.py index 6a9b7a85e..b3af10c54 100644 --- a/cinder/volume/manager.py +++ b/cinder/volume/manager.py @@ -190,7 +190,7 @@ def locked_snapshot_operation(f): class VolumeManager(manager.SchedulerDependentManager): """Manages attachable block storage devices.""" - RPC_API_VERSION = '1.32' + RPC_API_VERSION = '1.33' target = messaging.Target(version=RPC_API_VERSION) @@ -376,7 +376,7 @@ class VolumeManager(manager.SchedulerDependentManager): # Initialize backend capabilities list self.driver.init_capabilities() - volumes = self.db.volume_get_all_by_host(ctxt, self.host) + volumes = objects.VolumeList.get_all_by_host(ctxt, self.host) snapshots = self.db.snapshot_get_by_host(ctxt, self.host) self._sync_provider_info(ctxt, volumes, snapshots) # FIXME volume count for exporting is wrong @@ -397,9 +397,8 @@ class VolumeManager(manager.SchedulerDependentManager): LOG.exception(_LE("Failed to re-export volume, " "setting to ERROR."), resource=volume) - self.db.volume_update(ctxt, - volume['id'], - {'status': 'error'}) + volume.status = 'error' + volume.save() elif volume['status'] in ('downloading', 'creating'): LOG.warning(_LW("Detected volume stuck " "in %(curr_status)s " @@ -409,9 +408,8 @@ class VolumeManager(manager.SchedulerDependentManager): if volume['status'] == 'downloading': self.driver.clear_download(ctxt, volume) - self.db.volume_update(ctxt, - volume['id'], - {'status': 'error'}) + volume.status = 'error' + volume.save() else: pass snapshots = objects.SnapshotList.get_by_host( @@ -579,7 +577,8 @@ class VolumeManager(manager.SchedulerDependentManager): return vol_ref.id @locked_volume_operation - def delete_volume(self, context, volume_id, unmanage_only=False): + def delete_volume(self, context, volume_id, unmanage_only=False, + volume=None): """Deletes and unexports volume. 1. Delete a volume(normal case) @@ -592,8 +591,13 @@ class VolumeManager(manager.SchedulerDependentManager): context = context.elevated() + # FIXME(thangp): Remove this in v2.0 of RPC API. + if volume is not None: + volume_id = volume.id + try: - volume_ref = self.db.volume_get(context, volume_id) + # TODO(thangp): Replace with volume.refresh() when it is available + volume = objects.Volume.get_by_id(context, volume_id) except exception.VolumeNotFound: # NOTE(thingee): It could be possible for a volume to # be deleted when resuming deletes from init_host(). @@ -601,51 +605,51 @@ class VolumeManager(manager.SchedulerDependentManager): volume_id) return True - if context.project_id != volume_ref['project_id']: - project_id = volume_ref['project_id'] + if context.project_id != volume.project_id: + project_id = volume.project_id else: project_id = context.project_id - if volume_ref['attach_status'] == "attached": + if volume['attach_status'] == "attached": # Volume is still attached, need to detach first raise exception.VolumeAttached(volume_id=volume_id) - if vol_utils.extract_host(volume_ref['host']) != self.host: + if vol_utils.extract_host(volume.host) != self.host: raise exception.InvalidVolume( reason=_("volume is not local to this node")) # The status 'deleting' is not included, because it only applies to # the source volume to be deleted after a migration. No quota # needs to be handled for it. - is_migrating = volume_ref['migration_status'] not in (None, 'error', - 'success') + is_migrating = volume.migration_status not in (None, 'error', + 'success') is_migrating_dest = (is_migrating and - volume_ref['migration_status'].startswith( + volume.migration_status.startswith( 'target:')) - self._notify_about_volume_usage(context, volume_ref, "delete.start") + self._notify_about_volume_usage(context, volume, "delete.start") try: # NOTE(flaper87): Verify the driver is enabled # before going forward. The exception will be caught # and the volume status updated. utils.require_driver_initialized(self.driver) - self.driver.remove_export(context, volume_ref) + self.driver.remove_export(context, volume) if unmanage_only: - self.driver.unmanage(volume_ref) + self.driver.unmanage(volume) else: - self.driver.delete_volume(volume_ref) + self.driver.delete_volume(volume) except exception.VolumeIsBusy: LOG.error(_LE("Unable to delete busy volume."), - resource=volume_ref) + resource=volume) # If this is a destination volume, we have to clear the database # record to avoid user confusion. - self._clear_db(context, is_migrating_dest, volume_ref, + self._clear_db(context, is_migrating_dest, volume, 'available') return True except Exception: with excutils.save_and_reraise_exception(): # If this is a destination volume, we have to clear the # database record to avoid user confusion. - self._clear_db(context, is_migrating_dest, volume_ref, + self._clear_db(context, is_migrating_dest, volume, 'error_deleting') # If deleting source/destination volume in a migration, we should @@ -654,39 +658,39 @@ class VolumeManager(manager.SchedulerDependentManager): # Get reservations try: reserve_opts = {'volumes': -1, - 'gigabytes': -volume_ref['size']} + 'gigabytes': -volume.size} QUOTAS.add_volume_type_opts(context, reserve_opts, - volume_ref.get('volume_type_id')) + volume.volume_type_id) reservations = QUOTAS.reserve(context, project_id=project_id, **reserve_opts) except Exception: reservations = None LOG.exception(_LE("Failed to update usages deleting volume."), - resource=volume_ref) + resource=volume) # Delete glance metadata if it exists self.db.volume_glance_metadata_delete_by_volume(context, volume_id) - self.db.volume_destroy(context, volume_id) + volume.destroy() # If deleting source/destination volume in a migration, we should # skip quotas. if not is_migrating: - self._notify_about_volume_usage(context, volume_ref, "delete.end") + self._notify_about_volume_usage(context, volume, "delete.end") # Commit the reservations if reservations: QUOTAS.commit(context, reservations, project_id=project_id) - pool = vol_utils.extract_host(volume_ref['host'], 'pool') + pool = vol_utils.extract_host(volume.host, 'pool') if pool is None: # Legacy volume, put them into default pool pool = self.driver.configuration.safe_get( 'volume_backend_name') or vol_utils.extract_host( - volume_ref['host'], 'pool', True) - size = volume_ref['size'] + volume.host, 'pool', True) + size = volume.size try: self.stats['pools'][pool]['allocated_capacity_gb'] -= size @@ -696,7 +700,7 @@ class VolumeManager(manager.SchedulerDependentManager): self.publish_service_capabilities(context) - LOG.info(_LI("Deleted volume successfully."), resource=volume_ref) + LOG.info(_LI("Deleted volume successfully."), resource=volume) return True def _clear_db(self, context, is_migrating_dest, volume_ref, status): @@ -704,14 +708,13 @@ class VolumeManager(manager.SchedulerDependentManager): # driver.delete_volume() fails in delete_volume(), so it is already # in the exception handling part. if is_migrating_dest: - self.db.volume_destroy(context, volume_ref['id']) + volume_ref.destroy() LOG.error(_LE("Unable to delete the destination volume " "during volume migration, (NOTE: database " "record needs to be deleted)."), resource=volume_ref) else: - self.db.volume_update(context, - volume_ref['id'], - {'status': status}) + volume_ref.status = status + volume_ref.save() def create_snapshot(self, context, volume_id, snapshot): """Creates and exports the snapshot.""" diff --git a/cinder/volume/rpcapi.py b/cinder/volume/rpcapi.py index 91f1a4245..24798e1c4 100644 --- a/cinder/volume/rpcapi.py +++ b/cinder/volume/rpcapi.py @@ -80,6 +80,7 @@ class VolumeAPI(object): and delete_cgsnapshot() to cast method only with necessary args. Forwarding CGSnapshot object instead of CGSnapshot_id. 1.32 - Adds support for sending objects over RPC in create_volume(). + 1.33 - Adds support for sending objects over RPC in delete_volume(). """ BASE_RPC_API_VERSION = '1.0' @@ -153,11 +154,16 @@ class VolumeAPI(object): cctxt.cast(ctxt, 'create_volume', **msg_args) def delete_volume(self, ctxt, volume, unmanage_only=False): - new_host = utils.extract_host(volume['host']) - cctxt = self.client.prepare(server=new_host, version='1.15') - cctxt.cast(ctxt, 'delete_volume', - volume_id=volume['id'], - unmanage_only=unmanage_only) + msg_args = {'volume_id': volume.id, 'unmanage_only': unmanage_only} + if self.client.can_send_version('1.33'): + version = '1.33' + msg_args['volume'] = volume + else: + version = '1.15' + + new_host = utils.extract_host(volume.host) + cctxt = self.client.prepare(server=new_host, version=version) + cctxt.cast(ctxt, 'delete_volume', **msg_args) def create_snapshot(self, ctxt, volume, snapshot): new_host = utils.extract_host(volume['host'])