From 380825fcf8e5bdba10c3b2758dfb772e28f9461e Mon Sep 17 00:00:00 2001 From: Rodolfo Alonso Hernandez Date: Thu, 22 Aug 2019 11:07:14 +0000 Subject: [PATCH] Check mech driver connectivity during port binding In [1] the concept of "connectivity" was introduced for the ML2 drivers. This parameter defines the mech driver connectivity type (layer 2, layer 3 only or legacy - not defined). The spec defined in the blueprint allows to spawn a VM with ports without IP addresses. As commented in the Nova spec [2], those ports can be bound only to "l2" drivers. [1] https://review.opendev.org/#/c/645645/ [2] https://review.opendev.org/#/c/641670/ bp boot-vm-with-unaddressed-port Related-Bug: #1821058 Change-Id: I438cbab43b45b5f7afc820b77fcf5a0e823d0eff --- neutron/plugins/ml2/managers.py | 24 +++++++++++++++++ .../unit/plugins/ml2/_test_mech_agent.py | 13 ++++++--- .../plugins/ml2/drivers/mech_fake_agent.py | 14 +++++++++- .../tests/unit/plugins/ml2/test_managers.py | 27 ++++++++++++++++--- setup.cfg | 1 + 5 files changed, 72 insertions(+), 7 deletions(-) diff --git a/neutron/plugins/ml2/managers.py b/neutron/plugins/ml2/managers.py index 10c97170878..e37b6d0a964 100644 --- a/neutron/plugins/ml2/managers.py +++ b/neutron/plugins/ml2/managers.py @@ -826,10 +826,19 @@ class MechanismManager(stevedore.named.NamedExtensionManager): 'host': context.host}) return False + drivers = self._check_drivers_connectivity(drivers, context) + if not drivers: + LOG.error("Port %(port)s does not have an IP address assigned and " + "there are no driver with 'connectivity' = 'l2'. The " + "port cannot be bound.", + {'port': context.current['id']}) + return False + for driver in drivers: if not self._check_driver_to_bind(driver, segments_to_bind, context._binding_levels): continue + try: context._prepare_to_bind(segments_to_bind) driver.obj.bind_port(context) @@ -1028,6 +1037,21 @@ class MechanismManager(stevedore.named.NamedExtensionManager): return False return True + def _check_drivers_connectivity(self, drivers, port_context): + """If port does not have an IP address, driver connectivity must be l2 + + A port without an IP address can be bound only to a mech driver with + "connectivity" = "l2". "legacy" or "l3" (e.g.: Calico) drivers cannot + have a port bound without an IP allocated. + """ + if port_context.current.get('fixed_ips'): + return drivers + + return [d for d in drivers if + getattr(d.obj, 'vif_details', {}).get( + portbindings.VIF_DETAILS_CONNECTIVITY) == + portbindings.CONNECTIVITY_L2] + def get_workers(self): workers = [] for driver in self.ordered_mech_drivers: diff --git a/neutron/tests/unit/plugins/ml2/_test_mech_agent.py b/neutron/tests/unit/plugins/ml2/_test_mech_agent.py index edc30ce9ced..ade8676f00e 100644 --- a/neutron/tests/unit/plugins/ml2/_test_mech_agent.py +++ b/neutron/tests/unit/plugins/ml2/_test_mech_agent.py @@ -13,6 +13,8 @@ # License for the specific language governing permissions and limitations # under the License. +import copy + from oslo_config import cfg from oslo_config import fixture as config_fixture @@ -59,9 +61,14 @@ class FakePortContext(api.PortContext): @property def current(self): - return {'id': PORT_ID, - portbindings.VNIC_TYPE: self._bound_vnic_type, - portbindings.PROFILE: self._bound_profile} + current_data = {'id': PORT_ID, + portbindings.VNIC_TYPE: self._bound_vnic_type, + portbindings.PROFILE: self._bound_profile} + ret_value = current_data + if self._original: + ret_value = copy.deepcopy(self.original) + ret_value.update(current_data) + return ret_value @property def original(self): diff --git a/neutron/tests/unit/plugins/ml2/drivers/mech_fake_agent.py b/neutron/tests/unit/plugins/ml2/drivers/mech_fake_agent.py index c71c0f2ff5c..ed0bad07c43 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/mech_fake_agent.py +++ b/neutron/tests/unit/plugins/ml2/drivers/mech_fake_agent.py @@ -45,7 +45,10 @@ class FakeAgentMechanismDriver(mech_agent.SimpleAgentMechanismDriverBase): def __init__(self): sg_enabled = securitygroups_rpc.is_firewall_enabled() vif_details = {portbindings.CAP_PORT_FILTER: sg_enabled, - portbindings.OVS_HYBRID_PLUG: sg_enabled} + portbindings.OVS_HYBRID_PLUG: sg_enabled, + portbindings.VIF_DETAILS_CONNECTIVITY: + portbindings.CONNECTIVITY_L2, + } super(FakeAgentMechanismDriver, self).__init__( # NOTE(yamamoto): l2pop driver has a hardcoded list of # supported agent types. @@ -64,3 +67,12 @@ class FakeAgentMechanismDriver(mech_agent.SimpleAgentMechanismDriverBase): class AnotherFakeAgentMechanismDriver(FakeAgentMechanismDriver): pass + + +class FakeAgentMechanismDriverL3(FakeAgentMechanismDriver): + """ML2 mechanism driver for testing, with L3 connectivity only""" + + def __init__(self): + super(FakeAgentMechanismDriverL3, self).__init__() + self.vif_details[portbindings.VIF_DETAILS_CONNECTIVITY] = ( + portbindings.CONNECTIVITY_L3) diff --git a/neutron/tests/unit/plugins/ml2/test_managers.py b/neutron/tests/unit/plugins/ml2/test_managers.py index f585f16508a..3be4e9884b8 100644 --- a/neutron/tests/unit/plugins/ml2/test_managers.py +++ b/neutron/tests/unit/plugins/ml2/test_managers.py @@ -41,9 +41,10 @@ class TestManagers(base.BaseTestCase): 'network_type': 'vlan', 'physical_network': 'public', api.SEGMENTATION_ID: 49}] - self.context = FakePortContext(None, - None, - self.segments_to_bind) + original_port = {'fixed_ips': [{'subnet_id': mock.ANY, + 'ip_address': mock.ANY}]} + self.context = FakePortContext(None, None, self.segments_to_bind, + original=original_port) self.context._binding = mock.Mock() self.context._binding_levels = [] self.context._new_bound_segment = self.segment_id @@ -73,6 +74,26 @@ class TestManagers(base.BaseTestCase): manager._bind_port_level(self.context, 0, self.segments_to_bind) self.assertEqual(0, bind_port.call_count) + def _check_drivers_connectivity(self, agents): + cfg.CONF.set_override('mechanism_drivers', agents, group='ml2') + manager = managers.MechanismManager() + return (manager.ordered_mech_drivers, + manager._check_drivers_connectivity( + manager.ordered_mech_drivers, self.context)) + + def test__check_drivers_connectivity(self): + self.assertEqual(*self._check_drivers_connectivity(['fake_agent'])) + + def test__check_drivers_connectivity_ip_less_port(self): + self.context._original['fixed_ips'] = [] + self.assertEqual(*self._check_drivers_connectivity(['fake_agent'])) + + def test__check_drivers_connectivity_ip_less_port_l3_only_driver(self): + self.context._original['fixed_ips'] = [] + self.assertEqual( + [], + self._check_drivers_connectivity(['fake_agent_l3'])[1]) + def test__infer_driver_from_allocation_positive(self): cfg.CONF.set_override( 'mechanism_drivers', ['fake_agent'], group='ml2') diff --git a/setup.cfg b/setup.cfg index adc74a30259..2a524c6caba 100644 --- a/setup.cfg +++ b/setup.cfg @@ -95,6 +95,7 @@ neutron.ml2.mechanism_drivers = l2population = neutron.plugins.ml2.drivers.l2pop.mech_driver:L2populationMechanismDriver sriovnicswitch = neutron.plugins.ml2.drivers.mech_sriov.mech_driver.mech_driver:SriovNicSwitchMechanismDriver fake_agent = neutron.tests.unit.plugins.ml2.drivers.mech_fake_agent:FakeAgentMechanismDriver + fake_agent_l3 = neutron.tests.unit.plugins.ml2.drivers.mech_fake_agent:FakeAgentMechanismDriverL3 another_fake_agent = neutron.tests.unit.plugins.ml2.drivers.mech_fake_agent:AnotherFakeAgentMechanismDriver faulty_agent = neutron.tests.unit.plugins.ml2.drivers.mech_faulty_agent:FaultyAgentMechanismDriver neutron.ml2.extension_drivers =