diff --git a/etc/ironic/ironic.conf.sample b/etc/ironic/ironic.conf.sample index 1ff5b09746..0f092b718f 100644 --- a/etc/ironic/ironic.conf.sample +++ b/etc/ironic/ironic.conf.sample @@ -30,6 +30,24 @@ # developer documentation online. (list value) #enabled_drivers = pxe_ipmitool +# Specify the list of network interfaces to load during +# service initialization. Missing network interfaces, or +# network interfaces which fail to initialize, will prevent +# the conductor service from starting. The option default is a +# recommended set of production-oriented network interfaces. A +# complete list of network interfaces present on your system +# may be found by enumerating the +# "ironic.hardware.interfaces.network" entrypoint. (list +# value) +#enabled_network_interfaces = flat,noop + +# Default network interface to be used for nodes that do not +# have network_interface field set. A complete list of network +# interfaces present on your system may be found by +# enumerating the "ironic.hardware.interfaces.network" +# entrypoint. (string value) +#default_network_interface = + # Used if there is a formatting error when generating an # exception message (a programming error). If True, raise an # exception; if False, use the unformatted message. (boolean @@ -1410,12 +1428,12 @@ # (list value) #hash_algorithms = md5 -# Authentication type to load (string value) +# Authentication type to load (unknown value) # Deprecated group/name - [keystone_authtoken]/auth_plugin #auth_type = # Config Section from which to load plugin specific options -# (string value) +# (unknown value) #auth_section = @@ -1499,8 +1517,11 @@ # Allowed values: keystone, noauth #auth_strategy = keystone -# UUID of the network to create Neutron ports on, when booting -# to a ramdisk for cleaning using Neutron DHCP. (string value) +# Neutron network UUID for the ramdisk to be booted into for +# cleaning nodes. Required if cleaning (either automatic or +# manual) is run for flat network interface, and, if DHCP +# providers are still being used, for neutron DHCP provider. +# (string value) #cleaning_network_uuid = diff --git a/ironic/common/driver_factory.py b/ironic/common/driver_factory.py index 6e834d4f71..bd4439f9e4 100644 --- a/ironic/common/driver_factory.py +++ b/ironic/common/driver_factory.py @@ -41,6 +41,23 @@ driver_opts = [ 'be found by enumerating the "ironic.drivers" ' 'entrypoint. An example may be found in the ' 'developer documentation online.')), + cfg.ListOpt('enabled_network_interfaces', + default=['flat', 'noop'], + help=_('Specify the list of network interfaces to load during ' + 'service initialization. Missing network interfaces, ' + 'or network interfaces which fail to initialize, will ' + 'prevent the conductor service from starting. The ' + 'option default is a recommended set of ' + 'production-oriented network interfaces. A complete ' + 'list of network interfaces present on your system may ' + 'be found by enumerating the ' + '"ironic.hardware.interfaces.network" entrypoint.')), + cfg.StrOpt('default_network_interface', + help=_('Default network interface to be used for nodes that ' + 'do not have network_interface field set. A complete ' + 'list of network interfaces present on your system may ' + 'be found by enumerating the ' + '"ironic.hardware.interfaces.network" entrypoint.')) ] CONF = cfg.CONF @@ -76,6 +93,20 @@ def _attach_interfaces_to_driver(driver, node, driver_name=None): impl = getattr(driver_singleton, iface, None) setattr(driver, iface, impl) + network_iface = node.network_interface + if network_iface is None: + network_iface = (CONF.default_network_interface or + ('flat' if CONF.dhcp.dhcp_provider == 'neutron' + else 'noop')) + network_factory = NetworkInterfaceFactory() + try: + net_driver = network_factory.get_driver(network_iface) + except KeyError: + raise exception.DriverNotFoundInEntrypoint( + driver_name=network_iface, + entrypoint=network_factory._entrypoint_name) + driver.network = net_driver + def get_driver(driver_name): """Simple method to get a ref to an instance of a driver. @@ -93,7 +124,7 @@ def get_driver(driver_name): try: factory = DriverFactory() - return factory[driver_name].obj + return factory.get_driver(driver_name) except KeyError: raise exception.DriverNotFound(driver_name=driver_name) @@ -109,8 +140,11 @@ def drivers(): for name in factory.names) -class DriverFactory(object): - """Discover, load and manage the drivers available.""" +class BaseDriverFactory(object): + """Discover, load and manage the drivers available. + + This is subclassed to load both main drivers and extra interfaces. + """ # NOTE(deva): loading the _extension_manager as a class member will break # stevedore when it loads a driver, because the driver will @@ -119,13 +153,25 @@ class DriverFactory(object): # once, the first time DriverFactory.__init__ is called. _extension_manager = None + # Entrypoint name containing the list of all available drivers/interfaces + _entrypoint_name = None + # Name of the [DEFAULT] section config option containing a list of enabled + # drivers/interfaces + _enabled_driver_list_config_option = '' + # This field will contain the list of the enabled drivers/interfaces names + # without duplicates + _enabled_driver_list = None + def __init__(self): - if not DriverFactory._extension_manager: - DriverFactory._init_extension_manager() + if not self.__class__._extension_manager: + self.__class__._init_extension_manager() def __getitem__(self, name): return self._extension_manager[name] + def get_driver(self, name): + return self[name].obj + # NOTE(deva): Use lockutils to avoid a potential race in eventlet # that might try to create two driver factories. @classmethod @@ -136,19 +182,24 @@ class DriverFactory(object): # creation of multiple NameDispatchExtensionManagers. if cls._extension_manager: return + enabled_drivers = getattr(CONF, cls._enabled_driver_list_config_option, + []) # Check for duplicated driver entries and warn the operator # about them - counter = collections.Counter(CONF.enabled_drivers).items() - duplicated_drivers = list(dup for (dup, i) in counter if i > 1) + counter = collections.Counter(enabled_drivers).items() + duplicated_drivers = [] + cls._enabled_driver_list = [] + for item, cnt in counter: + if cnt > 1: + duplicated_drivers.append(item) + cls._enabled_driver_list.append(item) if duplicated_drivers: LOG.warning(_LW('The driver(s) "%s" is/are duplicated in the ' 'list of enabled_drivers. Please check your ' 'configuration file.'), ', '.join(duplicated_drivers)) - enabled_drivers = set(CONF.enabled_drivers) - # NOTE(deva): Drivers raise "DriverLoadError" if they are unable to be # loaded, eg. due to missing external dependencies. # We capture that exception, and, only if it is for an @@ -160,30 +211,31 @@ class DriverFactory(object): def _catch_driver_not_found(mgr, ep, exc): # NOTE(deva): stevedore loads plugins *before* evaluating # _check_func, so we need to check here, too. - if ep.name in enabled_drivers: + if ep.name in cls._enabled_driver_list: if not isinstance(exc, exception.DriverLoadError): raise exception.DriverLoadError(driver=ep.name, reason=exc) raise exc def _check_func(ext): - return ext.name in enabled_drivers + return ext.name in cls._enabled_driver_list cls._extension_manager = ( dispatch.NameDispatchExtensionManager( - 'ironic.drivers', + cls._entrypoint_name, _check_func, invoke_on_load=True, on_load_failure_callback=_catch_driver_not_found)) # NOTE(deva): if we were unable to load any configured driver, perhaps # because it is not present on the system, raise an error. - if (sorted(enabled_drivers) != + if (sorted(cls._enabled_driver_list) != sorted(cls._extension_manager.names())): found = cls._extension_manager.names() - names = [n for n in enabled_drivers if n not in found] + names = [n for n in cls._enabled_driver_list if n not in found] # just in case more than one could not be found ... names = ', '.join(names) - raise exception.DriverNotFound(driver_name=names) + raise exception.DriverNotFoundInEntrypoint( + driver_name=names, entrypoint=cls._entrypoint_name) LOG.info(_LI("Loaded the following drivers: %s"), cls._extension_manager.names()) @@ -192,3 +244,13 @@ class DriverFactory(object): def names(self): """The list of driver names available.""" return self._extension_manager.names() + + +class DriverFactory(BaseDriverFactory): + _entrypoint_name = 'ironic.drivers' + _enabled_driver_list_config_option = 'enabled_drivers' + + +class NetworkInterfaceFactory(BaseDriverFactory): + _entrypoint_name = 'ironic.hardware.interfaces.network' + _enabled_driver_list_config_option = 'enabled_network_interfaces' diff --git a/ironic/common/exception.py b/ironic/common/exception.py index 139bbf997f..5622203b2d 100644 --- a/ironic/common/exception.py +++ b/ironic/common/exception.py @@ -241,6 +241,11 @@ class DriverNotFound(NotFound): _msg_fmt = _("Could not find the following driver(s): %(driver_name)s.") +class DriverNotFoundInEntrypoint(DriverNotFound): + _msg_fmt = _("Could not find the following driver(s) in the " + "'%(entrypoint)s' entrypoint: %(driver_name)s.") + + class ImageNotFound(NotFound): _msg_fmt = _("Image %(image_id)s could not be found.") @@ -591,3 +596,7 @@ class OneViewError(IronicException): class NodeTagNotFound(IronicException): _msg_fmt = _("Node %(node_id)s doesn't have a tag '%(tag)s'") + + +class NetworkError(IronicException): + _msg_fmt = _("Network operation failure.") diff --git a/ironic/common/neutron.py b/ironic/common/neutron.py index 99d73043e9..fc200207be 100644 --- a/ironic/common/neutron.py +++ b/ironic/common/neutron.py @@ -10,12 +10,20 @@ # License for the specific language governing permissions and limitations # under the License. +from neutronclient.common import exceptions as neutron_exceptions from neutronclient.v2_0 import client as clientv20 from oslo_config import cfg +from oslo_log import log +from ironic.common import exception from ironic.common.i18n import _ +from ironic.common.i18n import _LE +from ironic.common.i18n import _LI +from ironic.common.i18n import _LW from ironic.common import keystone +LOG = log.getLogger(__name__) + CONF = cfg.CONF CONF.import_opt('my_ip', 'ironic.netconf') @@ -42,8 +50,11 @@ neutron_opts = [ 'but not affected by this setting) is insecure and ' 'should only be used for testing.')), cfg.StrOpt('cleaning_network_uuid', - help=_('UUID of the network to create Neutron ports on, when ' - 'booting to a ramdisk for cleaning using Neutron DHCP.')) + help=_('Neutron network UUID for the ramdisk to be booted ' + 'into for cleaning nodes. Required if cleaning (either ' + 'automatic or manual) is run for flat network interface,' + ' and, if DHCP providers are still being used, for ' + 'neutron DHCP provider.')) ] CONF.register_opts(neutron_opts, group='neutron') @@ -73,3 +84,191 @@ def get_client(token=None): params['token'] = token return clientv20.Client(**params) + + +def add_ports_to_network(task, network_uuid, is_flat=False): + """Create neutron ports to boot the ramdisk. + + Create neutron ports for each pxe_enabled port on task.node to boot + the ramdisk. + + :param task: a TaskManager instance. + :param network_uuid: UUID of a neutron network where ports will be + created. + :param is_flat: Indicates whether it is a flat network or not. + :raises: NetworkError + :returns: a dictionary in the form {port.uuid: neutron_port['id']} + """ + client = get_client(task.context.auth_token) + node = task.node + + LOG.debug('For node %(node)s, creating neutron ports on network ' + '%(network_uuid)s using %(net_iface)s network interface.', + {'net_iface': task.driver.network.__class__.__name__, + 'node': node.uuid, 'network_uuid': network_uuid}) + body = { + 'port': { + 'network_id': network_uuid, + 'admin_state_up': True, + 'binding:vnic_type': 'baremetal', + 'device_owner': 'baremetal:none', + } + } + + if not is_flat: + # NOTE(vdrok): It seems that change + # I437290affd8eb87177d0626bf7935a165859cbdd to neutron broke the + # possibility to always bind port. Set binding:host_id only in + # case of non flat network. + body['port']['binding:host_id'] = node.uuid + + # Since instance_uuid will not be available during cleaning + # operations, we need to check that and populate them only when + # available + body['port']['device_id'] = node.instance_uuid or node.uuid + + ports = {} + failures = [] + portmap = get_node_portmap(task) + pxe_enabled_ports = [p for p in task.ports if p.pxe_enabled] + for ironic_port in pxe_enabled_ports: + body['port']['mac_address'] = ironic_port.address + binding_profile = {'local_link_information': + [portmap[ironic_port.uuid]]} + body['port']['binding:profile'] = binding_profile + try: + port = client.create_port(body) + except neutron_exceptions.NeutronClientException as e: + rollback_ports(task, network_uuid) + msg = (_('Could not create neutron port for ironic port ' + '%(ir-port)s on given network %(net)s from node ' + '%(node)s. %(exc)s') % + {'net': network_uuid, 'node': node.uuid, + 'ir-port': ironic_port.uuid, 'exc': e}) + LOG.exception(msg) + raise exception.NetworkError(msg) + + try: + ports[ironic_port.uuid] = port['port']['id'] + except KeyError: + failures.append(ironic_port.uuid) + + if failures: + if len(failures) == len(pxe_enabled_ports): + raise exception.NetworkError(_( + "Failed to update vif_port_id for any PXE enabled port " + "on node %s.") % node.uuid) + else: + LOG.warning(_LW("Some errors were encountered when updating " + "vif_port_id for node %(node)s on " + "the following ports: %(ports)s."), + {'node': node.uuid, 'ports': failures}) + else: + LOG.info(_LI('Successfully created ports for node %(node_uuid)s in ' + 'network %(net)s.'), + {'node_uuid': node.uuid, 'net': network_uuid}) + + return ports + + +def remove_ports_from_network(task, network_uuid): + """Deletes the neutron ports created for booting the ramdisk. + + :param task: a TaskManager instance. + :param network_uuid: UUID of a neutron network ports will be deleted from. + :raises: NetworkError + """ + macs = [p.address for p in task.ports if p.pxe_enabled] + if macs: + params = { + 'network_id': network_uuid, + 'mac_address': macs, + } + LOG.debug("Removing ports on network %(net)s on node %(node)s.", + {'net': network_uuid, 'node': task.node.uuid}) + + remove_neutron_ports(task, params) + + +def remove_neutron_ports(task, params): + """Deletes the neutron ports matched by params. + + :param task: a TaskManager instance. + :param params: Dict of params to filter ports. + :raises: NetworkError + """ + client = get_client(task.context.auth_token) + node_uuid = task.node.uuid + + try: + response = client.list_ports(**params) + except neutron_exceptions.NeutronClientException as e: + msg = (_('Could not get given network VIF for %(node)s ' + 'from neutron, possible network issue. %(exc)s') % + {'node': node_uuid, 'exc': e}) + LOG.exception(msg) + raise exception.NetworkError(msg) + + ports = response.get('ports', []) + if not ports: + LOG.debug('No ports to remove for node %s', node_uuid) + return + + for port in ports: + if not port['id']: + # TODO(morgabra) client.list_ports() sometimes returns + # port objects with null ids. It's unclear why this happens. + LOG.warning(_LW("Deleting neutron port failed, missing 'id'. " + "Node: %(node)s, neutron port: %(port)s."), + {'node': node_uuid, 'port': port}) + continue + + LOG.debug('Deleting neutron port %(vif_port_id)s of node ' + '%(node_id)s.', + {'vif_port_id': port['id'], 'node_id': node_uuid}) + + try: + client.delete_port(port['id']) + except neutron_exceptions.NeutronClientException as e: + msg = (_('Could not remove VIF %(vif)s of node %(node)s, possibly ' + 'a network issue: %(exc)s') % + {'vif': port['id'], 'node': node_uuid, 'exc': e}) + LOG.exception(msg) + raise exception.NetworkError(msg) + + LOG.info(_LI('Successfully removed node %(node_uuid)s neutron ports.'), + {'node_uuid': node_uuid}) + + +def get_node_portmap(task): + """Extract the switch port information for the node. + + :param task: a task containing the Node object. + :returns: a dictionary in the form {port.uuid: port.local_link_connection} + """ + + portmap = {} + for port in task.ports: + portmap[port.uuid] = port.local_link_connection + return portmap + # TODO(jroll) raise InvalidParameterValue if a port doesn't have the + # necessary info? (probably) + + +def rollback_ports(task, network_uuid): + """Attempts to delete any ports created by cleaning/provisioning + + Purposefully will not raise any exceptions so error handling can + continue. + + :param task: a TaskManager instance. + :param network_uuid: UUID of a neutron network. + """ + try: + remove_ports_from_network(task, network_uuid) + except exception.NetworkError: + # Only log the error + LOG.exception(_LE( + 'Failed to rollback port changes for node %(node)s ' + 'on network %(network)s'), {'node': task.node.uuid, + 'network': network_uuid}) diff --git a/ironic/conductor/base_manager.py b/ironic/conductor/base_manager.py index da5db5a1fb..d1c2b1261b 100644 --- a/ironic/conductor/base_manager.py +++ b/ironic/conductor/base_manager.py @@ -83,8 +83,12 @@ class BaseConductorManager(object): self.ring_manager = hash.HashRingManager() """Consistent hash ring which maps drivers to conductors.""" - # NOTE(deva): this call may raise DriverLoadError or DriverNotFound + # NOTE(deva): these calls may raise DriverLoadError or DriverNotFound + # NOTE(vdrok): instantiate network interface factory on startup so that + # all the network interfaces are loaded at the very beginning, and + # failures prevent the conductor from starting. drivers = driver_factory.drivers() + driver_factory.NetworkInterfaceFactory() if not drivers: msg = _LE("Conductor %s cannot be started because no drivers " "were loaded. This could be because no drivers were " diff --git a/ironic/dhcp/neutron.py b/ironic/dhcp/neutron.py index 15935a491b..084300d1a7 100644 --- a/ironic/dhcp/neutron.py +++ b/ironic/dhcp/neutron.py @@ -34,6 +34,9 @@ from ironic import objects CONF = cfg.CONF LOG = logging.getLogger(__name__) +create_cleaning_ports_deprecation = False +delete_cleaning_ports_deprecation = False + class NeutronDHCPApi(base.BaseDHCP): """API for communicating to neutron 2.x API.""" @@ -271,95 +274,37 @@ class NeutronDHCPApi(base.BaseDHCP): return port_ip_addresses + portgroup_ip_addresses + # TODO(vsaienko) Remove this method when deprecation period is passed + # in Ocata. def create_cleaning_ports(self, task): """Create neutron ports for each port on task.node to boot the ramdisk. :param task: a TaskManager instance. - :raises: InvalidParameterValue if the cleaning network is None + :raises: NetworkError, InvalidParameterValue :returns: a dictionary in the form {port.uuid: neutron_port['id']} """ - if not CONF.neutron.cleaning_network_uuid: - raise exception.InvalidParameterValue(_('Valid cleaning network ' - 'UUID not provided')) - neutron_client = neutron.get_client(task.context.auth_token) - body = { - 'port': { - 'network_id': CONF.neutron.cleaning_network_uuid, - 'admin_state_up': True, - } - } - ports = {} - for ironic_port in task.ports: - body['port']['mac_address'] = ironic_port.address - try: - port = neutron_client.create_port(body) - except neutron_client_exc.ConnectionFailed as e: - self._rollback_cleaning_ports(task) - msg = (_('Could not create cleaning port on network %(net)s ' - 'from %(node)s. %(exc)s') % - {'net': CONF.neutron.cleaning_network_uuid, - 'node': task.node.uuid, - 'exc': e}) - LOG.exception(msg) - raise exception.NodeCleaningFailure(msg) - if not port.get('port') or not port['port'].get('id'): - self._rollback_cleaning_ports(task) - msg = (_('Failed to create cleaning ports for node ' - '%(node)s') % {'node': task.node.uuid}) - LOG.error(msg) - raise exception.NodeCleaningFailure(msg) - # Match return value of get_node_vif_ids() - ports[ironic_port.uuid] = port['port']['id'] - return ports + global create_cleaning_ports_deprecation + if not create_cleaning_ports_deprecation: + LOG.warning(_LW('create_cleaning_ports via dhcp provider is ' + 'deprecated. The node.network_interface setting ' + 'should be used instead.')) + create_cleaning_ports_deprecation = True + return task.driver.network.add_cleaning_network(task) + + # TODO(vsaienko) Remove this method when deprecation period is passed + # in Ocata. def delete_cleaning_ports(self, task): """Deletes the neutron port created for booting the ramdisk. :param task: a TaskManager instance. + :raises: NetworkError, InvalidParameterValue """ - neutron_client = neutron.get_client(task.context.auth_token) - macs = [p.address for p in task.ports] - params = { - 'network_id': CONF.neutron.cleaning_network_uuid - } - try: - ports = neutron_client.list_ports(**params) - except neutron_client_exc.ConnectionFailed as e: - msg = (_('Could not get cleaning network vif for %(node)s ' - 'from Neutron, possible network issue. %(exc)s') % - {'node': task.node.uuid, - 'exc': e}) - LOG.exception(msg) - raise exception.NodeCleaningFailure(msg) + global delete_cleaning_ports_deprecation + if not delete_cleaning_ports_deprecation: + LOG.warning(_LW('delete_cleaning_ports via dhcp provider is ' + 'deprecated. The node.network_interface setting ' + 'should be used instead.')) + delete_cleaning_ports_deprecation = True - # Iterate the list of Neutron port dicts, remove the ones we added - for neutron_port in ports.get('ports', []): - # Only delete ports using the node's mac addresses - if neutron_port.get('mac_address') in macs: - try: - neutron_client.delete_port(neutron_port.get('id')) - except neutron_client_exc.ConnectionFailed as e: - msg = (_('Could not remove cleaning ports on network ' - '%(net)s from %(node)s, possible network issue. ' - '%(exc)s') % - {'net': CONF.neutron.cleaning_network_uuid, - 'node': task.node.uuid, - 'exc': e}) - LOG.exception(msg) - raise exception.NodeCleaningFailure(msg) - - def _rollback_cleaning_ports(self, task): - """Attempts to delete any ports created by cleaning - - Purposefully will not raise any exceptions so error handling can - continue. - - :param task: a TaskManager instance. - """ - try: - self.delete_cleaning_ports(task) - except Exception: - # Log the error, but let the caller invoke the - # manager.cleaning_error_handler(). - LOG.exception(_LE('Failed to rollback cleaning port ' - 'changes for node %s') % task.node.uuid) + task.driver.network.remove_cleaning_network(task) diff --git a/ironic/drivers/base.py b/ironic/drivers/base.py index 962048c23a..742aa5a129 100644 --- a/ironic/drivers/base.py +++ b/ironic/drivers/base.py @@ -158,8 +158,14 @@ class BareDriver(BaseDriver): Any composable interfaces should be added as class attributes of this class, as well as appended to core_interfaces or standard_interfaces here. """ + def __init__(self): - pass + self.network = None + """`Core` attribute for network connectivity. + + A reference to an instance of :class:NetworkInterface. + """ + self.core_interfaces.append('network') class BaseInterface(object): @@ -1020,6 +1026,74 @@ class RAIDInterface(BaseInterface): return raid.get_logical_disk_properties(self.raid_schema) +@six.add_metaclass(abc.ABCMeta) +class NetworkInterface(object): + """Base class for network interfaces.""" + + def get_properties(self): + """Return the properties of the interface. + + :returns: dictionary of : entries. + """ + return {} + + def validate(self, task): + """Validates the network interface. + + :param task: a TaskManager instance. + :raises: InvalidParameterValue, if the network interface configuration + is invalid. + :raises: MissingParameterValue, if some parameters are missing. + """ + + @abc.abstractmethod + def add_provisioning_network(self, task): + """Add the provisioning network to a node. + + :param task: A TaskManager instance. + :raises: NetworkError + """ + + @abc.abstractmethod + def remove_provisioning_network(self, task): + """Remove the provisioning network from a node. + + :param task: A TaskManager instance. + """ + + @abc.abstractmethod + def configure_tenant_networks(self, task): + """Configure tenant networks for a node. + + :param task: A TaskManager instance. + :raises: NetworkError + """ + + @abc.abstractmethod + def unconfigure_tenant_networks(self, task): + """Unconfigure tenant networks for a node. + + :param task: A TaskManager instance. + """ + + @abc.abstractmethod + def add_cleaning_network(self, task): + """Add the cleaning network to a node. + + :param task: A TaskManager instance. + :returns: a dictionary in the form {port.uuid: neutron_port['id']} + :raises: NetworkError + """ + + @abc.abstractmethod + def remove_cleaning_network(self, task): + """Remove the cleaning network from a node. + + :param task: A TaskManager instance. + :raises: NetworkError + """ + + def _validate_argsinfo(argsinfo): """Validate args info. diff --git a/ironic/drivers/modules/agent.py b/ironic/drivers/modules/agent.py index 971bd63ed8..79b19d5bd6 100644 --- a/ironic/drivers/modules/agent.py +++ b/ironic/drivers/modules/agent.py @@ -380,8 +380,10 @@ class AgentDeploy(base.DeployInterface): """Boot into the agent to prepare for cleaning. :param task: a TaskManager object containing the node - :raises NodeCleaningFailure: if the previous cleaning ports cannot - be removed or if new cleaning ports cannot be created + :raises: NodeCleaningFailure, NetworkError if the previous cleaning + ports cannot be removed or if new cleaning ports cannot be created. + :raises: InvalidParameterValue if cleaning network UUID config option + has an invalid value. :returns: states.CLEANWAIT to signify an asynchronous prepare """ return deploy_utils.prepare_inband_cleaning( @@ -391,8 +393,8 @@ class AgentDeploy(base.DeployInterface): """Clean up the PXE and DHCP files after cleaning. :param task: a TaskManager object containing the node - :raises NodeCleaningFailure: if the cleaning ports cannot be - removed + :raises: NodeCleaningFailure, NetworkError if the cleaning ports cannot + be removed """ deploy_utils.tear_down_inband_cleaning( task, manage_boot=CONF.agent.manage_agent_boot) diff --git a/ironic/drivers/modules/deploy_utils.py b/ironic/drivers/modules/deploy_utils.py index b37bec11a9..2a23b39d4e 100644 --- a/ironic/drivers/modules/deploy_utils.py +++ b/ironic/drivers/modules/deploy_utils.py @@ -910,6 +910,8 @@ def get_boot_option(node): return capabilities.get('boot_option', 'netboot').lower() +# TODO(vdrok): This method is left here for backwards compatibility with out of +# tree DHCP providers implementing cleaning methods. Remove it in Ocata def prepare_cleaning_ports(task): """Prepare the Ironic ports of the node for cleaning. @@ -919,17 +921,39 @@ def prepare_cleaning_ports(task): of each Ironic port, after creating the cleaning ports. :param task: a TaskManager object containing the node - :raises NodeCleaningFailure: if the previous cleaning ports cannot - be removed or if new cleaning ports cannot be created + :raises: NodeCleaningFailure, NetworkError if the previous cleaning ports + cannot be removed or if new cleaning ports cannot be created. + :raises: InvalidParameterValue if cleaning network UUID config option has + an invalid value. """ provider = dhcp_factory.DHCPFactory() + provider_manages_delete_cleaning = hasattr(provider.provider, + 'delete_cleaning_ports') + provider_manages_create_cleaning = hasattr(provider.provider, + 'create_cleaning_ports') + # NOTE(vdrok): The neutron DHCP provider was changed to call network + # interface's add_cleaning_network anyway, so call it directly to avoid + # duplication of some actions + if (CONF.dhcp.dhcp_provider == 'neutron' or + (not provider_manages_delete_cleaning and + not provider_manages_create_cleaning)): + task.driver.network.add_cleaning_network(task) + return + + LOG.warning(_LW("delete_cleaning_ports and create_cleaning_ports " + "functions in DHCP providers are deprecated, please move " + "this logic to the network interface's " + "remove_cleaning_network or add_cleaning_network methods " + "respectively and remove the old DHCP provider methods. " + "Possibility to do the cleaning via DHCP providers will " + "be removed in Ocata release.")) # If we have left over ports from a previous cleaning, remove them - if getattr(provider.provider, 'delete_cleaning_ports', None): + if provider_manages_delete_cleaning: # Allow to raise if it fails, is caught and handled in conductor provider.provider.delete_cleaning_ports(task) # Create cleaning ports if necessary - if getattr(provider.provider, 'create_cleaning_ports', None): + if provider_manages_create_cleaning: # Allow to raise if it fails, is caught and handled in conductor ports = provider.provider.create_cleaning_ports(task) @@ -953,6 +977,8 @@ def prepare_cleaning_ports(task): port.save() +# TODO(vdrok): This method is left here for backwards compatibility with out of +# tree DHCP providers implementing cleaning methods. Remove it in Ocata def tear_down_cleaning_ports(task): """Deletes the cleaning ports created for each of the Ironic ports. @@ -960,22 +986,36 @@ def tear_down_cleaning_ports(task): was started. :param task: a TaskManager object containing the node - :raises NodeCleaningFailure: if the cleaning ports cannot be + :raises: NodeCleaningFailure, NetworkError if the cleaning ports cannot be removed. """ # If we created cleaning ports, delete them provider = dhcp_factory.DHCPFactory() - if getattr(provider.provider, 'delete_cleaning_ports', None): + provider_manages_delete_cleaning = hasattr(provider.provider, + 'delete_cleaning_ports') + try: + # NOTE(vdrok): The neutron DHCP provider was changed to call network + # interface's remove_cleaning_network anyway, so call it directly to + # avoid duplication of some actions + if (CONF.dhcp.dhcp_provider == 'neutron' or + not provider_manages_delete_cleaning): + task.driver.network.remove_cleaning_network(task) + return + + # NOTE(vdrok): No need for another deprecation warning here, if + # delete_cleaning_ports is in the DHCP provider the warning was + # printed in prepare_cleaning_ports # Allow to raise if it fails, is caught and handled in conductor provider.provider.delete_cleaning_ports(task) - for port in task.ports: if 'cleaning_vif_port_id' in port.internal_info: internal_info = port.internal_info del internal_info['cleaning_vif_port_id'] port.internal_info = internal_info port.save() - elif 'vif_port_id' in port.extra: + finally: + for port in task.ports: + if 'vif_port_id' in port.extra: # TODO(vdrok): This piece is left for backwards compatibility, # if ironic was upgraded during cleaning, vif_port_id # containing cleaning neutron port UUID should be cleared, @@ -1028,8 +1068,10 @@ def prepare_inband_cleaning(task, manage_boot=True): automatically boot agent ramdisk every time bare metal node is rebooted. :returns: states.CLEANWAIT to signify an asynchronous prepare. - :raises NodeCleaningFailure: if the previous cleaning ports cannot - be removed or if new cleaning ports cannot be created + :raises: NetworkError, NodeCleaningFailure if the previous cleaning ports + cannot be removed or if new cleaning ports cannot be created. + :raises: InvalidParameterValue if cleaning network UUID config option has + an invalid value. """ prepare_cleaning_ports(task) @@ -1062,7 +1104,7 @@ def tear_down_inband_cleaning(task, manage_boot=True): :param manage_boot: If this is set to True, this method calls the 'clean_up_ramdisk' method of boot interface to boot the agent ramdisk. If False, it skips this step. - :raises NodeCleaningFailure: if the cleaning ports cannot be + :raises: NetworkError, NodeCleaningFailure if the cleaning ports cannot be removed. """ manager_utils.node_power_action(task, states.POWER_OFF) diff --git a/ironic/drivers/modules/ilo/deploy.py b/ironic/drivers/modules/ilo/deploy.py index a285ed4e1b..9dc57f262a 100644 --- a/ironic/drivers/modules/ilo/deploy.py +++ b/ironic/drivers/modules/ilo/deploy.py @@ -274,8 +274,8 @@ class IloVirtualMediaAgentDeploy(agent.AgentDeploy): :param task: a TaskManager object containing the node :returns: states.CLEANWAIT to signify an asynchronous prepare. - :raises NodeCleaningFailure: if the previous cleaning ports cannot - be removed or if new cleaning ports cannot be created + :raises: NodeCleaningFailure, NetworkError if the previous cleaning + ports cannot be removed or if new cleaning ports cannot be created :raises: IloOperationError, if some operation on iLO failed. """ # Powering off the Node before initiating boot for node cleaning. diff --git a/ironic/drivers/modules/network/__init__.py b/ironic/drivers/modules/network/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/ironic/drivers/modules/network/flat.py b/ironic/drivers/modules/network/flat.py new file mode 100644 index 0000000000..591c4e8dfe --- /dev/null +++ b/ironic/drivers/modules/network/flat.py @@ -0,0 +1,121 @@ +# 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. + +""" +Flat network interface. Useful for shared, flat networks. +""" + +from oslo_config import cfg +from oslo_log import log +from oslo_utils import uuidutils + +from ironic.common import exception +from ironic.common.i18n import _ +from ironic.common.i18n import _LI +from ironic.common.i18n import _LW +from ironic.common import neutron +from ironic.drivers import base + + +LOG = log.getLogger(__name__) + +CONF = cfg.CONF + + +class FlatNetwork(base.NetworkInterface): + """Flat network interface.""" + + def __init__(self): + cleaning_net = CONF.neutron.cleaning_network_uuid + # TODO(vdrok): Switch to DriverLoadError in Ocata + if not uuidutils.is_uuid_like(cleaning_net): + LOG.warning(_LW( + 'Please specify a valid UUID for ' + '[neutron]/cleaning_network_uuid configuration option so that ' + 'this interface is able to perform cleaning. It will be ' + 'required starting with the Ocata release, and if not ' + 'specified then, the conductor service will fail to start if ' + '"flat" is in the list of values for ' + '[DEFAULT]enabled_network_interfaces configuration option.')) + + def add_provisioning_network(self, task): + """Add the provisioning network to a node. + + :param task: A TaskManager instance. + """ + pass + + def remove_provisioning_network(self, task): + """Remove the provisioning network from a node. + + :param task: A TaskManager instance. + """ + pass + + def configure_tenant_networks(self, task): + """Configure tenant networks for a node. + + :param task: A TaskManager instance. + """ + pass + + def unconfigure_tenant_networks(self, task): + """Unconfigure tenant networks for a node. + + :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() + + def add_cleaning_network(self, task): + """Add the cleaning network to a node. + + :param task: A TaskManager instance. + :returns: a dictionary in the form {port.uuid: neutron_port['id']} + :raises: NetworkError, InvalidParameterValue + """ + if not uuidutils.is_uuid_like(CONF.neutron.cleaning_network_uuid): + raise exception.InvalidParameterValue(_( + 'You must provide a valid cleaning network UUID in ' + '[neutron]cleaning_network_uuid configuration option.')) + # If we have left over ports from a previous cleaning, remove them + neutron.rollback_ports(task, CONF.neutron.cleaning_network_uuid) + LOG.info(_LI('Adding cleaning network to node %s'), task.node.uuid) + vifs = neutron.add_ports_to_network( + task, CONF.neutron.cleaning_network_uuid, is_flat=True) + for port in task.ports: + if port.uuid in vifs: + internal_info = port.internal_info + internal_info['cleaning_vif_port_id'] = vifs[port.uuid] + port.internal_info = internal_info + port.save() + return vifs + + def remove_cleaning_network(self, task): + """Remove the cleaning network from a node. + + :param task: A TaskManager instance. + :raises: NetworkError + """ + LOG.info(_LI('Removing ports from cleaning network for node %s'), + task.node.uuid) + neutron.remove_ports_from_network( + task, CONF.neutron.cleaning_network_uuid) + for port in task.ports: + if 'cleaning_vif_port_id' in port.internal_info: + internal_info = port.internal_info + del internal_info['cleaning_vif_port_id'] + port.internal_info = internal_info + port.save() diff --git a/ironic/drivers/modules/network/noop.py b/ironic/drivers/modules/network/noop.py new file mode 100644 index 0000000000..f4c530022d --- /dev/null +++ b/ironic/drivers/modules/network/noop.py @@ -0,0 +1,59 @@ +# 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 ironic.drivers import base + + +class NoopNetwork(base.NetworkInterface): + """Noop network interface.""" + + def add_provisioning_network(self, task): + """Add the provisioning network to a node. + + :param task: A TaskManager instance. + """ + pass + + def remove_provisioning_network(self, task): + """Remove the provisioning network from a node. + + :param task: A TaskManager instance. + """ + pass + + def configure_tenant_networks(self, task): + """Configure tenant networks for a node. + + :param task: A TaskManager instance. + """ + pass + + def unconfigure_tenant_networks(self, task): + """Unconfigure tenant networks for a node. + + :param task: A TaskManager instance. + """ + pass + + def add_cleaning_network(self, task): + """Add the cleaning network to a node. + + :param task: A TaskManager instance. + """ + pass + + def remove_cleaning_network(self, task): + """Remove the cleaning network from a node. + + :param task: A TaskManager instance. + """ + pass diff --git a/ironic/tests/base.py b/ironic/tests/base.py index dd58349392..b4b7b71759 100644 --- a/ironic/tests/base.py +++ b/ironic/tests/base.py @@ -32,6 +32,7 @@ import fixtures from oslo_config import cfg from oslo_config import fixture as config_fixture from oslo_log import log as logging +from oslo_utils import uuidutils import testtools from ironic.common import config as ironic_config @@ -43,6 +44,7 @@ from ironic.tests.unit import policy_fixture CONF = cfg.CONF CONF.import_opt('host', 'ironic.common.service') +CONF.import_opt('cleaning_network_uuid', 'ironic.common.neutron', 'neutron') logging.register_options(CONF) logging.setup(CONF, 'ironic') @@ -115,6 +117,9 @@ class TestCase(testtools.TestCase): self.config(use_stderr=False, fatal_exception_format_errors=True, tempdir=tempfile.tempdir) + self.config(cleaning_network_uuid=uuidutils.generate_uuid(), + group='neutron') + self.config(enabled_network_interfaces=['flat', 'noop']) self.set_defaults(host='fake-mini', debug=True) self.set_defaults(connection="sqlite://", diff --git a/ironic/tests/unit/common/test_driver_factory.py b/ironic/tests/unit/common/test_driver_factory.py index e42fe15410..a8b286140f 100644 --- a/ironic/tests/unit/common/test_driver_factory.py +++ b/ironic/tests/unit/common/test_driver_factory.py @@ -17,8 +17,11 @@ from stevedore import dispatch from ironic.common import driver_factory from ironic.common import exception +from ironic.conductor import task_manager from ironic.drivers import base as drivers_base from ironic.tests import base +from ironic.tests.unit.db import base as db_base +from ironic.tests.unit.objects import utils as obj_utils class FakeEp(object): @@ -86,3 +89,76 @@ class GetDriverTestCase(base.TestCase): def test_get_driver_unknown(self): self.assertRaises(exception.DriverNotFound, driver_factory.get_driver, 'unknown_driver') + + +class NetworkInterfaceFactoryTestCase(db_base.DbTestCase): + def setUp(self): + super(NetworkInterfaceFactoryTestCase, self).setUp() + driver_factory.DriverFactory._extension_manager = None + driver_factory.NetworkInterfaceFactory._extension_manager = None + self.config(enabled_drivers=['fake']) + + def test_build_driver_for_task(self): + # flat and noop network interfaces are enabled in base test case + factory = driver_factory.NetworkInterfaceFactory + node = obj_utils.create_test_node(self.context, driver='fake', + network_interface='flat') + with task_manager.acquire(self.context, node.id) as task: + extension_mgr = factory._extension_manager + self.assertIn('flat', extension_mgr) + self.assertIn('noop', extension_mgr) + self.assertEqual(extension_mgr['flat'].obj, task.driver.network) + self.assertEqual('ironic.hardware.interfaces.network', + factory._entrypoint_name) + self.assertEqual(['flat', 'noop'], + sorted(factory._enabled_driver_list)) + + def test_build_driver_for_task_default_is_none(self): + # flat and noop network interfaces are enabled in base test case + factory = driver_factory.NetworkInterfaceFactory + self.config(dhcp_provider='none', group='dhcp') + node = obj_utils.create_test_node(self.context, driver='fake') + with task_manager.acquire(self.context, node.id) as task: + extension_mgr = factory._extension_manager + self.assertIn('flat', extension_mgr) + self.assertIn('noop', extension_mgr) + self.assertEqual(extension_mgr['noop'].obj, task.driver.network) + + def test_build_driver_for_task_default_network_interface_is_set(self): + # flat and noop network interfaces are enabled in base test case + factory = driver_factory.NetworkInterfaceFactory + self.config(dhcp_provider='none', group='dhcp') + self.config(default_network_interface='flat') + node = obj_utils.create_test_node(self.context, driver='fake') + with task_manager.acquire(self.context, node.id) as task: + extension_mgr = factory._extension_manager + self.assertIn('flat', extension_mgr) + self.assertIn('noop', extension_mgr) + self.assertEqual(extension_mgr['flat'].obj, task.driver.network) + + def test_build_driver_for_task_default_is_flat(self): + # flat and noop network interfaces are enabled in base test case + factory = driver_factory.NetworkInterfaceFactory + node = obj_utils.create_test_node(self.context, driver='fake') + with task_manager.acquire(self.context, node.id) as task: + extension_mgr = factory._extension_manager + self.assertIn('flat', extension_mgr) + self.assertIn('noop', extension_mgr) + self.assertEqual(extension_mgr['flat'].obj, task.driver.network) + + def test_build_driver_for_task_unknown_network_interface(self): + node = obj_utils.create_test_node(self.context, driver='fake', + network_interface='meow') + self.assertRaises(exception.DriverNotFoundInEntrypoint, + task_manager.acquire, self.context, node.id) + + +class NewDriverFactory(driver_factory.BaseDriverFactory): + _entrypoint_name = 'woof' + + +class NewFactoryTestCase(db_base.DbTestCase): + def test_new_driver_factory_unknown_entrypoint(self): + factory = NewDriverFactory() + self.assertEqual('woof', factory._entrypoint_name) + self.assertEqual([], factory._enabled_driver_list) diff --git a/ironic/tests/unit/common/test_neutron.py b/ironic/tests/unit/common/test_neutron.py index c55c115a3a..0076d741ae 100644 --- a/ironic/tests/unit/common/test_neutron.py +++ b/ironic/tests/unit/common/test_neutron.py @@ -11,11 +11,18 @@ # under the License. import mock +from neutronclient.common import exceptions as neutron_client_exc from neutronclient.v2_0 import client 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.tests import base +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 object_utils class TestNeutronClient(base.TestCase): @@ -107,3 +114,249 @@ class TestNeutronClient(base.TestCase): self.assertRaises(ValueError, cfg.CONF.set_override, 'auth_strategy', 'fake', 'neutron', enforce_type=True) + + +class TestNeutronNetworkActions(db_base.DbTestCase): + + def setUp(self): + super(TestNeutronNetworkActions, self).setUp() + mgr_utils.mock_the_extension_manager(driver='fake') + self.config(enabled_drivers=['fake']) + self.node = object_utils.create_test_node(self.context) + self.ports = [object_utils.create_test_port( + self.context, node_id=self.node.id, + uuid='1be26c0b-03f2-4d2e-ae87-c02d7f33c782', + address='52:54:00:cf:2d:32', + extra={'vif_port_id': uuidutils.generate_uuid()} + )] + # Very simple neutron port representation + self.neutron_port = {'id': '132f871f-eaec-4fed-9475-0d54465e0f00', + 'mac_address': '52:54:00:cf:2d:32'} + self.network_uuid = uuidutils.generate_uuid() + + @mock.patch.object(client.Client, 'create_port') + def test_add_ports_to_vlan_network(self, create_mock): + # Ports will be created only if pxe_enabled is True + object_utils.create_test_port( + self.context, node_id=self.node.id, + uuid=uuidutils.generate_uuid(), + address='52:54:00:cf:2d:22', + pxe_enabled=False + ) + port = self.ports[0] + expected_body = { + 'port': { + 'network_id': self.network_uuid, + 'admin_state_up': True, + 'binding:vnic_type': 'baremetal', + 'device_owner': 'baremetal:none', + 'binding:host_id': self.node.uuid, + 'device_id': self.node.uuid, + 'mac_address': port.address, + 'binding:profile': { + 'local_link_information': [port.local_link_connection] + } + } + } + # Ensure we can create ports + create_mock.return_value = {'port': self.neutron_port} + expected = {port.uuid: self.neutron_port['id']} + with task_manager.acquire(self.context, self.node.uuid) as task: + ports = neutron.add_ports_to_network(task, self.network_uuid) + self.assertEqual(expected, ports) + create_mock.assert_called_once_with(expected_body) + + @mock.patch.object(client.Client, 'create_port') + def test_add_ports_to_flat_network(self, create_mock): + port = self.ports[0] + expected_body = { + 'port': { + 'network_id': self.network_uuid, + 'admin_state_up': True, + 'binding:vnic_type': 'baremetal', + 'device_owner': 'baremetal:none', + 'device_id': self.node.uuid, + 'mac_address': port.address, + 'binding:profile': { + 'local_link_information': [port.local_link_connection] + } + } + } + # Ensure we can create ports + create_mock.return_value = {'port': self.neutron_port} + expected = {port.uuid: self.neutron_port['id']} + with task_manager.acquire(self.context, self.node.uuid) as task: + ports = neutron.add_ports_to_network(task, self.network_uuid, + is_flat=True) + self.assertEqual(expected, ports) + create_mock.assert_called_once_with(expected_body) + + @mock.patch.object(client.Client, 'create_port') + def test_add_ports_to_flat_network_no_neutron_port_id(self, create_mock): + port = self.ports[0] + expected_body = { + 'port': { + 'network_id': self.network_uuid, + 'admin_state_up': True, + 'binding:vnic_type': 'baremetal', + 'device_owner': 'baremetal:none', + 'device_id': self.node.uuid, + 'mac_address': port.address, + 'binding:profile': { + 'local_link_information': [port.local_link_connection] + } + } + } + del self.neutron_port['id'] + create_mock.return_value = {'port': self.neutron_port} + with task_manager.acquire(self.context, self.node.uuid) as task: + self.assertRaises(exception.NetworkError, + neutron.add_ports_to_network, + task, self.network_uuid, is_flat=True) + create_mock.assert_called_once_with(expected_body) + + @mock.patch.object(client.Client, 'create_port') + def test_add_ports_to_vlan_network_instance_uuid(self, create_mock): + self.node.instance_uuid = uuidutils.generate_uuid() + self.node.save() + port = self.ports[0] + expected_body = { + 'port': { + 'network_id': self.network_uuid, + 'admin_state_up': True, + 'binding:vnic_type': 'baremetal', + 'device_owner': 'baremetal:none', + 'binding:host_id': self.node.uuid, + 'device_id': self.node.instance_uuid, + 'mac_address': port.address, + 'binding:profile': { + 'local_link_information': [port.local_link_connection] + } + } + } + # Ensure we can create ports + create_mock.return_value = {'port': self.neutron_port} + expected = {port.uuid: self.neutron_port['id']} + with task_manager.acquire(self.context, self.node.uuid) as task: + ports = neutron.add_ports_to_network(task, self.network_uuid) + self.assertEqual(expected, ports) + create_mock.assert_called_once_with(expected_body) + + @mock.patch.object(neutron, 'rollback_ports') + @mock.patch.object(client.Client, 'create_port') + def test_add_network_fail(self, create_mock, rollback_mock): + # Check that if creating a port fails, the ports are cleaned up + create_mock.side_effect = neutron_client_exc.ConnectionFailed + + with task_manager.acquire(self.context, self.node.uuid) as task: + self.assertRaisesRegex( + exception.NetworkError, 'Could not create neutron port', + neutron.add_ports_to_network, task, self.network_uuid) + rollback_mock.assert_called_once_with(task, self.network_uuid) + + @mock.patch.object(neutron, 'rollback_ports') + @mock.patch.object(client.Client, 'create_port', return_value={}) + def test_add_network_fail_create_any_port_empty(self, create_mock, + rollback_mock): + with task_manager.acquire(self.context, self.node.uuid) as task: + self.assertRaisesRegex( + exception.NetworkError, 'any PXE enabled port', + neutron.add_ports_to_network, task, self.network_uuid) + self.assertFalse(rollback_mock.called) + + @mock.patch.object(neutron, 'LOG') + @mock.patch.object(neutron, 'rollback_ports') + @mock.patch.object(client.Client, 'create_port') + def test_add_network_fail_create_some_ports_empty(self, create_mock, + rollback_mock, log_mock): + port2 = object_utils.create_test_port( + self.context, node_id=self.node.id, + uuid=uuidutils.generate_uuid(), + address='52:54:55:cf:2d:32', + extra={'vif_port_id': uuidutils.generate_uuid()} + ) + create_mock.side_effect = [{'port': self.neutron_port}, {}] + with task_manager.acquire(self.context, self.node.uuid) as task: + neutron.add_ports_to_network(task, self.network_uuid) + self.assertIn(str(port2.uuid), + # Call #0, argument #1 + log_mock.warning.call_args[0][1]['ports']) + self.assertFalse(rollback_mock.called) + + @mock.patch.object(neutron, 'remove_neutron_ports') + def test_remove_ports_from_network(self, remove_mock): + with task_manager.acquire(self.context, self.node.uuid) as task: + neutron.remove_ports_from_network(task, self.network_uuid) + remove_mock.assert_called_once_with( + task, + {'network_id': self.network_uuid, + 'mac_address': [self.ports[0].address]} + ) + + @mock.patch.object(neutron, 'remove_neutron_ports') + def test_remove_ports_from_network_not_all_pxe_enabled(self, remove_mock): + object_utils.create_test_port( + self.context, node_id=self.node.id, + uuid=uuidutils.generate_uuid(), + address='52:54:55:cf:2d:32', + pxe_enabled=False + ) + with task_manager.acquire(self.context, self.node.uuid) as task: + neutron.remove_ports_from_network(task, self.network_uuid) + remove_mock.assert_called_once_with( + task, + {'network_id': self.network_uuid, + 'mac_address': [self.ports[0].address]} + ) + + @mock.patch.object(client.Client, 'delete_port') + @mock.patch.object(client.Client, 'list_ports') + def test_remove_neutron_ports(self, list_mock, delete_mock): + with task_manager.acquire(self.context, self.node.uuid) as task: + list_mock.return_value = {'ports': [self.neutron_port]} + neutron.remove_neutron_ports(task, {'param': 'value'}) + list_mock.assert_called_once_with(**{'param': 'value'}) + delete_mock.assert_called_once_with(self.neutron_port['id']) + + @mock.patch.object(client.Client, 'list_ports') + def test_remove_neutron_ports_list_fail(self, list_mock): + with task_manager.acquire(self.context, self.node.uuid) as task: + list_mock.side_effect = neutron_client_exc.ConnectionFailed + self.assertRaisesRegex( + exception.NetworkError, 'Could not get given network VIF', + neutron.remove_neutron_ports, task, {'param': 'value'}) + list_mock.assert_called_once_with(**{'param': 'value'}) + + @mock.patch.object(client.Client, 'delete_port') + @mock.patch.object(client.Client, 'list_ports') + def test_remove_neutron_ports_delete_fail(self, list_mock, delete_mock): + with task_manager.acquire(self.context, self.node.uuid) as task: + delete_mock.side_effect = neutron_client_exc.ConnectionFailed + list_mock.return_value = {'ports': [self.neutron_port]} + self.assertRaisesRegex( + exception.NetworkError, 'Could not remove VIF', + neutron.remove_neutron_ports, task, {'param': 'value'}) + list_mock.assert_called_once_with(**{'param': 'value'}) + delete_mock.assert_called_once_with(self.neutron_port['id']) + + def test_get_node_portmap(self): + with task_manager.acquire(self.context, self.node.uuid) as task: + portmap = neutron.get_node_portmap(task) + self.assertEqual( + {self.ports[0].uuid: self.ports[0].local_link_connection}, + portmap + ) + + @mock.patch.object(neutron, 'remove_ports_from_network') + def test_rollback_ports(self, remove_mock): + with task_manager.acquire(self.context, self.node.uuid) as task: + neutron.rollback_ports(task, self.network_uuid) + remove_mock.assert_called_once_with(task, self.network_uuid) + + @mock.patch.object(neutron, 'LOG') + @mock.patch.object(neutron, 'remove_ports_from_network') + def test_rollback_ports_exception(self, remove_mock, log_mock): + remove_mock.side_effect = exception.NetworkError('boom') + with task_manager.acquire(self.context, self.node.uuid) as task: + neutron.rollback_ports(task, self.network_uuid) + self.assertTrue(log_mock.exception.called) diff --git a/ironic/tests/unit/conductor/test_base_manager.py b/ironic/tests/unit/conductor/test_base_manager.py index 4416f86f56..215228ed62 100644 --- a/ironic/tests/unit/conductor/test_base_manager.py +++ b/ironic/tests/unit/conductor/test_base_manager.py @@ -77,7 +77,8 @@ class StartStopTestCase(mgr_utils.ServiceSetUpMixin, tests_db_base.DbTestCase): @mock.patch.object(driver_factory.DriverFactory, '__getitem__', lambda *args: mock.MagicMock()) - def test_start_registers_driver_names(self): + @mock.patch.object(driver_factory, 'NetworkInterfaceFactory') + def test_start_registers_driver_names(self, net_factory): init_names = ['fake1', 'fake2'] restart_names = ['fake3', 'fake4'] @@ -99,6 +100,7 @@ class StartStopTestCase(mgr_utils.ServiceSetUpMixin, tests_db_base.DbTestCase): res = objects.Conductor.get_by_hostname(self.context, self.hostname) self.assertEqual(restart_names, res['drivers']) + self.assertEqual(2, net_factory.call_count) @mock.patch.object(driver_factory.DriverFactory, '__getitem__') def test_start_registers_driver_specific_tasks(self, get_mock): diff --git a/ironic/tests/unit/conductor/test_manager.py b/ironic/tests/unit/conductor/test_manager.py index b7ef21c6b4..74483b8153 100644 --- a/ironic/tests/unit/conductor/test_manager.py +++ b/ironic/tests/unit/conductor/test_manager.py @@ -2341,7 +2341,8 @@ class MiscTestCase(mgr_utils.ServiceSetUpMixin, mgr_utils.CommonMixIn, 'management': {'result': True}, 'boot': {'result': True}, 'raid': {'result': True}, - 'deploy': {'result': True}} + 'deploy': {'result': True}, + 'network': {'result': True}} self.assertEqual(expected, ret) mock_iwdi.assert_called_once_with(self.context, node.instance_info) diff --git a/ironic/tests/unit/dhcp/test_neutron.py b/ironic/tests/unit/dhcp/test_neutron.py index df576022f8..f93476813e 100644 --- a/ironic/tests/unit/dhcp/test_neutron.py +++ b/ironic/tests/unit/dhcp/test_neutron.py @@ -18,7 +18,6 @@ import mock from neutronclient.common import exceptions as neutron_client_exc from neutronclient.v2_0 import client -from oslo_config import cfg from oslo_utils import uuidutils from ironic.common import dhcp_factory @@ -474,127 +473,34 @@ class TestNeutron(db_base.DbTestCase): [mock.call(task, task.ports[0], mock.ANY), mock.call(task, task.portgroups[0], mock.ANY)]) - @mock.patch.object(client.Client, 'create_port') - def test_create_cleaning_ports(self, create_mock): - # Ensure we can create cleaning ports for in band cleaning - create_mock.return_value = {'port': self.neutron_port} - expected = {self.ports[0].uuid: self.neutron_port['id']} + @mock.patch.object(neutron, 'create_cleaning_ports_deprecation', False) + @mock.patch.object(neutron, 'LOG', autospec=True) + def test_create_cleaning_ports(self, log_mock): + self.config(cleaning_network_uuid=uuidutils.generate_uuid(), + group='neutron') api = dhcp_factory.DHCPFactory().provider with task_manager.acquire(self.context, self.node.uuid) as task: - ports = api.create_cleaning_ports(task) - self.assertEqual(expected, ports) - create_mock.assert_called_once_with({'port': { - 'network_id': '00000000-0000-0000-0000-000000000000', - 'admin_state_up': True, 'mac_address': self.ports[0].address}}) + with mock.patch.object( + task.driver.network, 'add_cleaning_network', + autospec=True) as add_net_mock: + api.create_cleaning_ports(task) + add_net_mock.assert_called_once_with(task) - @mock.patch.object(neutron.NeutronDHCPApi, '_rollback_cleaning_ports') - @mock.patch.object(client.Client, 'create_port') - def test_create_cleaning_ports_fail(self, create_mock, rollback_mock): - # Check that if creating a port fails, the ports are cleaned up - create_mock.side_effect = neutron_client_exc.ConnectionFailed + api.create_cleaning_ports(task) + self.assertEqual(1, log_mock.warning.call_count) + + @mock.patch.object(neutron, 'delete_cleaning_ports_deprecation', False) + @mock.patch.object(neutron, 'LOG', autospec=True) + def test_delete_cleaning_ports(self, log_mock): api = dhcp_factory.DHCPFactory().provider with task_manager.acquire(self.context, self.node.uuid) as task: - self.assertRaises(exception.NodeCleaningFailure, - api.create_cleaning_ports, - task) - create_mock.assert_called_once_with({'port': { - 'network_id': '00000000-0000-0000-0000-000000000000', - 'admin_state_up': True, 'mac_address': self.ports[0].address}}) - rollback_mock.assert_called_once_with(task) + with mock.patch.object( + task.driver.network, 'remove_cleaning_network', + autospec=True) as rm_net_mock: + api.delete_cleaning_ports(task) + rm_net_mock.assert_called_once_with(task) - @mock.patch.object(neutron.NeutronDHCPApi, '_rollback_cleaning_ports') - @mock.patch.object(client.Client, 'create_port') - def test_create_cleaning_ports_fail_delayed(self, create_mock, - rollback_mock): - """Check ports are cleaned up on failure to create them - - This test checks that the port clean-up occurs - when the port create call was successful, - but the port in fact was not created. - - """ - # NOTE(pas-ha) this is trying to emulate the complex port object - # with both methods and dictionary access with methods on elements - mockport = mock.MagicMock() - create_mock.return_value = mockport - # fail only on second 'or' branch to fool lazy eval - # and actually execute both expressions to assert on both mocks - mockport.get.return_value = True - mockitem = mock.Mock() - mockport.__getitem__.return_value = mockitem - mockitem.get.return_value = None - api = dhcp_factory.DHCPFactory().provider - - with task_manager.acquire(self.context, self.node.uuid) as task: - self.assertRaises(exception.NodeCleaningFailure, - api.create_cleaning_ports, - task) - create_mock.assert_called_once_with({'port': { - 'network_id': '00000000-0000-0000-0000-000000000000', - 'admin_state_up': True, 'mac_address': self.ports[0].address}}) - rollback_mock.assert_called_once_with(task) - mockport.get.assert_called_once_with('port') - mockitem.get.assert_called_once_with('id') - mockport.__getitem__.assert_called_once_with('port') - - @mock.patch.object(client.Client, 'create_port') - def test_create_cleaning_ports_bad_config(self, create_mock): - # Check an error is raised if the cleaning network is not set - self.config(cleaning_network_uuid=None, group='neutron') - api = dhcp_factory.DHCPFactory().provider - - with task_manager.acquire(self.context, self.node.uuid) as task: - self.assertRaises(exception.InvalidParameterValue, - api.create_cleaning_ports, task) - - @mock.patch.object(client.Client, 'delete_port') - @mock.patch.object(client.Client, 'list_ports') - def test_delete_cleaning_ports(self, list_mock, delete_mock): - # Ensure that we can delete cleaning ports, and that ports with - # different macs don't get deleted - other_port = {'id': '132f871f-eaec-4fed-9475-0d54465e0f01', - 'mac_address': 'aa:bb:cc:dd:ee:ff'} - list_mock.return_value = {'ports': [self.neutron_port, other_port]} - api = dhcp_factory.DHCPFactory().provider - - with task_manager.acquire(self.context, self.node.uuid) as task: - api.delete_cleaning_ports(task) - list_mock.assert_called_once_with( - network_id='00000000-0000-0000-0000-000000000000') - delete_mock.assert_called_once_with(self.neutron_port['id']) - - @mock.patch.object(client.Client, 'list_ports') - def test_delete_cleaning_ports_list_fail(self, list_mock): - # Check that if listing ports fails, the node goes to cleanfail - list_mock.side_effect = neutron_client_exc.ConnectionFailed - api = dhcp_factory.DHCPFactory().provider - - with task_manager.acquire(self.context, self.node.uuid) as task: - self.assertRaises(exception.NodeCleaningFailure, - api.delete_cleaning_ports, - task) - list_mock.assert_called_once_with( - network_id='00000000-0000-0000-0000-000000000000') - - @mock.patch.object(client.Client, 'delete_port') - @mock.patch.object(client.Client, 'list_ports') - def test_delete_cleaning_ports_delete_fail(self, list_mock, delete_mock): - # Check that if deleting ports fails, the node goes to cleanfail - list_mock.return_value = {'ports': [self.neutron_port]} - delete_mock.side_effect = neutron_client_exc.ConnectionFailed - api = dhcp_factory.DHCPFactory().provider - - with task_manager.acquire(self.context, self.node.uuid) as task: - self.assertRaises(exception.NodeCleaningFailure, - api.delete_cleaning_ports, - task) - list_mock.assert_called_once_with( - network_id='00000000-0000-0000-0000-000000000000') - delete_mock.assert_called_once_with(self.neutron_port['id']) - - def test_out_range_auth_strategy(self): - self.assertRaises(ValueError, cfg.CONF.set_override, - 'auth_strategy', 'fake', 'neutron', - enforce_type=True) + api.delete_cleaning_ports(task) + self.assertEqual(1, log_mock.warning.call_count) diff --git a/ironic/tests/unit/drivers/modules/network/__init__.py b/ironic/tests/unit/drivers/modules/network/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/ironic/tests/unit/drivers/modules/network/test_flat.py b/ironic/tests/unit/drivers/modules/network/test_flat.py new file mode 100644 index 0000000000..bf4b36b353 --- /dev/null +++ b/ironic/tests/unit/drivers/modules/network/test_flat.py @@ -0,0 +1,85 @@ +# 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 +from ironic.conductor import task_manager +from ironic.drivers.modules.network import flat as flat_interface +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 + +CONF = cfg.CONF + + +class TestFlatInterface(db_base.DbTestCase): + + def setUp(self): + super(TestFlatInterface, self).setUp() + self.config(enabled_drivers=['fake']) + mgr_utils.mock_the_extension_manager() + self.interface = flat_interface.FlatNetwork() + self.node = utils.create_test_node(self.context) + self.port = utils.create_test_port( + self.context, node_id=self.node.id, + internal_info={ + 'cleaning_vif_port_id': uuidutils.generate_uuid()}) + + @mock.patch.object(flat_interface, 'LOG') + def test_init_incorrect_cleaning_net(self, mock_log): + self.config(cleaning_network_uuid=None, group='neutron') + flat_interface.FlatNetwork() + self.assertTrue(mock_log.warning.called) + + @mock.patch.object(neutron, 'add_ports_to_network') + @mock.patch.object(neutron, 'rollback_ports') + def test_add_cleaning_network(self, rollback_mock, add_mock): + add_mock.return_value = {self.port.uuid: 'vif-port-id'} + with task_manager.acquire(self.context, self.node.id) as task: + self.interface.add_cleaning_network(task) + rollback_mock.assert_called_once_with( + task, CONF.neutron.cleaning_network_uuid) + add_mock.assert_called_once_with( + task, CONF.neutron.cleaning_network_uuid, is_flat=True) + self.port.refresh() + self.assertEqual('vif-port-id', + self.port.internal_info['cleaning_vif_port_id']) + + @mock.patch.object(neutron, 'add_ports_to_network') + @mock.patch.object(neutron, 'rollback_ports') + def test_add_cleaning_network_no_cleaning_net_uuid(self, rollback_mock, + add_mock): + self.config(cleaning_network_uuid='abc', group='neutron') + with task_manager.acquire(self.context, self.node.id) as task: + self.assertRaises(exception.InvalidParameterValue, + self.interface.add_cleaning_network, task) + self.assertFalse(rollback_mock.called) + self.assertFalse(add_mock.called) + + @mock.patch.object(neutron, 'remove_ports_from_network') + def test_remove_cleaning_network(self, remove_mock): + with task_manager.acquire(self.context, self.node.id) as task: + self.interface.remove_cleaning_network(task) + remove_mock.assert_called_once_with( + task, CONF.neutron.cleaning_network_uuid) + self.port.refresh() + self.assertNotIn('cleaning_vif_port_id', self.port.internal_info) + + def test_unconfigure_tenant_networks(self): + 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) diff --git a/ironic/tests/unit/drivers/modules/test_deploy_utils.py b/ironic/tests/unit/drivers/modules/test_deploy_utils.py index bb362b7c40..78412e71fc 100644 --- a/ironic/tests/unit/drivers/modules/test_deploy_utils.py +++ b/ironic/tests/unit/drivers/modules/test_deploy_utils.py @@ -28,6 +28,7 @@ import testtools from testtools import matchers from ironic.common import boot_devices +from ironic.common import dhcp_factory from ironic.common import exception from ironic.common import image_service from ironic.common import keystone @@ -1735,38 +1736,66 @@ class AgentMethodsTestCase(db_base.DbTestCase): self.assertEqual(True, task.node.driver_internal_info[ 'agent_continue_if_ata_erase_failed']) - @mock.patch('ironic.dhcp.neutron.NeutronDHCPApi.delete_cleaning_ports', - autospec=True) - @mock.patch('ironic.dhcp.neutron.NeutronDHCPApi.create_cleaning_ports', - autospec=True) - def _test_prepare_inband_cleaning_ports( - self, create_mock, delete_mock, return_vif_port_id=True): + @mock.patch.object(utils.LOG, 'warning', autospec=True) + @mock.patch.object(dhcp_factory, 'DHCPFactory', autospec=True) + def _test_prepare_inband_cleaning_ports_out_of_tree( + self, dhcp_factory_mock, log_mock, return_vif_port_id=True): + self.config(group='dhcp', dhcp_provider='my_shiny_dhcp_provider') + dhcp_provider = dhcp_factory_mock.return_value.provider + create = dhcp_provider.create_cleaning_ports + delete = dhcp_provider.delete_cleaning_ports if return_vif_port_id: - create_mock.return_value = {self.ports[0].uuid: 'vif-port-id'} + create.return_value = {self.ports[0].uuid: 'vif-port-id'} else: - create_mock.return_value = {} + create.return_value = {} with task_manager.acquire( self.context, self.node.uuid, shared=False) as task: utils.prepare_cleaning_ports(task) - create_mock.assert_called_once_with(mock.ANY, task) - delete_mock.assert_called_once_with(mock.ANY, task) + create.assert_called_once_with(task) + delete.assert_called_once_with(task) + self.assertTrue(log_mock.called) self.ports[0].refresh() self.assertEqual('vif-port-id', self.ports[0].internal_info['cleaning_vif_port_id']) - def test_prepare_inband_cleaning_ports(self): - self._test_prepare_inband_cleaning_ports() + def test_prepare_inband_cleaning_ports_out_of_tree(self): + self._test_prepare_inband_cleaning_ports_out_of_tree() - def test_prepare_inband_cleaning_ports_no_vif_port_id(self): + def test_prepare_inband_cleaning_ports_out_of_tree_no_vif_port_id(self): self.assertRaises( exception.NodeCleaningFailure, - self._test_prepare_inband_cleaning_ports, + self._test_prepare_inband_cleaning_ports_out_of_tree, return_vif_port_id=False) - @mock.patch('ironic.dhcp.neutron.NeutronDHCPApi.delete_cleaning_ports', - autospec=True) - def test_tear_down_inband_cleaning_ports(self, neutron_mock): + @mock.patch('ironic.drivers.modules.network.flat.FlatNetwork.' + 'add_cleaning_network') + def test_prepare_inband_cleaning_ports_neutron(self, add_clean_net_mock): + with task_manager.acquire( + self.context, self.node.uuid, shared=False) as task: + utils.prepare_cleaning_ports(task) + add_clean_net_mock.assert_called_once_with(task) + + @mock.patch('ironic.drivers.modules.network.noop.NoopNetwork.' + 'add_cleaning_network') + @mock.patch.object(dhcp_factory, 'DHCPFactory', autospec=True) + def test_prepare_inband_cleaning_ports_provider_does_not_create( + self, dhcp_factory_mock, add_clean_net_mock): + self.config(group='dhcp', dhcp_provider='my_shiny_dhcp_provider') + dhcp_provider = dhcp_factory_mock.return_value.provider + del dhcp_provider.delete_cleaning_ports + del dhcp_provider.create_cleaning_ports + with task_manager.acquire( + self.context, self.node.uuid, shared=False) as task: + utils.prepare_cleaning_ports(task) + add_clean_net_mock.assert_called_once_with(task) + + @mock.patch.object(dhcp_factory, 'DHCPFactory', autospec=True) + def test_tear_down_inband_cleaning_ports_out_of_tree(self, + dhcp_factory_mock): + self.config(group='dhcp', dhcp_provider='my_shiny_dhcp_provider') + dhcp_provider = dhcp_factory_mock.return_value.provider + delete = dhcp_provider.delete_cleaning_ports internal_info = self.ports[0].internal_info internal_info['cleaning_vif_port_id'] = 'vif-port-id-1' self.ports[0].internal_info = internal_info @@ -1774,18 +1803,46 @@ class AgentMethodsTestCase(db_base.DbTestCase): with task_manager.acquire( self.context, self.node.uuid, shared=False) as task: utils.tear_down_cleaning_ports(task) - neutron_mock.assert_called_once_with(mock.ANY, task) + delete.assert_called_once_with(task) self.ports[0].refresh() self.assertNotIn('cleaning_vif_port_id', self.ports[0].internal_info) self.assertNotIn('vif_port_id', self.ports[0].extra) + @mock.patch('ironic.drivers.modules.network.flat.FlatNetwork.' + 'remove_cleaning_network') + def test_tear_down_inband_cleaning_ports_neutron(self, rm_clean_net_mock): + extra_port = obj_utils.create_test_port( + self.context, node_id=self.node.id, address='10:00:00:00:00:01', + extra={'vif_port_id': 'vif-port'}, uuid=uuidutils.generate_uuid() + ) + with task_manager.acquire( + self.context, self.node.uuid, shared=False) as task: + utils.tear_down_cleaning_ports(task) + rm_clean_net_mock.assert_called_once_with(task) + extra_port.refresh() + self.assertNotIn('vif_port_id', extra_port.extra) + + @mock.patch('ironic.drivers.modules.network.noop.NoopNetwork.' + 'remove_cleaning_network') + @mock.patch.object(dhcp_factory, 'DHCPFactory', autospec=True) + def test_tear_down_inband_cleaning_ports_provider_does_not_delete( + self, dhcp_factory_mock, rm_clean_net_mock): + self.config(group='dhcp', dhcp_provider='my_shiny_dhcp_provider') + dhcp_provider = dhcp_factory_mock.return_value.provider + del dhcp_provider.delete_cleaning_ports + with task_manager.acquire( + self.context, self.node.uuid, shared=False) as task: + utils.tear_down_cleaning_ports(task) + rm_clean_net_mock.assert_called_once_with(task) + @mock.patch.object(pxe.PXEBoot, 'prepare_ramdisk', autospec=True) @mock.patch('ironic.conductor.utils.node_power_action', autospec=True) @mock.patch.object(utils, 'build_agent_options', autospec=True) - @mock.patch.object(utils, 'prepare_cleaning_ports', autospec=True) + @mock.patch('ironic.drivers.modules.network.flat.FlatNetwork.' + 'add_cleaning_network') def _test_prepare_inband_cleaning( - self, prepare_cleaning_ports_mock, + self, add_cleaning_network_mock, build_options_mock, power_mock, prepare_ramdisk_mock, manage_boot=True): build_options_mock.return_value = {'a': 'b'} @@ -1794,7 +1851,7 @@ class AgentMethodsTestCase(db_base.DbTestCase): self.assertEqual( states.CLEANWAIT, utils.prepare_inband_cleaning(task, manage_boot=manage_boot)) - prepare_cleaning_ports_mock.assert_called_once_with(task) + add_cleaning_network_mock.assert_called_once_with(task) power_mock.assert_called_once_with(task, states.REBOOT) self.assertEqual(1, task.node.driver_internal_info[ 'agent_erase_devices_iterations']) @@ -1815,16 +1872,17 @@ class AgentMethodsTestCase(db_base.DbTestCase): self._test_prepare_inband_cleaning(manage_boot=False) @mock.patch.object(pxe.PXEBoot, 'clean_up_ramdisk', autospec=True) - @mock.patch.object(utils, 'tear_down_cleaning_ports', autospec=True) + @mock.patch('ironic.drivers.modules.network.flat.FlatNetwork.' + 'remove_cleaning_network') @mock.patch('ironic.conductor.utils.node_power_action', autospec=True) def _test_tear_down_inband_cleaning( - self, power_mock, tear_down_ports_mock, + self, power_mock, remove_cleaning_network_mock, clean_up_ramdisk_mock, manage_boot=True): with task_manager.acquire( self.context, self.node.uuid, shared=False) as task: utils.tear_down_inband_cleaning(task, manage_boot=manage_boot) power_mock.assert_called_once_with(task, states.POWER_OFF) - tear_down_ports_mock.assert_called_once_with(task) + remove_cleaning_network_mock.assert_called_once_with(task) if manage_boot: clean_up_ramdisk_mock.assert_called_once_with( task.driver.boot, task) diff --git a/releasenotes/notes/add-network-interfaces-0a13c4aba252573e.yaml b/releasenotes/notes/add-network-interfaces-0a13c4aba252573e.yaml new file mode 100644 index 0000000000..d77c5ade47 --- /dev/null +++ b/releasenotes/notes/add-network-interfaces-0a13c4aba252573e.yaml @@ -0,0 +1,29 @@ +--- +features: + - | + Added network interface. Introduced two network interface implementations: + ``flat``, which replicates the flat network behavior present previously and + ``noop`` when neutron is not used, which is basically a noop interface. + The network interface is used to switch network for node during + provisioning/cleaning. Added ``enabled_network_interfaces`` option in + DEFAULT config section. This option defines a list of enabled network + interfaces on the conductor. +deprecations: + - | + ``create_cleaning_ports`` and ``delete_cleaning_ports`` methods in DHCP + providers are deprecated and will be removed completely in the Ocata + release. The logic they are implementing should be moved to a custom + network interface's ``add_cleaning_network`` and + ``remove_cleaning_network`` methods respectively. After that, the methods + themselves should be removed from DHCP provider so that network interface + is used instead. ``flat`` network interface does not require + ``[neutron]cleaning_network_uuid`` for now so as not to break standalone + deployments, but it will be required in the Ocata release. +upgrade: + - | + ``[DEFAULT]default_network_interface`` configuration option is introduced, + with empty default value. If set, the specified interface will be used as + the network interface for nodes that don't have ``network_interface`` field + set. If it is not set, the network interface is determined by looking at + the ``[dhcp]dhcp_provider`` value. If it is ``neutron`` - ``flat`` network + interface is the default, ``noop`` otherwise. diff --git a/setup.cfg b/setup.cfg index d7e5196aad..4d1f319fdf 100644 --- a/setup.cfg +++ b/setup.cfg @@ -87,6 +87,10 @@ ironic.drivers = pxe_iscsi_cimc = ironic.drivers.pxe:PXEAndCIMCDriver pxe_agent_cimc = ironic.drivers.agent:AgentAndCIMCDriver +ironic.hardware.interfaces.network = + flat = ironic.drivers.modules.network.flat:FlatNetwork + noop = ironic.drivers.modules.network.noop:NoopNetwork + ironic.database.migration_backend = sqlalchemy = ironic.db.sqlalchemy.migration