diff --git a/ironic/common/exception.py b/ironic/common/exception.py index eeabccb944..9f8993d02f 100644 --- a/ironic/common/exception.py +++ b/ironic/common/exception.py @@ -226,6 +226,21 @@ class VolumeTargetBootIndexAlreadyExists(Conflict): "for the same node already exists.") +class VifAlreadyAttached(Conflict): + _msg_fmt = _("Unable to attach VIF because VIF %(vif)s is already " + "attached to Ironic port %(port_uuid)s") + + +class NoFreePhysicalPorts(Invalid): + _msg_fmt = _("Unable to attach VIF %(vif)s, not " + "enough free physical ports.") + + +class VifNotAttached(Invalid): + _msg_fmt = _("Unable to detach VIF %(vif)s from node %(node)s " + "because it is not attached to it.") + + class InvalidUUID(Invalid): _msg_fmt = _("Expected a UUID but received %(uuid)s.") diff --git a/ironic/common/network.py b/ironic/common/network.py index 78aabd58e0..17d753be50 100644 --- a/ironic/common/network.py +++ b/ironic/common/network.py @@ -32,20 +32,12 @@ def get_node_vif_ids(task): portgroup_vifs = {} port_vifs = {} for portgroup in task.portgroups: - # NOTE(vdrok): We are booting the node only in one network at a time, - # and presence of cleaning_vif_port_id means we're doing cleaning, of - # provisioning_vif_port_id - provisioning. Otherwise it's a tenant - # network - vif = (portgroup.internal_info.get('cleaning_vif_port_id') or - portgroup.internal_info.get('provisioning_vif_port_id') or - portgroup.extra.get('vif_port_id')) + vif = task.driver.network.get_current_vif(task, portgroup) if vif: portgroup_vifs[portgroup.uuid] = vif vifs['portgroups'] = portgroup_vifs for port in task.ports: - vif = (port.internal_info.get('cleaning_vif_port_id') or - port.internal_info.get('provisioning_vif_port_id') or - port.extra.get('vif_port_id')) + vif = task.driver.network.get_current_vif(task, port) if vif: port_vifs[port.uuid] = vif vifs['ports'] = port_vifs diff --git a/ironic/common/neutron.py b/ironic/common/neutron.py index 3f63c8066e..8efecdcb5b 100644 --- a/ironic/common/neutron.py +++ b/ironic/common/neutron.py @@ -69,6 +69,69 @@ def get_client(token=None): return clientv20.Client(**params) +def unbind_neutron_port(port_id, client=None, token=None): + """Unbind a neutron port + + Remove a neutron port's binding profile and host ID so that it returns to + an unbound state. + + :param port_id: Neutron port ID. + :param token: Optional auth token. + :param client: Optional a Neutron client object. + :raises: NetworkError + """ + + if not client: + client = get_client(token) + + body = {'port': {'binding:host_id': '', + 'binding:profile': {}}} + + try: + client.update_port(port_id, body) + except neutron_exceptions.NeutronClientException as e: + msg = (_('Unable to clear binding profile for ' + 'neutron port %(port_id)s. Error: ' + '%(err)s') % {'port_id': port_id, 'err': e}) + LOG.exception(msg) + raise exception.NetworkError(msg) + + +def update_port_address(port_id, address, token=None): + """Update a port's mac address. + + :param port_id: Neutron port id. + :param address: new MAC address. + :param token: optional auth token. + :raises: FailedToUpdateMacOnPort + """ + client = get_client(token) + port_req_body = {'port': {'mac_address': address}} + + try: + msg = (_("Failed to get the current binding on Neutron " + "port %s.") % port_id) + port = client.show_port(port_id).get('port', {}) + binding_host_id = port.get('binding:host_id') + binding_profile = port.get('binding:profile') + + if binding_host_id: + # Unbind port before we update it's mac address, because you can't + # change a bound port's mac address. + msg = (_("Failed to remove the current binding from " + "Neutron port %s, while updating its MAC " + "address.") % port_id) + unbind_neutron_port(port_id, client=client) + port_req_body['port']['binding:host_id'] = binding_host_id + port_req_body['port']['binding:profile'] = binding_profile + + msg = (_("Failed to update MAC address on Neutron port %s.") % port_id) + client.update_port(port_id, port_req_body) + except (neutron_exceptions.NeutronClientException, exception.NetworkError): + LOG.exception(msg) + raise exception.FailedToUpdateMacOnPort(port_id=port_id) + + def _verify_security_groups(security_groups, client): """Verify that the security groups exist. diff --git a/ironic/conductor/manager.py b/ironic/conductor/manager.py index d288de3cbc..c53ac5e6c9 100644 --- a/ironic/conductor/manager.py +++ b/ironic/conductor/manager.py @@ -53,7 +53,6 @@ import oslo_messaging as messaging from oslo_utils import excutils from oslo_utils import uuidutils -from ironic.common import dhcp_factory from ironic.common import driver_factory from ironic.common import exception from ironic.common.glance_service import service_utils as glance_utils @@ -1727,7 +1726,9 @@ class ConductorManager(base_manager.BaseConductorManager): exception.MACAlreadyExists, exception.InvalidState, exception.FailedToUpdateDHCPOptOnPort, - exception.Conflict) + exception.Conflict, + exception.InvalidParameterValue, + exception.NetworkError) def update_port(self, context, port_obj): """Update a port. @@ -1751,10 +1752,6 @@ class ConductorManager(base_manager.BaseConductorManager): with task_manager.acquire(context, port_obj.node_id, purpose='port update') as task: node = task.node - portgroup_obj = None - if port_obj.portgroup_id: - portgroup_obj = [pg for pg in task.portgroups if - pg.id == port_obj.portgroup_id][0] # Only allow updating MAC addresses for active nodes if maintenance # mode is on. @@ -1792,58 +1789,9 @@ class ConductorManager(base_manager.BaseConductorManager): 'connect': ', '.join(connectivity_attr), 'allowed': ', '.join(allowed_update_states)}) - if 'address' in port_obj.obj_what_changed(): - vif = port_obj.extra.get('vif_port_id') - if vif: - api = dhcp_factory.DHCPFactory() - api.provider.update_port_address(vif, port_obj.address, - token=context.auth_token) - # Log warning if there is no vif_port_id and an instance - # is associated with the node. - elif node.instance_uuid: - LOG.warning(_LW( - "No VIF found for instance %(instance)s " - "port %(port)s when attempting to update port MAC " - "address."), - {'port': port_uuid, 'instance': node.instance_uuid}) - - vif = port_obj.extra.get('vif_port_id') - if 'extra' in port_obj.obj_what_changed(): - orignal_port = objects.Port.get_by_id(context, port_obj.id) - updated_client_id = port_obj.extra.get('client-id') - if (orignal_port.extra.get('client-id') != - updated_client_id): - # DHCP Option with opt_value=None will remove it - # from the neutron port - if vif: - api = dhcp_factory.DHCPFactory() - client_id_opt = {'opt_name': 'client-id', - 'opt_value': updated_client_id} - - api.provider.update_port_dhcp_opts( - vif, [client_id_opt], token=context.auth_token) - # Log warning if there is no vif_port_id and an instance - # is associated with the node. - elif node.instance_uuid: - LOG.warning(_LW( - "No VIF found for instance %(instance)s " - "port %(port)s when attempting to update port " - "client-id."), - {'port': port_uuid, - 'instance': node.instance_uuid}) - - if portgroup_obj and ((set(port_obj.obj_what_changed()) & - {'pxe_enabled', 'portgroup_id'}) or vif): - if ((port_obj.pxe_enabled or vif) and not - portgroup_obj.standalone_ports_supported): - msg = (_("Port group %(portgroup)s doesn't support " - "standalone ports. This port %(port)s cannot be " - " a member of that port group because either " - "'extra/vif_port_id' was specified or " - "'pxe_enabled' was set to True.") % - {"portgroup": portgroup_obj.uuid, - "port": port_uuid}) - raise exception.Conflict(msg) + task.driver.network.validate(task) + # Handle mac_address update and VIF attach/detach stuff. + task.driver.network.port_changed(task, port_obj) port_obj.save() @@ -1855,7 +1803,9 @@ class ConductorManager(base_manager.BaseConductorManager): exception.PortgroupMACAlreadyExists, exception.PortgroupNotEmpty, exception.InvalidState, - exception.Conflict) + exception.Conflict, + exception.InvalidParameterValue, + exception.NetworkError) def update_portgroup(self, context, portgroup_obj): """Update a portgroup. @@ -1917,39 +1867,9 @@ class ConductorManager(base_manager.BaseConductorManager): action % {'portgroup': portgroup_uuid, 'node': node.uuid}) - if 'address' in portgroup_obj.obj_what_changed(): - vif = portgroup_obj.extra.get('vif_port_id') - if vif: - api = dhcp_factory.DHCPFactory() - api.provider.update_port_address( - vif, - portgroup_obj.address, - token=context.auth_token) - # Log warning if there is no vif_port_id and an instance - # is associated with the node. - elif node.instance_uuid: - LOG.warning(_LW( - "No VIF was found for instance %(instance)s " - "on node %(node)s, when attempting to update " - "portgroup %(portgroup)s MAC address."), - {'portgroup': portgroup_uuid, - 'instance': node.instance_uuid, - 'node': node.uuid}) - - if ('standalone_ports_supported' in - portgroup_obj.obj_what_changed()): - if not portgroup_obj.standalone_ports_supported: - ports = [p for p in task.ports if - p.portgroup_id == portgroup_obj.id] - for p in ports: - extra = p.extra - vif = extra.get('vif_port_id') - if vif or p.pxe_enabled: - msg = _("standalone_ports_supported can not be " - "set to False, because the port group %s " - "contains ports with 'extra/vif_port_id' " - "or pxe_enabled=True") % portgroup_uuid - raise exception.Conflict(msg) + task.driver.network.validate(task) + # Handle mac_address update and VIF attach/detach stuff. + task.driver.network.portgroup_changed(task, portgroup_obj) portgroup_obj.save() diff --git a/ironic/dhcp/base.py b/ironic/dhcp/base.py index 6eb013ad9b..e0663186ec 100644 --- a/ironic/dhcp/base.py +++ b/ironic/dhcp/base.py @@ -47,7 +47,8 @@ class BaseDHCP(object): :raises: FailedToUpdateDHCPOptOnPort """ - @abc.abstractmethod + # TODO(vsaienko) Remove this method when deprecation period is passed + # in Pike. def update_port_address(self, port_id, address, token=None): """Update a port's MAC address. @@ -57,6 +58,7 @@ class BaseDHCP(object): :raises: FailedToUpdateMacOnPort """ + pass @abc.abstractmethod def update_dhcp_opts(self, task, options, vifs=None): diff --git a/ironic/dhcp/neutron.py b/ironic/dhcp/neutron.py index eccb8ec1ba..ef706ecae2 100644 --- a/ironic/dhcp/neutron.py +++ b/ironic/dhcp/neutron.py @@ -31,6 +31,8 @@ from ironic import objects LOG = logging.getLogger(__name__) +update_port_address_deprecation = False + class NeutronDHCPApi(base.BaseDHCP): """API for communicating to neutron 2.x API.""" @@ -65,16 +67,8 @@ class NeutronDHCPApi(base.BaseDHCP): LOG.exception(_LE("Failed to update Neutron port %s."), port_id) raise exception.FailedToUpdateDHCPOptOnPort(port_id=port_id) - def _get_binding(self, client, port_id): - """Get binding:host_id property from Neutron.""" - try: - return client.show_port(port_id).get('port', {}).get( - 'binding:host_id') - except neutron_client_exc.NeutronClientException: - LOG.exception(_LE('Failed to get the current binding on Neutron ' - 'port %s.'), port_id) - raise exception.FailedToUpdateMacOnPort(port_id=port_id) - + # TODO(vsaienko) Remove this method when deprecation period is passed + # in Pike. def update_port_address(self, port_id, address, token=None): """Update a port's mac address. @@ -83,27 +77,14 @@ class NeutronDHCPApi(base.BaseDHCP): :param token: optional auth token. :raises: FailedToUpdateMacOnPort """ - client = neutron.get_client(token) - port_req_body = {'port': {'mac_address': address}} + global update_port_address_deprecation + if not update_port_address_deprecation: + LOG.warning(_LW('update_port_address via DHCP provider is ' + 'deprecated. The node.network_interface ' + 'port_changed() should be used instead.')) + update_port_address_deprecation = True - current_binding = self._get_binding(client, port_id) - if current_binding: - binding_clean_body = {'port': {'binding:host_id': ''}} - try: - client.update_port(port_id, binding_clean_body) - except neutron_client_exc.NeutronClientException: - LOG.exception(_LE("Failed to remove the current binding from " - "Neutron port %s."), port_id) - raise exception.FailedToUpdateMacOnPort(port_id=port_id) - - port_req_body['port']['binding:host_id'] = current_binding - - try: - neutron.get_client(token).update_port(port_id, port_req_body) - except neutron_client_exc.NeutronClientException: - LOG.exception(_LE("Failed to update MAC address on Neutron " - "port %s."), port_id) - raise exception.FailedToUpdateMacOnPort(port_id=port_id) + neutron.update_port_address(port_id, address, token) def update_dhcp_opts(self, task, options, vifs=None): """Send or update the DHCP BOOT options for this node. @@ -227,13 +208,7 @@ class NeutronDHCPApi(base.BaseDHCP): :raises: InvalidIPv4Address """ - # NOTE(vdrok): We are booting the node only in one network at a time, - # and presence of cleaning_vif_port_id means we're doing cleaning, of - # provisioning_vif_port_id - provisioning. Otherwise it's a tenant - # network - vif = (p_obj.internal_info.get('cleaning_vif_port_id') or - p_obj.internal_info.get('provisioning_vif_port_id') or - p_obj.extra.get('vif_port_id')) + vif = task.driver.network.get_current_vif(task, p_obj) if not vif: obj_name = 'portgroup' if isinstance(p_obj, objects.Port): diff --git a/ironic/dhcp/none.py b/ironic/dhcp/none.py index bfe6c2d699..65b4f52a84 100644 --- a/ironic/dhcp/none.py +++ b/ironic/dhcp/none.py @@ -25,6 +25,8 @@ class NoneDHCPApi(base.BaseDHCP): def update_dhcp_opts(self, task, options, vifs=None): pass + # TODO(vsaienko) Remove this method when deprecation period is passed + # in Pike. def update_port_address(self, port_id, address, token=None): pass diff --git a/ironic/drivers/base.py b/ironic/drivers/base.py index 2d49f6fe2e..74871cc54b 100644 --- a/ironic/drivers/base.py +++ b/ironic/drivers/base.py @@ -31,6 +31,7 @@ import six from ironic.common import exception from ironic.common.i18n import _, _LE, _LW from ironic.common import raid +from ironic.drivers.modules.network import common as net_common LOG = logging.getLogger(__name__) @@ -924,6 +925,15 @@ class NetworkInterface(BaseInterface): interface_type = 'network' + # Use these booleans to prevent conductor logs being spammed by deprecation + # warnings. + _deprecated_port_change_shown = False + _deprecated_portgroup_change_shown = False + _deprecated_vif_attach_shown = False + _deprecated_vif_detach_shown = False + _deprecated_vif_list_shown = False + _deprecated_get_current_vif_shown = False + def get_properties(self): """Return the properties of the interface. @@ -940,6 +950,121 @@ class NetworkInterface(BaseInterface): :raises: MissingParameterValue, if some parameters are missing. """ + # TODO(vsaienko) Make this method abstract in Pike. + def port_changed(self, task, port_obj): + """Handle any actions required when a port changes + + :param task: a TaskManager instance. + :param port_obj: a changed Port object. + :raises: Conflict, FailedToUpdateDHCPOptOnPort + """ + default_impl = net_common.VIFPortIDMixin() + if not self._deprecated_port_change_shown: + self.__class__._deprecated_port_change_shown = True + LOG.warning(_LW('The network interface %s should be updated to ' + 'implement the port_changed function. Falling ' + 'back to default implementation, this behaviour ' + 'will be removed in Pike'), + self.__class__.__name__) + return default_impl.port_changed(task, port_obj) + + # TODO(vsaienko) Make this method abstract in Pike. + def portgroup_changed(self, task, portgroup_obj): + """Handle any actions required when a port changes + + :param task: a TaskManager instance. + :param portgroup_obj: a changed Port object. + :raises: Conflict, FailedToUpdateDHCPOptOnPort + """ + default_impl = net_common.VIFPortIDMixin() + if not self._deprecated_portgroup_change_shown: + self.__class__._deprecated_port_change_shown = True + LOG.warning(_LW('The network interface %s should be updated to ' + 'implement the portgroup_changed function. ' + 'Falling back to default implementation, this ' + 'behaviour will be removed in Pike'), + self.__class__.__name__) + return default_impl.portgroup_changed(task, portgroup_obj) + + # TODO(vsaienko) Make this method abstract in Pike. + def vif_attach(self, task, vif_info): + """Attach a virtual network interface to a node + + :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 + """ + default_impl = net_common.VIFPortIDMixin() + if not self._deprecated_vif_attach_shown: + self.__class__._deprecated_vif_attach_shown = True + LOG.warning(_LW('The network interface %s should be updated to ' + 'implement the vif_attach function. Falling ' + 'back to default implementation, this behaviour ' + 'will be removed in Pike'), + self.__class__.__name__) + return default_impl.vif_attach(task, vif_info) + + # TODO(vsaienko) Make this method abstract in Pike. + def vif_detach(self, task, vif_id): + """Detach a virtual network interface from a node + + :param task: A TaskManager instance. + :param vif_id: A VIF ID to detach + :raises: NetworkError, VifNotAttached + """ + default_impl = net_common.VIFPortIDMixin() + if not self._deprecated_vif_detach_shown: + self.__class__._deprecated_vif_detach_shown = True + LOG.warning(_LW('The network interface %s should be updated to ' + 'implement the vif_detach function. Falling ' + 'back to default implementation, this behaviour ' + 'will be removed in Pike'), + self.__class__.__name__) + return default_impl.vif_detach(task, vif_id) + + # TODO(vsaienko) Make this method abstract in Pike. + def vif_list(self, task): + """List attached VIF IDs for a node + + :param task: A TaskManager instance. + :returns: List of VIF dictionaries, each dictionary will have an 'id' + entry with the ID of the VIF. + """ + default_impl = net_common.VIFPortIDMixin() + if not self._deprecated_vif_list_shown: + self.__class__._deprecated_vif_list_shown = True + LOG.warning(_LW('The network interface %s should be updated to ' + 'implement the vif_list function. Falling ' + 'back to default implementation, this behaviour ' + 'will be removed in Pike'), + self.__class__.__name__) + return default_impl.vif_list(task) + + # TODO(vsaienko) Make this method abstract in Pike. + def get_current_vif(self, task, p_obj): + """Returns the currently used VIF associated with port or portgroup + + We are booting the node only in one network at a time, and presence of + cleaning_vif_port_id means we're doing cleaning, of + provisioning_vif_port_id - provisioning. + Otherwise it's a tenant network + + :param task: A TaskManager instance. + :param p_obj: Ironic port or portgroup object. + :returns: VIF ID associated with p_obj or None. + """ + default_impl = net_common.VIFPortIDMixin() + if not self.__class__._deprecated_get_current_vif_shown: + self.__class__._deprecated_get_current_vif_shown = True + LOG.warning(_LW('The network interface %s should be updated to ' + 'implement the get_current_vif function. Falling ' + 'back to default implementation, this behaviour ' + 'will be removed in Pike'), + self.__class__.__name__) + return default_impl.get_current_vif(task, p_obj) + @abc.abstractmethod def add_provisioning_network(self, task): """Add the provisioning network to a node. diff --git a/ironic/drivers/modules/deploy_utils.py b/ironic/drivers/modules/deploy_utils.py index 8342e6ffbb..8fede02c06 100644 --- a/ironic/drivers/modules/deploy_utils.py +++ b/ironic/drivers/modules/deploy_utils.py @@ -534,9 +534,7 @@ def get_single_nic_with_vif_port_id(task): # and presence of cleaning_vif_port_id means we're doing cleaning, of # provisioning_vif_port_id - provisioning. Otherwise it's a tenant network for port in task.ports: - if (port.internal_info.get('cleaning_vif_port_id') or - port.internal_info.get('provisioning_vif_port_id') or - port.extra.get('vif_port_id')): + if task.driver.network.get_current_vif(task, port): return port.address diff --git a/ironic/drivers/modules/network/common.py b/ironic/drivers/modules/network/common.py new file mode 100644 index 0000000000..23c07e353b --- /dev/null +++ b/ironic/drivers/modules/network/common.py @@ -0,0 +1,251 @@ +# Copyright 2016 Cisco Systems +# All Rights Reserved +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +from neutronclient.common import exceptions as neutron_exceptions +from oslo_config import cfg +from oslo_log import log + +from ironic.common import dhcp_factory +from ironic.common import exception +from ironic.common.i18n import _, _LW +from ironic.common import neutron +from ironic import objects + +CONF = cfg.CONF +LOG = log.getLogger(__name__) + +TENANT_VIF_KEY = 'tenant_vif_port_id' + + +class VIFPortIDMixin(object): + + def port_changed(self, task, port_obj): + """Handle any actions required when a port changes + + :param task: a TaskManager instance. + :param port_obj: a changed Port object from the API before it is saved + to database. + :raises: FailedToUpdateDHCPOptOnPort, Conflict + """ + context = task.context + node = task.node + port_uuid = port_obj.uuid + portgroup_obj = None + if port_obj.portgroup_id: + portgroup_obj = [pg for pg in task.portgroups if + pg.id == port_obj.portgroup_id][0] + vif = (port_obj.internal_info.get(TENANT_VIF_KEY) or + port_obj.extra.get('vif_port_id')) + if 'address' in port_obj.obj_what_changed(): + if vif: + neutron.update_port_address(vif, + port_obj.address, + token=context.auth_token) + + if 'extra' in port_obj.obj_what_changed(): + original_port = objects.Port.get_by_id(context, port_obj.id) + updated_client_id = port_obj.extra.get('client-id') + if (original_port.extra.get('client-id') != + updated_client_id): + # DHCP Option with opt_value=None will remove it + # from the neutron port + if vif: + api = dhcp_factory.DHCPFactory() + client_id_opt = {'opt_name': 'client-id', + 'opt_value': updated_client_id} + + api.provider.update_port_dhcp_opts( + vif, [client_id_opt], token=context.auth_token) + # Log warning if there is no VIF and an instance + # is associated with the node. + elif node.instance_uuid: + LOG.warning(_LW( + "No VIF found for instance %(instance)s " + "port %(port)s when attempting to update port " + "client-id."), + {'port': port_uuid, + 'instance': node.instance_uuid}) + + if portgroup_obj and ((set(port_obj.obj_what_changed()) & + {'pxe_enabled', 'portgroup_id'}) or vif): + if not portgroup_obj.standalone_ports_supported: + reason = [] + if port_obj.pxe_enabled: + reason.append("'pxe_enabled' was set to True") + if vif: + reason.append('VIF %s is attached to the port' % vif) + + if reason: + msg = (_("Port group %(portgroup)s doesn't support " + "standalone ports. This port %(port)s cannot be " + " a member of that port group because of: " + "%(reason)s") % {"reason": ', '.join(reason), + "portgroup": portgroup_obj.uuid, + "port": port_uuid}) + raise exception.Conflict(msg) + + def portgroup_changed(self, task, portgroup_obj): + """Handle any actions required when a portgroup changes + + :param task: a TaskManager instance. + :param portgroup_obj: a changed Portgroup object from the API before + it is saved to database. + :raises: FailedToUpdateDHCPOptOnPort, Conflict + """ + + context = task.context + portgroup_uuid = portgroup_obj.uuid + if 'address' in portgroup_obj.obj_what_changed(): + pg_vif = portgroup_obj.extra.get('vif_port_id') + if pg_vif: + neutron.update_port_address(pg_vif, + portgroup_obj.address, + token=context.auth_token) + + if ('standalone_ports_supported' in + portgroup_obj.obj_what_changed()): + if not portgroup_obj.standalone_ports_supported: + ports = [p for p in task.ports if + p.portgroup_id == portgroup_obj.id] + for p in ports: + vif = p.internal_info.get( + TENANT_VIF_KEY, p.extra.get('vif_port_id')) + reason = [] + if p.pxe_enabled: + reason.append("'pxe_enabled' is set to True") + if vif: + reason.append('VIF %s is attached to this port' % vif) + + if reason: + msg = (_("standalone_ports_supported can not be set " + "to False, because the port group %(pg_id)s " + "contains port with %(reason)s") % { + 'pg_id': portgroup_uuid, + 'reason': ', '.join(reason)}) + raise exception.Conflict(msg) + + def vif_list(self, task): + """List attached VIF IDs for a node + + :param task: A TaskManager instance. + :returns: List of VIF dictionaries, each dictionary will have an 'id' + entry with the ID of the VIF. + """ + vifs = [] + for port in task.ports: + vif_id = port.internal_info.get( + TENANT_VIF_KEY, port.extra.get('vif_port_id')) + if vif_id: + vifs.append({'id': vif_id}) + return vifs + + def vif_attach(self, task, vif_info): + """Attach a virtual network interface to a node + + :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 + """ + vif_id = vif_info['id'] + # Sort ports by pxe_enabled to ensure we always bind pxe_enabled ports + # first + sorted_ports = sorted(task.ports, key=lambda p: p.pxe_enabled, + reverse=True) + free_ports = [] + # Check all ports to ensure this VIF isn't already attached + for port in sorted_ports: + port_id = port.internal_info.get(TENANT_VIF_KEY, + port.extra.get('vif_port_id')) + if port_id is None: + free_ports.append(port) + elif port_id == vif_id: + raise exception.VifAlreadyAttached( + vif=vif_id, port_uuid=port.uuid) + + if not free_ports: + raise exception.NoFreePhysicalPorts(vif=vif_id) + + # Get first free port + port = free_ports.pop(0) + + # Check if the requested vif_id is a neutron port. If it is + # then attempt to update the port's MAC address. + try: + client = neutron.get_client(task.context.auth_token) + client.show_port(vif_id) + except neutron_exceptions.NeutronClientException: + # 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 + else: + try: + neutron.update_port_address(vif_id, port.address) + except exception.FailedToUpdateMacOnPort: + raise exception.NetworkError(_( + "Unable to attach VIF %(vif)s because Ironic can not " + "update Neutron port %(port)s MAC address to match " + "physical MAC address %(mac)s") % { + 'vif': vif_id, 'port': vif_id, 'mac': port.address}) + + int_info = port.internal_info + int_info[TENANT_VIF_KEY] = vif_id + port.internal_info = int_info + port.save() + + def vif_detach(self, task, vif_id): + """Detach a virtual network interface from a node + + :param task: A TaskManager instance. + :param vif_id: A VIF ID to detach + :raises: VifNotAttached + """ + for port in task.ports: + # FIXME(sambetts) Remove this when we no longer support a nova + # driver that uses port.extra + if (port.extra.get("vif_port_id") == vif_id or + port.internal_info.get(TENANT_VIF_KEY) == vif_id): + int_info = port.internal_info + extra = port.extra + int_info.pop(TENANT_VIF_KEY, None) + extra.pop('vif_port_id', None) + port.extra = extra + port.internal_info = int_info + port.save() + break + else: + raise exception.VifNotAttached(vif=vif_id, node=task.node.uuid) + + def get_current_vif(self, task, p_obj): + """Returns the currently used VIF associated with port or portgroup + + We are booting the node only in one network at a time, and presence of + cleaning_vif_port_id means we're doing cleaning, of + provisioning_vif_port_id - provisioning. + Otherwise it's a tenant network + + :param task: A TaskManager instance. + :param p_obj: Ironic port or portgroup object. + :returns: VIF ID associated with p_obj or None. + """ + + return (p_obj.internal_info.get('cleaning_vif_port_id') or + p_obj.internal_info.get('provisioning_vif_port_id') or + p_obj.internal_info.get('tenant_vif_port_id') or + p_obj.extra.get('vif_port_id') or None) diff --git a/ironic/drivers/modules/network/flat.py b/ironic/drivers/modules/network/flat.py index af4b886662..dda7b584c5 100644 --- a/ironic/drivers/modules/network/flat.py +++ b/ironic/drivers/modules/network/flat.py @@ -14,12 +14,15 @@ Flat network interface. Useful for shared, flat networks. """ +from neutronclient.common import exceptions as neutron_exceptions from oslo_config import cfg from oslo_log import log -from ironic.common.i18n import _LI, _LW +from ironic.common import exception +from ironic.common.i18n import _, _LI, _LW from ironic.common import neutron from ironic.drivers import base +from ironic.drivers.modules.network import common LOG = log.getLogger(__name__) @@ -27,7 +30,8 @@ LOG = log.getLogger(__name__) CONF = cfg.CONF -class FlatNetwork(neutron.NeutronNetworkInterfaceMixin, base.NetworkInterface): +class FlatNetwork(common.VIFPortIDMixin, neutron.NeutronNetworkInterfaceMixin, + base.NetworkInterface): """Flat network interface.""" def __init__(self): @@ -57,8 +61,40 @@ class FlatNetwork(neutron.NeutronNetworkInterfaceMixin, base.NetworkInterface): """Add the provisioning network to a node. :param task: A TaskManager instance. + :raises: NetworkError when failed to set binding:host_id """ - pass + LOG.debug("Binding flat network ports") + node = task.node + host_id = node.instance_info.get('nova_host_id') + if not host_id: + return + + # FIXME(sambetts): Uncomment when we support vifs attached to + # portgroups + # + # ports = [p for p in task.ports if not p.portgroup_id] + # portgroups = task.portgroups + + client = neutron.get_client(task.context.auth_token) + for port_like_obj in task.ports: # + portgroups: + vif_port_id = (port_like_obj.extra.get('vif_port_id') or + port_like_obj.internal_info.get( + 'tenant_vif_port_id')) + if not vif_port_id: + continue + body = { + 'port': { + 'binding:host_id': host_id + } + } + try: + client.update_port(vif_port_id, body) + except neutron_exceptions.NeutronClientException as e: + msg = (_('Unable to set binding:host_id for ' + 'neutron port %(port_id)s. Error: ' + '%(err)s') % {'port_id': vif_port_id, 'err': e}) + LOG.exception(msg) + raise exception.NetworkError(msg) def remove_provisioning_network(self, task): """Remove the provisioning network from a node. @@ -79,11 +115,7 @@ class FlatNetwork(neutron.NeutronNetworkInterfaceMixin, base.NetworkInterface): :param task: A TaskManager instance. """ - for port in task.ports: - extra_dict = port.extra - extra_dict.pop('vif_port_id', None) - port.extra = extra_dict - port.save() + pass def add_cleaning_network(self, task): """Add the cleaning network to a node. diff --git a/ironic/drivers/modules/network/neutron.py b/ironic/drivers/modules/network/neutron.py index 82918e114d..bfd0272582 100644 --- a/ironic/drivers/modules/network/neutron.py +++ b/ironic/drivers/modules/network/neutron.py @@ -22,6 +22,7 @@ from ironic.common import exception from ironic.common.i18n import _, _LI from ironic.common import neutron from ironic.drivers import base +from ironic.drivers.modules.network import common from ironic import objects LOG = log.getLogger(__name__) @@ -29,7 +30,8 @@ LOG = log.getLogger(__name__) CONF = cfg.CONF -class NeutronNetwork(neutron.NeutronNetworkInterfaceMixin, +class NeutronNetwork(common.VIFPortIDMixin, + neutron.NeutronNetworkInterfaceMixin, base.NetworkInterface): """Neutron v2 network interface""" @@ -163,7 +165,9 @@ class NeutronNetwork(neutron.NeutronNetworkInterfaceMixin, client = neutron.get_client(task.context.auth_token) pobj_without_vif = 0 for port_like_obj in ports + portgroups: - vif_port_id = port_like_obj.extra.get('vif_port_id') + vif_port_id = ( + port_like_obj.internal_info.get('tenant_vif_port_id') or + port_like_obj.extra.get('vif_port_id')) if not vif_port_id: pobj_without_vif += 1 @@ -222,15 +226,23 @@ class NeutronNetwork(neutron.NeutronNetworkInterfaceMixin, def unconfigure_tenant_networks(self, task): """Unconfigure tenant networks for a node. - Even though nova takes care of port removal from tenant network, we - remove it here/now to avoid the possibility of the ironic port being - bound to the tenant and cleaning networks at the same time. + Nova takes care of port removal from tenant network, we unbind it + here/now to avoid the possibility of the ironic port being bound to the + tenant and cleaning networks at the same time. :param task: A TaskManager instance. :raises: NetworkError """ node = task.node - LOG.info(_LI('Unmapping instance ports from node %s'), node.uuid) - params = {'device_id': node.instance_uuid or node.uuid} + LOG.info(_LI('Unbinding instance ports from node %s'), node.uuid) - neutron.remove_neutron_ports(task, params) + ports = [p for p in task.ports if not p.portgroup_id] + portgroups = task.portgroups + for port_like_obj in ports + portgroups: + vif_port_id = ( + port_like_obj.internal_info.get('tenant_vif_port_id') or + port_like_obj.extra.get('vif_port_id')) + if not vif_port_id: + continue + neutron.unbind_neutron_port(vif_port_id, + token=task.context.auth_token) diff --git a/ironic/drivers/modules/network/noop.py b/ironic/drivers/modules/network/noop.py index f4c530022d..ac0232ae77 100644 --- a/ironic/drivers/modules/network/noop.py +++ b/ironic/drivers/modules/network/noop.py @@ -16,6 +16,67 @@ from ironic.drivers import base class NoopNetwork(base.NetworkInterface): """Noop network interface.""" + def port_changed(self, task, port_obj): + """Handle any actions required when a port changes + + :param task: a TaskManager instance. + :param port_obj: a changed Port object. + :raises: Conflict, FailedToUpdateDHCPOptOnPort + """ + pass + + def portgroup_changed(self, task, portgroup_obj): + """Handle any actions required when a portgroup changes + + :param task: a TaskManager instance. + :param portgroup_obj: a changed Portgroup object. + :raises: Conflict, FailedToUpdateDHCPOptOnPort + """ + pass + + def vif_attach(self, task, vif_info): + """Attach a virtual network interface to a node + + :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 + """ + pass + + def vif_detach(self, task, vif_id): + """Detach a virtual network interface from a node + + :param task: A TaskManager instance. + :param vif_id: A VIF ID to detach + :raises: NetworkError, VifNotAttached + """ + pass + + def vif_list(self, task): + """List attached VIF IDs for a node. + + :param task: A TaskManager instance. + :returns: List of VIF dictionaries, each dictionary will have an 'id' + entry with the ID of the VIF. + """ + pass + + def get_current_vif(self, task, p_obj): + """Returns the currently used VIF associated with port or portgroup + + We are booting the node only in one network at a time, and presence of + cleaning_vif_port_id means we're doing cleaning, of + provisioning_vif_port_id - provisioning. + Otherwise it's a tenant network + + :param task: A TaskManager instance. + :param p_obj: Ironic port or portgroup object. + :returns: VIF ID associated with p_obj or None. + """ + pass + def add_provisioning_network(self, task): """Add the provisioning network to a node. diff --git a/ironic/tests/unit/common/test_network.py b/ironic/tests/unit/common/test_network.py index a37267a062..30212b61e5 100644 --- a/ironic/tests/unit/common/test_network.py +++ b/ironic/tests/unit/common/test_network.py @@ -37,22 +37,34 @@ class TestNetwork(db_base.DbTestCase): result = network.get_node_vif_ids(task) self.assertEqual(expected, result) - def test_get_node_vif_ids_one_port(self): + def _test_get_node_vif_ids_one_port(self, key): + if key == "extra": + kwargs1 = {key: {'vif_port_id': 'test-vif-A'}} + else: + kwargs1 = {key: {'tenant_vif_port_id': 'test-vif-A'}} port1 = db_utils.create_test_port(node_id=self.node.id, address='aa:bb:cc:dd:ee:ff', uuid=uuidutils.generate_uuid(), - extra={'vif_port_id': 'test-vif-A'}, - driver='fake') + driver='fake', **kwargs1) expected = {'portgroups': {}, 'ports': {port1.uuid: 'test-vif-A'}} with task_manager.acquire(self.context, self.node.uuid) as task: result = network.get_node_vif_ids(task) self.assertEqual(expected, result) - def test_get_node_vif_ids_one_portgroup(self): + def test_get_node_vif_ids_one_port_extra(self): + self._test_get_node_vif_ids_one_port("extra") + + def test_get_node_vif_ids_one_port_int_info(self): + self._test_get_node_vif_ids_one_port("internal_info") + + def _test_get_node_vif_ids_one_portgroup(self, key): + if key == "extra": + kwargs1 = {key: {'vif_port_id': 'test-vif-A'}} + else: + kwargs1 = {key: {'tenant_vif_port_id': 'test-vif-A'}} pg1 = db_utils.create_test_portgroup( - node_id=self.node.id, - extra={'vif_port_id': 'test-vif-A'}) + node_id=self.node.id, **kwargs1) expected = {'portgroups': {pg1.uuid: 'test-vif-A'}, 'ports': {}} @@ -60,17 +72,27 @@ class TestNetwork(db_base.DbTestCase): result = network.get_node_vif_ids(task) self.assertEqual(expected, result) - def test_get_node_vif_ids_two_ports(self): + def test_get_node_vif_ids_one_portgroup_extra(self): + self._test_get_node_vif_ids_one_portgroup("extra") + + def test_get_node_vif_ids_one_portgroup_int_info(self): + self._test_get_node_vif_ids_one_portgroup("internal_info") + + def _test_get_node_vif_ids_two_ports(self, key): + if key == "extra": + kwargs1 = {key: {'vif_port_id': 'test-vif-A'}} + kwargs2 = {key: {'vif_port_id': 'test-vif-B'}} + else: + kwargs1 = {key: {'tenant_vif_port_id': 'test-vif-A'}} + kwargs2 = {key: {'tenant_vif_port_id': 'test-vif-B'}} port1 = db_utils.create_test_port(node_id=self.node.id, address='aa:bb:cc:dd:ee:ff', uuid=uuidutils.generate_uuid(), - extra={'vif_port_id': 'test-vif-A'}, - driver='fake') + driver='fake', **kwargs1) port2 = db_utils.create_test_port(node_id=self.node.id, address='dd:ee:ff:aa:bb:cc', uuid=uuidutils.generate_uuid(), - extra={'vif_port_id': 'test-vif-B'}, - driver='fake') + driver='fake', **kwargs2) expected = {'portgroups': {}, 'ports': {port1.uuid: 'test-vif-A', port2.uuid: 'test-vif-B'}} @@ -78,16 +100,26 @@ class TestNetwork(db_base.DbTestCase): result = network.get_node_vif_ids(task) self.assertEqual(expected, result) - def test_get_node_vif_ids_two_portgroups(self): + def test_get_node_vif_ids_two_ports_extra(self): + self._test_get_node_vif_ids_two_ports('extra') + + def test_get_node_vif_ids_two_ports_int_info(self): + self._test_get_node_vif_ids_two_ports('internal_info') + + def _test_get_node_vif_ids_two_portgroups(self, key): + if key == "extra": + kwargs1 = {key: {'vif_port_id': 'test-vif-A'}} + kwargs2 = {key: {'vif_port_id': 'test-vif-B'}} + else: + kwargs1 = {key: {'tenant_vif_port_id': 'test-vif-A'}} + kwargs2 = {key: {'tenant_vif_port_id': 'test-vif-B'}} pg1 = db_utils.create_test_portgroup( - node_id=self.node.id, - extra={'vif_port_id': 'test-vif-A'}) + node_id=self.node.id, **kwargs1) pg2 = db_utils.create_test_portgroup( uuid=uuidutils.generate_uuid(), address='dd:ee:ff:aa:bb:cc', node_id=self.node.id, - name='barname', - extra={'vif_port_id': 'test-vif-B'}) + name='barname', **kwargs2) expected = {'portgroups': {pg1.uuid: 'test-vif-A', pg2.uuid: 'test-vif-B'}, 'ports': {}} @@ -95,6 +127,12 @@ class TestNetwork(db_base.DbTestCase): result = network.get_node_vif_ids(task) self.assertEqual(expected, result) + def test_get_node_vif_ids_two_portgroups_extra(self): + self._test_get_node_vif_ids_two_portgroups('extra') + + def test_get_node_vif_ids_two_portgroups_int_info(self): + self._test_get_node_vif_ids_two_portgroups('internal_info') + def _test_get_node_vif_ids_multitenancy(self, int_info_key): port = db_utils.create_test_port( node_id=self.node.id, address='aa:bb:cc:dd:ee:ff', diff --git a/ironic/tests/unit/common/test_neutron.py b/ironic/tests/unit/common/test_neutron.py index bcaefa21fd..f6175f2e44 100644 --- a/ironic/tests/unit/common/test_neutron.py +++ b/ironic/tests/unit/common/test_neutron.py @@ -513,3 +513,145 @@ class TestValidateNetwork(base.TestCase): neutron.validate_network, 'name') net_mock.assert_called_once_with(fields=['id'], name='name') + + +@mock.patch.object(neutron, 'get_client', autospec=True) +class TestUpdatePortAddress(base.TestCase): + + def test_update_port_address(self, mock_client): + address = 'fe:54:00:77:07:d9' + port_id = 'fake-port-id' + expected = {'port': {'mac_address': address}} + mock_client.return_value.show_port.return_value = {} + + neutron.update_port_address(port_id, address) + mock_client.return_value.update_port.assert_called_once_with(port_id, + expected) + + @mock.patch.object(neutron, 'unbind_neutron_port', autospec=True) + def test_update_port_address_with_binding(self, mock_unp, mock_client): + address = 'fe:54:00:77:07:d9' + port_id = 'fake-port-id' + expected = {'port': {'mac_address': address, + 'binding:host_id': 'host', + 'binding:profile': 'foo'}} + mock_client.return_value.show_port.return_value = { + 'port': {'binding:host_id': 'host', + 'binding:profile': 'foo'}} + + neutron.update_port_address(port_id, address) + mock_unp.assert_called_once_with(port_id, client=mock_client()) + mock_client.return_value.update_port.assert_any_call(port_id, expected) + + @mock.patch.object(neutron, 'unbind_neutron_port', autospec=True) + def test_update_port_address_without_binding(self, mock_unp, mock_client): + address = 'fe:54:00:77:07:d9' + port_id = 'fake-port-id' + expected = {'port': {'mac_address': address}} + mock_client.return_value.show_port.return_value = { + 'port': {'binding:profile': 'foo'}} + + neutron.update_port_address(port_id, address) + self.assertFalse(mock_unp.called) + mock_client.return_value.update_port.assert_any_call(port_id, expected) + + def test_update_port_address_show_failed(self, mock_client): + address = 'fe:54:00:77:07:d9' + port_id = 'fake-port-id' + mock_client.return_value.show_port.side_effect = ( + neutron_client_exc.NeutronClientException()) + + self.assertRaises(exception.FailedToUpdateMacOnPort, + neutron.update_port_address, port_id, address) + self.assertFalse(mock_client.return_value.update_port.called) + + @mock.patch.object(neutron, 'unbind_neutron_port', autospec=True) + def test_update_port_address_unbind_port_failed(self, mock_unp, + mock_client): + address = 'fe:54:00:77:07:d9' + port_id = 'fake-port-id' + mock_client.return_value.show_port.return_value = { + 'port': {'binding:profile': 'foo', + 'binding:host_id': 'host'}} + mock_unp.side_effect = (exception.NetworkError('boom')) + self.assertRaises(exception.FailedToUpdateMacOnPort, + neutron.update_port_address, port_id, address) + mock_unp.assert_called_once_with(port_id, client=mock_client()) + self.assertFalse(mock_client.return_value.update_port.called) + + @mock.patch.object(neutron, 'unbind_neutron_port', autospec=True) + def test_update_port_address_with_exception(self, mock_unp, + mock_client): + address = 'fe:54:00:77:07:d9' + port_id = 'fake-port-id' + mock_client.return_value.show_port.return_value = {} + mock_client.return_value.update_port.side_effect = ( + neutron_client_exc.NeutronClientException()) + + self.assertRaises(exception.FailedToUpdateMacOnPort, + neutron.update_port_address, + port_id, address) + + +@mock.patch.object(neutron, 'get_client', autospec=True) +class TestUnbindPort(base.TestCase): + + def test_unbind_neutron_port_client_passed(self, mock_client): + port_id = 'fake-port-id' + body = { + 'port': { + 'binding:host_id': '', + 'binding:profile': {} + } + } + neutron.unbind_neutron_port(port_id, mock_client()) + self.assertEqual(1, mock_client.call_count) + mock_client.return_value.update_port.assert_called_once_with(port_id, + body) + + def test_unbind_neutron_port_token_passed(self, mock_client): + port_id = 'fake-port-id' + token = 'token' + body = { + 'port': { + 'binding:host_id': '', + 'binding:profile': {} + } + } + neutron.unbind_neutron_port(port_id, token=token) + mock_client.assert_called_once_with(token) + mock_client.return_value.update_port.assert_called_once_with(port_id, + body) + + @mock.patch.object(neutron, 'LOG') + def test_unbind_neutron_port_failure(self, mock_log, mock_client): + mock_client.return_value.update_port.side_effect = ( + neutron_client_exc.NeutronClientException()) + body = { + 'port': { + 'binding:host_id': '', + 'binding:profile': {} + } + } + port_id = 'fake-port-id' + token = 'token' + self.assertRaises(exception.NetworkError, neutron.unbind_neutron_port, + port_id, token=token) + mock_client.assert_called_once_with(token) + mock_client.return_value.update_port.assert_called_once_with(port_id, + body) + mock_log.exception.assert_called_once() + + def test_unbind_neutron_port(self, mock_client): + port_id = 'fake-port-id' + token = 'token' + body = { + 'port': { + 'binding:host_id': '', + 'binding:profile': {} + } + } + neutron.unbind_neutron_port(port_id, token=token) + mock_client.assert_called_once_with(token) + mock_client.return_value.update_port.assert_called_once_with(port_id, + body) diff --git a/ironic/tests/unit/conductor/test_manager.py b/ironic/tests/unit/conductor/test_manager.py index c3e2b75a7e..7f98d7fee4 100644 --- a/ironic/tests/unit/conductor/test_manager.py +++ b/ironic/tests/unit/conductor/test_manager.py @@ -41,6 +41,7 @@ from ironic.conductor import utils as conductor_utils from ironic.db import api as dbapi from ironic.drivers import base as drivers_base from ironic.drivers.modules import fake +from ironic.drivers.modules.network import flat as n_flat from ironic import objects from ironic.objects import base as obj_base from ironic.objects import fields as obj_fields @@ -2886,7 +2887,10 @@ class DestroyNodeTestCase(mgr_utils.ServiceSetUpMixin, @mgr_utils.mock_record_keepalive class UpdatePortTestCase(mgr_utils.ServiceSetUpMixin, tests_db_base.DbTestCase): - def test_update_port(self): + + @mock.patch.object(n_flat.FlatNetwork, 'port_changed', autospec=True) + @mock.patch.object(n_flat.FlatNetwork, 'validate', autospec=True) + def test_update_port(self, mock_val, mock_pc): node = obj_utils.create_test_node(self.context, driver='fake') port = obj_utils.create_test_port(self.context, @@ -2896,6 +2900,8 @@ class UpdatePortTestCase(mgr_utils.ServiceSetUpMixin, port.extra = new_extra res = self.service.update_port(self.context, port) self.assertEqual(new_extra, res.extra) + mock_val.assert_called_once_with(mock.ANY, mock.ANY) + mock_pc.assert_called_once_with(mock.ANY, mock.ANY, port) def test_update_port_node_locked(self): node = obj_utils.create_test_node(self.context, driver='fake', @@ -2909,56 +2915,28 @@ class UpdatePortTestCase(mgr_utils.ServiceSetUpMixin, # Compare true exception hidden by @messaging.expected_exceptions self.assertEqual(exception.NodeLocked, exc.exc_info[0]) - @mock.patch('ironic.dhcp.neutron.NeutronDHCPApi.update_port_address') - def test_update_port_address(self, mac_update_mock): - node = obj_utils.create_test_node(self.context, driver='fake', - instance_uuid=None, - provision_state='available') - port = obj_utils.create_test_port(self.context, - node_id=node.id, - extra={'vif_port_id': 'fake-id'}) - new_address = '11:22:33:44:55:bb' - port.address = new_address - res = self.service.update_port(self.context, port) - self.assertEqual(new_address, res.address) - mac_update_mock.assert_called_once_with('fake-id', new_address, - token=self.context.auth_token) + @mock.patch.object(n_flat.FlatNetwork, 'port_changed', autospec=True) + @mock.patch.object(n_flat.FlatNetwork, 'validate', autospec=True) + def test_update_port_port_changed_failure(self, mock_val, mock_pc): + node = obj_utils.create_test_node(self.context, driver='fake') - @mock.patch('ironic.dhcp.neutron.NeutronDHCPApi.update_port_address') - def test_update_port_address_fail(self, mac_update_mock): - node = obj_utils.create_test_node(self.context, driver='fake', - instance_uuid=None, - provision_state='available') port = obj_utils.create_test_port(self.context, - node_id=node.id, - extra={'vif_port_id': 'fake-id'}) + node_id=node.id) old_address = port.address port.address = '11:22:33:44:55:bb' - mac_update_mock.side_effect = ( - exception.FailedToUpdateMacOnPort(port_id=port.uuid)) + mock_pc.side_effect = (exception.FailedToUpdateMacOnPort('boom')) exc = self.assertRaises(messaging.rpc.ExpectedException, self.service.update_port, self.context, port) - # Compare true exception hidden by @messaging.expected_exceptions + mock_pc.assert_called_once_with(mock.ANY, mock.ANY, port) + mock_val.assert_called_once_with(mock.ANY, mock.ANY) self.assertEqual(exception.FailedToUpdateMacOnPort, exc.exc_info[0]) port.refresh() self.assertEqual(old_address, port.address) - @mock.patch('ironic.dhcp.neutron.NeutronDHCPApi.update_port_address') - def test_update_port_address_no_vif_id(self, mac_update_mock): - node = obj_utils.create_test_node(self.context, driver='fake', - instance_uuid=None, - provision_state='available') - port = obj_utils.create_test_port(self.context, node_id=node.id) - - new_address = '11:22:33:44:55:bb' - port.address = new_address - res = self.service.update_port(self.context, port) - self.assertEqual(new_address, res.address) - self.assertFalse(mac_update_mock.called) - - @mock.patch('ironic.dhcp.neutron.NeutronDHCPApi.update_port_address') - def test_update_port_address_active_node(self, mac_update_mock): + @mock.patch.object(n_flat.FlatNetwork, 'port_changed', autospec=True) + @mock.patch.object(n_flat.FlatNetwork, 'validate', autospec=True) + def test_update_port_address_active_node(self, mock_val, mock_pc): node = obj_utils.create_test_node(self.context, driver='fake', instance_uuid=None, provision_state='active') @@ -2975,28 +2953,12 @@ class UpdatePortTestCase(mgr_utils.ServiceSetUpMixin, self.assertEqual(exception.InvalidState, exc.exc_info[0]) port.refresh() self.assertEqual(old_address, port.address) + self.assertFalse(mock_pc.called) + self.assertFalse(mock_val.called) - @mock.patch('ironic.dhcp.neutron.NeutronDHCPApi.update_port_address') - def test_update_port_address_instance_uuid(self, mac_update_mock): - node = obj_utils.create_test_node(self.context, driver='fake', - instance_uuid='uuid', - provision_state='error') - port = obj_utils.create_test_port(self.context, - node_id=node.id, - extra={'vif_port_id': 'fake-id'}) - old_address = port.address - new_address = '11:22:33:44:55:bb' - port.address = new_address - exc = self.assertRaises(messaging.rpc.ExpectedException, - self.service.update_port, - self.context, port) - # Compare true exception hidden by @messaging.expected_exceptions - self.assertEqual(exception.InvalidState, exc.exc_info[0]) - port.refresh() - self.assertEqual(old_address, port.address) - - @mock.patch('ironic.dhcp.neutron.NeutronDHCPApi.update_port_address') - def test_update_port_address_maintenance(self, mac_update_mock): + @mock.patch.object(n_flat.FlatNetwork, 'port_changed', autospec=True) + @mock.patch.object(n_flat.FlatNetwork, 'validate', autospec=True) + def test_update_port_address_maintenance(self, mock_val, mock_pc): node = obj_utils.create_test_node(self.context, driver='fake', instance_uuid='uuid', provision_state='active', @@ -3008,11 +2970,12 @@ class UpdatePortTestCase(mgr_utils.ServiceSetUpMixin, port.address = new_address res = self.service.update_port(self.context, port) self.assertEqual(new_address, res.address) - mac_update_mock.assert_called_once_with('fake-id', new_address, - token=self.context.auth_token) + mock_val.assert_called_once_with(mock.ANY, mock.ANY) + mock_pc.assert_called_once_with(mock.ANY, mock.ANY, port) - @mock.patch('ironic.dhcp.neutron.NeutronDHCPApi.update_port_address') - def test_update_port_portgroup_active_node(self, mac_update_mock): + @mock.patch.object(n_flat.FlatNetwork, 'port_changed', autospec=True) + @mock.patch.object(n_flat.FlatNetwork, 'validate', autospec=True) + def test_update_port_portgroup_active_node(self, mock_val, mock_pc): node = obj_utils.create_test_node(self.context, driver='fake', instance_uuid=None, provision_state='active') @@ -3031,9 +2994,12 @@ class UpdatePortTestCase(mgr_utils.ServiceSetUpMixin, self.assertEqual(exception.InvalidState, exc.exc_info[0]) port.refresh() self.assertEqual(pg1.id, port.portgroup_id) + self.assertFalse(mock_pc.called) + self.assertFalse(mock_val.called) - @mock.patch('ironic.dhcp.neutron.NeutronDHCPApi.update_port_address') - def test_update_port_portgroup_enroll_node(self, mac_update_mock): + @mock.patch.object(n_flat.FlatNetwork, 'port_changed', autospec=True) + @mock.patch.object(n_flat.FlatNetwork, 'validate', autospec=True) + def test_update_port_portgroup_enroll_node(self, mock_val, mock_pc): node = obj_utils.create_test_node(self.context, driver='fake', instance_uuid=None, provision_state='enroll') @@ -3048,65 +3014,8 @@ class UpdatePortTestCase(mgr_utils.ServiceSetUpMixin, self.service.update_port(self.context, port) port.refresh() self.assertEqual(pg2.id, port.portgroup_id) - - @mock.patch('ironic.dhcp.neutron.NeutronDHCPApi.update_port_dhcp_opts') - def test_update_port_client_id(self, dhcp_update_mock): - node = obj_utils.create_test_node(self.context, driver='fake') - port = obj_utils.create_test_port(self.context, - node_id=node.id, - extra={'vif_port_id': 'fake-id', - 'client-id': 'fake1'}) - expected_extra = {'vif_port_id': 'fake-id', 'client-id': 'fake2'} - expected_dhcp_opts = [{'opt_name': 'client-id', 'opt_value': 'fake2'}] - port.extra = expected_extra - res = self.service.update_port(self.context, port) - self.assertEqual(expected_extra, res.extra) - dhcp_update_mock.assert_called_once_with('fake-id', expected_dhcp_opts, - token=self.context.auth_token) - - @mock.patch('ironic.dhcp.neutron.NeutronDHCPApi.update_port_dhcp_opts') - def test_update_port_vif(self, dhcp_update_mock): - node = obj_utils.create_test_node(self.context, driver='fake') - port = obj_utils.create_test_port(self.context, - node_id=node.id, - extra={'vif_port_id': 'fake-id', - 'client-id': 'fake1'}) - expected_extra = {'vif_port_id': 'new_ake-id', 'client-id': 'fake1'} - port.extra = expected_extra - res = self.service.update_port(self.context, port) - self.assertEqual(expected_extra, res.extra) - self.assertFalse(dhcp_update_mock.called) - - @mock.patch('ironic.dhcp.neutron.NeutronDHCPApi.update_port_dhcp_opts') - def test_update_port_client_id_fail(self, dhcp_update_mock): - node = obj_utils.create_test_node(self.context, driver='fake') - expected_extra = {'vif_port_id': 'fake-id', 'client-id': 'fake1'} - port = obj_utils.create_test_port(self.context, - node_id=node.id, - extra=expected_extra) - extra = {'vif_port_id': 'fake-id', 'client-id': 'fake2'} - port.extra = extra - dhcp_update_mock.side_effect = ( - exception.FailedToUpdateDHCPOptOnPort(port_id=port.uuid)) - exc = self.assertRaises(messaging.rpc.ExpectedException, - self.service.update_port, - self.context, port) - # Compare true exception hidden by @messaging.expected_exceptions - self.assertEqual( - exception.FailedToUpdateDHCPOptOnPort, exc.exc_info[0]) - port.refresh() - self.assertEqual(expected_extra, port.extra) - - @mock.patch('ironic.dhcp.neutron.NeutronDHCPApi.update_port_dhcp_opts') - def test_update_port_client_id_no_vif_id(self, dhcp_update_mock): - node = obj_utils.create_test_node(self.context, driver='fake') - port = obj_utils.create_test_port(self.context, node_id=node.id) - - expected_extra = {'client-id': 'fake2'} - port.extra = expected_extra - res = self.service.update_port(self.context, port) - self.assertEqual(expected_extra, res.extra) - self.assertFalse(dhcp_update_mock.called) + mock_pc.assert_called_once_with(mock.ANY, mock.ANY, port) + mock_val.assert_called_once_with(mock.ANY, mock.ANY) def test_update_port_node_deleting_state(self): node = obj_utils.create_test_node(self.context, driver='fake', @@ -3123,7 +3032,10 @@ class UpdatePortTestCase(mgr_utils.ServiceSetUpMixin, port.refresh() self.assertEqual(old_pxe, port.pxe_enabled) - def test_update_port_node_manageable_state(self): + @mock.patch.object(n_flat.FlatNetwork, 'port_changed', autospec=True) + @mock.patch.object(n_flat.FlatNetwork, 'validate', autospec=True) + def test_update_port_node_manageable_state(self, mock_val, + mock_pc): node = obj_utils.create_test_node(self.context, driver='fake', provision_state=states.MANAGEABLE) port = obj_utils.create_test_port(self.context, @@ -3133,8 +3045,13 @@ class UpdatePortTestCase(mgr_utils.ServiceSetUpMixin, self.service.update_port(self.context, port) port.refresh() self.assertEqual(True, port.pxe_enabled) + mock_val.assert_called_once_with(mock.ANY, mock.ANY) + mock_pc.assert_called_once_with(mock.ANY, mock.ANY, port) - def test_update_port_node_active_state_and_maintenance(self): + @mock.patch.object(n_flat.FlatNetwork, 'port_changed', autospec=True) + @mock.patch.object(n_flat.FlatNetwork, 'validate', autospec=True) + def test_update_port_node_active_state_and_maintenance(self, mock_val, + mock_pc): node = obj_utils.create_test_node(self.context, driver='fake', provision_state=states.ACTIVE, maintenance=True) @@ -3145,143 +3062,8 @@ class UpdatePortTestCase(mgr_utils.ServiceSetUpMixin, self.service.update_port(self.context, port) port.refresh() self.assertEqual(True, port.pxe_enabled) - - def _test_update_port(self, has_vif=False, in_portgroup=False, - pxe_enabled=True, standalone_ports=True, - expect_errors=False): - node = obj_utils.create_test_node(self.context, driver='fake', - provision_state=states.ENROLL) - - pg = obj_utils.create_test_portgroup( - self.context, node_id=node.id, - standalone_ports_supported=standalone_ports) - - extra_vif = {'vif_port_id': uuidutils.generate_uuid()} - if has_vif: - extra = extra_vif - opposite_extra = {} - else: - extra = {} - opposite_extra = extra_vif - opposite_pxe_enabled = not pxe_enabled - - pg_id = None - if in_portgroup: - pg_id = pg.id - - ports = [] - - # Update only portgroup id on existed port with different - # combinations of pxe_enabled/vif_port_id - p1 = obj_utils.create_test_port(self.context, node_id=node.id, - uuid=uuidutils.generate_uuid(), - address="aa:bb:cc:dd:ee:01", - extra=extra, - pxe_enabled=pxe_enabled) - p1.portgroup_id = pg_id - ports.append(p1) - - # Update portgroup_id/pxe_enabled/vif_port_id in one request - p2 = obj_utils.create_test_port(self.context, node_id=node.id, - uuid=uuidutils.generate_uuid(), - address="aa:bb:cc:dd:ee:02", - extra=opposite_extra, - pxe_enabled=opposite_pxe_enabled) - p2.extra = extra - p2.pxe_enabled = pxe_enabled - p2.portgroup_id = pg_id - ports.append(p2) - - # Update portgroup_id and pxe_enabled - p3 = obj_utils.create_test_port(self.context, node_id=node.id, - uuid=uuidutils.generate_uuid(), - address="aa:bb:cc:dd:ee:03", - extra=extra, - pxe_enabled=opposite_pxe_enabled) - p3.pxe_enabled = pxe_enabled - p3.portgroup_id = pg_id - ports.append(p3) - - # Update portgroup_id and vif_port_id - p4 = obj_utils.create_test_port(self.context, node_id=node.id, - uuid=uuidutils.generate_uuid(), - address="aa:bb:cc:dd:ee:04", - pxe_enabled=pxe_enabled, - extra=opposite_extra) - p4.extra = extra - p4.portgroup_id = pg_id - ports.append(p4) - - for port in ports: - if not expect_errors: - res = self.service.update_port(self.context, port) - self.assertEqual(port.pxe_enabled, res.pxe_enabled) - self.assertEqual(port.portgroup_id, res.portgroup_id) - self.assertEqual(port.extra, res.extra) - else: - self.assertRaises(messaging.rpc.ExpectedException, - self.service.update_port, - self.context, port) - - def test_update_port_novif_pxe_noportgroup(self): - self._test_update_port(has_vif=False, in_portgroup=False, - pxe_enabled=True, - expect_errors=False) - - def test_update_port_novif_nopxe_noportgroup(self): - self._test_update_port(has_vif=False, in_portgroup=False, - pxe_enabled=False, - expect_errors=False) - - def test_update_port_vif_pxe_noportgroup(self): - self._test_update_port(has_vif=True, in_portgroup=False, - pxe_enabled=True, - expect_errors=False) - - def test_update_port_vif_nopxe_noportgroup(self): - self._test_update_port(has_vif=True, in_portgroup=False, - pxe_enabled=False, - expect_errors=False) - - def test_update_port_novif_pxe_portgroup_standalone_ports(self): - self._test_update_port(has_vif=False, in_portgroup=True, - pxe_enabled=True, standalone_ports=True, - expect_errors=False) - - def test_update_port_novif_pxe_portgroup_nostandalone_ports(self): - self._test_update_port(has_vif=False, in_portgroup=True, - pxe_enabled=True, standalone_ports=False, - expect_errors=True) - - def test_update_port_novif_nopxe_portgroup_standalone_ports(self): - self._test_update_port(has_vif=False, in_portgroup=True, - pxe_enabled=False, standalone_ports=True, - expect_errors=False) - - def test_update_port_novif_nopxe_portgroup_nostandalone_ports(self): - self._test_update_port(has_vif=False, in_portgroup=True, - pxe_enabled=False, standalone_ports=False, - expect_errors=False) - - def test_update_port_vif_pxe_portgroup_standalone_ports(self): - self._test_update_port(has_vif=True, in_portgroup=True, - pxe_enabled=True, standalone_ports=True, - expect_errors=False) - - def test_update_port_vif_pxe_portgroup_nostandalone_ports(self): - self._test_update_port(has_vif=True, in_portgroup=True, - pxe_enabled=True, standalone_ports=False, - expect_errors=True) - - def test_update_port_vif_nopxe_portgroup_standalone_ports(self): - self._test_update_port(has_vif=True, in_portgroup=True, - pxe_enabled=True, standalone_ports=True, - expect_errors=False) - - def test_update_port_vif_nopxe_portgroup_nostandalone_ports(self): - self._test_update_port(has_vif=True, in_portgroup=True, - pxe_enabled=False, standalone_ports=False, - expect_errors=True) + mock_val.assert_called_once_with(mock.ANY, mock.ANY) + mock_pc.assert_called_once_with(mock.ANY, mock.ANY, port) def test__filter_out_unsupported_types_all(self): self._start_service() @@ -3486,7 +3268,9 @@ class UpdatePortTestCase(mgr_utils.ServiceSetUpMixin, @mgr_utils.mock_record_keepalive class UpdatePortgroupTestCase(mgr_utils.ServiceSetUpMixin, tests_db_base.DbTestCase): - def test_update_portgroup(self): + @mock.patch.object(n_flat.FlatNetwork, 'portgroup_changed', autospec=True) + @mock.patch.object(n_flat.FlatNetwork, 'validate', autospec=True) + def test_update_portgroup(self, mock_val, mock_pc): node = obj_utils.create_test_node(self.context, driver='fake') portgroup = obj_utils.create_test_portgroup(self.context, node_id=node.id, @@ -3496,6 +3280,27 @@ class UpdatePortgroupTestCase(mgr_utils.ServiceSetUpMixin, self.service.update_portgroup(self.context, portgroup) portgroup.refresh() self.assertEqual(new_extra, portgroup.extra) + mock_val.assert_called_once_with(mock.ANY, mock.ANY) + mock_pc.assert_called_once_with(mock.ANY, mock.ANY, portgroup) + + @mock.patch.object(n_flat.FlatNetwork, 'portgroup_changed', autospec=True) + @mock.patch.object(n_flat.FlatNetwork, 'validate', autospec=True) + def test_update_portgroup_failure(self, mock_val, mock_pc): + node = obj_utils.create_test_node(self.context, driver='fake') + portgroup = obj_utils.create_test_portgroup(self.context, + node_id=node.id, + extra={'foo': 'bar'}) + old_extra = portgroup.extra + new_extra = {'foo': 'baz'} + portgroup.extra = new_extra + mock_pc.side_effect = (exception.FailedToUpdateMacOnPort('boom')) + self.assertRaises(messaging.rpc.ExpectedException, + self.service.update_portgroup, + self.context, portgroup) + portgroup.refresh() + self.assertEqual(old_extra, portgroup.extra) + mock_val.assert_called_once_with(mock.ANY, mock.ANY) + mock_pc.assert_called_once_with(mock.ANY, mock.ANY, portgroup) def test_update_portgroup_node_locked(self): node = obj_utils.create_test_node(self.context, driver='fake', @@ -3532,7 +3337,10 @@ class UpdatePortgroupTestCase(mgr_utils.ServiceSetUpMixin, self.assertEqual(old_node_id, portgroup.node_id) @mock.patch.object(dbapi.IMPL, 'get_ports_by_portgroup_id') - def test_update_portgroup_to_node_in_manageable_state(self, + @mock.patch.object(n_flat.FlatNetwork, 'portgroup_changed', autospec=True) + @mock.patch.object(n_flat.FlatNetwork, 'validate', autospec=True) + def test_update_portgroup_to_node_in_manageable_state(self, mock_val, + mock_pgc, mock_get_ports): node = obj_utils.create_test_node(self.context, driver='fake') portgroup = obj_utils.create_test_portgroup(self.context, @@ -3551,10 +3359,14 @@ class UpdatePortgroupTestCase(mgr_utils.ServiceSetUpMixin, portgroup.refresh() self.assertEqual(update_node.id, portgroup.node_id) mock_get_ports.assert_called_once_with(portgroup.uuid) + mock_val.assert_called_once_with(mock.ANY, mock.ANY) + mock_pgc.assert_called_once_with(mock.ANY, mock.ANY, portgroup) @mock.patch.object(dbapi.IMPL, 'get_ports_by_portgroup_id') + @mock.patch.object(n_flat.FlatNetwork, 'portgroup_changed', autospec=True) + @mock.patch.object(n_flat.FlatNetwork, 'validate', autospec=True) def test_update_portgroup_to_node_in_active_state_and_maintenance( - self, mock_get_ports): + self, mock_val, mock_pgc, mock_get_ports): node = obj_utils.create_test_node(self.context, driver='fake') portgroup = obj_utils.create_test_portgroup(self.context, node_id=node.id, @@ -3573,9 +3385,14 @@ class UpdatePortgroupTestCase(mgr_utils.ServiceSetUpMixin, portgroup.refresh() self.assertEqual(update_node.id, portgroup.node_id) mock_get_ports.assert_called_once_with(portgroup.uuid) + mock_val.assert_called_once_with(mock.ANY, mock.ANY) + mock_pgc.assert_called_once_with(mock.ANY, mock.ANY, portgroup) @mock.patch.object(dbapi.IMPL, 'get_ports_by_portgroup_id') - def test_update_portgroup_association_with_ports(self, mock_get_ports): + @mock.patch.object(n_flat.FlatNetwork, 'portgroup_changed', autospec=True) + @mock.patch.object(n_flat.FlatNetwork, 'validate', autospec=True) + def test_update_portgroup_association_with_ports(self, mock_val, + mock_pgc, mock_get_ports): node = obj_utils.create_test_node(self.context, driver='fake') portgroup = obj_utils.create_test_portgroup(self.context, node_id=node.id, @@ -3597,128 +3414,8 @@ class UpdatePortgroupTestCase(mgr_utils.ServiceSetUpMixin, portgroup.refresh() self.assertEqual(old_node_id, portgroup.node_id) mock_get_ports.assert_called_once_with(portgroup.uuid) - - @mock.patch('ironic.dhcp.neutron.NeutronDHCPApi.update_port_address') - def test_update_portgroup_address(self, mac_update_mock): - node = obj_utils.create_test_node(self.context, driver='fake') - pg = obj_utils.create_test_portgroup( - self.context, node_id=node.id, - extra={'vif_port_id': 'fake-id'}) - new_address = '11:22:33:44:55:bb' - pg.address = new_address - self.service.update_portgroup(self.context, pg) - pg.refresh() - self.assertEqual(new_address, pg.address) - mac_update_mock.assert_called_once_with('fake-id', new_address, - token=self.context.auth_token) - - @mock.patch('ironic.dhcp.neutron.NeutronDHCPApi.update_port_address') - def test_update_portgroup_address_fail(self, mac_update_mock): - node = obj_utils.create_test_node(self.context, driver='fake') - pg = obj_utils.create_test_portgroup( - self.context, node_id=node.id, - extra={'vif_port_id': 'fake-id'}) - old_address = pg.address - pg.address = '11:22:33:44:55:bb' - mac_update_mock.side_effect = ( - exception.FailedToUpdateMacOnPort(port_id=pg.uuid)) - exc = self.assertRaises(messaging.rpc.ExpectedException, - self.service.update_portgroup, - self.context, pg) - # Compare true exception hidden by @messaging.expected_exceptions - self.assertEqual(exception.FailedToUpdateMacOnPort, exc.exc_info[0]) - pg.refresh() - self.assertEqual(old_address, pg.address) - - @mock.patch('ironic.dhcp.neutron.NeutronDHCPApi.update_port_address') - def test_update_portgroup_address_no_vif_id(self, mac_update_mock): - node = obj_utils.create_test_node(self.context, driver='fake') - pg = obj_utils.create_test_portgroup(self.context, node_id=node.id) - - new_address = '11:22:33:44:55:bb' - pg.address = new_address - self.service.update_portgroup(self.context, pg) - pg.refresh() - self.assertEqual(new_address, pg.address) - self.assertFalse(mac_update_mock.called) - - def _test_update_portgroup(self, has_vif=False, with_ports=False, - pxe_enabled=True, standalone_ports=True, - expect_errors=False): - node = obj_utils.create_test_node(self.context, driver='fake', - provision_state=states.ENROLL) - - # NOTE(vsaienko) make sure that old values are opposite to new, - # to guarantee that object.what_changes() returns true. - old_standalone_ports_supported = not standalone_ports - - pg = obj_utils.create_test_portgroup( - self.context, node_id=node.id, - standalone_ports_supported=old_standalone_ports_supported) - - if with_ports: - extra = {} - if has_vif: - extra = {'vif_port_id': uuidutils.generate_uuid()} - - obj_utils.create_test_port( - self.context, node_id=node.id, extra=extra, - pxe_enabled=pxe_enabled, portgroup_id=pg.id) - - pg.standalone_ports_supported = standalone_ports - - if not expect_errors: - res = self.service.update_portgroup(self.context, pg) - self.assertEqual(pg.standalone_ports_supported, - res.standalone_ports_supported) - else: - self.assertRaises(messaging.rpc.ExpectedException, - self.service.update_portgroup, - self.context, pg) - - def test_update_portgroup_standalone_ports_noports(self): - self._test_update_portgroup(with_ports=False, standalone_ports=True, - expect_errors=False) - - def test_update_portgroup_standalone_ports_novif_pxe_ports(self): - self._test_update_portgroup(with_ports=True, standalone_ports=True, - has_vif=False, pxe_enabled=True, - expect_errors=False) - - def test_update_portgroup_nostandalone_ports_novif_pxe_ports(self): - self._test_update_portgroup(with_ports=True, standalone_ports=False, - has_vif=False, pxe_enabled=True, - expect_errors=True) - - def test_update_portgroup_nostandalone_ports_novif_nopxe_ports(self): - self._test_update_portgroup(with_ports=True, standalone_ports=False, - has_vif=False, pxe_enabled=False, - expect_errors=False) - - def test_update_portgroup_standalone_ports_novif_nopxe_ports(self): - self._test_update_portgroup(with_ports=True, standalone_ports=True, - has_vif=False, pxe_enabled=False, - expect_errors=False) - - def test_update_portgroup_standalone_ports_vif_pxe_ports(self): - self._test_update_portgroup(with_ports=True, standalone_ports=True, - has_vif=True, pxe_enabled=True, - expect_errors=False) - - def test_update_portgroup_nostandalone_ports_vif_pxe_ports(self): - self._test_update_portgroup(with_ports=True, standalone_ports=False, - has_vif=True, pxe_enabled=True, - expect_errors=True) - - def test_update_portgroup_standalone_ports_vif_nopxe_ports(self): - self._test_update_portgroup(with_ports=True, standalone_ports=True, - has_vif=True, pxe_enabled=False, - expect_errors=False) - - def test_update_portgroup_nostandalone_ports_vif_nopxe_ports(self): - self._test_update_portgroup(with_ports=True, standalone_ports=False, - has_vif=True, pxe_enabled=False, - expect_errors=True) + self.assertFalse(mock_val.called) + self.assertFalse(mock_pgc.called) @mgr_utils.mock_record_keepalive @@ -4536,21 +4233,6 @@ class ManagerCheckDeployTimeoutsTestCase(mgr_utils.CommonMixIn, self.assertEqual([process_event_call] * 2, self.task.process_event.call_args_list) - @mock.patch.object(dbapi.IMPL, 'update_port') - @mock.patch('ironic.dhcp.neutron.NeutronDHCPApi.update_port_address') - def test_update_port_duplicate_mac(self, get_nodeinfo_mock, mapped_mock, - acquire_mock, mac_update_mock, mock_up): - node = utils.create_test_node(driver='fake') - port = obj_utils.create_test_port(self.context, node_id=node.id) - mock_up.side_effect = exception.MACAlreadyExists(mac=port.address) - exc = self.assertRaises(messaging.rpc.ExpectedException, - self.service.update_port, - self.context, port) - # Compare true exception hidden by @messaging.expected_exceptions - self.assertEqual(exception.MACAlreadyExists, exc.exc_info[0]) - # ensure Neutron wasn't updated - self.assertFalse(mac_update_mock.called) - @mgr_utils.mock_record_keepalive class ManagerTestProperties(tests_db_base.DbTestCase): diff --git a/ironic/tests/unit/dhcp/test_neutron.py b/ironic/tests/unit/dhcp/test_neutron.py index ce300519af..b429699fc6 100644 --- a/ironic/tests/unit/dhcp/test_neutron.py +++ b/ironic/tests/unit/dhcp/test_neutron.py @@ -98,74 +98,14 @@ class TestNeutron(db_base.DbTestCase): api.provider.update_port_dhcp_opts, port_id, opts) - @mock.patch.object(client.Client, 'show_port') - @mock.patch.object(client.Client, 'update_port') - @mock.patch.object(client.Client, '__init__') - def test_update_port_address(self, mock_client_init, mock_update_port, - mock_show_port): + @mock.patch('ironic.common.neutron.update_port_address') + def test_update_port_address(self, mock_update_port_addr): address = 'fe:54:00:77:07:d9' port_id = 'fake-port-id' - expected = {'port': {'mac_address': address}} - mock_client_init.return_value = None - mock_show_port.return_value = {} api = dhcp_factory.DHCPFactory() api.provider.update_port_address(port_id, address) - mock_update_port.assert_called_once_with(port_id, expected) - - @mock.patch.object(client.Client, 'show_port') - @mock.patch.object(client.Client, 'update_port') - @mock.patch.object(client.Client, '__init__') - def test_update_port_address_with_binding(self, mock_client_init, - mock_update_port, - mock_show_port): - address = 'fe:54:00:77:07:d9' - port_id = 'fake-port-id' - expected = {'port': {'mac_address': address, - 'binding:host_id': 'host'}} - mock_client_init.return_value = None - mock_show_port.return_value = {'port': {'binding:host_id': 'host'}} - - api = dhcp_factory.DHCPFactory() - api.provider.update_port_address(port_id, address) - mock_update_port.assert_any_call(port_id, - {'port': {'binding:host_id': ''}}) - mock_update_port.assert_any_call(port_id, expected) - - @mock.patch.object(client.Client, 'show_port') - @mock.patch.object(client.Client, 'update_port') - @mock.patch.object(client.Client, '__init__') - def test_update_port_address_show_failed(self, mock_client_init, - mock_update_port, - mock_show_port): - address = 'fe:54:00:77:07:d9' - port_id = 'fake-port-id' - mock_client_init.return_value = None - mock_show_port.side_effect = ( - neutron_client_exc.NeutronClientException()) - - api = dhcp_factory.DHCPFactory() - self.assertRaises(exception.FailedToUpdateMacOnPort, - api.provider.update_port_address, port_id, address) - self.assertFalse(mock_update_port.called) - - @mock.patch.object(client.Client, 'show_port') - @mock.patch.object(client.Client, 'update_port') - @mock.patch.object(client.Client, '__init__') - def test_update_port_address_with_exception(self, mock_client_init, - mock_update_port, - mock_show_port): - address = 'fe:54:00:77:07:d9' - port_id = 'fake-port-id' - mock_client_init.return_value = None - mock_show_port.return_value = {} - - api = dhcp_factory.DHCPFactory() - mock_update_port.side_effect = ( - neutron_client_exc.NeutronClientException()) - self.assertRaises(exception.FailedToUpdateMacOnPort, - api.provider.update_port_address, - port_id, address) + mock_update_port_addr.assert_called_once_with(port_id, address, None) @mock.patch('ironic.dhcp.neutron.NeutronDHCPApi.update_port_dhcp_opts') @mock.patch('ironic.common.network.get_node_vif_ids') @@ -370,13 +310,14 @@ class TestNeutron(db_base.DbTestCase): port = object_utils.create_test_port( self.context, node_id=self.node.id, address='aa:bb:cc:dd:ee:ff', uuid=uuidutils.generate_uuid(), - extra={'vif_port_id': fake_vif} if network == 'tenant' else {}, internal_info={ 'cleaning_vif_port_id': (fake_vif if network == 'cleaning' else None), 'provisioning_vif_port_id': (fake_vif if network == 'provisioning' else None), + 'tenant_vif_port_id': (fake_vif if network == 'tenant' + else None), } ) mock_gfia.return_value = expected @@ -400,12 +341,10 @@ class TestNeutron(db_base.DbTestCase): @mock.patch('ironic.dhcp.neutron.NeutronDHCPApi._get_fixed_ip_address') def test__get_port_ip_address_for_portgroup(self, mock_gfia): expected = "192.168.1.3" - pg = object_utils.create_test_portgroup(self.context, - node_id=self.node.id, - address='aa:bb:cc:dd:ee:ff', - uuid=uuidutils.generate_uuid(), - extra={'vif_port_id': - 'test-vif-A'}) + pg = object_utils.create_test_portgroup( + self.context, node_id=self.node.id, address='aa:bb:cc:dd:ee:ff', + uuid=uuidutils.generate_uuid(), + internal_info={'tenant_vif_port_id': 'test-vif-A'}) mock_gfia.return_value = expected with task_manager.acquire(self.context, self.node.uuid) as task: @@ -447,15 +386,18 @@ class TestNeutron(db_base.DbTestCase): mock.sentinel.client) @mock.patch('ironic.dhcp.neutron.NeutronDHCPApi._get_fixed_ip_address') - def test__get_ip_addresses_ports(self, mock_gfia): + def _test__get_ip_addresses_ports(self, key, mock_gfia): + if key == "extra": + kwargs1 = {key: {'vif_port_id': 'test-vif-A'}} + else: + kwargs1 = {key: {'tenant_vif_port_id': 'test-vif-A'}} ip_address = '10.10.0.1' expected = [ip_address] port = object_utils.create_test_port(self.context, node_id=self.node.id, address='aa:bb:cc:dd:ee:ff', uuid=uuidutils.generate_uuid(), - extra={'vif_port_id': - 'test-vif-A'}) + **kwargs1) mock_gfia.return_value = ip_address with task_manager.acquire(self.context, self.node.uuid) as task: api = dhcp_factory.DHCPFactory().provider @@ -463,22 +405,36 @@ class TestNeutron(db_base.DbTestCase): mock.sentinel.client) self.assertEqual(expected, result) + def test__get_ip_addresses_ports_extra(self): + self._test__get_ip_addresses_ports('extra') + + def test__get_ip_addresses_ports_int_info(self): + self._test__get_ip_addresses_ports('internal_info') + @mock.patch('ironic.dhcp.neutron.NeutronDHCPApi._get_fixed_ip_address') - def test__get_ip_addresses_portgroup(self, mock_gfia): + def _test__get_ip_addresses_portgroup(self, key, mock_gfia): + if key == "extra": + kwargs1 = {key: {'vif_port_id': 'test-vif-A'}} + else: + kwargs1 = {key: {'tenant_vif_port_id': 'test-vif-A'}} ip_address = '10.10.0.1' expected = [ip_address] - pg = object_utils.create_test_portgroup(self.context, - node_id=self.node.id, - address='aa:bb:cc:dd:ee:ff', - uuid=uuidutils.generate_uuid(), - extra={'vif_port_id': - 'test-vif-A'}) + pg = object_utils.create_test_portgroup( + self.context, node_id=self.node.id, + address='aa:bb:cc:dd:ee:ff', uuid=uuidutils.generate_uuid(), + **kwargs1) mock_gfia.return_value = ip_address with task_manager.acquire(self.context, self.node.uuid) as task: api = dhcp_factory.DHCPFactory().provider result = api._get_ip_addresses(task, [pg], mock.sentinel.client) self.assertEqual(expected, result) + def test__get_ip_addresses_portgroup_extra(self): + self._test__get_ip_addresses_portgroup('extra') + + def test__get_ip_addresses_portgroup_int_info(self): + self._test__get_ip_addresses_portgroup('internal_info') + @mock.patch('ironic.dhcp.neutron.NeutronDHCPApi._get_port_ip_address') def test_get_ip_addresses(self, get_ip_mock): ip_address = '10.10.0.1' @@ -495,12 +451,11 @@ class TestNeutron(db_base.DbTestCase): @mock.patch('ironic.dhcp.neutron.NeutronDHCPApi._get_port_ip_address') def test_get_ip_addresses_for_port_and_portgroup(self, get_ip_mock): - object_utils.create_test_portgroup(self.context, - node_id=self.node.id, - address='aa:bb:cc:dd:ee:ff', - uuid=uuidutils.generate_uuid(), - extra={'vif_port_id': - 'test-vif-A'}) + object_utils.create_test_portgroup( + self.context, node_id=self.node.id, address='aa:bb:cc:dd:ee:ff', + uuid=uuidutils.generate_uuid(), + internal_info={'tenant_vif_port_id': 'test-vif-A'}) + with task_manager.acquire(self.context, self.node.uuid) as task: api = dhcp_factory.DHCPFactory().provider api.get_ip_addresses(task) diff --git a/ironic/tests/unit/drivers/modules/network/test_common.py b/ironic/tests/unit/drivers/modules/network/test_common.py new file mode 100644 index 0000000000..4f320f0559 --- /dev/null +++ b/ironic/tests/unit/drivers/modules/network/test_common.py @@ -0,0 +1,555 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +import mock +from oslo_config import cfg +from oslo_utils import uuidutils + +from ironic.common import exception +from ironic.common import neutron as neutron_common +from ironic.conductor import task_manager +from ironic.drivers.modules.network import common +from ironic.tests.unit.conductor import mgr_utils +from ironic.tests.unit.db import base as db_base +from ironic.tests.unit.objects import utils as obj_utils + +CONF = cfg.CONF + + +class TestVifPortIDMixin(db_base.DbTestCase): + + def setUp(self): + super(TestVifPortIDMixin, self).setUp() + self.config(enabled_drivers=['fake']) + mgr_utils.mock_the_extension_manager() + self.interface = common.VIFPortIDMixin() + self.node = obj_utils.create_test_node(self.context, + network_interface='neutron') + self.port = obj_utils.create_test_port( + self.context, node_id=self.node.id, + address='52:54:00:cf:2d:32', + extra={'vif_port_id': uuidutils.generate_uuid(), + 'client-id': 'fake1'}) + self.neutron_port = {'id': '132f871f-eaec-4fed-9475-0d54465e0f00', + 'mac_address': '52:54:00:cf:2d:32'} + + def test_vif_list_extra(self): + vif_id = uuidutils.generate_uuid() + self.port.extra = {'vif_port_id': vif_id} + self.port.save() + with task_manager.acquire(self.context, self.node.id) as task: + vifs = self.interface.vif_list(task) + self.assertEqual(vifs, [{'id': vif_id}]) + + def test_vif_list_internal(self): + vif_id = uuidutils.generate_uuid() + self.port.internal_info = {common.TENANT_VIF_KEY: vif_id} + self.port.save() + with task_manager.acquire(self.context, self.node.id) as task: + vifs = self.interface.vif_list(task) + self.assertEqual(vifs, [{'id': vif_id}]) + + def test_vif_list_extra_and_internal_priority(self): + vif_id = uuidutils.generate_uuid() + vif_id2 = uuidutils.generate_uuid() + self.port.extra = {'vif_port_id': vif_id2} + self.port.internal_info = {common.TENANT_VIF_KEY: vif_id} + self.port.save() + with task_manager.acquire(self.context, self.node.id) as task: + vifs = self.interface.vif_list(task) + self.assertEqual(vifs, [{'id': vif_id}]) + + @mock.patch.object(neutron_common, 'get_client') + @mock.patch.object(neutron_common, 'update_port_address') + def test_vif_attach(self, mock_upa, mock_client): + self.port.extra = {} + self.port.save() + vif = {'id': "fake_vif_id"} + with task_manager.acquire(self.context, self.node.id) as task: + self.interface.vif_attach(task, vif) + self.port.refresh() + self.assertEqual(self.port.internal_info.get( + common.TENANT_VIF_KEY), "fake_vif_id") + mock_upa.assert_called_once_with("fake_vif_id", self.port.address) + + @mock.patch.object(neutron_common, 'get_client') + @mock.patch.object(neutron_common, 'update_port_address') + def test_vif_attach_update_port_exception(self, mock_upa, mock_client): + self.port.extra = {} + self.port.save() + vif = {'id': "fake_vif_id"} + mock_upa.side_effect = ( + exception.FailedToUpdateMacOnPort(port_id='fake')) + with task_manager.acquire(self.context, self.node.id) as task: + self.assertRaisesRegexp( + exception.NetworkError, "can not update Neutron port", + self.interface.vif_attach, task, vif) + + def test_attach_port_no_ports_left_extra(self): + vif = {'id': "fake_vif_id"} + with task_manager.acquire(self.context, self.node.id) as task: + self.assertRaisesRegexp( + exception.NoFreePhysicalPorts, "not enough free physical", + self.interface.vif_attach, task, vif) + + def test_attach_port_no_ports_left_internal_info(self): + self.port.internal_info = { + common.TENANT_VIF_KEY: self.port.extra['vif_port_id']} + self.port.extra = {} + self.port.save() + vif = {'id': "fake_vif_id"} + with task_manager.acquire(self.context, self.node.id) as task: + self.assertRaisesRegexp( + exception.NoFreePhysicalPorts, "not enough free physical", + self.interface.vif_attach, task, vif) + + def test_attach_port_vif_already_attached_extra(self): + vif = {'id': self.port.extra['vif_port_id']} + with task_manager.acquire(self.context, self.node.id) as task: + self.assertRaisesRegexp( + exception.VifAlreadyAttached, "already attached", + self.interface.vif_attach, task, vif) + + def test_attach_port_vif_already_attach_internal_info(self): + vif = {'id': self.port.extra['vif_port_id']} + self.port.internal_info = { + common.TENANT_VIF_KEY: self.port.extra['vif_port_id']} + self.port.extra = {} + self.port.save() + with task_manager.acquire(self.context, self.node.id) as task: + self.assertRaisesRegexp( + exception.VifAlreadyAttached, "already attached", + self.interface.vif_attach, task, vif) + + def test_vif_detach_in_extra(self): + with task_manager.acquire(self.context, self.node.id) as task: + self.interface.vif_detach( + task, self.port.extra['vif_port_id']) + self.port.refresh() + self.assertFalse('vif_port_id' in self.port.extra) + self.assertFalse(common.TENANT_VIF_KEY in self.port.internal_info) + + def test_vif_detach_in_internal_info(self): + self.port.internal_info = { + common.TENANT_VIF_KEY: self.port.extra['vif_port_id']} + self.port.extra = {} + self.port.save() + with task_manager.acquire(self.context, self.node.id) as task: + self.interface.vif_detach( + task, self.port.internal_info[common.TENANT_VIF_KEY]) + self.port.refresh() + self.assertFalse('vif_port_id' in self.port.extra) + self.assertFalse(common.TENANT_VIF_KEY in self.port.internal_info) + + def test_vif_detach_not_attached(self): + with task_manager.acquire(self.context, self.node.id) as task: + self.assertRaisesRegexp( + exception.VifNotAttached, "it is not attached to it.", + self.interface.vif_detach, task, 'aaa') + + def test_get_current_vif_extra_vif_port_id(self): + extra = {'vif_port_id': 'foo'} + self.port.extra = extra + self.port.save() + with task_manager.acquire(self.context, self.node.id) as task: + vif = self.interface.get_current_vif(task, self.port) + self.assertEqual('foo', vif) + + def test_get_current_vif_internal_info_cleaning(self): + internal_info = {'cleaning_vif_port_id': 'foo', + 'tenant_vif_port_id': 'bar'} + self.port.internal_info = internal_info + self.port.save() + with task_manager.acquire(self.context, self.node.id) as task: + vif = self.interface.get_current_vif(task, self.port) + self.assertEqual('foo', vif) + + def test_get_current_vif_internal_info_provisioning(self): + internal_info = {'provisioning_vif_port_id': 'foo', + 'vif_port_id': 'bar'} + self.port.internal_info = internal_info + self.port.save() + with task_manager.acquire(self.context, self.node.id) as task: + vif = self.interface.get_current_vif(task, self.port) + self.assertEqual('foo', vif) + + def test_get_current_vif_internal_info_tenant_vif(self): + internal_info = {'tenant_vif_port_id': 'bar'} + self.port.internal_info = internal_info + self.port.save() + with task_manager.acquire(self.context, self.node.id) as task: + vif = self.interface.get_current_vif(task, self.port) + self.assertEqual('bar', vif) + + def test_get_current_vif_none(self): + internal_info = extra = {} + self.port.internal_info = internal_info + self.port.extra = extra + self.port.save() + with task_manager.acquire(self.context, self.node.id) as task: + vif = self.interface.get_current_vif(task, self.port) + self.assertIsNone(vif) + + @mock.patch.object(neutron_common, 'update_port_address', autospec=True) + def test_port_changed_address(self, mac_update_mock): + new_address = '11:22:33:44:55:bb' + self.port.address = new_address + with task_manager.acquire(self.context, self.node.id) as task: + self.interface.port_changed(task, self.port) + mac_update_mock.assert_called_once_with( + self.port.extra['vif_port_id'], + new_address, token=task.context.auth_token) + + @mock.patch.object(neutron_common, 'update_port_address', autospec=True) + def test_port_changed_address_VIF_MAC_update_fail(self, mac_update_mock): + new_address = '11:22:33:44:55:bb' + self.port.address = new_address + mac_update_mock.side_effect = ( + exception.FailedToUpdateMacOnPort(port_id=self.port.uuid)) + with task_manager.acquire(self.context, self.node.id) as task: + self.assertRaises(exception.FailedToUpdateMacOnPort, + self.interface.port_changed, + task, self.port) + mac_update_mock.assert_called_once_with( + self.port.extra['vif_port_id'], + new_address, token=task.context.auth_token) + + @mock.patch.object(neutron_common, 'update_port_address', autospec=True) + def test_port_changed_address_no_vif_id(self, mac_update_mock): + self.port.extra = {} + self.port.save() + self.port.address = '11:22:33:44:55:bb' + with task_manager.acquire(self.context, self.node.id) as task: + self.interface.port_changed(task, self.port) + self.assertFalse(mac_update_mock.called) + + @mock.patch('ironic.dhcp.neutron.NeutronDHCPApi.update_port_dhcp_opts') + def test_port_changed_client_id(self, dhcp_update_mock): + expected_extra = {'vif_port_id': 'fake-id', 'client-id': 'fake2'} + expected_dhcp_opts = [{'opt_name': 'client-id', 'opt_value': 'fake2'}] + self.port.extra = expected_extra + with task_manager.acquire(self.context, self.node.id) as task: + self.interface.port_changed(task, self.port) + dhcp_update_mock.assert_called_once_with( + 'fake-id', expected_dhcp_opts, token=self.context.auth_token) + + @mock.patch('ironic.dhcp.neutron.NeutronDHCPApi.update_port_dhcp_opts') + def test_port_changed_vif(self, dhcp_update_mock): + expected_extra = {'vif_port_id': 'new_ake-id', 'client-id': 'fake1'} + self.port.extra = expected_extra + with task_manager.acquire(self.context, self.node.id) as task: + self.interface.port_changed(task, self.port) + self.assertFalse(dhcp_update_mock.called) + + @mock.patch('ironic.dhcp.neutron.NeutronDHCPApi.update_port_dhcp_opts') + def test_port_changed_client_id_fail(self, dhcp_update_mock): + self.port.extra = {'vif_port_id': 'fake-id', 'client-id': 'fake2'} + dhcp_update_mock.side_effect = ( + exception.FailedToUpdateDHCPOptOnPort(port_id=self.port.uuid)) + with task_manager.acquire(self.context, self.node.id) as task: + self.assertRaises(exception.FailedToUpdateDHCPOptOnPort, + self.interface.port_changed, + task, self.port) + + @mock.patch('ironic.dhcp.neutron.NeutronDHCPApi.update_port_dhcp_opts') + def test_port_changed_client_id_no_vif_id(self, dhcp_update_mock): + self.port.extra = {'client-id': 'fake1'} + self.port.save() + self.port.extra = {'client-id': 'fake2'} + with task_manager.acquire(self.context, self.node.id) as task: + self.interface.port_changed(task, self.port) + self.assertFalse(dhcp_update_mock.called) + + @mock.patch('ironic.dhcp.neutron.NeutronDHCPApi.update_port_dhcp_opts') + def test_port_changed_message_format_failure(self, dhcp_update_mock): + pg = obj_utils.create_test_portgroup( + self.context, node_id=self.node.id, + standalone_ports_supported=False) + + port = obj_utils.create_test_port(self.context, node_id=self.node.id, + uuid=uuidutils.generate_uuid(), + address="aa:bb:cc:dd:ee:01", + extra={'vif_port_id': 'blah'}, + pxe_enabled=False) + port.portgroup_id = pg.id + + with task_manager.acquire(self.context, self.node.id) as task: + self.assertRaisesRegex(exception.Conflict, + "VIF blah is attached to the port", + self.interface.port_changed, + task, port) + + def _test_port_changed(self, has_vif=False, in_portgroup=False, + pxe_enabled=True, standalone_ports=True, + expect_errors=False): + pg = obj_utils.create_test_portgroup( + self.context, node_id=self.node.id, + standalone_ports_supported=standalone_ports) + + extra_vif = {'vif_port_id': uuidutils.generate_uuid()} + if has_vif: + extra = extra_vif + opposite_extra = {} + else: + extra = {} + opposite_extra = extra_vif + opposite_pxe_enabled = not pxe_enabled + + pg_id = None + if in_portgroup: + pg_id = pg.id + + ports = [] + + # Update only portgroup id on existed port with different + # combinations of pxe_enabled/vif_port_id + p1 = obj_utils.create_test_port(self.context, node_id=self.node.id, + uuid=uuidutils.generate_uuid(), + address="aa:bb:cc:dd:ee:01", + extra=extra, + pxe_enabled=pxe_enabled) + p1.portgroup_id = pg_id + ports.append(p1) + + # Update portgroup_id/pxe_enabled/vif_port_id in one request + p2 = obj_utils.create_test_port(self.context, node_id=self.node.id, + uuid=uuidutils.generate_uuid(), + address="aa:bb:cc:dd:ee:02", + extra=opposite_extra, + pxe_enabled=opposite_pxe_enabled) + p2.extra = extra + p2.pxe_enabled = pxe_enabled + p2.portgroup_id = pg_id + ports.append(p2) + + # Update portgroup_id and pxe_enabled + p3 = obj_utils.create_test_port(self.context, node_id=self.node.id, + uuid=uuidutils.generate_uuid(), + address="aa:bb:cc:dd:ee:03", + extra=extra, + pxe_enabled=opposite_pxe_enabled) + p3.pxe_enabled = pxe_enabled + p3.portgroup_id = pg_id + ports.append(p3) + + # Update portgroup_id and vif_port_id + p4 = obj_utils.create_test_port(self.context, node_id=self.node.id, + uuid=uuidutils.generate_uuid(), + address="aa:bb:cc:dd:ee:04", + pxe_enabled=pxe_enabled, + extra=opposite_extra) + p4.extra = extra + p4.portgroup_id = pg_id + ports.append(p4) + + for port in ports: + with task_manager.acquire(self.context, self.node.id) as task: + if not expect_errors: + self.interface.port_changed(task, port) + else: + self.assertRaises(exception.Conflict, + self.interface.port_changed, + task, port) + + def test_port_changed_novif_pxe_noportgroup(self): + self._test_port_changed(has_vif=False, in_portgroup=False, + pxe_enabled=True, + expect_errors=False) + + def test_port_changed_novif_nopxe_noportgroup(self): + self._test_port_changed(has_vif=False, in_portgroup=False, + pxe_enabled=False, + expect_errors=False) + + def test_port_changed_vif_pxe_noportgroup(self): + self._test_port_changed(has_vif=True, in_portgroup=False, + pxe_enabled=True, + expect_errors=False) + + def test_port_changed_vif_nopxe_noportgroup(self): + self._test_port_changed(has_vif=True, in_portgroup=False, + pxe_enabled=False, + expect_errors=False) + + def test_port_changed_novif_pxe_portgroup_standalone_ports(self): + self._test_port_changed(has_vif=False, in_portgroup=True, + pxe_enabled=True, standalone_ports=True, + expect_errors=False) + + def test_port_changed_novif_pxe_portgroup_nostandalone_ports(self): + self._test_port_changed(has_vif=False, in_portgroup=True, + pxe_enabled=True, standalone_ports=False, + expect_errors=True) + + def test_port_changed_novif_nopxe_portgroup_standalone_ports(self): + self._test_port_changed(has_vif=False, in_portgroup=True, + pxe_enabled=False, standalone_ports=True, + expect_errors=False) + + def test_port_changed_novif_nopxe_portgroup_nostandalone_ports(self): + self._test_port_changed(has_vif=False, in_portgroup=True, + pxe_enabled=False, standalone_ports=False, + expect_errors=False) + + def test_port_changed_vif_pxe_portgroup_standalone_ports(self): + self._test_port_changed(has_vif=True, in_portgroup=True, + pxe_enabled=True, standalone_ports=True, + expect_errors=False) + + def test_port_changed_vif_pxe_portgroup_nostandalone_ports(self): + self._test_port_changed(has_vif=True, in_portgroup=True, + pxe_enabled=True, standalone_ports=False, + expect_errors=True) + + def test_port_changed_vif_nopxe_portgroup_standalone_ports(self): + self._test_port_changed(has_vif=True, in_portgroup=True, + pxe_enabled=True, standalone_ports=True, + expect_errors=False) + + def test_port_changed_vif_nopxe_portgroup_nostandalone_ports(self): + self._test_port_changed(has_vif=True, in_portgroup=True, + pxe_enabled=False, standalone_ports=False, + expect_errors=True) + + @mock.patch.object(neutron_common, 'update_port_address', autospec=True) + def test_update_portgroup_address(self, mac_update_mock): + pg = obj_utils.create_test_portgroup( + self.context, node_id=self.node.id, + extra={'vif_port_id': 'fake-id'}) + new_address = '11:22:33:44:55:bb' + pg.address = new_address + with task_manager.acquire(self.context, self.node.id) as task: + self.interface.portgroup_changed(task, pg) + mac_update_mock.assert_called_once_with('fake-id', new_address, + token=self.context.auth_token) + + @mock.patch.object(neutron_common, 'update_port_address', autospec=True) + def test_update_portgroup_address_fail(self, mac_update_mock): + pg = obj_utils.create_test_portgroup( + self.context, node_id=self.node.id, + extra={'vif_port_id': 'fake-id'}) + new_address = '11:22:33:44:55:bb' + pg.address = new_address + mac_update_mock.side_effect = ( + exception.FailedToUpdateMacOnPort('boom')) + with task_manager.acquire(self.context, self.node.id) as task: + self.assertRaises(exception.FailedToUpdateMacOnPort, + self.interface.portgroup_changed, + task, pg) + mac_update_mock.assert_called_once_with('fake-id', new_address, + token=self.context.auth_token) + + @mock.patch.object(neutron_common, 'update_port_address', autospec=True) + def test_update_portgroup_address_no_vif(self, mac_update_mock): + pg = obj_utils.create_test_portgroup( + self.context, node_id=self.node.id) + new_address = '11:22:33:44:55:bb' + pg.address = new_address + with task_manager.acquire(self.context, self.node.id) as task: + self.interface.portgroup_changed(task, pg) + self.assertEqual(new_address, pg.address) + self.assertFalse(mac_update_mock.called) + + @mock.patch.object(neutron_common, 'update_port_address', autospec=True) + def test_update_portgroup_nostandalone_ports_pxe_ports_exc( + self, mac_update_mock): + pg = obj_utils.create_test_portgroup( + self.context, node_id=self.node.id) + extra = {'vif_port_id': 'foo'} + obj_utils.create_test_port( + self.context, node_id=self.node.id, extra=extra, + pxe_enabled=True, portgroup_id=pg.id, + address="aa:bb:cc:dd:ee:01", + uuid=uuidutils.generate_uuid()) + + pg.standalone_ports_supported = False + with task_manager.acquire(self.context, self.node.id) as task: + self.assertRaisesRegex(exception.Conflict, + "VIF foo is attached to this port", + self.interface.portgroup_changed, + task, pg) + + def _test_update_portgroup(self, has_vif=False, with_ports=False, + pxe_enabled=True, standalone_ports=True, + expect_errors=False): + # NOTE(vsaienko) make sure that old values are opposite to new, + # to guarantee that object.what_changes() returns true. + old_standalone_ports_supported = not standalone_ports + + pg = obj_utils.create_test_portgroup( + self.context, node_id=self.node.id, + standalone_ports_supported=old_standalone_ports_supported) + + if with_ports: + extra = {} + if has_vif: + extra = {'vif_port_id': uuidutils.generate_uuid()} + + obj_utils.create_test_port( + self.context, node_id=self.node.id, extra=extra, + pxe_enabled=pxe_enabled, portgroup_id=pg.id, + address="aa:bb:cc:dd:ee:01", + uuid=uuidutils.generate_uuid()) + + pg.standalone_ports_supported = standalone_ports + + with task_manager.acquire(self.context, self.node.id) as task: + if not expect_errors: + self.interface.portgroup_changed(task, pg) + else: + self.assertRaises(exception.Conflict, + self.interface.portgroup_changed, + task, pg) + + def test_update_portgroup_standalone_ports_noports(self): + self._test_update_portgroup(with_ports=False, standalone_ports=True, + expect_errors=False) + + def test_update_portgroup_standalone_ports_novif_pxe_ports(self): + self._test_update_portgroup(with_ports=True, standalone_ports=True, + has_vif=False, pxe_enabled=True, + expect_errors=False) + + def test_update_portgroup_nostandalone_ports_novif_pxe_ports(self): + self._test_update_portgroup(with_ports=True, standalone_ports=False, + has_vif=False, pxe_enabled=True, + expect_errors=True) + + def test_update_portgroup_nostandalone_ports_novif_nopxe_ports(self): + self._test_update_portgroup(with_ports=True, standalone_ports=False, + has_vif=False, pxe_enabled=False, + expect_errors=False) + + def test_update_portgroup_standalone_ports_novif_nopxe_ports(self): + self._test_update_portgroup(with_ports=True, standalone_ports=True, + has_vif=False, pxe_enabled=False, + expect_errors=False) + + def test_update_portgroup_standalone_ports_vif_pxe_ports(self): + self._test_update_portgroup(with_ports=True, standalone_ports=True, + has_vif=True, pxe_enabled=True, + expect_errors=False) + + def test_update_portgroup_nostandalone_ports_vif_pxe_ports(self): + self._test_update_portgroup(with_ports=True, standalone_ports=False, + has_vif=True, pxe_enabled=True, + expect_errors=True) + + def test_update_portgroup_standalone_ports_vif_nopxe_ports(self): + self._test_update_portgroup(with_ports=True, standalone_ports=True, + has_vif=True, pxe_enabled=False, + expect_errors=False) + + def test_update_portgroup_nostandalone_ports_vif_nopxe_ports(self): + self._test_update_portgroup(with_ports=True, standalone_ports=False, + has_vif=True, pxe_enabled=False, + expect_errors=True) diff --git a/ironic/tests/unit/drivers/modules/network/test_flat.py b/ironic/tests/unit/drivers/modules/network/test_flat.py index 240da257dc..a70a4dbf11 100644 --- a/ironic/tests/unit/drivers/modules/network/test_flat.py +++ b/ironic/tests/unit/drivers/modules/network/test_flat.py @@ -11,9 +11,11 @@ # under the License. import mock +from neutronclient.common import exceptions as neutron_exceptions from oslo_config import cfg from oslo_utils import uuidutils +from ironic.common import exception from ironic.common import neutron from ironic.conductor import task_manager from ironic.drivers.modules.network import flat as flat_interface @@ -22,6 +24,7 @@ from ironic.tests.unit.db import base as db_base from ironic.tests.unit.objects import utils CONF = cfg.CONF +VIFMIXINPATH = 'ironic.drivers.modules.network.common.VIFPortIDMixin' class TestFlatInterface(db_base.DbTestCase): @@ -37,6 +40,33 @@ class TestFlatInterface(db_base.DbTestCase): internal_info={ 'cleaning_vif_port_id': uuidutils.generate_uuid()}) + @mock.patch('%s.vif_list' % VIFMIXINPATH) + def test_vif_list(self, mock_vif_list): + with task_manager.acquire(self.context, self.node.id) as task: + self.interface.vif_list(task) + mock_vif_list.assert_called_once_with(task) + + @mock.patch('%s.vif_attach' % VIFMIXINPATH) + def test_vif_attach(self, mock_vif_attach): + vif = mock.MagicMock() + with task_manager.acquire(self.context, self.node.id) as task: + self.interface.vif_attach(task, vif) + mock_vif_attach.assert_called_once_with(task, vif) + + @mock.patch('%s.vif_detach' % VIFMIXINPATH) + def test_vif_detach(self, mock_vif_detach): + vif_id = "vif" + with task_manager.acquire(self.context, self.node.id) as task: + self.interface.vif_detach(task, vif_id) + mock_vif_detach.assert_called_once_with(task, vif_id) + + @mock.patch('%s.port_changed' % VIFMIXINPATH) + def test_vif_port_changed(self, mock_p_changed): + port = mock.MagicMock() + with task_manager.acquire(self.context, self.node.id) as task: + self.interface.port_changed(task, port) + mock_p_changed.assert_called_once_with(task, port) + @mock.patch.object(flat_interface, 'LOG') def test_init_incorrect_cleaning_net(self, mock_log): self.config(cleaning_network=None, group='neutron') @@ -84,8 +114,55 @@ class TestFlatInterface(db_base.DbTestCase): self.port.refresh() self.assertNotIn('cleaning_vif_port_id', self.port.internal_info) - def test_unconfigure_tenant_networks(self): + @mock.patch.object(neutron, 'get_client') + def test_add_provisioning_network_set_binding_host_id( + self, client_mock): + upd_mock = mock.Mock() + client_mock.return_value.update_port = upd_mock + instance_info = self.node.instance_info + instance_info['nova_host_id'] = 'nova_host_id' + self.node.instance_info = instance_info + self.node.save() + extra = {'vif_port_id': 'foo'} + utils.create_test_port(self.context, node_id=self.node.id, + address='52:54:00:cf:2d:33', extra=extra, + uuid=uuidutils.generate_uuid()) + exp_body = {'port': {'binding:host_id': 'nova_host_id'}} with task_manager.acquire(self.context, self.node.id) as task: - self.interface.unconfigure_tenant_networks(task) - self.port.refresh() - self.assertNotIn('vif_port_id', self.port.extra) + self.interface.add_provisioning_network(task) + upd_mock.assert_called_once_with('foo', exp_body) + + @mock.patch.object(neutron, 'get_client') + def test_add_provisioning_network_no_binding_host_id( + self, client_mock): + upd_mock = mock.Mock() + client_mock.return_value.update_port = upd_mock + instance_info = self.node.instance_info + instance_info.pop('nova_host_id', None) + self.node.instance_info = instance_info + self.node.save() + extra = {'vif_port_id': 'foo'} + utils.create_test_port(self.context, node_id=self.node.id, + address='52:54:00:cf:2d:33', extra=extra, + uuid=uuidutils.generate_uuid()) + with task_manager.acquire(self.context, self.node.id) as task: + self.interface.add_provisioning_network(task) + self.assertFalse(upd_mock.called) + + @mock.patch.object(neutron, 'get_client') + def test_add_provisioning_network_binding_host_id_raise( + self, client_mock): + client_mock.return_value.update_port.side_effect = \ + (neutron_exceptions.ConnectionFailed()) + instance_info = self.node.instance_info + instance_info['nova_host_id'] = 'nova_host_id' + self.node.instance_info = instance_info + self.node.save() + extra = {'vif_port_id': 'foo'} + utils.create_test_port(self.context, node_id=self.node.id, + address='52:54:00:cf:2d:33', extra=extra, + uuid=uuidutils.generate_uuid()) + with task_manager.acquire(self.context, self.node.id) as task: + self.assertRaises(exception.NetworkError, + self.interface.add_provisioning_network, + task) diff --git a/ironic/tests/unit/drivers/modules/network/test_neutron.py b/ironic/tests/unit/drivers/modules/network/test_neutron.py index 22742a6d84..efe0f22fa7 100644 --- a/ironic/tests/unit/drivers/modules/network/test_neutron.py +++ b/ironic/tests/unit/drivers/modules/network/test_neutron.py @@ -28,6 +28,7 @@ from ironic.tests.unit.objects import utils CONF = cfg.CONF CLIENT_ID1 = '20:00:55:04:01:fe:80:00:00:00:00:00:00:00:02:c9:02:00:23:13:92' CLIENT_ID2 = '20:00:55:04:01:fe:80:00:00:00:00:00:00:00:02:c9:02:00:23:13:93' +VIFMIXINPATH = 'ironic.drivers.modules.network.common.VIFPortIDMixin' class NeutronInterfaceTestCase(db_base.DbTestCase): @@ -46,6 +47,33 @@ class NeutronInterfaceTestCase(db_base.DbTestCase): self.neutron_port = {'id': '132f871f-eaec-4fed-9475-0d54465e0f00', 'mac_address': '52:54:00:cf:2d:32'} + @mock.patch('%s.vif_list' % VIFMIXINPATH) + def test_vif_list(self, mock_vif_list): + with task_manager.acquire(self.context, self.node.id) as task: + self.interface.vif_list(task) + mock_vif_list.assert_called_once_with(task) + + @mock.patch('%s.vif_attach' % VIFMIXINPATH) + def test_vif_attach(self, mock_vif_attach): + vif = mock.MagicMock() + with task_manager.acquire(self.context, self.node.id) as task: + self.interface.vif_attach(task, vif) + mock_vif_attach.assert_called_once_with(task, vif) + + @mock.patch('%s.vif_detach' % VIFMIXINPATH) + def test_vif_detach(self, mock_vif_detach): + vif_id = "vif" + with task_manager.acquire(self.context, self.node.id) as task: + self.interface.vif_detach(task, vif_id) + mock_vif_detach.assert_called_once_with(task, vif_id) + + @mock.patch('%s.port_changed' % VIFMIXINPATH) + def test_vif_port_changed(self, mock_p_changed): + port = mock.MagicMock() + with task_manager.acquire(self.context, self.node.id) as task: + self.interface.port_changed(task, port) + mock_p_changed.assert_called_once_with(task, port) + def test_init_incorrect_provisioning_net(self): self.config(provisioning_network=None, group='neutron') self.assertRaises(exception.DriverLoadError, neutron.NeutronNetwork) @@ -187,12 +215,12 @@ class NeutronInterfaceTestCase(db_base.DbTestCase): self.port.refresh() self.assertNotIn('cleaning_vif_port_id', self.port.internal_info) - @mock.patch.object(neutron_common, 'remove_neutron_ports') - def test_unconfigure_tenant_networks(self, remove_ports_mock): + @mock.patch.object(neutron_common, 'unbind_neutron_port') + def test_unconfigure_tenant_networks(self, mock_unbind_port): with task_manager.acquire(self.context, self.node.id) as task: self.interface.unconfigure_tenant_networks(task) - remove_ports_mock.assert_called_once_with( - task, {'device_id': task.node.uuid}) + mock_unbind_port.assert_called_once_with( + self.port.extra['vif_port_id'], token=None) def test_configure_tenant_networks_no_ports_for_node(self): n = utils.create_test_node(self.context, network_interface='neutron', @@ -258,16 +286,25 @@ class NeutronInterfaceTestCase(db_base.DbTestCase): client_mock.assert_called_once_with(task.context.auth_token) @mock.patch.object(neutron_common, 'get_client') - def _test_configure_tenant_networks(self, client_mock, is_client_id=False): + def _test_configure_tenant_networks(self, client_mock, is_client_id=False, + vif_int_info=False): upd_mock = mock.Mock() client_mock.return_value.update_port = upd_mock + if vif_int_info: + kwargs = {'internal_info': { + 'tenant_vif_port_id': uuidutils.generate_uuid()}} + self.port.internal_info = { + 'tenant_vif_port_id': self.port.extra['vif_port_id']} + self.port.extra = {} + else: + kwargs = {'extra': {'vif_port_id': uuidutils.generate_uuid()}} second_port = utils.create_test_port( self.context, node_id=self.node.id, address='52:54:00:cf:2d:33', - extra={'vif_port_id': uuidutils.generate_uuid()}, uuid=uuidutils.generate_uuid(), local_link_connection={'switch_id': '0a:1b:2c:3d:4e:ff', 'port_id': 'Ethernet1/1', - 'switch_info': 'switch2'} + 'switch_info': 'switch2'}, + **kwargs ) if is_client_id: client_ids = (CLIENT_ID1, CLIENT_ID2) @@ -303,17 +340,28 @@ class NeutronInterfaceTestCase(db_base.DbTestCase): with task_manager.acquire(self.context, self.node.id) as task: self.interface.configure_tenant_networks(task) client_mock.assert_called_once_with(task.context.auth_token) + if vif_int_info: + portid1 = self.port.internal_info['tenant_vif_port_id'] + portid2 = second_port.internal_info['tenant_vif_port_id'] + else: + portid1 = self.port.extra['vif_port_id'] + portid2 = second_port.extra['vif_port_id'] upd_mock.assert_has_calls( - [mock.call(self.port.extra['vif_port_id'], port1_body), - mock.call(second_port.extra['vif_port_id'], port2_body)], + [mock.call(portid1, port1_body), + mock.call(portid2, port2_body)], any_order=True ) - def test_configure_tenant_networks(self): + def test_configure_tenant_networks_vif_extra(self): self.node.instance_uuid = uuidutils.generate_uuid() self.node.save() self._test_configure_tenant_networks() + def test_configure_tenant_networks_vif_int_info(self): + self.node.instance_uuid = uuidutils.generate_uuid() + self.node.save() + self._test_configure_tenant_networks(vif_int_info=True) + def test_configure_tenant_networks_no_instance_uuid(self): self._test_configure_tenant_networks() diff --git a/ironic/tests/unit/drivers/modules/test_deploy_utils.py b/ironic/tests/unit/drivers/modules/test_deploy_utils.py index f3c5a4abdb..639c6e0868 100644 --- a/ironic/tests/unit/drivers/modules/test_deploy_utils.py +++ b/ironic/tests/unit/drivers/modules/test_deploy_utils.py @@ -1169,6 +1169,16 @@ class VirtualMediaDeployUtilsTestCase(db_base.DbTestCase): self.context, driver='iscsi_ilo', driver_info=info_dict) def test_get_single_nic_with_vif_port_id(self): + obj_utils.create_test_port( + self.context, node_id=self.node.id, address='aa:bb:cc:dd:ee:ff', + uuid=uuidutils.generate_uuid(), + internal_info={'tenant_vif_port_id': 'test-vif-A'}) + with task_manager.acquire(self.context, self.node.uuid, + shared=False) as task: + address = utils.get_single_nic_with_vif_port_id(task) + self.assertEqual('aa:bb:cc:dd:ee:ff', address) + + def test_get_single_nic_with_vif_port_id_extra(self): obj_utils.create_test_port( self.context, node_id=self.node.id, address='aa:bb:cc:dd:ee:ff', uuid=uuidutils.generate_uuid(), diff --git a/ironic/tests/unit/drivers/test_base.py b/ironic/tests/unit/drivers/test_base.py index cc9d4b0c95..50c4627684 100644 --- a/ironic/tests/unit/drivers/test_base.py +++ b/ironic/tests/unit/drivers/test_base.py @@ -21,6 +21,7 @@ from ironic.common import exception from ironic.common import raid from ironic.drivers import base as driver_base from ironic.drivers.modules import fake +from ironic.drivers.modules.network import common as net_common from ironic.tests import base @@ -408,3 +409,92 @@ class TestDeployInterface(base.TestCase): driver='driver')), 'url') self.assertTrue(mock_log.called) + + +class TestNetwork(driver_base.NetworkInterface): + + def add_provisioning_network(self, task): + pass + + def remove_provisioning_network(self, task): + pass + + def add_cleaning_network(self, task): + pass + + def remove_cleaning_network(self, task): + pass + + def configure_tenant_networks(self, task): + pass + + def unconfigure_tenant_networks(self, task): + pass + + +class NetworkInterfaceTestCase(base.TestCase): + + @mock.patch.object(driver_base.LOG, 'warning', autospec=True) + @mock.patch.object(net_common.VIFPortIDMixin, 'vif_list') + def test_vif_list(self, mock_vif, mock_warn): + mock_task = mock.MagicMock() + network = TestNetwork() + network.vif_list(mock_task) + mock_vif.assert_called_once_with(mock_task) + mock_warn.assert_called_once_with(mock.ANY, 'TestNetwork') + # Test if the log is only called once even if we recreate a new + # NetworkInterface and call it again. + # NOTE(sambetts): This must be done inside this test instead of a + # separate test because otherwise we end up race conditions between the + # test cases regarding the class variables that are set. + network = TestNetwork() + network.vif_list(mock_task) + mock_warn.assert_called_once_with(mock.ANY, 'TestNetwork') + + @mock.patch.object(driver_base.LOG, 'warning', autospec=True) + @mock.patch.object(net_common.VIFPortIDMixin, 'vif_attach') + def test_vif_attach(self, mock_attach, mock_warn): + mock_task = mock.MagicMock() + network = TestNetwork() + network.vif_attach(mock_task, {'id': 'fake'}) + mock_attach.assert_called_once_with(mock_task, {'id': 'fake'}) + self.assertTrue(mock_warn.called) + + @mock.patch.object(driver_base.LOG, 'warning', autospec=True) + @mock.patch.object(net_common.VIFPortIDMixin, 'vif_detach') + def test_vif_detach(self, mock_detach, mock_warn): + mock_task = mock.MagicMock() + network = TestNetwork() + network.vif_detach(mock_task, 'fake-vif-id') + mock_detach.assert_called_once_with(mock_task, 'fake-vif-id') + self.assertTrue(mock_warn.called) + + @mock.patch.object(driver_base.LOG, 'warning', autospec=True) + @mock.patch.object(net_common.VIFPortIDMixin, 'port_changed') + def test_port_changed(self, mock_pc, mock_warn): + port = mock.MagicMock() + mock_task = mock.MagicMock() + network = TestNetwork() + network.port_changed(mock_task, port) + mock_pc.assert_called_once_with(mock_task, port) + self.assertTrue(mock_warn.called) + + @mock.patch.object(driver_base.LOG, 'warning', autospec=True) + @mock.patch.object(net_common.VIFPortIDMixin, 'portgroup_changed') + def test_portgroup_changed(self, mock_pgc, mock_warn): + port = mock.MagicMock() + mock_task = mock.MagicMock() + network = TestNetwork() + network.portgroup_changed(mock_task, port) + mock_pgc.assert_called_once_with(mock_task, port) + self.assertTrue(mock_warn.called) + + @mock.patch.object(driver_base.LOG, 'warning', autospec=True) + @mock.patch.object(net_common.VIFPortIDMixin, 'get_current_vif') + def test_get_current_vif(self, mock_gcv, mock_warn): + port = mock.MagicMock() + mock_task = mock.MagicMock() + network = TestNetwork() + network.get_current_vif(mock_task, port) + mock_gcv.assert_called_once_with(mock_task, port) + self.assertTrue(mock_warn.called) diff --git a/releasenotes/notes/deprecate-dhcp-update-mac-address-f12a4959432c8e20.yaml b/releasenotes/notes/deprecate-dhcp-update-mac-address-f12a4959432c8e20.yaml new file mode 100644 index 0000000000..32c39a9e98 --- /dev/null +++ b/releasenotes/notes/deprecate-dhcp-update-mac-address-f12a4959432c8e20.yaml @@ -0,0 +1,25 @@ +--- +features: + - | + Adds new methods to network interfaces, they will become + mandatory in Pike release: + + * ``vif_list`` - List attached VIF IDs for a node + * ``vif_attach`` - Attach a virtual network interface to a node + * ``vif_detach`` - Detach a virtual network interface from a node + * ``port_changed`` - Handle any actions required when a port + changes + * ``portgroup_changed`` - Handle any actions required when a + portgroup changes + * ``get_current_vif`` - Return VIF ID attached to port or portgroup + object. +deprecations: + - | + ``update_mac_address`` method in DHCP providers is + deprecated and will be removed in the Pike release. + The logic should be moved to a custom network + interface's ``port_changed`` and ``portgroup_changed``. +fixes: + - | + An issue when pre-created tenant port was automatically + deleted by Ironic on instance delete.