diff --git a/neutron/agent/l3_agent.py b/neutron/agent/l3_agent.py index f632996d23..c2b551421f 100644 --- a/neutron/agent/l3_agent.py +++ b/neutron/agent/l3_agent.py @@ -258,6 +258,7 @@ class RouterInfo(object): root_helper=root_helper, use_ipv6=use_ipv6, namespace=self.ns_name) + self.snat_iptables_manager = None self.routes = [] # DVR Data # Linklocal subnet for router and floating IP namespace link @@ -951,10 +952,13 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback, manager.Manager): # This is safe because if use_namespaces is set as False # then the agent can only configure one router, otherwise # each router's SNAT rules will be in their own namespace - if ri.router['distributed']: + if not ri.router['distributed']: + iptables_manager = ri.iptables_manager + elif ri.snat_iptables_manager: iptables_manager = ri.snat_iptables_manager else: - iptables_manager = ri.iptables_manager + LOG.debug("DVR router: no snat rules to be handled") + return iptables_manager.ipv4['nat'].empty_chain('POSTROUTING') iptables_manager.ipv4['nat'].empty_chain('snat') @@ -1212,11 +1216,10 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback, manager.Manager): self._snat_redirect_add(ri, gateway['fixed_ips'][0] ['ip_address'], p, id_name) - 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, - snat_ports) + if (self.conf.agent_mode == 'dvr_snat' and + ri.router['gw_port_host'] == self.host): + 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'], @@ -1591,9 +1594,10 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback, manager.Manager): self._queue.add(update) def _update_arp_entry(self, ri, ip, mac, subnet_id, operation): - """Add or delete arp entry into router namespace.""" + """Add or delete arp entry into router namespace for the subnet.""" port = self.get_internal_port(ri, subnet_id) - if 'id' in port: + # update arp entry only if the subnet is attached to the router + if port: ip_cidr = str(ip) + '/32' try: # TODO(mrsmith): optimize the calls below for bulk calls diff --git a/neutron/tests/unit/test_l3_agent.py b/neutron/tests/unit/test_l3_agent.py index b10353e89e..42d0d09ef3 100644 --- a/neutron/tests/unit/test_l3_agent.py +++ b/neutron/tests/unit/test_l3_agent.py @@ -840,6 +840,16 @@ class TestBasicRouterOperations(base.BaseTestCase): agent.add_arp_entry(None, payload) self.assertFalse(agent._update_arp_entry.called) + def test__update_arp_entry_with_no_subnet(self): + agent = l3_agent.L3NATAgent(HOSTNAME, self.conf) + ri = l3_agent.RouterInfo( + 'foo_router_id', mock.ANY, True, + {'distributed': True, 'gw_port_host': HOSTNAME}) + with mock.patch.object(l3_agent.ip_lib, 'IPDevice') as f: + agent._update_arp_entry(ri, mock.ANY, mock.ANY, + 'foo_subnet_id', 'add') + self.assertFalse(f.call_count) + def test_del_arp_entry(self): agent = l3_agent.L3NATAgent(HOSTNAME, self.conf) router = prepare_router_data(num_internal_ports=2) @@ -1456,6 +1466,19 @@ class TestBasicRouterOperations(base.BaseTestCase): mock.ANY, ri.router_id, {fip_id: l3_constants.FLOATINGIP_STATUS_ERROR}) + def test_handle_router_snat_rules_distributed_without_snat_manager(self): + ri = l3_agent.RouterInfo( + 'foo_router_id', mock.ANY, True, {'distributed': True}) + ri.iptables_manager = mock.Mock() + + agent = l3_agent.L3NATAgent(HOSTNAME, self.conf) + with mock.patch.object(l3_agent.LOG, 'debug') as log_debug: + agent._handle_router_snat_rules( + ri, mock.ANY, mock.ANY, mock.ANY, mock.ANY) + self.assertIsNone(ri.snat_iptables_manager) + self.assertFalse(ri.iptables_manager.called) + self.assertTrue(log_debug.called) + def test_handle_router_snat_rules_add_back_jump(self): agent = l3_agent.L3NATAgent(HOSTNAME, self.conf) ri = mock.MagicMock()