Simple refactor to stop passing around an unused parameter

There is no functional change in this patch.  However, this refactor
allows another patch [1] to make a simpler change to the code base.

This patch includes localizing the initialization of "internal_cidrs"
to a later spot in the "process_router" method.  This has the
advantage that it will be initialized under the same condition that it
will be used.

[1] https://review.openstack.org/#/c/109776

Change-Id: I3ca993d0a33b7c925526099e97999c9d9f76efc2

diff --git a/neutron/agent/l3_agent.py b/neutron/agent/l3_agent.py
index cf66df7b1b7e401493d1c0b35a8b7930652c30c5..8c538984e2f9d8cecf60297d36660751bed8cae1 100644 (file)
--- a/neutron/agent/l3_agent.py
+++ b/neutron/agent/l3_agent.py
@@ -752,9 +752,6 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback, manager.Manager):
                                namespace=ri.ns_name,
                                prefix=INTERNAL_DEV_PREFIX)
 
-        # Get IPv4 only internal CIDRs
-        internal_cidrs = [p['ip_cidr'] for p in ri.internal_ports
-                          if netaddr.IPNetwork(p['ip_cidr']).version == 4]
         # TODO(salv-orlando): RouterInfo would be a better place for
         # this logic too
         ex_gw_port_id = (ex_gw_port and ex_gw_port['id'] or
@@ -765,11 +762,9 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback, manager.Manager):
             interface_name = self.get_external_device_name(ex_gw_port_id)
         if ex_gw_port and ex_gw_port != ri.ex_gw_port:
             self._set_subnet_info(ex_gw_port)
-            self.external_gateway_added(ri, ex_gw_port,
-                                        interface_name, internal_cidrs)
+            self.external_gateway_added(ri, ex_gw_port, interface_name)
         elif not ex_gw_port and ri.ex_gw_port:
-            self.external_gateway_removed(ri, ri.ex_gw_port,
-                                          interface_name, internal_cidrs)
+            self.external_gateway_removed(ri, ri.ex_gw_port, interface_name)
 
         stale_devs = [dev for dev in existing_devices
                       if dev.startswith(EXTERNAL_DEV_PREFIX)
@@ -787,6 +782,9 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback, manager.Manager):
         # Process SNAT rules for external gateway
         if (not ri.router['distributed'] or
             ex_gw_port and ri.router['gw_port_host'] == self.host):
+            # Get IPv4 only internal CIDRs
+            internal_cidrs = [p['ip_cidr'] for p in ri.internal_ports
+                              if netaddr.IPNetwork(p['ip_cidr']).version == 4]
             ri.perform_snat_action(self._handle_router_snat_rules,
                                    internal_cidrs, interface_name)
 
@@ -1055,7 +1053,7 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback, manager.Manager):
             LOG.error(_('DVR: no map match_port found!'))
 
     def _create_dvr_gateway(self, ri, ex_gw_port, gw_interface_name,
-                            internal_cidrs, snat_ports):
+                            snat_ports):
         """Create SNAT namespace."""
         snat_ns_name = self.get_snat_ns_name(ri.router['id'])
         self._create_namespace(snat_ns_name)
@@ -1069,16 +1067,14 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback, manager.Manager):
                                          port['mac_address'], interface_name,
                                          SNAT_INT_DEV_PREFIX)
         self._external_gateway_added(ri, ex_gw_port, gw_interface_name,
-                                     internal_cidrs, snat_ns_name,
-                                     preserve_ips=[])
+                                     snat_ns_name, preserve_ips=[])
         ri.snat_iptables_manager = (
             iptables_manager.IptablesManager(
                 root_helper=self.root_helper, namespace=snat_ns_name
             )
         )
 
-    def external_gateway_added(self, ri, ex_gw_port,
-                               interface_name, internal_cidrs):
+    def external_gateway_added(self, ri, ex_gw_port, interface_name):
         if ri.router['distributed']:
             ip_wrapr = ip_lib.IPWrapper(self.root_helper, namespace=ri.ns_name)
             ip_wrapr.netns.execute(['sysctl', '-w',
@@ -1094,9 +1090,8 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback, manager.Manager):
             if self.conf.agent_mode == 'dvr_snat' and (
                     ri.router['gw_port_host'] == self.host):
                 if snat_ports:
-                    self._create_dvr_gateway(ri, ex_gw_port,
-                                             interface_name,
-                                             internal_cidrs, snat_ports)
+                    self._create_dvr_gateway(ri, ex_gw_port, interface_name,
+                                             snat_ports)
             for port in snat_ports:
                 for ip in port['fixed_ips']:
                     self._update_arp_entry(ri, ip['ip_address'],
@@ -1112,11 +1107,10 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback, manager.Manager):
                         for ip in floating_ips]
 
         self._external_gateway_added(ri, ex_gw_port, interface_name,
-                                     internal_cidrs, ri.ns_name,
-                                     preserve_ips)
+                                     ri.ns_name, preserve_ips)
 
     def _external_gateway_added(self, ri, ex_gw_port, interface_name,
-                                internal_cidrs, ns_name, preserve_ips):
+                                ns_name, preserve_ips):
         if not ip_lib.device_exists(interface_name,
                                     root_helper=self.root_helper,
                                     namespace=ns_name):
@@ -1170,8 +1164,7 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback, manager.Manager):
         ip_wrapper.netns.execute(['ip', 'addr', 'add',
                                   ip_cidr, 'dev', interface_name])
 
-    def external_gateway_removed(self, ri, ex_gw_port,
-                                 interface_name, internal_cidrs):
+    def external_gateway_removed(self, ri, ex_gw_port, interface_name):
         if ri.router['distributed']:
             for p in ri.internal_ports:
                 internal_interface = self.get_internal_device_name(p['id'])

diff --git a/neutron/tests/unit/test_l3_agent.py b/neutron/tests/unit/test_l3_agent.py
index 3a8b87da2c2d5762a27cdbe1e67374c6e455ad9e..94c9d3bc9c116b0b614c36b1a66bd0efb87261ee 100644 (file)
--- a/neutron/tests/unit/test_l3_agent.py
+++ b/neutron/tests/unit/test_l3_agent.py
@@ -337,7 +337,6 @@ class TestBasicRouterOperations(base.BaseTestCase):
         ri = l3_agent.RouterInfo(router['id'], self.conf.root_helper,
                                  self.conf.use_namespaces, router=router)
         agent = l3_agent.L3NATAgent(HOSTNAME, self.conf)
-        internal_cidrs = ['100.0.1.0/24', '200.74.0.0/16']
         ex_gw_port = {'fixed_ips': [{'ip_address': '20.0.0.30',
                                      'subnet_id': _uuid()}],
                       'subnet': {'gateway_ip': '20.0.0.1'},
@@ -355,8 +354,7 @@ class TestBasicRouterOperations(base.BaseTestCase):
                                          'fixed_ip_address': '192.168.0.1',
                                          'port_id': _uuid()}]}
             router[l3_constants.FLOATINGIP_KEY] = fake_fip['floatingips']
-            agent.external_gateway_added(ri, ex_gw_port,
-                                         interface_name, internal_cidrs)
+            agent.external_gateway_added(ri, ex_gw_port, interface_name)
             self.assertEqual(self.mock_driver.plug.call_count, 1)
             self.assertEqual(self.mock_driver.init_l3.call_count, 1)
             self.send_arp.assert_called_once_with(ri.ns_name, interface_name,
@@ -371,8 +369,7 @@ class TestBasicRouterOperations(base.BaseTestCase):
 
         elif action == 'remove':
             self.device_exists.return_value = True
-            agent.external_gateway_removed(ri, ex_gw_port,
-                                           interface_name, internal_cidrs)
+            agent.external_gateway_removed(ri, ex_gw_port, interface_name)
             self.assertEqual(self.mock_driver.unplug.call_count, 1)
         else:
             raise Exception("Invalid action %s" % action)
@@ -1716,11 +1713,9 @@ class TestBasicRouterOperations(base.BaseTestCase):
                        'id': _uuid(), 'device_id': _uuid()}]
 
         interface_name = agent.get_snat_int_device_name(port_id)
-        internal_cidrs = None
         self.device_exists.return_value = False
 
-        agent._create_dvr_gateway(ri, dvr_gw_port, interface_name,
-                                  internal_cidrs, snat_ports)
+        agent._create_dvr_gateway(ri, dvr_gw_port, interface_name, snat_ports)
 
         # check 2 internal ports are plugged
         # check 1 ext-gw-port is plugged