From ccd30a8cab6b91259cfb09b16a8fbbf69747cdf4 Mon Sep 17 00:00:00 2001 From: Cedric Brandily Date: Wed, 22 Oct 2014 14:03:13 +0200 Subject: [PATCH] Correct inconsistent enable_snat management Neutron resets enable_snat attribute when external_gateway_info is cleared but not when external_gateway_info is only updated which implies the following sets of actions have different behaviors: neutron router-gateway-set router1 pub1 --disable-snat neutron router-gateway-set router1 pub2 enable_snat is False after the last command neutron router-gateway-set router1 pub1 --disable-snat neutron router-gateway-clear router1 neutron router-gateway-set router1 pub2 enable_snat is True after the 2nd command resets the gateway AND enable_snat. This change proposes to always reset the attribute enable_snat when enable_snat is not provided in external_gateway_info on POST/PUT for consistency. APIImpact Change-Id: Ibab289936c55b1cf9614b44a4f18f54c959ee9e8 Closes-Bug: #1384146 --- neutron/db/l3_gwmode_db.py | 5 ++-- .../unit/extensions/test_l3_ext_gw_mode.py | 28 ++++++++++++++++--- 2 files changed, 26 insertions(+), 7 deletions(-) diff --git a/neutron/db/l3_gwmode_db.py b/neutron/db/l3_gwmode_db.py index 6d4b33061..558adff15 100644 --- a/neutron/db/l3_gwmode_db.py +++ b/neutron/db/l3_gwmode_db.py @@ -55,9 +55,8 @@ class L3_NAT_dbonly_mixin(l3_db.L3_NAT_dbonly_mixin): # Load the router only if necessary if not router: router = self._get_router(context, router_id) - # if enable_snat is not specified use the value - # stored in the database (default:True) - enable_snat = not info or info.get('enable_snat', router.enable_snat) + # if enable_snat is not specified then use the default value (True) + enable_snat = not info or info.get('enable_snat', True) with context.session.begin(subtransactions=True): router.enable_snat = enable_snat diff --git a/neutron/tests/unit/extensions/test_l3_ext_gw_mode.py b/neutron/tests/unit/extensions/test_l3_ext_gw_mode.py index 291c88fcb..cba57a6bf 100644 --- a/neutron/tests/unit/extensions/test_l3_ext_gw_mode.py +++ b/neutron/tests/unit/extensions/test_l3_ext_gw_mode.py @@ -209,7 +209,14 @@ class TestL3GwModeMixin(testlib_api.SqlTestCase): self.context.session.add(self.router) self.context.session.flush() - def _test_update_router_gw(self, gw_info, expected_enable_snat): + def _test_update_router_gw(self, current_enable_snat, gw_info=None, + expected_enable_snat=True): + if not current_enable_snat: + previous_gw_info = {'network_id': self.ext_net_id, + 'enable_snat': current_enable_snat} + self.target_object._update_router_gw_info( + self.context, self.router.id, previous_gw_info) + self.target_object._update_router_gw_info( self.context, self.router.id, gw_info) router = self.target_object._get_router( @@ -224,16 +231,29 @@ class TestL3GwModeMixin(testlib_api.SqlTestCase): self.assertEqual(expected_enable_snat, router.enable_snat) def test_update_router_gw_with_gw_info_none(self): - self._test_update_router_gw(None, True) + self._test_update_router_gw(current_enable_snat=True) + + def test_update_router_gw_without_info_and_snat_disabled_previously(self): + self._test_update_router_gw(current_enable_snat=False) def test_update_router_gw_with_network_only(self): info = {'network_id': self.ext_net_id} - self._test_update_router_gw(info, True) + self._test_update_router_gw(current_enable_snat=True, gw_info=info) + + def test_update_router_gw_with_network_and_snat_disabled_previously(self): + info = {'network_id': self.ext_net_id} + self._test_update_router_gw(current_enable_snat=False, gw_info=info) def test_update_router_gw_with_snat_disabled(self): info = {'network_id': self.ext_net_id, 'enable_snat': False} - self._test_update_router_gw(info, False) + self._test_update_router_gw( + current_enable_snat=True, gw_info=info, expected_enable_snat=False) + + def test_update_router_gw_with_snat_enabled(self): + info = {'network_id': self.ext_net_id, + 'enable_snat': True} + self._test_update_router_gw(current_enable_snat=False, gw_info=info) def test_make_router_dict_no_ext_gw(self): self._reset_ext_gw() -- 2.45.2