openstack/neutron
Code Issues Proposed changes

Do not rewrite original exception for IPAM part 2

Browse Source 
If subnet allocation fails on _save_subnet stage, then rollback mechnism
is executed. Currently if exception is raised on rollback stage it
replaces original exception. But original exception is needed by
retry wrappers to work properly also it is more informative for user.

Using _safe_rollback method to prevent replacing original exception with
one from rollback stage.

Closes-Bug: #1571666
Change-Id: I388272820451bb117281948fef0000bb46a951d4
This commit is contained in:
Pavel Bondar 2016-06-21 14:34:57 +03:00
parent b59bb0fcfa
commit 1d8eabaff1
2 changed files with 22 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
neutron/db/ipam_pluggable_backend.py
View File

@ -467,5 +467,7 @@ class IpamPluggableBackend(ipam_backend_mixin.IpamBackendMixin):
with excutils.save_and_reraise_exception(): with excutils.save_and_reraise_exception():
LOG.debug("An exception occurred during subnet creation. " LOG.debug("An exception occurred during subnet creation. "
"Reverting subnet allocation.") "Reverting subnet allocation.")
self.delete_subnet(context, subnet_request.subnet_id) self._safe_rollback(self.delete_subnet,
context,
subnet_request.subnet_id)
return subnet, ipam_subnet return subnet, ipam_subnet

25
neutron/tests/unit/db/test_ipam_pluggable_backend.py
View File

@ -385,9 +385,7 @@ class TestDbBasePluginIpam(test_db_base.NeutronDbPluginV2TestCase):
net = self.deserialize(self.fmt, res) net = self.deserialize(self.fmt, res)
self.assertEqual(0, len(net['network']['subnets'])) self.assertEqual(0, len(net['network']['subnets']))
@mock.patch('neutron.ipam.driver.Pool') def _test_rollback_on_subnet_creation(self, pool_mock, driver_mocks):
def test_ipam_subnet_deallocated_if_create_fails(self, pool_mock):
mocks = self._prepare_mocks_with_pool_mock(pool_mock)
cidr = '10.0.2.0/24' cidr = '10.0.2.0/24'
with mock.patch.object( with mock.patch.object(
ipam_backend_mixin.IpamBackendMixin, '_save_subnet', ipam_backend_mixin.IpamBackendMixin, '_save_subnet',
@ -396,14 +394,29 @@ class TestDbBasePluginIpam(test_db_base.NeutronDbPluginV2TestCase):
cidr, expected_res_status=500) cidr, expected_res_status=500)
pool_mock.get_instance.assert_any_call(None, mock.ANY) pool_mock.get_instance.assert_any_call(None, mock.ANY)
self.assertEqual(2, pool_mock.get_instance.call_count) self.assertEqual(2, pool_mock.get_instance.call_count)
self.assertTrue(mocks['driver'].allocate_subnet.called) self.assertTrue(driver_mocks['driver'].allocate_subnet.called)
request = mocks['driver'].allocate_subnet.call_args[0][0] request = driver_mocks['driver'].allocate_subnet.call_args[0][0]
self.assertIsInstance(request, ipam_req.SpecificSubnetRequest) self.assertIsInstance(request, ipam_req.SpecificSubnetRequest)
self.assertEqual(netaddr.IPNetwork(cidr), request.subnet_cidr) self.assertEqual(netaddr.IPNetwork(cidr), request.subnet_cidr)
# Verify remove ipam subnet was called # Verify remove ipam subnet was called
mocks['driver'].remove_subnet.assert_called_once_with( driver_mocks['driver'].remove_subnet.assert_called_once_with(
self.subnet_id) self.subnet_id)
@mock.patch('neutron.ipam.driver.Pool')
def test_ipam_subnet_deallocated_if_create_fails(self, pool_mock):
driver_mocks = self._prepare_mocks_with_pool_mock(pool_mock)
self._test_rollback_on_subnet_creation(pool_mock, driver_mocks)
@mock.patch('neutron.ipam.driver.Pool')
def test_ipam_subnet_create_and_rollback_fails(self, pool_mock):
driver_mocks = self._prepare_mocks_with_pool_mock(pool_mock)
# remove_subnet is called on rollback stage and n_exc.NotFound
# typically produces 404 error. Validate that exception from
# rollback stage is silenced and main exception (ValueError in this
# case) is reraised. So resulting http status should be 500.
driver_mocks['driver'].remove_subnet.side_effect = n_exc.NotFound
self._test_rollback_on_subnet_creation(pool_mock, driver_mocks)
@mock.patch('neutron.ipam.driver.Pool') @mock.patch('neutron.ipam.driver.Pool')
def test_update_subnet_over_ipam(self, pool_mock): def test_update_subnet_over_ipam(self, pool_mock):
mocks = self._prepare_mocks_with_pool_mock(pool_mock) mocks = self._prepare_mocks_with_pool_mock(pool_mock)