diff --git a/neutron/services/trunk/drivers/base.py b/neutron/services/trunk/drivers/base.py index 896c7e32638..2a143c60b3a 100644 --- a/neutron/services/trunk/drivers/base.py +++ b/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. diff --git a/neutron/services/trunk/exceptions.py b/neutron/services/trunk/exceptions.py index aa07e8c08be..481766c8c00 100644 --- a/neutron/services/trunk/exceptions.py +++ b/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.") diff --git a/neutron/services/trunk/plugin.py b/neutron/services/trunk/plugin.py index 7791c7415de..bdac896d6e2 100644 --- a/neutron/services/trunk/plugin.py +++ b/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.""" diff --git a/neutron/services/trunk/rules.py b/neutron/services/trunk/rules.py index bb88c2d5cda..8454174d213 100644 --- a/neutron/services/trunk/rules.py +++ b/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): diff --git a/neutron/services/trunk/utils.py b/neutron/services/trunk/utils.py index 69172c38658..e4ff9525d96 100644 --- a/neutron/services/trunk/utils.py +++ b/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)) diff --git a/neutron/tests/unit/services/trunk/drivers/openvswitch/test_driver.py b/neutron/tests/unit/services/trunk/drivers/openvswitch/test_driver.py index 3e21388215b..58f04e8ce89 100644 --- a/neutron/tests/unit/services/trunk/drivers/openvswitch/test_driver.py +++ b/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', diff --git a/neutron/tests/unit/services/trunk/fakes.py b/neutron/tests/unit/services/trunk/fakes.py new file mode 100644 index 00000000000..3dd93a1aff1 --- /dev/null +++ b/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") diff --git a/neutron/tests/unit/services/trunk/test_plugin.py b/neutron/tests/unit/services/trunk/test_plugin.py index 9103549d383..5df3ca81edb 100644 --- a/neutron/tests/unit/services/trunk/test_plugin.py +++ b/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) diff --git a/neutron/tests/unit/services/trunk/test_rules.py b/neutron/tests/unit/services/trunk/test_rules.py index d3238ba7824..86ababa70d8 100644 --- a/neutron/tests/unit/services/trunk/test_rules.py +++ b/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) diff --git a/neutron/tests/unit/services/trunk/test_utils.py b/neutron/tests/unit/services/trunk/test_utils.py new file mode 100644 index 00000000000..96dfa5859b8 --- /dev/null +++ b/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'}]))