Merge "Fix KeyError during sync_routers"

neutron/db/l3_db.py

@@ -871,7 +871,8 @@ class L3_NAT_dbonly_mixin(l3.RouterPluginBase):
     def _build_routers_list(self, context, routers, gw_ports):
         for router in routers:
             gw_port_id = router['gw_port_id']
-            if 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]
         return routers
 
@@ -904,6 +905,11 @@ class L3_NAT_dbonly_mixin(l3.RouterPluginBase):
             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
         return self._build_routers_list(context, router_dicts, gw_ports)
 
     def _get_sync_floating_ips(self, context, router_ids):

neutron/db/l3_dvr_db.py

@@ -237,7 +237,8 @@ class L3_NAT_with_dvr_db_mixin(l3_db.L3_NAT_db_mixin,
 
         for rtr in routers:
             gw_port_id = rtr['gw_port_id']
-            if gw_port_id:
+            # Collect gw ports only if available
+            if gw_port_id and gw_ports.get(gw_port_id):
                 rtr['gw_port'] = gw_ports[gw_port_id]
                 if 'enable_snat' in rtr[l3.EXTERNAL_GW_INFO]:
                     rtr['enable_snat'] = (

neutron/db/l3_gwmode_db.py

@@ -65,7 +65,8 @@ class L3_NAT_dbonly_mixin(l3_db.L3_NAT_dbonly_mixin):
     def _build_routers_list(self, context, routers, gw_ports):
         for rtr in routers:
             gw_port_id = rtr['gw_port_id']
-            if gw_port_id:
+            # Collect gw ports only if available
+            if gw_port_id and gw_ports.get(gw_port_id):
                 rtr['gw_port'] = gw_ports[gw_port_id]
                 # Add enable_snat key
                 rtr['enable_snat'] = rtr[EXTERNAL_GW_INFO]['enable_snat']

neutron/tests/unit/db/test_l3_dvr_db.py

@@ -167,3 +167,9 @@ class L3DvrTestCase(base.BaseTestCase):
             self.mixin._create_gw_port(
                 self.ctx, router_id, router_db, mock.ANY)
             self.assertFalse(cs.call_count)
+
+    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(self.ctx, routers, gw_ports)
+        self.assertIsNone(routers[0].get('gw_port'))

neutron/tests/unit/test_extension_ext_gw_mode.py

@@ -288,6 +288,15 @@ class TestL3GwModeMixin(base.BaseTestCase):
         self.assertEqual(FAKE_GW_PORT_ID, router['gw_port']['id'])
         self.assertFalse(router.get('enable_snat'))
 
+    def test_build_routers_list_with_gw_port_mismatch(self):
+        router_dict = self.target_object._make_router_dict(self.router)
+        routers = self.target_object._build_routers_list(
+            self.context, [router_dict], {})
+        self.assertEqual(1, len(routers))
+        router = routers[0]
+        self.assertIsNone(router.get('gw_port'))
+        self.assertIsNone(router.get('enable_snat'))
+
 
 class ExtGwModeIntTestCase(test_db_plugin.NeutronDbPluginV2TestCase,
                            test_l3_plugin.L3NatTestCaseMixin):

neutron/tests/unit/test_l3_plugin.py

@@ -313,6 +313,19 @@ class TestL3NatAgentSchedulingServicePlugin(TestL3NatServicePlugin,
     supported_extension_aliases = ["router", "l3_agent_scheduler"]
 
 
+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,