From dabc7dedbb367b2eddf8db24bec6a91174d4245e Mon Sep 17 00:00:00 2001 From: Thang Pham Date: Tue, 3 Nov 2015 06:15:37 -0800 Subject: [PATCH] Update get/delete_volume API to use versionedobjects The following patch updates get_volume and delete_volume API to use volume versionedobjects. Changes were made to be backwards compatible with older RPC clients. It only includes changes to the core cinder code. Changes in the drivers are left to each driver maintainer to update. Note that this patch DOES NOT try to use object dot notation everywhere, since it would increase the size of the patch. Instead, it will be done in subsequent patches. Co-Authored-By: Michal Dulko Co-Authored-By: Szymon Wroblewski Change-Id: Ifb36726f8372e21d1d9825d6ab04a072e5db4a6a Partial-Implements: blueprint cinder-objects --- cinder/api/contrib/volume_manage.py | 1 - cinder/api/v1/volumes.py | 14 +- cinder/api/v2/views/volumes.py | 14 +- cinder/api/v2/volumes.py | 4 +- cinder/cmd/manage.py | 28 +--- .../unit/api/contrib/test_admin_actions.py | 105 +++++++------- .../unit/api/contrib/test_volume_actions.py | 31 ++-- .../api/contrib/test_volume_host_attribute.py | 17 ++- .../api/contrib/test_volume_image_metadata.py | 18 ++- .../unit/api/contrib/test_volume_manage.py | 6 +- .../test_volume_migration_status_attribute.py | 16 ++- .../contrib/test_volume_tenant_attribute.py | 21 +-- .../unit/api/contrib/test_volume_transfer.py | 1 + .../unit/api/contrib/test_volume_unmanage.py | 3 +- .../unit/api/v1/test_snapshot_metadata.py | 26 ++-- .../tests/unit/api/v1/test_volume_metadata.py | 27 ++-- cinder/tests/unit/api/v1/test_volumes.py | 131 ++++++++++------- cinder/tests/unit/api/v2/stubs.py | 56 +++++++- .../unit/api/v2/test_snapshot_metadata.py | 27 ++-- .../tests/unit/api/v2/test_volume_metadata.py | 19 ++- cinder/tests/unit/api/v2/test_volumes.py | 132 +++++++++++------- cinder/tests/unit/test_cmd.py | 55 +++----- cinder/tests/unit/test_quota.py | 24 ++-- cinder/tests/unit/test_volume.py | 28 ++-- cinder/tests/unit/test_volume_rpcapi.py | 17 ++- cinder/volume/api.py | 48 +++---- cinder/volume/manager.py | 77 +++++----- cinder/volume/rpcapi.py | 16 ++- 28 files changed, 543 insertions(+), 419 deletions(-) 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']) -- 2.45.2