From 25f67e66ae92c989fda5d4e6c5db44eef5b178d2 Mon Sep 17 00:00:00 2001 From: Adit Sarfaty Date: Sun, 18 Jun 2017 15:40:36 +0300 Subject: [PATCH] NSX|V3: Use QoS precommit callback to validate rules Commit I44dd7e049eab054363063021f07ade81ef2d1a90 introduced the qos driver precommit callbacks. The NSX-V3 uses this callback to validate the BW rule limit and raise an exception if the values are not supported by the backend. Change-Id: Ie47c147cb5bb5ece01d8c272544994eacd295c52 --- vmware_nsx/services/qos/nsx_v3/driver.py | 6 ++ vmware_nsx/services/qos/nsx_v3/utils.py | 24 ++++--- .../services/qos/test_nsxv3_notification.py | 69 +++++-------------- 3 files changed, 38 insertions(+), 61 deletions(-) diff --git a/vmware_nsx/services/qos/nsx_v3/driver.py b/vmware_nsx/services/qos/nsx_v3/driver.py index 1c3478d83f..2af7b33beb 100644 --- a/vmware_nsx/services/qos/nsx_v3/driver.py +++ b/vmware_nsx/services/qos/nsx_v3/driver.py @@ -85,6 +85,12 @@ class NSXv3QosDriver(base.DriverBase): def delete_policy(self, context, policy): self.handler.delete_policy(context, policy.id) + def update_policy_precommit(self, context, policy): + """Validate rules values, before creation""" + if (hasattr(policy, "rules")): + for rule in policy["rules"]: + self.handler.validate_policy_rule(context, policy.id, rule) + def register(): """Register the NSX-V3 QoS driver.""" diff --git a/vmware_nsx/services/qos/nsx_v3/utils.py b/vmware_nsx/services/qos/nsx_v3/utils.py index f54b25f12c..4959de88dc 100644 --- a/vmware_nsx/services/qos/nsx_v3/utils.py +++ b/vmware_nsx/services/qos/nsx_v3/utils.py @@ -17,7 +17,9 @@ from oslo_config import cfg from oslo_log import log as logging +from neutron.services.qos import qos_consts from neutron_lib.api import validators +from neutron_lib import exceptions as n_exc from neutron_lib.plugins import directory from vmware_nsx._i18n import _ @@ -95,18 +97,18 @@ class QosNotificationsHandler(object): # Validate the max bandwidth value minimum value # (max value is above what neutron allows so no need to check it) if (bw_rule.max_kbps < MAX_KBPS_MIN_VALUE): - LOG.warning("Invalid input for max_kbps. " - "The minimal legal value is %s", - MAX_KBPS_MIN_VALUE) - bw_rule.max_kbps = MAX_KBPS_MIN_VALUE + msg = (_("Invalid input for max_kbps. " + "The minimal legal value is %s") % MAX_KBPS_MIN_VALUE) + LOG.error(msg) + raise n_exc.InvalidInput(error_message=msg) # validate the burst size value max value # (max value is 0, and neutron already validates this) if (bw_rule.max_burst_kbps > MAX_BURST_MAX_VALUE): - LOG.warning("Invalid input for burst_size. " - "The maximal legal value is %s", - MAX_BURST_MAX_VALUE) - bw_rule.max_burst_kbps = MAX_BURST_MAX_VALUE + msg = (_("Invalid input for burst_size. " + "The maximal legal value is %s") % MAX_BURST_MAX_VALUE) + LOG.error(msg) + raise n_exc.InvalidInput(error_message=msg) def _get_bw_values_from_rule(self, bw_rule): """Translate the neutron bandwidth_limit_rule values, into the @@ -115,7 +117,6 @@ class QosNotificationsHandler(object): """ if bw_rule: shaping_enabled = True - self._validate_bw_values(bw_rule) # translate kbps -> bytes burst_size = int(bw_rule.max_burst_kbps) * 128 @@ -168,3 +169,8 @@ class QosNotificationsHandler(object): average_bandwidth=average_bw, qos_marking=qos_marking, dscp=dscp) + + def validate_policy_rule(self, context, policy_id, rule): + """Raise an exception if the rule values are not supported""" + if rule.rule_type == qos_consts.RULE_TYPE_BANDWIDTH_LIMIT: + self._validate_bw_values(rule) diff --git a/vmware_nsx/tests/unit/services/qos/test_nsxv3_notification.py b/vmware_nsx/tests/unit/services/qos/test_nsxv3_notification.py index 2a7813fc9a..88ee7bd71e 100644 --- a/vmware_nsx/tests/unit/services/qos/test_nsxv3_notification.py +++ b/vmware_nsx/tests/unit/services/qos/test_nsxv3_notification.py @@ -18,6 +18,7 @@ from neutron_lib import context from oslo_config import cfg from oslo_utils import uuidutils +from neutron.common import exceptions from neutron.objects import base as base_object from neutron.objects.qos import policy as policy_object from neutron.objects.qos import rule as rule_object @@ -173,8 +174,7 @@ class TestQosNsxV3Notification(base.BaseQosTestCase, @mock.patch.object(policy_object.QosPolicy, '_reload_rules') def test_bw_rule_create_profile_minimal_val(self, *mocks): - # test the switch profile update when a QoS rule is created - # with an invalid limit value + # test driver precommit with an invalid limit value bad_limit = qos_utils.MAX_KBPS_MIN_VALUE - 1 rule_data = { 'bandwidth_limit_rule': {'id': uuidutils.generate_uuid(), @@ -189,35 +189,17 @@ class TestQosNsxV3Notification(base.BaseQosTestCase, # add a rule to the policy setattr(_policy, "rules", [rule]) with mock.patch('neutron.objects.qos.policy.QosPolicy.get_object', - return_value=_policy): - with mock.patch( - 'vmware_nsxlib.v3.core_resources.NsxLibQosSwitchingProfile.' - 'update_shaping' - ) as update_profile: - with mock.patch('neutron.objects.db.api.update_object', - return_value=rule_data): - self.qos_plugin.update_policy_bandwidth_limit_rule( - self.ctxt, rule.id, _policy.id, rule_data) - - # validate the data on the profile - rule_dict = rule_data['bandwidth_limit_rule'] - expected_bw = qos_utils.MAX_KBPS_MIN_VALUE / 1024 - expected_burst = rule_dict['max_burst_kbps'] * 128 - expected_peak = int(expected_bw * self.peak_bw_multiplier) - update_profile.assert_called_once_with( - self.fake_profile_id, - average_bandwidth=expected_bw, - burst_size=expected_burst, - peak_bandwidth=expected_peak, - shaping_enabled=True, - dscp=0, - qos_marking='trusted' - ) + return_value=_policy),\ + mock.patch('neutron.objects.db.api.update_object', + return_value=rule_data): + self.assertRaises( + exceptions.DriverCallError, + self.qos_plugin.update_policy_bandwidth_limit_rule, + self.ctxt, rule.id, _policy.id, rule_data) @mock.patch.object(policy_object.QosPolicy, '_reload_rules') def test_bw_rule_create_profile_maximal_val(self, *mocks): - # test the switch profile update when a QoS rule is created - # with an invalid burst value + # test driver precommit with an invalid burst value bad_burst = qos_utils.MAX_BURST_MAX_VALUE + 1 rule_data = { 'bandwidth_limit_rule': {'id': uuidutils.generate_uuid(), @@ -232,30 +214,13 @@ class TestQosNsxV3Notification(base.BaseQosTestCase, # add a rule to the policy setattr(_policy, "rules", [rule]) with mock.patch('neutron.objects.qos.policy.QosPolicy.get_object', - return_value=_policy): - with mock.patch( - 'vmware_nsxlib.v3.core_resources.NsxLibQosSwitchingProfile.' - 'update_shaping' - ) as update_profile: - with mock.patch('neutron.objects.db.api.update_object', - return_value=rule_data): - self.qos_plugin.update_policy_bandwidth_limit_rule( - self.ctxt, rule.id, _policy.id, rule_data) - - # validate the data on the profile - rule_dict = rule_data['bandwidth_limit_rule'] - expected_burst = qos_utils.MAX_BURST_MAX_VALUE * 128 - expected_bw = int(rule_dict['max_kbps'] / 1024) - expected_peak = int(expected_bw * self.peak_bw_multiplier) - update_profile.assert_called_once_with( - self.fake_profile_id, - average_bandwidth=expected_bw, - burst_size=expected_burst, - peak_bandwidth=expected_peak, - shaping_enabled=True, - dscp=0, - qos_marking='trusted' - ) + return_value=_policy),\ + mock.patch('neutron.objects.db.api.update_object', + return_value=rule_data): + self.assertRaises( + exceptions.DriverCallError, + self.qos_plugin.update_policy_bandwidth_limit_rule, + self.ctxt, rule.id, _policy.id, rule_data) @mock.patch.object(policy_object.QosPolicy, '_reload_rules') def test_dscp_rule_create_profile(self, *mocks):