x/vmware-nsx
Code Issues Proposed changes

Moves the HA resource creations outside of transaction

Browse Source 
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
This commit is contained in:
Sylvain Afchain 2014-09-26 13:49:43 +00:00
parent 0c1223862c
commit 4be52c0780
3 changed files with 48 additions and 39 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

21
neutron/db/l3_hamode_db.py
View File

@ -333,18 +333,19 @@ class L3_HA_NAT_db_mixin(l3_dvr_db.L3_NAT_with_dvr_db_mixin):
ha = cfg.CONF.l3_ha ha = cfg.CONF.l3_ha
return ha return ha
def _create_router_db(self, context, router, tenant_id): def create_router(self, context, router):
router['ha'] = self._is_ha(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() raise l3_ha.DistributedHARouterNotSupported()
with context.session.begin(subtransactions=True): router['router']['ha'] = is_ha
router_db = super(L3_HA_NAT_db_mixin, self)._create_router_db( router_dict = super(L3_HA_NAT_db_mixin,
context, router, tenant_id) self).create_router(context, router)
if router['ha']: if is_ha:
try: try:
router_db = self._get_router(context, router_dict['id'])
ha_network = self.get_ha_network(context, ha_network = self.get_ha_network(context,
router_db.tenant_id) router_db.tenant_id)
if not ha_network: 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) self._notify_ha_interfaces_updated(context, router_db.id)
except Exception: except Exception:
with excutils.save_and_reraise_exception(): with excutils.save_and_reraise_exception():
self.delete_router(context, router_db.id) self.delete_router(context, router_dict['id'])
router_dict['ha_vr_id'] = router_db.extra_attributes.ha_vr_id
return router_db return router_dict
def _update_router_db(self, context, router_id, data, gw_info): def _update_router_db(self, context, router_id, data, gw_info):
ha = data.pop('ha', None) ha = data.pop('ha', None)

61
neutron/tests/unit/db/test_l3_ha_db.py
View File

@ -55,12 +55,13 @@ class L3HATestFramework(testlib_api.SqlTestCase,
cfg.CONF.set_override('allow_overlapping_ips', True) cfg.CONF.set_override('allow_overlapping_ips', True)
def _create_router(self, ha=True, tenant_id='tenant1', distributed=None): 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} router = {'name': 'router1', 'admin_state_up': True}
if ha is not None: if ha is not None:
router['ha'] = ha router['ha'] = ha
if distributed is not None: if distributed is not None:
router['distributed'] = distributed 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): def _update_router(self, router_id, ha=True, distributed=None):
data = {'ha': ha} if ha is not None else {} data = {'ha': ha} if ha is not None else {}
@ -100,7 +101,7 @@ class L3HAGetSyncDataTestCase(L3HATestFramework):
def test_l3_agent_routers_query_interface(self): def test_l3_agent_routers_query_interface(self):
router = self._create_router() 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, routers = self.plugin.get_ha_sync_data_for_host(self.admin_ctx,
self.agent1['host']) self.agent1['host'])
self.assertEqual(1, len(routers)) self.assertEqual(1, len(routers))
@ -117,13 +118,13 @@ class L3HAGetSyncDataTestCase(L3HATestFramework):
def test_update_state(self): def test_update_state(self):
router = self._create_router() 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, routers = self.plugin.get_ha_sync_data_for_host(self.admin_ctx,
self.agent1['host']) self.agent1['host'])
state = routers[0].get(constants.HA_ROUTER_STATE_KEY) state = routers[0].get(constants.HA_ROUTER_STATE_KEY)
self.assertEqual('standby', state) 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']) self.agent1['host'])
routers = self.plugin.get_ha_sync_data_for_host(self.admin_ctx, 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): def test_ha_router_create(self):
router = self._create_router() router = self._create_router()
self.assertTrue(router.extra_attributes['ha']) self.assertTrue(router['ha'])
def test_ha_router_create_with_distributed(self): def test_ha_router_create_with_distributed(self):
self.assertRaises(l3_ext_ha_mode.DistributedHARouterNotSupported, self.assertRaises(l3_ext_ha_mode.DistributedHARouterNotSupported,
@ -172,37 +173,37 @@ class L3HATestCase(L3HATestFramework):
def test_no_ha_router_create(self): def test_no_ha_router_create(self):
router = self._create_router(ha=False) 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): def test_router_create_with_ha_conf_enabled(self):
cfg.CONF.set_override('l3_ha', True) cfg.CONF.set_override('l3_ha', True)
router = self._create_router(ha=None) router = self._create_router(ha=None)
self.assertTrue(router.extra_attributes['ha']) self.assertTrue(router['ha'])
def test_migration_from_ha(self): def test_migration_from_ha(self):
router = self._create_router() 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.assertFalse(router.extra_attributes['ha'])
self.assertIsNone(router.extra_attributes['ha_vr_id']) self.assertIsNone(router.extra_attributes['ha_vr_id'])
def test_migration_to_ha(self): def test_migration_to_ha(self):
router = self._create_router(ha=False) 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.assertTrue(router.extra_attributes['ha'])
self.assertIsNotNone(router.extra_attributes['ha_vr_id']) self.assertIsNotNone(router.extra_attributes['ha_vr_id'])
def test_migrate_ha_router_to_distributed(self): def test_migrate_ha_router_to_distributed(self):
router = self._create_router() router = self._create_router()
self.assertTrue(router.extra_attributes['ha']) self.assertTrue(router['ha'])
self.assertRaises(l3_ext_ha_mode.DistributedHARouterNotSupported, self.assertRaises(l3_ext_ha_mode.DistributedHARouterNotSupported,
self._update_router, self._update_router,
router.id, router['id'],
distributed=True) distributed=True)
def test_unique_ha_network_per_tenant(self): def test_unique_ha_network_per_tenant(self):
@ -247,7 +248,7 @@ class L3HATestCase(L3HATestFramework):
def test_update_router_to_ha_notifies_agent(self): def test_update_router_to_ha_notifies_agent(self):
router = self._create_router(ha=False) router = self._create_router(ha=False)
self.notif_m.reset_mock() 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) self.assertTrue(self.notif_m.called)
def test_unique_vr_id_between_routers(self): 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) @mock.patch('neutron.db.l3_hamode_db.MAX_ALLOCATION_TRIES', new=2)
def test_vr_id_allocation_contraint_conflict(self): def test_vr_id_allocation_contraint_conflict(self):
router = self._create_router() 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', with mock.patch.object(self.plugin, '_get_allocated_vr_id',
return_value=set()) as alloc: return_value=set()) as alloc:
self.assertRaises(l3_ext_ha_mode.MaxVRIDAllocationTriesReached, self.assertRaises(l3_ext_ha_mode.MaxVRIDAllocationTriesReached,
self.plugin._allocate_vr_id, self.admin_ctx, 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)) self.assertEqual(2, len(alloc.mock_calls))
def test_vr_id_allocation_delete_router(self): def test_vr_id_allocation_delete_router(self):
router = self._create_router() 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, allocs_before = self.plugin._get_allocated_vr_id(self.admin_ctx,
network.network_id) network.network_id)
@ -292,19 +295,20 @@ class L3HATestCase(L3HATestFramework):
network.network_id) network.network_id)
self.assertNotEqual(allocs_before, allocs_current) 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, allocs_after = self.plugin._get_allocated_vr_id(self.admin_ctx,
network.network_id) network.network_id)
self.assertEqual(allocs_before, allocs_after) self.assertEqual(allocs_before, allocs_after)
def test_vr_id_allocation_router_migration(self): def test_vr_id_allocation_router_migration(self):
router = self._create_router() 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, allocs_before = self.plugin._get_allocated_vr_id(self.admin_ctx,
network.network_id) network.network_id)
router = self._create_router() 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, allocs_after = self.plugin._get_allocated_vr_id(self.admin_ctx,
network.network_id) network.network_id)
self.assertEqual(allocs_before, allocs_after) self.assertEqual(allocs_before, allocs_after)
@ -321,16 +325,17 @@ class L3HATestCase(L3HATestFramework):
def test_add_ha_port_binding_failure_rolls_back_port(self): def test_add_ha_port_binding_failure_rolls_back_port(self):
router = self._create_router() router = self._create_router()
device_filter = {'device_id': [router.id]} device_filter = {'device_id': [router['id']]}
ports_before = self.core_plugin.get_ports( ports_before = self.core_plugin.get_ports(
self.admin_ctx, filters=device_filter) 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', with mock.patch.object(self.plugin, '_create_ha_port_binding',
side_effect=ValueError): side_effect=ValueError):
self.assertRaises(ValueError, self.plugin.add_ha_port, self.assertRaises(ValueError, self.plugin.add_ha_port,
self.admin_ctx, router.id, network.network_id, self.admin_ctx, router['id'], network.network_id,
router.tenant_id) router['tenant_id'])
ports_after = self.core_plugin.get_ports( ports_after = self.core_plugin.get_ports(
self.admin_ctx, filters=device_filter) self.admin_ctx, filters=device_filter)
@ -362,15 +367,17 @@ class L3HATestCase(L3HATestFramework):
def test_create_ha_interfaces_binding_failure_rolls_back_ports(self): def test_create_ha_interfaces_binding_failure_rolls_back_ports(self):
router = self._create_router() 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,
device_filter = {'device_id': [router.id]} router['tenant_id'])
device_filter = {'device_id': [router['id']]}
ports_before = self.core_plugin.get_ports( ports_before = self.core_plugin.get_ports(
self.admin_ctx, filters=device_filter) 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', with mock.patch.object(self.plugin, '_create_ha_port_binding',
side_effect=ValueError): side_effect=ValueError):
self.assertRaises(ValueError, self.plugin._create_ha_interfaces, 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( ports_after = self.core_plugin.get_ports(
self.admin_ctx, filters=device_filter) self.admin_ctx, filters=device_filter)

5
neutron/tests/unit/test_l3_schedulers.py
View File

@ -1056,11 +1056,12 @@ class L3HATestCaseMixin(testlib_api.SqlTestCase,
self._register_l3_agents() self._register_l3_agents()
def _create_ha_router(self, ha=True, tenant_id='tenant1'): def _create_ha_router(self, ha=True, tenant_id='tenant1'):
self.adminContext.tenant_id = tenant_id
router = {'name': 'router1', 'admin_state_up': True} router = {'name': 'router1', 'admin_state_up': True}
if ha is not None: if ha is not None:
router['ha'] = ha router['ha'] = ha
return self.plugin._create_router_db(self.adminContext, return self.plugin.create_router(self.adminContext,
router, tenant_id) {'router': router})
class L3_HA_scheduler_db_mixinTestCase(L3HATestCaseMixin): class L3_HA_scheduler_db_mixinTestCase(L3HATestCaseMixin):