]> review.fuel-infra Code Review - openstack-build/neutron-build.git/commitdiff
Update OVS driver to work with objects
authorMoshe Levi <moshele@mellanox.com>
Tue, 28 Jul 2015 12:46:10 +0000 (15:46 +0300)
committerIhar Hrachyshka <ihrachys@redhat.com>
Tue, 4 Aug 2015 12:58:56 +0000 (14:58 +0200)
This patch updates the QoS OVS driver to work with policy NeutronObjects
that are passed by the agent extension manager, instead of lists of rule
dicts, as we originally expected.  It also adds validation that the
rules that are sent by the neutron-server are actually supported by the
backend.

Finally, port dict was not really enough to determine the name of the
port in ovsdb. 'name' field is not really present in all port dicts, and
does not reflect what is known to ovs anyway. So instead, we should
rely on vif_port object to determine the ovs port name. Since ovs agent
only added the vif_port value to details dict when binding was desired,
I made adding the vif_port object unconditional, and covered that fact
with unit tests.

With this patch in place, I was able to get policy rules applied to a
port in devstack installation. Functional tests will belong to a
follow-up.

Partially-Implements: blueprint quantum-qos-api
Change-Id: I8926adb0a30728e4f82e55d71ad7e76676a22086

neutron/agent/l2/extensions/qos_agent.py
neutron/plugins/ml2/drivers/openvswitch/agent/extension_drivers/qos_driver.py
neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py
neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/extension_drivers/test_qos_driver.py
neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py

index 16f2e8762276d28752158ff34390a333746d7be2..50e1d8de9827af8bd9e634e73d6424b5b424c239 100644 (file)
@@ -27,44 +27,44 @@ from neutron import manager
 
 @six.add_metaclass(abc.ABCMeta)
 class QosAgentDriver(object):
-    """Define stable abstract interface for Qos Agent Driver.
+    """Define stable abstract interface for QoS Agent Driver.
 
-    Qos Agent driver defines the interface to be implemented by Agent
-    for applying Qos Rules on a port.
+    QoS Agent driver defines the interface to be implemented by Agent
+    for applying QoS Rules on a port.
     """
 
     @abc.abstractmethod
     def initialize(self):
-        """Perform Qos agent driver initialization.
+        """Perform QoS agent driver initialization.
         """
         pass
 
     @abc.abstractmethod
-    def create(self, port, rules):
-        """Apply Qos rules on port for the first time.
+    def create(self, port, qos_policy):
+        """Apply QoS rules on port for the first time.
 
         :param port: port object.
-        :param rules: the list of rules to apply on port.
+        :param qos_policy: the QoS policy to be apply on port.
         """
-        #TODO(Qos) we may want to provide default implementations of calling
+        #TODO(QoS) we may want to provide default implementations of calling
         #delete and then update
         pass
 
     @abc.abstractmethod
-    def update(self, port, rules):
-        """Apply Qos rules on port.
+    def update(self, port, qos_policy):
+        """Apply QoS rules on port.
 
         :param port: port object.
-        :param rules: the list of rules to be apply on port.
+        :param qos_policy: the QoS policy to be apply on port.
         """
         pass
 
     @abc.abstractmethod
-    def delete(self, port, rules):
-        """Remove Qos rules from port.
+    def delete(self, port, qos_policy):
+        """Remove QoS rules from port.
 
         :param port: port object.
-        :param rules: the list of rules to be removed from port.
+        :param qos_policy: the QoS policy to be removed from port.
         """
         pass
 
@@ -84,11 +84,11 @@ class QosAgentExtension(agent_extension.AgentCoreResourceExtension):
         self.known_ports = set()
 
     def handle_port(self, context, port):
-        """Handle agent qos extension for port.
+        """Handle agent QoS extension for port.
 
         This method subscribes to qos_policy_id changes
         with a callback and get all the qos_policy_ports and apply
-        them using the qos driver.
+        them using the QoS driver.
         Updates and delete event should be handle by the registered
         callback.
         """
index 2902218beeab2c41d5a6730e5324d7e6d3fc2d71..3dd9285316d98cefa491036213977980e843a599 100644 (file)
@@ -16,51 +16,61 @@ from oslo_config import cfg
 from oslo_log import log as logging
 
 from neutron.agent.common import ovs_lib
+from neutron.i18n import _LE, _LW
 from neutron.agent.l2.extensions import qos_agent
-from neutron.services.qos import qos_consts
+from neutron.plugins.ml2.drivers.openvswitch.mech_driver import (
+    mech_openvswitch)
 
 LOG = logging.getLogger(__name__)
 
 
 class QosOVSAgentDriver(qos_agent.QosAgentDriver):
 
+    _SUPPORTED_RULES = (
+        mech_openvswitch.OpenvswitchMechanismDriver.supported_qos_rule_types)
+
     def __init__(self):
         super(QosOVSAgentDriver, self).__init__()
         # TODO(QoS) check if we can get this configuration
         #  as constructor arguments
         self.br_int_name = cfg.CONF.OVS.integration_bridge
         self.br_int = None
-        self.handlers = {}
 
     def initialize(self):
-        self.handlers[('update', qos_consts.RULE_TYPE_BANDWIDTH_LIMIT)] = (
-            self._update_bw_limit_rule)
-        self.handlers[('create', qos_consts.RULE_TYPE_BANDWIDTH_LIMIT)] = (
-            self._update_bw_limit_rule)
-        self.handlers[('delete', qos_consts.RULE_TYPE_BANDWIDTH_LIMIT)] = (
-            self._delete_bw_limit_rule)
-
         self.br_int = ovs_lib.OVSBridge(self.br_int_name)
 
-    def create(self, port, rules):
-        self._handle_rules('create', port, rules)
-
-    def update(self, port, rules):
-        self._handle_rules('update', port, rules)
-
-    def delete(self, port, rules):
-        self._handle_rules('delete', port, rules)
-
-    def _handle_rules(self, action, port, rules):
-        for rule in rules:
-            handler = self.handlers.get((action, rule.get('type')))
-            if handler is not None:
-                handler(port, rule)
-
-    def _update_bw_limit_rule(self, port, rule):
-        port_name = port.get('name')
-        max_kbps = rule.get('max_kbps')
-        max_burst_kbps = rule.get('max_burst_kbps')
+    def create(self, port, qos_policy):
+        self._handle_rules('create', port, qos_policy)
+
+    def update(self, port, qos_policy):
+        self._handle_rules('update', port, qos_policy)
+
+    def delete(self, port, qos_policy):
+        self._handle_rules('delete', port, qos_policy)
+
+    def _handle_rules(self, action, port, qos_policy):
+        for rule in qos_policy.rules:
+            if rule.rule_type in self._SUPPORTED_RULES:
+                handler_name = ("".join(("_", action, "_", rule.rule_type)))
+                try:
+                    handler = getattr(self, handler_name)
+                    handler(port, rule)
+                except AttributeError:
+                    LOG.error(
+                        _LE('Failed to locate a handler for %(rule_type) '
+                        'rules; skipping.'), handler_name)
+            else:
+                LOG.warning(_LW('Unsupported QoS rule type for %(rule_id)s: '
+                            '%(rule_type)s; skipping'),
+                            {'rule_id': rule.id, 'rule_type': rule.rule_type})
+
+    def _create_bandwidth_limit(self, port, rule):
+        self._update_bandwidth_limit(port, rule)
+
+    def _update_bandwidth_limit(self, port, rule):
+        port_name = port['vif_port'].port_name
+        max_kbps = rule.max_kbps
+        max_burst_kbps = rule.max_burst_kbps
 
         current_max_kbps, current_max_burst = (
             self.br_int.get_qos_bw_limit_for_port(port_name))
@@ -71,8 +81,8 @@ class QosOVSAgentDriver(qos_agent.QosAgentDriver):
                                                  max_kbps,
                                                  max_burst_kbps)
 
-    def _delete_bw_limit_rule(self, port, rule):
-        port_name = port.get('name')
+    def _delete_bandwidth_limit(self, port, rule):
+        port_name = port['vif_port'].port_name
         current_max_kbps, current_max_burst = (
             self.br_int.get_qos_bw_limit_for_port(port_name))
         if current_max_kbps is not None or current_max_burst is not None:
index e9de955f81daf60029b5ace8d1935053e9a09040..9caaae219f30c0f642bc4cc5b7aab3f431cdf91c 100644 (file)
@@ -1233,6 +1233,7 @@ class OVSNeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin,
             if 'port_id' in details:
                 LOG.info(_LI("Port %(device)s updated. Details: %(details)s"),
                          {'device': device, 'details': details})
+                details['vif_port'] = port
                 need_binding = self.treat_vif_port(port, details['port_id'],
                                                    details['network_id'],
                                                    details['network_type'],
@@ -1246,7 +1247,6 @@ class OVSNeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin,
                     self.setup_arp_spoofing_protection(self.int_br,
                                                        port, details)
                 if need_binding:
-                    details['vif_port'] = port
                     need_binding_devices.append(details)
                 self.agent_extensions_mgr.handle_port(self.context, details)
             else:
index 3a55fce8d481501ce72fded82880054fabecc0f7..7b6c430b7f026a05e58c4127ece45f1897de1845 100644 (file)
 #    under the License.
 
 import mock
+from oslo_utils import uuidutils
 
+from neutron import context
+from neutron.objects.qos import policy
+from neutron.objects.qos import rule
 from neutron.plugins.ml2.drivers.openvswitch.agent.extension_drivers import (
     qos_driver)
-from neutron.services.qos import qos_consts
 from neutron.tests.unit.plugins.ml2.drivers.openvswitch.agent import (
     ovs_test_base)
 
 
-class OVSQoSAgentDriverBwLimitRule(ovs_test_base.OVSAgentConfigTestBase):
+class QosOVSAgentDriverTestCase(ovs_test_base.OVSAgentConfigTestBase):
 
     def setUp(self):
-        super(OVSQoSAgentDriverBwLimitRule, self).setUp()
+        super(QosOVSAgentDriverTestCase, self).setUp()
+        self.context = context.get_admin_context()
         self.qos_driver = qos_driver.QosOVSAgentDriver()
         self.qos_driver.initialize()
         self.qos_driver.br_int = mock.Mock()
@@ -33,47 +37,61 @@ class OVSQoSAgentDriverBwLimitRule(ovs_test_base.OVSAgentConfigTestBase):
         self.delete = self.qos_driver.br_int.del_qos_bw_limit_for_port
         self.qos_driver.br_int.create_qos_bw_limit_for_port = mock.Mock()
         self.create = self.qos_driver.br_int.create_qos_bw_limit_for_port
-        self.rule = self._create_bw_limit_rule()
+        self.rule = self._create_bw_limit_rule_obj()
+        self.qos_policy = self._create_qos_policy_obj([self.rule])
         self.port = self._create_fake_port()
 
-    def _create_bw_limit_rule(self):
-        return {'type': qos_consts.RULE_TYPE_BANDWIDTH_LIMIT,
-                'max_kbps': '200',
-                'max_burst_kbps': '2'}
+    def _create_bw_limit_rule_obj(self):
+        rule_obj = rule.QosBandwidthLimitRule()
+        rule_obj.id = uuidutils.generate_uuid()
+        rule_obj.max_kbps = 2
+        rule_obj.max_burst_kbps = 200
+        rule_obj.obj_reset_changes()
+        return rule_obj
+
+    def _create_qos_policy_obj(self, rules):
+        policy_dict = {'id': uuidutils.generate_uuid(),
+                'tenant_id': uuidutils.generate_uuid(),
+                'name': 'test',
+                'description': 'test',
+                'shared': False,
+                'rules': rules}
+        policy_obj = policy.QosPolicy(self.context, **policy_dict)
+        policy_obj.obj_reset_changes()
+        return policy_obj
 
     def _create_fake_port(self):
-        return {'name': 'fakeport'}
+        self.port_name = 'fakeport'
+
+        class FakeVifPort(object):
+            port_name = self.port_name
+
+        return {'vif_port': FakeVifPort()}
 
     def test_create_new_rule(self):
         self.qos_driver.br_int.get_qos_bw_limit_for_port = mock.Mock(
             return_value=(None, None))
-        self.qos_driver.create(self.port, [self.rule])
+        self.qos_driver.create(self.port, self.qos_policy)
         # Assert create is the last call
         self.assertEqual(
             'create_qos_bw_limit_for_port',
             self.qos_driver.br_int.method_calls[-1][0])
         self.assertEqual(0, self.delete.call_count)
         self.create.assert_called_once_with(
-            self.port['name'], self.rule['max_kbps'],
-            self.rule['max_burst_kbps'])
+            self.port_name, self.rule.max_kbps,
+            self.rule.max_burst_kbps)
 
     def test_create_existing_rules(self):
-        self.qos_driver.create(self.port, [self.rule])
+        self.qos_driver.create(self.port, self.qos_policy)
         self._assert_rule_create_updated()
 
     def test_update_rules(self):
-        self.qos_driver.update(self.port, [self.rule])
+        self.qos_driver.update(self.port, self.qos_policy)
         self._assert_rule_create_updated()
 
     def test_delete_rules(self):
-        self.qos_driver.delete(self.port, [self.rule])
-        self.delete.assert_called_once_with(self.port['name'])
-
-    def test_unknown_rule_id(self):
-        self.rule['type'] = 'unknown'
-        self.qos_driver.create(self.port, [self.rule])
-        self.assertEqual(0, self.create.call_count)
-        self.assertEqual(0, self.delete.call_count)
+        self.qos_driver.delete(self.port, self.qos_policy)
+        self.delete.assert_called_once_with(self.port_name)
 
     def _assert_rule_create_updated(self):
         # Assert create is the last call
@@ -81,8 +99,8 @@ class OVSQoSAgentDriverBwLimitRule(ovs_test_base.OVSAgentConfigTestBase):
             'create_qos_bw_limit_for_port',
             self.qos_driver.br_int.method_calls[-1][0])
 
-        self.delete.assert_called_once_with(self.port['name'])
+        self.delete.assert_called_once_with(self.port_name)
 
         self.create.assert_called_once_with(
-            self.port['name'], self.rule['max_kbps'],
-            self.rule['max_burst_kbps'])
+            self.port_name, self.rule.max_kbps,
+            self.rule.max_burst_kbps)
index 19bcd520d9914bbabde62679e28c8154d081fc82..301a5cf5fb0744456924b8bdca4f20d3a6417032 100644 (file)
@@ -380,6 +380,28 @@ class TestOvsNeutronAgent(object):
         self.assertTrue(self._mock_treat_devices_added_updated(
             details, mock.Mock(), 'treat_vif_port'))
 
+    def test_treat_devices_added_updated_sends_vif_port_into_extension_manager(
+        self, *args):
+        details = mock.MagicMock()
+        details.__contains__.side_effect = lambda x: True
+        port = mock.MagicMock()
+
+        def fake_handle_port(context, port):
+            self.assertIn('vif_port', port)
+
+        with mock.patch.object(self.agent.plugin_rpc,
+                               'get_devices_details_list',
+                               return_value=[details]),\
+            mock.patch.object(self.agent.agent_extensions_mgr,
+                              'handle_port', new=fake_handle_port),\
+            mock.patch.object(self.agent.int_br,
+                              'get_vifs_by_ids',
+                              return_value={details['device']: port}),\
+            mock.patch.object(self.agent, 'treat_vif_port',
+                              return_value=False):
+
+            self.agent.treat_devices_added_or_updated([{}], False)
+
     def test_treat_devices_added_updated_skips_if_port_not_found(self):
         dev_mock = mock.MagicMock()
         dev_mock.__getitem__.return_value = 'the_skipped_one'