openstack/neutron
Code Issues Proposed changes

create_router: Report the original exception

Browse Source 
In case delete_router, which is an attempt to revert
the operation, failed, report the original exception
rather than the one from delete_router.  The former is
more likely useful for users.  Besides that, it's critical
to preserve the original exception for special exceptions
like RetryRequest.

Also, add some relevant unit tests.

Closes-Bug: #1490917
Change-Id: I1cef944115c1911d4b7e60fb4dff389b1911ddbb
This commit is contained in:
YAMAMOTO Takashi 2015-09-01 19:07:50 +09:00
parent 36e3145994
commit e25151be3d
3 changed files with 88 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

25
neutron/db/l3_db.py
View File

@ -12,6 +12,7 @@
# License for the specific language governing permissions and limitations
# under the License.
import functools
import itertools
import netaddr
@ -37,6 +38,7 @@ from neutron.common import constants as n_const
from neutron.common import ipv6_utils
from neutron.common import rpc as n_rpc
from neutron.common import utils
from neutron.db import common_db_mixin
from neutron.db import l3_agentschedulers_db as l3_agt
from neutron.db import model_base
from neutron.db import models_v2
@ -226,18 +228,23 @@ class L3_NAT_dbonly_mixin(l3.RouterPluginBase,
context.session.add(router_db)
return router_db
def _update_gw_for_create_router(self, context, gw_info, router_id):
if gw_info:
router_db = self._get_router(context, router_id)
self._update_router_gw_info(context, router_id,
gw_info, router=router_db)
def create_router(self, context, router):
r = router['router']
gw_info = r.pop(EXTERNAL_GW_INFO, None)
router_db = self._create_router_db(context, r, r['tenant_id'])
try:
if gw_info:
self._update_router_gw_info(context, router_db['id'],
gw_info, router=router_db)
except Exception:
with excutils.save_and_reraise_exception():
LOG.debug("Could not update gateway info, deleting router.")
self.delete_router(context, router_db.id)
create = functools.partial(self._create_router_db, context, r,
r['tenant_id'])
delete = functools.partial(self.delete_router, context)
update_gw = functools.partial(self._update_gw_for_create_router,
context, gw_info)
router_db, _unused = common_db_mixin.safe_creation(context, create,
delete, update_gw,
transaction=False)
return self._make_router_dict(router_db)

2
neutron/tests/unit/db/test_l3_db.py
View File

@ -228,6 +228,8 @@ class L3_NAT_db_mixin(base.BaseTestCase):
return_value=router_dict),\
mock.patch.object(l3_db.L3_NAT_dbonly_mixin,
'_update_router_gw_info') as urgi,\
mock.patch.object(l3_db.L3_NAT_dbonly_mixin, '_get_router',
return_value=router_db),\
mock.patch.object(l3_db.L3_NAT_db_mixin, 'notify_router_updated')\
as nru:

70
neutron/tests/unit/extensions/test_l3.py
View File

@ -2797,6 +2797,76 @@ class L3NatTestCaseBase(L3NatTestCaseMixin):
fip['floatingip']['floating_ip_address'])
self.assertEqual(4, floating_ip.version)
def test_create_router_gateway_fails_nested(self):
# Force _update_router_gw_info failure
plugin = manager.NeutronManager.get_service_plugins()[
service_constants.L3_ROUTER_NAT]
if not isinstance(plugin, l3_db.L3_NAT_dbonly_mixin):
self.skipTest("Plugin is not L3_NAT_dbonly_mixin")
ctx = context.Context('', 'foo')
data = {'router': {
'name': 'router1', 'admin_state_up': True,
'external_gateway_info': {'network_id': 'some_uuid'},
'tenant_id': 'some_tenant'}}
def mock_fail__update_router_gw_info(ctx, router_id, info,
router=None):
# Fail with breaking transaction
with ctx.session.begin(subtransactions=True):
raise n_exc.NeutronException
mock.patch.object(plugin, '_update_router_gw_info',
side_effect=mock_fail__update_router_gw_info).start()
def create_router_with_transaction(ctx, data):
# Emulates what many plugins do
with ctx.session.begin(subtransactions=True):
plugin.create_router(ctx, data)
# Verify router doesn't persist on failure
self.assertRaises(n_exc.NeutronException,
create_router_with_transaction, ctx, data)
routers = plugin.get_routers(ctx)
self.assertEqual(0, len(routers))
def test_create_router_gateway_fails_nested_delete_router_failed(self):
# Force _update_router_gw_info failure
plugin = manager.NeutronManager.get_service_plugins()[
service_constants.L3_ROUTER_NAT]
if not isinstance(plugin, l3_db.L3_NAT_dbonly_mixin):
self.skipTest("Plugin is not L3_NAT_dbonly_mixin")
ctx = context.Context('', 'foo')
data = {'router': {
'name': 'router1', 'admin_state_up': True,
'external_gateway_info': {'network_id': 'some_uuid'},
'tenant_id': 'some_tenant'}}
def mock_fail__update_router_gw_info(ctx, router_id, info,
router=None):
# Fail with breaking transaction
with ctx.session.begin(subtransactions=True):
raise n_exc.NeutronException
def mock_fail_delete_router(ctx, router_id):
with ctx.session.begin(subtransactions=True):
raise Exception()
mock.patch.object(plugin, '_update_router_gw_info',
side_effect=mock_fail__update_router_gw_info).start()
mock.patch.object(plugin, 'delete_router',
mock_fail_delete_router).start()
def create_router_with_transaction(ctx, data):
# Emulates what many plugins do
with ctx.session.begin(subtransactions=True):
plugin.create_router(ctx, data)
# Verify router doesn't persist on failure
self.assertRaises(n_exc.NeutronException,
create_router_with_transaction, ctx, data)
routers = plugin.get_routers(ctx)
self.assertEqual(0, len(routers))
def test_update_subnet_gateway_for_external_net(self):
"""Test to make sure notification to routers occurs when the gateway
ip address of a subnet of the external network is changed.