openstack
/
neutron
Code Issues Proposed changes

Merge "Fix error message when duplicate QoS rule is created" into stable/pike

This commit is contained in:
Zuul 2018-03-08 14:29:07 +00:00 committed by Gerrit Code Review
parent 0bed6e24b4 30b86eb169
commit 7cb197fb9b
6 changed files with 124 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
neutron/common/exceptions.py
View File

@ -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.")

21
neutron/objects/qos/qos_policy_validator.py
View File

@ -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)

23
neutron/objects/qos/rule.py
View File

@ -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):

2
neutron/services/qos/qos_plugin.py
View File

@ -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)

50
neutron/tests/unit/objects/qos/test_rule.py
View File

@ -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):

23
neutron/tests/unit/services/qos/test_qos_plugin.py
View File

@ -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()