[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

@@ -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))

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)

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)
+        s_groups = self.network_helper.neutron_api.security_group_list({
-                    if s.name == name]
+            "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 "\
+                description = ("This security group is intended "
-                              "to be used by share service."
+                               "to be used by share service.")
             LOG.debug("Creating security group with name '%s'.", name)
-            sg = self.compute_api.security_group_create(
+            sg = self.network_helper.neutron_api.security_group_create(
-                context, name, description)
+                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"]))

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):
 

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")
+        name = "fake_sg_name_from_config"
-        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"
         desc = "fake_sg_description"
-        fake_secgroup = fake_compute.FakeSecurityGroup(name=name,
+        fake_secgroup = {'id': 'fake_sg_id', 'name': name, 'description': desc}
-                                                       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',
+        neutron_api = self._manager.network_helper.neutron_api
-                         mock.Mock(return_value=[]))
+        neutron_api.security_group_list.return_value = {
-        self.mock_object(self._manager.compute_api, 'security_group_create',
+            'security_groups': [fake_secgroup]}
-                         mock.Mock(return_value=fake_secgroup))
+
-        self.mock_object(self._manager.compute_api,
+        result = self._manager._get_or_create_security_group(
-                         'security_group_rule_create')
+            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_secgroup1 = {'id': 'fake_sg_id1', 'name': name}
-        fake_secgroup2 = fake_compute.FakeSecurityGroup(name=name)
+        fake_secgroup2 = {'id': 'fake_sg_id2', 'name': name}
-        self.mock_object(self._manager.compute_api, 'security_group_list',
+        neutron_api = self._manager.network_helper.neutron_api
-                         mock.Mock(return_value=[fake_secgroup1,
+        neutron_api.security_group_list.return_value = {
-                                                 fake_secgroup2]))
+            '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(