Merge "l3_agent: make process_router more robust"

neutron/agent/l3_agent.py

@@ -391,13 +391,13 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback, manager.Manager):
                      p['id'] not in current_port_ids]
         for p in new_ports:
             self._set_subnet_info(p)
-            ri.internal_ports.append(p)
             self.internal_network_added(ri, p['network_id'], p['id'],
                                         p['ip_cidr'], p['mac_address'])
+            ri.internal_ports.append(p)
 
         for p in old_ports:
-            ri.internal_ports.remove(p)
             self.internal_network_removed(ri, p['id'], p['ip_cidr'])
+            ri.internal_ports.remove(p)
 
         internal_cidrs = [p['ip_cidr'] for p in ri.internal_ports]
         # TODO(salv-orlando): RouterInfo would be a better place for

neutron/tests/unit/test_l3_agent.py

@@ -582,6 +582,61 @@ class TestBasicRouterOperations(base.BaseTestCase):
         self._verify_snat_rules(nat_rules_delta, router, negate=True)
         self.send_arp.assert_called_once()
 
+    def test_process_router_internal_network_added_unexpected_error(self):
+        agent = l3_agent.L3NATAgent(HOSTNAME, self.conf)
+        router = self._prepare_router_data()
+        ri = l3_agent.RouterInfo(router['id'], self.conf.root_helper,
+                                 self.conf.use_namespaces, router=router)
+        with mock.patch.object(
+                l3_agent.L3NATAgent,
+                'internal_network_added') as internal_network_added:
+            # raise RuntimeError to simulate that an unexpected exception
+            # occurrs
+            internal_network_added.side_effect = RuntimeError
+            self.assertRaises(RuntimeError, agent.process_router, ri)
+            self.assertNotIn(
+                router[l3_constants.INTERFACE_KEY][0], ri.internal_ports)
+
+            # The unexpected exception has been fixed manually
+            internal_network_added.side_effect = None
+
+            # _sync_routers_task finds out that _rpc_loop failed to process the
+            # router last time, it will retry in the next run.
+            agent.process_router(ri)
+            # We were able to add the port to ri.internal_ports
+            self.assertIn(
+                router[l3_constants.INTERFACE_KEY][0], ri.internal_ports)
+
+    def test_process_router_internal_network_removed_unexpected_error(self):
+        agent = l3_agent.L3NATAgent(HOSTNAME, self.conf)
+        router = self._prepare_router_data()
+        ri = l3_agent.RouterInfo(router['id'], self.conf.root_helper,
+                                 self.conf.use_namespaces, router=router)
+        # add an internal port
+        agent.process_router(ri)
+
+        with mock.patch.object(
+                l3_agent.L3NATAgent,
+                'internal_network_removed') as internal_net_removed:
+            # raise RuntimeError to simulate that an unexpected exception
+            # occurrs
+            internal_net_removed.side_effect = RuntimeError
+            ri.internal_ports[0]['admin_state_up'] = False
+            # The above port is set to down state, remove it.
+            self.assertRaises(RuntimeError, agent.process_router, ri)
+            self.assertIn(
+                router[l3_constants.INTERFACE_KEY][0], ri.internal_ports)
+
+            # The unexpected exception has been fixed manually
+            internal_net_removed.side_effect = None
+
+            # _sync_routers_task finds out that _rpc_loop failed to process the
+            # router last time, it will retry in the next run.
+            agent.process_router(ri)
+            # We were able to remove the port from ri.internal_ports
+            self.assertNotIn(
+                router[l3_constants.INTERFACE_KEY][0], ri.internal_ports)
+
     def test_handle_router_snat_rules_add_back_jump(self):
         agent = l3_agent.L3NATAgent(HOSTNAME, self.conf)
         ri = mock.MagicMock()