diff --git a/neutron/plugins/ml2/driver_api.py b/neutron/plugins/ml2/driver_api.py index 78a37d9825..d55240faa3 100644 --- a/neutron/plugins/ml2/driver_api.py +++ b/neutron/plugins/ml2/driver_api.py @@ -594,26 +594,3 @@ class MechanismDriver(object): details. """ pass - - def validate_port_binding(self, context): - """Check whether existing port binding is still valid. - - :param context: PortContext instance describing the port - :returns: True if binding is valid, otherwise False - - Called inside transaction context on session to validate that - the MechanismDriver's existing binding for the port is still - valid. - """ - return False - - def unbind_port(self, context): - """Undo existing port binding. - - :param context: PortContext instance describing the port - - Called inside transaction context on session to notify the - MechanismDriver that its existing binding for the port is no - longer valid. - """ - pass diff --git a/neutron/plugins/ml2/drivers/cisco/nexus/mech_cisco_nexus.py b/neutron/plugins/ml2/drivers/cisco/nexus/mech_cisco_nexus.py index a5153f7aa3..eca142a422 100644 --- a/neutron/plugins/ml2/drivers/cisco/nexus/mech_cisco_nexus.py +++ b/neutron/plugins/ml2/drivers/cisco/nexus/mech_cisco_nexus.py @@ -153,7 +153,7 @@ class CiscoNexusMechanismDriver(api.MechanismDriver): host_id = context.current.get(portbindings.HOST_ID) # Workaround until vlan can be retrieved during delete_port_postcommit - # (or prehaps unbind_port) event. + # event. if func == self._delete_switch_entry: vlan_id = self._delete_port_postcommit_vlan else: @@ -168,7 +168,7 @@ class CiscoNexusMechanismDriver(api.MechanismDriver): raise excep.NexusMissingRequiredFields(fields=fields) # Workaround until vlan can be retrieved during delete_port_postcommit - # (or prehaps unbind_port) event. + # event. if func == self._delete_nxos_db: self._delete_port_postcommit_vlan = vlan_id else: diff --git a/neutron/plugins/ml2/drivers/mech_agent.py b/neutron/plugins/ml2/drivers/mech_agent.py index e0a5f70edb..62134439be 100644 --- a/neutron/plugins/ml2/drivers/mech_agent.py +++ b/neutron/plugins/ml2/drivers/mech_agent.py @@ -35,8 +35,7 @@ class AgentMechanismDriverBase(api.MechanismDriver): at least one segment of the port's network. MechanismDrivers using this base class must pass the agent type to - __init__(), and must implement try_to_bind_segment_for_agent() and - check_segment_for_agent(). + __init__(), and must implement try_to_bind_segment_for_agent(). """ def __init__(self, agent_type, @@ -75,26 +74,6 @@ class AgentMechanismDriverBase(api.MechanismDriver): LOG.warning(_("Attempting to bind with dead agent: %s"), agent) - def validate_port_binding(self, context): - LOG.debug(_("Validating binding for port %(port)s on " - "network %(network)s"), - {'port': context.current['id'], - 'network': context.network.current['id']}) - for agent in context.host_agents(self.agent_type): - LOG.debug(_("Checking agent: %s"), agent) - if agent['alive'] and self.check_segment_for_agent( - context.bound_segment, agent): - LOG.debug(_("Binding valid")) - return True - LOG.warning(_("Binding invalid for port: %s"), context.current) - return False - - def unbind_port(self, context): - LOG.debug(_("Unbinding port %(port)s on " - "network %(network)s"), - {'port': context.current['id'], - 'network': context.network.current['id']}) - @abstractmethod def try_to_bind_segment_for_agent(self, context, segment, agent): """Try to bind with segment for agent. @@ -114,21 +93,6 @@ class AgentMechanismDriverBase(api.MechanismDriver): return True. Otherwise, it must return False. """ - @abstractmethod - def check_segment_for_agent(self, segment, agent): - """Check if segment can be bound for agent. - - :param segment: segment dictionary describing segment to bind - :param agent: agents_db entry describing agent to bind - :returns: True iff segment can be bound for agent - - Called inside transaction during validate_port_binding() so - that derived MechanismDrivers can use agent_db data along with - built-in knowledge of the corresponding agent's capabilities - to determine whether or not the specified network segment can - be bound for the agent. - """ - @six.add_metaclass(ABCMeta) class SimpleAgentMechanismDriverBase(AgentMechanismDriverBase): @@ -144,9 +108,7 @@ class SimpleAgentMechanismDriverBase(AgentMechanismDriverBase): MechanismDrivers using this base class must pass the agent type and the values for binding:vif_type and binding:vif_details to - __init__(). They must implement check_segment_for_agent() as - defined in AgentMechanismDriverBase, which will be called during - both binding establishment and validation. + __init__(), and must implement check_segment_for_agent(). """ def __init__(self, agent_type, vif_type, vif_details, @@ -171,3 +133,18 @@ class SimpleAgentMechanismDriverBase(AgentMechanismDriverBase): return True else: return False + + @abstractmethod + def check_segment_for_agent(self, segment, agent): + """Check if segment can be bound for agent. + + :param segment: segment dictionary describing segment to bind + :param agent: agents_db entry describing agent to bind + :returns: True iff segment can be bound for agent + + Called inside transaction during bind_port so that derived + MechanismDrivers can use agent_db data along with built-in + knowledge of the corresponding agent's capabilities to + determine whether or not the specified network segment can be + bound for the agent. + """ diff --git a/neutron/plugins/ml2/drivers/mechanism_odl.py b/neutron/plugins/ml2/drivers/mechanism_odl.py index 79372544af..b099a5f98d 100644 --- a/neutron/plugins/ml2/drivers/mechanism_odl.py +++ b/neutron/plugins/ml2/drivers/mechanism_odl.py @@ -344,18 +344,6 @@ class OpenDaylightMechanismDriver(api.MechanismDriver): 'physnet': segment[api.PHYSICAL_NETWORK], 'nettype': segment[api.NETWORK_TYPE]}) - def validate_port_binding(self, context): - if self.check_segment(context.bound_segment): - LOG.debug(_('Binding valid.')) - return True - LOG.warning(_("Binding invalid for port: %s"), context.current) - - def unbind_port(self, context): - LOG.debug(_("Unbinding port %(port)s on " - "network %(network)s"), - {'port': context.current['id'], - 'network': context.network.current['id']}) - def check_segment(self, segment): """Verify a segment is valid for the OpenDaylight MechanismDriver. diff --git a/neutron/plugins/ml2/managers.py b/neutron/plugins/ml2/managers.py index e84f86f304..d29d935754 100644 --- a/neutron/plugins/ml2/managers.py +++ b/neutron/plugins/ml2/managers.py @@ -471,47 +471,3 @@ class MechanismManager(stevedore.named.NamedExtensionManager): LOG.warning(_("Failed to bind port %(port)s on host %(host)s"), {'port': context._port['id'], 'host': binding.host}) - - def validate_port_binding(self, context): - """Check whether existing port binding is still valid. - - :param context: PortContext instance describing the port - :returns: True if binding is valid, otherwise False - - Called inside transaction context on session to validate that - the bound MechanismDriver's existing binding for the port is - still valid. - """ - binding = context._binding - driver = self.mech_drivers.get(binding.driver, None) - if driver: - try: - return driver.obj.validate_port_binding(context) - except Exception: - LOG.exception(_("Mechanism driver %s failed in " - "validate_port_binding"), - driver.name) - return False - - def unbind_port(self, context): - """Undo existing port binding. - - :param context: PortContext instance describing the port - - Called inside transaction context on session to notify the - bound MechanismDriver that its existing binding for the port - is no longer valid. - """ - binding = context._binding - driver = self.mech_drivers.get(binding.driver, None) - if driver: - try: - driver.obj.unbind_port(context) - except Exception: - LOG.exception(_("Mechanism driver %s failed in " - "unbind_port"), - driver.name) - binding.vif_type = portbindings.VIF_TYPE_UNBOUND - binding.vif_details = '' - binding.driver = None - binding.segment = None diff --git a/neutron/plugins/ml2/plugin.py b/neutron/plugins/ml2/plugin.py index f548b9d19a..a51dd35076 100644 --- a/neutron/plugins/ml2/plugin.py +++ b/neutron/plugins/ml2/plugin.py @@ -226,11 +226,9 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2, if binding.vif_type != portbindings.VIF_TYPE_UNBOUND: if (not host_set and not vnic_type_set and not profile_set and - binding.segment and - self.mechanism_manager.validate_port_binding(mech_context)): + binding.segment): return False - self.mechanism_manager.unbind_port(mech_context) - self._update_port_dict_binding(port, binding) + self._delete_port_binding(mech_context) # Return True only if an agent notification is needed. # This will happen if a new host, vnic_type, or profile was specified @@ -294,10 +292,12 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2, def _delete_port_binding(self, mech_context): binding = mech_context._binding + binding.vif_type = portbindings.VIF_TYPE_UNBOUND + binding.vif_details = '' + binding.driver = None + binding.segment = None port = mech_context.current self._update_port_dict_binding(port, binding) - self.mechanism_manager.unbind_port(mech_context) - self._update_port_dict_binding(port, binding) def _ml2_extend_port_dict_binding(self, port_res, port_db): # None when called during unit tests for other plugins. @@ -720,7 +720,6 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2, mech_context = driver_context.PortContext(self, context, port, network) self.mechanism_manager.delete_port_precommit(mech_context) - self._delete_port_binding(mech_context) self._delete_port_security_group_bindings(context, id) LOG.debug(_("Calling base delete_port")) if l3plugin: diff --git a/neutron/tests/unit/ml2/_test_mech_agent.py b/neutron/tests/unit/ml2/_test_mech_agent.py index 65505749ac..4fbdc10e5c 100644 --- a/neutron/tests/unit/ml2/_test_mech_agent.py +++ b/neutron/tests/unit/ml2/_test_mech_agent.py @@ -142,8 +142,6 @@ class AgentMechanismLocalTestCase(AgentMechanismBaseTestCase): self.LOCAL_SEGMENTS) self.driver.bind_port(context) self._check_bound(context, self.LOCAL_SEGMENTS[1]) - self.assertTrue(self.driver.validate_port_binding(context)) - self.driver.unbind_port(context) def test_type_local_dead(self): context = FakePortContext(self.AGENT_TYPE, @@ -166,8 +164,6 @@ class AgentMechanismFlatTestCase(AgentMechanismBaseTestCase): self.FLAT_SEGMENTS) self.driver.bind_port(context) self._check_bound(context, self.FLAT_SEGMENTS[1]) - self.assertTrue(self.driver.validate_port_binding(context)) - self.driver.unbind_port(context) def test_type_flat_bad(self): context = FakePortContext(self.AGENT_TYPE, @@ -191,8 +187,6 @@ class AgentMechanismVlanTestCase(AgentMechanismBaseTestCase): self.VLAN_SEGMENTS) self.driver.bind_port(context) self._check_bound(context, self.VLAN_SEGMENTS[1]) - self.assertTrue(self.driver.validate_port_binding(context)) - self.driver.unbind_port(context) def test_type_vlan_bad(self): context = FakePortContext(self.AGENT_TYPE, @@ -215,8 +209,6 @@ class AgentMechanismGreTestCase(AgentMechanismBaseTestCase): self.GRE_SEGMENTS) self.driver.bind_port(context) self._check_bound(context, self.GRE_SEGMENTS[1]) - self.assertTrue(self.driver.validate_port_binding(context)) - self.driver.unbind_port(context) def test_type_gre_bad(self): context = FakePortContext(self.AGENT_TYPE, diff --git a/neutron/tests/unit/ml2/drivers/mechanism_logger.py b/neutron/tests/unit/ml2/drivers/mechanism_logger.py index c23ef81aaa..401badb166 100644 --- a/neutron/tests/unit/ml2/drivers/mechanism_logger.py +++ b/neutron/tests/unit/ml2/drivers/mechanism_logger.py @@ -118,9 +118,3 @@ class LoggerMechanismDriver(api.MechanismDriver): def bind_port(self, context): self._log_port_call("bind_port", context) - - def validate_port_binding(self, context): - self._log_port_call("validate_port_binding", context) - - def unbind_port(self, context): - self._log_port_call("unbind_port", context) diff --git a/neutron/tests/unit/ml2/drivers/mechanism_test.py b/neutron/tests/unit/ml2/drivers/mechanism_test.py index a0c05c962e..64b793ae4e 100644 --- a/neutron/tests/unit/ml2/drivers/mechanism_test.py +++ b/neutron/tests/unit/ml2/drivers/mechanism_test.py @@ -21,7 +21,7 @@ class TestMechanismDriver(api.MechanismDriver): """Test mechanism driver for testing mechanism driver api.""" def initialize(self): - pass + self.bound_ports = set() def _check_network_context(self, context, original_expected): assert(isinstance(context, api.NetworkContext)) @@ -87,13 +87,16 @@ class TestMechanismDriver(api.MechanismDriver): vif_type = context.current.get(portbindings.VIF_TYPE) assert(vif_type is not None) + if vif_type in (portbindings.VIF_TYPE_UNBOUND, portbindings.VIF_TYPE_BINDING_FAILED): assert(context.bound_segment is None) assert(context.bound_driver is None) + assert(context.current['id'] not in self.bound_ports) else: assert(isinstance(context.bound_segment, dict)) assert(context.bound_driver == 'test') + assert(context.current['id'] in self.bound_ports) if original_expected: assert(isinstance(context.original, dict)) @@ -123,6 +126,9 @@ class TestMechanismDriver(api.MechanismDriver): self._check_port_context(context, False) def update_port_precommit(self, context): + if (context.original_bound_driver == 'test' and + context.bound_driver != 'test'): + self.bound_ports.remove(context.original['id']) self._check_port_context(context, True) def update_port_postcommit(self, context): @@ -135,6 +141,12 @@ class TestMechanismDriver(api.MechanismDriver): self._check_port_context(context, False) def bind_port(self, context): + # REVISIT(rkukura): The upcoming fix for bug 1276391 will + # ensure the MDs see the unbinding of the port as a port + # update prior to re-binding, at which point this should be + # removed. + self.bound_ports.discard(context.current['id']) + # REVISIT(rkukura): Currently, bind_port() is called as part # of either a create or update transaction. The fix for bug # 1276391 will change it to be called outside any transaction, @@ -146,23 +158,8 @@ class TestMechanismDriver(api.MechanismDriver): if host == "host-ovs-no_filter": context.set_binding(segment, portbindings.VIF_TYPE_OVS, {portbindings.CAP_PORT_FILTER: False}) + self.bound_ports.add(context.current['id']) elif host == "host-bridge-filter": context.set_binding(segment, portbindings.VIF_TYPE_BRIDGE, {portbindings.CAP_PORT_FILTER: True}) - - def validate_port_binding(self, context): - # REVISIT(rkukura): Currently, validate_port_binding() is - # called as part of either a create or update transaction. The - # fix for bug 1276391 will change it to be called outside any - # transaction (or eliminate it altogether), so the - # context.original* will no longer be available. - self._check_port_context(context, context.original is not None) - return True - - def unbind_port(self, context): - # REVISIT(rkukura): Currently, unbind_port() is called as part - # of either an update or delete transaction. The fix for bug - # 1276391 will change it to be called outside any transaction - # (or eliminate it altogether), so the context.original* will - # no longer be available. - self._check_port_context(context, context.original is not None) + self.bound_ports.add(context.current['id'])