From: Aaron Rosen <aaronorosen@gmail.com>
Date: Tue, 8 Apr 2014 20:20:49 +0000 (-0700)
Subject: NSX: Fix KeyError in sync if nsx_router_id not found
X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=422e6e9a0eef379315f3404304e65583e8b65f92;p=openstack-build%2Fneutron-build.git

NSX: Fix KeyError in sync if nsx_router_id not found

Previously, a KeyError would occur in the sync code which would
cause the sync thread to stop running. This would occur if there
was a router entry in the database but no nsx_router_mapping and
the router was not found in nsx. Note: this should never happen unless
one did not run the db migration which introduced and migrated the data
for the NeutronNsxRouterMapping table.

(cherry picked from commit d90d71cfdae1d9c9c3b54cc33adfabce683c633c)
Change-Id: I44f3e7de9323f594501db63d3ad33e80e617bfdc
Closes-bug: #1304647
---

diff --git a/neutron/plugins/vmware/common/sync.py b/neutron/plugins/vmware/common/sync.py
index 1b9c4926b..d032f43fe 100644
--- a/neutron/plugins/vmware/common/sync.py
+++ b/neutron/plugins/vmware/common/sync.py
@@ -334,16 +334,16 @@ class NsxSynchronizer():
                 # This query will return the logical router status too
                 nsx_router_id = nsx_utils.get_nsx_router_id(
                     context.session, self._cluster, neutron_router_data['id'])
-                lrouter = routerlib.get_lrouter(
-                    self._cluster, nsx_router_id)
+                if nsx_router_id:
+                    lrouter = routerlib.get_lrouter(
+                        self._cluster, nsx_router_id)
             except exceptions.NotFound:
                 # NOTE(salv-orlando): We should be catching
                 # api_exc.ResourceNotFound here
                 # The logical router was not found
                 LOG.warning(_("Logical router for neutron router %s not "
                               "found on NSX."), neutron_router_data['id'])
-                lrouter = None
-            else:
+            if lrouter:
                 # Update the cache
                 self._nsx_cache.update_lrouter(lrouter)
 
diff --git a/neutron/tests/unit/vmware/test_nsx_sync.py b/neutron/tests/unit/vmware/test_nsx_sync.py
index b9de0a156..6fbb5d7da 100644
--- a/neutron/tests/unit/vmware/test_nsx_sync.py
+++ b/neutron/tests/unit/vmware/test_nsx_sync.py
@@ -30,6 +30,7 @@ from neutron.plugins.vmware.api_client import client
 from neutron.plugins.vmware.api_client import exception as api_exc
 from neutron.plugins.vmware.api_client import version
 from neutron.plugins.vmware.common import sync
+from neutron.plugins.vmware.dbexts import db
 from neutron.plugins.vmware import nsx_cluster as cluster
 from neutron.plugins.vmware import nsxlib
 from neutron.plugins.vmware import plugin
@@ -621,6 +622,34 @@ class SyncTestCase(base.BaseTestCase):
                     exp_status = constants.NET_STATUS_ACTIVE
                 self.assertEqual(exp_status, q_rtr['status'])
 
+    def test_synchronize_router_nsx_mapping_not_found(self):
+        ctx = context.get_admin_context()
+        with self._populate_data(ctx):
+            # Put a router down to verify synchronization
+            lr_uuid = self.fc._fake_lrouter_dict.keys()[0]
+            q_rtr_id = self._get_tag_dict(
+                self.fc._fake_lrouter_dict[lr_uuid]['tags'])['q_router_id']
+            self.fc._fake_lrouter_dict[lr_uuid]['status'] = 'false'
+            q_rtr_data = self._plugin._get_router(ctx, q_rtr_id)
+
+            # delete router mapping from db.
+            db.delete_neutron_nsx_router_mapping(ctx.session, q_rtr_id)
+            # pop router from fake nsx client
+            router_data = self.fc._fake_lrouter_dict.pop(lr_uuid)
+
+            self._plugin._synchronizer.synchronize_router(ctx, q_rtr_data)
+            # Reload from db
+            q_routers = self._plugin.get_routers(ctx)
+            for q_rtr in q_routers:
+                if q_rtr['id'] == q_rtr_id:
+                    exp_status = constants.NET_STATUS_ERROR
+                else:
+                    exp_status = constants.NET_STATUS_ACTIVE
+                self.assertEqual(exp_status, q_rtr['status'])
+            # put the router database since we don't handle missing
+            # router data in the fake nsx api_client
+            self.fc._fake_lrouter_dict[lr_uuid] = router_data
+
     def test_synchronize_router_on_get(self):
         cfg.CONF.set_override('always_read_status', True, 'NSX_SYNC')
         ctx = context.get_admin_context()