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 8398384a94..50a53054f0 100644 --- a/ironic/conductor/manager.py +++ b/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, 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 454034aa7d..7adef48a9c 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: