From 8e554059e2b88e0093ee44c1a549b5f76624b4f5 Mon Sep 17 00:00:00 2001 From: Karthik Prabhu Vinod Date: Wed, 19 Oct 2016 15:57:11 +0000 Subject: [PATCH] Switch ManageableSnaphots & ManageableVolumes list to OVO Currently, the results from ManageableVolumes & ManageableSnapshots list are being returned as Dict. This needs to be modeled as OVO as this is being returned via rpc from the driver to the api layer. We also change all occurences of List Manageable Volumes & snapshots to ovo Change-Id: Id63e4c35deec6dccc0ae6a82b004618cd214d96e --- cinder/hacking/checks.py | 3 +- cinder/objects/__init__.py | 1 + cinder/objects/base.py | 4 + cinder/objects/manageableresources.py | 103 +++++++++++++++++ .../tests/unit/api/v3/test_snapshot_manage.py | 5 +- .../tests/unit/api/v3/test_volume_manage.py | 5 +- .../test_manageable_volumes_snapshots.py | 107 ++++++++++++++++++ cinder/tests/unit/objects/test_objects.py | 4 + cinder/tests/unit/volume/test_rpcapi.py | 58 ++++++++++ cinder/volume/manager.py | 10 +- cinder/volume/rpcapi.py | 40 +++++-- 11 files changed, 324 insertions(+), 16 deletions(-) create mode 100644 cinder/objects/manageableresources.py create mode 100644 cinder/tests/unit/objects/test_manageable_volumes_snapshots.py diff --git a/cinder/hacking/checks.py b/cinder/hacking/checks.py index 73033af7ea9..804b54cf145 100644 --- a/cinder/hacking/checks.py +++ b/cinder/hacking/checks.py @@ -32,7 +32,8 @@ Guidelines for writing new hacking checks """ # NOTE(thangp): Ignore N323 pep8 error caused by importing cinder objects -UNDERSCORE_IMPORT_FILES = ['cinder/objects/__init__.py'] +UNDERSCORE_IMPORT_FILES = ['cinder/objects/__init__.py', + 'cinder/objects/manageableresources.py'] translated_log = re.compile( r"(.)*LOG\.(audit|error|info|warn|warning|critical|exception)" diff --git a/cinder/objects/__init__.py b/cinder/objects/__init__.py index 4f3980ce870..af38e187da4 100644 --- a/cinder/objects/__init__.py +++ b/cinder/objects/__init__.py @@ -40,3 +40,4 @@ def register_all(): __import__('cinder.objects.group_type') __import__('cinder.objects.group') __import__('cinder.objects.group_snapshot') + __import__('cinder.objects.manageableresources') diff --git a/cinder/objects/base.py b/cinder/objects/base.py index 4f89e88b0a4..21e0c00c39c 100644 --- a/cinder/objects/base.py +++ b/cinder/objects/base.py @@ -125,6 +125,10 @@ OBJ_VERSIONS.add('1.17', {'VolumeAttachment': '1.1'}) OBJ_VERSIONS.add('1.18', {'Snapshot': '1.3'}) OBJ_VERSIONS.add('1.19', {'ConsistencyGroup': '1.4', 'CGSnapshot': '1.1'}) OBJ_VERSIONS.add('1.20', {'Cluster': '1.1'}) +OBJ_VERSIONS.add('1.21', {'ManageableSnapshot': '1.0', + 'ManageableVolume': '1.0', + 'ManageableVolumeList': '1.0', + 'ManageableSnapshotList': '1.0'}) class CinderObjectRegistry(base.VersionedObjectRegistry): diff --git a/cinder/objects/manageableresources.py b/cinder/objects/manageableresources.py new file mode 100644 index 00000000000..3ccb38f9734 --- /dev/null +++ b/cinder/objects/manageableresources.py @@ -0,0 +1,103 @@ +# Copyright 2016 Intel Corporation +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +from oslo_versionedobjects import fields + +from cinder.objects import base + + +class ManageableObject(object): + + fields = { + 'reference': fields.DictOfNullableStringsField(nullable=False), + 'size': fields.IntegerField(nullable=True), + 'safe_to_manage': fields.BooleanField(default=False, nullable=True), + 'reason_not_safe': fields.StringField(nullable=True), + 'cinder_id': fields.UUIDField(nullable=True), + 'extra_info': fields.DictOfNullableStringsField(nullable=True), + } + + @classmethod + def from_primitives(cls, context, dict_resource): + resource = cls() + driverkeys = set(dict_resource.keys()) - set(cls.fields.keys()) + for name, field in cls.fields.items(): + value = dict_resource.get(name) + resource[name] = value + + for key in driverkeys: + if resource['extra_info'] is None: + resource['extra_info'] = {key: dict_resource[key]} + + resource._context = context + resource.obj_reset_changes() + return resource + + +@base.CinderObjectRegistry.register +class ManageableVolume(base.CinderObject, base.CinderObjectDictCompat, + base.CinderComparableObject, ManageableObject): + # Version 1.0: Initial version + VERSION = '1.0' + + +@base.CinderObjectRegistry.register +class ManageableSnapshot(base.CinderObject, base.CinderObjectDictCompat, + ManageableObject): + # Version 1.0: Initial version + VERSION = '1.0' + + fields = { + 'source_reference': fields.DictOfNullableStringsField(), + } + + +@base.CinderObjectRegistry.register +class ManageableVolumeList(base.ObjectListBase, base.CinderObject): + # Version 1.0: Initial version + VERSION = '1.0' + + fields = { + 'objects': fields.ListOfObjectsField('ManageableVolume'), + } + + @classmethod + def from_primitives(cls, context, data): + ManageableVolumeList.objects = [] + + for item in data: + manage_vol_obj = ManageableVolume.from_primitives(context, item) + ManageableVolumeList.objects.append(manage_vol_obj) + ManageableVolumeList._context = context + return ManageableVolumeList.objects + + +@base.CinderObjectRegistry.register +class ManageableSnapshotList(base.ObjectListBase, base.CinderObject): + # Version 1.0: Initial version + VERSION = '1.0' + + fields = { + 'objects': fields.ListOfObjectsField('ManageableSnapshot'), + } + + @classmethod + def from_primitives(cls, context, data): + ManageableSnapshotList.objects = [] + + for item in data: + manage_snap_obj = ManageableSnapshot.from_primitives(context, item) + ManageableSnapshotList.objects.append(manage_snap_obj) + ManageableSnapshotList._context = context + return ManageableSnapshotList.objects diff --git a/cinder/tests/unit/api/v3/test_snapshot_manage.py b/cinder/tests/unit/api/v3/test_snapshot_manage.py index f797bfe91bc..8b40d664d3b 100644 --- a/cinder/tests/unit/api/v3/test_snapshot_manage.py +++ b/cinder/tests/unit/api/v3/test_snapshot_manage.py @@ -174,11 +174,12 @@ class SnapshotManageTest(test.TestCase): **kwargs) self.assertEqual(200, res.status_int) - get_cctxt_mock.assert_called_once_with(service.service_topic_queue) + get_cctxt_mock.assert_called_once_with(service.service_topic_queue, + version=('3.10', '3.0')) get_cctxt_mock.return_value.call.assert_called_once_with( mock.ANY, 'get_manageable_snapshots', marker=None, limit=CONF.osapi_max_limit, offset=0, sort_keys=['reference'], - sort_dirs=['desc']) + sort_dirs=['desc'], want_objects=True) detail_view_mock.assert_called_once_with(mock.ANY, snaps, len(snaps)) get_service_mock.assert_called_once_with( mock.ANY, None, host=host, binary='cinder-volume', diff --git a/cinder/tests/unit/api/v3/test_volume_manage.py b/cinder/tests/unit/api/v3/test_volume_manage.py index 5f9a7c2535b..2a3fc29dab8 100644 --- a/cinder/tests/unit/api/v3/test_volume_manage.py +++ b/cinder/tests/unit/api/v3/test_volume_manage.py @@ -175,11 +175,12 @@ class VolumeManageTest(test.TestCase): **kwargs) self.assertEqual(200, res.status_int) - get_cctxt_mock.assert_called_once_with(service.service_topic_queue) + get_cctxt_mock.assert_called_once_with(service.service_topic_queue, + version=('3.10', '3.0')) get_cctxt_mock.return_value.call.assert_called_once_with( mock.ANY, 'get_manageable_volumes', marker=None, limit=CONF.osapi_max_limit, offset=0, sort_keys=['reference'], - sort_dirs=['desc']) + sort_dirs=['desc'], want_objects=True) detail_view_mock.assert_called_once_with(mock.ANY, volumes, len(volumes)) get_service_mock.assert_called_once_with( diff --git a/cinder/tests/unit/objects/test_manageable_volumes_snapshots.py b/cinder/tests/unit/objects/test_manageable_volumes_snapshots.py new file mode 100644 index 00000000000..a2b45ad3b7b --- /dev/null +++ b/cinder/tests/unit/objects/test_manageable_volumes_snapshots.py @@ -0,0 +1,107 @@ +# Copyright 2016 Intel Corporation +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +import ddt + +from cinder import objects +from cinder.tests.unit import objects as test_objects + + +@ddt.ddt +class TestManageableResources(test_objects.BaseObjectsTestCase): + + def resource_test(self, resource, resource_type): + if resource_type == "manageable_volume_obj": + resource.manageable_volume_obj.wrong_key + elif resource_type == "manageable_snapshot_obj": + resource.manageable_snapshot_obj.wrong_key + + def setUp(self): + super(TestManageableResources, self).setUp() + self.manageable_volume_dict = [ + {'cinder_id': + 'e334aab4-c987-4eb0-9c81-d4a773b4f7a6', + 'extra_info': None, + 'reason_not_safe': 'already managed', + 'reference': + {'source-name': + 'volume-e334aab4-c987-4eb0-9c81-d4a773b4f7a6'}, + 'safe_to_manage': False, + 'size': 1, + 'foo': 'bar'}, + {'cinder_id': + 'da25ac53-3fe0-4f56-9369-4d289d8902fd', + 'extra_info': None, + 'reason_not_safe': 'already managed', + 'reference': + {'source-name': + 'volume-da25ac53-3fe0-4f56-9369-4d289d8902fd'}, + 'safe_to_manage': False, + 'size': 2} + ] + + self.manageable_snapshot_dict = [ + {'cinder_id': + 'e334aab4-c987-4eb0-9c81-d4a773b4f7a6', + 'reference': + {'source-name': + 'volume-e334aab4-c987-4eb0-9c81-d4a773b4f7a6'}, + 'extra_info': None, + 'reason_not_safe': 'already managed', + 'source_reference': + {'source-name': + 'volume-e334aab4-c987-4eb0-9c81-d4a773b4f7a6'}, + 'safe_to_manage': False, + 'size': 1, + 'foo': 'bar'}, + {'cinder_id': + 'da25ac53-3fe0-4f56-9369-4d289d8902fd', + 'reference': + {'source-name': + 'volume-da25ac53-3fe0-4f56-9369-4d289d8902fd'}, + 'extra_info': None, + 'reason_not_safe': 'already managed', + 'source_reference': + {'source-name': + 'da25ac53-3fe0-4f56-9369-4d289d8902fd'}, + 'safe_to_manage': False, + 'size': 2} + ] + + vol_mang_list = (objects.ManageableVolumeList.from_primitives + (self.context, self.manageable_volume_dict)) + self.manageable_volume_obj_list = vol_mang_list + + snap_mang_list = (objects.ManageableSnapshotList.from_primitives + (self.context, self.manageable_snapshot_dict)) + self.manageable_snapshot_obj_list = snap_mang_list + + self.manageable_volume_obj = self.manageable_volume_obj_list[0] + self.manageable_snapshot_obj = self.manageable_snapshot_obj_list[0] + + @ddt.data('manageable_volume_obj', 'manageable_snapshot_obj') + def test_extra_info(self, obj): + # Making sure that any new key assignment gets stored in extra_info + # field of manageable_volume_object & manageable_snapshot_object + self.assertEqual( + 'bar', + getattr(self, obj).extra_info['foo']) + + @ddt.data('manageable_volume_obj', 'manageable_snapshot_obj') + def test_extra_info_wrong_key(self, obj): + # Making sure referring an attribute before setting it raises an + # Attribute Error for manageable_volume_object & + # manageable_snapshot_object + getattr(self, obj).foo = "test" + self.assertRaises(AttributeError, self.resource_test, self, obj) diff --git a/cinder/tests/unit/objects/test_objects.py b/cinder/tests/unit/objects/test_objects.py index 61f0fcb9aed..a1fa431ccdb 100644 --- a/cinder/tests/unit/objects/test_objects.py +++ b/cinder/tests/unit/objects/test_objects.py @@ -34,6 +34,10 @@ object_data = { 'CGSnapshotList': '1.0-15ecf022a68ddbb8c2a6739cfc9f8f5e', 'ConsistencyGroup': '1.4-7bf01a79b82516639fc03cd3ab6d9c01', 'ConsistencyGroupList': '1.1-15ecf022a68ddbb8c2a6739cfc9f8f5e', + 'ManageableSnapshot': '1.0-5be933366eb17d12db0115c597158d0d', + 'ManageableSnapshotList': '1.0-15ecf022a68ddbb8c2a6739cfc9f8f5e', + 'ManageableVolume': '1.0-5fd0152237ec9dfb7b5c7095b8b09ffa', + 'ManageableVolumeList': '1.0-15ecf022a68ddbb8c2a6739cfc9f8f5e', 'QualityOfServiceSpecs': '1.0-0b212e0a86ee99092229874e03207fe8', 'QualityOfServiceSpecsList': '1.0-1b54e51ad0fc1f3a8878f5010e7e16dc', 'RequestSpec': '1.1-b0bd1a28d191d75648901fa853e8a733', diff --git a/cinder/tests/unit/volume/test_rpcapi.py b/cinder/tests/unit/volume/test_rpcapi.py index 5403ef68c16..1ff2571b8a5 100644 --- a/cinder/tests/unit/volume/test_rpcapi.py +++ b/cinder/tests/unit/volume/test_rpcapi.py @@ -346,6 +346,7 @@ class VolumeRPCAPITestCase(test.RPCAPITestCase): def test_migrate_volume(self): class FakeBackend(object): + def __init__(self): self.host = 'fake_host' self.cluster_name = 'cluster_name' @@ -374,6 +375,7 @@ class VolumeRPCAPITestCase(test.RPCAPITestCase): def test_retype(self): class FakeBackend(object): + def __init__(self): self.host = 'fake_host' self.cluster_name = 'cluster_name' @@ -588,3 +590,59 @@ class VolumeRPCAPITestCase(test.RPCAPITestCase): self.context, cleanup_request) can_send_mock.assert_called_once_with('3.7') + + @ddt.data(('myhost', None, '3.10'), ('myhost', 'mycluster', '3.10'), + ('myhost', None, '3.0')) + @ddt.unpack + @mock.patch('oslo_messaging.RPCClient.can_send_version') + def test_get_manageable_volumes( + self, + host, + cluster_name, + version, + can_send_version): + can_send_version.side_effect = lambda x: x == version + service = objects.Service(self.context, host=host, + cluster_name=cluster_name) + expected_kwargs_diff = { + 'want_objects': True} if version == '3.10' else {} + self._test_rpc_api('get_manageable_volumes', + rpc_method='call', + service=service, + server=cluster_name or host, + marker=5, + limit=20, + offset=5, + sort_keys='fake_keys', + sort_dirs='fake_dirs', + expected_kwargs_diff=expected_kwargs_diff, + version=version) + can_send_version.assert_has_calls([mock.call('3.10')]) + + @ddt.data(('myhost', None, '3.10'), ('myhost', 'mycluster', '3.10'), + ('myhost', None, '3.0')) + @ddt.unpack + @mock.patch('oslo_messaging.RPCClient.can_send_version') + def test_get_manageable_snapshots( + self, + host, + cluster_name, + version, + can_send_version): + can_send_version.side_effect = lambda x: x == version + service = objects.Service(self.context, host=host, + cluster_name=cluster_name) + expected_kwargs_diff = { + 'want_objects': True} if version == '3.10' else {} + self._test_rpc_api('get_manageable_snapshots', + rpc_method='call', + service=service, + server=cluster_name or host, + marker=5, + limit=20, + offset=5, + sort_keys='fake_keys', + sort_dirs='fake_dirs', + expected_kwargs_diff=expected_kwargs_diff, + version=version) + can_send_version.assert_has_calls([mock.call('3.10')]) diff --git a/cinder/volume/manager.py b/cinder/volume/manager.py index a580e3d59b5..96ccba89835 100644 --- a/cinder/volume/manager.py +++ b/cinder/volume/manager.py @@ -2475,7 +2475,7 @@ class VolumeManager(manager.CleanableManager, return self._get_my_resources(ctxt, objects.SnapshotList) def get_manageable_volumes(self, ctxt, marker, limit, offset, sort_keys, - sort_dirs): + sort_dirs, want_objects=False): try: utils.require_driver_initialized(self.driver) except exception.DriverNotInitialized: @@ -2487,6 +2487,9 @@ class VolumeManager(manager.CleanableManager, try: driver_entries = self.driver.get_manageable_volumes( cinder_volumes, marker, limit, offset, sort_keys, sort_dirs) + if want_objects: + driver_entries = (objects.ManageableVolumeList. + from_primitives(ctxt, driver_entries)) except Exception: with excutils.save_and_reraise_exception(): LOG.exception(_LE("Listing manageable volumes failed, due " @@ -4402,7 +4405,7 @@ class VolumeManager(manager.CleanableManager, return snapshot.id def get_manageable_snapshots(self, ctxt, marker, limit, offset, - sort_keys, sort_dirs): + sort_keys, sort_dirs, want_objects=False): try: utils.require_driver_initialized(self.driver) except exception.DriverNotInitialized: @@ -4414,6 +4417,9 @@ class VolumeManager(manager.CleanableManager, try: driver_entries = self.driver.get_manageable_snapshots( cinder_snapshots, marker, limit, offset, sort_keys, sort_dirs) + if want_objects: + driver_entries = (objects.ManageableSnapshotList. + from_primitives(ctxt, driver_entries)) except Exception: with excutils.save_and_reraise_exception(): LOG.exception(_LE("Listing manageable snapshots failed, due " diff --git a/cinder/volume/rpcapi.py b/cinder/volume/rpcapi.py index ec28be9308b..2092bdebccf 100644 --- a/cinder/volume/rpcapi.py +++ b/cinder/volume/rpcapi.py @@ -124,9 +124,11 @@ class VolumeAPI(rpc.RPCAPI): that we were doing in init_host. 3.8 - Make failover_host cluster aware and add failover_completed. 3.9 - Adds new attach/detach methods + 3.10 - Returning objects instead of raw dictionaries in + get_manageable_volumes & get_manageable_snapshots """ - RPC_API_VERSION = '3.9' + RPC_API_VERSION = '3.10' RPC_DEFAULT_VERSION = '3.0' TOPIC = constants.VOLUME_TOPIC BINARY = 'cinder-volume' @@ -366,17 +368,37 @@ class VolumeAPI(rpc.RPCAPI): def get_manageable_volumes(self, ctxt, service, marker, limit, offset, sort_keys, sort_dirs): - cctxt = self._get_cctxt(service.service_topic_queue) - return cctxt.call(ctxt, 'get_manageable_volumes', marker=marker, - limit=limit, offset=offset, sort_keys=sort_keys, - sort_dirs=sort_dirs) + version = ('3.10', '3.0') + cctxt = self._get_cctxt(service.service_topic_queue, version=version) + + msg_args = {'marker': marker, + 'limit': limit, + 'offset': offset, + 'sort_keys': sort_keys, + 'sort_dirs': sort_dirs, + } + + if cctxt.can_send_version('3.10'): + msg_args['want_objects'] = True + + return cctxt.call(ctxt, 'get_manageable_volumes', **msg_args) def get_manageable_snapshots(self, ctxt, service, marker, limit, offset, sort_keys, sort_dirs): - cctxt = self._get_cctxt(service.service_topic_queue) - return cctxt.call(ctxt, 'get_manageable_snapshots', marker=marker, - limit=limit, offset=offset, sort_keys=sort_keys, - sort_dirs=sort_dirs) + version = ('3.10', '3.0') + cctxt = self._get_cctxt(service.service_topic_queue, version=version) + + msg_args = {'marker': marker, + 'limit': limit, + 'offset': offset, + 'sort_keys': sort_keys, + 'sort_dirs': sort_dirs, + } + + if cctxt.can_send_version('3.10'): + msg_args['want_objects'] = True + + return cctxt.call(ctxt, 'get_manageable_snapshots', **msg_args) def create_group(self, ctxt, group): cctxt = self._get_cctxt(group.service_topic_queue)