nova-net: Remove use of legacy 'FloatingIP' object

We want to get rid of the 'FloatingIP' object. Alas, that's easier said
than done because there are still a few users of this. The
'get_floating_ip', 'get_floating_ip_by_address', and
'get_floating_ips_by_project' methods are examples. These are only
called by the legacy 'os-floating-ips' API and the 'FloatingIP' object
is simply used as an unnecessary container between the two. Convert
things so the aforementioned API can handle mostly intact responses from
neutron instead, eliminating this user of the 'FloatingIP' object.

Change-Id: I77b1cfeab3c00c6c3d7743ba51e12414806b71d2
Signed-off-by: Stephen Finucane <[email protected]>

Author: stephenfin

nova/api/openstack/compute/floating_ip_pools.py

@@ -19,16 +19,16 @@
 from nova.policies import floating_ip_pools as fip_policies
 
 
-def _translate_floating_ip_view(pool_name):
+def _translate_floating_ip_view(pool):
     return {
-        'name': pool_name,
+        'name': pool['name'] or pool['id'],
     }
 
 
 def _translate_floating_ip_pools_view(pools):
     return {
-        'floating_ip_pools': [_translate_floating_ip_view(pool_name)
-                              for pool_name in pools]
+        'floating_ip_pools': [_translate_floating_ip_view(pool)
+                              for pool in pools]
     }
 
 

nova/api/openstack/compute/floating_ips.py

@@ -17,7 +17,6 @@
 
 from oslo_log import log as logging
 from oslo_utils import netutils
-from oslo_utils import uuidutils
 import webob
 
 from nova.api.openstack.api_version_request \
@@ -37,45 +36,21 @@
 
 
 def _translate_floating_ip_view(floating_ip):
-    result = {
-        'id': floating_ip['id'],
-        'ip': floating_ip['address'],
-        'pool': floating_ip['pool'],
+    instance_id = None
+    if floating_ip['port_details']:
+        instance_id = floating_ip['port_details']['device_id']
+
+    return {
+        'floating_ip': {
+            'id': floating_ip['id'],
+            'ip': floating_ip['floating_ip_address'],
+            'pool': floating_ip['network_details']['name'] or (
+                floating_ip['network_details']['id']),
+            'fixed_ip': floating_ip['fixed_ip_address'],
+            'instance_id': instance_id,
+        }
     }
 
-    # If fixed_ip is unset on floating_ip, then we can't get any of the next
-    # stuff, so we'll just short-circuit
-    if 'fixed_ip' not in floating_ip:
-        result['fixed_ip'] = None
-        result['instance_id'] = None
-        return {'floating_ip': result}
-
-    # TODO(rlrossit): These look like dicts, but they're actually versioned
-    # objects, so we need to do these contain checks because they will not be
-    # caught by the exceptions below (it raises NotImplementedError and
-    # OrphanedObjectError. This comment can probably be removed when
-    # the dict syntax goes away.
-    try:
-        if 'address' in floating_ip['fixed_ip']:
-            result['fixed_ip'] = floating_ip['fixed_ip']['address']
-        else:
-            result['fixed_ip'] = None
-    except (TypeError, KeyError, AttributeError):
-        result['fixed_ip'] = None
-    try:
-        if 'instance_uuid' in floating_ip['fixed_ip']:
-            result['instance_id'] = floating_ip['fixed_ip']['instance_uuid']
-        else:
-            result['instance_id'] = None
-    except (TypeError, KeyError, AttributeError):
-        result['instance_id'] = None
-    return {'floating_ip': result}
-
-
-def _translate_floating_ips_view(floating_ips):
-    return {'floating_ips': [_translate_floating_ip_view(ip)['floating_ip']
-                             for ip in floating_ips]}
-
 
 def get_instance_by_floating_ip_addr(self, context, address):
     try:
@@ -136,7 +111,8 @@ def index(self, req):
 
         floating_ips = self.network_api.get_floating_ips_by_project(context)
 
-        return _translate_floating_ips_view(floating_ips)
+        return {'floating_ips': [_translate_floating_ip_view(ip)['floating_ip']
+                                 for ip in floating_ips]}
 
     @wsgi.Controller.api_version("2.1", MAX_PROXY_API_SUPPORT_VERSION)
     @wsgi.expected_errors((400, 403, 404))
@@ -185,7 +161,7 @@ def delete(self, req, id):
         except exception.InvalidID as e:
             raise webob.exc.HTTPBadRequest(explanation=e.format_message())
 
-        address = floating_ip['address']
+        address = floating_ip['floating_ip_address']
 
         # get the associated instance object (if any)
         instance = get_instance_by_floating_ip_addr(self, context, address)
@@ -311,11 +287,7 @@ def _remove_floating_ip(self, req, id, body):
         instance = get_instance_by_floating_ip_addr(self, context, address)
 
         # disassociate if associated
-        if (instance and
-            floating_ip.get('fixed_ip_id') and
-            (uuidutils.is_uuid_like(id) and
-             [instance.uuid == id] or
-             [instance.id == id])[0]):
+        if instance and floating_ip['port_id'] and instance.uuid == id:
             try:
                 disassociate_floating_ip(self, context, instance, address)
             except exception.FloatingIpNotAssociated:

nova/network/neutron.py

@@ -2578,27 +2578,6 @@ def get_fixed_ip_by_address(self, context, address):
             raise exception.FixedIpAssociatedWithMultipleInstances(
                 address=address)
 
-    def _setup_net_dict(self, client, network_id):
-        if not network_id:
-            return {}
-        pool = client.show_network(network_id)['network']
-        return {pool['id']: pool}
-
-    def _setup_port_dict(self, context, client, port_id):
-        if not port_id:
-            return {}
-        port = self._show_port(context, port_id, neutron_client=client)
-        return {port['id']: port}
-
-    def _setup_pools_dict(self, client):
-        pools = self._get_floating_ip_pools(client)
-        return {i['id']: i for i in pools}
-
-    def _setup_ports_dict(self, client, project_id=None):
-        search_opts = {'tenant_id': project_id} if project_id else {}
-        ports = client.list_ports(**search_opts)['ports']
-        return {p['id']: p for p in ports}
-
     def get_floating_ip(self, context, id):
         """Return floating IP object given the floating IP id."""
         client = get_client(context)
@@ -2607,72 +2586,64 @@ def get_floating_ip(self, context, id):
         except neutron_client_exc.NeutronClientException as e:
             if e.status_code == 404:
                 raise exception.FloatingIpNotFound(id=id)
-            else:
-                with excutils.save_and_reraise_exception():
-                    LOG.exception('Unable to access floating IP %s', id)
-        pool_dict = self._setup_net_dict(client,
-                                         fip['floating_network_id'])
-        port_dict = self._setup_port_dict(context, client, fip['port_id'])
-        return self._make_floating_ip_obj(context, fip, pool_dict, port_dict)
-
-    def _get_floating_ip_pools(self, client, project_id=None):
-        search_opts = {constants.NET_EXTERNAL: True}
-        if project_id:
-            search_opts.update({'tenant_id': project_id})
-        data = client.list_networks(**search_opts)
-        return data['networks']
+
+            with excutils.save_and_reraise_exception():
+                LOG.exception('Unable to access floating IP %s', id)
+
+        # retrieve and cache the network details now since many callers need
+        # the network name which isn't present in the response from neutron
+        network_uuid = fip['floating_network_id']
+        try:
+            fip['network_details'] = client.show_network(
+                network_uuid)['network']
+        except neutron_client_exc.NetworkNotFoundClient:
+            raise exception.NetworkNotFound(network_id=network_uuid)
+
+        return fip
 
     def get_floating_ip_pools(self, context):
-        """Return floating IP pool names."""
+        """Return floating IP pools a.k.a. external networks."""
         client = get_client(context)
-        pools = self._get_floating_ip_pools(client)
-        return [n['name'] or n['id'] for n in pools]
-
-    def _make_floating_ip_obj(self, context, fip, pool_dict, port_dict):
-        pool = pool_dict[fip['floating_network_id']]
-        # NOTE(danms): Don't give these objects a context, since they're
-        # not lazy-loadable anyway
-        floating = objects.floating_ip.NeutronFloatingIP(
-            id=fip['id'], address=fip['floating_ip_address'],
-            pool=(pool['name'] or pool['id']), project_id=fip['tenant_id'],
-            fixed_ip_id=fip['port_id'])
-        # In Neutron v2 API fixed_ip_address and instance uuid
-        # (= device_id) are known here, so pass it as a result.
-        if fip['fixed_ip_address']:
-            floating.fixed_ip = objects.FixedIP(
-                address=fip['fixed_ip_address'])
-        else:
-            floating.fixed_ip = None
-        if fip['port_id']:
-            instance_uuid = port_dict[fip['port_id']]['device_id']
-            # NOTE(danms): This could be .refresh()d, so give it context
-            floating.instance = objects.Instance(context=context,
-                                                 uuid=instance_uuid)
-            if floating.fixed_ip:
-                floating.fixed_ip.instance_uuid = instance_uuid
-        else:
-            floating.instance = None
-        return floating
+        data = client.list_networks(**{constants.NET_EXTERNAL: True})
+        return data['networks']
 
     def get_floating_ip_by_address(self, context, address):
         """Return a floating IP given an address."""
         client = get_client(context)
         fip = self._get_floating_ip_by_address(client, address)
-        pool_dict = self._setup_net_dict(client,
-                                         fip['floating_network_id'])
-        port_dict = self._setup_port_dict(context, client, fip['port_id'])
-        return self._make_floating_ip_obj(context, fip, pool_dict, port_dict)
+
+        # retrieve and cache the network details now since many callers need
+        # the network name which isn't present in the response from neutron
+        network_uuid = fip['floating_network_id']
+        try:
+            fip['network_details'] = client.show_network(
+                network_uuid)['network']
+        except neutron_client_exc.NetworkNotFoundClient:
+            raise exception.NetworkNotFound(network_id=network_uuid)
+
+        return fip
 
     def get_floating_ips_by_project(self, context):
         client = get_client(context)
         project_id = context.project_id
         fips = self._safe_get_floating_ips(client, tenant_id=project_id)
-        if not fips:
-            return []
-        pool_dict = self._setup_pools_dict(client)
-        port_dict = self._setup_ports_dict(client, project_id)
-        return [self._make_floating_ip_obj(context, fip, pool_dict, port_dict)
-                for fip in fips]
+
+        # retrieve and cache the network details now since many callers need
+        # the network name which isn't present in the response from neutron
+        networks = {}
+        for fip in fips:
+            network_uuid = fip['floating_network_id']
+            if network_uuid not in networks:
+                try:
+                    network = client.show_network(network_uuid)['network']
+                except neutron_client_exc.NetworkNotFoundClient:
+                    raise exception.NetworkNotFound(network_id=network_uuid)
+
+                networks[network['id']] = network
+
+            fip['network_details'] = networks[network_uuid]
+
+        return fips
 
     def get_instance_id_by_floating_address(self, context, address):
         """Return the instance id a floating IP's fixed IP is allocated to."""
@@ -2787,7 +2758,7 @@ def release_floating_ip(self, context, address,
         self._release_floating_ip(context, address)
 
     def disassociate_and_release_floating_ip(self, context, instance,
-                                           floating_ip):
+                                             floating_ip):
         """Removes (deallocates) and deletes the floating IP.
 
         This api call was added to allow this to be done in one operation
@@ -2797,14 +2768,17 @@ def disassociate_and_release_floating_ip(self, context, instance,
         @refresh_cache
         def _release_floating_ip_and_refresh_cache(self, context, instance,
                                                    floating_ip):
-            self._release_floating_ip(context, floating_ip['address'],
-                                      raise_if_associated=False)
+            self._release_floating_ip(
+                context, floating_ip['floating_ip_address'],
+                raise_if_associated=False)
+
         if instance:
             _release_floating_ip_and_refresh_cache(self, context, instance,
                                                    floating_ip)
         else:
-            self._release_floating_ip(context, floating_ip['address'],
-                                      raise_if_associated=False)
+            self._release_floating_ip(
+                context, floating_ip['floating_ip_address'],
+                raise_if_associated=False)
 
     def _release_floating_ip(self, context, address,
                              raise_if_associated=True):

nova/tests/functional/api_sample_tests/test_floating_ip_pools.py

@@ -19,7 +19,10 @@ class FloatingIPPoolsSampleTests(api_sample_base.ApiSampleTestBaseV21):
     sample_dir = "os-floating-ip-pools"
 
     def test_list_floatingippools(self):
-        pool_list = ["pool1", "pool2"]
+        pool_list = [
+            {'name': 'pool1'},
+            {'name': 'pool2'},
+        ]
 
         def fake_get_floating_ip_pools(self, context):
             return pool_list
@@ -28,7 +31,7 @@ def fake_get_floating_ip_pools(self, context):
                       fake_get_floating_ip_pools)
         response = self._do_get('os-floating-ip-pools')
         subs = {
-            'pool1': pool_list[0],
-            'pool2': pool_list[1]
+            'pool1': pool_list[0]['name'],
+            'pool2': pool_list[1]['name'],
         }
         self._verify_response('floatingippools-list-resp', subs, response, 200)

nova/tests/functional/api_sample_tests/test_servers.py

@@ -785,7 +785,7 @@ def test_server_remove_floating_ip(self):
         self.stub_out(
             'nova.network.neutron.API.get_floating_ip_by_address',
             lambda *a, **k: {
-                'fixed_ip_id': 'a0c566f0-faab-406f-b77f-2b286dc6dd7e'})
+                'port_id': 'a0c566f0-faab-406f-b77f-2b286dc6dd7e'})
         self.stub_out(
             'nova.network.neutron.API.'
             'get_instance_id_by_floating_address',

nova/tests/unit/api/openstack/compute/test_floating_ip_pools.py

@@ -24,7 +24,10 @@
 
 
 def fake_get_floating_ip_pools(*args, **kwargs):
-    return ['nova', 'other']
+    return [
+        {'name': 'nova'},
+        {'name': 'other'},
+    ]
 
 
 class FloatingIpPoolTestV21(test.NoDBTestCase):
@@ -42,9 +45,9 @@ def test_translate_floating_ip_pools_view(self):
         view = self.floating_ip_pools._translate_floating_ip_pools_view(pools)
         self.assertIn('floating_ip_pools', view)
         self.assertEqual(view['floating_ip_pools'][0]['name'],
-                         pools[0])
+                         pools[0]['name'])
         self.assertEqual(view['floating_ip_pools'][1]['name'],
-                         pools[1])
+                         pools[1]['name'])
 
     def test_floating_ips_pools_list(self):
         with mock.patch.object(self.controller.network_api,
@@ -53,8 +56,10 @@ def test_floating_ips_pools_list(self):
             res_dict = self.controller.index(self.req)
 
         pools = fake_get_floating_ip_pools(None, self.context)
-        response = {'floating_ip_pools': [{'name': name} for name in pools]}
-        self.assertEqual(res_dict, response)
+        response = {
+            'floating_ip_pools': [{'name': pool['name']} for pool in pools],
+        }
+        self.assertEqual(response, res_dict)
 
 
 class FloatingIPPoolsPolicyEnforcementV21(test.NoDBTestCase):