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

Some drivers, like the Open vSwitch one, may restrict the creation of a
trunk from a bound port (i.e. prevents from creating trunks after a VM
is already booted on the port).

This limitation frees us from worrying about dataplane migrations for OVS
as trunk ports and regular ports are wired differently.

However, other drivers, like the Linux Bridge one, may not be affected by
this limitation, and the existing rule should be relaxed to allow the
operation based on whether not the driver can support it.

Partially-implements: blueprint vlan-aware-vms

Change-Id: Ia938eb7cc845f6f144924acdb0e333d23516fcca

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'}]))