Merge "Allow bound ports to be trunked if the driver can support it"

neutron/services/trunk/drivers/base.py

@@ -53,6 +53,14 @@ class DriverBase(object):
         required.
         """
 
+    def is_interface_compatible(self, interface):
+        """True if the driver is compatible with the interface."""
+        return interface in self.interfaces
+
+    def is_agent_compatible(self, agent_type):
+        """True if the driver is compatible with the agent type."""
+        return agent_type == self.agent_type
+
     def register(self, resource, event, trigger, **kwargs):
         """Register the trunk driver.
 

neutron/services/trunk/exceptions.py

@@ -67,3 +67,8 @@ class TrunkInErrorState(n_exc.Conflict):
 class IncompatibleTrunkPluginConfiguration(n_exc.NeutronException):
     message = _("Cannot load trunk plugin: no compatible core plugin "
                 "configuration is found.")
+
+
+class TrunkPluginDriverConflict(n_exc.Conflict):
+    message = _("A misconfiguration in the environment prevents the "
+                "operation from completing, please, contact the admin.")

neutron/services/trunk/plugin.py

@@ -86,6 +86,11 @@ class TrunkPlugin(service_base.ServicePluginBase,
         self._interfaces = self._interfaces | set(driver.interfaces)
         self._drivers.append(driver)
 
+    @property
+    def registered_drivers(self):
+        """The registered drivers."""
+        return self._drivers
+
     @property
     def supported_interfaces(self):
         """A set of supported interfaces."""

neutron/services/trunk/rules.py

@@ -22,6 +22,7 @@ from neutron.extensions import portbindings
 from neutron import manager
 from neutron.objects import trunk as trunk_objects
 from neutron.services.trunk import exceptions as trunk_exc
+from neutron.services.trunk import utils
 
 
 # This layer is introduced for keeping business logic and
@@ -56,6 +57,7 @@ class TrunkPortValidator(object):
 
     def __init__(self, port_id):
         self.port_id = port_id
+        self._port = None
 
     def validate(self, context):
         """Validate that the port can be used in a trunk."""
@@ -74,7 +76,7 @@ class TrunkPortValidator(object):
         if trunks:
             raise trunk_exc.ParentPortInUse(port_id=self.port_id)
 
-        if self.is_bound(context):
+        if not self.can_be_trunked(context):
             raise trunk_exc.ParentPortInUse(port_id=self.port_id)
 
         return self.port_id
@@ -83,10 +85,37 @@ class TrunkPortValidator(object):
         """Return true if the port is bound, false otherwise."""
         # Validate that the given port_id does not have a port binding.
         core_plugin = manager.NeutronManager.get_plugin()
-        port = core_plugin.get_port(context, self.port_id)
-        device_owner = port.get('device_owner', '')
-        return port.get(portbindings.HOST_ID) or \
-            device_owner.startswith(n_const.DEVICE_OWNER_COMPUTE_PREFIX)
+        self._port = core_plugin.get_port(context, self.port_id)
+        device_owner = self._port.get('device_owner', '')
+        return (
+            self._port.get(portbindings.HOST_ID) or
+            device_owner.startswith(n_const.DEVICE_OWNER_COMPUTE_PREFIX))
+
+    def can_be_trunked(self, context):
+        """"Return true if a port can be trunked."""
+        if not self.is_bound(context):
+            # An unbound port can be trunked, always.
+            return True
+
+        trunk_plugin = manager.NeutronManager.get_service_plugins()['trunk']
+        vif_type = self._port.get(portbindings.VIF_TYPE)
+        binding_host = self._port.get(portbindings.HOST_ID)
+
+        # Determine the driver that will be in charge of the trunk: this
+        # can be determined based on the vif type, whether or not the
+        # driver is agent-based, and whether the host is running the agent
+        # associated to the driver itself.
+        drivers = [
+            driver for driver in trunk_plugin.registered_drivers
+            if utils.is_driver_compatible(
+                context, driver, vif_type, binding_host)
+        ]
+        if len(drivers) > 1:
+            raise trunk_exc.TrunkPluginDriverConflict()
+        elif len(drivers) == 1:
+            return drivers[0].can_trunk_bound_port
+        else:
+            return False
 
 
 class SubPortsValidator(object):

neutron/services/trunk/utils.py

@@ -14,9 +14,49 @@
 
 from neutron_lib import constants
 
+from neutron.common import utils
+from neutron import manager
 from neutron.services.trunk.drivers.openvswitch import constants as ovs_const
 
 
 def gen_trunk_br_name(trunk_id):
     return ((ovs_const.TRUNK_BR_PREFIX + trunk_id)
             [:constants.DEVICE_NAME_MAX_LEN - 1])
+
+
+def are_agent_types_available_on_host(context, agent_types, host):
+    """Return true if agent types are present on the host."""
+    core_plugin = manager.NeutronManager.get_plugin()
+    if utils.is_extension_supported(core_plugin, 'agent'):
+        return bool(core_plugin.get_agents(
+            context.elevated(),
+            filters={'host': [host], 'agent_type': agent_types}))
+    return False
+
+
+def is_driver_compatible(context, driver, interface, binding_host):
+    """Return true if the driver is compatible with the interface and host.
+
+    There may be edge cases where a stale view or the deployment may make the
+    following test fail to detect the right driver in charge of the bound port.
+    """
+
+    # NOTE(armax): this logic stems from the fact that the way Neutron is
+    # architected we do not have a univocal mapping between VIF type and the
+    # Driver serving it, in that the same vif type can be supported by
+    # multiple drivers. A practical example of this is OVS and OVN in the
+    # same deployment. In order to uniquely identify the driver, we cannot
+    # simply look at the vif type, and we need to look at whether the host
+    # to which the port is bound is actually managed by one driver or the
+    # other.
+    is_interface_compatible = driver.is_interface_compatible(interface)
+
+    # For an agentless driver, only interface compatibility is required.
+    if not driver.agent_type:
+        return is_interface_compatible
+
+    # For an agent-based driver, both interface and agent compat is required.
+    return (
+        is_interface_compatible and
+        are_agent_types_available_on_host(
+            context, [driver.agent_type], binding_host))

neutron/tests/unit/services/trunk/drivers/openvswitch/test_driver.py

@@ -35,6 +35,10 @@ class OVSDriverTestCase(base.BaseTestCase):
                          ovs_driver.segmentation_types)
         self.assertEqual(constants.AGENT_TYPE_OVS, ovs_driver.agent_type)
         self.assertFalse(ovs_driver.can_trunk_bound_port)
+        self.assertTrue(
+            ovs_driver.is_agent_compatible(constants.AGENT_TYPE_OVS))
+        self.assertTrue(
+            ovs_driver.is_interface_compatible(driver.SUPPORTED_INTERFACES[0]))
 
     def test_driver_is_loaded(self):
         cfg.CONF.set_override('mechanism_drivers',

neutron/tests/unit/services/trunk/fakes.py

@@ -0,0 +1,59 @@
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
+# implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+from neutron.services.trunk.drivers import base
+
+
+class FakeDriver(base.DriverBase):
+
+    @property
+    def is_loaded(self):
+        return True
+
+    @classmethod
+    def create(cls):
+        return cls('foo_name', ('foo_intfs',), ('foo_seg_types',))
+
+
+class FakeDriver2(base.DriverBase):
+
+    @property
+    def is_loaded(self):
+        return True
+
+    @classmethod
+    def create(cls):
+        return cls('foo_name2', ('foo_intf2',), ('foo_seg_types',))
+
+
+class FakeDriverCanTrunkBoundPort(base.DriverBase):
+
+    @property
+    def is_loaded(self):
+        return True
+
+    @classmethod
+    def create(cls):
+        return cls('foo_name3', ('foo_intfs',),
+                   ('foo_seg_types',), can_trunk_bound_port=True)
+
+
+class FakeDriverWithAgent(base.DriverBase):
+
+    @property
+    def is_loaded(self):
+        return True
+
+    @classmethod
+    def create(cls):
+        return cls('foo_name4', ('foo_intfs',), ('foo_seg_types',), "foo_type")

neutron/tests/unit/services/trunk/test_plugin.py

@@ -24,10 +24,10 @@ from neutron.objects import trunk as trunk_objects
 from neutron.services.trunk import callbacks
 from neutron.services.trunk import constants
 from neutron.services.trunk import drivers
-from neutron.services.trunk.drivers import base
 from neutron.services.trunk import exceptions as trunk_exc
 from neutron.services.trunk import plugin as trunk_plugin
 from neutron.tests.unit.plugins.ml2 import test_plugin
+from neutron.tests.unit.services.trunk import fakes
 
 
 def create_subport_dict(port_id):
@@ -267,17 +267,6 @@ class TrunkPluginTestCase(test_plugin.Ml2PluginV2TestCase):
             self.assertEqual(constants.PENDING_STATUS, trunk['status'])
 
 
-class FakeDriver(base.DriverBase):
-
-    @property
-    def is_loaded(self):
-        return True
-
-    @classmethod
-    def create(cls):
-        return FakeDriver('foo_name', ('foo_intfs',), ('foo_seg_types',))
-
-
 class TrunkPluginDriversTestCase(test_plugin.Ml2PluginV2TestCase):
 
     def setUp(self):
@@ -290,8 +279,9 @@ class TrunkPluginDriversTestCase(test_plugin.Ml2PluginV2TestCase):
             trunk_plugin.TrunkPlugin()
 
     def test_plugin_with_fake_driver(self):
-        fake_driver = FakeDriver.create()
+        fake_driver = fakes.FakeDriver.create()
         plugin = trunk_plugin.TrunkPlugin()
         self.assertTrue(fake_driver.is_loaded)
         self.assertEqual(set([]), plugin.supported_agent_types)
         self.assertEqual(set(['foo_intfs']), plugin.supported_interfaces)
+        self.assertEqual([fake_driver], plugin.registered_drivers)

neutron/tests/unit/services/trunk/test_rules.py

@@ -15,6 +15,8 @@
 
 import mock
 
+import testtools
+
 from neutron_lib import constants as n_const
 from neutron_lib import exceptions as n_exc
 from oslo_utils import uuidutils
@@ -26,8 +28,10 @@ from neutron.services.trunk import drivers
 from neutron.services.trunk import exceptions as trunk_exc
 from neutron.services.trunk import plugin as trunk_plugin
 from neutron.services.trunk import rules
+from neutron.services.trunk import utils as trunk_utils
 from neutron.tests import base
 from neutron.tests.unit.plugins.ml2 import test_plugin
+from neutron.tests.unit.services.trunk import fakes
 
 
 class SubPortsValidatorTestCase(base.BaseTestCase):
@@ -158,9 +162,7 @@ class TrunkPortValidatorTestCase(test_plugin.Ml2PluginV2TestCase):
             port['port']['binding:host_id'] = 'host'
             core_plugin.update_port(self.context, port['port']['id'], port)
             validator = rules.TrunkPortValidator(port['port']['id'])
-            self.assertRaises(trunk_exc.ParentPortInUse,
-                              validator.validate,
-                              self.context)
+            self.assertTrue(validator.is_bound(self.context))
 
     def test_validate_port_has_device_owner_compute(self):
         with self.port() as port:
@@ -169,6 +171,68 @@ class TrunkPortValidatorTestCase(test_plugin.Ml2PluginV2TestCase):
             port['port']['device_owner'] = device_owner
             core_plugin.update_port(self.context, port['port']['id'], port)
             validator = rules.TrunkPortValidator(port['port']['id'])
-            self.assertRaises(trunk_exc.ParentPortInUse,
-                              validator.validate,
-                              self.context)
+            self.assertTrue(validator.is_bound(self.context))
+
+    def test_validate_port_cannot_be_trunked_raises(self):
+        with self.port() as port, \
+             mock.patch.object(rules.TrunkPortValidator,
+                               "can_be_trunked", return_value=False), \
+             testtools.ExpectedException(trunk_exc.ParentPortInUse):
+            validator = rules.TrunkPortValidator(port['port']['id'])
+            validator.validate(self.context)
+
+    def test_can_be_trunked_returns_false(self):
+        # need to trigger a driver registration
+        fakes.FakeDriverCanTrunkBoundPort.create()
+        self.trunk_plugin = trunk_plugin.TrunkPlugin()
+        with self.port() as port, \
+                mock.patch.object(manager.NeutronManager,
+                                  "get_service_plugins") as f:
+            f.return_value = {'trunk': self.trunk_plugin}
+            core_plugin = manager.NeutronManager.get_plugin()
+            port['port']['binding:host_id'] = 'host'
+            core_plugin.update_port(self.context, port['port']['id'], port)
+            validator = rules.TrunkPortValidator(port['port']['id'])
+            # port cannot be trunked because of binding mismatch
+            self.assertFalse(validator.can_be_trunked(self.context))
+
+    def test_can_be_trunked_returns_true(self):
+        # need to trigger a driver registration
+        fakes.FakeDriverCanTrunkBoundPort.create()
+        self.trunk_plugin = trunk_plugin.TrunkPlugin()
+        with self.port() as port, \
+                mock.patch.object(manager.NeutronManager,
+                                  "get_service_plugins") as f, \
+                mock.patch.object(trunk_utils, "is_driver_compatible",
+                                  return_value=True) as g:
+            f.return_value = {'trunk': self.trunk_plugin}
+            core_plugin = manager.NeutronManager.get_plugin()
+            port['port']['binding:host_id'] = 'host'
+            core_plugin.update_port(self.context, port['port']['id'], port)
+            validator = rules.TrunkPortValidator(port['port']['id'])
+            self.assertTrue(validator.can_be_trunked(self.context))
+            self.assertTrue(g.call_count)
+
+    def test_can_be_trunked_unbound_port(self):
+        with self.port() as port:
+            validator = rules.TrunkPortValidator(port['port']['id'])
+            self.assertTrue(validator.can_be_trunked(self.context))
+
+    def test_can_be_trunked_raises_conflict(self):
+        d1 = fakes.FakeDriver.create()
+        d2 = fakes.FakeDriverWithAgent.create()
+        self.trunk_plugin = trunk_plugin.TrunkPlugin()
+        self.trunk_plugin._drivers = [d1, d2]
+        with self.port() as port, \
+                mock.patch.object(manager.NeutronManager,
+                                  "get_service_plugins") as f, \
+                mock.patch.object(trunk_utils, "is_driver_compatible",
+                                  return_value=True):
+            f.return_value = {'trunk': self.trunk_plugin}
+            core_plugin = manager.NeutronManager.get_plugin()
+            port['port']['binding:host_id'] = 'host'
+            core_plugin.update_port(self.context, port['port']['id'], port)
+            validator = rules.TrunkPortValidator(port['port']['id'])
+            self.assertRaises(
+                trunk_exc.TrunkPluginDriverConflict,
+                validator.can_be_trunked, self.context)

neutron/tests/unit/services/trunk/test_utils.py

@@ -0,0 +1,69 @@
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
+# implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+import mock
+
+from neutron.services.trunk import utils
+from neutron.tests.unit.plugins.ml2 import test_plugin
+from neutron.tests.unit.services.trunk import fakes
+
+
+class UtilsTestCase(test_plugin.Ml2PluginV2TestCase):
+
+    def test_are_agent_types_available_on_host_returns_false(self):
+        self.assertFalse(
+            utils.are_agent_types_available_on_host(
+                self.context, ['foo_type'], 'foo_host'))
+
+    def test_are_agent_types_available_on_host_returns_true(self):
+        with mock.patch("neutron.db.agents_db.AgentDbMixin.get_agents") as f:
+            f.return_value = ['foo_agent']
+            self.assertTrue(
+                utils.are_agent_types_available_on_host(
+                    self.context, ['foo_type'], 'foo_host'))
+
+    def _test_is_driver_compatible(self, driver, interface, host, agents=None):
+        with mock.patch("neutron.db.agents_db.AgentDbMixin.get_agents") as f:
+            f.return_value = agents or []
+            return utils.is_driver_compatible(self.context,
+                                              driver,
+                                              interface,
+                                              host)
+
+    def test_is_driver_compatible(self):
+        driver = fakes.FakeDriverWithAgent.create()
+        self.assertTrue(self._test_is_driver_compatible(
+            driver, 'foo_intfs', 'foo_host', [{'agent_type': 'foo_type'}]))
+
+    def test_is_driver_compatible_agent_based_agent_mismatch(self):
+        driver = fakes.FakeDriverWithAgent.create()
+        self.assertFalse(self._test_is_driver_compatible(
+            driver, 'foo_intfs', 'foo_host'))
+
+    def test_is_driver_incompatible_because_of_interface_mismatch(self):
+        driver = fakes.FakeDriverWithAgent.create()
+        self.assertFalse(self._test_is_driver_compatible(
+            driver, 'not_my_interface', 'foo_host'))
+
+    def test_is_driver_compatible_agentless(self):
+        driver = fakes.FakeDriver.create()
+        self.assertTrue(self._test_is_driver_compatible(
+            driver, 'foo_intfs', 'foo_host'))
+
+    def test_is_driver_compatible_multiple_drivers(self):
+        driver1 = fakes.FakeDriverWithAgent.create()
+        driver2 = fakes.FakeDriver2.create()
+        self.assertTrue(self._test_is_driver_compatible(
+            driver1, 'foo_intfs', 'foo_host', [{'agent_type': 'foo_type'}]))
+        self.assertFalse(self._test_is_driver_compatible(
+            driver2, 'foo_intfs', 'foo_host', [{'agent_type': 'foo_type'}]))