always push down metadata rules for router, not just if gateway exists

bug 1051842

the l3-agent only pushes down some of the DNAT rules for metadata mapping
if there is a gateway set on a router. However, the nova-api server could
itself be running in the local router namespace (or on the same box as
the quantum-l3-agent in the case were namespaces are disabled. In fact,
with namespaces disabled, this is the likely setup for a simple
environment). Thus, we should simply always push down the metadata nat
rules for a router, router than waiting until a gateway is added.

This patch also refactors the actions taken by the l3-agent when a router
is added or removed into a separate function for improved readability,
as add/removing the nat rules made these code segments even longer.

Change-Id: I3c6eb35b51df3babf747dbcff7f943b850e69838

diff --git a/quantum/agent/l3_agent.py b/quantum/agent/l3_agent.py
index 9714ec2e5e0c1dbb0072cc708fdc835a4fac5a53..5527ea12c67e539d855b1a50afcd1c71d5dcffce 100644 (file)
--- a/quantum/agent/l3_agent.py
+++ b/quantum/agent/l3_agent.py
@@ -219,21 +219,38 @@ class L3NATAgent(object):
             else:
                 continue
             if r['id'] not in self.router_info:
-                self.router_info[r['id']] = RouterInfo(
-                    r['id'], self.conf.root_helper, self.conf.use_namespaces)
-                if self.conf.use_namespaces:
-                    self._create_router_namespace(self.router_info[r['id']])
+                self._router_added(r['id'])
 
             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())
+            self._router_removed(router_id)
         prev_router_ids = cur_router_ids
 
+    def _router_added(self, router_id):
+        ri = RouterInfo(router_id, self.conf.root_helper,
+                        self.conf.use_namespaces)
+        self.router_info[router_id] = ri
+        if self.conf.use_namespaces:
+            self._create_router_namespace(ri)
+        for c, r in self.metadata_filter_rules():
+            ri.iptables_manager.ipv4['filter'].add_rule(c, r)
+        for c, r in self.metadata_nat_rules():
+            ri.iptables_manager.ipv4['nat'].add_rule(c, r)
+        ri.iptables_manager.apply()
+
+    def _router_removed(self, router_id):
+        ri = self.router_info[router_id]
+        for c, r in self.metadata_filter_rules():
+            ri.iptables_manager.ipv4['filter'].remove_rule(c, r)
+        for c, r in self.metadata_nat_rules():
+            ri.iptables_manager.ipv4['nat'].remove_rule(c, r)
+        ri.iptables_manager.apply()
+        del self.router_info[router_id]
+        self._destroy_router_namespace(ri.ns_name())
+
     def _set_subnet_info(self, port):
         ips = port['fixed_ips']
         if not ips:
@@ -372,8 +389,6 @@ class L3NATAgent(object):
                 utils.execute(cmd, check_exit_code=False,
                               root_helper=self.conf.root_helper)
 
-        for (c, r) in self.external_gateway_filter_rules():
-            ri.iptables_manager.ipv4['filter'].add_rule(c, r)
         for (c, r) in self.external_gateway_nat_rules(ex_gw_ip,
                                                       internal_cidrs,
                                                       interface_name):
@@ -392,14 +407,12 @@ class L3NATAgent(object):
                                prefix=EXTERNAL_DEV_PREFIX)
 
         ex_gw_ip = ex_gw_port['fixed_ips'][0]['ip_address']
-        for c, r in self.external_gateway_filter_rules():
-            ri.iptables_manager.ipv4['filter'].remove_rule(c, r)
         for c, r in self.external_gateway_nat_rules(ex_gw_ip, internal_cidrs,
                                                     interface_name):
             ri.iptables_manager.ipv4['nat'].remove_rule(c, r)
         ri.iptables_manager.apply()
 
-    def external_gateway_filter_rules(self):
+    def metadata_filter_rules(self):
         rules = []
         if self.conf.metadata_ip:
             rules.append(('INPUT', '-s 0.0.0.0/0 -d %s '
@@ -408,16 +421,20 @@ class L3NATAgent(object):
                          (self.conf.metadata_ip, self.conf.metadata_port)))
         return rules
 
-    def external_gateway_nat_rules(self, ex_gw_ip, internal_cidrs,
-                                   interface_name):
-        rules = [('POSTROUTING', '! -i %(interface_name)s '
-                  '! -o %(interface_name)s -m conntrack ! '
-                  '--ctstate DNAT -j ACCEPT' % locals())]
+    def metadata_nat_rules(self):
+        rules = []
         if self.conf.metadata_ip:
             rules.append(('PREROUTING', '-s 0.0.0.0/0 -d 169.254.169.254/32 '
                          '-p tcp -m tcp --dport 80 -j DNAT '
                          '--to-destination %s:%s' %
                          (self.conf.metadata_ip, self.conf.metadata_port)))
+        return rules
+
+    def external_gateway_nat_rules(self, ex_gw_ip, internal_cidrs,
+                                   interface_name):
+        rules = [('POSTROUTING', '! -i %(interface_name)s '
+                  '! -o %(interface_name)s -m conntrack ! '
+                  '--ctstate DNAT -j ACCEPT' % locals())]
         for cidr in internal_cidrs:
             rules.extend(self.internal_network_nat_rules(ex_gw_ip, cidr))
         return rules