diff --git a/networking_bgpvpn/neutron/db/bgpvpn_db.py b/networking_bgpvpn/neutron/db/bgpvpn_db.py index eb82550d..50abade4 100644 --- a/networking_bgpvpn/neutron/db/bgpvpn_db.py +++ b/networking_bgpvpn/neutron/db/bgpvpn_db.py @@ -264,7 +264,7 @@ class BGPVPNPluginDb(common_db_mixin.CommonDbMixin): LOG.info(_LI("deleting network association %(id)s for " "BGPVPN %(bgpvpn)s"), {'id': assoc_id, 'bgpvpn': bgpvpn_id}) - with context.session.begin(): + with context.session.begin(subtransactions=True): net_assoc_db = self._get_net_assoc(context, assoc_id, bgpvpn_id) net_assoc = self._make_net_assoc_dict(net_assoc_db) context.session.delete(net_assoc_db) @@ -321,7 +321,7 @@ class BGPVPNPluginDb(common_db_mixin.CommonDbMixin): LOG.info(_LI("deleting router association %(id)s for " "BGPVPN %(bgpvpn)s"), {'id': assoc_id, 'bgpvpn': bgpvpn_id}) - with context.session.begin(): + with context.session.begin(subtransactions=True): router_assoc_db = self._get_router_assoc(context, assoc_id, bgpvpn_id) router_assoc = self._make_router_assoc_dict(router_assoc_db) diff --git a/networking_bgpvpn/neutron/services/service_drivers/driver_api.py b/networking_bgpvpn/neutron/services/service_drivers/driver_api.py index 82e06a8f..ec84442c 100644 --- a/networking_bgpvpn/neutron/services/service_drivers/driver_api.py +++ b/networking_bgpvpn/neutron/services/service_drivers/driver_api.py @@ -124,7 +124,9 @@ class BGPVPNDriverDBMixin(BGPVPNDriverBase): return bgpvpn def delete_bgpvpn(self, context, id): - bgpvpn = self.bgpvpn_db.delete_bgpvpn(context, id) + with context.session.begin(subtransactions=True): + bgpvpn = self.bgpvpn_db.delete_bgpvpn(context, id) + self.delete_bgpvpn_precommit(context, bgpvpn) self.delete_bgpvpn_postcommit(context, bgpvpn) def create_net_assoc(self, context, bgpvpn_id, network_association): @@ -145,9 +147,11 @@ class BGPVPNDriverDBMixin(BGPVPNDriverBase): filters, fields) def delete_net_assoc(self, context, assoc_id, bgpvpn_id): - net_assoc = self.bgpvpn_db.delete_net_assoc(context, - assoc_id, - bgpvpn_id) + with context.session.begin(subtransactions=True): + net_assoc = self.bgpvpn_db.delete_net_assoc(context, + assoc_id, + bgpvpn_id) + self.delete_net_assoc_precommit(context, net_assoc) self.delete_net_assoc_postcommit(context, net_assoc) def create_router_assoc(self, context, bgpvpn_id, router_association): @@ -167,9 +171,11 @@ class BGPVPNDriverDBMixin(BGPVPNDriverBase): filters, fields) def delete_router_assoc(self, context, assoc_id, bgpvpn_id): - router_assoc = self.bgpvpn_db.delete_router_assoc(context, - assoc_id, - bgpvpn_id) + with context.session.begin(subtransactions=True): + router_assoc = self.bgpvpn_db.delete_router_assoc(context, + assoc_id, + bgpvpn_id) + self.delete_router_assoc_precommit(context, router_assoc) self.delete_router_assoc_postcommit(context, router_assoc) @abc.abstractmethod @@ -241,6 +247,9 @@ class BGPVPNDriver(BGPVPNDriverDBMixin): def update_bgpvpn_postcommit(self, context, old_bgpvpn, bgpvpn): pass + def delete_bgpvpn_precommit(self, context, bgpvpn): + pass + def delete_bgpvpn_postcommit(self, context, bgpvpn): pass @@ -250,6 +259,9 @@ class BGPVPNDriver(BGPVPNDriverDBMixin): def create_net_assoc_postcommit(self, context, net_assoc): pass + def delete_net_assoc_precommit(self, context, net_assoc): + pass + def delete_net_assoc_postcommit(self, context, net_assoc): pass @@ -259,5 +271,8 @@ class BGPVPNDriver(BGPVPNDriverDBMixin): def create_router_assoc_postcommit(self, context, router_assoc): pass + def delete_router_assoc_precommit(self, context, router_assoc): + pass + def delete_router_assoc_postcommit(self, context, router_assoc): pass diff --git a/networking_bgpvpn/tests/unit/services/test_plugin.py b/networking_bgpvpn/tests/unit/services/test_plugin.py index c16904a7..9af6d173 100644 --- a/networking_bgpvpn/tests/unit/services/test_plugin.py +++ b/networking_bgpvpn/tests/unit/services/test_plugin.py @@ -518,6 +518,23 @@ class TestBGPVPNServiceDriverDB(BgpvpnTestCaseMixin): list = self._list('bgpvpn/bgpvpns', fmt='json') self.assertEqual([], list['bgpvpns']) + def test_delete_bgpvpn_precommit_fails(self): + with self.bgpvpn(do_delete=False) as bgpvpn, \ + mock.patch.object(bgpvpn_db.BGPVPNPluginDb, + 'delete_bgpvpn', + return_value=self.converted_data), \ + mock.patch.object(driver_api.BGPVPNDriver, + 'delete_bgpvpn_precommit', + new=self._raise_bgpvpn_driver_precommit_exc): + bgpvpn_req = self.new_delete_request('bgpvpn/bgpvpns', + bgpvpn['bgpvpn']['id']) + res = bgpvpn_req.get_response(self.ext_api) + self.assertEqual(webob.exc.HTTPError.code, + res.status_int) + # Assert that existing bgpvpn remains + list = self._list('bgpvpn/bgpvpns', fmt='json') + self.assertEqual([bgpvpn['bgpvpn']], list['bgpvpns']) + @mock.patch.object(driver_api.BGPVPNDriver, 'delete_bgpvpn_postcommit') def test_delete_bgpvpn(self, mock_delete_postcommit): @@ -697,6 +714,28 @@ class TestBGPVPNServiceDriverDB(BgpvpnTestCaseMixin): bgpvpn_id, mock.ANY, mock.ANY) + @mock.patch.object(driver_api.BGPVPNDriver, + 'delete_net_assoc_precommit') + @mock.patch.object(bgpvpn_db.BGPVPNPluginDb, 'delete_net_assoc') + def test_delete_bgpvpn_net_assoc_precommit_fails(self, mock_db_del, + mock_precommit): + with self.bgpvpn() as bgpvpn: + bgpvpn_id = bgpvpn['bgpvpn']['id'] + with self.network() as net: + net_id = net['network']['id'] + with self.assoc_net(bgpvpn_id, net_id=net_id) as assoc: + assoc_id = assoc['network_association']['id'] + net_assoc = {'id': assoc_id, + 'network_id': net_id, + 'bgpvpn_id': bgpvpn_id} + mock_db_del.return_value = net_assoc + mock_precommit.return_value = \ + self._raise_bgpvpn_driver_precommit_exc + # Assert that existing bgpvpn and net-assoc remains + list = self._list('bgpvpn/bgpvpns', fmt='json') + bgpvpn['bgpvpn']['networks'] = [net_assoc['network_id']] + self.assertEqual([bgpvpn['bgpvpn']], list['bgpvpns']) + @mock.patch.object(driver_api.BGPVPNDriver, 'delete_net_assoc_postcommit') @mock.patch.object(bgpvpn_db.BGPVPNPluginDb, 'delete_net_assoc') @@ -803,6 +842,28 @@ class TestBGPVPNServiceDriverDB(BgpvpnTestCaseMixin): bgpvpn_id, mock.ANY, mock.ANY) + @mock.patch.object(driver_api.BGPVPNDriver, + 'delete_router_assoc_precommit') + @mock.patch.object(bgpvpn_db.BGPVPNPluginDb, 'delete_router_assoc') + def test_delete_bgpvpn_router_assoc_precommit_fails(self, mock_db_del, + mock_precommit): + with self.bgpvpn() as bgpvpn: + bgpvpn_id = bgpvpn['bgpvpn']['id'] + with self.router(tenant_id=self._tenant_id) as router: + router_id = router['router']['id'] + with self.assoc_router(bgpvpn_id, router_id) as assoc: + assoc_id = assoc['router_association']['id'] + router_assoc = {'id': assoc_id, + 'router_id': router_id, + 'bgpvpn_id': bgpvpn_id} + mock_db_del.return_value = router_assoc + mock_precommit.return_value = \ + self._raise_bgpvpn_driver_precommit_exc + # Assert that existing bgpvpn and router-assoc remains + list = self._list('bgpvpn/bgpvpns', fmt='json') + bgpvpn['bgpvpn']['routers'] = [router_assoc['router_id']] + self.assertEqual([bgpvpn['bgpvpn']], list['bgpvpns']) + @mock.patch.object(driver_api.BGPVPNDriver, 'delete_router_assoc_postcommit') @mock.patch.object(bgpvpn_db.BGPVPNPluginDb, 'delete_router_assoc')

AltStyle によって変換されたページ (->オリジナル) /