Merge "NSX|V3: Use QoS precommit callback to validate rules"

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

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. "
+            msg = (_("Invalid input for max_kbps. "
-                        "The minimal legal value is %s",
+                     "The minimal legal value is %s") % MAX_KBPS_MIN_VALUE)
-                        MAX_KBPS_MIN_VALUE)
+            LOG.error(msg)
-            bw_rule.max_kbps = MAX_KBPS_MIN_VALUE
+            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. "
+            msg = (_("Invalid input for burst_size. "
-                        "The maximal legal value is %s",
+                     "The maximal legal value is %s") % MAX_BURST_MAX_VALUE)
-                        MAX_BURST_MAX_VALUE)
+            LOG.error(msg)
-            bw_rule.max_burst_kbps = MAX_BURST_MAX_VALUE
+            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)

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
+        # test driver precommit with an invalid limit value
-        # 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):
+                        return_value=_policy),\
-            with mock.patch(
+            mock.patch('neutron.objects.db.api.update_object',
-                'vmware_nsxlib.v3.core_resources.NsxLibQosSwitchingProfile.'
+                       return_value=rule_data):
-                'update_shaping'
+            self.assertRaises(
-            ) as update_profile:
+                exceptions.DriverCallError,
-                with mock.patch('neutron.objects.db.api.update_object',
+                self.qos_plugin.update_policy_bandwidth_limit_rule,
-                    return_value=rule_data):
+                self.ctxt, rule.id, _policy.id, 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'
-                    )
 
     @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
+        # test driver precommit with an invalid burst value
-        # 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):
+                        return_value=_policy),\
-            with mock.patch(
+            mock.patch('neutron.objects.db.api.update_object',
-                'vmware_nsxlib.v3.core_resources.NsxLibQosSwitchingProfile.'
+                       return_value=rule_data):
-                'update_shaping'
+                self.assertRaises(
-            ) as update_profile:
+                    exceptions.DriverCallError,
-                with mock.patch('neutron.objects.db.api.update_object',
+                    self.qos_plugin.update_policy_bandwidth_limit_rule,
-                    return_value=rule_data):
+                    self.ctxt, rule.id, _policy.id, 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'
-                    )
 
     @mock.patch.object(policy_object.QosPolicy, '_reload_rules')
     def test_dscp_rule_create_profile(self, *mocks):