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

diff --git a/neutron/plugins/ml2/drivers/mech_agent.py b/neutron/plugins/ml2/drivers/mech_agent.py
index 4e9b55434bb5defcecace6ac2859c964d48e36a4..61a24af6140d41e3f2cc1f29e6daebf98284de30 100644 (file)
--- 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 969d3e67f12f1c1a8e30494bf462c3b0fa52d16e..1eb584f12c3b7258200e407d84da61da295a17bf 100644 (file)
--- 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 11d82d766e5a94c0cc812fdb3bcea3d73ed1c88d..c5e4539424314a448445ef88509ffc9334cbd87f 100644 (file)
--- 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 3deb5e338a4745f3b45364fe0bef76eec6faa0d9..04ed8251971c538a762198e47826396b4f141e9d 100644 (file)
--- 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 678e92c18376ee086d630ed9f315ba3866cd12b7..5e20a4b354f8405153ebb23bdc24e0022490967e 100644 (file)
--- 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 c2ef3e44af67d26029b5e202430583ec119b72a7..b1d0f1ba8f975e6339994a615a4a930b20bb534f 100644 (file)
--- 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 0f6f6e850431e37f5cd7758a5e8369303976f9a4..c776e7a7a68e8a2436ece8af299d52f9b69f35fc 100644 (file)
--- 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 5cf13ae302f7a1cc5b6f05bd5ba794a9b01d9bd2..1192ab489ca264812d233d12dc2ebc032affddf5 100644 (file)
--- 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 f40626741327709e5640e23dc51e6d63d0e92680..6bb8659d03df2f043e9c0cec740e9daa16eb6aaf 100644 (file)
--- 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 60ac1a620700d250003658cb3fec7c3b6c4aa8d9..d607bdb821aa4374a2b940c001b12a6c71089518 100644 (file)
--- 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 66903c02bf6421ce2ad7ddba2d5c86a267a945ed..6fc708b0acec61f51e0ce3aa446d502e6c790e24 100644 (file)
--- 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 456d6f02cc318b9508c51dc65d6712fe8c57ae6f..fd709fb87ac4f62f4405b2061842f2e7fd396197 100644 (file)
--- 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()