From ccd650732729451aa8e5ce3401f9570c70c4f066 Mon Sep 17 00:00:00 2001 From: Sylvain Afchain Date: Fri, 26 Sep 2014 13:49:43 +0000 Subject: [PATCH] Moves the HA resource creations outside of transaction Currently the HA resources are created in the _create_router_db which includes calls to the plugin and generates RPC calls. Even if the resource creations are outside of any transaction from the _create_router_db point of view, this method is called in a transaction from the create_router method. This patch moves the resource creations to the create_router method outside the transaction. The failures are handled as previously with a try/expect. Change-Id: If8fcfd012f8e992175e49bbefb2ae667881a620a Closes-bug: #1374461 --- neutron/db/l3_hamode_db.py | 21 ++++---- neutron/tests/unit/db/test_l3_ha_db.py | 61 +++++++++++++----------- neutron/tests/unit/test_l3_schedulers.py | 5 +- 3 files changed, 48 insertions(+), 39 deletions(-) diff --git a/neutron/db/l3_hamode_db.py b/neutron/db/l3_hamode_db.py index a0ed58085..2aa78bdea 100644 --- a/neutron/db/l3_hamode_db.py +++ b/neutron/db/l3_hamode_db.py @@ -333,18 +333,19 @@ class L3_HA_NAT_db_mixin(l3_dvr_db.L3_NAT_with_dvr_db_mixin): ha = cfg.CONF.l3_ha return ha - def _create_router_db(self, context, router, tenant_id): - router['ha'] = self._is_ha(router) + def create_router(self, context, router): + is_ha = self._is_ha(router['router']) - if router['ha'] and l3_dvr_db.is_distributed_router(router): + if is_ha and l3_dvr_db.is_distributed_router(router['router']): raise l3_ha.DistributedHARouterNotSupported() - with context.session.begin(subtransactions=True): - router_db = super(L3_HA_NAT_db_mixin, self)._create_router_db( - context, router, tenant_id) + router['router']['ha'] = is_ha + router_dict = super(L3_HA_NAT_db_mixin, + self).create_router(context, router) - if router['ha']: + if is_ha: try: + router_db = self._get_router(context, router_dict['id']) ha_network = self.get_ha_network(context, router_db.tenant_id) if not ha_network: @@ -356,9 +357,9 @@ class L3_HA_NAT_db_mixin(l3_dvr_db.L3_NAT_with_dvr_db_mixin): self._notify_ha_interfaces_updated(context, router_db.id) except Exception: with excutils.save_and_reraise_exception(): - self.delete_router(context, router_db.id) - - return router_db + self.delete_router(context, router_dict['id']) + router_dict['ha_vr_id'] = router_db.extra_attributes.ha_vr_id + return router_dict def _update_router_db(self, context, router_id, data, gw_info): ha = data.pop('ha', None) diff --git a/neutron/tests/unit/db/test_l3_ha_db.py b/neutron/tests/unit/db/test_l3_ha_db.py index 4616612bb..807436caa 100644 --- a/neutron/tests/unit/db/test_l3_ha_db.py +++ b/neutron/tests/unit/db/test_l3_ha_db.py @@ -55,12 +55,13 @@ class L3HATestFramework(testlib_api.SqlTestCase, cfg.CONF.set_override('allow_overlapping_ips', True) def _create_router(self, ha=True, tenant_id='tenant1', distributed=None): + self.admin_ctx.tenant_id = tenant_id router = {'name': 'router1', 'admin_state_up': True} if ha is not None: router['ha'] = ha if distributed is not None: router['distributed'] = distributed - return self.plugin._create_router_db(self.admin_ctx, router, tenant_id) + return self.plugin.create_router(self.admin_ctx, {'router': router}) def _update_router(self, router_id, ha=True, distributed=None): data = {'ha': ha} if ha is not None else {} @@ -100,7 +101,7 @@ class L3HAGetSyncDataTestCase(L3HATestFramework): def test_l3_agent_routers_query_interface(self): router = self._create_router() - self._bind_router(router.id) + self._bind_router(router['id']) routers = self.plugin.get_ha_sync_data_for_host(self.admin_ctx, self.agent1['host']) self.assertEqual(1, len(routers)) @@ -117,13 +118,13 @@ class L3HAGetSyncDataTestCase(L3HATestFramework): def test_update_state(self): router = self._create_router() - self._bind_router(router.id) + self._bind_router(router['id']) routers = self.plugin.get_ha_sync_data_for_host(self.admin_ctx, self.agent1['host']) state = routers[0].get(constants.HA_ROUTER_STATE_KEY) self.assertEqual('standby', state) - self.plugin.update_router_state(self.admin_ctx, router.id, 'active', + self.plugin.update_router_state(self.admin_ctx, router['id'], 'active', self.agent1['host']) routers = self.plugin.get_ha_sync_data_for_host(self.admin_ctx, @@ -163,7 +164,7 @@ class L3HATestCase(L3HATestFramework): def test_ha_router_create(self): router = self._create_router() - self.assertTrue(router.extra_attributes['ha']) + self.assertTrue(router['ha']) def test_ha_router_create_with_distributed(self): self.assertRaises(l3_ext_ha_mode.DistributedHARouterNotSupported, @@ -172,37 +173,37 @@ class L3HATestCase(L3HATestFramework): def test_no_ha_router_create(self): router = self._create_router(ha=False) - self.assertFalse(router.extra_attributes['ha']) + self.assertFalse(router['ha']) def test_router_create_with_ha_conf_enabled(self): cfg.CONF.set_override('l3_ha', True) router = self._create_router(ha=None) - self.assertTrue(router.extra_attributes['ha']) + self.assertTrue(router['ha']) def test_migration_from_ha(self): router = self._create_router() - self.assertTrue(router.extra_attributes['ha']) + self.assertTrue(router['ha']) - router = self._update_router(router.id, ha=False) + router = self._update_router(router['id'], ha=False) self.assertFalse(router.extra_attributes['ha']) self.assertIsNone(router.extra_attributes['ha_vr_id']) def test_migration_to_ha(self): router = self._create_router(ha=False) - self.assertFalse(router.extra_attributes['ha']) + self.assertFalse(router['ha']) - router = self._update_router(router.id, ha=True) + router = self._update_router(router['id'], ha=True) self.assertTrue(router.extra_attributes['ha']) self.assertIsNotNone(router.extra_attributes['ha_vr_id']) def test_migrate_ha_router_to_distributed(self): router = self._create_router() - self.assertTrue(router.extra_attributes['ha']) + self.assertTrue(router['ha']) self.assertRaises(l3_ext_ha_mode.DistributedHARouterNotSupported, self._update_router, - router.id, + router['id'], distributed=True) def test_unique_ha_network_per_tenant(self): @@ -247,7 +248,7 @@ class L3HATestCase(L3HATestFramework): def test_update_router_to_ha_notifies_agent(self): router = self._create_router(ha=False) self.notif_m.reset_mock() - self._update_router(router.id, ha=True) + self._update_router(router['id'], ha=True) self.assertTrue(self.notif_m.called) def test_unique_vr_id_between_routers(self): @@ -272,18 +273,20 @@ class L3HATestCase(L3HATestFramework): @mock.patch('neutron.db.l3_hamode_db.MAX_ALLOCATION_TRIES', new=2) def test_vr_id_allocation_contraint_conflict(self): router = self._create_router() - network = self.plugin.get_ha_network(self.admin_ctx, router.tenant_id) + network = self.plugin.get_ha_network(self.admin_ctx, + router['tenant_id']) with mock.patch.object(self.plugin, '_get_allocated_vr_id', return_value=set()) as alloc: self.assertRaises(l3_ext_ha_mode.MaxVRIDAllocationTriesReached, self.plugin._allocate_vr_id, self.admin_ctx, - network.network_id, router.id) + network.network_id, router['id']) self.assertEqual(2, len(alloc.mock_calls)) def test_vr_id_allocation_delete_router(self): router = self._create_router() - network = self.plugin.get_ha_network(self.admin_ctx, router.tenant_id) + network = self.plugin.get_ha_network(self.admin_ctx, + router['tenant_id']) allocs_before = self.plugin._get_allocated_vr_id(self.admin_ctx, network.network_id) @@ -292,19 +295,20 @@ class L3HATestCase(L3HATestFramework): network.network_id) self.assertNotEqual(allocs_before, allocs_current) - self.plugin.delete_router(self.admin_ctx, router.id) + self.plugin.delete_router(self.admin_ctx, router['id']) allocs_after = self.plugin._get_allocated_vr_id(self.admin_ctx, network.network_id) self.assertEqual(allocs_before, allocs_after) def test_vr_id_allocation_router_migration(self): router = self._create_router() - network = self.plugin.get_ha_network(self.admin_ctx, router.tenant_id) + network = self.plugin.get_ha_network(self.admin_ctx, + router['tenant_id']) allocs_before = self.plugin._get_allocated_vr_id(self.admin_ctx, network.network_id) router = self._create_router() - self._update_router(router.id, ha=False) + self._update_router(router['id'], ha=False) allocs_after = self.plugin._get_allocated_vr_id(self.admin_ctx, network.network_id) self.assertEqual(allocs_before, allocs_after) @@ -321,16 +325,17 @@ class L3HATestCase(L3HATestFramework): def test_add_ha_port_binding_failure_rolls_back_port(self): router = self._create_router() - device_filter = {'device_id': [router.id]} + device_filter = {'device_id': [router['id']]} ports_before = self.core_plugin.get_ports( self.admin_ctx, filters=device_filter) - network = self.plugin.get_ha_network(self.admin_ctx, router.tenant_id) + network = self.plugin.get_ha_network(self.admin_ctx, + router['tenant_id']) with mock.patch.object(self.plugin, '_create_ha_port_binding', side_effect=ValueError): self.assertRaises(ValueError, self.plugin.add_ha_port, - self.admin_ctx, router.id, network.network_id, - router.tenant_id) + self.admin_ctx, router['id'], network.network_id, + router['tenant_id']) ports_after = self.core_plugin.get_ports( self.admin_ctx, filters=device_filter) @@ -362,15 +367,17 @@ class L3HATestCase(L3HATestFramework): def test_create_ha_interfaces_binding_failure_rolls_back_ports(self): router = self._create_router() - network = self.plugin.get_ha_network(self.admin_ctx, router.tenant_id) - device_filter = {'device_id': [router.id]} + network = self.plugin.get_ha_network(self.admin_ctx, + router['tenant_id']) + device_filter = {'device_id': [router['id']]} ports_before = self.core_plugin.get_ports( self.admin_ctx, filters=device_filter) + router_db = self.plugin._get_router(self.admin_ctx, router['id']) with mock.patch.object(self.plugin, '_create_ha_port_binding', side_effect=ValueError): self.assertRaises(ValueError, self.plugin._create_ha_interfaces, - self.admin_ctx, router, network) + self.admin_ctx, router_db, network) ports_after = self.core_plugin.get_ports( self.admin_ctx, filters=device_filter) diff --git a/neutron/tests/unit/test_l3_schedulers.py b/neutron/tests/unit/test_l3_schedulers.py index b3eb44859..09346758e 100644 --- a/neutron/tests/unit/test_l3_schedulers.py +++ b/neutron/tests/unit/test_l3_schedulers.py @@ -1056,11 +1056,12 @@ class L3HATestCaseMixin(testlib_api.SqlTestCase, self._register_l3_agents() def _create_ha_router(self, ha=True, tenant_id='tenant1'): + self.adminContext.tenant_id = tenant_id router = {'name': 'router1', 'admin_state_up': True} if ha is not None: router['ha'] = ha - return self.plugin._create_router_db(self.adminContext, - router, tenant_id) + return self.plugin.create_router(self.adminContext, + {'router': router}) class L3_HA_scheduler_db_mixinTestCase(L3HATestCaseMixin): -- 2.45.2