Fix for status always in PENDING_CREATE for Edge service router

The root cause is when deployment finished, we only update router status to
active if the status is in pending create. The problem happens when the
background sync thread update router status to active, so the status update
for vcns_router_binding table is skipped. We fixed this by seperating
checking and updating status for router and binding table.

Also fixed an issue where Edge is not deleted if neutron service is
restarted. The root cause is when neutron service restarts, the cache for
router type is empty. And because we delete the router from db before
we delete Edge, we're not able to locate the router from db to determine
the router type. The fix is to use binding table to determine the router
type.

Also piggyback a missing attribute for updating Edge interface. It must have
been removed by accident when resolving conflict during service plugin merge.

Closes-Bug: #1226229
Change-Id: I3d0639d245e71ea2a3faba70fef1a0ebb87e19fd

diff --git a/neutron/plugins/nicira/NeutronServicePlugin.py b/neutron/plugins/nicira/NeutronServicePlugin.py
index 576e047303fbc3d3f251355fcd72bf0d818a26a7..9e030469bf7875af4c0541d9b1ed6a4964543c64 100644 (file)
--- a/neutron/plugins/nicira/NeutronServicePlugin.py
+++ b/neutron/plugins/nicira/NeutronServicePlugin.py
@@ -25,6 +25,7 @@ from neutron.db import l3_db
 from neutron.db.loadbalancer import loadbalancer_db
 from neutron.db import routedserviceinsertion_db as rsi_db
 from neutron.extensions import firewall as fw_ext
+from neutron.extensions import l3
 from neutron.openstack.common import excutils
 from neutron.openstack.common import log as logging
 from neutron.plugins.common import constants as service_constants
@@ -473,14 +474,10 @@ class NvpAdvancedPlugin(sr_db.ServiceRouter_mixin,
         return lrouter
 
     def _delete_lrouter(self, context, id):
-        if not self._is_advanced_service_router(context, id):
-            super(NvpAdvancedPlugin, self)._delete_lrouter(context, id)
-            if id in self._router_type:
-                del self._router_type[id]
-            return
-
         binding = vcns_db.get_vcns_router_binding(context.session, id)
-        if binding:
+        if not binding:
+            super(NvpAdvancedPlugin, self)._delete_lrouter(context, id)
+        else:
             vcns_db.update_vcns_router_binding(
                 context.session, id, status=service_constants.PENDING_DELETE)
 
@@ -499,8 +496,9 @@ class NvpAdvancedPlugin(sr_db.ServiceRouter_mixin,
             }
             self.vcns_driver.delete_edge(id, edge_id, jobdata=jobdata)
 
-        # delete LR
-        nvplib.delete_lrouter(self.cluster, id)
+            # delete LR
+            nvplib.delete_lrouter(self.cluster, id)
+
         if id in self._router_type:
             del self._router_type[id]
 
@@ -1528,24 +1526,33 @@ class VcnsCallbacks(object):
         lrouter = jobdata['lrouter']
         context = jobdata['context']
         name = task.userdata['router_name']
-        router_db = self.plugin._get_router(context, lrouter['uuid'])
+        router_db = None
+        try:
+            router_db = self.plugin._get_router(context, lrouter['uuid'])
+        except l3.RouterNotFound:
+            # Router might have been deleted before deploy finished
+            LOG.exception(_("Router %s not found"), lrouter['uuid'])
+
         if task.status == TaskStatus.COMPLETED:
             LOG.debug(_("Successfully deployed %(edge_id)s for "
                         "router %(name)s"), {
                             'edge_id': task.userdata['edge_id'],
                             'name': name})
-            if router_db['status'] == service_constants.PENDING_CREATE:
+            if (router_db and
+                    router_db['status'] == service_constants.PENDING_CREATE):
                 router_db['status'] = service_constants.ACTIVE
-                binding = vcns_db.get_vcns_router_binding(
-                    context.session, lrouter['uuid'])
-                # only update status to active if its status is pending create
-                if binding['status'] == service_constants.PENDING_CREATE:
-                    vcns_db.update_vcns_router_binding(
-                        context.session, lrouter['uuid'],
-                        status=service_constants.ACTIVE)
+
+            binding = vcns_db.get_vcns_router_binding(
+                context.session, lrouter['uuid'])
+            # only update status to active if its status is pending create
+            if binding['status'] == service_constants.PENDING_CREATE:
+                vcns_db.update_vcns_router_binding(
+                    context.session, lrouter['uuid'],
+                    status=service_constants.ACTIVE)
         else:
             LOG.debug(_("Failed to deploy Edge for router %s"), name)
-            router_db['status'] = service_constants.ERROR
+            if router_db:
+                router_db['status'] = service_constants.ERROR
             vcns_db.update_vcns_router_binding(
                 context.session, lrouter['uuid'],
                 status=service_constants.ERROR)

diff --git a/neutron/plugins/nicira/vshield/edge_appliance_driver.py b/neutron/plugins/nicira/vshield/edge_appliance_driver.py
index 9d15dc16daa11b2fc43ec0e04f2838fffde9001f..8072249180a38f34bca79bb8baa45f134932c089 100644 (file)
--- a/neutron/plugins/nicira/vshield/edge_appliance_driver.py
+++ b/neutron/plugins/nicira/vshield/edge_appliance_driver.py
@@ -101,7 +101,8 @@ class EdgeApplianceDriver(object):
             }
             if secondary:
                 address_group['secondaryAddresses'] = {
-                    'ipAddress': secondary
+                    'ipAddress': secondary,
+                    'type': 'IpAddressesDto'
                 }
 
             vnic['addressGroups'] = {

diff --git a/neutron/tests/unit/nicira/test_edge_router.py b/neutron/tests/unit/nicira/test_edge_router.py
index ad772d77ab35369ef4b4fce5a299da3ab9793333..60f690d8d0f994939767b4bf1697e8cef85f8f0d 100644 (file)
--- a/neutron/tests/unit/nicira/test_edge_router.py
+++ b/neutron/tests/unit/nicira/test_edge_router.py
@@ -166,7 +166,7 @@ class ServiceRouterTestCase(ServiceRouterTest, NvpRouterTestCase):
                           ('admin_state_up', True),
                           ('external_gateway_info', None),
                           ('service_router', True)]
-        with self.router(name='router1', admin_state_up=True,
+        with self.router(name=name, admin_state_up=True,
                          tenant_id=tenant_id) as router:
             expected_value_1 = expected_value + [('status', 'PENDING_CREATE')]
             for k, v in expected_value_1:
@@ -196,6 +196,21 @@ class ServiceRouterTestCase(ServiceRouterTest, NvpRouterTestCase):
             if lswitch['display_name'] == lswitch_name:
                 self.fail("Integration switch is not deleted")
 
+    def test_router_delete_after_plugin_restart(self):
+        name = 'router1'
+        tenant_id = _uuid()
+        with self.router(name=name, admin_state_up=True,
+                         tenant_id=tenant_id):
+            # clear router type cache to mimic plugin restart
+            plugin = NeutronManager.get_plugin()
+            plugin._router_type = {}
+
+        # check an integration lswitch is deleted
+        lswitch_name = "%s-ls" % name
+        for lswitch_id, lswitch in self.fc2._lswitches.iteritems():
+            if lswitch['display_name'] == lswitch_name:
+                self.fail("Integration switch is not deleted")
+
     def test_router_show(self):
         name = 'router1'
         tenant_id = _uuid()