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

diff --git a/neutron/db/l3_hamode_db.py b/neutron/db/l3_hamode_db.py
index a0ed5808502bee3d6aaea0ba8f716b306085597a..2aa78bdea9bf51e42bc1904901f028d358424d32 100644 (file)
--- 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 4616612bbd861c8af52d97766a80fa7029827f5e..807436caa707573fae317562f29e8fd1caaae237 100644 (file)
--- 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 b3eb448591173a43830bc376e8649cc4bfa5874c..09346758e057136cc296735ee0162cfc4d9e2c18 100644 (file)
--- 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):