From 5d5666b10e57a94bcde06c8a296002175d935c7b Mon Sep 17 00:00:00 2001 From: Valeriy Ponomaryov Date: Tue, 30 May 2017 17:47:28 +0300 Subject: [PATCH] [Generic driver] Fix incompatibility with novaclient Novaclient has removed lots of proxies to other clients such as 'images' [1], 'networks' [2] and 'security_groups' [3]. So, fix incompatibilities the way we can work with old and new novaclient versions. [1] I2d9fd0243d42538bd1417a42357c17b09368d2a5 [2] I8c520100a0016eed3959619c71dae037ebd72939 [3] I6fa14f43d48f1e035ef54bd2d0078506f0c6d6e0 Change-Id: Id7383329b2491f76579f042cbed06585c2214815 Closes-Bug: #1691445 --- manila/compute/nova.py | 38 +----- manila/network/neutron/api.py | 31 +++++ manila/share/drivers/service_instance.py | 20 +-- manila/tests/compute/test_nova.py | 65 +++++----- .../share/drivers/test_service_instance.py | 114 +++++++++--------- 5 files changed, 134 insertions(+), 134 deletions(-) diff --git a/manila/compute/nova.py b/manila/compute/nova.py index 0f59db21..a38476fa 100644 --- a/manila/compute/nova.py +++ b/manila/compute/nova.py @@ -306,39 +306,13 @@ class API(base.Base): return novaclient(context).keypairs.list() def image_list(self, context): - return novaclient(context).images.list() + client = novaclient(context) + if hasattr(client, 'images'): + # Old novaclient with 'images' API proxy + return client.images.list() + # New novaclient without 'images' API proxy + return client.glance.list() def add_security_group_to_server(self, context, server, security_group): return novaclient(context).servers.add_security_group(server, security_group) - - def security_group_create(self, context, name, description=""): - return novaclient(context).security_groups.create(name, description) - - def security_group_get(self, context, group_id): - return novaclient(context).security_groups.get(group_id) - - def security_group_list(self, context, search_opts=None): - return novaclient(context).security_groups.list(search_opts) - - def security_group_rule_create(self, context, parent_group_id, - ip_protocol=None, from_port=None, - to_port=None, cidr=None, group_id=None): - return novaclient(context).security_group_rules.create( - parent_group_id, ip_protocol, from_port, to_port, cidr, group_id) - - def security_group_rule_delete(self, context, rule): - return novaclient(context).security_group_rules.delete(rule) - - def fixed_ip_reserve(self, context, fixed_ip): - return novaclient(context).fixed_ips.reserve(fixed_ip) - - def fixed_ip_unreserve(self, context, fixed_ip): - return novaclient(context).fixed_ips.unreserve(fixed_ip) - - def fixed_ip_get(self, context, fixed_ip): - return _to_dict(novaclient(context).fixed_ips.get(fixed_ip)) - - def network_get(self, context, network_id): - """Return network data by its ID.""" - return _to_dict(novaclient(context).networks.get(network_id)) diff --git a/manila/network/neutron/api.py b/manila/network/neutron/api.py index 56ecfa98..be74e7e5 100644 --- a/manila/network/neutron/api.py +++ b/manila/network/neutron/api.py @@ -371,3 +371,34 @@ class API(object): except neutron_client_exc.NeutronClientException as e: raise exception.NetworkException(code=e.status_code, message=e.message) + + def security_group_list(self, search_opts=None): + try: + return self.client.list_security_groups(**search_opts) + except neutron_client_exc.NeutronClientException as e: + raise exception.NetworkException( + code=e.status_code, message=e.message) + + def security_group_create(self, name, description=""): + try: + return self.client.create_security_group( + {"name": name, "description": description}) + except neutron_client_exc.NeutronClientException as e: + raise exception.NetworkException( + code=e.status_code, message=e.message) + + def security_group_rule_create(self, parent_group_id, + ip_protocol=None, from_port=None, + to_port=None, cidr=None, group_id=None): + try: + return self.client.create_security_group_rule({ + "parent_group_id": parent_group_id, + "ip_protocol": ip_protocol, + "from_port": from_port, + "to_port": to_port, + "cidr": cidr, + "group_id": group_id, + }) + except neutron_client_exc.NeutronClientException as e: + raise exception.NetworkException( + code=e.status_code, message=e.message) diff --git a/manila/share/drivers/service_instance.py b/manila/share/drivers/service_instance.py index 73cb1019..cbd84d9e 100644 --- a/manila/share/drivers/service_instance.py +++ b/manila/share/drivers/service_instance.py @@ -322,20 +322,22 @@ class ServiceInstanceManager(object): LOG.warning("Name for service instance security group is not " "provided. Skipping security group step.") return None - s_groups = [s for s in self.compute_api.security_group_list(context) - if s.name == name] + s_groups = self.network_helper.neutron_api.security_group_list({ + "name": name, + })['security_groups'] + s_groups = [s for s in s_groups if s['name'] == name] if not s_groups: # Creating security group if not description: - description = "This security group is intended "\ - "to be used by share service." + description = ("This security group is intended " + "to be used by share service.") LOG.debug("Creating security group with name '%s'.", name) - sg = self.compute_api.security_group_create( - context, name, description) + sg = self.network_helper.neutron_api.security_group_create( + name, description)['security_group'] for protocol, ports in const.SERVICE_INSTANCE_SECGROUP_DATA: - self.compute_api.security_group_rule_create( + self.network_helper.neutron_api.security_group_rule_create( context, - parent_group_id=sg.id, + parent_group_id=sg['id'], ip_protocol=protocol, from_port=ports[0], to_port=ports[1], @@ -542,7 +544,7 @@ class ServiceInstanceManager(object): security_group = self._get_or_create_security_group(context) if security_group: - sg_id = security_group.id + sg_id = security_group['id'] LOG.debug( "Adding security group '%(sg)s' to server '%(si)s'.", dict(sg=sg_id, si=service_instance["id"])) diff --git a/manila/tests/compute/test_nova.py b/manila/tests/compute/test_nova.py index 09ef9d9b..83eab40e 100644 --- a/manila/tests/compute/test_nova.py +++ b/manila/tests/compute/test_nova.py @@ -69,22 +69,11 @@ class FakeNovaClient(object): def get(self, net_id): return Network(net_id) - class FixedIPs(object): - def get(self, fixed_ip): - return dict(address=fixed_ip) - - def reserve(self, fixed_ip): - return None - - def unreserve(self, fixed_ip): - return None - def __init__(self): self.servers = self.Servers() self.volumes = self.Volumes() self.keypairs = self.servers self.networks = self.Networks() - self.fixed_ips = self.FixedIPs() @nova.translate_server_exception @@ -221,6 +210,38 @@ class NovaApiTestCase(test.TestCase): self.mock_object(nova, '_untranslate_server_summary_view', lambda server: server) + def test_image_list_novaclient_has_no_proxy(self): + image_list = ['fake', 'image', 'list'] + + class FakeGlanceClient(object): + def list(self): + return image_list + + self.novaclient.glance = FakeGlanceClient() + + result = self.api.image_list(self.ctx) + + self.assertEqual(image_list, result) + + def test_image_list_novaclient_has_proxy(self): + image_list1 = ['fake', 'image', 'list1'] + image_list2 = ['fake', 'image', 'list2'] + + class FakeImagesClient(object): + def list(self): + return image_list1 + + class FakeGlanceClient(object): + def list(self): + return image_list2 + + self.novaclient.images = FakeImagesClient() + self.novaclient.glance = FakeGlanceClient() + + result = self.api.image_list(self.ctx) + + self.assertEqual(image_list1, result) + def test_server_create(self): result = self.api.server_create(self.ctx, 'server_name', 'fake_image', 'fake_flavor', None, None, None) @@ -372,28 +393,6 @@ class NovaApiTestCase(test.TestCase): self.assertEqual([{'id': 'id1'}, {'id': 'id2'}], self.api.keypair_list(self.ctx)) - def test_fixed_ip_get(self): - fixed_ip = 'fake_fixed_ip' - result = self.api.fixed_ip_get(self.ctx, fixed_ip) - self.assertIsInstance(result, dict) - self.assertEqual(fixed_ip, result['address']) - - def test_fixed_ip_reserve(self): - fixed_ip = 'fake_fixed_ip' - result = self.api.fixed_ip_reserve(self.ctx, fixed_ip) - self.assertIsNone(result) - - def test_fixed_ip_unreserve(self): - fixed_ip = 'fake_fixed_ip' - result = self.api.fixed_ip_unreserve(self.ctx, fixed_ip) - self.assertIsNone(result) - - def test_network_get(self): - net_id = 'fake_net_id' - net = self.api.network_get(self.ctx, net_id) - self.assertIsInstance(net, dict) - self.assertEqual(net_id, net['id']) - class ToDictTestCase(test.TestCase): diff --git a/manila/tests/share/drivers/test_service_instance.py b/manila/tests/share/drivers/test_service_instance.py index b5ab8963..76e1dd94 100644 --- a/manila/tests/share/drivers/test_service_instance.py +++ b/manila/tests/share/drivers/test_service_instance.py @@ -97,6 +97,12 @@ class FakeNetworkHelper(service_instance.BaseNetworkhelper): def NAME(self): return service_instance.NEUTRON_NAME + @property + def neutron_api(self): + if not hasattr(self, '_neutron_api'): + self._neutron_api = mock.Mock() + return self._neutron_api + def __init__(self, service_instance_manager): self.get_config_option = service_instance_manager.get_config_option @@ -356,78 +362,66 @@ class ServiceInstanceManagerTestCase(test.TestCase): 'service_instance_security_group') def test_security_group_name_from_config_and_sg_exist(self): - fake_secgroup = fake_compute.FakeSecurityGroup(name="fake_sg_name") - self.mock_object(self._manager, 'get_config_option', - mock.Mock(return_value="fake_sg_name")) - self.mock_object(self._manager.compute_api, 'security_group_list', - mock.Mock(return_value=[fake_secgroup, ])) - result = self._manager._get_or_create_security_group( - self._manager.admin_context) - self.assertEqual(fake_secgroup, result) - self._manager.get_config_option.assert_has_calls([ - mock.call('service_instance_security_group'), - ]) - self._manager.compute_api.security_group_list.assert_called_once_with( - self._manager.admin_context) - - def test_security_group_creation_with_name_from_config(self): - name = "fake_sg_name" + name = "fake_sg_name_from_config" desc = "fake_sg_description" - fake_secgroup = fake_compute.FakeSecurityGroup(name=name, - description=desc) + fake_secgroup = {'id': 'fake_sg_id', 'name': name, 'description': desc} self.mock_object(self._manager, 'get_config_option', mock.Mock(return_value=name)) - self.mock_object(self._manager.compute_api, 'security_group_list', - mock.Mock(return_value=[])) - self.mock_object(self._manager.compute_api, 'security_group_create', - mock.Mock(return_value=fake_secgroup)) - self.mock_object(self._manager.compute_api, - 'security_group_rule_create') + neutron_api = self._manager.network_helper.neutron_api + neutron_api.security_group_list.return_value = { + 'security_groups': [fake_secgroup]} + + result = self._manager._get_or_create_security_group( + self._manager.admin_context) + + self.assertEqual(fake_secgroup, result) + self._manager.get_config_option.assert_called_once_with( + 'service_instance_security_group') + neutron_api.security_group_list.assert_called_once_with({"name": name}) + + @ddt.data(None, 'fake_name') + def test_security_group_creation_with_name_from_config(self, name): + config_name = "fake_sg_name_from_config" + desc = "fake_sg_description" + fake_secgroup = {'id': 'fake_sg_id', 'name': name, 'description': desc} + self.mock_object(self._manager, 'get_config_option', + mock.Mock(return_value=name or config_name)) + neutron_api = self._manager.network_helper.neutron_api + neutron_api.security_group_list.return_value = {'security_groups': []} + neutron_api.security_group_create.return_value = { + 'security_group': fake_secgroup, + } + result = self._manager._get_or_create_security_group( context=self._manager.admin_context, - name=None, + name=name, description=desc, ) - self.assertEqual(fake_secgroup, result) - self._manager.compute_api.security_group_list.assert_called_once_with( - self._manager.admin_context) - self._manager.compute_api.security_group_create.\ - assert_called_once_with(self._manager.admin_context, name, desc) - self._manager.get_config_option.assert_has_calls([ - mock.call('service_instance_security_group'), - ]) - def test_security_group_creation_with_provided_name(self): - name = "fake_sg_name" - fake_secgroup = fake_compute.FakeSecurityGroup(name=name) - self.mock_object(self._manager.compute_api, 'security_group_list', - mock.Mock(return_value=[])) - self.mock_object(self._manager.compute_api, 'security_group_create', - mock.Mock(return_value=fake_secgroup)) - self.mock_object(self._manager.compute_api, - 'security_group_rule_create') - result = self._manager._get_or_create_security_group( - context=self._manager.admin_context, name=name) - self._manager.compute_api.security_group_list.assert_called_once_with( - self._manager.admin_context) - self._manager.compute_api.security_group_create.\ - assert_called_once_with( - self._manager.admin_context, name, mock.ANY) self.assertEqual(fake_secgroup, result) + if not name: + self._manager.get_config_option.assert_called_once_with( + 'service_instance_security_group') + neutron_api.security_group_list.assert_called_once_with( + {"name": name or config_name}) + neutron_api.security_group_create.assert_called_once_with( + name or config_name, desc) def test_security_group_two_sg_in_list(self): name = "fake_name" - fake_secgroup1 = fake_compute.FakeSecurityGroup(name=name) - fake_secgroup2 = fake_compute.FakeSecurityGroup(name=name) - self.mock_object(self._manager.compute_api, 'security_group_list', - mock.Mock(return_value=[fake_secgroup1, - fake_secgroup2])) + fake_secgroup1 = {'id': 'fake_sg_id1', 'name': name} + fake_secgroup2 = {'id': 'fake_sg_id2', 'name': name} + neutron_api = self._manager.network_helper.neutron_api + neutron_api.security_group_list.return_value = { + 'security_groups': [fake_secgroup1, fake_secgroup2]} + self.assertRaises(exception.ServiceInstanceException, self._manager._get_or_create_security_group, self._manager.admin_context, name) - self._manager.compute_api.security_group_list.assert_called_once_with( - self._manager.admin_context) + + neutron_api.security_group_list.assert_called_once_with( + {"name": name}) @ddt.data( dict(), @@ -897,7 +891,7 @@ class ServiceInstanceManagerTestCase(test.TestCase): server_create = dict(id='fakeid', status='CREATING', networks=dict()) net_name = self._manager.get_config_option("service_network_name") - sg = type('FakeSG', (object, ), dict(id='fakeid', name='fakename')) + sg = {'id': 'fakeid', 'name': 'fakename'} ip_address = 'fake_ip_address' service_image_id = 'fake_service_image_id' key_data = 'fake_key_name', 'fake_key_path' @@ -972,7 +966,7 @@ class ServiceInstanceManagerTestCase(test.TestCase): self._manager.admin_context, server_create['id']) self._manager.compute_api.add_security_group_to_server.\ assert_called_once_with( - self._manager.admin_context, server_get['id'], sg.id) + self._manager.admin_context, server_get['id'], sg['id']) self._manager.network_helper.get_network_name.assert_has_calls([]) def test___create_service_instance_neutron_no_admin_ip(self): @@ -986,7 +980,7 @@ class ServiceInstanceManagerTestCase(test.TestCase): server_create = {'id': 'fakeid', 'status': 'CREATING', 'networks': {}} net_name = self._manager.get_config_option("service_network_name") - sg = type('FakeSG', (object, ), {'id': 'fakeid', 'name': 'fakename'}) + sg = {'id': 'fakeid', 'name': 'fakename'} ip_address = 'fake_ip_address' service_image_id = 'fake_service_image_id' key_data = 'fake_key_name', 'fake_key_path' @@ -1047,7 +1041,7 @@ class ServiceInstanceManagerTestCase(test.TestCase): self._manager.admin_context, server_create['id']) self._manager.compute_api.add_security_group_to_server.\ assert_called_once_with( - self._manager.admin_context, server_get['id'], sg.id) + self._manager.admin_context, server_get['id'], sg['id']) self._manager.network_helper.get_network_name.assert_has_calls([]) @ddt.data(