diff options
| author | Pavlo Shchelokovskyy <shchelokovskyy@gmail.com> | 2017-06-21 07:46:03 +0000 |
|---|---|---|
| committer | Pavlo Shchelokovskyy <shchelokovskyy@gmail.com> | 2017-12-14 13:36:03 +0200 |
| commit | 4d43262955f8882cdeee2a042e852eaa8396178b (patch) | |
| tree | 2a9a2e15712b316b9acac17e03bc7927191ecb69 | |
| parent | 39a63602c7aefe2ea36999c64bb499435422ac59 (diff) | |
| download | ironic-4d43262955f8882cdeee2a042e852eaa8396178b.tar.gz | |
Use adapters for neutronclient
deprecates the following options in [neutron] section:
- url
- url_timeout
- auth_strategy
Changes some internal networking-related functions/methods
to accept a request context as optional keyword argument (defaults to
None).
This allows to pass a global request id to neutron client and
in future will simplify creating a user auth plugin from request
context.
For backward compatibility, when calling those functions/methods
without a request context, a dummy request context will be generated
automatically.
Change-Id: Ib327c7a141cfbca63b870027ad8e901c0f48bb2d
Partial-Bug: #1699547
20 files changed, 517 insertions, 292 deletions
diff --git a/devstack/lib/ironic b/devstack/lib/ironic index 0600ab3dc..04b23579e 100644 --- a/devstack/lib/ironic +++ b/devstack/lib/ironic @@ -1122,7 +1122,7 @@ function configure_ironic_conductor { # TODO(pas-ha) this block is for transition period only, # after all clients are moved to use keystoneauth adapters, # it will be deleted - local sections_with_adapter="service_catalog glance cinder inspector swift" + local sections_with_adapter="service_catalog glance cinder inspector swift neutron" for conf_section in $sections_with_adapter; do configure_adapter_for $conf_section done diff --git a/doc/source/install/include/configure-ironic-conductor.rst b/doc/source/install/include/configure-ironic-conductor.rst index 2279a6307..cfb473499 100644 --- a/doc/source/install/include/configure-ironic-conductor.rst +++ b/doc/source/install/include/configure-ironic-conductor.rst @@ -150,7 +150,7 @@ Configuring ironic-conductor service [neutron] # URL for connecting to neutron. (string value) - url=<NETWORKING_SERVICE_ENDPOINT> + endpoint_override = <NETWORKING_SERVICE_ENDPOINT> #. Configure a specific ironic-api service URL - only if you do not want to use discovery of the Baremetal service endpoint from keystone catalog diff --git a/etc/ironic/ironic.conf.sample b/etc/ironic/ironic.conf.sample index e821dcbf6..9fe966a53 100644 --- a/etc/ironic/ironic.conf.sample +++ b/etc/ironic/ironic.conf.sample @@ -2587,11 +2587,16 @@ # Authentication URL (string value) #auth_url = <None> -# Authentication strategy to use when connecting to neutron. -# Running neutron in noauth mode (related to but not affected -# by this setting) is insecure and should only be used for -# testing. (string value) +# DEPRECATED: Authentication strategy to use when connecting +# to neutron. Running neutron in noauth mode (related to but +# not affected by this setting) is insecure and should only be +# used for testing. (string value) # Allowed values: keystone, noauth +# This option is deprecated for removal. +# Its value may be silently ignored in the future. +# Reason: To configure neutron for noauth mode, set +# [neutron]/auth_type = none and +# [neutron]/endpoint_override=<NEUTRON_API_URL> instead #auth_strategy = keystone # Authentication type to load (string value) @@ -2637,12 +2642,28 @@ # Domain name to scope to (string value) #domain_name = <None> +# Always use this endpoint URL for requests for this client. +# (string value) +#endpoint_override = <None> + # Verify HTTPS connections. (boolean value) #insecure = false # PEM encoded client certificate key file (string value) #keyfile = <None> +# The maximum major version of a given API, intended to be +# used as the upper bound of a range with min_version. +# Mutually exclusive with version. (string value) +#max_version = <None> + +# The minimum major version of a given API, intended to be +# used as the lower bound of a range with max_version. +# Mutually exclusive with version. If min_version is given +# with no max_version it is as if max version is "latest". +# (string value) +#min_version = <None> + # User's password (string value) #password = <None> @@ -2679,10 +2700,22 @@ # is used. (list value) #provisioning_network_security_groups = +# The default region_name for endpoint URL discovery. (string +# value) +#region_name = <None> + # Client retries in the case of a failed request. (integer # value) #retries = 3 +# The default service_name for endpoint URL discovery. (string +# value) +#service_name = <None> + +# The default service_type for endpoint URL discovery. (string +# value) +#service_type = network + # Tenant ID (string value) #tenant_id = <None> @@ -2695,14 +2728,24 @@ # Trust ID (string value) #trust_id = <None> -# URL for connecting to neutron. Default value translates to -# 'http://$my_ip:9696' when auth_strategy is 'noauth', and to -# discovery from Keystone catalog when auth_strategy is -# 'keystone'. (string value) +# DEPRECATED: URL for connecting to neutron. Default value +# translates to 'http://$my_ip:9696' when auth_strategy is +# 'noauth', and to discovery from Keystone catalog when +# auth_strategy is 'keystone'. (string value) +# This option is deprecated for removal. +# Its value may be silently ignored in the future. +# Reason: Use [neutron]/endpoint_override option instead. It +# has no default value and must be set explicitly if required +# to connect to specific neutron URL, for example when +# [neutron]auth_strategy is noauth. #url = <None> -# Timeout value for connecting to neutron in seconds. (integer -# value) +# DEPRECATED: Timeout value for connecting to neutron in +# seconds. (integer value) +# This option is deprecated for removal. +# Its value may be silently ignored in the future. +# Reason: Use [neutron]/timeout option instead. It has no +# default value and must be set explicitly. #url_timeout = 30 # User's domain id (string value) @@ -2718,6 +2761,15 @@ # Deprecated group/name - [neutron]/user_name #username = <None> +# List of interfaces, in order of preference, for endpoint +# URL. (list value) +#valid_interfaces = internal,public + +# Minimum Major API version within a given Major API version +# for endpoint URL discovery. Mutually exclusive with +# min_version and max_version (string value) +#version = <None> + [oneview] diff --git a/ironic/common/keystone.py b/ironic/common/keystone.py index 53f263716..ba167a1b1 100644 --- a/ironic/common/keystone.py +++ b/ironic/common/keystone.py @@ -22,7 +22,6 @@ from oslo_log import log as logging import six from ironic.common import exception -from ironic.conf import auth as auth_conf from ironic.conf import CONF @@ -118,24 +117,3 @@ def get_service_auth(context, endpoint, service_auth): user_auth = token_endpoint.Token(endpoint, context.auth_token) return service_token.ServiceTokenAuthWrapper(user_auth=user_auth, service_auth=service_auth) - - -# NOTE(pas-ha) Used by neutronclient only -# FIXME(pas-ha) remove this while moving to kesytoneauth adapters -@ks_exceptions -def get_service_url(session, **kwargs): - """Find endpoint for given service in keystone catalog. - - If 'interface' is provided, fetches service url of this interface. - Otherwise, first tries to fetch 'internal' endpoint, - and then the 'public' one. - - :param session: keystoneauth Session object - :param kwargs: any other arguments accepted by Session.get_endpoint method - - """ - - if 'interface' in kwargs: - return session.get_endpoint(**kwargs) - return session.get_endpoint(interface=auth_conf.DEFAULT_VALID_INTERFACES, - **kwargs) diff --git a/ironic/common/neutron.py b/ironic/common/neutron.py index 4bba8d124..aa30c0a50 100644 --- a/ironic/common/neutron.py +++ b/ironic/common/neutron.py @@ -15,6 +15,7 @@ from neutronclient.v2_0 import client as clientv20 from oslo_log import log from oslo_utils import uuidutils +from ironic.common import context as ironic_context from ironic.common import exception from ironic.common.i18n import _ from ironic.common import keystone @@ -23,6 +24,8 @@ from ironic.conf import CONF LOG = log.getLogger(__name__) +# TODO(pas-ha) remove in Rocky, until then it is a default +# for CONF.neutron.url in noauth case when endpoint_override is not set DEFAULT_NEUTRON_URL = 'http://%s:9696' % CONF.my_ip _NEUTRON_SESSION = None @@ -39,49 +42,53 @@ SEGMENTS_PARAM_NAME = 'segments' def _get_neutron_session(): global _NEUTRON_SESSION if not _NEUTRON_SESSION: - auth = keystone.get_auth('neutron') - _NEUTRON_SESSION = keystone.get_session('neutron', auth=auth) + _NEUTRON_SESSION = keystone.get_session( + 'neutron', + # TODO(pas-ha) remove in Rocky + timeout=CONF.neutron.timeout or CONF.neutron.url_timeout) return _NEUTRON_SESSION -def get_client(token=None): - params = {'retries': CONF.neutron.retries} - url = CONF.neutron.url - if CONF.neutron.auth_strategy == 'noauth': - params['endpoint_url'] = url or DEFAULT_NEUTRON_URL - params['auth_strategy'] = 'noauth' - params.update({ - 'timeout': CONF.neutron.url_timeout or CONF.neutron.timeout, - 'insecure': CONF.neutron.insecure, - 'ca_cert': CONF.neutron.cafile}) +# TODO(pas-ha) remove deprecated options handling in Rocky +# until then it might look ugly due to all if's. +def get_client(token=None, context=None): + if not context: + context = ironic_context.RequestContext(auth_token=token) + # NOTE(pas-ha) neutronclient supports passing both session + # and the auth to client separately, makes things easier + session = _get_neutron_session() + service_auth = keystone.get_auth('neutron') + + # TODO(pas-ha) remove in Rocky, always simply load from config + # 'noauth' then would correspond to 'auth_type=none' and + # 'endpoint_override' + adapter_params = {} + if (CONF.neutron.auth_strategy == 'noauth' and + CONF.neutron.auth_type is None): + CONF.set_override('auth_type', 'none', group='neutron') + if not CONF.neutron.endpoint_override: + adapter_params['endpoint_override'] = (CONF.neutron.url or + DEFAULT_NEUTRON_URL) else: - session = _get_neutron_session() - if token is None: - params['session'] = session - # NOTE(pas-ha) endpoint_override==None will auto-discover - # endpoint from Keystone catalog. - # Region is needed only in this case. - # SSL related options are ignored as they are already embedded - # in keystoneauth Session object - if url: - params['endpoint_override'] = url - else: - params['region_name'] = CONF.keystone.region_name - else: - params['token'] = token - params['endpoint_url'] = url or keystone.get_service_url( - session, - service_type='network', - region_name=CONF.keystone.region_name) - params.update({ - 'timeout': CONF.neutron.url_timeout or CONF.neutron.timeout, - 'insecure': CONF.neutron.insecure, - 'ca_cert': CONF.neutron.cafile}) - - return clientv20.Client(**params) - - -def unbind_neutron_port(port_id, client=None): + if CONF.keystone.region_name and not CONF.neutron.region_name: + adapter_params['region_name'] = CONF.keystone.region_name + if CONF.neutron.url and not CONF.neutron.endpoint_override: + adapter_params['endpoint_override'] = CONF.neutron.url + adapter = keystone.get_adapter('neutron', session=session, + auth=service_auth, **adapter_params) + endpoint = adapter.get_endpoint() + + user_auth = None + if CONF.neutron.auth_type != 'none' and context.auth_token: + user_auth = keystone.get_service_auth(context, endpoint, service_auth) + return clientv20.Client(session=session, + auth=user_auth or service_auth, + endpoint_override=endpoint, + retries=CONF.neutron.retries, + global_request_id=context.global_id) + + +def unbind_neutron_port(port_id, client=None, context=None): """Unbind a neutron port Remove a neutron port's binding profile and host ID so that it returns to @@ -89,11 +96,13 @@ def unbind_neutron_port(port_id, client=None): :param port_id: Neutron port ID. :param client: Optional a Neutron client object. + :param context: request context + :type context: ironic.common.context.RequestContext :raises: NetworkError """ if not client: - client = get_client() + client = get_client(context=context) body = {'port': {'binding:host_id': '', 'binding:profile': {}}} @@ -111,14 +120,16 @@ def unbind_neutron_port(port_id, client=None): raise exception.NetworkError(msg) -def update_port_address(port_id, address): +def update_port_address(port_id, address, context=None): """Update a port's mac address. :param port_id: Neutron port id. :param address: new MAC address. + :param context: request context + :type context: ironic.common.context.RequestContext :raises: FailedToUpdateMacOnPort """ - client = get_client() + client = get_client(context=context) port_req_body = {'port': {'mac_address': address}} try: @@ -134,7 +145,7 @@ def update_port_address(port_id, 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) + unbind_neutron_port(port_id, client=client, context=context) msg = (_("Failed to update MAC address on Neutron port %s.") % port_id) client.update_port(port_id, port_req_body) @@ -196,7 +207,7 @@ def add_ports_to_network(task, network_uuid, security_groups=None): :raises: NetworkError :returns: a dictionary in the form {port.uuid: neutron_port['id']} """ - client = get_client() + client = get_client(context=task.context) node = task.node # If Security Groups are specified, verify that they exist @@ -300,7 +311,7 @@ def remove_neutron_ports(task, params): :param params: Dict of params to filter ports. :raises: NetworkError """ - client = get_client() + client = get_client(context=task.context) node_uuid = task.node.uuid try: @@ -410,11 +421,13 @@ def rollback_ports(task, network_uuid): {'node': task.node.uuid, 'network': network_uuid}) -def validate_network(uuid_or_name, net_type=_('network')): +def validate_network(uuid_or_name, net_type=_('network'), context=None): """Check that the given network is present. :param uuid_or_name: network UUID or name :param net_type: human-readable network type for error messages + :param context: request context + :type context: ironic.common.context.RequestContext :return: network UUID :raises: MissingParameterValue if uuid_or_name is empty :raises: NetworkError on failure to contact Neutron @@ -424,7 +437,7 @@ def validate_network(uuid_or_name, net_type=_('network')): raise exception.MissingParameterValue( _('UUID or name of %s is not set in configuration') % net_type) - client = get_client() + client = get_client(context=context) network = _get_network_by_uuid_or_name(client, uuid_or_name, net_type=net_type, fields=['id']) return network['id'] @@ -554,16 +567,16 @@ class NeutronNetworkInterfaceMixin(object): _cleaning_network_uuid = None _provisioning_network_uuid = None - def get_cleaning_network_uuid(self): + def get_cleaning_network_uuid(self, context=None): if self._cleaning_network_uuid is None: self._cleaning_network_uuid = validate_network( CONF.neutron.cleaning_network, - _('cleaning network')) + _('cleaning network'), context=context) return self._cleaning_network_uuid - def get_provisioning_network_uuid(self): + def get_provisioning_network_uuid(self, context=None): if self._provisioning_network_uuid is None: self._provisioning_network_uuid = validate_network( CONF.neutron.provisioning_network, - _('provisioning network')) + _('provisioning network'), context=context) return self._provisioning_network_uuid diff --git a/ironic/conf/neutron.py b/ironic/conf/neutron.py index 2012daf37..86d0fca1b 100644 --- a/ironic/conf/neutron.py +++ b/ironic/conf/neutron.py @@ -21,6 +21,13 @@ from ironic.conf import auth opts = [ cfg.StrOpt('url', + deprecated_for_removal=True, + deprecated_reason=_("Use [neutron]/endpoint_override option " + "instead. It has no default value and must " + "be set explicitly if required to connect " + "to specific neutron URL, for example " + "in stand alone mode when " + "[neutron]/auth_type is 'none'."), help=_("URL for connecting to neutron. " "Default value translates to 'http://$my_ip:9696' " "when auth_strategy is 'noauth', " @@ -28,6 +35,9 @@ opts = [ "when auth_strategy is 'keystone'.")), cfg.IntOpt('url_timeout', default=30, + deprecated_for_removal=True, + deprecated_reason=_("Set the desired value explicitly using " + "the [neutron]/timeout option instead."), help=_('Timeout value for connecting to neutron in seconds.')), cfg.IntOpt('port_setup_delay', default=0, @@ -40,6 +50,11 @@ opts = [ cfg.StrOpt('auth_strategy', default='keystone', choices=['keystone', 'noauth'], + deprecated_for_removal=True, + deprecated_reason=_("To configure neutron for noauth mode, " + "set [neutron]/auth_type = none and " + "[neutron]/endpoint_override=" + "<NEUTRON_API_URL> instead"), help=_('Authentication strategy to use when connecting to ' 'neutron. Running neutron in noauth mode (related to ' 'but not affected by this setting) is insecure and ' @@ -80,8 +95,8 @@ opts = [ def register_opts(conf): conf.register_opts(opts, group='neutron') - auth.register_auth_opts(conf, 'neutron') + auth.register_auth_opts(conf, 'neutron', service_type='network') def list_opts(): - return auth.add_auth_opts(opts) + return auth.add_auth_opts(opts, service_type='network') diff --git a/ironic/dhcp/base.py b/ironic/dhcp/base.py index 6e694be01..88dd3e4e0 100644 --- a/ironic/dhcp/base.py +++ b/ironic/dhcp/base.py @@ -19,15 +19,19 @@ Abstract base class for dhcp providers. import abc +from oslo_log import log as logging import six +LOG = logging.getLogger(__name__) + @six.add_metaclass(abc.ABCMeta) class BaseDHCP(object): """Base class for DHCP provider APIs.""" @abc.abstractmethod - def update_port_dhcp_opts(self, port_id, dhcp_options, token=None): + def update_port_dhcp_opts(self, port_id, dhcp_options, token=None, + context=None): """Update one or more DHCP options on the specified port. :param port_id: designate which port these attributes @@ -40,10 +44,16 @@ class BaseDHCP(object): 'opt_value': 'pxelinux.0'}, {'opt_name': '66', 'opt_value': '123.123.123.456'}] - :param token: An optional authentication token. - + :param token: An optional authentication token. Deprecated, use context + :param context: request context + :type context: ironic.common.context.RequestContext :raises: FailedToUpdateDHCPOptOnPort """ + # TODO(pas-ha) ignore token arg in Rocky + if token: + LOG.warning("Using the 'token' argument is deprecated, " + "use the 'context' argument to pass the " + "full request context instead.") @abc.abstractmethod def update_dhcp_opts(self, task, options, vifs=None): diff --git a/ironic/dhcp/neutron.py b/ironic/dhcp/neutron.py index b885c1851..27c4366d0 100644 --- a/ironic/dhcp/neutron.py +++ b/ironic/dhcp/neutron.py @@ -34,7 +34,8 @@ LOG = logging.getLogger(__name__) class NeutronDHCPApi(base.BaseDHCP): """API for communicating to neutron 2.x API.""" - def update_port_dhcp_opts(self, port_id, dhcp_options, token=None): + def update_port_dhcp_opts(self, port_id, dhcp_options, token=None, + context=None): """Update a port's attributes. Update one or more DHCP options on the specified port. @@ -51,13 +52,17 @@ class NeutronDHCPApi(base.BaseDHCP): 'opt_value': 'pxelinux.0'}, {'opt_name': '66', 'opt_value': '123.123.123.456'}] - :param token: optional auth token. - + :param token: optional auth token. Deprecated, use context. + :param context: request context + :type context: ironic.common.context.RequestContext :raises: FailedToUpdateDHCPOptOnPort """ + super(NeutronDHCPApi, self).update_port_dhcp_opts( + port_id, dhcp_options, token=token, context=context) port_req_body = {'port': {'extra_dhcp_opts': dhcp_options}} try: - neutron.get_client(token).update_port(port_id, port_req_body) + neutron.get_client(token=token, context=context).update_port( + port_id, port_req_body) except neutron_client_exc.NeutronClientException: LOG.exception("Failed to update Neutron port %s.", port_id) raise exception.FailedToUpdateDHCPOptOnPort(port_id=port_id) @@ -99,7 +104,7 @@ class NeutronDHCPApi(base.BaseDHCP): vif_list = [vif for pdict in vifs.values() for vif in pdict.values()] for vif in vif_list: try: - self.update_port_dhcp_opts(vif, options) + self.update_port_dhcp_opts(vif, options, context=task.context) except exception.FailedToUpdateDHCPOptOnPort: failures.append(vif) @@ -228,7 +233,7 @@ class NeutronDHCPApi(base.BaseDHCP): :returns: List of IP addresses associated with task's ports/portgroups. """ - client = neutron.get_client() + client = neutron.get_client(context=task.context) port_ip_addresses = self._get_ip_addresses(task, task.ports, client) portgroup_ip_addresses = self._get_ip_addresses( diff --git a/ironic/dhcp/none.py b/ironic/dhcp/none.py index eadf11474..c38484b0b 100644 --- a/ironic/dhcp/none.py +++ b/ironic/dhcp/none.py @@ -19,7 +19,8 @@ from ironic.dhcp import base class NoneDHCPApi(base.BaseDHCP): """No-op DHCP API.""" - def update_port_dhcp_opts(self, port_id, dhcp_options, token=None): + def update_port_dhcp_opts(self, port_id, dhcp_options, token=None, + context=None): pass def update_dhcp_opts(self, task, options, vifs=None): diff --git a/ironic/drivers/modules/network/common.py b/ironic/drivers/modules/network/common.py index d01f9cfb3..563f09130 100644 --- a/ironic/drivers/modules/network/common.py +++ b/ironic/drivers/modules/network/common.py @@ -267,7 +267,7 @@ def plug_port_to_tenant_network(task, port_like_obj, client=None): body['port']['extra_dhcp_opts'] = [client_id_opt] if not client: - client = neutron.get_client() + client = neutron.get_client(context=task.context) try: client.update_port(vif_id, body) @@ -402,7 +402,8 @@ class NeutronVIFPortIDMixin(VIFPortIDMixin): vif = self._get_vif_id_by_port_like_obj(port_obj) if 'address' in port_obj.obj_what_changed(): if vif: - neutron.update_port_address(vif, port_obj.address) + neutron.update_port_address(vif, port_obj.address, + context=task.context) if 'extra' in port_obj.obj_what_changed(): original_port = objects.Port.get_by_id(context, port_obj.id) @@ -421,7 +422,7 @@ class NeutronVIFPortIDMixin(VIFPortIDMixin): 'opt_value': updated_client_id} api.provider.update_port_dhcp_opts( - vif, [client_id_opt]) + vif, [client_id_opt], context=task.context) # Log warning if there is no VIF and an instance # is associated with the node. elif node.instance_uuid: @@ -467,7 +468,8 @@ class NeutronVIFPortIDMixin(VIFPortIDMixin): portgroup_obj.address): pg_vif = self._get_vif_id_by_port_like_obj(portgroup_obj) if pg_vif: - neutron.update_port_address(pg_vif, portgroup_obj.address) + neutron.update_port_address(pg_vif, portgroup_obj.address, + context=task.context) if 'extra' in portgroup_obj.obj_what_changed(): original_portgroup = objects.Portgroup.get_by_id(context, @@ -522,7 +524,7 @@ class NeutronVIFPortIDMixin(VIFPortIDMixin): network. """ vif_id = vif_info['id'] - client = neutron.get_client() + client = neutron.get_client(context=task.context) # Determine whether any of the node's ports have a physical network. If # not, we don't need to check the VIF's network's physical networks as @@ -549,7 +551,8 @@ class NeutronVIFPortIDMixin(VIFPortIDMixin): # Address is optional for portgroups if port_like_obj.address: try: - neutron.update_port_address(vif_id, port_like_obj.address) + neutron.update_port_address(vif_id, port_like_obj.address, + context=task.context) except exception.FailedToUpdateMacOnPort: raise exception.NetworkError(_( "Unable to attach VIF %(vif)s because Ironic can not " @@ -580,4 +583,4 @@ class NeutronVIFPortIDMixin(VIFPortIDMixin): # NOTE(vsaienko) allow to unplug VIFs from ACTIVE instance. if task.node.provision_state == states.ACTIVE: - neutron.unbind_neutron_port(vif_id) + neutron.unbind_neutron_port(vif_id, context=task.context) diff --git a/ironic/drivers/modules/network/flat.py b/ironic/drivers/modules/network/flat.py index bdb6486fb..51d5ffa06 100644 --- a/ironic/drivers/modules/network/flat.py +++ b/ironic/drivers/modules/network/flat.py @@ -51,7 +51,7 @@ class FlatNetwork(common.NeutronVIFPortIDMixin, is invalid. :raises: MissingParameterValue, if some parameters are missing. """ - self.get_cleaning_network_uuid() + self.get_cleaning_network_uuid(context=task.context) def add_provisioning_network(self, task): """Add the provisioning network to a node. @@ -65,7 +65,7 @@ class FlatNetwork(common.NeutronVIFPortIDMixin, if not host_id: return - client = neutron.get_client() + client = neutron.get_client(context=task.context) for port_like_obj in task.ports + task.portgroups: vif_port_id = ( port_like_obj.internal_info.get(common.TENANT_VIF_KEY) or @@ -116,10 +116,12 @@ class FlatNetwork(common.NeutronVIFPortIDMixin, :raises: NetworkError, InvalidParameterValue """ # If we have left over ports from a previous cleaning, remove them - neutron.rollback_ports(task, self.get_cleaning_network_uuid()) + neutron.rollback_ports(task, + self.get_cleaning_network_uuid( + context=task.context)) LOG.info('Adding cleaning network to node %s', task.node.uuid) vifs = neutron.add_ports_to_network( - task, self.get_cleaning_network_uuid()) + task, self.get_cleaning_network_uuid(context=task.context)) for port in task.ports: if port.uuid in vifs: internal_info = port.internal_info @@ -136,8 +138,8 @@ class FlatNetwork(common.NeutronVIFPortIDMixin, """ LOG.info('Removing ports from cleaning network for node %s', task.node.uuid) - neutron.remove_ports_from_network(task, - self.get_cleaning_network_uuid()) + neutron.remove_ports_from_network( + task, self.get_cleaning_network_uuid(context=task.context)) for port in task.ports: if 'cleaning_vif_port_id' in port.internal_info: internal_info = port.internal_info diff --git a/ironic/drivers/modules/network/neutron.py b/ironic/drivers/modules/network/neutron.py index fef7e76d8..b88b47703 100644 --- a/ironic/drivers/modules/network/neutron.py +++ b/ironic/drivers/modules/network/neutron.py @@ -57,8 +57,8 @@ class NeutronNetwork(common.NeutronVIFPortIDMixin, is invalid. :raises: MissingParameterValue, if some parameters are missing. """ - self.get_cleaning_network_uuid() - self.get_provisioning_network_uuid() + self.get_cleaning_network_uuid(context=task.context) + self.get_provisioning_network_uuid(context=task.context) def add_provisioning_network(self, task): """Add the provisioning network to a node. @@ -68,11 +68,12 @@ class NeutronNetwork(common.NeutronVIFPortIDMixin, """ # If we have left over ports from a previous provision attempt, remove # them - neutron.rollback_ports(task, self.get_provisioning_network_uuid()) + neutron.rollback_ports( + task, self.get_provisioning_network_uuid(context=task.context)) LOG.info('Adding provisioning network to node %s', task.node.uuid) vifs = neutron.add_ports_to_network( - task, self.get_provisioning_network_uuid(), + task, self.get_provisioning_network_uuid(context=task.context), security_groups=CONF.neutron.provisioning_network_security_groups) for port in task.ports: if port.uuid in vifs: @@ -90,7 +91,7 @@ class NeutronNetwork(common.NeutronVIFPortIDMixin, LOG.info('Removing provisioning network from node %s', task.node.uuid) neutron.remove_ports_from_network( - task, self.get_provisioning_network_uuid()) + task, self.get_provisioning_network_uuid(context=task.context)) for port in task.ports: if 'provisioning_vif_port_id' in port.internal_info: internal_info = port.internal_info @@ -106,12 +107,14 @@ class NeutronNetwork(common.NeutronVIFPortIDMixin, :returns: a dictionary in the form {port.uuid: neutron_port['id']} """ # If we have left over ports from a previous cleaning, remove them - neutron.rollback_ports(task, self.get_cleaning_network_uuid()) + neutron.rollback_ports(task, self.get_cleaning_network_uuid( + context=task.context)) LOG.info('Adding cleaning network to node %s', task.node.uuid) security_groups = CONF.neutron.cleaning_network_security_groups - vifs = neutron.add_ports_to_network(task, - self.get_cleaning_network_uuid(), - security_groups=security_groups) + vifs = neutron.add_ports_to_network( + task, + self.get_cleaning_network_uuid(context=task.context), + security_groups=security_groups) for port in task.ports: if port.uuid in vifs: internal_info = port.internal_info @@ -128,8 +131,8 @@ class NeutronNetwork(common.NeutronVIFPortIDMixin, """ LOG.info('Removing cleaning network from node %s', task.node.uuid) - neutron.remove_ports_from_network(task, - self.get_cleaning_network_uuid()) + neutron.remove_ports_from_network( + task, self.get_cleaning_network_uuid(context=task.context)) for port in task.ports: if 'cleaning_vif_port_id' in port.internal_info: internal_info = port.internal_info @@ -158,7 +161,7 @@ class NeutronNetwork(common.NeutronVIFPortIDMixin, ports = [p for p in ports if not p.portgroup_id] portgroups = task.portgroups - client = neutron.get_client() + client = neutron.get_client(context=task.context) pobj_without_vif = 0 for port_like_obj in ports + portgroups: @@ -196,4 +199,4 @@ class NeutronNetwork(common.NeutronVIFPortIDMixin, port_like_obj.extra.get('vif_port_id')) if not vif_port_id: continue - neutron.unbind_neutron_port(vif_port_id) + neutron.unbind_neutron_port(vif_port_id, context=task.context) diff --git a/ironic/tests/unit/common/test_keystone.py b/ironic/tests/unit/common/test_keystone.py index 9babe028c..c9c54cff0 100644 --- a/ironic/tests/unit/common/test_keystone.py +++ b/ironic/tests/unit/common/test_keystone.py @@ -70,21 +70,6 @@ class KeystoneTestCase(base.TestCase): keystone.get_auth, self.test_group) - def test_get_service_url_with_interface(self): - session = mock.Mock() - session.get_endpoint.return_value = 'spam' - params = {'interface': 'admin', 'ham': 'eggs'} - self.assertEqual('spam', keystone.get_service_url(session, **params)) - session.get_endpoint.assert_called_once_with(**params) - - def test_get_service_url(self): - session = mock.Mock() - session.get_endpoint.return_value = 'spam' - params = {'ham': 'eggs'} - self.assertEqual('spam', keystone.get_service_url(session, **params)) - session.get_endpoint.assert_called_once_with( - interface=['internal', 'public'], **params) - def test_get_adapter_from_config(self): self.config(valid_interfaces=['internal', 'public'], group=self.test_group) diff --git a/ironic/tests/unit/common/test_neutron.py b/ironic/tests/unit/common/test_neutron.py index e498e0631..15cb50e68 100644 --- a/ironic/tests/unit/common/test_neutron.py +++ b/ironic/tests/unit/common/test_neutron.py @@ -10,12 +10,14 @@ # License for the specific language governing permissions and limitations # under the License. +from keystoneauth1 import loading as kaloading 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 context from ironic.common import exception from ironic.common import neutron from ironic.conductor import task_manager @@ -25,85 +27,145 @@ from ironic.tests.unit.db import base as db_base from ironic.tests.unit.objects import utils as object_utils -@mock.patch.object(neutron, '_get_neutron_session', autospec=True) -@mock.patch.object(client.Client, "__init__", autospec=True) +@mock.patch('ironic.common.keystone.get_service_auth', autospec=True, + return_value=mock.sentinel.sauth) +@mock.patch('ironic.common.keystone.get_auth', autospec=True, + return_value=mock.sentinel.auth) +@mock.patch('ironic.common.keystone.get_adapter', autospec=True) +@mock.patch('ironic.common.keystone.get_session', autospec=True, + return_value=mock.sentinel.session) +@mock.patch.object(client.Client, "__init__", return_value=None, autospec=True) class TestNeutronClient(base.TestCase): def setUp(self): super(TestNeutronClient, self).setUp() - self.config(url_timeout=30, - retries=2, + # NOTE(pas-ha) register keystoneauth dynamic options manually + plugin = kaloading.get_plugin_loader('password') + opts = kaloading.get_auth_plugin_conf_options(plugin) + self.cfg_fixture.register_opts(opts, group='neutron') + self.config(retries=2, group='neutron') - self.config(admin_user='test-admin-user', - admin_tenant_name='test-admin-tenant', - admin_password='test-admin-password', - auth_uri='test-auth-uri', - group='keystone_authtoken') - # TODO(pas-ha) register session options to test legacy path - self.config(insecure=False, - cafile='test-file', + self.config(username='test-admin-user', + project_name='test-admin-tenant', + password='test-admin-password', + auth_url='test-auth-uri', + auth_type='password', + interface='internal', + service_type='network', + timeout=10, group='neutron') - - def test_get_neutron_client_with_token(self, mock_client_init, - mock_session): - token = 'test-token-123' - sess = mock.Mock() - sess.get_endpoint.return_value = 'fake-url' - mock_session.return_value = sess - expected = {'timeout': 30, - 'retries': 2, - 'insecure': False, - 'ca_cert': 'test-file', - 'token': token, - 'endpoint_url': 'fake-url'} - - mock_client_init.return_value = None - neutron.get_client(token=token) - mock_client_init.assert_called_once_with(mock.ANY, **expected) + # force-reset the global session object + neutron._NEUTRON_SESSION = None + self.context = context.RequestContext(global_request_id='global') + + def _call_and_assert_client(self, client_mock, url, + auth=mock.sentinel.auth): + neutron.get_client(context=self.context) + client_mock.assert_called_once_with(mock.ANY, # this is 'self' + session=mock.sentinel.session, + auth=auth, retries=2, + endpoint_override=url, + global_request_id='global') + + @mock.patch('ironic.common.context.RequestContext', autospec=True) + def test_get_neutron_client_with_token(self, mock_ctxt, mock_client_init, + mock_session, mock_adapter, + mock_auth, mock_sauth): + mock_ctxt.return_value = ctxt = mock.Mock() + ctxt.auth_token = 'test-token-123' + mock_adapter.return_value = adapter = mock.Mock() + adapter.get_endpoint.return_value = 'neutron_url' + neutron.get_client(token='test-token-123') + mock_ctxt.assert_called_once_with(auth_token='test-token-123') + mock_client_init.assert_called_once_with( + mock.ANY, # this is 'self' + session=mock.sentinel.session, + auth=mock.sentinel.sauth, + retries=2, + endpoint_override='neutron_url', + global_request_id=ctxt.global_id) + + # testing handling of default url_timeout + mock_session.assert_called_once_with('neutron', timeout=10) + mock_adapter.assert_called_once_with('neutron', + session=mock.sentinel.session, + auth=mock.sentinel.auth) + mock_sauth.assert_called_once_with(mock_ctxt.return_value, + 'neutron_url', mock.sentinel.auth) + + def test_get_neutron_client_with_context(self, mock_client_init, + mock_session, mock_adapter, + mock_auth, mock_sauth): + self.context = context.RequestContext(global_request_id='global', + auth_token='test-token-123') + mock_adapter.return_value = adapter = mock.Mock() + adapter.get_endpoint.return_value = 'neutron_url' + self._call_and_assert_client(mock_client_init, 'neutron_url', + auth=mock.sentinel.sauth) + # testing handling of default url_timeout + mock_session.assert_called_once_with('neutron', timeout=10) + mock_adapter.assert_called_once_with('neutron', + session=mock.sentinel.session, + auth=mock.sentinel.auth) + mock_sauth.assert_called_once_with(self.context, 'neutron_url', + mock.sentinel.auth) def test_get_neutron_client_without_token(self, mock_client_init, - mock_session): - self.config(url='test-url', - group='neutron') - sess = mock.Mock() - mock_session.return_value = sess - expected = {'retries': 2, - 'endpoint_override': 'test-url', - 'session': sess} - mock_client_init.return_value = None - neutron.get_client(token=None) - mock_client_init.assert_called_once_with(mock.ANY, **expected) - - def test_get_neutron_client_with_region(self, mock_client_init, - mock_session): + mock_session, mock_adapter, + mock_auth, mock_sauth): + mock_adapter.return_value = adapter = mock.Mock() + adapter.get_endpoint.return_value = 'neutron_url' + self._call_and_assert_client(mock_client_init, 'neutron_url') + mock_session.assert_called_once_with('neutron', timeout=10) + mock_adapter.assert_called_once_with('neutron', + session=mock.sentinel.session, + auth=mock.sentinel.auth) + self.assertEqual(0, mock_sauth.call_count) + + def test_get_neutron_client_with_deprecated_opts(self, mock_client_init, + mock_session, + mock_adapter, mock_auth, + mock_sauth): self.config(region_name='fake_region', group='keystone') - sess = mock.Mock() - mock_session.return_value = sess - expected = {'retries': 2, - 'region_name': 'fake_region', - 'session': sess} - - mock_client_init.return_value = None - neutron.get_client(token=None) - mock_client_init.assert_called_once_with(mock.ANY, **expected) - - def test_get_neutron_client_noauth(self, mock_client_init, mock_session): + self.config(url='neutron_url', + url_timeout=10, + timeout=None, + service_type=None, + group='neutron') + mock_adapter.return_value = adapter = mock.Mock() + adapter.get_endpoint.return_value = 'neutron_url' + self._call_and_assert_client(mock_client_init, 'neutron_url') + mock_session.assert_called_once_with('neutron', timeout=10) + mock_adapter.assert_called_once_with('neutron', + session=mock.sentinel.session, + auth=mock.sentinel.auth, + region_name='fake_region', + endpoint_override='neutron_url') + + def test_get_neutron_client_noauth(self, mock_client_init, mock_session, + mock_adapter, mock_auth, mock_sauth): self.config(auth_strategy='noauth', - url='test-url', + endpoint_override='neutron_url', + url_timeout=None, + auth_type=None, + timeout=10, group='neutron') - expected = {'ca_cert': 'test-file', - 'insecure': False, - 'endpoint_url': 'test-url', - 'timeout': 30, - 'retries': 2, - 'auth_strategy': 'noauth'} - - mock_client_init.return_value = None - neutron.get_client(token=None) - mock_client_init.assert_called_once_with(mock.ANY, **expected) - - def test_out_range_auth_strategy(self, mock_client_init, mock_session): + mock_adapter.return_value = adapter = mock.Mock() + adapter.get_endpoint.return_value = 'neutron_url' + + self._call_and_assert_client(mock_client_init, 'neutron_url') + + self.assertEqual('none', neutron.CONF.neutron.auth_type) + mock_session.assert_called_once_with('neutron', timeout=10) + mock_adapter.assert_called_once_with('neutron', + session=mock.sentinel.session, + auth=mock.sentinel.auth) + mock_auth.assert_called_once_with('neutron') + self.assertEqual(0, mock_sauth.call_count) + + def test_out_range_auth_strategy(self, mock_client_init, mock_session, + mock_adapter, mock_auth, mock_eauth): self.assertRaises(ValueError, cfg.CONF.set_override, 'auth_strategy', 'fake', 'neutron') @@ -473,6 +535,7 @@ class TestValidateNetwork(base.TestCase): super(TestValidateNetwork, self).setUp() self.uuid = uuidutils.generate_uuid() + self.context = context.RequestContext() def test_by_uuid(self, client_mock): net_mock = client_mock.return_value.list_networks @@ -482,7 +545,8 @@ class TestValidateNetwork(base.TestCase): ] } - self.assertEqual(self.uuid, neutron.validate_network(self.uuid)) + self.assertEqual(self.uuid, neutron.validate_network( + self.uuid, context=self.context)) net_mock.assert_called_once_with(fields=['id'], id=self.uuid) @@ -494,7 +558,8 @@ class TestValidateNetwork(base.TestCase): ] } - self.assertEqual(self.uuid, neutron.validate_network('name')) + self.assertEqual(self.uuid, neutron.validate_network( + 'name', context=self.context)) net_mock.assert_called_once_with(fields=['id'], name='name') @@ -506,7 +571,8 @@ class TestValidateNetwork(base.TestCase): self.assertRaisesRegex(exception.InvalidParameterValue, 'was not found', - neutron.validate_network, self.uuid) + neutron.validate_network, + self.uuid, context=self.context) net_mock.assert_called_once_with(fields=['id'], id=self.uuid) @@ -515,7 +581,8 @@ class TestValidateNetwork(base.TestCase): net_mock.side_effect = neutron_client_exc.NeutronClientException('foo') self.assertRaisesRegex(exception.NetworkError, 'foo', - neutron.validate_network, 'name') + neutron.validate_network, 'name', + context=self.context) net_mock.assert_called_once_with(fields=['id'], name='name') @@ -528,7 +595,8 @@ class TestValidateNetwork(base.TestCase): self.assertRaisesRegex(exception.InvalidParameterValue, 'More than one network', - neutron.validate_network, 'name') + neutron.validate_network, 'name', + context=self.context) net_mock.assert_called_once_with(fields=['id'], name='name') @@ -536,13 +604,17 @@ class TestValidateNetwork(base.TestCase): @mock.patch.object(neutron, 'get_client', autospec=True) class TestUpdatePortAddress(base.TestCase): + def setUp(self): + super(TestUpdatePortAddress, self).setUp() + self.context = context.RequestContext() + 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) + neutron.update_port_address(port_id, address, context=self.context) mock_client.return_value.update_port.assert_called_once_with(port_id, expected) @@ -559,8 +631,11 @@ class TestUpdatePortAddress(base.TestCase): mock.call(port_id, {'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()) + neutron.update_port_address(port_id, address, context=self.context) + mock_unp.assert_called_once_with( + port_id, + client=mock_client(context=self.context), + context=self.context) mock_client.return_value.update_port.assert_has_calls(calls) @mock.patch.object(neutron, 'unbind_neutron_port', autospec=True) @@ -571,7 +646,7 @@ class TestUpdatePortAddress(base.TestCase): mock_client.return_value.show_port.return_value = { 'port': {'binding:profile': 'foo'}} - neutron.update_port_address(port_id, address) + neutron.update_port_address(port_id, address, context=self.context) self.assertFalse(mock_unp.called) mock_client.return_value.update_port.assert_any_call(port_id, expected) @@ -582,7 +657,8 @@ class TestUpdatePortAddress(base.TestCase): neutron_client_exc.NeutronClientException()) self.assertRaises(exception.FailedToUpdateMacOnPort, - neutron.update_port_address, port_id, address) + neutron.update_port_address, + port_id, address, context=self.context) self.assertFalse(mock_client.return_value.update_port.called) @mock.patch.object(neutron, 'unbind_neutron_port', autospec=True) @@ -595,8 +671,12 @@ class TestUpdatePortAddress(base.TestCase): '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()) + neutron.update_port_address, + port_id, address, context=self.context) + mock_unp.assert_called_once_with( + port_id, + client=mock_client(context=self.context), + context=self.context) self.assertFalse(mock_client.return_value.update_port.called) @mock.patch.object(neutron, 'unbind_neutron_port', autospec=True) @@ -610,12 +690,16 @@ class TestUpdatePortAddress(base.TestCase): self.assertRaises(exception.FailedToUpdateMacOnPort, neutron.update_port_address, - port_id, address) + port_id, address, context=self.context) @mock.patch.object(neutron, 'get_client', autospec=True) class TestUnbindPort(base.TestCase): + def setUp(self): + super(TestUnbindPort, self).setUp() + self.context = context.RequestContext() + def test_unbind_neutron_port_client_passed(self, mock_client): port_id = 'fake-port-id' body = { @@ -624,7 +708,9 @@ class TestUnbindPort(base.TestCase): 'binding:profile': {} } } - neutron.unbind_neutron_port(port_id, mock_client()) + neutron.unbind_neutron_port(port_id, + mock_client(context=self.context), + context=self.context) self.assertEqual(1, mock_client.call_count) mock_client.return_value.update_port.assert_called_once_with(port_id, body) @@ -641,8 +727,8 @@ class TestUnbindPort(base.TestCase): } port_id = 'fake-port-id' self.assertRaises(exception.NetworkError, neutron.unbind_neutron_port, - port_id) - mock_client.assert_called_once_with() + port_id, context=self.context) + mock_client.assert_called_once_with(context=self.context) mock_client.return_value.update_port.assert_called_once_with(port_id, body) mock_log.exception.assert_called_once() @@ -655,8 +741,8 @@ class TestUnbindPort(base.TestCase): 'binding:profile': {} } } - neutron.unbind_neutron_port(port_id) - mock_client.assert_called_once_with() + neutron.unbind_neutron_port(port_id, context=self.context) + mock_client.assert_called_once_with(context=self.context) mock_client.return_value.update_port.assert_called_once_with(port_id, body) @@ -671,8 +757,8 @@ class TestUnbindPort(base.TestCase): 'binding:profile': {} } } - neutron.unbind_neutron_port(port_id) - mock_client.assert_called_once_with() + neutron.unbind_neutron_port(port_id, context=self.context) + mock_client.assert_called_once_with(context=self.context) mock_client.return_value.update_port.assert_called_once_with(port_id, body) mock_log.info.assert_called_once_with('Port %s was not found while ' diff --git a/ironic/tests/unit/dhcp/test_neutron.py b/ironic/tests/unit/dhcp/test_neutron.py index 0f10d66be..ac7fee7f0 100644 --- a/ironic/tests/unit/dhcp/test_neutron.py +++ b/ironic/tests/unit/dhcp/test_neutron.py @@ -63,7 +63,9 @@ class TestNeutron(db_base.DbTestCase): expected = {'port': {'extra_dhcp_opts': opts}} api = dhcp_factory.DHCPFactory() - api.provider.update_port_dhcp_opts(port_id, opts) + with task_manager.acquire(self.context, self.node.uuid) as task: + api.provider.update_port_dhcp_opts(port_id, opts, + context=task.context) client_mock.return_value.update_port.assert_called_once_with( port_id, expected) @@ -75,10 +77,11 @@ class TestNeutron(db_base.DbTestCase): neutron_client_exc.NeutronClientException()) api = dhcp_factory.DHCPFactory() - self.assertRaises( - exception.FailedToUpdateDHCPOptOnPort, - api.provider.update_port_dhcp_opts, - port_id, opts) + with task_manager.acquire(self.context, self.node.uuid) as task: + self.assertRaises( + exception.FailedToUpdateDHCPOptOnPort, + api.provider.update_port_dhcp_opts, + port_id, opts, context=task.context) @mock.patch('ironic.dhcp.neutron.NeutronDHCPApi.update_port_dhcp_opts', autospec=True) @@ -92,7 +95,8 @@ class TestNeutron(db_base.DbTestCase): opts = pxe_utils.dhcp_options_for_instance(task) api = dhcp_factory.DHCPFactory() api.update_dhcp(task, opts) - mock_updo.assert_called_once_with(mock.ANY, 'vif-uuid', opts) + mock_updo.assert_called_once_with(mock.ANY, 'vif-uuid', opts, + context=task.context) @mock.patch('ironic.dhcp.neutron.NeutronDHCPApi.update_port_dhcp_opts', autospec=True) @@ -144,10 +148,8 @@ class TestNeutron(db_base.DbTestCase): @mock.patch.object(neutron, 'LOG', autospec=True) @mock.patch('time.sleep', autospec=True) - @mock.patch.object(neutron.NeutronDHCPApi, 'update_port_dhcp_opts', - autospec=True) @mock.patch('ironic.common.network.get_node_vif_ids', autospec=True) - def test_update_dhcp_set_sleep_and_fake(self, mock_gnvi, mock_updo, + def test_update_dhcp_set_sleep_and_fake(self, mock_gnvi, mock_ts, mock_log): mock_gnvi.return_value = {'ports': {'port-uuid': 'vif-uuid'}, 'portgroups': {}} @@ -156,27 +158,30 @@ class TestNeutron(db_base.DbTestCase): self.node.uuid) as task: opts = pxe_utils.dhcp_options_for_instance(task) api = dhcp_factory.DHCPFactory() - api.update_dhcp(task, opts) - mock_log.debug.assert_called_once_with( - "Waiting %d seconds for Neutron.", 30) - mock_ts.assert_called_with(30) - mock_updo.assert_called_once_with(mock.ANY, 'vif-uuid', opts) + with mock.patch.object(api.provider, 'update_port_dhcp_opts', + autospec=True) as mock_updo: + api.update_dhcp(task, opts) + mock_log.debug.assert_called_once_with( + "Waiting %d seconds for Neutron.", 30) + mock_ts.assert_called_with(30) + mock_updo.assert_called_once_with('vif-uuid', opts, + context=task.context) @mock.patch.object(neutron, 'LOG', autospec=True) - @mock.patch.object(neutron.NeutronDHCPApi, 'update_port_dhcp_opts', - autospec=True) @mock.patch('ironic.common.network.get_node_vif_ids', autospec=True) - def test_update_dhcp_unset_sleep_and_fake(self, mock_gnvi, mock_updo, - mock_log): + def test_update_dhcp_unset_sleep_and_fake(self, mock_gnvi, mock_log): mock_gnvi.return_value = {'ports': {'port-uuid': 'vif-uuid'}, 'portgroups': {}} with task_manager.acquire(self.context, self.node.uuid) as task: opts = pxe_utils.dhcp_options_for_instance(task) api = dhcp_factory.DHCPFactory() - api.update_dhcp(task, opts) - mock_log.debug.assert_not_called() - mock_updo.assert_called_once_with(mock.ANY, 'vif-uuid', opts) + with mock.patch.object(api.provider, 'update_port_dhcp_opts', + autospec=True) as mock_updo: + api.update_dhcp(task, opts) + mock_log.debug.assert_not_called() + mock_updo.assert_called_once_with('vif-uuid', opts, + context=task.context) def test__get_fixed_ip_address(self): port_id = 'fake-port-id' diff --git a/ironic/tests/unit/drivers/modules/network/test_common.py b/ironic/tests/unit/drivers/modules/network/test_common.py index 6c76540b2..4fc0d513a 100644 --- a/ironic/tests/unit/drivers/modules/network/test_common.py +++ b/ironic/tests/unit/drivers/modules/network/test_common.py @@ -680,10 +680,11 @@ class TestNeutronVifPortIDMixin(db_base.DbTestCase): mock_gfp.return_value = self.port with task_manager.acquire(self.context, self.node.id) as task: self.interface.vif_attach(task, vif) - mock_client.assert_called_once_with() + mock_client.assert_called_once_with(context=task.context) + mock_upa.assert_called_once_with( + "fake_vif_id", self.port.address, context=task.context) self.assertFalse(mock_gpbpi.called) mock_gfp.assert_called_once_with(task, 'fake_vif_id', set()) - mock_upa.assert_called_once_with("fake_vif_id", self.port.address) mock_save.assert_called_once_with(self.port, "fake_vif_id") @mock.patch.object(common.VIFPortIDMixin, '_save_vif_to_port_like_obj') @@ -717,11 +718,12 @@ class TestNeutronVifPortIDMixin(db_base.DbTestCase): mock_gfp.return_value = self.port with task_manager.acquire(self.context, self.node.id) as task: self.interface.vif_attach(task, vif) - mock_client.assert_called_once_with() + mock_client.assert_called_once_with(context=task.context) + mock_upa.assert_called_once_with( + "fake_vif_id", self.port.address, context=task.context) mock_gpbpi.assert_called_once_with(mock_client.return_value, 'fake_vif_id') mock_gfp.assert_called_once_with(task, 'fake_vif_id', {'physnet1'}) - mock_upa.assert_called_once_with("fake_vif_id", self.port.address) mock_save.assert_called_once_with(self.port, "fake_vif_id") @mock.patch.object(common.VIFPortIDMixin, '_save_vif_to_port_like_obj') @@ -739,10 +741,12 @@ class TestNeutronVifPortIDMixin(db_base.DbTestCase): mock_gfp.return_value = self.port with task_manager.acquire(self.context, self.node.id) as task: self.interface.vif_attach(task, vif) - mock_client.assert_called_once_with() + mock_client.assert_called_once_with(context=task.context) + mock_upa.assert_called_once_with( + "fake_vif_id", self.port.address, context=task.context) self.assertFalse(mock_gpbpi.called) mock_gfp.assert_called_once_with(task, 'fake_vif_id', set()) - mock_upa.assert_called_once_with("fake_vif_id", self.port.address) + mock_save.assert_called_once_with(self.port, "fake_vif_id") mock_plug.assert_called_once_with(task, self.port, mock.ANY) @mock.patch.object(common.VIFPortIDMixin, '_save_vif_to_port_like_obj') @@ -763,10 +767,11 @@ class TestNeutronVifPortIDMixin(db_base.DbTestCase): with task_manager.acquire(self.context, self.node.id) as task: self.assertRaises(exception.NetworkError, self.interface.vif_attach, task, vif) - mock_client.assert_called_once_with() + mock_client.assert_called_once_with(context=task.context) + mock_upa.assert_called_once_with( + "fake_vif_id", self.port.address, context=task.context) self.assertFalse(mock_gpbpi.called) mock_gfp.assert_called_once_with(task, 'fake_vif_id', set()) - mock_upa.assert_called_once_with("fake_vif_id", self.port.address) mock_save.assert_called_once_with(self.port, "fake_vif_id") mock_plug.assert_called_once_with(task, self.port, mock.ANY) @@ -784,7 +789,7 @@ class TestNeutronVifPortIDMixin(db_base.DbTestCase): vif = {'id': "fake_vif_id"} with task_manager.acquire(self.context, self.node.id) as task: self.interface.vif_attach(task, vif) - mock_client.assert_called_once_with() + mock_client.assert_called_once_with(context=task.context) self.assertFalse(mock_gpbpi.called) mock_gfp.assert_called_once_with(task, 'fake_vif_id', set()) self.assertFalse(mock_client.return_value.show_port.called) @@ -809,7 +814,7 @@ class TestNeutronVifPortIDMixin(db_base.DbTestCase): self.assertRaisesRegex( exception.NetworkError, "can not update Neutron port", self.interface.vif_attach, task, vif) - mock_client.assert_called_once_with() + mock_client.assert_called_once_with(context=task.context) mock_gpbpi.assert_called_once_with(mock_client.return_value, 'fake_vif_id') self.assertFalse(mock_save.called) @@ -833,7 +838,7 @@ class TestNeutronVifPortIDMixin(db_base.DbTestCase): self.assertRaises( exception.PortgroupPhysnetInconsistent, self.interface.vif_attach, task, vif) - mock_client.assert_called_once_with() + mock_client.assert_called_once_with(context=task.context) mock_gpbpi.assert_called_once_with(mock_client.return_value, 'fake_vif_id') self.assertFalse(mock_upa.called) @@ -859,7 +864,7 @@ class TestNeutronVifPortIDMixin(db_base.DbTestCase): self.assertRaises( exception.VifInvalidForAttach, self.interface.vif_attach, task, vif) - mock_client.assert_called_once_with() + mock_client.assert_called_once_with(context=task.context) mock_gpbpi.assert_called_once_with(mock_client.return_value, 'fake_vif_id') self.assertFalse(mock_gfp.called) @@ -913,9 +918,10 @@ class TestNeutronVifPortIDMixin(db_base.DbTestCase): mock_get.return_value = self.port with task_manager.acquire(self.context, self.node.id) as task: self.interface.vif_detach(task, 'fake_vif_id') + mock_unp.assert_called_once_with('fake_vif_id', + context=task.context) mock_get.assert_called_once_with(task, 'fake_vif_id') mock_clear.assert_called_once_with(self.port) - mock_unp.assert_called_once_with('fake_vif_id') @mock.patch.object(common.VIFPortIDMixin, '_clear_vif_from_port_like_obj') @mock.patch.object(neutron_common, 'unbind_neutron_port', autospec=True) @@ -929,9 +935,10 @@ class TestNeutronVifPortIDMixin(db_base.DbTestCase): with task_manager.acquire(self.context, self.node.id) as task: self.assertRaises(exception.NetworkError, self.interface.vif_detach, task, 'fake_vif_id') + mock_unp.assert_called_once_with('fake_vif_id', + context=task.context) mock_get.assert_called_once_with(task, 'fake_vif_id') mock_clear.assert_called_once_with(self.port) - mock_unp.assert_called_once_with('fake_vif_id') @mock.patch.object(common_utils, 'warn_about_deprecated_extra_vif_port_id', autospec=True) @@ -942,7 +949,8 @@ class TestNeutronVifPortIDMixin(db_base.DbTestCase): 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) + self.port.extra['vif_port_id'], new_address, + context=task.context) self.assertFalse(mock_warn.called) @mock.patch.object(neutron_common, 'update_port_address', autospec=True) @@ -956,7 +964,8 @@ class TestNeutronVifPortIDMixin(db_base.DbTestCase): self.interface.port_changed, task, self.port) mac_update_mock.assert_called_once_with( - self.port.extra['vif_port_id'], new_address) + self.port.extra['vif_port_id'], new_address, + context=task.context) @mock.patch.object(neutron_common, 'update_port_address', autospec=True) def test_port_changed_address_no_vif_id(self, mac_update_mock): @@ -975,7 +984,7 @@ class TestNeutronVifPortIDMixin(db_base.DbTestCase): 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) + 'fake-id', expected_dhcp_opts, context=task.context) @mock.patch.object(common_utils, 'warn_about_deprecated_extra_vif_port_id', autospec=True) @@ -1194,7 +1203,8 @@ class TestNeutronVifPortIDMixin(db_base.DbTestCase): 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) + mac_update_mock.assert_called_once_with( + 'fake-id', new_address, context=task.context) @mock.patch.object(neutron_common, 'update_port_address', autospec=True) def test_update_portgroup_remove_address(self, mac_update_mock): @@ -1261,7 +1271,8 @@ class TestNeutronVifPortIDMixin(db_base.DbTestCase): self.assertRaises(exception.FailedToUpdateMacOnPort, self.interface.portgroup_changed, task, pg) - mac_update_mock.assert_called_once_with('fake-id', new_address) + mac_update_mock.assert_called_once_with( + 'fake-id', new_address, context=task.context) @mock.patch.object(common_utils, 'warn_about_deprecated_extra_vif_port_id', autospec=True) diff --git a/ironic/tests/unit/drivers/modules/network/test_flat.py b/ironic/tests/unit/drivers/modules/network/test_flat.py index 1c4228dbb..0a72122f6 100644 --- a/ironic/tests/unit/drivers/modules/network/test_flat.py +++ b/ironic/tests/unit/drivers/modules/network/test_flat.py @@ -77,11 +77,12 @@ class TestFlatInterface(db_base.DbTestCase): def test_validate(self, validate_mock): with task_manager.acquire(self.context, self.node.id) as task: self.interface.validate(task) - validate_mock.assert_called_once_with(CONF.neutron.cleaning_network, - 'cleaning network') + validate_mock.assert_called_once_with( + CONF.neutron.cleaning_network, + 'cleaning network', context=task.context) @mock.patch.object(neutron, 'validate_network', - side_effect=lambda n, t: n) + side_effect=lambda n, t, context=None: n) @mock.patch.object(neutron, 'add_ports_to_network') @mock.patch.object(neutron, 'rollback_ports') def test_add_cleaning_network(self, rollback_mock, add_mock, @@ -95,13 +96,13 @@ class TestFlatInterface(db_base.DbTestCase): task, CONF.neutron.cleaning_network) validate_mock.assert_called_once_with( CONF.neutron.cleaning_network, - 'cleaning network') + 'cleaning network', context=task.context) self.port.refresh() self.assertEqual('vif-port-id', self.port.internal_info['cleaning_vif_port_id']) @mock.patch.object(neutron, 'validate_network', - side_effect=lambda n, t: n) + side_effect=lambda n, t, context=None: n) @mock.patch.object(neutron, 'remove_ports_from_network') def test_remove_cleaning_network(self, remove_mock, validate_mock): with task_manager.acquire(self.context, self.node.id) as task: @@ -110,7 +111,7 @@ class TestFlatInterface(db_base.DbTestCase): task, CONF.neutron.cleaning_network) validate_mock.assert_called_once_with( CONF.neutron.cleaning_network, - 'cleaning network') + 'cleaning network', context=task.context) self.port.refresh() self.assertNotIn('cleaning_vif_port_id', self.port.internal_info) diff --git a/ironic/tests/unit/drivers/modules/network/test_neutron.py b/ironic/tests/unit/drivers/modules/network/test_neutron.py index 65c413b0e..704b6f998 100644 --- a/ironic/tests/unit/drivers/modules/network/test_neutron.py +++ b/ironic/tests/unit/drivers/modules/network/test_neutron.py @@ -86,14 +86,16 @@ class NeutronInterfaceTestCase(db_base.DbTestCase): def test_validate(self, validate_mock): with task_manager.acquire(self.context, self.node.id) as task: self.interface.validate(task) - self.assertEqual([mock.call(CONF.neutron.cleaning_network, - 'cleaning network'), - mock.call(CONF.neutron.provisioning_network, - 'provisioning network')], - validate_mock.call_args_list) + self.assertEqual([mock.call(CONF.neutron.cleaning_network, + 'cleaning network', + context=task.context), + mock.call(CONF.neutron.provisioning_network, + 'provisioning network', + context=task.context)], + validate_mock.call_args_list) @mock.patch.object(neutron_common, 'validate_network', - side_effect=lambda n, t: n) + side_effect=lambda n, t, context=None: n) @mock.patch.object(neutron_common, 'rollback_ports') @mock.patch.object(neutron_common, 'add_ports_to_network') def test_add_provisioning_network(self, add_ports_mock, rollback_mock, @@ -110,13 +112,13 @@ class NeutronInterfaceTestCase(db_base.DbTestCase): security_groups=[]) validate_mock.assert_called_once_with( CONF.neutron.provisioning_network, - 'provisioning network') + 'provisioning network', context=task.context) self.port.refresh() self.assertEqual(self.neutron_port['id'], self.port.internal_info['provisioning_vif_port_id']) @mock.patch.object(neutron_common, 'validate_network', - lambda n, t: n) + lambda n, t, context=None: n) @mock.patch.object(neutron_common, 'rollback_ports') @mock.patch.object(neutron_common, 'add_ports_to_network') def test_add_provisioning_network_with_sg(self, add_ports_mock, @@ -141,7 +143,7 @@ class NeutronInterfaceTestCase(db_base.DbTestCase): self.port.internal_info['provisioning_vif_port_id']) @mock.patch.object(neutron_common, 'validate_network', - side_effect=lambda n, t: n) + side_effect=lambda n, t, context=None: n) @mock.patch.object(neutron_common, 'remove_ports_from_network') def test_remove_provisioning_network(self, remove_ports_mock, validate_mock): @@ -153,12 +155,12 @@ class NeutronInterfaceTestCase(db_base.DbTestCase): task, CONF.neutron.provisioning_network) validate_mock.assert_called_once_with( CONF.neutron.provisioning_network, - 'provisioning network') + 'provisioning network', context=task.context) self.port.refresh() self.assertNotIn('provisioning_vif_port_id', self.port.internal_info) @mock.patch.object(neutron_common, 'validate_network', - side_effect=lambda n, t: n) + side_effect=lambda n, t, context=None: n) @mock.patch.object(neutron_common, 'rollback_ports') @mock.patch.object(neutron_common, 'add_ports_to_network') def test_add_cleaning_network(self, add_ports_mock, rollback_mock, @@ -171,13 +173,13 @@ class NeutronInterfaceTestCase(db_base.DbTestCase): self.assertEqual(res, add_ports_mock.return_value) validate_mock.assert_called_once_with( CONF.neutron.cleaning_network, - 'cleaning network') + 'cleaning network', context=task.context) self.port.refresh() self.assertEqual(self.neutron_port['id'], self.port.internal_info['cleaning_vif_port_id']) @mock.patch.object(neutron_common, 'validate_network', - lambda n, t: n) + lambda n, t, context=None: n) @mock.patch.object(neutron_common, 'rollback_ports') @mock.patch.object(neutron_common, 'add_ports_to_network') def test_add_cleaning_network_with_sg(self, add_ports_mock, rollback_mock): @@ -199,7 +201,7 @@ class NeutronInterfaceTestCase(db_base.DbTestCase): self.port.internal_info['cleaning_vif_port_id']) @mock.patch.object(neutron_common, 'validate_network', - side_effect=lambda n, t: n) + side_effect=lambda n, t, context=None: n) @mock.patch.object(neutron_common, 'remove_ports_from_network') def test_remove_cleaning_network(self, remove_ports_mock, validate_mock): @@ -211,7 +213,7 @@ class NeutronInterfaceTestCase(db_base.DbTestCase): task, CONF.neutron.cleaning_network) validate_mock.assert_called_once_with( CONF.neutron.cleaning_network, - 'cleaning network') + 'cleaning network', context=task.context) self.port.refresh() self.assertNotIn('cleaning_vif_port_id', self.port.internal_info) @@ -220,7 +222,7 @@ class NeutronInterfaceTestCase(db_base.DbTestCase): with task_manager.acquire(self.context, self.node.id) as task: self.interface.unconfigure_tenant_networks(task) mock_unbind_port.assert_called_once_with( - self.port.extra['vif_port_id']) + self.port.extra['vif_port_id'], context=task.context) def test_configure_tenant_networks_no_ports_for_node(self): n = utils.create_test_node(self.context, network_interface='neutron', @@ -243,7 +245,7 @@ class NeutronInterfaceTestCase(db_base.DbTestCase): 'associated with node', self.interface.configure_tenant_networks, task) - client_mock.assert_called_once_with() + client_mock.assert_called_once_with(context=task.context) upd_mock.assert_not_called() self.assertIn('No neutron ports or portgroups are associated with', log_mock.error.call_args[0][0]) @@ -267,7 +269,7 @@ class NeutronInterfaceTestCase(db_base.DbTestCase): client_mock.return_value.update_port = upd_mock with task_manager.acquire(self.context, self.node.id) as task: self.interface.configure_tenant_networks(task) - client_mock.assert_called_once_with() + client_mock.assert_called_once_with(context=task.context) upd_mock.assert_called_once_with(self.port.extra['vif_port_id'], expected_body) @@ -280,7 +282,7 @@ class NeutronInterfaceTestCase(db_base.DbTestCase): self.assertRaisesRegex( exception.NetworkError, 'Could not add', self.interface.configure_tenant_networks, task) - client_mock.assert_called_once_with() + client_mock.assert_called_once_with(context=task.context) @mock.patch.object(neutron_common, 'get_client') def _test_configure_tenant_networks(self, client_mock, is_client_id=False, @@ -333,7 +335,7 @@ class NeutronInterfaceTestCase(db_base.DbTestCase): [{'opt_name': '61', 'opt_value': client_ids[1]}]) with task_manager.acquire(self.context, self.node.id) as task: self.interface.configure_tenant_networks(task) - client_mock.assert_called_once_with() + client_mock.assert_called_once_with(context=task.context) if vif_int_info: portid1 = self.port.internal_info['tenant_vif_port_id'] portid2 = second_port.internal_info['tenant_vif_port_id'] @@ -414,7 +416,7 @@ class NeutronInterfaceTestCase(db_base.DbTestCase): # this portgroup object. task.portgroups = [pg] self.interface.configure_tenant_networks(task) - client_mock.assert_called_once_with() + client_mock.assert_called_once_with(context=task.context) glgi_mock.assert_called_once_with(task, pg) upd_mock.assert_has_calls( [mock.call(self.port.extra['vif_port_id'], call1_body), diff --git a/releasenotes/notes/deprecated-neutron-opts-2e1d9e65f00301d3.yaml b/releasenotes/notes/deprecated-neutron-opts-2e1d9e65f00301d3.yaml new file mode 100644 index 000000000..dd280e24b --- /dev/null +++ b/releasenotes/notes/deprecated-neutron-opts-2e1d9e65f00301d3.yaml @@ -0,0 +1,49 @@ +--- +deprecations: + - | + Configuration option ``[neutron]/url`` is deprecated + and will be ignored in the Rocky release. + Instead, use ``[neutron]/endpoint_override`` configuration option to set + specific neutron API address when automatic discovery of neutron API + endpoint from keystone catalog is not desired. + This option has no default value, and must be set explicitly + for a stand alone deployment of ironic and neutron + (when ``[neutron]/auth_type`` is set to ``none``), since the + service catalog is not available in this case. + Otherwise it is generally recommended to rely on keystone service catalog + for service endpoint discovery. + + - | + Configuration option ``[neutron]/url_timeout`` is deprecated + and will be ignored in the Rocky release. + Instead, use ``[neutron]/timeout`` configuration option. + This new option has no default value and must be set explicitly + to ``30`` to keep previous default behavior. + + - | + Configuration option ``[neutron]/auth_strategy`` is deprecated + and will be ignored in the Rocky release. + Instead, set ``[neutron]/auth_type`` configuration option to ``none``, + and provide neutron API address as ``[neutron]/endpoint_override`` + configuration option. + +other: + - | + Signatures of several networking-related functions/methods have been + changed to include request context as an optional keyword argument. + + The functions/methods in question are: + + - ``ironic.common.neutron.get_client`` + - ``ironic.common.neutron.unbind_neutron_port`` + - ``ironic.common.neutron.update_port_address`` + - ``ironic.common.neutron.validate_network`` + - ``ironic.common.neutron.NeutronNetworkInterfaceMixin.get_cleaning_network`` + - ``ironic.common.neutron.NeutronNetworkInterfaceMixin.get_provisioning_network`` + - ``ironic.dhcp.neutron.NeutronDHCPApi.update_port_dhcp_opts`` + - ``ironic.dhcp.none.NeutronDHCPApi.update_port_dhcp_opts`` + + If you are using any of the above functions/methods in your out-of-tree + ironic driver or driver interface code, you should update the code + to pass an instance of ``ironic.common.context.RequestContext`` class + as a ``context`` keyword argument to those functions/methods. diff --git a/releasenotes/notes/keystoneauth-config-1baa45a0a2dd93b4.yaml b/releasenotes/notes/keystoneauth-config-1baa45a0a2dd93b4.yaml index 81a3f0753..5d9ad4d4c 100644 --- a/releasenotes/notes/keystoneauth-config-1baa45a0a2dd93b4.yaml +++ b/releasenotes/notes/keystoneauth-config-1baa45a0a2dd93b4.yaml @@ -16,3 +16,7 @@ features: Adds the ability to set keystoneauth settings in the ``[swift]`` configuration section for service automatic discovery. + - | + Adds the ability to set keystoneauth settings in the + ``[neutron]`` configuration section for service automatic + discovery. |