From 6325b6c13c82911cb144646a614b943bdf496518 Mon Sep 17 00:00:00 2001 From: Hamdy Khader Date: Sun, 13 Jan 2019 15:40:30 +0200 Subject: [PATCH] Expose is_smartnic in port API Changes are made to support ironic handling is_smarting port attribute that was added in this change Ic2ffbd6f1035907ea5a18bda6d2b21e617194195 This change expose is_smartnic port field in REST API, updated API reference to include is_smartnic field and relevent documentations. API version updated to 1.53. Story: #2003346 Change-Id: I89ce54a0c034f2a5f82ff961ab06cfcc6d853bd4 --- api-ref/regenerate-samples.sh | 2 +- .../source/baremetal-api-v1-nodes-ports.inc | 7 ++ .../baremetal-api-v1-portgroups-ports.inc | 7 ++ api-ref/source/baremetal-api-v1-ports.inc | 20 ++++ api-ref/source/parameters.yaml | 12 +++ .../samples/node-port-detail-response.json | 1 + .../source/samples/port-create-request.json | 1 + .../source/samples/port-create-response.json | 1 + .../samples/port-list-detail-response.json | 1 + .../source/samples/port-update-response.json | 1 + .../portgroup-port-detail-response.json | 1 + doc/source/admin/multitenancy.rst | 26 ++++- doc/source/admin/notifications.rst | 3 +- .../contributor/webapi-version-history.rst | 7 ++ ironic/api/controllers/v1/port.py | 21 ++++- ironic/api/controllers/v1/types.py | 58 ++++++++++-- ironic/api/controllers/v1/utils.py | 10 ++ ironic/api/controllers/v1/versions.py | 4 +- ironic/common/neutron.py | 18 +++- ironic/common/release_mappings.py | 2 +- .../unit/api/controllers/v1/test_port.py | 94 ++++++++++++++++++- .../unit/api/controllers/v1/test_types.py | 32 ++++++- .../unit/api/controllers/v1/test_utils.py | 7 ++ ironic/tests/unit/api/utils.py | 3 - ironic/tests/unit/common/test_neutron.py | 85 +++++++++++++++-- ...add-port-is-smartnic-4ce6974c8fe2732d.yaml | 21 +++++ 26 files changed, 409 insertions(+), 36 deletions(-) create mode 100644 releasenotes/notes/add-port-is-smartnic-4ce6974c8fe2732d.yaml diff --git a/api-ref/regenerate-samples.sh b/api-ref/regenerate-samples.sh index aebcd5f78f..7fad46fed5 100755 --- a/api-ref/regenerate-samples.sh +++ b/api-ref/regenerate-samples.sh @@ -11,7 +11,7 @@ fi OS_AUTH_TOKEN=$(openstack token issue | grep ' id ' | awk '{print $4}') IRONIC_URL="http://127.0.0.1:6385" -IRONIC_API_VERSION="1.37" +IRONIC_API_VERSION="1.53" export OS_AUTH_TOKEN IRONIC_URL diff --git a/api-ref/source/baremetal-api-v1-nodes-ports.inc b/api-ref/source/baremetal-api-v1-nodes-ports.inc index 40a7d5e9d7..9c8a0386d3 100644 --- a/api-ref/source/baremetal-api-v1-nodes-ports.inc +++ b/api-ref/source/baremetal-api-v1-nodes-ports.inc @@ -32,6 +32,9 @@ Return a list of bare metal Ports associated with ``node_ident``. .. versionadded:: 1.34 Added the ``physical_network`` field. +.. versionadded:: 1.53 + Added the ``is_smartnic`` response fields. + Normal response code: 200 Error codes: TBD @@ -79,6 +82,9 @@ Return a detailed list of bare metal Ports associated with ``node_ident``. .. versionadded:: 1.34 Added the ``physical_network`` field. +.. versionadded:: 1.53 + Added the ``is_smartnic`` response fields. + Normal response code: 200 Error codes: TBD @@ -112,6 +118,7 @@ Response - created_at: created_at - updated_at: updated_at - links: links + - is_smartnic: is_smartnic **Example details of a Node's Ports:** diff --git a/api-ref/source/baremetal-api-v1-portgroups-ports.inc b/api-ref/source/baremetal-api-v1-portgroups-ports.inc index 1f4f2ed367..073bdfde13 100644 --- a/api-ref/source/baremetal-api-v1-portgroups-ports.inc +++ b/api-ref/source/baremetal-api-v1-portgroups-ports.inc @@ -25,6 +25,9 @@ Response to include only the specified fields, rather than the default set. .. versionadded:: 1.34 Added the ``physical_network`` field. +.. versionadded:: 1.53 + Added the ``is_smartnic`` response fields. + Normal response code: 200 Error codes: 400,401,403,404 @@ -66,6 +69,9 @@ Return a detailed list of bare metal Ports associated with ``portgroup_ident``. .. versionadded:: 1.34 Added the ``physical_network`` field. +.. versionadded:: 1.53 + Added the ``is_smartnic`` response fields. + Normal response code: 200 Error codes: 400,401,403,404 @@ -99,6 +105,7 @@ Response - created_at: created_at - updated_at: updated_at - links: links + - is_smartnic: is_smartnic **Example details of a Portgroup's Ports:** diff --git a/api-ref/source/baremetal-api-v1-ports.inc b/api-ref/source/baremetal-api-v1-ports.inc index de93d15e53..b7ea31c903 100644 --- a/api-ref/source/baremetal-api-v1-ports.inc +++ b/api-ref/source/baremetal-api-v1-ports.inc @@ -46,6 +46,9 @@ By default, this query will return the uuid and address for each Port. Added the ``detail`` boolean request parameter. When specified ``True`` this causes the response to include complete details about each port. +.. versionadded:: 1.53 + Added the ``is_smartnic`` field. + Normal response code: 200 Request @@ -100,6 +103,9 @@ This method requires a Node UUID and the physical hardware address for the Port .. versionadded:: 1.34 Added the ``physical_network`` request and response fields. +.. versionadded:: 1.53 + Added the ``is_smartnic`` request and response fields. + Normal response code: 201 Request @@ -114,6 +120,7 @@ Request - pxe_enabled: req_pxe_enabled - physical_network: req_physical_network - extra: req_extra + - is_smartnic: req_is_smartnic **Example Port creation request:** @@ -137,6 +144,7 @@ Response - created_at: created_at - updated_at: updated_at - links: links + - is_smartnic: is_smartnic **Example Port creation response:** @@ -165,6 +173,9 @@ Return a list of bare metal Ports, with detailed information. .. versionadded:: 1.34 Added the ``physical_network`` response field. +.. versionadded:: 1.53 + Added the ``is_smartnic`` response fields. + Normal response code: 200 Request @@ -199,6 +210,7 @@ Response - created_at: created_at - updated_at: updated_at - links: links + - is_smartnic: is_smartnic **Example detailed Port list response:** @@ -227,6 +239,9 @@ Show details for the given Port. .. versionadded:: 1.34 Added the ``physical_network`` response field. +.. versionadded:: 1.53 + Added the ``is_smartnic`` response fields. + Normal response code: 200 Request @@ -254,6 +269,7 @@ Response - created_at: created_at - updated_at: updated_at - links: links + - is_smartnic: is_smartnic **Example Port details:** @@ -277,6 +293,9 @@ Update a Port. .. versionadded:: 1.34 Added the ``physical_network`` field. +.. versionadded:: 1.53 + Added the ``is_smartnic`` fields. + Normal response code: 200 Request @@ -311,6 +330,7 @@ Response - created_at: created_at - updated_at: updated_at - links: links + - is_smartnic: is_smartnic **Example Port update response:** diff --git a/api-ref/source/parameters.yaml b/api-ref/source/parameters.yaml index 404d0ad35d..65be25aea1 100644 --- a/api-ref/source/parameters.yaml +++ b/api-ref/source/parameters.yaml @@ -727,6 +727,12 @@ internal_info: in: body required: true type: JSON +is_smartnic: + description: | + Indicates whether the Port is a Smart NIC port. + in: body + required: false + type: boolean last_error: description: | Any error from the most recent (last) transaction that started but failed to finish. @@ -1133,6 +1139,12 @@ req_inspect_interface: in: body required: false type: string +req_is_smartnic: + description: | + Indicates whether the Port is a Smart NIC port. + in: body + required: false + type: boolean req_local_link_connection: description: | The Port binding profile. If specified, must contain ``switch_id`` (only diff --git a/api-ref/source/samples/node-port-detail-response.json b/api-ref/source/samples/node-port-detail-response.json index e0f4292bd5..4b7a657999 100644 --- a/api-ref/source/samples/node-port-detail-response.json +++ b/api-ref/source/samples/node-port-detail-response.json @@ -5,6 +5,7 @@ "created_at": "2016-08-18T22:28:48.643434+11:11", "extra": {}, "internal_info": {}, + "is_smartnic": true, "links": [ { "href": "http://127.0.0.1:6385/v1/ports/d2b30520-907d-46c8-bfee-c5586e6fb3a1", diff --git a/api-ref/source/samples/port-create-request.json b/api-ref/source/samples/port-create-request.json index 8018f8dec7..22b6c7ef0d 100644 --- a/api-ref/source/samples/port-create-request.json +++ b/api-ref/source/samples/port-create-request.json @@ -2,6 +2,7 @@ "node_uuid": "6d85703a-565d-469a-96ce-30b6de53079d", "portgroup_uuid": "e43c722c-248e-4c6e-8ce8-0d8ff129387a", "address": "11:11:11:11:11:11", + "is_smartnic": true, "local_link_connection": { "switch_id": "0a:1b:2c:3d:4e:5f", "port_id": "Ethernet3/1", diff --git a/api-ref/source/samples/port-create-response.json b/api-ref/source/samples/port-create-response.json index ef88e965f1..67c5cfd16f 100644 --- a/api-ref/source/samples/port-create-response.json +++ b/api-ref/source/samples/port-create-response.json @@ -3,6 +3,7 @@ "created_at": "2016-08-18T22:28:48.643434+11:11", "extra": {}, "internal_info": {}, + "is_smartnic": true, "links": [ { "href": "http://127.0.0.1:6385/v1/ports/d2b30520-907d-46c8-bfee-c5586e6fb3a1", diff --git a/api-ref/source/samples/port-list-detail-response.json b/api-ref/source/samples/port-list-detail-response.json index aa99ef7436..b62d6bda7a 100644 --- a/api-ref/source/samples/port-list-detail-response.json +++ b/api-ref/source/samples/port-list-detail-response.json @@ -5,6 +5,7 @@ "created_at": "2016-08-18T22:28:48.643434+11:11", "extra": {}, "internal_info": {}, + "is_smartnic": true, "links": [ { "href": "http://127.0.0.1:6385/v1/ports/d2b30520-907d-46c8-bfee-c5586e6fb3a1", diff --git a/api-ref/source/samples/port-update-response.json b/api-ref/source/samples/port-update-response.json index e0a1e07444..5aacd1b76f 100644 --- a/api-ref/source/samples/port-update-response.json +++ b/api-ref/source/samples/port-update-response.json @@ -3,6 +3,7 @@ "created_at": "2016-08-18T22:28:48.643434+11:11", "extra": {}, "internal_info": {}, + "is_smartnic": true, "links": [ { "href": "http://127.0.0.1:6385/v1/ports/d2b30520-907d-46c8-bfee-c5586e6fb3a1", diff --git a/api-ref/source/samples/portgroup-port-detail-response.json b/api-ref/source/samples/portgroup-port-detail-response.json index e0f4292bd5..4b7a657999 100644 --- a/api-ref/source/samples/portgroup-port-detail-response.json +++ b/api-ref/source/samples/portgroup-port-detail-response.json @@ -5,6 +5,7 @@ "created_at": "2016-08-18T22:28:48.643434+11:11", "extra": {}, "internal_info": {}, + "is_smartnic": true, "links": [ { "href": "http://127.0.0.1:6385/v1/ports/d2b30520-907d-46c8-bfee-c5586e6fb3a1", diff --git a/doc/source/admin/multitenancy.rst b/doc/source/admin/multitenancy.rst index 8d40bf498e..ab48680e25 100644 --- a/doc/source/admin/multitenancy.rst +++ b/doc/source/admin/multitenancy.rst @@ -58,16 +58,22 @@ network. - Required. Identifies a switch and can be a MAC address or an OpenFlow-based ``datapath_id``. * - ``port_id`` - - Required. Port ID on the switch, for example, Gig0/1. + - Required. Port ID on the switch/Smart NIC, for example, Gig0/1, rep0-0. * - ``switch_info`` - Optional. Used to distinguish different switch models or other vendor-specific identifier. Some ML2 plugins may require this field. - + * - ``hostname`` + - Required in case of a Smart NIC port. + Hostname of Smart NIC device. .. note:: This isn't applicable to Infiniband ports because the network topology is discoverable by the Infiniband Subnet Manager. If specified, local_link_connection information will be ignored. + If port is Smart NIC port then: + + 1. ``port_id`` is the representor port name on the Smart NIC. + 2. ``switch_id`` is not mandatory. .. _multitenancy-physnets: @@ -113,8 +119,11 @@ Configuring nodes * Physical network support for ironic ports was added in API version 1.34, and is supported by python-ironicclient version 1.15.0 or higher. + * Smart NIC support for ironic ports was added in API version 1.53, + and is supported by python-ironicclient version 2.7.0 or higher. + The following examples assume you are using python-ironicclient version - 1.15.0 or higher. + 2.7.0 or higher. Export the following variable:: @@ -165,6 +174,17 @@ Configuring nodes --extra client-id=$CLIENT_ID \ --physical-network physnet1 +#. Create a Smart NIC port as follows:: + + openstack baremetal port create $HW_MAC_ADDRESS --node $NODE_UUID \ + --local-link-connection hostname=$HOSTNAME \ + --local-link-connection port_id=$REP_NAME \ + --pxe-enabled true \ + --physical-network physnet1 \ + --is-smartnic true + + A Smart NIC port requires ``hostname`` which is the hostname of the Smart NIC, + and ``port_id`` which is the representor port name within the Smart NIC. #. Check the port configuration:: diff --git a/doc/source/admin/notifications.rst b/doc/source/admin/notifications.rst index 1196b64af4..f9b5397906 100644 --- a/doc/source/admin/notifications.rst +++ b/doc/source/admin/notifications.rst @@ -211,12 +211,13 @@ Example of port CRUD notification:: "payload":{ "ironic_object.namespace":"ironic", "ironic_object.name":"PortCRUDPayload", - "ironic_object.version":"1.2", + "ironic_object.version":"1.3", "ironic_object.data":{ "address": "77:66:23:34:11:b7", "created_at": "2016-02-11T15:23:03+00:00", "node_uuid": "5b236cab-ad4e-4220-b57c-e827e858745a", "extra": {}, + "is_smartnic": True, "local_link_connection": {}, "physical_network": "physnet1", "portgroup_uuid": "bd2f385e-c51c-4752-82d1-7a9ec2c25f24", diff --git a/doc/source/contributor/webapi-version-history.rst b/doc/source/contributor/webapi-version-history.rst index dd76f120cf..e16cf29993 100644 --- a/doc/source/contributor/webapi-version-history.rst +++ b/doc/source/contributor/webapi-version-history.rst @@ -2,6 +2,13 @@ REST API Version History ======================== +1.53 (Stein, master) +-------------------- + +Added ``is_smartnic`` field to the port object to enable Smart NIC port +creation in addition to local link connection attributes ``port_id`` and +``hostname``. + 1.52 (Stein, master) -------------------- diff --git a/ironic/api/controllers/v1/port.py b/ironic/api/controllers/v1/port.py index b72f0ddbf5..c16ff72315 100644 --- a/ironic/api/controllers/v1/port.py +++ b/ironic/api/controllers/v1/port.py @@ -59,6 +59,9 @@ def hide_fields_in_newer_versions(obj): # if requested version is < 1.34, hide physical_network field. if not api_utils.allow_port_physical_network(): obj.physical_network = wsme.Unset + # if requested version is < 1.53, hide is_smartnic field. + if not api_utils.allow_port_is_smartnic(): + obj.is_smartnic = wsme.Unset class Port(base.APIBase): @@ -156,6 +159,9 @@ class Port(base.APIBase): links = wsme.wsattr([link.Link], readonly=True) """A list containing a self link and associated port links""" + is_smartnic = types.boolean + """Indicates whether this port is a Smart NIC port.""" + def __init__(self, **kwargs): self.fields = [] fields = list(objects.Port.fields) @@ -245,7 +251,8 @@ class Port(base.APIBase): local_link_connection={ 'switch_info': 'host', 'port_id': 'Gig0/1', 'switch_id': 'aa:bb:cc:dd:ee:ff'}, - physical_network='physnet1') + physical_network='physnet1', + is_smartnic=False) # NOTE(lucasagomes): node_uuid getter() method look at the # _node_uuid variable sample._node_uuid = '7ae81bb3-dec3-4289-8d6c-da80bd8001ae' @@ -425,6 +432,9 @@ class PortsController(rest.RestController): if ('physical_network' in fields and not api_utils.allow_port_physical_network()): raise exception.NotAcceptable() + if ('is_smartnic' in fields + and not api_utils.allow_port_is_smartnic()): + raise exception.NotAcceptable() @METRICS.timer('PortsController.get_all') @expose.expose(PortCollection, types.uuid_or_name, types.uuid, @@ -577,6 +587,12 @@ class PortsController(rest.RestController): pdict = port.as_dict() self._check_allowed_port_fields(pdict) + if (port.is_smartnic and not types.locallinkconnectiontype + .validate_for_smart_nic(port.local_link_connection)): + raise exception.Invalid( + "Smart NIC port must have port_id " + "and hostname in local_link_connection") + create_remotely = pecan.request.rpcapi.can_send_create_port() if (not create_remotely and pdict.get('portgroup_uuid')): # NOTE(mgoddard): In RPC API v1.41, port creation was moved to the @@ -652,7 +668,8 @@ class PortsController(rest.RestController): fields_to_check = set() for field in (self.advanced_net_fields - + ['portgroup_uuid', 'physical_network']): + + ['portgroup_uuid', 'physical_network', + 'is_smartnic']): field_path = '/%s' % field if (api_utils.get_patch_values(patch, field_path) or api_utils.is_path_removed(patch, field_path)): diff --git a/ironic/api/controllers/v1/types.py b/ironic/api/controllers/v1/types.py index 87f5749540..e2b04bd728 100644 --- a/ironic/api/controllers/v1/types.py +++ b/ironic/api/controllers/v1/types.py @@ -18,6 +18,7 @@ import inspect import json +from oslo_log import log from oslo_utils import strutils from oslo_utils import uuidutils import six @@ -30,6 +31,9 @@ from ironic.common.i18n import _ from ironic.common import utils +LOG = log.getLogger(__name__) + + class MacAddressType(wtypes.UserType): """A simple MAC address type.""" @@ -266,9 +270,12 @@ class LocalLinkConnectionType(wtypes.UserType): basetype = wtypes.DictType name = 'locallinkconnection' - mandatory_fields = {'switch_id', - 'port_id'} - valid_fields = mandatory_fields.union({'switch_info'}) + local_link_mandatory_fields = {'port_id', 'switch_id'} + smart_nic_mandatory_fields = {'port_id', 'hostname'} + mandatory_fields_list = [local_link_mandatory_fields, + smart_nic_mandatory_fields] + optional_field = {'switch_info'} + valid_fields = set.union(optional_field, *mandatory_fields_list) @staticmethod def validate(value): @@ -276,7 +283,7 @@ class LocalLinkConnectionType(wtypes.UserType): :param value: A dictionary of values to validate, switch_id is a MAC address or an OpenFlow based datapath_id, switch_info is an - optional field. + optional field. Required Smart NIC fields are port_id and hostname. For example:: @@ -286,6 +293,13 @@ class LocalLinkConnectionType(wtypes.UserType): 'switch_info': 'switch1' } + Or for Smart NIC:: + + { + 'port_id': 'rep0-0', + 'hostname': 'host1-bf' + } + :returns: A dictionary. :raises: Invalid if some of the keys in the dictionary being validated are unknown, invalid, or some required ones are missing. @@ -304,10 +318,20 @@ class LocalLinkConnectionType(wtypes.UserType): if invalid: raise exception.Invalid(_('%s are invalid keys') % (invalid)) - # Check all mandatory fields are present - missing = LocalLinkConnectionType.mandatory_fields - keys - if missing: - msg = _('Missing mandatory keys: %s') % missing + # Check any mandatory fields sets are present + for mandatory_set in LocalLinkConnectionType.mandatory_fields_list: + if mandatory_set <= keys: + break + else: + msg = _('Missing mandatory keys. Required keys are ' + '%(required_fields)s. Or in case of Smart NIC ' + '%(smart_nic_required_fields)s. ' + 'Submitted keys are %(keys)s .') % { + 'required_fields': + LocalLinkConnectionType.local_link_mandatory_fields, + 'smart_nic_required_fields': + LocalLinkConnectionType.smart_nic_mandatory_fields, + 'keys': keys} raise exception.Invalid(msg) # Check switch_id is either a valid mac address or @@ -321,6 +345,9 @@ class LocalLinkConnectionType(wtypes.UserType): value['switch_id']) except exception.InvalidDatapathID: raise exception.InvalidSwitchID(switch_id=value['switch_id']) + except KeyError: + # In Smart NIC case 'switch_id' is optional. + pass return value @@ -330,6 +357,21 @@ class LocalLinkConnectionType(wtypes.UserType): return None return LocalLinkConnectionType.validate(value) + @staticmethod + def validate_for_smart_nic(value): + """Validates Smart NIC field are present 'port_id' and 'hostname' + + :param value: local link information of type Dictionary. + :return: True if both fields 'port_id' and 'hostname' are present + in 'value', False otherwise. + """ + wtypes.DictType(wtypes.text, wtypes.text).validate(value) + keys = set(value) + + if LocalLinkConnectionType.smart_nic_mandatory_fields <= keys: + return True + return False + locallinkconnectiontype = LocalLinkConnectionType() diff --git a/ironic/api/controllers/v1/utils.py b/ironic/api/controllers/v1/utils.py index 770dec62dd..f2c7a5ec94 100644 --- a/ironic/api/controllers/v1/utils.py +++ b/ironic/api/controllers/v1/utils.py @@ -1012,3 +1012,13 @@ def allow_allocations(): field for the node. """ return pecan.request.version.minor >= versions.MINOR_52_ALLOCATION + + +def allow_port_is_smartnic(): + """Check if port is_smartnic field is allowed. + + Version 1.53 of the API added is_smartnic field to the port object. + """ + return ((pecan.request.version.minor + >= versions.MINOR_53_PORT_SMARTNIC) + and objects.Port.supports_is_smartnic()) diff --git a/ironic/api/controllers/v1/versions.py b/ironic/api/controllers/v1/versions.py index d456738de8..5c5da878eb 100644 --- a/ironic/api/controllers/v1/versions.py +++ b/ironic/api/controllers/v1/versions.py @@ -90,6 +90,7 @@ BASE_VERSION = 1 # v1.50: Add owner to the node object. # v1.51: Add description to the node object. # v1.52: Add allocation API. +# v1.53: Add support for Smart NIC port MINOR_0_JUNO = 0 MINOR_1_INITIAL_VERSION = 1 @@ -144,6 +145,7 @@ MINOR_49_CONDUCTORS = 49 MINOR_50_NODE_OWNER = 50 MINOR_51_NODE_DESCRIPTION = 51 MINOR_52_ALLOCATION = 52 +MINOR_53_PORT_SMARTNIC = 53 # When adding another version, update: # - MINOR_MAX_VERSION @@ -151,7 +153,7 @@ MINOR_52_ALLOCATION = 52 # explanation of what changed in the new version # - common/release_mappings.py, RELEASE_MAPPING['master']['api'] -MINOR_MAX_VERSION = MINOR_52_ALLOCATION +MINOR_MAX_VERSION = MINOR_53_PORT_SMARTNIC # String representations of the minor and maximum versions _MIN_VERSION_STRING = '{}.{}'.format(BASE_VERSION, MINOR_1_INITIAL_VERSION) diff --git a/ironic/common/neutron.py b/ironic/common/neutron.py index f365fb1a8e..0a4759d292 100644 --- a/ironic/common/neutron.py +++ b/ironic/common/neutron.py @@ -16,6 +16,7 @@ from oslo_log import log from oslo_utils import uuidutils import retrying +from ironic.api.controllers.v1 import types from ironic.common import context as ironic_context from ironic.common import exception from ironic.common.i18n import _ @@ -259,9 +260,10 @@ def add_ports_to_network(task, network_uuid, security_groups=None): binding_profile = {'local_link_information': [portmap[ironic_port.uuid]]} body['port']['binding:profile'] = binding_profile - link_info = binding_profile['local_link_information'][0] + is_smart_nic = is_smartnic_port(ironic_port) if is_smart_nic: + link_info = binding_profile['local_link_information'][0] LOG.debug('Setting hostname as host_id in case of Smart NIC, ' 'port %(port_id)s, hostname %(hostname)s', {'port_id': ironic_port.uuid, @@ -504,11 +506,21 @@ def validate_port_info(node, port): "in the nodes %(node)s port %(port)s", {'node': node.uuid, 'port': port.uuid}) return False + if (port.is_smartnic and not types.locallinkconnectiontype + .validate_for_smart_nic(port.local_link_connection)): + LOG.error("Smart NIC port must have port_id and hostname in " + "local_link_connection, port: %s", port['id']) + return False + if (not port.is_smartnic and types.locallinkconnectiontype + .validate_for_smart_nic(port.local_link_connection)): + LOG.error("Only Smart NIC ports can have port_id and hostname " + "in local_link_connection, port: %s", port['id']) + return False return True -def validate_agent(client, **kwargs): +def _validate_agent(client, **kwargs): """Check that the given neutron agent is alive :param client: Neutron client @@ -670,7 +682,7 @@ def wait_for_host_agent(client, host_id, target_state='up'): LOG.debug('Validating host %(host_id)s agent is %(status)s', {'host_id': host_id, 'status': target_state}) - is_alive = validate_agent(client, host=host_id) + is_alive = _validate_agent(client, host=host_id) LOG.debug('Agent on host %(host_id)s is %(status)s', {'host_id': host_id, 'status': 'up' if is_alive else 'down'}) diff --git a/ironic/common/release_mappings.py b/ironic/common/release_mappings.py index ad8ed48c45..3688be00ca 100644 --- a/ironic/common/release_mappings.py +++ b/ironic/common/release_mappings.py @@ -131,7 +131,7 @@ RELEASE_MAPPING = { } }, 'master': { - 'api': '1.51', + 'api': '1.53', 'rpc': '1.48', 'objects': { 'Allocation': ['1.0'], diff --git a/ironic/tests/unit/api/controllers/v1/test_port.py b/ironic/tests/unit/api/controllers/v1/test_port.py index 3a7013f168..2a81e7d0e1 100644 --- a/ironic/tests/unit/api/controllers/v1/test_port.py +++ b/ironic/tests/unit/api/controllers/v1/test_port.py @@ -351,6 +351,18 @@ class TestListPorts(test_api_base.BaseApiTest): headers={api_base.Version.string: "1.34"}) self.assertNotIn('physical_network', data) + def test_hide_fields_in_newer_versions_is_smartnic(self): + port = obj_utils.create_test_port(self.context, node_id=self.node.id, + is_smartnic=True) + data = self.get_json( + '/ports/%s' % port.uuid, + headers={api_base.Version.string: "1.52"}) + self.assertNotIn('is_smartnic', data) + + data = self.get_json('/ports/%s' % port.uuid, + headers={api_base.Version.string: "1.53"}) + self.assertTrue(data['is_smartnic']) + def test_get_collection_custom_fields(self): fields = 'uuid,extra' for i in range(3): @@ -436,6 +448,24 @@ class TestListPorts(test_api_base.BaseApiTest): expect_errors=True) self.assertEqual(http_client.NOT_ACCEPTABLE, response.status_int) + def test_get_custom_fields_is_smartnic(self): + port = obj_utils.create_test_port(self.context, node_id=self.node.id, + is_smartnic=True) + fields = 'uuid,is_smartnic' + response = self.get_json( + '/ports/%s?fields=%s' % (port.uuid, fields), + headers={api_base.Version.string: "1.52"}, + expect_errors=True) + self.assertEqual(http_client.NOT_ACCEPTABLE, response.status_int) + + response = self.get_json( + '/ports/%s?fields=%s' % (port.uuid, fields), + headers={api_base.Version.string: "1.53"}) + + # 'links' field is always retrieved in the response + # regardless of which fields are specified. + self.assertItemsEqual(['uuid', 'is_smartnic', 'links'], response) + def test_detail(self): llc = {'switch_info': 'switch', 'switch_id': 'aa:bb:cc:dd:ee:ff', 'port_id': 'Gig0/1'} @@ -445,7 +475,8 @@ class TestListPorts(test_api_base.BaseApiTest): portgroup_id=portgroup.id, pxe_enabled=False, local_link_connection=llc, - physical_network='physnet1') + physical_network='physnet1', + is_smartnic=True) data = self.get_json( '/ports/detail', headers={api_base.Version.string: str(api_v1.max_version())} @@ -458,6 +489,7 @@ class TestListPorts(test_api_base.BaseApiTest): self.assertIn('local_link_connection', data['ports'][0]) self.assertIn('portgroup_uuid', data['ports'][0]) self.assertIn('physical_network', data['ports'][0]) + self.assertIn('is_smartnic', data['ports'][0]) # never expose the node_id and portgroup_id self.assertNotIn('node_id', data['ports'][0]) self.assertNotIn('portgroup_id', data['ports'][0]) @@ -1680,6 +1712,7 @@ class TestPost(test_api_base.BaseApiTest): pdict.pop('pxe_enabled') pdict.pop('extra') pdict.pop('physical_network') + pdict.pop('is_smartnic') headers = {api_base.Version.string: str(api_v1.min_version())} response = self.post_json('/ports', pdict, headers=headers) self.assertEqual('application/json', response.content_type) @@ -2071,6 +2104,7 @@ class TestPost(test_api_base.BaseApiTest): pdict = post_get_test_port(pxe_enabled=False, extra={'vif_port_id': 'foo'}) pdict.pop('physical_network') + pdict.pop('is_smartnic') response = self.post_json('/ports', pdict, headers=headers) self.assertEqual('application/json', response.content_type) self.assertEqual(http_client.CREATED, response.status_int) @@ -2227,6 +2261,64 @@ class TestPost(test_api_base.BaseApiTest): self.assertIn('maximum character', response.json['error_message']) self.assertFalse(mock_create.called) + def test_create_port_with_is_smartnic(self, mock_create): + llc = {'hostname': 'host1', 'port_id': 'rep0-0'} + pdict = post_get_test_port(is_smartnic=True, node_uuid=self.node.uuid, + local_link_connection=llc) + response = self.post_json('/ports', pdict, headers=self.headers) + self.assertEqual('application/json', response.content_type) + self.assertEqual(http_client.CREATED, response.status_int) + mock_create.assert_called_once_with(mock.ANY, mock.ANY, mock.ANY, + 'test-topic') + self.assertTrue(response.json['is_smartnic']) + port = objects.Port.get(self.context, pdict['uuid']) + self.assertTrue(port.is_smartnic) + + def test_create_port_with_is_smartnic_default_value(self, mock_create): + pdict = post_get_test_port(node_uuid=self.node.uuid) + response = self.post_json('/ports', pdict, headers=self.headers) + self.assertEqual('application/json', response.content_type) + self.assertEqual(http_client.CREATED, response.status_int) + mock_create.assert_called_once_with(mock.ANY, mock.ANY, mock.ANY, + 'test-topic') + self.assertFalse(response.json['is_smartnic']) + port = objects.Port.get(self.context, pdict['uuid']) + self.assertFalse(port.is_smartnic) + + def test_create_port_with_is_smartnic_old_api_version(self, mock_create): + pdict = post_get_test_port(is_smartnic=True, node_uuid=self.node.uuid) + headers = {api_base.Version.string: '1.52'} + response = self.post_json('/ports', pdict, + headers=headers, + expect_errors=True) + self.assertEqual('application/json', response.content_type) + self.assertEqual(http_client.NOT_ACCEPTABLE, response.status_int) + self.assertFalse(mock_create.called) + + def test_create_port_with_is_smartnic_missing_hostname(self, mock_create): + llc = {'switch_info': 'switch', + 'switch_id': 'aa:bb:cc:dd:ee:ff', + 'port_id': 'Gig0/1'} + pdict = post_get_test_port(is_smartnic=True, + node_uuid=self.node.uuid, + local_link_connection=llc) + response = self.post_json('/ports', pdict, + headers=self.headers, expect_errors=True) + self.assertEqual(http_client.BAD_REQUEST, response.status_int) + self.assertFalse(mock_create.called) + + def test_create_port_with_is_smartnic_missing_port_id(self, mock_create): + llc = {'switch_info': 'switch', + 'switch_id': 'aa:bb:cc:dd:ee:ff', + 'hostname': 'host'} + pdict = post_get_test_port(is_smartnic=True, + node_uuid=self.node.uuid, + local_link_connection=llc) + response = self.post_json('/ports', pdict, + headers=self.headers, expect_errors=True) + self.assertEqual(http_client.BAD_REQUEST, response.status_int) + self.assertFalse(mock_create.called) + @mock.patch.object(rpcapi.ConductorAPI, 'destroy_port') class TestDelete(test_api_base.BaseApiTest): diff --git a/ironic/tests/unit/api/controllers/v1/test_types.py b/ironic/tests/unit/api/controllers/v1/test_types.py index 1b5cd85c43..7c85c363e8 100644 --- a/ironic/tests/unit/api/controllers/v1/test_types.py +++ b/ironic/tests/unit/api/controllers/v1/test_types.py @@ -323,14 +323,14 @@ class TestLocalLinkConnectionType(base.TestCase): self.assertRaisesRegex(exception.Invalid, 'are invalid keys', v.validate, value) - def test_local_link_connection_type_missing_mandatory_key(self): + def test_local_link_connection_type_missing_local_link_mandatory_key(self): v = types.locallinkconnectiontype value = {'switch_id': '0a:1b:2c:3d:4e:5f', 'switch_info': 'value3'} self.assertRaisesRegex(exception.Invalid, 'Missing mandatory', v.validate, value) - def test_local_link_connection_type_without_optional_key(self): + def test_local_link_connection_type_local_link_keys_mandatory(self): v = types.locallinkconnectiontype value = {'switch_id': '0a:1b:2c:3d:4e:5f', 'port_id': 'value2'} @@ -341,6 +341,34 @@ class TestLocalLinkConnectionType(base.TestCase): value = {} self.assertItemsEqual(value, v.validate(value)) + def test_local_link_connection_type_smart_nic_keys_mandatory(self): + v = types.locallinkconnectiontype + value = {'port_id': 'rep0-0', + 'hostname': 'hostname'} + self.assertTrue(v.validate_for_smart_nic(value)) + self.assertTrue(v.validate(value)) + + def test_local_link_connection_type_smart_nic_keys_with_optional(self): + v = types.locallinkconnectiontype + value = {'port_id': 'rep0-0', + 'hostname': 'hostname', + 'switch_id': '0a:1b:2c:3d:4e:5f', + 'switch_info': 'sw_info'} + self.assertTrue(v.validate_for_smart_nic(value)) + self.assertTrue(v.validate(value)) + + def test_local_link_connection_type_smart_nic_keys_hostname_missing(self): + v = types.locallinkconnectiontype + value = {'port_id': 'rep0-0'} + self.assertFalse(v.validate_for_smart_nic(value)) + self.assertRaises(exception.Invalid, v.validate, value) + + def test_local_link_connection_type_smart_nic_keys_port_id_missing(self): + v = types.locallinkconnectiontype + value = {'hostname': 'hostname'} + self.assertFalse(v.validate_for_smart_nic(value)) + self.assertRaises(exception.Invalid, v.validate, value) + @mock.patch("pecan.request", mock.Mock(version=mock.Mock(minor=10))) class TestVifType(base.TestCase): diff --git a/ironic/tests/unit/api/controllers/v1/test_utils.py b/ironic/tests/unit/api/controllers/v1/test_utils.py index 0629fbdb6e..7b6411feb3 100644 --- a/ironic/tests/unit/api/controllers/v1/test_utils.py +++ b/ironic/tests/unit/api/controllers/v1/test_utils.py @@ -523,6 +523,13 @@ class TestApiUtils(base.TestCase): mock_request.version.minor = 40 self.assertFalse(utils.allow_inspect_abort()) + @mock.patch.object(pecan, 'request', spec_set=['version']) + def test_allow_port_is_smartnic(self, mock_request): + mock_request.version.minor = 53 + self.assertTrue(utils.allow_port_is_smartnic()) + mock_request.version.minor = 52 + self.assertFalse(utils.allow_port_is_smartnic()) + class TestNodeIdent(base.TestCase): diff --git a/ironic/tests/unit/api/utils.py b/ironic/tests/unit/api/utils.py index 9f73a4b39e..ea6335099b 100644 --- a/ironic/tests/unit/api/utils.py +++ b/ironic/tests/unit/api/utils.py @@ -119,9 +119,6 @@ def port_post_data(**kw): port.pop('version') port.pop('node_id') port.pop('portgroup_id') - - # TODO(hamdyk): remove when port API can handle this attribute - port.pop('is_smartnic') internal = port_controller.PortPatchType.internal_attrs() return remove_internal(port, internal) diff --git a/ironic/tests/unit/common/test_neutron.py b/ironic/tests/unit/common/test_neutron.py index 63c0646b7e..1c8767e678 100644 --- a/ironic/tests/unit/common/test_neutron.py +++ b/ironic/tests/unit/common/test_neutron.py @@ -586,15 +586,57 @@ class TestNeutronNetworkActions(db_base.DbTestCase): self.assertTrue(res) self.assertFalse(log_mock.warning.called) + @mock.patch.object(neutron, 'LOG', autospec=True) + def test_validate_port_info_neutron_with_smartnic_and_link_info( + self, log_mock): + self.node.network_interface = 'neutron' + self.node.save() + llc = {'hostname': 'host1', 'port_id': 'rep0-0'} + port = object_utils.create_test_port( + self.context, node_id=self.node.id, uuid=uuidutils.generate_uuid(), + address='52:54:00:cf:2d:33', local_link_connection=llc, + is_smartnic=True) + res = neutron.validate_port_info(self.node, port) + self.assertTrue(res) + self.assertFalse(log_mock.error.called) + + @mock.patch.object(neutron, 'LOG', autospec=True) + def test_validate_port_info_neutron_with_no_smartnic_and_link_info( + self, log_mock): + self.node.network_interface = 'neutron' + self.node.save() + llc = {'hostname': 'host1', 'port_id': 'rep0-0'} + port = object_utils.create_test_port( + self.context, node_id=self.node.id, uuid=uuidutils.generate_uuid(), + address='52:54:00:cf:2d:33', local_link_connection=llc, + is_smartnic=False) + res = neutron.validate_port_info(self.node, port) + self.assertFalse(res) + self.assertTrue(log_mock.error.called) + + @mock.patch.object(neutron, 'LOG', autospec=True) + def test_validate_port_info_neutron_with_smartnic_and_no_link_info( + self, log_mock): + self.node.network_interface = 'neutron' + self.node.save() + llc = {'switch_id': 'switch', 'port_id': 'rep0-0'} + port = object_utils.create_test_port( + self.context, node_id=self.node.id, uuid=uuidutils.generate_uuid(), + address='52:54:00:cf:2d:33', local_link_connection=llc, + is_smartnic=True) + res = neutron.validate_port_info(self.node, port) + self.assertFalse(res) + self.assertTrue(log_mock.error.called) + def test_validate_agent_up(self): self.client_mock.list_agents.return_value = { 'agents': [{'alive': True}]} - self.assertTrue(neutron.validate_agent(self.client_mock)) + self.assertTrue(neutron._validate_agent(self.client_mock)) def test_validate_agent_down(self): self.client_mock.list_agents.return_value = { 'agents': [{'alive': False}]} - self.assertFalse(neutron.validate_agent(self.client_mock)) + self.assertFalse(neutron._validate_agent(self.client_mock)) def test_is_smartnic_port_true(self): port = self.ports[0] @@ -605,19 +647,42 @@ class TestNeutronNetworkActions(db_base.DbTestCase): port = self.ports[0] self.assertFalse(neutron.is_smartnic_port(port)) - @mock.patch.object(neutron, 'validate_agent') + @mock.patch.object(neutron, '_validate_agent') @mock.patch.object(time, 'sleep') - def test_wait_for_host_agent_up(self, sleep_mock, validate_agent_mock): + def test_wait_for_host_agent_up_target_state_up( + self, sleep_mock, validate_agent_mock): validate_agent_mock.return_value = True - neutron.wait_for_host_agent(self.client_mock, 'hostname') + self.assertTrue(neutron.wait_for_host_agent( + self.client_mock, 'hostname')) sleep_mock.assert_not_called() - @mock.patch.object(neutron, 'validate_agent') + @mock.patch.object(neutron, '_validate_agent') @mock.patch.object(time, 'sleep') - def test_wait_for_host_agent_down(self, sleep_mock, validate_agent_mock): - validate_agent_mock.side_effect = [False, True] - neutron.wait_for_host_agent(self.client_mock, 'hostname') - sleep_mock.assert_called_once() + def test_wait_for_host_agent_down_target_state_up( + self, sleep_mock, validate_agent_mock): + validate_agent_mock.return_value = False + self.assertRaises(exception.NetworkError, + neutron.wait_for_host_agent, + self.client_mock, 'hostname') + + @mock.patch.object(neutron, '_validate_agent') + @mock.patch.object(time, 'sleep') + def test_wait_for_host_agent_up_target_state_down( + self, sleep_mock, validate_agent_mock): + validate_agent_mock.return_value = True + self.assertRaises(exception.NetworkError, + neutron.wait_for_host_agent, + self.client_mock, 'hostname', target_state='down') + + @mock.patch.object(neutron, '_validate_agent') + @mock.patch.object(time, 'sleep') + def test_wait_for_host_agent_down_target_state_down( + self, sleep_mock, validate_agent_mock): + validate_agent_mock.return_value = False + self.assertTrue( + neutron.wait_for_host_agent(self.client_mock, 'hostname', + target_state='down')) + sleep_mock.assert_not_called() @mock.patch.object(neutron, '_get_port_by_uuid') @mock.patch.object(time, 'sleep') diff --git a/releasenotes/notes/add-port-is-smartnic-4ce6974c8fe2732d.yaml b/releasenotes/notes/add-port-is-smartnic-4ce6974c8fe2732d.yaml new file mode 100644 index 0000000000..42891d4746 --- /dev/null +++ b/releasenotes/notes/add-port-is-smartnic-4ce6974c8fe2732d.yaml @@ -0,0 +1,21 @@ +--- +features: + - | + Adds an ``is_smartnic`` field to the port object in REST API version + 1.53. + + ``is_smartnic`` field indicates if this port is a Smart NIC port, + False by default. This field may be set by operator to use baremetal + nodes with Smart NICs as ironic nodes. + + The REST API endpoints related to ports provide support for the + ``is_smartnic`` field. The `ironic developer documentation + `_ + provides information on how to configure and use Smart NIC ports. +upgrade: + - | + Adds an ``is_smartnic`` field to the port object in REST API version + 1.53. + + Upgrading to this release will set ``is_smartnic`` to False for all + ports.