Fix for defect: Fail to clear default route

Issue: The cfg agent fails when clearing default route for the tenant
routers when the external gateway is cleared.

This happens only when the tenant router is the last tenant router
with an external gateway. Thus when it is cleared the global router
(owning the external interface) is also removed.  But currently global
routers are deleted before the tenant router is processed. As part of
the global router delete, the external subinterface is also removed.
But default route for the tenant router was based on this and when the
cfg agent comes to this part, it fails.

We have now fixed the order of this processing, so that removed routers
are processed after updated routers are processed. Also global routers
are processed last in either way.

Closes-bug: #1551781
Change-Id: If76edd8ff774c9d59da1098c0f607b2ad426fa59

networking_cisco/plugins/cisco/cfg_agent/service_helpers/routing_svc_helper.py

@@ -414,6 +414,8 @@ class RoutingServiceHelper(object):
         #ToDo(Hareesh): Simplify if possible
         for r in routers:
             if r[ROUTER_ROLE_ATTR] == c_constants.ROUTER_ROLE_GLOBAL:
+                LOG.debug("Global router:%s found. Moved to the end of list "
+                          "for processing", r['id'])
                 routers.remove(r)
                 routers.append(r)
 
@@ -449,26 +451,27 @@ class RoutingServiceHelper(object):
                 prev_router_ids = set(self.router_info) & set(
                     [router['id'] for router in routers])
             cur_router_ids = set()
-            deleted_id_list = []
+            deleted_routerids_list = []
 
             for r in routers:
                 if not r['admin_state_up']:
                         continue
                 cur_router_ids.add(r['id'])
 
-            # identify and remove routers that no longer exist
+            # identify list of routers(ids) that no longer exist
             for router_id in prev_router_ids - cur_router_ids:
-                self._router_removed(router_id)
-                deleted_id_list.append(router_id)
+                deleted_routerids_list.append(router_id)
             if removed_routers:
                 self._adjust_router_list_for_global_router(removed_routers)
                 for router in removed_routers:
-                    self._router_removed(router['id'])
-                    deleted_id_list.append(router['id'])
+                    deleted_routerids_list.append(router['id'])
 
             self._adjust_router_list_for_global_router(routers)
+            # First process create/updated routers
             for r in routers:
-                if r['id'] in deleted_id_list:
+                LOG.debug("Processing router[id:%(id)s, role:%(role)s]",
+                          {'id': r['id'], 'role': r[ROUTER_ROLE_ATTR]})
+                if r['id'] in deleted_routerids_list:
                     continue
                 try:
                     if not r['admin_state_up']:
@@ -493,6 +496,12 @@ class RoutingServiceHelper(object):
                                     "Error is %(e)s"), {'id': r['id'], 'e': e})
                     self.updated_routers.update([r['id']])
                     continue
+                LOG.debug("Done processing router[id:%(id)s, role:%(role)s]",
+                          {'id': r['id'], 'role': r[ROUTER_ROLE_ATTR]})
+            # Finally process removed routers
+            for router_id in deleted_routerids_list:
+                LOG.debug("Processing deleted router:%s", router_id)
+                self._router_removed(router_id)
         except Exception:
             LOG.exception(_LE("Exception in processing routers on device:%s"),
                           device_id)

networking_cisco/tests/unit/cisco/cfg_agent/test_routing_svc_helper.py

@@ -27,6 +27,8 @@ from networking_cisco.plugins.cisco.cfg_agent import cfg_agent
 from networking_cisco.plugins.cisco.cfg_agent import cfg_exceptions
 from networking_cisco.plugins.cisco.cfg_agent.service_helpers import (
     routing_svc_helper)
+from networking_cisco.plugins.cisco.common import (cisco_constants as
+                                                   c_constants)
 from networking_cisco.plugins.cisco.extensions import routerrole
 
 
@@ -34,6 +36,8 @@ _uuid = uuidutils.generate_uuid
 HOST = 'myhost'
 FAKE_ID = _uuid()
 
+ROUTER_ROLE_ATTR = routerrole.ROUTER_ROLE_ATTR
+
 
 def prepare_router_data(enable_snat=None, num_internal_ports=1):
     router_id = _uuid()
@@ -635,3 +639,31 @@ class TestBasicRoutingOperations(base.BaseTestCase):
 
     def test_process_routers_floatingips_remap(self):
         self._process_routers_floatingips(action="remap")
+
+    def test_process_routers_rearrange_for_global(self):
+        router1, port1 = prepare_router_data()
+
+        router2 = {'id': _uuid(),
+                   ROUTER_ROLE_ATTR: None}
+        router_G = {'id': _uuid(),
+                   ROUTER_ROLE_ATTR: c_constants.ROUTER_ROLE_GLOBAL}
+        removed_routers = [router_G, router2]
+
+        self.routing_helper._adjust_router_list_for_global_router(
+                removed_routers)
+        #We check if the routers where rearranged
+        self.assertEqual(router2['id'], removed_routers[0]['id'])
+
+        removed_routers = [router_G, router2]
+        driver = self._mock_driver_and_hosting_device(self.routing_helper)
+        self.routing_helper._process_router = mock.Mock()
+        self.routing_helper._router_removed = mock.Mock()
+        self.routing_helper._process_routers(
+                [router1], removed_routers=removed_routers, device_id=None)
+        ri1 = self.routing_helper.router_info[router1['id']]
+        driver.router_added.assert_called_with(ri1)
+        # This check ensures that the tenant router was deleted first, followed
+        # by the global router
+        self.routing_helper._router_removed.assert_called_with(router_G['id'])
+        self.routing_helper._router_removed.assert_any_call(router2['id'])
+        self.routing_helper._process_router.assert_called_with(ri1)