]> review.fuel-infra Code Review - openstack-build/neutron-build.git/commitdiff
ml2: don't consider drivers with no bind_port for qos supported rule types
authorIhar Hrachyshka <ihrachys@redhat.com>
Thu, 17 Sep 2015 17:13:22 +0000 (19:13 +0200)
committerMiguel Angel Ajo <mangelajo@redhat.com>
Tue, 22 Sep 2015 12:38:59 +0000 (14:38 +0200)
If the driver is not going to bind a port, we should not use it as
justification to limit the subset of rule types available for the user.

The most common scenario for that would be l2pop, that does not handle port
binding, but merely modifies the implementation details of node
interconnection.

Change-Id: Iba0f3530deff3bb2871ebf893c43ef628a80c7c0
Closes-Bug: #1488996

neutron/plugins/ml2/driver_api.py
neutron/plugins/ml2/managers.py
neutron/tests/fullstack/test_qos.py
neutron/tests/unit/plugins/ml2/test_plugin.py

index db25c8d5f12a2c16ed13407f80aa2116aac602d9..39bc6125998db57d3c1d7d23eecd3300a6038ed5 100644 (file)
@@ -876,9 +876,18 @@ class MechanismDriver(object):
         drivers should avoid making persistent state changes in
         bind_port, or else must ensure that such state changes are
         eventually cleaned up.
+
+        Implementing this method explicitly declares the mechanism
+        driver as having the intention to bind ports. This is inspected
+        by the QoS service to identify the available QoS rules you
+        can use with ports.
         """
         pass
 
+    @property
+    def _supports_port_binding(self):
+        return self.__class__.bind_port != MechanismDriver.bind_port
+
     def check_vlan_transparency(self, context):
         """Check if the network supports vlan transparency.
 
index 8f8920d5447b6a7bbf69cb9e97dcdfa248c43b24..89d742c67b1dd956022f6fb764a33fadd953597c 100644 (file)
@@ -320,30 +320,38 @@ class MechanismManager(stevedore.named.NamedExtensionManager):
             return []
 
         rule_types = set(qos_consts.VALID_RULE_TYPES)
+        binding_driver_found = False
 
         # Recalculate on every call to allow drivers determine supported rule
         # types dynamically
         for driver in self.ordered_mech_drivers:
-            if hasattr(driver.obj, 'supported_qos_rule_types'):
-                new_rule_types = \
-                    rule_types & set(driver.obj.supported_qos_rule_types)
-                dropped_rule_types = new_rule_types - rule_types
-                if dropped_rule_types:
-                    LOG.info(
-                        _LI("%(rule_types)s rule types disabled for ml2 "
-                            "because %(driver)s does not support them"),
-                        {'rule_types': ', '.join(dropped_rule_types),
-                         'driver': driver.name})
-                rule_types = new_rule_types
-            else:
-                # at least one of drivers does not support QoS, meaning there
-                # are no rule types supported by all of them
-                LOG.warn(
-                    _LW("%s does not support QoS; no rule types available"),
-                    driver.name)
-                return []
-
-        rule_types = list(rule_types)
+            driver_obj = driver.obj
+            if driver_obj._supports_port_binding:
+                binding_driver_found = True
+                if hasattr(driver_obj, 'supported_qos_rule_types'):
+                    new_rule_types = \
+                        rule_types & set(driver_obj.supported_qos_rule_types)
+                    dropped_rule_types = new_rule_types - rule_types
+                    if dropped_rule_types:
+                        LOG.info(
+                            _LI("%(rule_types)s rule types disabled for ml2 "
+                                "because %(driver)s does not support them"),
+                            {'rule_types': ', '.join(dropped_rule_types),
+                             'driver': driver.name})
+                    rule_types = new_rule_types
+                else:
+                    # at least one of drivers does not support QoS, meaning
+                    # there are no rule types supported by all of them
+                    LOG.warn(
+                        _LW("%s does not support QoS; "
+                            "no rule types available"),
+                        driver.name)
+                    return []
+
+        if binding_driver_found:
+            rule_types = list(rule_types)
+        else:
+            rule_types = []
         LOG.debug("Supported QoS rule types "
                   "(common subset for all mech drivers): %s", rule_types)
         return rule_types
index e44e63db1e9dde19ff03e2298240b0ec680a3987..d5c3e567fcff43959d778afaa1f3dcb2b01990c7 100644 (file)
@@ -20,6 +20,9 @@ from neutron.tests.fullstack import base
 from neutron.tests.fullstack.resources import environment
 from neutron.tests.fullstack.resources import machine
 
+from neutron.plugins.ml2.drivers.openvswitch.mech_driver import \
+    mech_openvswitch as mech_ovs
+
 
 BANDWIDTH_LIMIT = 500
 BANDWIDTH_BURST = 100
@@ -117,3 +120,19 @@ class TestQoSWithOvsAgent(base.BaseFullStackTestCase):
             vm.neutron_port['id'],
             body={'port': {'qos_policy_id': None}})
         _wait_for_rule_removed(vm)
+
+
+class TestQoSWithL2Population(base.BaseFullStackTestCase):
+
+    def setUp(self):
+        host_desc = [environment.HostDescription()]
+        env_desc = environment.EnvironmentDescription(qos=True, l2_pop=True)
+        env = environment.Environment(env_desc, host_desc)
+        super(TestQoSWithL2Population, self).setUp(env)
+
+    def test_supported_qos_rule_types(self):
+        res = self.client.list_qos_rule_types()
+        rule_types = {t['type'] for t in res['rule_types']}
+        expected_rules = (
+            set(mech_ovs.OpenvswitchMechanismDriver.supported_qos_rule_types))
+        self.assertEqual(expected_rules, rule_types)
index 5e6ad08015f2aaf47a9a96edcea1e667c81e9b68..3ca117ae803fd3a4919bc9530de09833c506778b 100644 (file)
@@ -170,6 +170,21 @@ class TestMl2SupportedQosRuleTypes(Ml2PluginV2TestCase):
             qos_consts.VALID_RULE_TYPES,
             self.driver.mechanism_manager.supported_qos_rule_types)
 
+    @mock.patch.object(mech_test.TestMechanismDriver,
+                       'supported_qos_rule_types',
+                       new_callable=mock.PropertyMock,
+                       return_value=qos_consts.VALID_RULE_TYPES,
+                       create=True)
+    @mock.patch.object(mech_logger.LoggerMechanismDriver,
+                       '_supports_port_binding',
+                       new_callable=mock.PropertyMock,
+                       return_value=False)
+    def test_rule_types_with_driver_that_does_not_implement_binding(self,
+                                                                    *mocks):
+        self.assertEqual(
+            qos_consts.VALID_RULE_TYPES,
+            self.driver.mechanism_manager.supported_qos_rule_types)
+
 
 class TestMl2BasicGet(test_plugin.TestBasicGet,
                       Ml2PluginV2TestCase):