diff --git a/karbor/api/v1/protectables.py b/karbor/api/v1/protectables.py index f40ca3ca..e339487b 100644 --- a/karbor/api/v1/protectables.py +++ b/karbor/api/v1/protectables.py @@ -77,6 +77,7 @@ class ProtectableViewBuilder(common.ViewBuilder): 'id': instance.get('id'), 'type': instance.get('type'), 'name': instance.get('name'), + 'extra_info': instance.get('extra_info'), 'dependent_resources': instance.get('dependent_resources'), } } diff --git a/karbor/resource.py b/karbor/resource.py index f1b36095..f30fa9fe 100644 --- a/karbor/resource.py +++ b/karbor/resource.py @@ -10,6 +10,11 @@ # License for the specific language governing permissions and limitations # under the License. + from collections import namedtuple -Resource = namedtuple("Resource", ('type', 'id', 'name')) +_Resource = namedtuple("Resource", ('type', 'id', 'name', 'extra_info')) + + +def Resource(type, id, name, extra_info=None): + return _Resource(type, id, name, extra_info) diff --git a/karbor/services/protection/flows/protect.py b/karbor/services/protection/flows/protect.py index 9e1164bb..f4fa9de1 100644 --- a/karbor/services/protection/flows/protect.py +++ b/karbor/services/protection/flows/protect.py @@ -46,7 +46,15 @@ class CompleteProtectTask(task.Task): def get_flow(context, protectable_registry, workflow_engine, plan, provider, checkpoint): - resources = set(Resource(**item) for item in plan.get("resources")) + # The 'extra-info' field of resources in plan is optional + # The extra_info field of the resource is a dict. + # The dict can not be handled by build_graph. It will throw a + # error. TypeError: unhashable type: 'dict' + resources = set() + for item in plan.get("resources"): + item["extra_info"] = None + resources.add(Resource(**item)) + resource_graph = protectable_registry.build_graph(context, resources) checkpoint.resource_graph = resource_graph diff --git a/karbor/services/protection/graph.py b/karbor/services/protection/graph.py index 68d96ae5..eab70316 100644 --- a/karbor/services/protection/graph.py +++ b/karbor/services/protection/graph.py @@ -228,6 +228,7 @@ def deserialize_resource_graph(serialized_resource_graph): for sid, node in packed_resource_graph.nodes.items(): packed_resource_graph.nodes[sid] = Resource(type=node[0], id=node[1], - name=node[2]) + name=node[2], + extra_info=node[3]) resource_graph = unpack_graph(packed_resource_graph) return resource_graph diff --git a/karbor/services/protection/manager.py b/karbor/services/protection/manager.py index d13cefd6..b182d7f8 100644 --- a/karbor/services/protection/manager.py +++ b/karbor/services/protection/manager.py @@ -293,7 +293,8 @@ class ProtectionManager(manager.Manager): result = [] for resource in resource_instances: - result.append(dict(id=resource.id, name=resource.name)) + result.append(dict(id=resource.id, name=resource.name, + extra_info=resource.extra_info)) return result @@ -320,7 +321,8 @@ class ProtectionManager(manager.Manager): raise return dict(id=resource_instance.id, name=resource_instance.name, - type=resource_instance.type) + type=resource_instance.type, + extra_info=resource_instance.extra_info) @messaging.expected_exceptions(exception.ListProtectableResourceFailed) def list_protectable_dependents(self, context, @@ -332,7 +334,7 @@ class ProtectionManager(manager.Manager): 'id': protectable_id}) parent_resource = Resource(type=protectable_type, id=protectable_id, - name="") + name="", extra_info=None) registry = self.protectable_registry try: @@ -347,7 +349,8 @@ class ProtectionManager(manager.Manager): result = [] for resource in dependent_resources: result.append(dict(type=resource.type, id=resource.id, - name=resource.name)) + name=resource.name, + extra_info=resource.extra_info)) return result diff --git a/karbor/services/protection/protectable_plugins/volume.py b/karbor/services/protection/protectable_plugins/volume.py index bb1542a0..c43e6211 100644 --- a/karbor/services/protection/protectable_plugins/volume.py +++ b/karbor/services/protection/protectable_plugins/volume.py @@ -52,10 +52,12 @@ class VolumeProtectablePlugin(protectable_plugin.ProtectablePlugin): type=self._SUPPORT_RESOURCE_TYPE, reason=six.text_type(e)) else: - return [resource.Resource(type=self._SUPPORT_RESOURCE_TYPE, - id=vol.id, name=vol.name) - for vol in volumes - if vol.status not in INVALID_VOLUME_STATUS] + return [resource.Resource( + type=self._SUPPORT_RESOURCE_TYPE, + id=vol.id, name=vol.name, + extra_info={'availability_zone': vol.availability_zone}) + for vol in volumes + if vol.status not in INVALID_VOLUME_STATUS] def show_resource(self, context, resource_id, parameters=None): try: @@ -71,8 +73,10 @@ class VolumeProtectablePlugin(protectable_plugin.ProtectablePlugin): raise exception.ProtectableResourceInvalidStatus( id=resource_id, type=self._SUPPORT_RESOURCE_TYPE, status=volume.status) - return resource.Resource(type=self._SUPPORT_RESOURCE_TYPE, - id=volume.id, name=volume.name) + return resource.Resource( + type=self._SUPPORT_RESOURCE_TYPE, + id=volume.id, name=volume.name, + extra_info={'availability_zone': volume.availability_zone}) def get_dependent_resources(self, context, parent_resource): def _is_attached_to(vol): @@ -93,6 +97,7 @@ class VolumeProtectablePlugin(protectable_plugin.ProtectablePlugin): type=self._SUPPORT_RESOURCE_TYPE, reason=six.text_type(e)) else: - return [resource.Resource(type=self._SUPPORT_RESOURCE_TYPE, - id=vol.id, name=vol.name) - for vol in volumes if _is_attached_to(vol)] + return [resource.Resource( + type=self._SUPPORT_RESOURCE_TYPE, id=vol.id, name=vol.name, + extra_info={'availability_zone': vol.availability_zone}) + for vol in volumes if _is_attached_to(vol)] diff --git a/karbor/services/protection/protectable_registry.py b/karbor/services/protection/protectable_registry.py index a7f62050..20cc0da4 100644 --- a/karbor/services/protection/protectable_registry.py +++ b/karbor/services/protection/protectable_registry.py @@ -12,6 +12,7 @@ from karbor import exception from karbor.i18n import _ +from karbor.resource import Resource from karbor.services.protection.graph import build_graph import six @@ -107,7 +108,15 @@ class ProtectableRegistry(object): def build_graph(self, context, resources): def fetch_dependent_resources_context(resource): - return self.fetch_dependent_resources(context, resource) + dependent_resources = self.fetch_dependent_resources( + context, resource) + # The extra_info field of the resource is a dict + # The dict can not be handled by build_graph, it will throw a + # error. TypeError: unhashable type: 'dict' + return [Resource(type=dependent_resource.type, + id=dependent_resource.id, + name=dependent_resource.name, extra_info=None) + for dependent_resource in dependent_resources] return build_graph( start_nodes=resources, diff --git a/karbor/tests/unit/plugins/test_volume_protectable_plugin.py b/karbor/tests/unit/plugins/test_volume_protectable_plugin.py index 9c77ca49..7cd28d36 100644 --- a/karbor/tests/unit/plugins/test_volume_protectable_plugin.py +++ b/karbor/tests/unit/plugins/test_volume_protectable_plugin.py @@ -24,7 +24,8 @@ from karbor.tests import base from oslo_config import cfg project_info = namedtuple('project_info', field_names=['id', 'type', 'name']) -vol_info = namedtuple('vol_info', ['id', 'attachments', 'name', 'status']) +vol_info = namedtuple('vol_info', ['id', 'attachments', 'name', 'status', + 'availability_zone']) class VolumeProtectablePluginTest(base.TestCase): @@ -69,23 +70,27 @@ class VolumeProtectablePluginTest(base.TestCase): @mock.patch.object(volumes.VolumeManager, 'list') def test_list_resources(self, mock_volume_list): plugin = VolumeProtectablePlugin(self._context) - mock_volume_list.return_value = [ - vol_info('123', [], 'name123', 'available'), - vol_info('456', [], 'name456', 'available'), + vol_info('123', [], 'name123', 'available', 'az1'), + vol_info('456', [], 'name456', 'available', 'az1'), ] - self.assertEqual([Resource('OS::Cinder::Volume', '123', 'name123'), - Resource('OS::Cinder::Volume', '456', 'name456')], + self.assertEqual([Resource('OS::Cinder::Volume', '123', 'name123', + {'availability_zone': 'az1'}), + Resource('OS::Cinder::Volume', '456', 'name456', + {'availability_zone': 'az1'})], plugin.list_resources(self._context)) @mock.patch.object(volumes.VolumeManager, 'get') def test_show_resource(self, mock_volume_get): plugin = VolumeProtectablePlugin(self._context) - vol_info = namedtuple('vol_info', ['id', 'name', 'status']) + vol_info = namedtuple('vol_info', ['id', 'name', 'status', + 'availability_zone']) mock_volume_get.return_value = vol_info(id='123', name='name123', - status='available') - self.assertEqual(Resource('OS::Cinder::Volume', '123', 'name123'), + status='available', + availability_zone='az1') + self.assertEqual(Resource('OS::Cinder::Volume', '123', 'name123', + {'availability_zone': 'az1'}), plugin.show_resource(self._context, "123")) @mock.patch.object(volumes.VolumeManager, 'list') @@ -94,13 +99,15 @@ class VolumeProtectablePluginTest(base.TestCase): attached = [{'server_id': 'abcdef', 'name': 'name'}] mock_volume_list.return_value = [ - vol_info('123', attached, 'name123', 'available'), - vol_info('456', [], 'name456', 'available'), + vol_info('123', attached, 'name123', 'available', 'az1'), + vol_info('456', [], 'name456', 'available', 'az1'), ] - self.assertEqual([Resource('OS::Cinder::Volume', '123', 'name123')], + self.assertEqual([Resource('OS::Cinder::Volume', '123', 'name123', + {'availability_zone': 'az1'})], plugin.get_dependent_resources( self._context, - Resource("OS::Nova::Server", 'abcdef', 'name'))) + Resource("OS::Nova::Server", 'abcdef', 'name', + {'availability_zone': 'az1'}))) @mock.patch.object(volumes.VolumeManager, 'list') def test_get_project_dependent_resources(self, mock_volume_list): @@ -109,8 +116,8 @@ class VolumeProtectablePluginTest(base.TestCase): plugin = VolumeProtectablePlugin(self._context) volumes = [ - mock.Mock(name='Volume', id='123'), - mock.Mock(name='Volume', id='456'), + mock.Mock(name='Volume', id='123', availability_zone='az1'), + mock.Mock(name='Volume', id='456', availability_zone='az1'), ] setattr(volumes[0], 'os-vol-tenant-attr:tenant_id', 'abcd') setattr(volumes[1], 'os-vol-tenant-attr:tenant_id', 'efgh') @@ -120,4 +127,5 @@ class VolumeProtectablePluginTest(base.TestCase): mock_volume_list.return_value = volumes self.assertEqual( plugin.get_dependent_resources(self._context, project), - [Resource('OS::Cinder::Volume', '123', 'name123')]) + [Resource('OS::Cinder::Volume', '123', 'name123', + {'availability_zone': 'az1'})]) diff --git a/karbor/tests/unit/protection/test_manager.py b/karbor/tests/unit/protection/test_manager.py index 347d05a5..54ee5695 100644 --- a/karbor/tests/unit/protection/test_manager.py +++ b/karbor/tests/unit/protection/test_manager.py @@ -82,7 +82,8 @@ class ProtectionServiceTest(base.TestCase): result = self.pro_manager.show_protectable_instance( fake_cntx, 'OS::Nova::Server', '123456') self.assertEqual( - {'id': '123456', 'name': 'name123', 'type': 'OS::Nova::Server'}, + {'id': '123456', 'name': 'name123', 'type': 'OS::Nova::Server', + 'extra_info': None}, result) @mock.patch.object(protectable_registry.ProtectableRegistry, @@ -98,8 +99,10 @@ class ProtectionServiceTest(base.TestCase): result = self.pro_manager.list_protectable_instances( fake_cntx, 'OS::Nova::Server') - self.assertEqual([{'id': '123456', 'name': 'name123'}, - {'id': '654321', 'name': 'name654'}], + self.assertEqual([{'id': '123456', 'name': 'name123', + 'extra_info': None}, + {'id': '654321', 'name': 'name654', + 'extra_info': None}], result) @mock.patch.object(protectable_registry.ProtectableRegistry, @@ -114,9 +117,9 @@ class ProtectionServiceTest(base.TestCase): result = self.pro_manager.list_protectable_dependents( fake_cntx, 'fake_id', 'OS::Nova::Server') self.assertEqual([{'type': 'OS::Cinder::Volume', 'id': '123456', - 'name': 'name123'}, + 'name': 'name123', 'extra_info': None}, {'type': 'OS::Cinder::Volume', 'id': '654321', - 'name': 'name654'}], + 'name': 'name654', 'extra_info': None}], result) @mock.patch.object(provider.ProviderRegistry, 'show_provider')