openstack
/
neutron
Code Issues Proposed changes

Fix reference to uninitialized iptables manager 

DvrEdgeRouter.process_address_scope() currently assumes that
snat_iptables_manager was initialized, however this is only done when an
external gateway is added. In case a new DVR+HA router was created
without an external gateway, the l3 agent will raise an exception and
will not create the router correctly. This patch adds a simple check to
make sure that it is defined before it's actually used.

Closes-Bug: #1560945

(cherry picked from commit a8b6067115)

Change-Id: I677e0837956a6d008a3935d961f078987a07d0c4
This commit is contained in:
John Schwarz 2016-03-23 14:05:37 +02:00 committed by Armando Migliaccio
parent 6d9774b058
commit 910744781c
4 changed files with 85 additions and 35 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
neutron/agent/l3/dvr_edge_router.py
View File

@ -216,8 +216,10 @@ class DvrEdgeRouter(dvr_local_router.DvrLocalRouter):
if not self._is_this_snat_host():
return
if not self.snat_iptables_manager:
LOG.debug("DVR router: no snat rules to be handled")
return
snat_iptables_manager = self.snat_iptables_manager
# Prepare address scope iptables rule for dvr snat interfaces
internal_ports = self.get_snat_interfaces()
ports_scopemark = self._get_port_devicename_scopemark(
@ -232,6 +234,6 @@ class DvrEdgeRouter(dvr_local_router.DvrLocalRouter):
ports_scopemark[ip_version].update(
external_port_scopemark[ip_version])
with snat_iptables_manager.defer_apply():
with self.snat_iptables_manager.defer_apply():
self._add_address_scope_mark(
snat_iptables_manager, ports_scopemark)
self.snat_iptables_manager, ports_scopemark)

4
neutron/tests/common/l3_test_common.py
View File

@ -53,7 +53,7 @@ def get_ha_interface(ip='169.254.192.1', mac='12:34:56:78:2b:5d'):
def prepare_router_data(ip_version=4, enable_snat=None, num_internal_ports=1,
enable_floating_ip=False, enable_ha=False,
extra_routes=False, dual_stack=False,
v6_ext_gw_with_sub=True, **kwargs):
enable_gw=True, v6_ext_gw_with_sub=True, **kwargs):
fixed_ips = []
subnets = []
gateway_mac = kwargs.get('gateway_mac', 'ca:fe:de:ad:be:ee')
@ -86,6 +86,8 @@ def prepare_router_data(ip_version=4, enable_snat=None, num_internal_ports=1,
raise ValueError("Invalid ip_version: %s" % ip_version)
router_id = _uuid()
ex_gw_port = {}
if enable_gw:
ex_gw_port = {'id': _uuid(),
'mac_address': gateway_mac,
'network_id': _uuid(),

47
neutron/tests/functional/agent/l3/test_dvr_router.py
View File

@ -42,6 +42,7 @@ class TestDvrRouter(framework.L3AgentTestFramework):
agent.fipnamespace_delete_on_ext_net(None, ext_net_id)
except RuntimeError:
pass
if router['gw_port']:
self.addCleanup(
_safe_fipnamespace_delete_on_ext_net,
router['gw_port']['network_id'])
@ -207,6 +208,7 @@ class TestDvrRouter(framework.L3AgentTestFramework):
def generate_dvr_router_info(self,
enable_ha=False,
enable_snat=False,
enable_gw=True,
agent=None,
extra_routes=False,
**kwargs):
@ -218,14 +220,18 @@ class TestDvrRouter(framework.L3AgentTestFramework):
enable_ha=enable_ha,
extra_routes=extra_routes,
num_internal_ports=2,
enable_gw=enable_gw,
**kwargs)
internal_ports = router.get(l3_constants.INTERFACE_KEY, [])
router['distributed'] = True
router['gw_port_host'] = agent.conf.host
router['gw_port'][portbindings.HOST_ID] = agent.conf.host
floating_ip = router['_floatingips'][0]
floating_ip['floating_network_id'] = router['gw_port']['network_id']
floating_ip['host'] = agent.conf.host
if enable_gw:
external_gw_port = router['gw_port']
router['gw_port'][portbindings.HOST_ID] = agent.conf.host
floating_ip['floating_network_id'] = external_gw_port['network_id']
floating_ip['port_id'] = internal_ports[0]['id']
floating_ip['status'] = 'ACTIVE'
@ -234,8 +240,8 @@ class TestDvrRouter(framework.L3AgentTestFramework):
# the snat_port info and fip_agent_gw_port_info irrespective of
# the agent type the dvr supports. The namespace creation is
# dependent on the agent_type.
external_gw_port = router['gw_port']
self._add_fip_agent_gw_port_info_to_router(router, external_gw_port)
self._add_fip_agent_gw_port_info_to_router(router,
external_gw_port)
return router
def _add_fip_agent_gw_port_info_to_router(self, router, external_gw_port):
@ -599,10 +605,11 @@ class TestDvrRouter(framework.L3AgentTestFramework):
self.assertFalse(sg_device)
self.assertTrue(qg_device)
def _mocked_dvr_ha_router(self, agent):
def _mocked_dvr_ha_router(self, agent, enable_gw=True):
r_info = self.generate_dvr_router_info(enable_ha=True,
enable_snat=True,
agent=agent)
agent=agent,
enable_gw=enable_gw)
r_snat_ns_name = namespaces.build_ns_name(dvr_snat_ns.SNAT_NS_PREFIX,
r_info['id'])
@ -631,14 +638,14 @@ class TestDvrRouter(framework.L3AgentTestFramework):
br_int_1.add_port(veth1.name)
br_int_2.add_port(veth2.name)
def _create_dvr_ha_router(self, agent):
def _create_dvr_ha_router(self, agent, enable_gw=True):
get_ns_name = mock.patch.object(namespaces.RouterNamespace,
'_get_ns_name').start()
get_snat_ns_name = mock.patch.object(dvr_snat_ns.SnatNamespace,
'get_snat_ns_name').start()
(r_info,
mocked_r_ns_name,
mocked_r_snat_ns_name) = self._mocked_dvr_ha_router(agent)
mocked_r_snat_ns_name) = self._mocked_dvr_ha_router(agent, enable_gw)
get_ns_name.return_value = mocked_r_ns_name
get_snat_ns_name.return_value = mocked_r_snat_ns_name
router = self.manage_router(agent, r_info)
@ -647,7 +654,11 @@ class TestDvrRouter(framework.L3AgentTestFramework):
def _assert_ip_addresses_in_dvr_ha_snat_namespace(self, router):
namespace = router.ha_namespace
ex_gw_port = router.get_ex_gw_port()
snat_port = router.get_snat_interfaces()[0]
snat_ports = router.get_snat_interfaces()
if not snat_ports:
return
snat_port = snat_ports[0]
ex_gw_port_name = router.get_external_device_name(
ex_gw_port['id'])
snat_port_name = router._get_snat_int_device_name(
@ -670,7 +681,11 @@ class TestDvrRouter(framework.L3AgentTestFramework):
def _assert_no_ip_addresses_in_dvr_ha_snat_namespace(self, router):
namespace = router.ha_namespace
ex_gw_port = router.get_ex_gw_port()
snat_port = router.get_snat_interfaces()[0]
snat_ports = router.get_snat_interfaces()
if not snat_ports:
return
snat_port = snat_ports[0]
ex_gw_port_name = router.get_external_device_name(
ex_gw_port['id'])
snat_port_name = router._get_snat_int_device_name(
@ -681,12 +696,12 @@ class TestDvrRouter(framework.L3AgentTestFramework):
self._assert_no_ip_addresses_on_interface(namespace,
ex_gw_port_name)
def test_dvr_ha_router_failover(self):
def _test_dvr_ha_router_failover(self, enable_gw):
self._setup_dvr_ha_agents()
self._setup_dvr_ha_bridges()
router1 = self._create_dvr_ha_router(self.agent)
router2 = self._create_dvr_ha_router(self.failover_agent)
router1 = self._create_dvr_ha_router(self.agent, enable_gw=enable_gw)
router2 = self._create_dvr_ha_router(self.failover_agent, enable_gw)
utils.wait_until_true(lambda: router1.ha_state == 'master')
utils.wait_until_true(lambda: router2.ha_state == 'backup')
@ -702,6 +717,12 @@ class TestDvrRouter(framework.L3AgentTestFramework):
self._assert_ip_addresses_in_dvr_ha_snat_namespace(router2)
self._assert_no_ip_addresses_in_dvr_ha_snat_namespace(router1)
def test_dvr_ha_router_failover_with_gw(self):
self._test_dvr_ha_router_failover(enable_gw=True)
def test_dvr_ha_router_failover_without_gw(self):
self._test_dvr_ha_router_failover(enable_gw=False)
def test_dvr_router_static_routes(self):
"""Test to validate the extra routes on dvr routers."""
self.agent.conf.agent_mode = 'dvr_snat'

25
neutron/tests/unit/agent/l3/test_agent.py
View File

@ -41,6 +41,7 @@ from neutron.agent.l3 import router_processing_queue
from neutron.agent.linux import dibbler
from neutron.agent.linux import external_process
from neutron.agent.linux import interface
from neutron.agent.linux import iptables_manager
from neutron.agent.linux import pd
from neutron.agent.linux import ra
from neutron.agent.metadata import driver as metadata_driver
@ -2203,6 +2204,30 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework):
self.assertEqual(3, self.mock_driver.plug.call_count)
self.assertEqual(3, self.mock_driver.init_router_port.call_count)
def test_process_address_scope(self):
router = l3_test_common.prepare_router_data()
router['distributed'] = True
router['gw_port_host'] = HOSTNAME
agent = l3_agent.L3NATAgent(HOSTNAME, self.conf)
ri = dvr_router.DvrEdgeRouter(agent,
HOSTNAME,
router['id'],
router,
**self.ri_kwargs)
ri.get_ex_gw_port = mock.Mock(return_value=None)
# Make sure the code doesn't crash if ri.snat_iptables_manager is None.
ri.process_address_scope()
with mock.patch.object(ri, '_add_address_scope_mark') as mocked_func:
ri.snat_iptables_manager = iptables_manager.IptablesManager(
namespace=mock.ANY, use_ipv6=False)
ri.snat_iptables_manager.defer_apply_off = mock.Mock()
ri.process_address_scope()
self.assertEqual(2, mocked_func.call_count)
def test_get_service_plugin_list(self):
service_plugins = [p_const.L3_ROUTER_NAT]
self.plugin_api.get_service_plugin_list.return_value = service_plugins