From 72e285ac1aad2f5a4b246435cf4d4b0126fb3431 Mon Sep 17 00:00:00 2001 From: Assaf Muller Date: Tue, 30 Sep 2014 19:31:57 +0300 Subject: [PATCH] Improve agent-based flat/vlan ml2 port binding failure logging Port binding failure is an error and should be reported as such. Additionally, if a port binding fails on a host due to missing bridge mappings, it's currently quite a mystery to find out. This should be logged instead of requiring users to debug code. Not everyone enjoys debugging Python, as weird as that is! I refactored out the common code in check_segment_for_agent in order to make logging more robust for all agent-based mechanism drivers. The OVS and LB mech drivers already log due to a bridge mappings mismatch and the other agent based mech drivers will now log as well. Closes-Bug: #1377710 Change-Id: I451a0763908adcd845721e8cda7f3189da6c8b81 --- neutron/plugins/ml2/drivers/mech_agent.py | 59 +++++++++++++++++++ neutron/plugins/ml2/drivers/mech_hyperv.py | 24 ++++---- .../plugins/ml2/drivers/mech_linuxbridge.py | 32 +++------- neutron/plugins/ml2/drivers/mech_ofagent.py | 27 +++------ .../plugins/ml2/drivers/mech_openvswitch.py | 32 +++------- neutron/plugins/ml2/drivers/mlnx/mech_mlnx.py | 18 ++---- neutron/plugins/ml2/managers.py | 8 +-- .../tests/unit/ml2/drivers/test_mech_mlnx.py | 12 ++-- .../unit/ml2/drivers/test_ofagent_mech.py | 12 ++-- neutron/tests/unit/ml2/test_mech_hyperv.py | 12 ++-- .../tests/unit/ml2/test_mech_linuxbridge.py | 12 ++-- .../tests/unit/ml2/test_mech_openvswitch.py | 12 ++-- 12 files changed, 143 insertions(+), 117 deletions(-) diff --git a/neutron/plugins/ml2/drivers/mech_agent.py b/neutron/plugins/ml2/drivers/mech_agent.py index 4e9b55434bb..61a24af6140 100644 --- a/neutron/plugins/ml2/drivers/mech_agent.py +++ b/neutron/plugins/ml2/drivers/mech_agent.py @@ -19,6 +19,7 @@ import six from neutron.extensions import portbindings from neutron.i18n import _LW from neutron.openstack.common import log +from neutron.plugins.common import constants as p_constants from neutron.plugins.ml2 import driver_api as api LOG = log.getLogger(__name__) @@ -136,6 +137,26 @@ class SimpleAgentMechanismDriverBase(AgentMechanismDriverBase): return False @abc.abstractmethod + def get_allowed_network_types(self, agent=None): + """Return the agent's or driver's allowed network types. + + For example: return ('flat', ...). You can also refer to the + configuration the given agent exposes. + """ + pass + + @abc.abstractmethod + def get_mappings(self, agent): + """Return the agent's bridge or interface mappings. + + For example: agent['configurations'].get('bridge_mappings', {}). + """ + pass + + def physnet_in_mappings(self, physnet, mappings): + """Is the physical network part of the given mappings?""" + return physnet in mappings + def check_segment_for_agent(self, segment, agent): """Check if segment can be bound for agent. @@ -149,3 +170,41 @@ class SimpleAgentMechanismDriverBase(AgentMechanismDriverBase): determine whether or not the specified network segment can be bound for the agent. """ + + mappings = self.get_mappings(agent) + allowed_network_types = self.get_allowed_network_types(agent) + + LOG.debug("Checking segment: %(segment)s " + "for mappings: %(mappings)s " + "with network types: %(network_types)s", + {'segment': segment, 'mappings': mappings, + 'network_types': allowed_network_types}) + + network_type = segment[api.NETWORK_TYPE] + if network_type not in allowed_network_types: + LOG.debug( + 'Network %(network_id)s is of type %(network_type)s ' + 'but agent %(agent)s or mechanism driver only ' + 'support %(allowed_network_types)s.', + {'network_id': segment['id'], + 'network_type': network_type, + 'agent': agent['host'], + 'allowed_network_types': allowed_network_types}) + return False + + if network_type in [p_constants.TYPE_FLAT, p_constants.TYPE_VLAN]: + physnet = segment[api.PHYSICAL_NETWORK] + if not self.physnet_in_mappings(physnet, mappings): + LOG.debug( + 'Network %(network_id)s is connected to physical ' + 'network %(physnet)s, but agent %(agent)s reported ' + 'physical networks %(mappings)s. ' + 'The physical network must be configured on the ' + 'agent if binding is to succeed.', + {'network_id': segment['id'], + 'physnet': physnet, + 'agent': agent['host'], + 'mappings': mappings}) + return False + + return True diff --git a/neutron/plugins/ml2/drivers/mech_hyperv.py b/neutron/plugins/ml2/drivers/mech_hyperv.py index 969d3e67f12..1eb584f12c3 100644 --- a/neutron/plugins/ml2/drivers/mech_hyperv.py +++ b/neutron/plugins/ml2/drivers/mech_hyperv.py @@ -18,7 +18,7 @@ import re from neutron.common import constants from neutron.extensions import portbindings from neutron.openstack.common import log -from neutron.plugins.ml2 import driver_api as api +from neutron.plugins.common import constants as p_constants from neutron.plugins.ml2.drivers import mech_agent LOG = log.getLogger(__name__) @@ -39,16 +39,12 @@ class HypervMechanismDriver(mech_agent.SimpleAgentMechanismDriverBase): portbindings.VIF_TYPE_HYPERV, {portbindings.CAP_PORT_FILTER: False}) - def check_segment_for_agent(self, segment, agent): - mappings = agent['configurations'].get('vswitch_mappings', {}) - LOG.debug("Checking segment: %(segment)s " - "for mappings: %(mappings)s", - {'segment': segment, 'mappings': mappings}) - network_type = segment[api.NETWORK_TYPE] - if network_type == 'local': - return True - elif network_type in ['flat', 'vlan']: - for pattern in mappings: - if re.match(pattern, segment[api.PHYSICAL_NETWORK]): - return True - return False + def get_allowed_network_types(self, agent=None): + return [p_constants.TYPE_LOCAL, p_constants.TYPE_FLAT, + p_constants.TYPE_VLAN] + + def get_mappings(self, agent): + return agent['configurations'].get('vswitch_mappings', {}) + + def physnet_in_mappings(self, physnet, mappings): + return any(re.match(pattern, physnet) for pattern in mappings) diff --git a/neutron/plugins/ml2/drivers/mech_linuxbridge.py b/neutron/plugins/ml2/drivers/mech_linuxbridge.py index 11d82d766e5..c5e45394243 100644 --- a/neutron/plugins/ml2/drivers/mech_linuxbridge.py +++ b/neutron/plugins/ml2/drivers/mech_linuxbridge.py @@ -16,9 +16,8 @@ from neutron.agent import securitygroups_rpc from neutron.common import constants from neutron.extensions import portbindings -from neutron.i18n import _LW from neutron.openstack.common import log -from neutron.plugins.ml2 import driver_api as api +from neutron.plugins.common import constants as p_constants from neutron.plugins.ml2.drivers import mech_agent LOG = log.getLogger(__name__) @@ -41,25 +40,10 @@ class LinuxbridgeMechanismDriver(mech_agent.SimpleAgentMechanismDriverBase): portbindings.VIF_TYPE_BRIDGE, {portbindings.CAP_PORT_FILTER: sg_enabled}) - def check_segment_for_agent(self, segment, agent): - mappings = agent['configurations'].get('interface_mappings', {}) - tunnel_types = agent['configurations'].get('tunnel_types', []) - LOG.debug("Checking segment: %(segment)s " - "for mappings: %(mappings)s " - "with tunnel_types: %(tunnel_types)s", - {'segment': segment, 'mappings': mappings, - 'tunnel_types': tunnel_types}) - network_type = segment[api.NETWORK_TYPE] - if network_type == 'local': - return True - elif network_type in tunnel_types: - return True - elif network_type in ['flat', 'vlan']: - is_mapping_present = segment[api.PHYSICAL_NETWORK] in mappings - if not is_mapping_present: - LOG.warn(_LW("Failed to find %(seg)s in mappings %(map)s"), - {'seg': segment[api.PHYSICAL_NETWORK], - 'map': mappings}) - return is_mapping_present - else: - return False + def get_allowed_network_types(self, agent): + return (agent['configurations'].get('tunnel_types', []) + + [p_constants.TYPE_LOCAL, p_constants.TYPE_FLAT, + p_constants.TYPE_VLAN]) + + def get_mappings(self, agent): + return agent['configurations'].get('interface_mappings', {}) diff --git a/neutron/plugins/ml2/drivers/mech_ofagent.py b/neutron/plugins/ml2/drivers/mech_ofagent.py index 3deb5e338a4..04ed8251971 100644 --- a/neutron/plugins/ml2/drivers/mech_ofagent.py +++ b/neutron/plugins/ml2/drivers/mech_ofagent.py @@ -23,8 +23,7 @@ from neutron.agent import securitygroups_rpc from neutron.common import constants from neutron.extensions import portbindings from neutron.openstack.common import log -from neutron.plugins.common import constants as p_const -from neutron.plugins.ml2 import driver_api as api +from neutron.plugins.common import constants as p_constants from neutron.plugins.ml2.drivers import mech_agent LOG = log.getLogger(__name__) @@ -49,20 +48,10 @@ class OfagentMechanismDriver(mech_agent.SimpleAgentMechanismDriverBase): portbindings.VIF_TYPE_OVS, vif_details) - def check_segment_for_agent(self, segment, agent): - interface_mappings = agent['configurations'].get('interface_mappings', - {}) - tunnel_types = agent['configurations'].get('tunnel_types', []) - LOG.debug("Checking segment: %(segment)s " - "for interface_mappings: %(interface_mappings)s " - "with tunnel_types: %(tunnel_types)s", - {'segment': segment, - 'interface_mappings': interface_mappings, - 'tunnel_types': tunnel_types}) - network_type = segment[api.NETWORK_TYPE] - return ( - network_type == p_const.TYPE_LOCAL or - network_type in tunnel_types or - (network_type in [p_const.TYPE_FLAT, p_const.TYPE_VLAN] and - segment[api.PHYSICAL_NETWORK] in interface_mappings) - ) + def get_allowed_network_types(self, agent): + return (agent['configurations'].get('tunnel_types', []) + + [p_constants.TYPE_LOCAL, p_constants.TYPE_FLAT, + p_constants.TYPE_VLAN]) + + def get_mappings(self, agent): + return dict(agent['configurations'].get('interface_mappings', {})) diff --git a/neutron/plugins/ml2/drivers/mech_openvswitch.py b/neutron/plugins/ml2/drivers/mech_openvswitch.py index 678e92c1837..5e20a4b354f 100644 --- a/neutron/plugins/ml2/drivers/mech_openvswitch.py +++ b/neutron/plugins/ml2/drivers/mech_openvswitch.py @@ -16,9 +16,8 @@ from neutron.agent import securitygroups_rpc from neutron.common import constants from neutron.extensions import portbindings -from neutron.i18n import _LW from neutron.openstack.common import log -from neutron.plugins.ml2 import driver_api as api +from neutron.plugins.common import constants as p_constants from neutron.plugins.ml2.drivers import mech_agent LOG = log.getLogger(__name__) @@ -43,25 +42,10 @@ class OpenvswitchMechanismDriver(mech_agent.SimpleAgentMechanismDriverBase): portbindings.VIF_TYPE_OVS, vif_details) - def check_segment_for_agent(self, segment, agent): - mappings = agent['configurations'].get('bridge_mappings', {}) - tunnel_types = agent['configurations'].get('tunnel_types', []) - LOG.debug("Checking segment: %(segment)s " - "for mappings: %(mappings)s " - "with tunnel_types: %(tunnel_types)s", - {'segment': segment, 'mappings': mappings, - 'tunnel_types': tunnel_types}) - network_type = segment[api.NETWORK_TYPE] - if network_type == 'local': - return True - elif network_type in tunnel_types: - return True - elif network_type in ['flat', 'vlan']: - is_mapping_present = segment[api.PHYSICAL_NETWORK] in mappings - if not is_mapping_present: - LOG.warn(_LW("Failed to find %(seg)s in mappings %(map)s"), - {'seg': segment[api.PHYSICAL_NETWORK], - 'map': mappings}) - return is_mapping_present - else: - return False + def get_allowed_network_types(self, agent): + return (agent['configurations'].get('tunnel_types', []) + + [p_constants.TYPE_LOCAL, p_constants.TYPE_FLAT, + p_constants.TYPE_VLAN]) + + def get_mappings(self, agent): + return agent['configurations'].get('bridge_mappings', {}) diff --git a/neutron/plugins/ml2/drivers/mlnx/mech_mlnx.py b/neutron/plugins/ml2/drivers/mlnx/mech_mlnx.py index c2ef3e44af6..b1d0f1ba8f9 100644 --- a/neutron/plugins/ml2/drivers/mlnx/mech_mlnx.py +++ b/neutron/plugins/ml2/drivers/mlnx/mech_mlnx.py @@ -19,6 +19,7 @@ from oslo.config import cfg from neutron.common import constants from neutron.extensions import portbindings from neutron.openstack.common import log +from neutron.plugins.common import constants as p_constants from neutron.plugins.ml2 import driver_api as api from neutron.plugins.ml2.drivers import mech_agent from neutron.plugins.ml2.drivers.mlnx import config # noqa @@ -49,19 +50,12 @@ class MlnxMechanismDriver(mech_agent.SimpleAgentMechanismDriverBase): {portbindings.CAP_PORT_FILTER: False}, portbindings.VNIC_TYPES) - def check_segment_for_agent(self, segment, agent): - mappings = agent['configurations'].get('interface_mappings', {}) - LOG.debug("Checking segment: %(segment)s " - "for mappings: %(mappings)s ", - {'segment': segment, 'mappings': mappings}) + def get_allowed_network_types(self, agent=None): + return [p_constants.TYPE_LOCAL, p_constants.TYPE_FLAT, + p_constants.TYPE_VLAN] - network_type = segment[api.NETWORK_TYPE] - if network_type == 'local': - return True - elif network_type in ['flat', 'vlan']: - return segment[api.PHYSICAL_NETWORK] in mappings - else: - return False + def get_mappings(self, agent): + return agent['configurations'].get('interface_mappings', {}) def try_to_bind_segment_for_agent(self, context, segment, agent): if self.check_segment_for_agent(segment, agent): diff --git a/neutron/plugins/ml2/managers.py b/neutron/plugins/ml2/managers.py index 0f6f6e85043..c776e7a7a68 100644 --- a/neutron/plugins/ml2/managers.py +++ b/neutron/plugins/ml2/managers.py @@ -21,7 +21,7 @@ from neutron.common import exceptions as exc from neutron.extensions import multiprovidernet as mpnet from neutron.extensions import portbindings from neutron.extensions import providernet as provider -from neutron.i18n import _LE, _LI, _LW +from neutron.i18n import _LE, _LI from neutron.openstack.common import log from neutron.plugins.ml2.common import exceptions as ml2_exc from neutron.plugins.ml2 import db @@ -594,9 +594,9 @@ class MechanismManager(stevedore.named.NamedExtensionManager): "bind_port"), driver.name) binding.vif_type = portbindings.VIF_TYPE_BINDING_FAILED - LOG.warning(_LW("Failed to bind port %(port)s on host %(host)s"), - {'port': context._port['id'], - 'host': binding.host}) + LOG.error(_LE("Failed to bind port %(port)s on host %(host)s"), + {'port': context._port['id'], + 'host': binding.host}) class ExtensionManager(stevedore.named.NamedExtensionManager): diff --git a/neutron/tests/unit/ml2/drivers/test_mech_mlnx.py b/neutron/tests/unit/ml2/drivers/test_mech_mlnx.py index 5cf13ae302f..1192ab489ca 100644 --- a/neutron/tests/unit/ml2/drivers/test_mech_mlnx.py +++ b/neutron/tests/unit/ml2/drivers/test_mech_mlnx.py @@ -33,13 +33,17 @@ class MlnxMechanismBaseTestCase(base.AgentMechanismBaseTestCase): BAD_CONFIGS = {'interface_mappings': BAD_MAPPINGS} AGENTS = [{'alive': True, - 'configurations': GOOD_CONFIGS}] + 'configurations': GOOD_CONFIGS, + 'host': 'host'}] AGENTS_DEAD = [{'alive': False, - 'configurations': GOOD_CONFIGS}] + 'configurations': GOOD_CONFIGS, + 'host': 'dead_host'}] AGENTS_BAD = [{'alive': False, - 'configurations': GOOD_CONFIGS}, + 'configurations': GOOD_CONFIGS, + 'host': 'bad_host_1'}, {'alive': True, - 'configurations': BAD_CONFIGS}] + 'configurations': BAD_CONFIGS, + 'host': 'bad_host_2'}] def setUp(self): super(MlnxMechanismBaseTestCase, self).setUp() diff --git a/neutron/tests/unit/ml2/drivers/test_ofagent_mech.py b/neutron/tests/unit/ml2/drivers/test_ofagent_mech.py index f4062674132..6bb8659d03d 100644 --- a/neutron/tests/unit/ml2/drivers/test_ofagent_mech.py +++ b/neutron/tests/unit/ml2/drivers/test_ofagent_mech.py @@ -38,13 +38,17 @@ class OfagentMechanismBaseTestCase(base.AgentMechanismBaseTestCase): 'tunnel_types': BAD_TUNNEL_TYPES} AGENTS = [{'alive': True, - 'configurations': GOOD_CONFIGS}] + 'configurations': GOOD_CONFIGS, + 'host': 'host'}] AGENTS_DEAD = [{'alive': False, - 'configurations': GOOD_CONFIGS}] + 'configurations': GOOD_CONFIGS, + 'host': 'dead_host'}] AGENTS_BAD = [{'alive': False, - 'configurations': GOOD_CONFIGS}, + 'configurations': GOOD_CONFIGS, + 'host': 'bad_host_1'}, {'alive': True, - 'configurations': BAD_CONFIGS}] + 'configurations': BAD_CONFIGS, + 'host': 'bad_host_2'}] def setUp(self): super(OfagentMechanismBaseTestCase, self).setUp() diff --git a/neutron/tests/unit/ml2/test_mech_hyperv.py b/neutron/tests/unit/ml2/test_mech_hyperv.py index 60ac1a62070..d607bdb821a 100644 --- a/neutron/tests/unit/ml2/test_mech_hyperv.py +++ b/neutron/tests/unit/ml2/test_mech_hyperv.py @@ -31,13 +31,17 @@ class HypervMechanismBaseTestCase(base.AgentMechanismBaseTestCase): BAD_CONFIGS = {'vswitch_mappings': BAD_MAPPINGS} AGENTS = [{'alive': True, - 'configurations': GOOD_CONFIGS}] + 'configurations': GOOD_CONFIGS, + 'host': 'host'}] AGENTS_DEAD = [{'alive': False, - 'configurations': GOOD_CONFIGS}] + 'configurations': GOOD_CONFIGS, + 'host': 'dead_host'}] AGENTS_BAD = [{'alive': False, - 'configurations': GOOD_CONFIGS}, + 'configurations': GOOD_CONFIGS, + 'host': 'bad_host_1'}, {'alive': True, - 'configurations': BAD_CONFIGS}] + 'configurations': BAD_CONFIGS, + 'host': 'bad_host_2'}] def setUp(self): super(HypervMechanismBaseTestCase, self).setUp() diff --git a/neutron/tests/unit/ml2/test_mech_linuxbridge.py b/neutron/tests/unit/ml2/test_mech_linuxbridge.py index 66903c02bf6..6fc708b0ace 100644 --- a/neutron/tests/unit/ml2/test_mech_linuxbridge.py +++ b/neutron/tests/unit/ml2/test_mech_linuxbridge.py @@ -35,13 +35,17 @@ class LinuxbridgeMechanismBaseTestCase(base.AgentMechanismBaseTestCase): 'tunnel_types': BAD_TUNNEL_TYPES} AGENTS = [{'alive': True, - 'configurations': GOOD_CONFIGS}] + 'configurations': GOOD_CONFIGS, + 'host': 'host'}] AGENTS_DEAD = [{'alive': False, - 'configurations': GOOD_CONFIGS}] + 'configurations': GOOD_CONFIGS, + 'host': 'dead_host'}] AGENTS_BAD = [{'alive': False, - 'configurations': GOOD_CONFIGS}, + 'configurations': GOOD_CONFIGS, + 'host': 'bad_host_1'}, {'alive': True, - 'configurations': BAD_CONFIGS}] + 'configurations': BAD_CONFIGS, + 'host': 'bad_host_2'}] def setUp(self): super(LinuxbridgeMechanismBaseTestCase, self).setUp() diff --git a/neutron/tests/unit/ml2/test_mech_openvswitch.py b/neutron/tests/unit/ml2/test_mech_openvswitch.py index 456d6f02cc3..fd709fb87ac 100644 --- a/neutron/tests/unit/ml2/test_mech_openvswitch.py +++ b/neutron/tests/unit/ml2/test_mech_openvswitch.py @@ -38,13 +38,17 @@ class OpenvswitchMechanismBaseTestCase(base.AgentMechanismBaseTestCase): 'tunnel_types': BAD_TUNNEL_TYPES} AGENTS = [{'alive': True, - 'configurations': GOOD_CONFIGS}] + 'configurations': GOOD_CONFIGS, + 'host': 'host'}] AGENTS_DEAD = [{'alive': False, - 'configurations': GOOD_CONFIGS}] + 'configurations': GOOD_CONFIGS, + 'host': 'dead_host'}] AGENTS_BAD = [{'alive': False, - 'configurations': GOOD_CONFIGS}, + 'configurations': GOOD_CONFIGS, + 'host': 'bad_host_1'}, {'alive': True, - 'configurations': BAD_CONFIGS}] + 'configurations': BAD_CONFIGS, + 'host': 'bad_host_2'}] def setUp(self): super(OpenvswitchMechanismBaseTestCase, self).setUp()