policy: made attach_* and detach_* methods more robust

Handle cases when a policy or a port or a network are not in the
database without exposing database level exceptions to object consumers.

Change-Id: I06a0b5c4f474b370072f2b6a13146f17a51eb847

diff --git a/neutron/common/exceptions.py b/neutron/common/exceptions.py
index 163dd9818278d2390cfe09c6afb4d45520f943c0..b0c434050952351c652a068660f77ce6505dc80f 100644 (file)
--- a/neutron/common/exceptions.py
+++ b/neutron/common/exceptions.py
@@ -94,6 +94,16 @@ class PortNotFoundOnNetwork(NotFound):
                 "on network %(net_id)s")
 
 
+class PortQosBindingNotFound(NotFound):
+    message = _("QoS binding for port %(port_id)s and policy %(policy_id)s "
+                "could not be found")
+
+
+class NetworkQosBindingNotFound(NotFound):
+    message = _("QoS binding for network %(net_id)s and policy %(policy_id)s "
+                "could not be found")
+
+
 class PolicyFileNotFound(NotFound):
     message = _("Policy configuration policy.json could not be found")
 

diff --git a/neutron/db/qos/api.py b/neutron/db/qos/api.py
index 40b8ab77b8e80395beb7fac87a7a1c6b6be331cd..cdc4bb44cdd34741fa3cdb019b18b3490a4c589b 100644 (file)
--- a/neutron/db/qos/api.py
+++ b/neutron/db/qos/api.py
@@ -10,35 +10,56 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+from oslo_db import exception as oslo_db_exception
+from sqlalchemy.orm import exc as orm_exc
+
+from neutron.common import exceptions as n_exc
 from neutron.db import common_db_mixin as db
 from neutron.db.qos import models
 
 
 def create_policy_network_binding(context, policy_id, network_id):
-    with context.session.begin(subtransactions=True):
-        db_obj = models.QosNetworkPolicyBinding(policy_id=policy_id,
-                                                network_id=network_id)
-        context.session.add(db_obj)
+    try:
+        with context.session.begin(subtransactions=True):
+            db_obj = models.QosNetworkPolicyBinding(policy_id=policy_id,
+                                                    network_id=network_id)
+            context.session.add(db_obj)
+    except oslo_db_exception.DBReferenceError:
+        raise n_exc.NetworkQosBindingNotFound(net_id=network_id,
+                                              policy_id=policy_id)
 
 
 def delete_policy_network_binding(context, policy_id, network_id):
-    with context.session.begin(subtransactions=True):
-        db_object = (db.model_query(context, models.QosNetworkPolicyBinding)
-                     .filter_by(policy_id=policy_id,
-                                network_id=network_id).one())
-        context.session.delete(db_object)
+    try:
+        with context.session.begin(subtransactions=True):
+            db_object = (db.model_query(context,
+                                        models.QosNetworkPolicyBinding)
+                         .filter_by(policy_id=policy_id,
+                                    network_id=network_id).one())
+            context.session.delete(db_object)
+    except orm_exc.NoResultFound:
+        raise n_exc.NetworkQosBindingNotFound(net_id=network_id,
+                                              policy_id=policy_id)
 
 
 def create_policy_port_binding(context, policy_id, port_id):
-    with context.session.begin(subtransactions=True):
-        db_obj = models.QosPortPolicyBinding(policy_id=policy_id,
-                                             port_id=port_id)
-        context.session.add(db_obj)
+    try:
+        with context.session.begin(subtransactions=True):
+            db_obj = models.QosPortPolicyBinding(policy_id=policy_id,
+                                                 port_id=port_id)
+            context.session.add(db_obj)
+    except oslo_db_exception.DBReferenceError:
+        raise n_exc.PortQosBindingNotFound(port_id=port_id,
+                                           policy_id=policy_id)
 
 
 def delete_policy_port_binding(context, policy_id, port_id):
-    with context.session.begin(subtransactions=True):
-        db_object = (db.model_query(context, models.QosPortPolicyBinding)
-                     .filter_by(policy_id=policy_id,
-                                port_id=port_id).one())
-        context.session.delete(db_object)
+    try:
+        with context.session.begin(subtransactions=True):
+            db_object = (db.model_query(context, models.QosPortPolicyBinding)
+                         .filter_by(policy_id=policy_id,
+                                    port_id=port_id).one())
+            context.session.delete(db_object)
+    except orm_exc.NoResultFound:
+        raise n_exc.PortQosBindingNotFound(port_id=port_id,
+                                           policy_id=policy_id)

diff --git a/neutron/objects/qos/policy.py b/neutron/objects/qos/policy.py
index 8f2c605c8e021a362dccf43286e50514617ca832..a5938d948736a0b1d3f08786b7f1b63dad99d998 100644 (file)
--- a/neutron/objects/qos/policy.py
+++ b/neutron/objects/qos/policy.py
@@ -103,7 +103,6 @@ class QosPolicy(base.NeutronObject):
     def _get_object_policy(cls, context, model, **kwargs):
         with db_api.autonested_transaction(context.session):
             binding_db_obj = db_api.get_object(context, model, **kwargs)
-            # TODO(QoS): rethink handling missing binding case
             if binding_db_obj:
                 return cls.get_by_id(context, binding_db_obj['policy_id'])
 

diff --git a/neutron/tests/unit/objects/qos/test_policy.py b/neutron/tests/unit/objects/qos/test_policy.py
index afd6a79829b22356452d2c31e45fa9ba46055913..ed8a1bf55b831250a7b3cfc3cd6afcd6827d07be 100644 (file)
--- a/neutron/tests/unit/objects/qos/test_policy.py
+++ b/neutron/tests/unit/objects/qos/test_policy.py
@@ -12,6 +12,7 @@
 
 import mock
 
+from neutron.common import exceptions as n_exc
 from neutron.db import api as db_api
 from neutron.db import models_v2
 from neutron.objects.qos import policy
@@ -78,10 +79,6 @@ class QosPolicyDbObjectTestCase(test_base.BaseDbObjectTestCase,
         super(QosPolicyDbObjectTestCase, self).setUp()
         self._create_test_network()
         self._create_test_port(self._network)
-        #TODO(QoS): move _create_test_policy here, as it's common
-        #           to all. Now the base DB Object test case breaks
-        #           that by introducing a duplicate object colliding
-        #           on PK.
 
     def _create_test_policy(self):
         policy_obj = policy.QosPolicy(self.context, **self.db_obj)
@@ -135,6 +132,30 @@ class QosPolicyDbObjectTestCase(test_base.BaseDbObjectTestCase,
                                                          self._network['id'])
         self.assertEqual(obj, policy_obj)
 
+    def test_attach_network_nonexistent_network(self):
+
+        obj = self._create_test_policy()
+        self.assertRaises(n_exc.NetworkQosBindingNotFound,
+                          obj.attach_network, 'non-existent-network')
+
+    def test_attach_port_nonexistent_port(self):
+
+        obj = self._create_test_policy()
+        self.assertRaises(n_exc.PortQosBindingNotFound,
+                          obj.attach_port, 'non-existent-port')
+
+    def test_attach_network_nonexistent_policy(self):
+
+        policy_obj = policy.QosPolicy(self.context, **self.db_obj)
+        self.assertRaises(n_exc.NetworkQosBindingNotFound,
+                          policy_obj.attach_network, self._network['id'])
+
+    def test_attach_port_nonexistent_policy(self):
+
+        policy_obj = policy.QosPolicy(self.context, **self.db_obj)
+        self.assertRaises(n_exc.PortQosBindingNotFound,
+                          policy_obj.attach_port, self._port['id'])
+
     def test_attach_port_get_port_policy(self):
 
         obj = self._create_test_policy()
@@ -169,6 +190,26 @@ class QosPolicyDbObjectTestCase(test_base.BaseDbObjectTestCase,
                                                          self._network['id'])
         self.assertIsNone(policy_obj)
 
+    def test_detach_port_nonexistent_port(self):
+        obj = self._create_test_policy()
+        self.assertRaises(n_exc.PortQosBindingNotFound,
+                          obj.detach_port, 'non-existent-port')
+
+    def test_detach_network_nonexistent_network(self):
+        obj = self._create_test_policy()
+        self.assertRaises(n_exc.NetworkQosBindingNotFound,
+                          obj.detach_network, 'non-existent-port')
+
+    def test_detach_port_nonexistent_policy(self):
+        policy_obj = policy.QosPolicy(self.context, **self.db_obj)
+        self.assertRaises(n_exc.PortQosBindingNotFound,
+                          policy_obj.detach_port, self._port['id'])
+
+    def test_detach_network_nonexistent_policy(self):
+        policy_obj = policy.QosPolicy(self.context, **self.db_obj)
+        self.assertRaises(n_exc.NetworkQosBindingNotFound,
+                          policy_obj.detach_network, self._network['id'])
+
     def test_synthetic_rule_fields(self):
         policy_obj, rule_obj = self._create_test_policy_with_rule()
         policy_obj = policy.QosPolicy.get_by_id(self.context, policy_obj.id)