From 1d8eabaff1cff9efbc122b68a54b94745e001cd6 Mon Sep 17 00:00:00 2001 From: Pavel Bondar Date: Tue, 21 Jun 2016 14:34:57 +0300 Subject: [PATCH] Do not rewrite original exception for IPAM part 2 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 --- neutron/db/ipam_pluggable_backend.py | 4 ++- .../unit/db/test_ipam_pluggable_backend.py | 25 ++++++++++++++----- 2 files changed, 22 insertions(+), 7 deletions(-) diff --git a/neutron/db/ipam_pluggable_backend.py b/neutron/db/ipam_pluggable_backend.py index 94b5035506f..39abb9c8e0a 100644 --- a/neutron/db/ipam_pluggable_backend.py +++ b/neutron/db/ipam_pluggable_backend.py @@ -467,5 +467,7 @@ class IpamPluggableBackend(ipam_backend_mixin.IpamBackendMixin): with excutils.save_and_reraise_exception(): LOG.debug("An exception occurred during subnet creation. " "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 diff --git a/neutron/tests/unit/db/test_ipam_pluggable_backend.py b/neutron/tests/unit/db/test_ipam_pluggable_backend.py index 49aa6a90532..d4079c9a06b 100644 --- a/neutron/tests/unit/db/test_ipam_pluggable_backend.py +++ b/neutron/tests/unit/db/test_ipam_pluggable_backend.py @@ -385,9 +385,7 @@ class TestDbBasePluginIpam(test_db_base.NeutronDbPluginV2TestCase): net = self.deserialize(self.fmt, res) self.assertEqual(0, len(net['network']['subnets'])) - @mock.patch('neutron.ipam.driver.Pool') - def test_ipam_subnet_deallocated_if_create_fails(self, pool_mock): - mocks = self._prepare_mocks_with_pool_mock(pool_mock) + def _test_rollback_on_subnet_creation(self, pool_mock, driver_mocks): cidr = '10.0.2.0/24' with mock.patch.object( ipam_backend_mixin.IpamBackendMixin, '_save_subnet', @@ -396,14 +394,29 @@ class TestDbBasePluginIpam(test_db_base.NeutronDbPluginV2TestCase): cidr, expected_res_status=500) pool_mock.get_instance.assert_any_call(None, mock.ANY) self.assertEqual(2, pool_mock.get_instance.call_count) - self.assertTrue(mocks['driver'].allocate_subnet.called) - request = mocks['driver'].allocate_subnet.call_args[0][0] + self.assertTrue(driver_mocks['driver'].allocate_subnet.called) + request = driver_mocks['driver'].allocate_subnet.call_args[0][0] self.assertIsInstance(request, ipam_req.SpecificSubnetRequest) self.assertEqual(netaddr.IPNetwork(cidr), request.subnet_cidr) # 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) + @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') def test_update_subnet_over_ipam(self, pool_mock): mocks = self._prepare_mocks_with_pool_mock(pool_mock)