From b9b820954da825f3c80c782fcffcaa5dd4643c22 Mon Sep 17 00:00:00 2001 From: Mark Goddard Date: Fri, 12 May 2017 18:00:02 +0100 Subject: [PATCH] Physical network aware VIF attachment When attaching virtual interfaces to ironic ports and portgroups, we need to take account of the physical network assigned to those ports and portgroups. A neutron virtual interface has a set of physical networks on which it can be attached which is governed by the segments of its network (of which there may be more than one). This change makes the ironic VIF attach API physical network-aware using the physical network information added to the port object. When selecting a port or portgroup to attach a virtual interface to, the following ordered criteria are applied: * Require ports or portgroups to have a physical network that is 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. The change is backwards compatible, as the old behaviour is maintained when ports have a physical_network field which is None. Change-Id: I3d13bfacfb5578f570791e3c06e769a9a0140a4c Partial-Bug: #1666009 --- ironic/common/exception.py | 5 + ironic/common/network.py | 14 + ironic/common/neutron.py | 134 +++++-- ironic/conductor/manager.py | 7 + ironic/drivers/modules/network/common.py | 194 ++++++++-- ironic/tests/unit/common/test_network.py | 28 ++ ironic/tests/unit/common/test_neutron.py | 225 +++++++++++ ironic/tests/unit/conductor/test_manager.py | 30 ++ .../drivers/modules/network/test_common.py | 360 ++++++++++++++++-- 9 files changed, 902 insertions(+), 95 deletions(-) diff --git a/ironic/common/exception.py b/ironic/common/exception.py index 283a7207be..745d01b1cd 100644 --- a/ironic/common/exception.py +++ b/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") diff --git a/ironic/common/network.py b/ironic/common/network.py index 895ff8741e..d36c9cbff7 100644 --- a/ironic/common/network.py +++ b/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) diff --git a/ironic/common/neutron.py b/ironic/common/neutron.py index a60a658f63..8263cf1b8c 100644 --- a/ironic/common/neutron.py +++ b/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): - filters = {'id': uuid_or_name} - else: - filters = {'name': uuid_or_name} - - 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] + client = get_client() + network = _get_network_by_uuid_or_name(client, uuid_or_name, + net_type=net_type, fields=['id']) + return network['id'] 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 diff --git a/ironic/conductor/manager.py b/ironic/conductor/manager.py index c2cea5fa74..5c21d4c530 100644 --- a/ironic/conductor/manager.py +++ b/ironic/conductor/manager.py @@ -2586,6 +2586,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 @@ -2601,6 +2603,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, diff --git a/ironic/drivers/modules/network/common.py b/ironic/drivers/modules/network/common.py index 1a123ca4b2..d00aca2000 100644 --- a/ironic/drivers/modules/network/common.py +++ b/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 - free_ports = [] + # This list contains ports and portgroups selected as candidates for + # 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 - first free port group. If there are no free port groups, then the first - available port (pxe_enabled preferably) is used. + Ensures that the VIF is not already attached to this node. When selecting + a port or portgroup to attach the virtual interface to, the following + 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 free_portgroups: - # portgroups are higher priority - return free_portgroups[0] - - if not free_ports: + if not free_port_like_objs: raise exception.NoFreePhysicalPorts(vif=vif_id) - # Sort ports by pxe_enabled to ensure we always bind pxe_enabled ports - # first - sorted_free_ports = sorted(free_ports, key=lambda p: p.pxe_enabled, - reverse=True) - return sorted_free_ports[0] + def sort_key(port_like_obj): + """Key function for sorting a combined list of ports and portgroups. + + We key the port-like objects using the following precedence: + + 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 - group. If there are no free port groups, then the first available port - (pxe_enabled preferably) is used. + Attach a virtual interface to a node. When selecting a port or + portgroup to attach the virtual interface to, the following 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_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 diff --git a/ironic/tests/unit/common/test_network.py b/ironic/tests/unit/common/test_network.py index 7c3b2b000f..7afb9f4467 100644 --- a/ironic/tests/unit/common/test_network.py +++ b/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) diff --git a/ironic/tests/unit/common/test_neutron.py b/ironic/tests/unit/common/test_neutron.py index e0c302c34c..1c6c87e8db 100644 --- a/ironic/tests/unit/common/test_neutron.py +++ b/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) diff --git a/ironic/tests/unit/conductor/test_manager.py b/ironic/tests/unit/conductor/test_manager.py index 47224a3f25..243ccc9d50 100644 --- a/ironic/tests/unit/conductor/test_manager.py +++ b/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): diff --git a/ironic/tests/unit/drivers/modules/network/test_common.py b/ironic/tests/unit/drivers/modules/network/test_common.py index c43b00a73c..9245514737 100644 --- a/ironic/tests/unit/drivers/modules/network/test_common.py +++ b/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 = ( - common._get_free_portgroups_and_ports(task, self.vif_id)) + free_port_like_objs = ( + 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]) - self.assertItemsEqual([pg1.uuid], [p.uuid for p in free_portgroups]) + [p.uuid for p in free_port_like_objs]) + + 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 = ( - common._get_free_portgroups_and_ports(task, self.vif_id)) - self.assertItemsEqual([], free_ports) + free_port_like_objs = ( + common._get_free_portgroups_and_ports(task, self.vif_id, + {'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 = ( - common._get_free_portgroups_and_ports(task, self.vif_id)) + free_port_like_objs = ( + 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 = ( - common._get_free_portgroups_and_ports(task, self.vif_id)) + free_port_like_objs = ( + 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_gfp): + @mock.patch.object(neutron_common, 'get_physnets_by_port_uuid', + 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: