Fix l3-agent crash on routers without ha_state
l3-agent checks the HA state of routers when a router is updated.
To ensure that the HA state is only checked on HA routers the following
check is performed: `if router.get('ha') and not is_dvr_only_agent`.
This check should ensure that the check is only performed on
DvrEdgeHaRouter and HaRouter objects.
Unfortunately, there are cases where we have DvrEdgeRouter objects
running on 'dvr_snat' agents. E.g. when deploying a loadbalancer with
neutron-lbaas in a landscape with 6 network nodes and
max_l3_agents_per_router set to 3, it may happen that the loadbalancer
is placed on a network node that does not have a DvrEdgeHaRouter running
on it. In such a case, neutron will deploy a DvrEdgeRouter object on the
network node to serve the loadbalancer, just like it would deploy a
DvrEdgeRouter on a compute node when deploying a VM.
Under such circumstances each update to the router will lead to an
AttributeError, because the DvrEdgeRouter object does not have the
ha_state attribute.
This patch circumvents the issue by doing an additional check on the
router object to ensure that it actually has the ha_state attribute.
Conflicts:
neutron/agent/l3/agent.py
neutron/tests/functional/agent/l3/test_dvr_router.py
Change-Id: I755990324db445efd0ee0b8a9db1f4d7bfb58e26
Closes-Bug: #1755243
(cherry picked from commit 8c2dae659a
)
This commit is contained in:
parent
1afa908c7f
commit
7c874cd89a
|
@ -468,13 +468,14 @@ class L3NATAgent(ha.AgentMixin,
|
|||
self.l3_ext_manager.add_router(self.context, router)
|
||||
|
||||
def _process_updated_router(self, router):
|
||||
ri = self.router_info[router['id']]
|
||||
is_dvr_only_agent = (
|
||||
self.conf.agent_mode == lib_const.L3_AGENT_MODE_DVR)
|
||||
is_ha_router = getattr(ri, 'ha_state', None) is not None
|
||||
# For HA routers check that DB state matches actual state
|
||||
if router.get('ha') and not is_dvr_only_agent:
|
||||
if router.get('ha') and not is_dvr_only_agent and is_ha_router:
|
||||
self.check_ha_state_for_router(
|
||||
router['id'], router.get(l3_constants.HA_ROUTER_STATE_KEY))
|
||||
ri = self.router_info[router['id']]
|
||||
ri.router = router
|
||||
registry.notify(resources.ROUTER, events.BEFORE_UPDATE,
|
||||
self, router=ri)
|
||||
|
|
|
@ -972,8 +972,8 @@ class TestDvrRouter(framework.L3AgentTestFramework):
|
|||
self.assertFalse(sg_device)
|
||||
self.assertTrue(qg_device)
|
||||
|
||||
def _mocked_dvr_ha_router(self, agent, enable_gw=True):
|
||||
r_info = self.generate_dvr_router_info(enable_ha=True,
|
||||
def _mocked_dvr_ha_router(self, agent, enable_ha=True, enable_gw=True):
|
||||
r_info = self.generate_dvr_router_info(enable_ha=enable_ha,
|
||||
enable_snat=True,
|
||||
agent=agent,
|
||||
enable_gw=enable_gw)
|
||||
|
@ -1005,14 +1005,19 @@ 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, enable_gw=True):
|
||||
def _create_dvr_ha_router(self, agent, enable_gw=True, ha_interface=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, enable_gw)
|
||||
mocked_r_snat_ns_name) = self._mocked_dvr_ha_router(
|
||||
agent, ha_interface, enable_gw)
|
||||
|
||||
if not ha_interface:
|
||||
r_info['ha'] = True
|
||||
|
||||
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)
|
||||
|
@ -1090,6 +1095,26 @@ class TestDvrRouter(framework.L3AgentTestFramework):
|
|||
def test_dvr_ha_router_failover_without_gw(self):
|
||||
self._test_dvr_ha_router_failover(enable_gw=False)
|
||||
|
||||
def test_dvr_non_ha_router_update(self):
|
||||
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,
|
||||
ha_interface=False)
|
||||
|
||||
r1_chsfr = mock.patch.object(self.agent,
|
||||
'check_ha_state_for_router').start()
|
||||
r2_chsfr = mock.patch.object(self.failover_agent,
|
||||
'check_ha_state_for_router').start()
|
||||
|
||||
utils.wait_until_true(lambda: router1.ha_state == 'master')
|
||||
|
||||
self.agent._process_updated_router(router1.router)
|
||||
self.assertTrue(r1_chsfr.called)
|
||||
self.failover_agent._process_updated_router(router2.router)
|
||||
self.assertFalse(r2_chsfr.called)
|
||||
|
||||
def _setup_dvr_router_static_routes(
|
||||
self, router_namespace=True, check_fpr_int_rule_delete=False):
|
||||
"""Test to validate the extra routes on dvr routers."""
|
||||
|
|
|
@ -2279,9 +2279,10 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework):
|
|||
'distributed': True, 'ha': True,
|
||||
'external_gateway_info': {}, 'routes': [],
|
||||
'admin_state_up': True}
|
||||
|
||||
agent._process_router_if_compatible(router)
|
||||
self.assertIn(router['id'], agent.router_info)
|
||||
with mock.patch.object(agent, 'check_ha_state_for_router') as chsfr:
|
||||
agent._process_router_if_compatible(router)
|
||||
self.assertIn(router['id'], agent.router_info)
|
||||
self.assertFalse(chsfr.called)
|
||||
|
||||
def test_process_router_if_compatible_with_no_ext_net_in_conf(self):
|
||||
self.conf.set_override('external_network_bridge', 'br-ex')
|
||||
|
|
Loading…
Reference in New Issue