Merge "Fix error message when duplicate QoS rule is created" into stable/pike11.0.3

6 files changed, 124 insertions, 1 deletions

diff --git a/neutron/common/exceptions.py b/neutron/common/exceptions.py
index 9b8e5f36af..8b518db791 100644
--- a/neutron/common/exceptions.py
+++ b/neutron/common/exceptions.py
@@ -184,6 +184,12 @@ class QoSRuleParameterConflict(e.Conflict):
                 "%(existing_value)s.")
 
 
+class QoSRulesConflict(e.Conflict):
+    message = _("Rule %(new_rule_type)s conflicts with "
+                "rule %(rule_id)s which already exists in "
+                "QoS Policy %(policy_id)s.")
+
+
 class ExtensionsNotFound(e.NotFound):
     message = _("Extensions not found: %(extensions)s.")
 
diff --git a/neutron/objects/qos/qos_policy_validator.py b/neutron/objects/qos/qos_policy_validator.py
index 078a74ceb5..b375f994fe 100644
--- a/neutron/objects/qos/qos_policy_validator.py
+++ b/neutron/objects/qos/qos_policy_validator.py
@@ -45,3 +45,24 @@ def check_bandwidth_rule_conflict(policy, rule_data):
                         policy_id=policy["id"],
                         existing_rule=rule.rule_type,
                         existing_value=rule.max_kbps)
+
+
+def check_rules_conflict(policy, rule_obj):
+    """Implementation of the QoS Policy rules conflicts.
+
+    This function checks if the new rule to be associated with policy
+    doesn't have any duplicate rule already in policy.
+    Raises an exception if conflict is identified.
+    """
+
+    for rule in policy.rules:
+        # NOTE(slaweq): we don't want to raise exception when compared rules
+        # have got same id as it means that it is probably exactly the same
+        # rule so there is no conflict
+        if rule.id == getattr(rule_obj, "id", None):
+            continue
+        if rule.duplicates(rule_obj):
+            raise n_exc.QoSRulesConflict(
+                new_rule_type=rule_obj.rule_type,
+                rule_id=rule.id,
+                policy_id=policy.id)
diff --git a/neutron/objects/qos/rule.py b/neutron/objects/qos/rule.py
index f53f2ffcb8..3bcbbe2dfd 100644
--- a/neutron/objects/qos/rule.py
+++ b/neutron/objects/qos/rule.py
@@ -68,6 +68,25 @@ class QosRule(base.NeutronDbObject):
     # should be redefined in subclasses
     rule_type = None
 
+    duplicates_compare_fields = ()
+
+    def duplicates(self, other_rule):
+        """Returns True if rules have got same values in fields defined in
+        'duplicates_compare_fields' list.
+
+        In case when subclass don't have defined any field in
+        duplicates_compare_fields, only rule types are compared.
+        """
+
+        if self.rule_type != other_rule.rule_type:
+            return False
+
+        if self.duplicates_compare_fields:
+            for field in self.duplicates_compare_fields:
+                if getattr(self, field) != getattr(other_rule, field):
+                    return False
+        return True
+
     def to_dict(self):
         dict_ = super(QosRule, self).to_dict()
         dict_['type'] = self.rule_type
@@ -113,6 +132,8 @@ class QosBandwidthLimitRule(QosRule):
             default=n_const.EGRESS_DIRECTION)
     }
 
+    duplicates_compare_fields = ['direction']
+
     rule_type = qos_consts.RULE_TYPE_BANDWIDTH_LIMIT
 
     def obj_make_compatible(self, primitive, target_version):
@@ -154,6 +175,8 @@ class QosMinimumBandwidthRule(QosRule):
         'direction': common_types.FlowDirectionEnumField(),
     }
 
+    duplicates_compare_fields = ['direction']
+
     rule_type = qos_consts.RULE_TYPE_MINIMUM_BANDWIDTH
 
     def obj_make_compatible(self, primitive, target_version):
diff --git a/neutron/services/qos/qos_plugin.py b/neutron/services/qos/qos_plugin.py
index b84f34d764..6dc56aab39 100644
--- a/neutron/services/qos/qos_plugin.py
+++ b/neutron/services/qos/qos_plugin.py
@@ -317,6 +317,7 @@ class QoSPlugin(qos.QoSPluginBase):
             policy = self._get_policy_obj(context, policy_id)
             checker.check_bandwidth_rule_conflict(policy, rule_data)
             rule = rule_cls(context, qos_policy_id=policy_id, **rule_data)
+            checker.check_rules_conflict(policy, rule)
             rule.create()
             policy.obj_load_attr('rules')
             self.validate_policy(context, policy)
@@ -356,6 +357,7 @@ class QoSPlugin(qos.QoSPluginBase):
             policy.get_rule_by_id(rule_id)
             rule = rule_cls(context, id=rule_id)
             rule.update_fields(rule_data, reset_changes=True)
+            checker.check_rules_conflict(policy, rule)
             rule.update()
             policy.obj_load_attr('rules')
             self.validate_policy(context, policy)
diff --git a/neutron/tests/unit/objects/qos/test_rule.py b/neutron/tests/unit/objects/qos/test_rule.py
index 931bcb41e6..00b336d587 100644
--- a/neutron/tests/unit/objects/qos/test_rule.py
+++ b/neutron/tests/unit/objects/qos/test_rule.py
@@ -138,6 +138,26 @@ class QosBandwidthLimitRuleObjectTestCase(test_base.BaseObjectIfaceTestCase):
             exception.IncompatibleObjectVersion,
             rule_obj.obj_to_primitive, '1.2')
 
+    def test_duplicate_rules(self):
+        policy_id = uuidutils.generate_uuid()
+        ingress_rule_1 = rule.QosBandwidthLimitRule(
+            self.context, qos_policy_id=policy_id,
+            max_kbps=1000, max_burst=500,
+            direction=constants.INGRESS_DIRECTION)
+        ingress_rule_2 = rule.QosBandwidthLimitRule(
+            self.context, qos_policy_id=policy_id,
+            max_kbps=2000, max_burst=500,
+            direction=constants.INGRESS_DIRECTION)
+        egress_rule = rule.QosBandwidthLimitRule(
+            self.context, qos_policy_id=policy_id,
+            max_kbps=1000, max_burst=500,
+            direction=constants.EGRESS_DIRECTION)
+        dscp_rule = rule.QosDscpMarkingRule(
+            self.context, qos_policy_id=policy_id, dscp_mark=16)
+        self.assertTrue(ingress_rule_1.duplicates(ingress_rule_2))
+        self.assertFalse(ingress_rule_1.duplicates(egress_rule))
+        self.assertFalse(ingress_rule_1.duplicates(dscp_rule))
+
 
 class QosBandwidthLimitRuleDbObjectTestCase(test_base.BaseDbObjectTestCase,
                                             testlib_api.SqlTestCase):
@@ -166,6 +186,19 @@ class QosDscpMarkingRuleObjectTestCase(test_base.BaseObjectIfaceTestCase):
         self.assertRaises(exception.IncompatibleObjectVersion,
                      dscp_rule.obj_to_primitive, '1.0')
 
+    def test_duplicate_rules(self):
+        policy_id = uuidutils.generate_uuid()
+        dscp_rule_1 = rule.QosDscpMarkingRule(
+            self.context, qos_policy_id=policy_id, dscp_mark=16)
+        dscp_rule_2 = rule.QosDscpMarkingRule(
+            self.context, qos_policy_id=policy_id, dscp_mark=32)
+        bw_limit_rule = rule.QosBandwidthLimitRule(
+            self.context, qos_policy_id=policy_id,
+            max_kbps=1000, max_burst=500,
+            direction=constants.EGRESS_DIRECTION)
+        self.assertTrue(dscp_rule_1.duplicates(dscp_rule_2))
+        self.assertFalse(dscp_rule_1.duplicates(bw_limit_rule))
+
 
 class QosDscpMarkingRuleDbObjectTestCase(test_base.BaseDbObjectTestCase,
                                          testlib_api.SqlTestCase):
@@ -194,6 +227,23 @@ class QosMinimumBandwidthRuleObjectTestCase(test_base.BaseObjectIfaceTestCase):
             self.assertRaises(exception.IncompatibleObjectVersion,
                               min_bw_rule.obj_to_primitive, version)
 
+    def test_duplicate_rules(self):
+        policy_id = uuidutils.generate_uuid()
+        ingress_rule_1 = rule.QosMinimumBandwidthRule(
+            self.context, qos_policy_id=policy_id,
+            min_kbps=1000, direction=constants.INGRESS_DIRECTION)
+        ingress_rule_2 = rule.QosMinimumBandwidthRule(
+            self.context, qos_policy_id=policy_id,
+            min_kbps=2000, direction=constants.INGRESS_DIRECTION)
+        egress_rule = rule.QosMinimumBandwidthRule(
+            self.context, qos_policy_id=policy_id,
+            min_kbps=1000, direction=constants.EGRESS_DIRECTION)
+        dscp_rule = rule.QosDscpMarkingRule(
+            self.context, qos_policy_id=policy_id, dscp_mark=16)
+        self.assertTrue(ingress_rule_1.duplicates(ingress_rule_2))
+        self.assertFalse(ingress_rule_1.duplicates(egress_rule))
+        self.assertFalse(ingress_rule_1.duplicates(dscp_rule))
+
 
 class QosMinimumBandwidthRuleDbObjectTestCase(test_base.BaseDbObjectTestCase,
                                               testlib_api.SqlTestCase):
diff --git a/neutron/tests/unit/services/qos/test_qos_plugin.py b/neutron/tests/unit/services/qos/test_qos_plugin.py
index c9edd85c42..78ce4bd4a6 100644
--- a/neutron/tests/unit/services/qos/test_qos_plugin.py
+++ b/neutron/tests/unit/services/qos/test_qos_plugin.py
@@ -10,6 +10,8 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+import copy
+
 import mock
 from neutron_lib import context
 from neutron_lib import exceptions as lib_exc
@@ -422,8 +424,10 @@ class TestQosPlugin(base.BaseQosTestCase):
             mock_manager.mock_calls.index(delete_mock_call))
 
     def test_create_policy_rule(self):
+        _policy = copy.copy(self.policy)
+        setattr(_policy, "rules", [])
         with mock.patch('neutron.objects.qos.policy.QosPolicy.get_object',
-                        return_value=self.policy), mock.patch(
+                        return_value=_policy), mock.patch(
             'neutron.objects.qos.qos_policy_validator'
             '.check_bandwidth_rule_conflict', return_value=None):
             self.qos_plugin.create_policy_bandwidth_limit_rule(
@@ -474,6 +478,23 @@ class TestQosPlugin(base.BaseQosTestCase):
                 self.ctxt, self.policy.id, self.rule_data)
             mock_qos_get_obj.assert_called_once_with(self.ctxt, id=_policy.id)
 
+    def test_create_policy_rule_duplicates(self):
+        _policy = self._get_policy()
+        setattr(_policy, "rules", [self.rule])
+        new_rule_data = {
+            'bandwidth_limit_rule': {
+                'max_kbps': 5000,
+                'direction': self.rule.direction
+            }
+        }
+        with mock.patch('neutron.objects.qos.policy.QosPolicy.get_object',
+                        return_value=_policy) as mock_qos_get_obj:
+            self.assertRaises(
+                n_exc.QoSRulesConflict,
+                self.qos_plugin.create_policy_bandwidth_limit_rule,
+                self.ctxt, _policy.id, new_rule_data)
+            mock_qos_get_obj.assert_called_once_with(self.ctxt, id=_policy.id)
+
     @mock.patch.object(rule_object.QosBandwidthLimitRule, 'update')
     def test_update_policy_rule(self, mock_qos_rule_update):
         mock_manager = mock.Mock()