Fix KeyError during sync_routers

Method sync_routers is used by the L3 agent to query
routers it knows about. Routers and GW ports lists
are populated in two different times, which means that
they can be interleaved by a delete request which
results in gateway ports being missing in one of the
two data structures.

This patch takes care of the race condition.

Closes-bug: #1355409

Change-Id: Id3a6fe145058f690e107bfe7023980ede61cff90

neutron/db/l3_db.py

@@ -883,7 +883,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
 
@@ -916,6 +917,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,