From d2291d6500d0db25ea62be70219fa62c4b024d17 Mon Sep 17 00:00:00 2001 From: Carl Baldwin Date: Mon, 4 Aug 2014 19:29:33 +0000 Subject: [PATCH] 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 --- neutron/agent/l3_agent.py | 33 ++++++++++++----------------- neutron/tests/unit/test_l3_agent.py | 11 +++------- 2 files changed, 16 insertions(+), 28 deletions(-) diff --git a/neutron/agent/l3_agent.py b/neutron/agent/l3_agent.py index cf66df7b1..8c538984e 100644 --- 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 3a8b87da2..94c9d3bc9 100644 --- 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 -- 2.45.2