]> review.fuel-infra Code Review - openstack-build/neutron-build.git/commitdiff
Use database session from the context in N1kv plugin
authorAbhishek Raut <abhraut@cisco.com>
Thu, 27 Feb 2014 03:38:42 +0000 (19:38 -0800)
committerAbhishek Raut <abhraut@cisco.com>
Mon, 3 Mar 2014 20:07:14 +0000 (12:07 -0800)
Avoid inconsistencies by using context.session for database
transactions wherever possible in the Cisco N1KV plugin.

Change-Id: Ic0784cbbf50beae6bb3b124c959ae90f3affb604
Closes-Bug: #1285473

neutron/plugins/cisco/db/n1kv_db_v2.py
neutron/tests/unit/cisco/n1kv/test_n1kv_db.py

index 01bd40966a332acf870c81d854f3acd7ca1ce601..886cfb8b81218481e740c97ebd00181599a0a34a 100644 (file)
@@ -885,7 +885,7 @@ def get_network_profile(db_session, id):
         raise c_exc.NetworkProfileNotFound(profile=id)
 
 
-def _get_network_profiles(**kwargs):
+def _get_network_profiles(db_session=None, physical_network=None):
     """
     Retrieve all network profiles.
 
@@ -893,10 +893,10 @@ def _get_network_profiles(**kwargs):
     network is specified. If no physical network is specified, return
     all network profiles.
     """
-    db_session = db.get_session()
-    if "physical_network" in kwargs:
+    db_session = db_session or db.get_session()
+    if physical_network:
         return (db_session.query(n1kv_models_v2.NetworkProfile).
-                filter_by(physical_network=kwargs["physical_network"]))
+                filter_by(physical_network=physical_network))
     return db_session.query(n1kv_models_v2.NetworkProfile)
 
 
@@ -939,15 +939,18 @@ def get_policy_profile(db_session, id):
         raise c_exc.PolicyProfileIdNotFound(profile_id=id)
 
 
-def create_profile_binding(tenant_id, profile_id, profile_type):
+def create_profile_binding(db_session, tenant_id, profile_id, profile_type):
     """Create Network/Policy Profile association with a tenant."""
+    db_session = db_session or db.get_session()
     if profile_type not in ["network", "policy"]:
         raise q_exc.NeutronException(_("Invalid profile type"))
 
-    if _profile_binding_exists(tenant_id, profile_id, profile_type):
-        return get_profile_binding(tenant_id, profile_id)
+    if _profile_binding_exists(db_session,
+                               tenant_id,
+                               profile_id,
+                               profile_type):
+        return get_profile_binding(db_session, tenant_id, profile_id)
 
-    db_session = db.get_session()
     with db_session.begin(subtransactions=True):
         binding = n1kv_models_v2.ProfileBinding(profile_type=profile_type,
                                                 profile_id=profile_id,
@@ -956,36 +959,29 @@ def create_profile_binding(tenant_id, profile_id, profile_type):
         return binding
 
 
-def _profile_binding_exists(tenant_id, profile_id, profile_type):
-    db_session = db.get_session()
+def _profile_binding_exists(db_session, tenant_id, profile_id, profile_type):
     LOG.debug(_("_profile_binding_exists()"))
     return (db_session.query(n1kv_models_v2.ProfileBinding).
             filter_by(tenant_id=tenant_id, profile_id=profile_id,
                       profile_type=profile_type).first())
 
 
-def _get_profile_binding(tenant_id, profile_id):
-    LOG.debug(_("_get_profile_binding"))
-    db_session = db.get_session()
-    return (db_session.query(n1kv_models_v2.ProfileBinding).filter_by(
-        tenant_id=tenant_id, profile_id=profile_id).one())
-
-
-def get_profile_binding(tenant_id, profile_id):
+def get_profile_binding(db_session, tenant_id, profile_id):
     """Get Network/Policy Profile - Tenant binding."""
     LOG.debug(_("get_profile_binding()"))
     try:
-        return _get_profile_binding(tenant_id, profile_id)
+        return (db_session.query(n1kv_models_v2.ProfileBinding).filter_by(
+            tenant_id=tenant_id, profile_id=profile_id).one())
     except exc.NoResultFound:
         c_exc.ProfileTenantBindingNotFound(profile_id=profile_id)
 
 
-def delete_profile_binding(tenant_id, profile_id):
+def delete_profile_binding(db_session, tenant_id, profile_id):
     """Delete Policy Binding."""
     LOG.debug(_("delete_profile_binding()"))
-    db_session = db.get_session()
+    db_session = db_session or db.get_session()
     try:
-        binding = get_profile_binding(tenant_id, profile_id)
+        binding = get_profile_binding(db_session, tenant_id, profile_id)
         with db_session.begin(subtransactions=True):
             db_session.delete(binding)
     except c_exc.ProfileTenantBindingNotFound:
@@ -995,7 +991,7 @@ def delete_profile_binding(tenant_id, profile_id):
         return
 
 
-def _get_profile_bindings(profile_type=None):
+def _get_profile_bindings(db_session, profile_type=None):
     """
     Retrieve a list of profile bindings.
 
@@ -1004,7 +1000,6 @@ def _get_profile_bindings(profile_type=None):
     profile types.
     """
     LOG.debug(_("_get_profile_bindings()"))
-    db_session = db.get_session()
     if profile_type:
         profile_bindings = (db_session.query(n1kv_models_v2.ProfileBinding).
                             filter_by(profile_type=profile_type))
@@ -1074,6 +1069,7 @@ class NetworkProfile_db_mixin(object):
         """
         if context.is_admin:
             profile_bindings = _get_profile_bindings(
+                context.session,
                 profile_type=c_const.NETWORK)
             return [self._make_profile_bindings_dict(pb)
                     for pb in profile_bindings]
@@ -1090,11 +1086,14 @@ class NetworkProfile_db_mixin(object):
         p = network_profile["network_profile"]
         self._validate_network_profile_args(context, p)
         net_profile = create_network_profile(context.session, p)
-        create_profile_binding(context.tenant_id,
+        create_profile_binding(context.session,
+                               context.tenant_id,
                                net_profile.id,
                                c_const.NETWORK)
         if p.get("add_tenant"):
-            self.add_network_profile_tenant(net_profile.id, p["add_tenant"])
+            self.add_network_profile_tenant(context.session,
+                                            net_profile.id,
+                                            p["add_tenant"])
         return self._make_network_profile_dict(net_profile)
 
     def delete_network_profile(self, context, id):
@@ -1121,11 +1120,13 @@ class NetworkProfile_db_mixin(object):
         """
         p = network_profile["network_profile"]
         if context.is_admin and "add_tenant" in p:
-            self.add_network_profile_tenant(id, p["add_tenant"])
+            self.add_network_profile_tenant(context.session,
+                                            id,
+                                            p["add_tenant"])
             return self._make_network_profile_dict(get_network_profile(
                 context.session, id))
         if context.is_admin and "remove_tenant" in p:
-            delete_profile_binding(p["remove_tenant"], id)
+            delete_profile_binding(context.session, p["remove_tenant"], id)
             return self._make_network_profile_dict(get_network_profile(
                 context.session, id))
         return self._make_network_profile_dict(
@@ -1170,15 +1171,20 @@ class NetworkProfile_db_mixin(object):
                                                        NetworkProfile,
                                                        context.tenant_id)
 
-    def add_network_profile_tenant(self, network_profile_id, tenant_id):
+    def add_network_profile_tenant(self,
+                                   db_session,
+                                   network_profile_id,
+                                   tenant_id):
         """
         Add a tenant to a network profile.
 
+        :param db_session: database session
         :param network_profile_id: UUID representing network profile
         :param tenant_id: UUID representing the tenant
         :returns: profile binding object
         """
-        return create_profile_binding(tenant_id,
+        return create_profile_binding(db_session,
+                                      tenant_id,
                                       network_profile_id,
                                       c_const.NETWORK)
 
@@ -1277,9 +1283,11 @@ class NetworkProfile_db_mixin(object):
         segment_type = net_p["segment_type"].lower()
         if segment_type == c_const.NETWORK_TYPE_VLAN:
             profiles = _get_network_profiles(
-                physical_network=net_p["physical_network"])
+                db_session=context.session,
+                physical_network=net_p["physical_network"]
+            )
         else:
-            profiles = _get_network_profiles()
+            profiles = _get_network_profiles(db_session=context.session)
         if profiles:
             for profile in profiles:
                 name = profile.name
@@ -1406,6 +1414,7 @@ class PolicyProfile_db_mixin(object):
         """
         if context.is_admin:
             profile_bindings = _get_profile_bindings(
+                context.session,
                 profile_type=c_const.POLICY)
             return [self._make_profile_bindings_dict(pb)
                     for pb in profile_bindings]
@@ -1423,25 +1432,32 @@ class PolicyProfile_db_mixin(object):
         """
         p = policy_profile["policy_profile"]
         if context.is_admin and "add_tenant" in p:
-            self.add_policy_profile_tenant(id, p["add_tenant"])
+            self.add_policy_profile_tenant(context.session,
+                                           id,
+                                           p["add_tenant"])
             return self._make_policy_profile_dict(get_policy_profile(
                 context.session, id))
         if context.is_admin and "remove_tenant" in p:
-            delete_profile_binding(p["remove_tenant"], id)
+            delete_profile_binding(context.session, p["remove_tenant"], id)
             return self._make_policy_profile_dict(get_policy_profile(
                 context.session, id))
         return self._make_policy_profile_dict(
             update_policy_profile(context.session, id, p))
 
-    def add_policy_profile_tenant(self, policy_profile_id, tenant_id):
+    def add_policy_profile_tenant(self,
+                                  db_session,
+                                  policy_profile_id,
+                                  tenant_id):
         """
         Add a tenant to a policy profile binding.
 
+        :param db_session: database session
         :param policy_profile_id: UUID representing policy profile
         :param tenant_id: UUID representing the tenant
         :returns: profile binding object
         """
-        return create_profile_binding(tenant_id,
+        return create_profile_binding(db_session,
+                                      tenant_id,
                                       policy_profile_id,
                                       c_const.POLICY)
 
@@ -1452,7 +1468,7 @@ class PolicyProfile_db_mixin(object):
         :param policy_profile_id: UUID representing policy profile
         :param tenant_id: UUID representing the tenant
         """
-        delete_profile_binding(tenant_id, policy_profile_id)
+        delete_profile_binding(None, tenant_id, policy_profile_id)
 
     def _delete_policy_profile(self, policy_profile_id):
         """Delete policy profile and associated binding."""
@@ -1515,4 +1531,7 @@ class PolicyProfile_db_mixin(object):
         tenant_id = tenant_id or c_const.TENANT_ID_NOT_SET
         if not self._policy_profile_exists(policy_profile_id):
             create_policy_profile(policy_profile)
-        create_profile_binding(tenant_id, policy_profile["id"], c_const.POLICY)
+        create_profile_binding(None,
+                               tenant_id,
+                               policy_profile["id"],
+                               c_const.POLICY)
index 6339c834ad0992076073bfb9bbe5deeaa2037546..de9f15e84eba4ec1998f2f6809edb1b6e834ef27 100644 (file)
@@ -103,9 +103,7 @@ class VlanAllocationsTest(base.BaseTestCase):
         db.configure_db()
         self.session = db.get_session()
         n1kv_db_v2.sync_vlan_allocations(self.session, VLAN_RANGES)
-
-    def tearDown(self):
-        super(VlanAllocationsTest, self).tearDown()
+        self.addCleanup(db.clear_db)
 
     def test_sync_vlan_allocations_outside_segment_range(self):
         self.assertRaises(c_exc.VlanIDNotFound,
@@ -287,9 +285,7 @@ class VxlanAllocationsTest(base.BaseTestCase,
         db.configure_db()
         self.session = db.get_session()
         n1kv_db_v2.sync_vxlan_allocations(self.session, VXLAN_RANGES)
-
-    def tearDown(self):
-        super(VxlanAllocationsTest, self).tearDown()
+        self.addCleanup(db.clear_db)
 
     def test_sync_vxlan_allocations_outside_segment_range(self):
         self.assertIsNone(n1kv_db_v2.get_vxlan_allocation(self.session,
@@ -394,9 +390,7 @@ class NetworkBindingsTest(test_plugin.NeutronDbPluginV2TestCase):
         super(NetworkBindingsTest, self).setUp()
         db.configure_db()
         self.session = db.get_session()
-
-    def tearDown(self):
-        super(NetworkBindingsTest, self).tearDown()
+        self.addCleanup(db.clear_db)
 
     def test_add_network_binding(self):
         with self.network() as network:
@@ -645,9 +639,7 @@ class NetworkProfileTests(base.BaseTestCase,
         super(NetworkProfileTests, self).setUp()
         db.configure_db()
         self.session = db.get_session()
-
-    def tearDown(self):
-        super(NetworkProfileTests, self).tearDown()
+        self.addCleanup(db.clear_db)
 
     def test_create_network_profile(self):
         _db_profile = n1kv_db_v2.create_network_profile(self.session,
@@ -820,7 +812,7 @@ class NetworkProfileTests(base.BaseTestCase,
         [n1kv_db_v2.create_network_profile(self.session, p)
          for p in test_profiles]
         # TODO(abhraut): Fix this test to work with real tenant_td
-        profiles = n1kv_db_v2._get_network_profiles()
+        profiles = n1kv_db_v2._get_network_profiles(db_session=self.session)
         self.assertEqual(len(test_profiles), len(list(profiles)))
 
 
@@ -830,9 +822,7 @@ class PolicyProfileTests(base.BaseTestCase):
         super(PolicyProfileTests, self).setUp()
         db.configure_db()
         self.session = db.get_session()
-
-    def tearDown(self):
-        super(PolicyProfileTests, self).tearDown()
+        self.addCleanup(db.clear_db)
 
     def test_create_policy_profile(self):
         _db_profile = n1kv_db_v2.create_policy_profile(TEST_POLICY_PROFILE)
@@ -878,9 +868,7 @@ class ProfileBindingTests(base.BaseTestCase,
         super(ProfileBindingTests, self).setUp()
         db.configure_db()
         self.session = db.get_session()
-
-    def tearDown(self):
-        super(ProfileBindingTests, self).tearDown()
+        self.addCleanup(db.clear_db)
 
     def _create_test_binding_if_not_there(self, tenant_id, profile_id,
                                           profile_type):
@@ -890,7 +878,8 @@ class ProfileBindingTests(base.BaseTestCase,
                                   tenant_id=tenant_id,
                                   profile_id=profile_id).one())
         except s_exc.NoResultFound:
-            _binding = n1kv_db_v2.create_profile_binding(tenant_id,
+            _binding = n1kv_db_v2.create_profile_binding(self.session,
+                                                         tenant_id,
                                                          profile_id,
                                                          profile_type)
         return _binding
@@ -899,7 +888,9 @@ class ProfileBindingTests(base.BaseTestCase,
         test_tenant_id = "d434dd90-76ec-11e2-bcfd-0800200c9a66"
         test_profile_id = "dd7b9741-76ec-11e2-bcfd-0800200c9a66"
         test_profile_type = "network"
-        n1kv_db_v2.create_profile_binding(test_tenant_id, test_profile_id,
+        n1kv_db_v2.create_profile_binding(self.session,
+                                          test_tenant_id,
+                                          test_profile_id,
                                           test_profile_type)
         try:
             self.session.query(n1kv_models_v2.ProfileBinding).filter_by(
@@ -918,7 +909,8 @@ class ProfileBindingTests(base.BaseTestCase,
         self._create_test_binding_if_not_there(test_tenant_id,
                                                test_profile_id,
                                                test_profile_type)
-        binding = n1kv_db_v2.get_profile_binding(test_tenant_id,
+        binding = n1kv_db_v2.get_profile_binding(self.session,
+                                                 test_tenant_id,
                                                  test_profile_id)
         self.assertEqual(binding.tenant_id, test_tenant_id)
         self.assertEqual(binding.profile_id, test_profile_id)
@@ -931,7 +923,9 @@ class ProfileBindingTests(base.BaseTestCase,
         self._create_test_binding_if_not_there(test_tenant_id,
                                                test_profile_id,
                                                test_profile_type)
-        n1kv_db_v2.delete_profile_binding(test_tenant_id, test_profile_id)
+        n1kv_db_v2.delete_profile_binding(self.session,
+                                          test_tenant_id,
+                                          test_profile_id)
         q = (self.session.query(n1kv_models_v2.ProfileBinding).filter_by(
              profile_type=test_profile_type,
              tenant_id=test_tenant_id,
@@ -943,15 +937,18 @@ class ProfileBindingTests(base.BaseTestCase,
         ctx.tenant_id = "d434dd90-76ec-11e2-bcfd-0800200c9a66"
         test_profile_id = "AAAAAAAA-76ec-11e2-bcfd-0800200c9a66"
         test_profile_type = "policy"
-        n1kv_db_v2.create_profile_binding(cisco_constants.TENANT_ID_NOT_SET,
+        n1kv_db_v2.create_profile_binding(self.session,
+                                          cisco_constants.TENANT_ID_NOT_SET,
                                           test_profile_id,
                                           test_profile_type)
         network_profile = {"network_profile": TEST_NETWORK_PROFILE}
         test_network_profile = self.create_network_profile(ctx,
                                                            network_profile)
-        binding = n1kv_db_v2.get_profile_binding(ctx.tenant_id,
+        binding = n1kv_db_v2.get_profile_binding(self.session,
+                                                 ctx.tenant_id,
                                                  test_profile_id)
         self.assertIsNone(n1kv_db_v2.get_profile_binding(
+            self.session,
             cisco_constants.TENANT_ID_NOT_SET,
             test_profile_id))
         self.assertNotEqual(binding.tenant_id,