x/vmware-nsx
Code Issues Proposed changes

Fix AttributeError when setting external gateway on DVR router

Browse Source 
DVR routers will have this manager initialized only after one
or more subnets have been attached to the router. To address
the issue, make sure the manager is defined and handle the snat
rules appropriately.

This patch also makes _update_arp_entry more defensive; this is
because the arp update process can be affected by the same issue:
the router may not have internal ports at the time the request
come in. This is likely when VM's port creation and router
configuration overlap slightly.

Closes-bug: #1353006

Change-Id: Ib46852f5b264e5ef2e2d499d3351f8974e393011
Co-authored-by: Rajeev Grover <rajeev.grover@hp.com>
This commit is contained in:
armando-migliaccio 2014-08-05 12:35:37 -07:00
parent 2227bb9365
commit 65543081f8
2 changed files with 36 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
neutron/agent/l3_agent.py
View File

@ -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,9 +1216,8 @@ 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 (
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)
for port in snat_ports:
@ -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

23
neutron/tests/unit/test_l3_agent.py
View File

@ -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()