enable router deletion logic in l3-agent

bug 1039387

related to bp quantum-l3-fwd-nat

previous diff logic did not attempt to detect that a router was deleted.

Also, enabled actual deletion of namespace, since 066e48be08f155c29887f5846902fbf5e41b8c89 fixed the issue that was causing that to break.

Also, make router loop pay attention to admin_state_up on router ports.

Change-Id: Id76736dd3207472dfc282097e5b27740e05aaa3a

diff --git a/quantum/agent/l3_agent.py b/quantum/agent/l3_agent.py
index 098db3b3e09cc39aa6ba6b9ef7e5a7a669f9c9e5..c3f81c0b0fe6873313d5008f8b0afa430c76b9e3 100644 (file)
--- a/quantum/agent/l3_agent.py
+++ b/quantum/agent/l3_agent.py
@@ -89,6 +89,7 @@ class L3NATAgent(object):
 
     def __init__(self, conf):
         self.conf = conf
+        self.router_info = {}
 
         if not conf.interface_driver:
             LOG.error(_('You must specify an interface driver'))
@@ -120,36 +121,35 @@ class L3NATAgent(object):
         linux_utils.execute(['sysctl', '-w', 'net.ipv4.ip_forward=0'],
                             self.conf.root_helper, check_exit_code=False)
 
-        self._destroy_router_namespaces()
+        self._destroy_all_router_namespaces()
 
         # enable forwarding
         linux_utils.execute(['sysctl', '-w', 'net.ipv4.ip_forward=1'],
                             self.conf.root_helper, check_exit_code=False)
 
-    def _destroy_router_namespaces(self):
+    def _destroy_all_router_namespaces(self):
         """Destroy all router namespaces on the host to eliminate
         all stale linux devices, iptables rules, and namespaces.
         """
         root_ip = ip_lib.IPWrapper(self.conf.root_helper)
         for ns in root_ip.get_namespaces(self.conf.root_helper):
             if ns.startswith(NS_PREFIX):
-                ns_ip = ip_lib.IPWrapper(self.conf.root_helper,
-                                         namespace=ns)
-                for d in ns_ip.get_devices():
-                    if d.name.startswith(INTERNAL_DEV_PREFIX):
-                        # device is on default bridge
-                        self.driver.unplug(d.name)
-                    elif d.name.startswith(EXTERNAL_DEV_PREFIX):
-                        self.driver.unplug(d.name,
-                                           bridge=
-                                           self.conf.external_network_bridge)
-
-                # FIXME(danwent): disabling actual deletion of namespace
-                # until we figure out why it fails.  Having deleted all
-                # devices, the only harm here should be the clutter of
-                # the namespace lying around.
-
-                # ns_ip.netns.delete(ns)
+                try:
+                    self._destroy_router_namespace(ns)
+                except:
+                    LOG.exception("couldn't delete namespace '%s'" % ns)
+
+    def _destroy_router_namespace(self, namespace):
+        ns_ip = ip_lib.IPWrapper(self.conf.root_helper,
+                                 namespace=namespace)
+        for d in ns_ip.get_devices():
+            if d.name.startswith(INTERNAL_DEV_PREFIX):
+                # device is on default bridge
+                self.driver.unplug(d.name)
+            elif d.name.startswith(EXTERNAL_DEV_PREFIX):
+                self.driver.unplug(d.name,
+                                   bridge=self.conf.external_network_bridge)
+        ns_ip.netns.delete(namespace)
 
     def daemon_loop(self):
 
@@ -159,22 +159,34 @@ class L3NATAgent(object):
         # update notifications.
         # Likewise, it does not handle removing routers
 
-        self.router_info = {}
         while True:
             try:
-                #TODO(danwent): provide way to limit this to a single
-                # router, for model where agent runs in dedicated VM
-                for r in self.qclient.list_routers()['routers']:
-                    if r['id'] not in self.router_info:
-                        self.router_info[r['id']] = (RouterInfo(r['id'],
-                                                     self.conf.root_helper))
-                    ri = self.router_info[r['id']]
-                    self.process_router(ri)
+                self.do_single_loop()
             except:
                 LOG.exception("Error running l3_nat daemon_loop")
 
             time.sleep(self.polling_interval)
 
+    def do_single_loop(self):
+        prev_router_ids = set(self.router_info)
+        cur_router_ids = set()
+
+        # identify and update new or modified routers
+        for r in self.qclient.list_routers()['routers']:
+            cur_router_ids.add(r['id'])
+            if r['id'] not in self.router_info:
+                self.router_info[r['id']] = RouterInfo(r['id'],
+                                                       self.conf.root_helper)
+            ri = self.router_info[r['id']]
+            self.process_router(ri)
+
+        # identify and remove routers that no longer exist
+        for router_id in prev_router_ids - cur_router_ids:
+            ri = self.router_info[router_id]
+            del self.router_info[router_id]
+            self._destroy_router_namespace(ri.ns_name())
+        prev_router_ids = cur_router_ids
+
     def _set_subnet_info(self, port):
         ips = port['fixed_ips']
         if not ips:
@@ -195,21 +207,24 @@ class L3NATAgent(object):
             device_owner=l3_db.DEVICE_OWNER_ROUTER_INTF)['ports']
 
         existing_port_ids = set([p['id'] for p in ri.internal_ports])
-        current_port_ids = set([p['id'] for p in internal_ports])
-
-        for p in internal_ports:
-            if p['id'] not in existing_port_ids:
-                self._set_subnet_info(p)
-                ri.internal_ports.append(p)
-                self.internal_network_added(ri, ex_gw_port, p['id'],
-                                            p['ip_cidr'], p['mac_address'])
-
-        port_ids_to_remove = existing_port_ids - current_port_ids
-        for p in ri.internal_ports:
-            if p['id'] in port_ids_to_remove:
-                ri.internal_ports.remove(p)
-                self.internal_network_removed(ri, ex_gw_port, p['id'],
-                                              p['ip_cidr'])
+        current_port_ids = set([p['id'] for p in internal_ports
+                                if p['admin_state_up']])
+        new_ports = [p for p in internal_ports if
+                     p['id'] in current_port_ids and
+                     p['id'] not in existing_port_ids]
+        old_ports = [p for p in ri.internal_ports if
+                     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, ex_gw_port, p['id'],
+                                        p['ip_cidr'], p['mac_address'])
+
+        for p in old_ports:
+            ri.internal_ports.remove(p)
+            self.internal_network_removed(ri, ex_gw_port, p['id'],
+                                          p['ip_cidr'])
 
         internal_cidrs = [p['ip_cidr'] for p in ri.internal_ports]
 

diff --git a/quantum/tests/unit/test_l3_agent.py b/quantum/tests/unit/test_l3_agent.py
index 57efc2faf3dde6e50fdfd0d51cbd99d83c67bb8b..3e559ea4bdf39f6ad3f00e2164f0a66245900cf3 100644 (file)
--- a/quantum/tests/unit/test_l3_agent.py
+++ b/quantum/tests/unit/test_l3_agent.py
@@ -189,6 +189,7 @@ class TestBasicRouterOperations(unittest.TestCase):
                       'fixed_ips': [{'ip_address': '19.4.4.4',
                                      'subnet_id': _uuid()}]}
         internal_port = {'id': _uuid(),
+                         'admin_state_up': True,
                          'fixed_ips': [{'ip_address': '35.4.4.4',
                                         'subnet_id': _uuid()}],
                          'mac_address': 'ca:fe:de:ad:be:ef'}
@@ -221,6 +222,21 @@ class TestBasicRouterOperations(unittest.TestCase):
         self.client_inst.list_ports.return_value = {'ports': []}
         agent.process_router(ri)
 
+    def testSingleLoopRouterRemoval(self):
+        agent = l3_agent.L3NATAgent(self.conf)
+
+        self.client_inst.list_ports.return_value = {'ports': []}
+
+        self.client_inst.list_routers.return_value = {'routers': [
+            {'id': _uuid()}]}
+        agent.do_single_loop()
+
+        self.client_inst.list_routers.return_value = {'routers': []}
+        agent.do_single_loop()
+
+        # verify that remove is called
+        self.assertEquals(self.mock_ip.get_devices.call_count, 1)
+
     def testDaemonLoop(self):
 
         # just take a pass through the loop, then raise on time.sleep()
@@ -231,8 +247,6 @@ class TestBasicRouterOperations(unittest.TestCase):
             pass
 
         time_sleep.side_effect = ExpectedException()
-        self.client_inst.list_routers.return_value = {'routers':
-                                                      [{'id': _uuid()}]}
 
         agent = l3_agent.L3NATAgent(self.conf)
         self.assertRaises(ExpectedException, agent.daemon_loop)
@@ -250,7 +264,7 @@ class TestBasicRouterOperations(unittest.TestCase):
                                                  FakeDev('qgw-aaaa')]
 
         agent = l3_agent.L3NATAgent(self.conf)
-        agent._destroy_router_namespaces()
+        agent._destroy_all_router_namespaces()
 
     def testMain(self):
         agent_mock_p = mock.patch('quantum.agent.l3_agent.L3NATAgent')