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

neutron/plugins/nicira/NeutronServicePlugin.py

@@ -25,6 +25,7 @@
 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 @@ def _create_lrouter(self, context, router, nexthop):
         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 @@ def _delete_lrouter(self, context, id):
             }
             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 @@ def edge_deploy_result(self, task):
         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)

neutron/plugins/nicira/vshield/edge_appliance_driver.py

@@ -101,7 +101,8 @@ def _assemble_edge_vnic(self, name, index, portgroup_id,
             }
             if secondary:
                 address_group['secondaryAddresses'] = {
-                    'ipAddress': secondary
+                    'ipAddress': secondary,
+                    'type': 'IpAddressesDto'
                 }
 
             vnic['addressGroups'] = {

neutron/tests/unit/nicira/test_edge_router.py

@@ -166,7 +166,7 @@ def test_router_create(self):
                           ('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 @@ def test_router_create(self):
             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()