Merge "Physical network aware VIF attachment"

ironic/common/exception.py

@@ -756,3 +756,8 @@ class PortgroupPhysnetInconsistent(IronicException):
     _msg_fmt = _("Port group %(portgroup)s has member ports with inconsistent "
                  "physical networks (%(physical_networks)s). All ports in a "
                  "port group must have the same physical network.")
+
+
+class VifInvalidForAttach(Conflict):
+    _msg_fmt = _("Unable to attach VIF %(vif)s to node %(node)s. Reason: "
+                 "%(reason)s")

ironic/common/network.py

@@ -64,3 +64,17 @@ def get_ports_by_portgroup_id(task, portgroup_id):
     :returns: A list of Port objects.
     """
     return [port for port in task.ports if port.portgroup_id == portgroup_id]
+
+
+def get_physnets_for_node(task):
+    """Return the set of physical networks for a node.
+
+    Returns the set of physical networks associated with a node's ports. The
+    physical network None is excluded from the set.
+
+    :param task: a TaskManager instance
+    :returns: A set of physical networks.
+    """
+    return set(port.physical_network
+               for port in task.ports
+               if port.physical_network is not None)

ironic/common/neutron.py

@@ -26,6 +26,12 @@ DEFAULT_NEUTRON_URL = 'http://%s:9696' % CONF.my_ip
 
 _NEUTRON_SESSION = None
 
+PHYSNET_PARAM_NAME = 'provider:physical_network'
+"""Name of the neutron network API physical network parameter."""
+
+SEGMENTS_PARAM_NAME = 'segments'
+"""Name of the neutron network API segments parameter."""
+
 
 def _get_neutron_session():
     global _NEUTRON_SESSION
@@ -365,32 +371,10 @@ def validate_network(uuid_or_name, net_type=_('network')):
         raise exception.MissingParameterValue(
             _('UUID or name of %s is not set in configuration') % net_type)
 
-    if uuidutils.is_uuid_like(uuid_or_name):
+    client = get_client()
-        filters = {'id': uuid_or_name}
+    network = _get_network_by_uuid_or_name(client, uuid_or_name,
-    else:
+                                           net_type=net_type, fields=['id'])
-        filters = {'name': uuid_or_name}
+    return network['id']
-
-    try:
-        client = get_client()
-        networks = client.list_networks(fields=['id'], **filters)
-    except neutron_exceptions.NeutronClientException as exc:
-        raise exception.NetworkError(_('Could not retrieve network list: %s') %
-                                     exc)
-
-    LOG.debug('Got list of networks matching %(cond)s: %(result)s',
-              {'cond': filters, 'result': networks})
-    networks = [n['id'] for n in networks.get('networks', [])]
-    if not networks:
-        raise exception.InvalidParameterValue(
-            _('%(type)s with name or UUID %(uuid_or_name)s was not found') %
-            {'type': net_type, 'uuid_or_name': uuid_or_name})
-    elif len(networks) > 1:
-        raise exception.InvalidParameterValue(
-            _('More than one %(type)s was found for name %(name)s: %(nets)s') %
-            {'name': uuid_or_name, 'nets': ', '.join(networks),
-             'type': net_type})
-
-    return networks[0]
 
 
 def validate_port_info(node, port):
@@ -414,6 +398,104 @@ def validate_port_info(node, port):
     return True
 
 
+def _get_network_by_uuid_or_name(client, uuid_or_name, net_type=_('network'),
+                                 **params):
+    """Return a neutron network by UUID or name.
+
+    :param client: A Neutron client object.
+    :param uuid_or_name: network UUID or name
+    :param net_type: human-readable network type for error messages
+    :param params: Additional parameters to pass to the neutron client
+        list_networks method.
+    :returns: A dict describing the neutron network.
+    :raises: NetworkError on failure to contact Neutron
+    :raises: InvalidParameterValue for missing or duplicated network
+    """
+    if uuidutils.is_uuid_like(uuid_or_name):
+        params['id'] = uuid_or_name
+    else:
+        params['name'] = uuid_or_name
+
+    try:
+        networks = client.list_networks(**params)
+    except neutron_exceptions.NeutronClientException as exc:
+        raise exception.NetworkError(_('Could not retrieve network list: %s') %
+                                     exc)
+
+    LOG.debug('Got list of networks matching %(cond)s: %(result)s',
+              {'cond': params, 'result': networks})
+    networks = networks.get('networks', [])
+    if not networks:
+        raise exception.InvalidParameterValue(
+            _('%(type)s with name or UUID %(uuid_or_name)s was not found') %
+            {'type': net_type, 'uuid_or_name': uuid_or_name})
+    elif len(networks) > 1:
+        network_ids = [n['id'] for n in networks]
+        raise exception.InvalidParameterValue(
+            _('More than one %(type)s was found for name %(name)s: %(nets)s') %
+            {'name': uuid_or_name, 'nets': ', '.join(network_ids),
+             'type': net_type})
+    return networks[0]
+
+
+def _get_port_by_uuid(client, port_uuid, **params):
+    """Return a neutron port by UUID.
+
+    :param client: A Neutron client object.
+    :param port_uuid: UUID of a Neutron port to query.
+    :param params: Additional parameters to pass to the neutron client
+        show_port method.
+    :returns: A dict describing the neutron port.
+    :raises: InvalidParameterValue if the port does not exist.
+    :raises: NetworkError on failure to contact Neutron.
+    """
+    try:
+        port = client.show_port(port_uuid, **params)
+    except neutron_exceptions.PortNotFoundClient:
+        raise exception.InvalidParameterValue(
+            _('Neutron port %(port_uuid)s was not found') %
+            {'port_uuid': port_uuid})
+    except neutron_exceptions.NeutronClientException as exc:
+        raise exception.NetworkError(_('Could not retrieve neutron port: %s') %
+                                     exc)
+    return port['port']
+
+
+def get_physnets_by_port_uuid(client, port_uuid):
+    """Return the set of physical networks associated with a neutron port.
+
+    Query the network to which the port is attached and return the set of
+    physical networks associated with the segments in that network.
+
+    :param client: A Neutron client object.
+    :param port_uuid: UUID of a Neutron port to query.
+    :returns: A set of physical networks.
+    :raises: NetworkError if the network query fails.
+    :raises: InvalidParameterValue for missing network.
+    """
+    port = _get_port_by_uuid(client, port_uuid, fields=['network_id'])
+    network_uuid = port['network_id']
+
+    fields = [PHYSNET_PARAM_NAME, SEGMENTS_PARAM_NAME]
+    network = _get_network_by_uuid_or_name(client, network_uuid, fields=fields)
+
+    if SEGMENTS_PARAM_NAME in network:
+        # A network with multiple segments will have a 'segments' parameter
+        # which will contain a list of segments. Each segment should have a
+        # 'provider:physical_network' parameter which contains the physical
+        # network of the segment.
+        segments = network[SEGMENTS_PARAM_NAME]
+    else:
+        # A network with a single segment will have a
+        # 'provider:physical_network' parameter which contains the network's
+        # physical network.
+        segments = [network]
+
+    return set(segment[PHYSNET_PARAM_NAME]
+               for segment in segments
+               if segment[PHYSNET_PARAM_NAME])
+
+
 class NeutronNetworkInterfaceMixin(object):
 
     _cleaning_network_uuid = None

ironic/conductor/manager.py

@@ -2582,6 +2582,8 @@ class ConductorManager(base_manager.BaseConductorManager):
                                    exception.NetworkError,
                                    exception.VifAlreadyAttached,
                                    exception.NoFreePhysicalPorts,
+                                   exception.PortgroupPhysnetInconsistent,
+                                   exception.VifInvalidForAttach,
                                    exception.InvalidParameterValue)
     def vif_attach(self, context, node_id, vif_info):
         """Attach a VIF to a node
@@ -2597,6 +2599,11 @@ class ConductorManager(base_manager.BaseConductorManager):
         :raises: NetworkError, if an error occurs during attaching the VIF.
         :raises: InvalidParameterValue, if a parameter that's required for
             VIF attach is wrong/missing.
+        :raises: PortgroupPhysnetInconsistent if one of the node's portgroups
+                 has ports which are not all assigned the same physical
+                 network.
+        :raises: VifInvalidForAttach if the VIF is not valid for attachment to
+                 the node.
         """
         LOG.debug("RPC vif_attach called for the node %(node_id)s with "
                   "vif_info %(vif_info)s", {'node_id': node_id,

ironic/drivers/modules/network/common.py

@@ -22,6 +22,7 @@ from oslo_log import log
 from ironic.common import dhcp_factory
 from ironic.common import exception
 from ironic.common.i18n import _
+from ironic.common import network
 from ironic.common import neutron
 from ironic.common import states
 from ironic.common import utils
@@ -54,7 +55,35 @@ def _vif_attached(port_like_obj, vif_id):
     return attached_vif_id is not None
 
 
-def _get_free_portgroups_and_ports(task, vif_id):
+def _is_port_physnet_allowed(port, physnets):
+    """Check whether a port's physical network is allowed for a VIF.
+
+    Supports VIFs on networks with no physical network configuration by
+    allowing all ports regardless of their physical network. This will be the
+    case when the port is not a neutron port because we're in standalone mode
+    or not using neutron.
+
+    Allows ports with physical_network=None to ensure backwards compatibility
+    and provide support for simple deployments with no physical network
+    configuration in ironic.
+
+    When the physnets set is not empty and the port's physical_network field is
+    not None, the port's physical_network field must be present in the physnets
+    set.
+
+    :param port: A Port object to check.
+    :param physnets: Set of physical networks on which the VIF may be
+        attached. This is governed by the segments of the VIF's network. An
+        empty set indicates that the ports' physical networks should be
+        ignored.
+    :returns: True if the port's physical network is allowed, False otherwise.
+    """
+    return (not physnets or
+            port.physical_network is None or
+            port.physical_network in physnets)
+
+
+def _get_free_portgroups_and_ports(task, vif_id, physnets):
     """Get free portgroups and ports.
 
     It only returns ports or portgroups that can be used for attachment of
@@ -62,13 +91,18 @@ def _get_free_portgroups_and_ports(task, vif_id):
 
     :param task: a TaskManager instance.
     :param vif_id: Name or UUID of a VIF.
-    :returns: tuple of: list of free portgroups, list of free ports.
+    :param physnets: Set of physical networks on which the VIF may be
+        attached. This is governed by the segments of the VIF's network. An
+        empty set indicates that the ports' physical networks should be
+        ignored.
+    :returns: list of free ports and portgroups.
     :raises: VifAlreadyAttached, if vif_id is attached to any of the
         node's ports or portgroups.
     """
 
-    # This list contains ports selected as candidates for attachment
+    # This list contains ports and portgroups selected as candidates for
-    free_ports = []
+    # attachment.
+    free_port_like_objs = []
     # This is a mapping of portgroup id to collection of its free ports
     ports_by_portgroup = collections.defaultdict(list)
     # This set contains IDs of portgroups that should be ignored, as they have
@@ -84,58 +118,105 @@ def _get_free_portgroups_and_ports(task, vif_id):
             # added in this set is not a problem
             non_usable_portgroups.add(p.portgroup_id)
             continue
+        if not _is_port_physnet_allowed(p, physnets):
+            continue
         if p.portgroup_id is None:
             # ports without portgroup_id are always considered candidates
-            free_ports.append(p)
+            free_port_like_objs.append(p)
         else:
             ports_by_portgroup[p.portgroup_id].append(p)
 
-    # This list contains portgroups selected as candidates for attachment
-    free_portgroups = []
-
     for pg in task.portgroups:
         if _vif_attached(pg, vif_id):
             continue
         if pg.id in non_usable_portgroups:
             # This portgroup has vifs attached to its ports, consider its
             # ports instead to avoid collisions
-            free_ports.extend(ports_by_portgroup[pg.id])
+            free_port_like_objs.extend(ports_by_portgroup[pg.id])
         # Also ignore empty portgroups
         elif ports_by_portgroup[pg.id]:
-            free_portgroups.append(pg)
+            free_port_like_objs.append(pg)
 
-    return free_portgroups, free_ports
+    return free_port_like_objs
 
 
-def get_free_port_like_object(task, vif_id):
+def _get_physnet_for_portgroup(task, portgroup):
+    """Return the physical network associated with a portgroup.
+
+    :param task: a TaskManager instance.
+    :param portgroup: a Portgroup object.
+    :returns: The physical network associated with the portgroup.
+    :raises: PortgroupPhysnetInconsistent if the portgroup's ports are not
+             assigned the same physical network.
+    """
+    pg_ports = network.get_ports_by_portgroup_id(task, portgroup.id)
+    pg_physnets = {port.physical_network for port in pg_ports}
+    # Sanity check: there should be at least one port in the portgroup and
+    # all ports should have the same physical network.
+    if len(pg_physnets) != 1:
+        raise exception.PortgroupPhysnetInconsistent(
+            portgroup=portgroup.uuid, physical_networks=", ".join(pg_physnets))
+    return pg_physnets.pop()
+
+
+def get_free_port_like_object(task, vif_id, physnets):
     """Find free port-like object (portgroup or port) VIF will be attached to.
 
-    Ensures that VIF is not already attached to this node. It will return the
+    Ensures that the VIF is not already attached to this node.  When selecting
-    first free port group. If there are no free port groups, then the first
+    a port or portgroup to attach the virtual interface to, the following
-    available port (pxe_enabled preferably) is used.
+    ordered criteria are applied:
+
+    * Require ports or portgroups to have a physical network that is either
+      None or one of the VIF's allowed physical networks.
+    * Prefer ports or portgroups with a physical network field which is not
+      None.
+    * Prefer portgroups to ports.
+    * Prefer ports with PXE enabled.
 
     :param task: a TaskManager instance.
     :param vif_id: Name or UUID of a VIF.
+    :param physnets: Set of physical networks on which the VIF may be
+        attached. This is governed by the segments of the VIF's network. An
+        empty set indicates that the ports' physical networks should be
+        ignored.
     :raises: VifAlreadyAttached, if VIF is already attached to the node.
     :raises: NoFreePhysicalPorts, if there is no port-like object VIF can be
         attached to.
+    :raises: PortgroupPhysnetInconsistent if one of the node's portgroups
+             has ports which are not all assigned the same physical network.
     :returns: port-like object VIF will be attached to.
     """
+    free_port_like_objs = _get_free_portgroups_and_ports(task, vif_id,
+                                                         physnets)
 
-    free_portgroups, free_ports = _get_free_portgroups_and_ports(task, vif_id)
+    if not free_port_like_objs:
-
-    if free_portgroups:
-        # portgroups are higher priority
-        return free_portgroups[0]
-
-    if not free_ports:
         raise exception.NoFreePhysicalPorts(vif=vif_id)
 
-    # Sort ports by pxe_enabled to ensure we always bind pxe_enabled ports
+    def sort_key(port_like_obj):
-    # first
+        """Key function for sorting a combined list of ports and portgroups.
-    sorted_free_ports = sorted(free_ports, key=lambda p: p.pxe_enabled,
+
-                               reverse=True)
+        We key the port-like objects using the following precedence:
-    return sorted_free_ports[0]
+
+        1. Prefer objects with a physical network field which is in the
+           physnets set.
+        2. Prefer portgroups to ports.
+        3. Prefer ports with PXE enabled.
+
+        :param port_like_obj: The port or portgroup to key.
+        :returns: A key value for sorting the object.
+        """
+        is_pg = isinstance(port_like_obj, objects.Portgroup)
+        if is_pg:
+            pg_physnet = _get_physnet_for_portgroup(task, port_like_obj)
+            physnet_matches = pg_physnet in physnets
+            pxe_enabled = True
+        else:
+            physnet_matches = port_like_obj.physical_network in physnets
+            pxe_enabled = port_like_obj.pxe_enabled
+        return (physnet_matches, is_pg, pxe_enabled)
+
+    sorted_free_plos = sorted(free_port_like_objs, key=sort_key, reverse=True)
+    return sorted_free_plos[0]
 
 
 def plug_port_to_tenant_network(task, port_like_obj, client=None):
@@ -347,21 +428,64 @@ class VIFPortIDMixin(object):
     def vif_attach(self, task, vif_info):
         """Attach a virtual network interface to a node
 
-        Attach a virtual interface to a node. It will use the first free port
+        Attach a virtual interface to a node.  When selecting a port or
-        group. If there are no free port groups, then the first available port
+        portgroup to attach the virtual interface to, the following ordered
-        (pxe_enabled preferably) is used.
+        criteria are applied:
+
+        * Require ports or portgroups to have a physical network that is either
+          None or one of the VIF's allowed physical networks.
+        * Prefer ports or portgroups with a physical network field which is not
+          None.
+        * Prefer portgroups to ports.
+        * Prefer ports with PXE enabled.
 
         :param task: A TaskManager instance.
         :param vif_info: a dictionary of information about a VIF.
              It must have an 'id' key, whose value is a unique
              identifier for that VIF.
         :raises: NetworkError, VifAlreadyAttached, NoFreePhysicalPorts
+        :raises: PortgroupPhysnetInconsistent if one of the node's portgroups
+                 has ports which are not all assigned the same physical
+                 network.
         """
         vif_id = vif_info['id']
-
-        port_like_obj = get_free_port_like_object(task, vif_id)
-
         client = neutron.get_client()
+
+        # Determine whether any of the node's ports have a physical network. If
+        # not, we don't need to check the VIF's network's physical networks as
+        # they will not affect the VIF to port mapping.
+        physnets = set()
+        if any(port.physical_network is not None for port in task.ports):
+            try:
+                physnets = neutron.get_physnets_by_port_uuid(client, vif_id)
+            except (exception.InvalidParameterValue, exception.NetworkError):
+                # TODO(mgoddard): Remove this except clause and handle errors
+                # properly. We can do this once a strategy has been determined
+                # for handling the tempest VIF tests in an environment that
+                # may not support neutron.
+                # NOTE(sambetts): If a client error occurs this is because
+                # either neutron doesn't exist because we're running in
+                # standalone environment or we can't find a matching neutron
+                # port which means a user might be requesting a non-neutron
+                # port. So skip trying to update the neutron port MAC address
+                # in these cases.
+                pass
+
+            if len(physnets) > 1:
+                # NOTE(mgoddard): Neutron cannot currently handle hosts which
+                # are mapped to multiple segments in the same routed network.
+                node_physnets = network.get_physnets_for_node(task)
+                if len(node_physnets.intersection(physnets)) > 1:
+                    reason = _("Node has ports which map to multiple segments "
+                               "of the routed network to which the VIF is "
+                               "attached. Currently neutron only supports "
+                               "hosts which map to one segment of a routed "
+                               "network")
+                    raise exception.VifInvalidForAttach(
+                        node=task.node.uuid, vif=vif_id, reason=reason)
+
+        port_like_obj = get_free_port_like_object(task, vif_id, physnets)
+
         # Address is optional for portgroups
         if port_like_obj.address:
             # Check if the requested vif_id is a neutron port. If it is
@@ -369,6 +493,10 @@ class VIFPortIDMixin(object):
             try:
                 client.show_port(vif_id)
             except neutron_exceptions.NeutronClientException:
+                # TODO(mgoddard): Remove this except clause and handle errors
+                # properly. We can do this once a strategy has been determined
+                # for handling the tempest VIF tests in an environment that
+                # may not support neutron.
                 # NOTE(sambetts): If a client error occurs this is because
                 # either neutron doesn't exist because we're running in
                 # standalone environment or we can't find a matching neutron

ironic/tests/unit/common/test_network.py

@@ -199,3 +199,31 @@ class GetPortsByPortgroupIdTestCase(db_base.DbTestCase):
         with task_manager.acquire(self.context, node.uuid) as task:
             res = network.get_ports_by_portgroup_id(task, portgroup.id)
         self.assertEqual([], res)
+
+
+class GetPhysnetsForNodeTestCase(db_base.DbTestCase):
+
+    def test_get_physnets_for_node_no_ports(self):
+        node = object_utils.create_test_node(self.context, driver='fake')
+        with task_manager.acquire(self.context, node.uuid) as task:
+            res = network.get_physnets_for_node(task)
+        self.assertEqual(set(), res)
+
+    def test_get_physnets_for_node_excludes_None(self):
+        node = object_utils.create_test_node(self.context, driver='fake')
+        object_utils.create_test_port(self.context, node_id=node.id)
+        with task_manager.acquire(self.context, node.uuid) as task:
+            res = network.get_physnets_for_node(task)
+        self.assertEqual(set(), res)
+
+    def test_get_physnets_for_node_multiple_ports(self):
+        node = object_utils.create_test_node(self.context, driver='fake')
+        object_utils.create_test_port(self.context, node_id=node.id,
+                                      physical_network='physnet1')
+        object_utils.create_test_port(self.context, node_id=node.id,
+                                      uuid=uuidutils.generate_uuid(),
+                                      address='00:11:22:33:44:55',
+                                      physical_network='physnet2')
+        with task_manager.acquire(self.context, node.uuid) as task:
+            res = network.get_physnets_for_node(task)
+        self.assertEqual({'physnet1', 'physnet2'}, res)

ironic/tests/unit/common/test_neutron.py

@@ -656,3 +656,228 @@ class TestUnbindPort(base.TestCase):
                                                                      body)
         mock_log.info.assert_called_once_with('Port %s was not found while '
                                               'unbinding.', port_id)
+
+
+class TestGetNetworkByUUIDOrName(base.TestCase):
+
+    def setUp(self):
+        super(TestGetNetworkByUUIDOrName, self).setUp()
+        self.client = mock.MagicMock()
+
+    def test__get_network_by_uuid_or_name_uuid(self):
+        network_uuid = '9acb0256-2c1b-420a-b9d7-62bee90b6ed7'
+        networks = {
+            'networks': [{
+                'field1': 'value1',
+                'field2': 'value2',
+            }],
+        }
+        fields = ['field1', 'field2']
+        self.client.list_networks.return_value = networks
+        result = neutron._get_network_by_uuid_or_name(
+            self.client, network_uuid, fields=fields)
+        self.client.list_networks.assert_called_once_with(
+            id=network_uuid, fields=fields)
+        self.assertEqual(networks['networks'][0], result)
+
+    def test__get_network_by_uuid_or_name_name(self):
+        network_name = 'test-net'
+        networks = {
+            'networks': [{
+                'field1': 'value1',
+                'field2': 'value2',
+            }],
+        }
+        fields = ['field1', 'field2']
+        self.client.list_networks.return_value = networks
+        result = neutron._get_network_by_uuid_or_name(
+            self.client, network_name, fields=fields)
+        self.client.list_networks.assert_called_once_with(
+            name=network_name, fields=fields)
+        self.assertEqual(networks['networks'][0], result)
+
+    def test__get_network_by_uuid_or_name_failure(self):
+        network_uuid = '9acb0256-2c1b-420a-b9d7-62bee90b6ed7'
+        self.client.list_networks.side_effect = (
+            neutron_client_exc.NeutronClientException())
+        self.assertRaises(exception.NetworkError,
+                          neutron._get_network_by_uuid_or_name,
+                          self.client, network_uuid)
+        self.client.list_networks.assert_called_once_with(id=network_uuid)
+
+    def test__get_network_by_uuid_or_name_missing(self):
+        network_uuid = '9acb0256-2c1b-420a-b9d7-62bee90b6ed7'
+        networks = {
+            'networks': [],
+        }
+        self.client.list_networks.return_value = networks
+        self.assertRaises(exception.InvalidParameterValue,
+                          neutron._get_network_by_uuid_or_name,
+                          self.client, network_uuid)
+        self.client.list_networks.assert_called_once_with(id=network_uuid)
+
+    def test__get_network_by_uuid_or_name_duplicate(self):
+        network_name = 'test-net'
+        networks = {
+            'networks': [
+                {'id': '9acb0256-2c1b-420a-b9d7-62bee90b6ed7'},
+                {'id': '9014b6a7-8291-4676-80b0-ab00988ce3c7'},
+            ],
+        }
+        self.client.list_networks.return_value = networks
+        self.assertRaises(exception.InvalidParameterValue,
+                          neutron._get_network_by_uuid_or_name,
+                          self.client, network_name)
+        self.client.list_networks.assert_called_once_with(name=network_name)
+
+
+@mock.patch.object(neutron, '_get_network_by_uuid_or_name', autospec=True)
+@mock.patch.object(neutron, '_get_port_by_uuid', autospec=True)
+class TestGetPhysnetsByPortUUID(base.TestCase):
+
+    PORT_FIELDS = ['network_id']
+    NETWORK_FIELDS = ['provider:physical_network', 'segments']
+
+    def setUp(self):
+        super(TestGetPhysnetsByPortUUID, self).setUp()
+        self.client = mock.MagicMock()
+
+    def test_get_physnets_by_port_uuid_single_segment(self, mock_gp, mock_gn):
+        port_uuid = 'fake-port-uuid'
+        network_uuid = 'fake-network-uuid'
+        physnet = 'fake-physnet'
+        mock_gp.return_value = {
+            'network_id': network_uuid,
+        }
+        mock_gn.return_value = {
+            'provider:physical_network': physnet,
+        }
+        result = neutron.get_physnets_by_port_uuid(self.client,
+                                                   port_uuid)
+        mock_gp.assert_called_once_with(self.client, port_uuid,
+                                        fields=self.PORT_FIELDS)
+        mock_gn.assert_called_once_with(self.client, network_uuid,
+                                        fields=self.NETWORK_FIELDS)
+        self.assertEqual({physnet}, result)
+
+    def test_get_physnets_by_port_uuid_single_segment_no_physnet(
+            self, mock_gp, mock_gn):
+        port_uuid = 'fake-port-uuid'
+        network_uuid = 'fake-network-uuid'
+        mock_gp.return_value = {
+            'network_id': network_uuid,
+        }
+        mock_gn.return_value = {
+            'provider:physical_network': None,
+        }
+        result = neutron.get_physnets_by_port_uuid(self.client,
+                                                   port_uuid)
+        mock_gp.assert_called_once_with(self.client, port_uuid,
+                                        fields=self.PORT_FIELDS)
+        mock_gn.assert_called_once_with(self.client, network_uuid,
+                                        fields=self.NETWORK_FIELDS)
+        self.assertEqual(set(), result)
+
+    def test_get_physnets_by_port_uuid_multiple_segments(self, mock_gp,
+                                                         mock_gn):
+        port_uuid = 'fake-port-uuid'
+        network_uuid = 'fake-network-uuid'
+        physnet1 = 'fake-physnet-1'
+        physnet2 = 'fake-physnet-2'
+        mock_gp.return_value = {
+            'network_id': network_uuid,
+        }
+        mock_gn.return_value = {
+            'segments': [
+                {
+                    'provider:physical_network': physnet1,
+                },
+                {
+                    'provider:physical_network': physnet2,
+                },
+            ],
+        }
+        result = neutron.get_physnets_by_port_uuid(self.client,
+                                                   port_uuid)
+        mock_gp.assert_called_once_with(self.client, port_uuid,
+                                        fields=self.PORT_FIELDS)
+        mock_gn.assert_called_once_with(self.client, network_uuid,
+                                        fields=self.NETWORK_FIELDS)
+        self.assertEqual({physnet1, physnet2}, result)
+
+    def test_get_physnets_by_port_uuid_multiple_segments_no_physnet(
+            self, mock_gp, mock_gn):
+        port_uuid = 'fake-port-uuid'
+        network_uuid = 'fake-network-uuid'
+        mock_gp.return_value = {
+            'network_id': network_uuid,
+        }
+        mock_gn.return_value = {
+            'segments': [
+                {
+                    'provider:physical_network': None,
+                },
+                {
+                    'provider:physical_network': None,
+                },
+            ],
+        }
+        result = neutron.get_physnets_by_port_uuid(self.client,
+                                                   port_uuid)
+        mock_gp.assert_called_once_with(self.client, port_uuid,
+                                        fields=self.PORT_FIELDS)
+        mock_gn.assert_called_once_with(self.client, network_uuid,
+                                        fields=self.NETWORK_FIELDS)
+        self.assertEqual(set(), result)
+
+    def test_get_physnets_by_port_uuid_port_missing(self, mock_gp, mock_gn):
+        port_uuid = 'fake-port-uuid'
+        mock_gp.side_effect = exception.InvalidParameterValue('error')
+        self.assertRaises(exception.InvalidParameterValue,
+                          neutron.get_physnets_by_port_uuid,
+                          self.client, port_uuid)
+        mock_gp.assert_called_once_with(self.client, port_uuid,
+                                        fields=self.PORT_FIELDS)
+        self.assertFalse(mock_gn.called)
+
+    def test_get_physnets_by_port_uuid_port_failure(self, mock_gp, mock_gn):
+        port_uuid = 'fake-port-uuid'
+        mock_gp.side_effect = exception.NetworkError
+        self.assertRaises(exception.NetworkError,
+                          neutron.get_physnets_by_port_uuid,
+                          self.client, port_uuid)
+        mock_gp.assert_called_once_with(self.client, port_uuid,
+                                        fields=self.PORT_FIELDS)
+        self.assertFalse(mock_gn.called)
+
+    def test_get_physnets_by_port_uuid_network_missing(
+            self, mock_gp, mock_gn):
+        port_uuid = 'fake-port-uuid'
+        network_uuid = 'fake-network-uuid'
+        mock_gp.return_value = {
+            'network_id': network_uuid,
+        }
+        mock_gn.side_effect = exception.InvalidParameterValue('error')
+        self.assertRaises(exception.InvalidParameterValue,
+                          neutron.get_physnets_by_port_uuid,
+                          self.client, port_uuid)
+        mock_gp.assert_called_once_with(self.client, port_uuid,
+                                        fields=self.PORT_FIELDS)
+        mock_gn.assert_called_once_with(self.client, network_uuid,
+                                        fields=self.NETWORK_FIELDS)
+
+    def test_get_physnets_by_port_uuid_network_failure(
+            self, mock_gp, mock_gn):
+        port_uuid = 'fake-port-uuid'
+        network_uuid = 'fake-network-uuid'
+        mock_gp.return_value = {
+            'network_id': network_uuid,
+        }
+        mock_gn.side_effect = exception.NetworkError
+        self.assertRaises(exception.NetworkError,
+                          neutron.get_physnets_by_port_uuid,
+                          self.client, port_uuid)
+        mock_gp.assert_called_once_with(self.client, port_uuid,
+                                        fields=self.PORT_FIELDS)
+        mock_gn.assert_called_once_with(self.client, network_uuid,
+                                        fields=self.NETWORK_FIELDS)

ironic/tests/unit/conductor/test_manager.py

@@ -3791,6 +3791,36 @@ class VifTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase):
         mock_valid.assert_called_once_with(mock.ANY, mock.ANY)
         mock_attach.assert_called_once_with(mock.ANY, mock.ANY, self.vif)
 
+    @mock.patch.object(n_flat.FlatNetwork, 'vif_attach', autpspec=True)
+    def test_vif_attach_raises_portgroup_physnet_inconsistent(
+            self, mock_attach, mock_valid):
+        mock_valid.side_effect = exception.PortgroupPhysnetInconsistent(
+            portgroup='fake-pg', physical_networks='fake-physnet')
+        node = obj_utils.create_test_node(self.context, driver='fake')
+        exc = self.assertRaises(messaging.rpc.ExpectedException,
+                                self.service.vif_attach,
+                                self.context, node.uuid, self.vif)
+        # Compare true exception hidden by @messaging.expected_exceptions
+        self.assertEqual(exception.PortgroupPhysnetInconsistent,
+                         exc.exc_info[0])
+        mock_valid.assert_called_once_with(mock.ANY, mock.ANY)
+        self.assertFalse(mock_attach.called)
+
+    @mock.patch.object(n_flat.FlatNetwork, 'vif_attach', autpspec=True)
+    def test_vif_attach_raises_vif_invalid_for_attach(
+            self, mock_attach, mock_valid):
+        mock_valid.side_effect = exception.VifInvalidForAttach(
+            node='fake-node', vif='fake-vif', reason='fake-reason')
+        node = obj_utils.create_test_node(self.context, driver='fake')
+        exc = self.assertRaises(messaging.rpc.ExpectedException,
+                                self.service.vif_attach,
+                                self.context, node.uuid, self.vif)
+        # Compare true exception hidden by @messaging.expected_exceptions
+        self.assertEqual(exception.VifInvalidForAttach,
+                         exc.exc_info[0])
+        mock_valid.assert_called_once_with(mock.ANY, mock.ANY)
+        self.assertFalse(mock_attach.called)
+
     @mock.patch.object(n_flat.FlatNetwork, 'vif_attach', autpspec=True)
     def test_vif_attach_validate_error(self, mock_attach,
                                        mock_valid):

ironic/tests/unit/drivers/modules/network/test_common.py

@@ -11,6 +11,7 @@
 #    under the License.
 
 import mock
+from neutronclient.common import exceptions as neutron_exceptions
 from oslo_config import cfg
 from oslo_utils import uuidutils
 
@@ -38,35 +39,45 @@ class TestCommonFunctions(db_base.DbTestCase):
         self.port = obj_utils.create_test_port(
             self.context, node_id=self.node.id, address='52:54:00:cf:2d:32')
         self.vif_id = "fake_vif_id"
+        self.client = mock.MagicMock()
 
-    def _objects_setup(self):
+    def _objects_setup(self, set_physnets):
         pg1 = obj_utils.create_test_portgroup(
             self.context, node_id=self.node.id)
         pg1_ports = []
-        # This portgroup contains 2 ports, both of them without VIF
+        # This portgroup contains 2 ports, both of them without VIF. The ports
+        # are assigned to physnet physnet1.
+        physical_network = 'physnet1' if set_physnets else None
         for i in range(2):
             pg1_ports.append(obj_utils.create_test_port(
                 self.context, node_id=self.node.id,
                 address='52:54:00:cf:2d:0%d' % i,
+                physical_network=physical_network,
                 uuid=uuidutils.generate_uuid(), portgroup_id=pg1.id))
         pg2 = obj_utils.create_test_portgroup(
             self.context, node_id=self.node.id, address='00:54:00:cf:2d:04',
             name='foo2', uuid=uuidutils.generate_uuid())
         pg2_ports = []
         # This portgroup contains 3 ports, one of them with 'some-vif'
-        # attached, so the two free ones should be considered standalone
+        # attached, so the two free ones should be considered standalone.
+        # The ports are assigned physnet physnet2.
+        physical_network = 'physnet2' if set_physnets else None
         for i in range(2, 4):
             pg2_ports.append(obj_utils.create_test_port(
                 self.context, node_id=self.node.id,
                 address='52:54:00:cf:2d:0%d' % i,
+                physical_network=physical_network,
                 uuid=uuidutils.generate_uuid(), portgroup_id=pg2.id))
         pg2_ports.append(obj_utils.create_test_port(
             self.context, node_id=self.node.id,
             address='52:54:00:cf:2d:04',
+            physical_network=physical_network,
             extra={'vif_port_id': 'some-vif'},
             uuid=uuidutils.generate_uuid(), portgroup_id=pg2.id))
         # This portgroup has 'some-vif-2' attached to it and contains one port,
-        # so neither portgroup nor port can be considered free
+        # so neither portgroup nor port can be considered free. The ports are
+        # assigned physnet3.
+        physical_network = 'physnet3' if set_physnets else None
         pg3 = obj_utils.create_test_portgroup(
             self.context, node_id=self.node.id, address='00:54:00:cf:2d:05',
             name='foo3', uuid=uuidutils.generate_uuid(),
@@ -74,37 +85,119 @@ class TestCommonFunctions(db_base.DbTestCase):
         pg3_ports = [obj_utils.create_test_port(
             self.context, node_id=self.node.id,
             address='52:54:00:cf:2d:05', uuid=uuidutils.generate_uuid(),
+            physical_network=physical_network,
             portgroup_id=pg3.id)]
         return pg1, pg1_ports, pg2, pg2_ports, pg3, pg3_ports
 
-    def test__get_free_portgroups_and_ports(self):
+    def test__get_free_portgroups_and_ports_no_port_physnets(self):
         self.node.network_interface = 'flat'
         self.node.save()
-        pg1, pg1_ports, pg2, pg2_ports, pg3, pg3_ports = self._objects_setup()
+        pg1, pg1_ports, pg2, pg2_ports, pg3, pg3_ports = self._objects_setup(
+            set_physnets=False)
         with task_manager.acquire(self.context, self.node.id) as task:
-            free_portgroups, free_ports = (
+            free_port_like_objs = (
-                common._get_free_portgroups_and_ports(task, self.vif_id))
+                common._get_free_portgroups_and_ports(task, self.vif_id,
+                                                      {'anyphysnet'}))
+        self.assertItemsEqual(
+            [pg1.uuid, self.port.uuid] + [p.uuid for p in pg2_ports[:2]],
+            [p.uuid for p in free_port_like_objs])
+
+    def test__get_free_portgroups_and_ports_no_physnets(self):
+        self.node.network_interface = 'flat'
+        self.node.save()
+        pg1, pg1_ports, pg2, pg2_ports, pg3, pg3_ports = self._objects_setup(
+            set_physnets=True)
+        with task_manager.acquire(self.context, self.node.id) as task:
+            free_port_like_objs = (
+                common._get_free_portgroups_and_ports(task, self.vif_id,
+                                                      set()))
+        self.assertItemsEqual(
+            [pg1.uuid, self.port.uuid] + [p.uuid for p in pg2_ports[:2]],
+            [p.uuid for p in free_port_like_objs])
+
+    def test__get_free_portgroups_and_ports_no_matching_physnet(self):
+        self.node.network_interface = 'flat'
+        self.node.save()
+        pg1, pg1_ports, pg2, pg2_ports, pg3, pg3_ports = self._objects_setup(
+            set_physnets=True)
+        with task_manager.acquire(self.context, self.node.id) as task:
+            free_port_like_objs = (
+                common._get_free_portgroups_and_ports(task, self.vif_id,
+                                                      {'notaphysnet'}))
+        self.assertItemsEqual(
+            [self.port.uuid],
+            [p.uuid for p in free_port_like_objs])
+
+    def test__get_free_portgroups_and_ports_physnet1(self):
+        self.node.network_interface = 'flat'
+        self.node.save()
+        pg1, pg1_ports, pg2, pg2_ports, pg3, pg3_ports = self._objects_setup(
+            set_physnets=True)
+        with task_manager.acquire(self.context, self.node.id) as task:
+            free_port_like_objs = (
+                common._get_free_portgroups_and_ports(task, self.vif_id,
+                                                      {'physnet1'}))
+        self.assertItemsEqual(
+            [pg1.uuid, self.port.uuid],
+            [p.uuid for p in free_port_like_objs])
+
+    def test__get_free_portgroups_and_ports_physnet2(self):
+        self.node.network_interface = 'flat'
+        self.node.save()
+        pg1, pg1_ports, pg2, pg2_ports, pg3, pg3_ports = self._objects_setup(
+            set_physnets=True)
+        with task_manager.acquire(self.context, self.node.id) as task:
+            free_port_like_objs = (
+                common._get_free_portgroups_and_ports(task, self.vif_id,
+                                                      {'physnet2'}))
         self.assertItemsEqual(
             [self.port.uuid] + [p.uuid for p in pg2_ports[:2]],
-            [p.uuid for p in free_ports])
+            [p.uuid for p in free_port_like_objs])
-        self.assertItemsEqual([pg1.uuid], [p.uuid for p in free_portgroups])
+
+    def test__get_free_portgroups_and_ports_physnet3(self):
+        self.node.network_interface = 'flat'
+        self.node.save()
+        pg1, pg1_ports, pg2, pg2_ports, pg3, pg3_ports = self._objects_setup(
+            set_physnets=True)
+        with task_manager.acquire(self.context, self.node.id) as task:
+            free_port_like_objs = (
+                common._get_free_portgroups_and_ports(task, self.vif_id,
+                                                      {'physnet3'}))
+        self.assertItemsEqual(
+            [self.port.uuid], [p.uuid for p in free_port_like_objs])
+
+    def test__get_free_portgroups_and_ports_all_physnets(self):
+        self.node.network_interface = 'flat'
+        self.node.save()
+        pg1, pg1_ports, pg2, pg2_ports, pg3, pg3_ports = self._objects_setup(
+            set_physnets=True)
+        physnets = {'physnet1', 'physnet2', 'physnet3'}
+        with task_manager.acquire(self.context, self.node.id) as task:
+            free_port_like_objs = (
+                common._get_free_portgroups_and_ports(task, self.vif_id,
+                                                      physnets))
+        self.assertItemsEqual(
+            [pg1.uuid, self.port.uuid] + [p.uuid for p in pg2_ports[:2]],
+            [p.uuid for p in free_port_like_objs])
 
     @mock.patch.object(neutron_common, 'validate_port_info', autospec=True)
     def test__get_free_portgroups_and_ports_neutron_missed(self, vpi_mock):
         vpi_mock.return_value = False
         with task_manager.acquire(self.context, self.node.id) as task:
-            free_portgroups, free_ports = (
+            free_port_like_objs = (
-                common._get_free_portgroups_and_ports(task, self.vif_id))
+                common._get_free_portgroups_and_ports(task, self.vif_id,
-        self.assertItemsEqual([], free_ports)
+                                                      {'anyphysnet'}))
+        self.assertItemsEqual([], free_port_like_objs)
 
     @mock.patch.object(neutron_common, 'validate_port_info', autospec=True)
     def test__get_free_portgroups_and_ports_neutron(self, vpi_mock):
         vpi_mock.return_value = True
         with task_manager.acquire(self.context, self.node.id) as task:
-            free_portgroups, free_ports = (
+            free_port_like_objs = (
-                common._get_free_portgroups_and_ports(task, self.vif_id))
+                common._get_free_portgroups_and_ports(task, self.vif_id,
+                                                      {'anyphysnet'}))
         self.assertItemsEqual(
-            [self.port.uuid], [p.uuid for p in free_ports])
+            [self.port.uuid], [p.uuid for p in free_port_like_objs])
 
     @mock.patch.object(neutron_common, 'validate_port_info', autospec=True)
     def test__get_free_portgroups_and_ports_flat(self, vpi_mock):
@@ -112,14 +205,18 @@ class TestCommonFunctions(db_base.DbTestCase):
         self.node.save()
         vpi_mock.return_value = True
         with task_manager.acquire(self.context, self.node.id) as task:
-            free_portgroups, free_ports = (
+            free_port_like_objs = (
-                common._get_free_portgroups_and_ports(task, self.vif_id))
+                common._get_free_portgroups_and_ports(task, self.vif_id,
+                                                      {'anyphysnet'}))
+        self.assertItemsEqual(
+            [self.port.uuid], [p.uuid for p in free_port_like_objs])
 
     @mock.patch.object(neutron_common, 'validate_port_info', autospec=True,
                        return_value=True)
     def test_get_free_port_like_object_ports(self, vpi_mock):
         with task_manager.acquire(self.context, self.node.id) as task:
-            res = common.get_free_port_like_object(task, self.vif_id)
+            res = common.get_free_port_like_object(task, self.vif_id,
+                                                   {'anyphysnet'})
             self.assertEqual(self.port.uuid, res.uuid)
 
     @mock.patch.object(neutron_common, 'validate_port_info', autospec=True,
@@ -131,9 +228,42 @@ class TestCommonFunctions(db_base.DbTestCase):
             self.context, node_id=self.node.id, address='52:54:00:cf:2d:33',
             uuid=uuidutils.generate_uuid())
         with task_manager.acquire(self.context, self.node.id) as task:
-            res = common.get_free_port_like_object(task, self.vif_id)
+            res = common.get_free_port_like_object(task, self.vif_id,
+                                                   {'anyphysnet'})
             self.assertEqual(other_port.uuid, res.uuid)
 
+    @mock.patch.object(neutron_common, 'validate_port_info', autospec=True,
+                       return_value=True)
+    def test_get_free_port_like_object_ports_physnet_match_first(self,
+                                                                 vpi_mock):
+        self.port.pxe_enabled = False
+        self.port.physical_network = 'physnet1'
+        self.port.save()
+        obj_utils.create_test_port(
+            self.context, node_id=self.node.id, address='52:54:00:cf:2d:33',
+            uuid=uuidutils.generate_uuid())
+        with task_manager.acquire(self.context, self.node.id) as task:
+            res = common.get_free_port_like_object(task, self.vif_id,
+                                                   {'physnet1'})
+            self.assertEqual(self.port.uuid, res.uuid)
+
+    @mock.patch.object(neutron_common, 'validate_port_info', autospec=True,
+                       return_value=True)
+    def test_get_free_port_like_object_ports_physnet_match_first2(self,
+                                                                  vpi_mock):
+        self.port.pxe_enabled = False
+        self.port.physical_network = 'physnet1'
+        self.port.save()
+        pg = obj_utils.create_test_portgroup(
+            self.context, node_id=self.node.id)
+        obj_utils.create_test_port(
+            self.context, node_id=self.node.id, address='52:54:00:cf:2d:01',
+            uuid=uuidutils.generate_uuid(), portgroup_id=pg.id)
+        with task_manager.acquire(self.context, self.node.id) as task:
+            res = common.get_free_port_like_object(task, self.vif_id,
+                                                   {'physnet1'})
+            self.assertEqual(self.port.uuid, res.uuid)
+
     @mock.patch.object(neutron_common, 'validate_port_info', autospec=True,
                        return_value=True)
     def test_get_free_port_like_object_portgroup_first(self, vpi_mock):
@@ -143,15 +273,38 @@ class TestCommonFunctions(db_base.DbTestCase):
             self.context, node_id=self.node.id, address='52:54:00:cf:2d:01',
             uuid=uuidutils.generate_uuid(), portgroup_id=pg.id)
         with task_manager.acquire(self.context, self.node.id) as task:
-            res = common.get_free_port_like_object(task, self.vif_id)
+            res = common.get_free_port_like_object(task, self.vif_id,
+                                                   {'anyphysnet'})
             self.assertEqual(pg.uuid, res.uuid)
 
+    @mock.patch.object(neutron_common, 'validate_port_info', autospec=True,
+                       return_value=True)
+    def test_get_free_port_like_object_portgroup_physnet_match_first(self,
+                                                                     vpi_mock):
+        pg1 = obj_utils.create_test_portgroup(
+            self.context, node_id=self.node.id)
+        obj_utils.create_test_port(
+            self.context, node_id=self.node.id, address='52:54:00:cf:2d:01',
+            uuid=uuidutils.generate_uuid(), portgroup_id=pg1.id)
+        pg2 = obj_utils.create_test_portgroup(
+            self.context, node_id=self.node.id, uuid=uuidutils.generate_uuid(),
+            name='pg2', address='52:54:00:cf:2d:01')
+        obj_utils.create_test_port(
+            self.context, node_id=self.node.id, address='52:54:00:cf:2d:02',
+            uuid=uuidutils.generate_uuid(), portgroup_id=pg2.id,
+            physical_network='physnet1')
+        with task_manager.acquire(self.context, self.node.id) as task:
+            res = common.get_free_port_like_object(task, self.vif_id,
+                                                   {'physnet1'})
+            self.assertEqual(pg2.uuid, res.uuid)
+
     @mock.patch.object(neutron_common, 'validate_port_info', autospec=True,
                        return_value=True)
     def test_get_free_port_like_object_ignores_empty_portgroup(self, vpi_mock):
         obj_utils.create_test_portgroup(self.context, node_id=self.node.id)
         with task_manager.acquire(self.context, self.node.id) as task:
-            res = common.get_free_port_like_object(task, self.vif_id)
+            res = common.get_free_port_like_object(task, self.vif_id,
+                                                   {'anyphysnet'})
             self.assertEqual(self.port.uuid, res.uuid)
 
     @mock.patch.object(neutron_common, 'validate_port_info', autospec=True,
@@ -169,7 +322,8 @@ class TestCommonFunctions(db_base.DbTestCase):
             self.context, node_id=self.node.id, address='52:54:00:cf:2d:02',
             uuid=uuidutils.generate_uuid(), portgroup_id=pg.id)
         with task_manager.acquire(self.context, self.node.id) as task:
-            res = common.get_free_port_like_object(task, self.vif_id)
+            res = common.get_free_port_like_object(task, self.vif_id,
+                                                   {'anyphysnet'})
             self.assertEqual(free_port.uuid, res.uuid)
 
     @mock.patch.object(neutron_common, 'validate_port_info', autospec=True,
@@ -186,7 +340,8 @@ class TestCommonFunctions(db_base.DbTestCase):
             self.assertRaisesRegex(
                 exception.VifAlreadyAttached,
                 r"already attached to Ironic Portgroup",
-                common.get_free_port_like_object, task, self.vif_id)
+                common.get_free_port_like_object,
+                task, self.vif_id, {'anyphysnet'})
 
     @mock.patch.object(neutron_common, 'validate_port_info', autospec=True,
                        return_value=True)
@@ -202,7 +357,8 @@ class TestCommonFunctions(db_base.DbTestCase):
             self.assertRaisesRegex(
                 exception.VifAlreadyAttached,
                 r"already attached to Ironic Portgroup",
-                common.get_free_port_like_object, task, self.vif_id)
+                common.get_free_port_like_object,
+                task, self.vif_id, {'anyphysnet'})
 
     @mock.patch.object(neutron_common, 'validate_port_info', autospec=True,
                        return_value=True)
@@ -213,7 +369,8 @@ class TestCommonFunctions(db_base.DbTestCase):
             self.assertRaisesRegex(
                 exception.VifAlreadyAttached,
                 r"already attached to Ironic Port\b",
-                common.get_free_port_like_object, task, self.vif_id)
+                common.get_free_port_like_object,
+                task, self.vif_id, {'anyphysnet'})
 
     @mock.patch.object(neutron_common, 'validate_port_info', autospec=True,
                        return_value=True)
@@ -225,7 +382,8 @@ class TestCommonFunctions(db_base.DbTestCase):
             self.assertRaisesRegex(
                 exception.VifAlreadyAttached,
                 r"already attached to Ironic Port\b",
-                common.get_free_port_like_object, task, self.vif_id)
+                common.get_free_port_like_object,
+                task, self.vif_id, {'anyphysnet'})
 
     @mock.patch.object(neutron_common, 'validate_port_info', autospec=True,
                        return_value=True)
@@ -235,7 +393,17 @@ class TestCommonFunctions(db_base.DbTestCase):
         with task_manager.acquire(self.context, self.node.id) as task:
             self.assertRaises(exception.NoFreePhysicalPorts,
                               common.get_free_port_like_object,
-                              task, self.vif_id)
+                              task, self.vif_id, {'anyphysnet'})
+
+    @mock.patch.object(neutron_common, 'validate_port_info', autospec=True,
+                       return_value=True)
+    def test_get_free_port_like_object_no_matching_physnets(self, vpi_mock):
+        self.port.physical_network = 'physnet1'
+        self.port.save()
+        with task_manager.acquire(self.context, self.node.id) as task:
+            self.assertRaises(exception.NoFreePhysicalPorts,
+                              common.get_free_port_like_object,
+                              task, self.vif_id, {'physnet2'})
 
     @mock.patch.object(neutron_common, 'get_client', autospec=True)
     def test_plug_port_to_tenant_network_client(self, mock_gc):
@@ -330,24 +498,58 @@ class TestVifPortIDMixin(db_base.DbTestCase):
     @mock.patch.object(common, 'get_free_port_like_object', autospec=True)
     @mock.patch.object(neutron_common, 'get_client', autospec=True)
     @mock.patch.object(neutron_common, 'update_port_address', autospec=True)
-    def test_vif_attach(self, mock_upa, mock_client, moc_gfp):
+    @mock.patch.object(neutron_common, 'get_physnets_by_port_uuid',
+                       autospec=True)
+    def test_vif_attach(self, mock_gpbpi, mock_upa, mock_client, mock_gfp):
         self.port.extra = {}
         self.port.save()
         vif = {'id': "fake_vif_id"}
-        moc_gfp.return_value = self.port
+        mock_gfp.return_value = self.port
         with task_manager.acquire(self.context, self.node.id) as task:
             self.interface.vif_attach(task, vif)
             self.port.refresh()
             self.assertEqual("fake_vif_id", self.port.internal_info.get(
                 common.TENANT_VIF_KEY))
             mock_client.assert_called_once_with()
+            self.assertFalse(mock_gpbpi.called)
+            mock_gfp.assert_called_once_with(task, 'fake_vif_id', set())
+            mock_client.return_value.show_port.assert_called_once_with(
+                'fake_vif_id')
             mock_upa.assert_called_once_with("fake_vif_id", self.port.address)
 
     @mock.patch.object(common, 'get_free_port_like_object', autospec=True)
-    @mock.patch.object(neutron_common, 'get_client')
+    @mock.patch.object(neutron_common, 'get_client', autospec=True)
+    @mock.patch.object(neutron_common, 'update_port_address', autospec=True)
+    @mock.patch.object(neutron_common, 'get_physnets_by_port_uuid',
+                       autospec=True)
+    def test_vif_attach_with_physnet(self, mock_gpbpi, mock_upa, mock_client,
+                                     mock_gfp):
+        self.port.extra = {}
+        self.port.physical_network = 'physnet1'
+        self.port.save()
+        vif = {'id': "fake_vif_id"}
+        mock_gpbpi.return_value = {'physnet1'}
+        mock_gfp.return_value = self.port
+        with task_manager.acquire(self.context, self.node.id) as task:
+            self.interface.vif_attach(task, vif)
+            self.port.refresh()
+            self.assertEqual("fake_vif_id", self.port.internal_info.get(
+                common.TENANT_VIF_KEY))
+            mock_client.assert_called_once_with()
+            mock_gpbpi.assert_called_once_with(mock_client.return_value,
+                                               'fake_vif_id')
+            mock_gfp.assert_called_once_with(task, 'fake_vif_id', {'physnet1'})
+            mock_client.return_value.show_port.assert_called_once_with(
+                'fake_vif_id')
+            mock_upa.assert_called_once_with("fake_vif_id", self.port.address)
+
+    @mock.patch.object(common, 'get_free_port_like_object', autospec=True)
+    @mock.patch.object(neutron_common, 'get_client', autospec=True)
     @mock.patch.object(neutron_common, 'update_port_address')
-    def test_vif_attach_portgroup_no_address(self, mock_upa, mock_client,
+    @mock.patch.object(neutron_common, 'get_physnets_by_port_uuid',
-                                             mock_gfp):
+                       autospec=True)
+    def test_vif_attach_portgroup_no_address(self, mock_gpbpi, mock_upa,
+                                             mock_client, mock_gfp):
         pg = obj_utils.create_test_portgroup(
             self.context, node_id=self.node.id, address=None)
         mock_gfp.return_value = pg
@@ -360,15 +562,23 @@ class TestVifPortIDMixin(db_base.DbTestCase):
             pg.refresh()
             self.assertEqual(vif['id'],
                              pg.internal_info[common.TENANT_VIF_KEY])
+            mock_client.assert_called_once_with()
+            self.assertFalse(mock_gpbpi.called)
+            mock_gfp.assert_called_once_with(task, 'fake_vif_id', set())
+            self.assertFalse(mock_client.return_value.show_port.called)
             self.assertFalse(mock_upa.called)
-            self.assertTrue(mock_client.called)
 
-    @mock.patch.object(neutron_common, 'get_client')
+    @mock.patch.object(neutron_common, 'get_client', autospec=True)
     @mock.patch.object(neutron_common, 'update_port_address')
-    def test_vif_attach_update_port_exception(self, mock_upa, mock_client):
+    @mock.patch.object(neutron_common, 'get_physnets_by_port_uuid',
+                       autospec=True)
+    def test_vif_attach_update_port_exception(self, mock_gpbpi, mock_upa,
+                                              mock_client):
         self.port.extra = {}
+        self.port.physical_network = 'physnet1'
         self.port.save()
         vif = {'id': "fake_vif_id"}
+        mock_gpbpi.return_value = {'physnet1'}
         mock_upa.side_effect = (
             exception.FailedToUpdateMacOnPort(port_id='fake'))
         with task_manager.acquire(self.context, self.node.id) as task:
@@ -376,6 +586,84 @@ class TestVifPortIDMixin(db_base.DbTestCase):
                 exception.NetworkError, "can not update Neutron port",
                 self.interface.vif_attach, task, vif)
             mock_client.assert_called_once_with()
+            mock_gpbpi.assert_called_once_with(mock_client.return_value,
+                                               'fake_vif_id')
+            mock_client.return_value.show_port.assert_called_once_with(
+                'fake_vif_id')
+
+    @mock.patch.object(common, 'get_free_port_like_object', autospec=True)
+    @mock.patch.object(neutron_common, 'get_client', autospec=True)
+    @mock.patch.object(neutron_common, 'update_port_address')
+    @mock.patch.object(neutron_common, 'get_physnets_by_port_uuid',
+                       autospec=True)
+    def test_vif_attach_neutron_absent(self, mock_gpbpi, mock_upa,
+                                       mock_client, mock_gfp):
+        self.port.extra = {}
+        self.port.physical_network = 'physnet1'
+        self.port.save()
+        vif = {'id': "fake_vif_id"}
+        mock_gfp.return_value = self.port
+        mock_client.return_value.show_port.side_effect = (
+            neutron_exceptions.NeutronClientException)
+        mock_gpbpi.side_effect = exception.NetworkError
+        with task_manager.acquire(self.context, self.node.id) as task:
+            self.interface.vif_attach(task, vif)
+            mock_client.assert_called_once_with()
+            mock_gpbpi.assert_called_once_with(mock_client.return_value,
+                                               'fake_vif_id')
+            mock_gfp.assert_called_once_with(task, 'fake_vif_id', set())
+            mock_client.return_value.show_port.assert_called_once_with(
+                'fake_vif_id')
+            self.assertFalse(mock_upa.called)
+
+    @mock.patch.object(common, 'get_free_port_like_object', autospec=True)
+    @mock.patch.object(neutron_common, 'get_client')
+    @mock.patch.object(neutron_common, 'update_port_address')
+    @mock.patch.object(neutron_common, 'get_physnets_by_port_uuid',
+                       autospec=True)
+    def test_vif_attach_portgroup_physnet_inconsistent(self, mock_gpbpi,
+                                                       mock_upa, mock_client,
+                                                       mock_gfp):
+        self.port.extra = {}
+        self.port.physical_network = 'physnet1'
+        self.port.save()
+        vif = {'id': "fake_vif_id"}
+        mock_gpbpi.return_value = {'anyphysnet'}
+        mock_gfp.side_effect = exception.PortgroupPhysnetInconsistent(
+            portgroup='fake-portgroup-id', physical_networks='physnet1')
+        with task_manager.acquire(self.context, self.node.id) as task:
+            self.assertRaises(
+                exception.PortgroupPhysnetInconsistent,
+                self.interface.vif_attach, task, vif)
+            mock_client.assert_called_once_with()
+            mock_gpbpi.assert_called_once_with(mock_client.return_value,
+                                               'fake_vif_id')
+            self.assertFalse(mock_upa.called)
+
+    @mock.patch.object(common, 'get_free_port_like_object', autospec=True)
+    @mock.patch.object(neutron_common, 'get_client')
+    @mock.patch.object(neutron_common, 'update_port_address')
+    @mock.patch.object(neutron_common, 'get_physnets_by_port_uuid',
+                       autospec=True)
+    def test_vif_attach_multiple_segment_mappings(self, mock_gpbpi, mock_upa,
+                                                  mock_client, mock_gfp):
+        self.port.extra = {}
+        self.port.physical_network = 'physnet1'
+        self.port.save()
+        obj_utils.create_test_port(
+            self.context, node_id=self.node.id, uuid=uuidutils.generate_uuid(),
+            address='52:54:00:cf:2d:33', physical_network='physnet2')
+        vif = {'id': "fake_vif_id"}
+        mock_gpbpi.return_value = {'physnet1', 'physnet2'}
+        with task_manager.acquire(self.context, self.node.id) as task:
+            self.assertRaises(
+                exception.VifInvalidForAttach,
+                self.interface.vif_attach, task, vif)
+            mock_client.assert_called_once_with()
+            mock_gpbpi.assert_called_once_with(mock_client.return_value,
+                                               'fake_vif_id')
+            self.assertFalse(mock_gfp.called)
+            self.assertFalse(mock_upa.called)
 
     def test_vif_detach_in_extra(self):
         with task_manager.acquire(self.context, self.node.id) as task: