openstack/tricircle
Code Issues Proposed changes

Router update should not touch nonexistent local router

Browse Source 
1. What is the problem
Set external gateway to a router, if error occurs during local router
creating/updating, exception will be raised, but this time central
router is already updated with external gateway information. So when we
try to unset the external gateway, central Neutron server will try to
remove external gateway for local router first, if local router doesn't
exist at this time, we have no way to unset the external gateway since
we fail to remove external gateway for local router.

2. What is the solution for the problem
Do not try to remove external gateway for a nonexistent local router.

3. What features need to be implemented to the Tricircle to
realize the solution
N/A

Change-Id: Id7261640405d4d24c8523f6518ec19edc031024b
Closes-Bug: #1693138
This commit is contained in:
zhiyuan_cai 2017-05-26 10:20:48 +08:00
parent 3f2480027d
commit 6c227a18bc
2 changed files with 34 additions and 9 deletions
tricircle
network
central_plugin.py
tests/unit/network
test_central_plugin.py

4
tricircle/network/central_plugin.py

@ -1553,6 +1553,10 @@ class TricirclePlugin(db_base_plugin_v2.NeutronDbPluginV2,
t_constants.RT_ROUTER if is_local_router
else t_constants.RT_NS_ROUTER)
if not b_router_id:
# local router doesn't exist, skip local gateway deletion
return
b_client = self._get_client(region_name)
b_client.action_routers(t_ctx, 'remove_gateway', b_router_id)

39
tricircle/tests/unit/network/test_central_plugin.py

@ -363,6 +363,7 @@ class FakeClient(object):
'ip_address': ip}
def create_resources(self, _type, ctx, body):
self._get_connection()
if _type == 'port':
res_list = self._res_map[self.region_name][_type]
subnet_ips_map = {}
@ -3292,9 +3293,10 @@ class PluginTest(unittest.TestCase,
new=fake_make_router_dict)
@patch.object(db_base_plugin_common.DbBasePluginCommon,
'_make_subnet_dict', new=fake_make_subnet_dict)
@patch.object(FakeClient, '_get_connection')
@patch.object(FakeClient, 'action_routers')
@patch.object(context, 'get_context_from_neutron_context')
def test_unset_gateway(self, mock_context, mock_action):
def test_unset_gateway(self, mock_context, mock_action, mock_connect):
self._basic_pod_route_setup()
fake_plugin = FakePlugin()
@ -3339,21 +3341,40 @@ class PluginTest(unittest.TestCase,
}
TOP_ROUTERS.append(DotDict(t_router))
# first add router gateway
fake_plugin.update_router(
q_ctx, t_router_id,
{'router': {'external_gateway_info': {
add_gw_body = {
'router': {'external_gateway_info': {
'network_id': t_net_id,
'enable_snat': False,
'external_fixed_ips': [{'subnet_id': t_subnet_id,
'ip_address': '100.64.0.5'}]}}})
'ip_address': '100.64.0.5'}]}}}
del_gw_body = {'router': {'external_gateway_info': {}}}
# exception case, central router has been updated but local router has
# not been created
mock_connect.side_effect = q_exceptions.ConnectionFailed
mock_action.side_effect = q_exceptions.ConnectionFailed
self.assertRaises(q_exceptions.ConnectionFailed,
fake_plugin.update_router, q_ctx, t_router_id,
copy.deepcopy(add_gw_body))
mappings = db_api.get_bottom_mappings_by_top_id(t_ctx, t_router_id,
constants.RT_NS_ROUTER)
self.assertEqual(0, len(mappings))
# local router is not created, but we can still remove gateway. local
# router is not touched, otherwise we will meet an exception here
# because mock_action is assigned a side effect
fake_plugin.update_router(q_ctx, t_router_id,
copy.deepcopy(del_gw_body))
# normal case
mock_connect.side_effect = None
mock_action.side_effect = None
# first add router gateway
fake_plugin.update_router(q_ctx, t_router_id, add_gw_body)
_, b_ns_router_id = db_api.get_bottom_mappings_by_top_id(
t_ctx, t_router_id, constants.RT_NS_ROUTER)[0]
# then remove router gateway
fake_plugin.update_router(
q_ctx, t_router_id,
{'router': {'external_gateway_info': {}}})
fake_plugin.update_router(q_ctx, t_router_id, del_gw_body)
mock_action.assert_called_with(t_ctx, 'remove_gateway', b_ns_router_id)
def _prepare_associate_floatingip_test(self, t_ctx, q_ctx, fake_plugin,