diff --git a/neutron/common/exceptions.py b/neutron/common/exceptions.py index ffa396956a2..0b5b495bd15 100644 --- a/neutron/common/exceptions.py +++ b/neutron/common/exceptions.py @@ -194,6 +194,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 cbcc9fd8813..08541f486fb 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 e63eb710175..8a84f83a0d7 100644 --- a/neutron/objects/qos/rule.py +++ b/neutron/objects/qos/rule.py @@ -65,6 +65,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 @@ -110,6 +129,8 @@ class QosBandwidthLimitRule(QosRule): default=constants.EGRESS_DIRECTION) } + duplicates_compare_fields = ['direction'] + rule_type = qos_consts.RULE_TYPE_BANDWIDTH_LIMIT def obj_make_compatible(self, primitive, target_version): @@ -151,6 +172,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 945cc1df68c..fe699631196 100644 --- a/neutron/services/qos/qos_plugin.py +++ b/neutron/services/qos/qos_plugin.py @@ -314,6 +314,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) @@ -353,6 +354,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 e816baa250d..7e401114f99 100644 --- a/neutron/tests/unit/objects/qos/test_rule.py +++ b/neutron/tests/unit/objects/qos/test_rule.py @@ -137,6 +137,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): @@ -165,6 +185,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): @@ -193,6 +226,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 0962021bd22..d1c7faaa02b 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 @@ -412,7 +414,9 @@ class TestQosPlugin(base.BaseQosTestCase): @mock.patch.object(rule_object.QosBandwidthLimitRule, 'create') def test_create_policy_rule(self, mock_qos_rule_create, mock_qos_policy_get): - mock_qos_policy_get.return_value = self.policy + _policy = copy.copy(self.policy) + setattr(_policy, "rules", []) + mock_qos_policy_get.return_value = _policy mock_manager = mock.Mock() mock_manager.attach_mock(mock_qos_rule_create, 'create') mock_manager.attach_mock(self.qos_plugin.driver_manager, 'driver') @@ -477,6 +481,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()