Merge "Eliminate extra queries used to retrieve gw_ports"

neutron/db/l3_db.py

@@ -1086,13 +1086,16 @@ class L3_NAT_dbonly_mixin(l3.RouterPluginBase):
         return router_ids
 
     def _build_routers_list(self, context, routers, gw_ports):
-        for router in routers:
-            gw_port_id = router['gw_port_id']
-            # Collect gw ports only if available
-            if gw_port_id and gw_ports.get(gw_port_id):
-                router['gw_port'] = gw_ports[gw_port_id]
+        """Subclasses can override this to add extra gateway info"""
         return routers
 
+    def _make_router_dict_with_gw_port(self, router, fields):
+        result = self._make_router_dict(router, fields)
+        if router.get('gw_port'):
+            result['gw_port'] = self._core_plugin._make_port_dict(
+                router['gw_port'], None)
+        return result
+
     def _get_sync_routers(self, context, router_ids=None, active=None):
         """Query routers and their gw ports for l3 agent.
 
@@ -1109,24 +1112,14 @@ class L3_NAT_dbonly_mixin(l3.RouterPluginBase):
         filters = {'id': router_ids} if router_ids else {}
         if active is not None:
             filters['admin_state_up'] = [active]
-        router_dicts = self.get_routers(context, filters=filters)
-        gw_port_ids = []
+        router_dicts = self._get_collection(
+            context, Router, self._make_router_dict_with_gw_port,
+            filters=filters)
         if not router_dicts:
             return []
-        for router_dict in router_dicts:
-            gw_port_id = router_dict['gw_port_id']
-            if gw_port_id:
-                gw_port_ids.append(gw_port_id)
-        gw_ports = []
-        if gw_port_ids:
-            gw_ports = dict((gw_port['id'], gw_port)
-                            for gw_port in
-                            self._get_sync_gw_ports(context, gw_port_ids))
-        # NOTE(armando-migliaccio): between get_routers and _get_sync_gw_ports
-        # gw ports may get deleted, which means that router_dicts may contain
-        # ports that gw_ports does not; we should rebuild router_dicts, but
-        # letting the callee check for missing gw_ports sounds like a good
-        # defensive approach regardless
+        gw_ports = dict((r['gw_port']['id'], r['gw_port'])
+                        for r in router_dicts
+                        if r.get('gw_port'))
         return self._build_routers_list(context, router_dicts, gw_ports)
 
     def _get_sync_floating_ips(self, context, router_ids):
@@ -1135,13 +1128,6 @@ class L3_NAT_dbonly_mixin(l3.RouterPluginBase):
             return []
         return self.get_floatingips(context, {'router_id': router_ids})
 
-    def _get_sync_gw_ports(self, context, gw_port_ids):
-        if not gw_port_ids:
-            return []
-        filters = {'id': gw_port_ids}
-        gw_ports = self._core_plugin.get_ports(context, filters)
-        return gw_ports
-
     def _get_sync_interfaces(self, context, router_ids, device_owners=None):
         """Query router interfaces that relate to list of router_ids."""
         device_owners = device_owners or [DEVICE_OWNER_ROUTER_INTF]

neutron/tests/unit/extensions/test_l3.py

@@ -313,19 +313,6 @@ class TestL3NatAgentSchedulingServicePlugin(TestL3NatServicePlugin,
             {l3_constants.AGENT_TYPE_L3: l3_rpc_agent_api.L3AgentNotifyAPI()})
 
 
-class L3NATdbonlyMixinTestCase(base.BaseTestCase):
-
-    def setUp(self):
-        super(L3NATdbonlyMixinTestCase, self).setUp()
-        self.mixin = l3_db.L3_NAT_dbonly_mixin()
-
-    def test_build_routers_list_with_gw_port_mismatch(self):
-        routers = [{'gw_port_id': 'foo_gw_port_id', 'id': 'foo_router_id'}]
-        gw_ports = {}
-        routers = self.mixin._build_routers_list(mock.ANY, routers, gw_ports)
-        self.assertIsNone(routers[0].get('gw_port'))
-
-
 class L3NatTestCaseMixin(object):
 
     def _create_router(self, fmt, tenant_id, name=None,