From 0a5549680d491894faad5133db38e1bc2b4c93d6 Mon Sep 17 00:00:00 2001 From: Laura Moore Date: Mon, 27 Jul 2015 17:29:57 -0400 Subject: [PATCH] Add multitenancy-related fields to port API object This commit adds new fields to port object: - port.pxe_enabled: indicates whether pxe is enabled or disabled for this port - port.local_link_connection: contains the port binding profile. Partial-bug: #1526403 Co-Authored-By: Jenny Moorehead Co-Authored-By: Will Stevenson Co-Authored-By: Vasyl Saienko Co-Authored-By: Vladyslav Drok Co-Authored-By: Zhenguo Niu Change-Id: Ie655fd59b06de7b84fba3b438d5e4c2ecd8075c3 --- doc/source/webapi/v1.rst | 5 + ironic/api/controllers/v1/port.py | 44 ++++- ironic/api/controllers/v1/types.py | 72 +++++++ ironic/api/controllers/v1/utils.py | 23 +++ ironic/api/controllers/v1/versions.py | 4 +- ironic/common/exception.py | 10 + ironic/common/utils.py | 33 ++++ ironic/tests/unit/api/utils.py | 3 - ironic/tests/unit/api/v1/test_ports.py | 177 ++++++++++++++++-- ironic/tests/unit/api/v1/test_types.py | 52 +++++ ironic/tests/unit/api/v1/test_utils.py | 32 ++++ ironic/tests/unit/common/test_utils.py | 17 ++ ...-advanced-net-fields-55465091f019d962.yaml | 8 + 13 files changed, 458 insertions(+), 22 deletions(-) create mode 100644 releasenotes/notes/add-port-advanced-net-fields-55465091f019d962.yaml diff --git a/doc/source/webapi/v1.rst b/doc/source/webapi/v1.rst index df79bb1daa..d6465e7afa 100644 --- a/doc/source/webapi/v1.rst +++ b/doc/source/webapi/v1.rst @@ -32,6 +32,11 @@ always requests the newest supported API version. API Versions History -------------------- +**1.19** + + This API version adds the multitenancy-related ``local_link_connection`` + and ``pxe_enabled`` fields to a port. + **1.18** Add ``internal_info`` readonly field to the port object, that will be used diff --git a/ironic/api/controllers/v1/port.py b/ironic/api/controllers/v1/port.py index b5f332c2c9..852078b478 100644 --- a/ironic/api/controllers/v1/port.py +++ b/ironic/api/controllers/v1/port.py @@ -40,6 +40,11 @@ def hide_fields_in_newer_versions(obj): # if requested version is < 1.18, hide internal_info field if not api_utils.allow_port_internal_info(): obj.internal_info = wsme.Unset + # if requested version is < 1.19, hide local_link_connection and + # pxe_enabled fields + if not api_utils.allow_port_advanced_net_fields(): + obj.pxe_enabled = wsme.Unset + obj.local_link_connection = wsme.Unset class Port(base.APIBase): @@ -90,6 +95,12 @@ class Port(base.APIBase): mandatory=True) """The UUID of the node this port belongs to""" + pxe_enabled = types.boolean + """Indicates whether pxe is enabled or disabled on the node.""" + + local_link_connection = types.locallinkconnectiontype + """The port binding profile for each port""" + links = wsme.wsattr([link.Link], readonly=True) """A list containing a self link and associated port links""" @@ -151,7 +162,11 @@ class Port(base.APIBase): extra={'foo': 'bar'}, internal_info={}, created_at=datetime.datetime.utcnow(), - updated_at=datetime.datetime.utcnow()) + updated_at=datetime.datetime.utcnow(), + pxe_enabled=True, + local_link_connection={ + 'switch_info': 'host', 'port_id': 'Gig0/1', + 'switch_id': 'aa:bb:cc:dd:ee:ff'}) # NOTE(lucasagomes): node_uuid getter() method look at the # _node_uuid variable sample._node_uuid = '7ae81bb3-dec3-4289-8d6c-da80bd8001ae' @@ -204,7 +219,9 @@ class PortsController(rest.RestController): 'detail': ['GET'], } - invalid_sort_key_list = ['extra', 'internal_info'] + invalid_sort_key_list = ['extra', 'internal_info', 'local_link_connection'] + + advanced_net_fields = ['pxe_enabled', 'local_link_connection'] def _get_ports_collection(self, node_ident, address, marker, limit, sort_key, sort_dir, resource_url=None, @@ -285,8 +302,13 @@ class PortsController(rest.RestController): :param sort_dir: direction to sort. "asc" or "desc". Default: asc. :param fields: Optional, a list with a specified set of fields of the resource to be returned. + :raises: NotAcceptable """ api_utils.check_allow_specify_fields(fields) + if (fields and not api_utils.allow_port_advanced_net_fields() and + set(fields).intersection(self.advanced_net_fields)): + raise exception.NotAcceptable() + if fields is None: fields = _DEFAULT_RETURN_FIELDS @@ -322,6 +344,7 @@ class PortsController(rest.RestController): :param limit: maximum number of resources to return in a single result. :param sort_key: column to sort results by. Default: id. :param sort_dir: direction to sort. "asc" or "desc". Default: asc. + :raises: NotAcceptable, HTTPNotFound """ if not node_uuid and node: # We're invoking this interface using positional notation, or @@ -348,6 +371,7 @@ class PortsController(rest.RestController): :param port_uuid: UUID of a port. :param fields: Optional, a list with a specified set of fields of the resource to be returned. + :raises: NotAcceptable """ if self.from_nodes: raise exception.OperationNotPermitted() @@ -362,12 +386,19 @@ class PortsController(rest.RestController): """Create a new port. :param port: a port within the request body. + :raises: NotAcceptable """ if self.from_nodes: raise exception.OperationNotPermitted() + pdict = port.as_dict() + if not api_utils.allow_port_advanced_net_fields(): + if set(pdict).intersection(self.advanced_net_fields): + raise exception.NotAcceptable() + new_port = objects.Port(pecan.request.context, - **port.as_dict()) + **pdict) + new_port.create() # Set the HTTP Location Header pecan.response.location = link.build_url('ports', new_port.uuid) @@ -380,9 +411,16 @@ class PortsController(rest.RestController): :param port_uuid: UUID of a port. :param patch: a json PATCH document to apply to this port. + :raises: NotAcceptable """ if self.from_nodes: raise exception.OperationNotPermitted() + if not api_utils.allow_port_advanced_net_fields(): + for field in self.advanced_net_fields: + field_path = '/%s' % field + if (api_utils.get_patch_values(patch, field_path) or + api_utils.is_path_removed(patch, field_path)): + raise exception.NotAcceptable() rpc_port = objects.Port.get_by_uuid(pecan.request.context, port_uuid) try: diff --git a/ironic/api/controllers/v1/types.py b/ironic/api/controllers/v1/types.py index b5fdd7f21c..7976af7535 100644 --- a/ironic/api/controllers/v1/types.py +++ b/ironic/api/controllers/v1/types.py @@ -255,3 +255,75 @@ class JsonPatchType(wtypes.Base): if patch.value is not wsme.Unset: ret['value'] = patch.value return ret + + +class LocalLinkConnectionType(wtypes.UserType): + """A type describing local link connection.""" + + basetype = wtypes.DictType + name = 'locallinkconnection' + + mandatory_fields = {'switch_id', + 'port_id'} + valid_fields = mandatory_fields.union({'switch_info'}) + + @staticmethod + def validate(value): + """Validate and convert the input to a LocalLinkConnectionType. + + :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. + + For example:: + { + 'switch_id': mac_or_datapath_id(), + 'port_id': 'Ethernet3/1', + 'switch_info': 'switch1' + } + + :returns: A dictionary. + :raises: Invalid if some of the keys in the dictionary being validated + are unknown, invalid, or some required ones are missing. + + """ + wtypes.DictType(wtypes.text, wtypes.text).validate(value) + + keys = set(value) + + # This is to workaround an issue when an API object is initialized from + # RPC object, in which dictionary fields that are set to None become + # empty dictionaries + if not keys: + return value + + invalid = keys - LocalLinkConnectionType.valid_fields + 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 + raise exception.Invalid(msg) + + # Check switch_id is either a valid mac address or + # OpenFlow datapath_id and normalize it. + if utils.is_valid_mac(value['switch_id']): + value['switch_id'] = utils.validate_and_normalize_mac( + value['switch_id']) + elif utils.is_valid_datapath_id(value['switch_id']): + value['switch_id'] = utils.validate_and_normalize_datapath_id( + value['switch_id']) + else: + raise exception.InvalidSwitchID(switch_id=value['switch_id']) + + return value + + @staticmethod + def frombasetype(value): + if value is None: + return None + return LocalLinkConnectionType.validate(value) + +locallinkconnectiontype = LocalLinkConnectionType() diff --git a/ironic/api/controllers/v1/utils.py b/ironic/api/controllers/v1/utils.py index d85baa7fbb..70098f4881 100644 --- a/ironic/api/controllers/v1/utils.py +++ b/ironic/api/controllers/v1/utils.py @@ -99,6 +99,20 @@ def get_patch_values(patch, path): if p['path'] == path and p['op'] != 'remove'] +def is_path_removed(patch, path): + """Returns whether the patch includes removal of the path (or subpath of). + + :param patch: HTTP PATCH request body. + :param path: the path to check. + :returns: True if path or subpath being removed, False otherwise. + """ + path = path.rstrip('/') + for p in patch: + if ((p['path'] == path or p['path'].startswith(path + '/')) and + p['op'] == 'remove'): + return True + + def allow_node_logical_names(): # v1.5 added logical name aliases return pecan.request.version.minor >= versions.MINOR_5_NODE_NAME @@ -299,6 +313,15 @@ def allow_port_internal_info(): versions.MINOR_18_PORT_INTERNAL_INFO) +def allow_port_advanced_net_fields(): + """Check if we should return local_link_connection and pxe_enabled fields. + + Version 1.19 of the API added support for these new fields in port object. + """ + return (pecan.request.version.minor >= + versions.MINOR_19_PORT_ADVANCED_NET_FIELDS) + + def get_controller_reserved_names(cls): """Get reserved names for a given controller. diff --git a/ironic/api/controllers/v1/versions.py b/ironic/api/controllers/v1/versions.py index 8f86def24e..9bbfd32c75 100644 --- a/ironic/api/controllers/v1/versions.py +++ b/ironic/api/controllers/v1/versions.py @@ -48,6 +48,7 @@ BASE_VERSION = 1 # v1.16: Add ability to filter nodes by driver. # v1.17: Add 'adopt' verb for ADOPTING active nodes. # v1.18: Add port.internal_info. +# v1.19: Add port.local_link_connection and port.pxe_enabled. MINOR_0_JUNO = 0 MINOR_1_INITIAL_VERSION = 1 @@ -68,11 +69,12 @@ MINOR_15_MANUAL_CLEAN = 15 MINOR_16_DRIVER_FILTER = 16 MINOR_17_ADOPT_VERB = 17 MINOR_18_PORT_INTERNAL_INFO = 18 +MINOR_19_PORT_ADVANCED_NET_FIELDS = 19 # When adding another version, update MINOR_MAX_VERSION and also update # doc/source/webapi/v1.rst with a detailed explanation of what the version has # changed. -MINOR_MAX_VERSION = MINOR_18_PORT_INTERNAL_INFO +MINOR_MAX_VERSION = MINOR_19_PORT_ADVANCED_NET_FIELDS # String representations of the minor and maximum versions MIN_VERSION_STRING = '{}.{}'.format(BASE_VERSION, MINOR_1_INITIAL_VERSION) diff --git a/ironic/common/exception.py b/ironic/common/exception.py index 5622203b2d..ca761cd1da 100644 --- a/ironic/common/exception.py +++ b/ironic/common/exception.py @@ -192,6 +192,16 @@ class InvalidMAC(Invalid): _msg_fmt = _("Expected a MAC address but received %(mac)s.") +class InvalidSwitchID(Invalid): + _msg_fmt = _("Expected a MAC address or OpenFlow datapath ID but " + "received %(switch_id)s.") + + +class InvalidDatapathId(Invalid): + _msg_fmt = _("Expected an OpenFlow datapath ID but received " + "%(datapath_id)s.") + + class InvalidStateRequested(Invalid): _msg_fmt = _('The requested action "%(action)s" can not be performed ' 'on node "%(node)s" while it is in state "%(state)s".') diff --git a/ironic/common/utils.py b/ironic/common/utils.py index 65acb5dd4f..1f76a9a17b 100644 --- a/ironic/common/utils.py +++ b/ironic/common/utils.py @@ -185,6 +185,22 @@ def is_valid_mac(address): re.match(m, address.lower())) +def is_valid_datapath_id(datapath_id): + """Verify the format of an OpenFlow datapath_id. + + Check if a datapath_id is valid and contains 16 hexadecimal digits. + Datapath ID format: the lower 48-bits are for a MAC address, + while the upper 16-bits are implementer-defined. + + :param datapath_id: OpenFlow datapath_id to be validated. + :returns: True if valid. False if not. + + """ + m = "^[0-9a-f]{16}$" + return (isinstance(datapath_id, six.string_types) and + re.match(m, datapath_id.lower())) + + _is_valid_logical_name_re = re.compile(r'^[A-Z0-9-._~]+$', re.I) # old is_hostname_safe() regex, retained for backwards compat @@ -284,6 +300,23 @@ def validate_and_normalize_mac(address): return address.lower() +def validate_and_normalize_datapath_id(datapath_id): + """Validate an OpenFlow datapath_id and return normalized form. + + Checks whether the supplied OpenFlow datapath_id is formally correct and + normalize it to all lower case. + + :param datapath_id: OpenFlow datapath_id to be validated and normalized. + :returns: Normalized and validated OpenFlow datapath_id. + :raises: InvalidDatapathId If an OpenFlow datapath_id is not valid. + + """ + + if not is_valid_datapath_id(datapath_id): + raise exception.InvalidDatapathId(datapath_id=datapath_id) + return datapath_id.lower() + + def is_valid_ipv6_cidr(address): try: str(netaddr.IPNetwork(address, version=6).cidr) diff --git a/ironic/tests/unit/api/utils.py b/ironic/tests/unit/api/utils.py index 024fd07e73..2b10351719 100644 --- a/ironic/tests/unit/api/utils.py +++ b/ironic/tests/unit/api/utils.py @@ -105,9 +105,6 @@ def port_post_data(**kw): port = utils.get_test_port(**kw) # node_id is not part of the API object port.pop('node_id') - # TODO(vsaienko): remove when API part is added - port.pop('local_link_connection') - port.pop('pxe_enabled') # portgroup_id is not part of the API object port.pop('portgroup_id') internal = port_controller.PortPatchType.internal_attrs() diff --git a/ironic/tests/unit/api/v1/test_ports.py b/ironic/tests/unit/api/v1/test_ports.py index e3ab27a77b..87b3f1ac1d 100644 --- a/ironic/tests/unit/api/v1/test_ports.py +++ b/ironic/tests/unit/api/v1/test_ports.py @@ -31,6 +31,7 @@ from ironic.api.controllers import base as api_base from ironic.api.controllers import v1 as api_v1 from ironic.api.controllers.v1 import port as api_port from ironic.api.controllers.v1 import utils as api_utils +from ironic.api.controllers.v1 import versions from ironic.common import exception from ironic.conductor import rpcapi from ironic.tests import base @@ -63,6 +64,7 @@ class TestListPorts(test_api_base.BaseApiTest): def setUp(self): super(TestListPorts, self).setUp() self.node = obj_utils.create_test_node(self.context) + self.headers = {api_base.Version.string: str(api_v1.MAX_VER)} def test_empty(self): data = self.get_json('/ports') @@ -145,7 +147,7 @@ class TestListPorts(test_api_base.BaseApiTest): self.assertEqual(http_client.NOT_ACCEPTABLE, response.status_int) def test_detail(self): - port = obj_utils.create_test_port(self.context, node_id=self.node.id,) + port = obj_utils.create_test_port(self.context, node_id=self.node.id) data = self.get_json( '/ports/detail', headers={api_base.Version.string: str(api_v1.MAX_VER)} @@ -154,6 +156,8 @@ class TestListPorts(test_api_base.BaseApiTest): self.assertIn('extra', data['ports'][0]) self.assertIn('internal_info', data['ports'][0]) self.assertIn('node_uuid', data['ports'][0]) + self.assertIn('pxe_enabled', data['ports'][0]) + self.assertIn('local_link_connection', data['ports'][0]) # never expose the node_id self.assertNotIn('node_id', data['ports'][0]) @@ -373,6 +377,8 @@ class TestPatch(test_api_base.BaseApiTest): self.mock_gtf = p.start() self.mock_gtf.return_value = 'test-topic' self.addCleanup(p.stop) + self.headers = {api_base.Version.string: str( + versions.MAX_VERSION_STRING)} def test_update_byid(self, mock_upd): extra = {'foo': 'bar'} @@ -456,6 +462,44 @@ class TestPatch(test_api_base.BaseApiTest): self.assertEqual('application/json', response.content_type) self.assertEqual(http_client.OK, response.status_code) + def test_replace_local_link_connection(self, mock_upd): + switch_id = 'aa:bb:cc:dd:ee:ff' + mock_upd.return_value = self.port + mock_upd.return_value.local_link_connection['switch_id'] = switch_id + response = self.patch_json('/ports/%s' % self.port.uuid, + [{'path': + '/local_link_connection/switch_id', + 'value': switch_id, + 'op': 'replace'}], + headers=self.headers) + self.assertEqual('application/json', response.content_type) + self.assertEqual(http_client.OK, response.status_code) + self.assertEqual(switch_id, + response.json['local_link_connection']['switch_id']) + self.assertTrue(mock_upd.called) + + kargs = mock_upd.call_args[0][1] + self.assertEqual(switch_id, kargs.local_link_connection['switch_id']) + + def test_remove_local_link_connection_old_api(self, mock_upd): + response = self.patch_json( + '/ports/%s' % self.port.uuid, + [{'path': '/local_link_connection/switch_id', 'op': 'remove'}], + expect_errors=True) + self.assertEqual('application/json', response.content_type) + self.assertTrue(response.json['error_message']) + self.assertEqual(http_client.NOT_ACCEPTABLE, response.status_code) + + def test_set_pxe_enabled_false_old_api(self, mock_upd): + response = self.patch_json('/ports/%s' % self.port.uuid, + [{'path': '/pxe_enabled', + 'value': False, + 'op': 'add'}], + expect_errors=True) + self.assertEqual('application/json', response.content_type) + self.assertTrue(response.json['error_message']) + self.assertEqual(http_client.NOT_ACCEPTABLE, response.status_code) + def test_add_node_uuid(self, mock_upd): mock_upd.return_value = self.port response = self.patch_json('/ports/%s' % self.port.uuid, @@ -661,21 +705,50 @@ class TestPatch(test_api_base.BaseApiTest): kargs = mock_upd.call_args[0][1] self.assertEqual(address.lower(), kargs.address) + def test_update_pxe_enabled_allowed(self, mock_upd): + pxe_enabled = True + mock_upd.return_value = self.port + mock_upd.return_value.pxe_enabled = pxe_enabled + response = self.patch_json('/ports/%s' % self.port.uuid, + [{'path': '/pxe_enabled', + 'value': pxe_enabled, + 'op': 'replace'}], + headers=self.headers) + self.assertEqual(http_client.OK, response.status_code) + self.assertEqual(pxe_enabled, response.json['pxe_enabled']) + + def test_update_pxe_enabled_old_api_version(self, mock_upd): + pxe_enabled = True + mock_upd.return_value = self.port + headers = {api_base.Version.string: '1.14'} + response = self.patch_json('/ports/%s' % self.port.uuid, + [{'path': '/pxe_enabled', + 'value': pxe_enabled, + 'op': 'replace'}], + expect_errors=True, + headers=headers) + self.assertEqual('application/json', response.content_type) + self.assertEqual(http_client.NOT_ACCEPTABLE, response.status_int) + self.assertFalse(mock_upd.called) + class TestPost(test_api_base.BaseApiTest): def setUp(self): super(TestPost, self).setUp() self.node = obj_utils.create_test_node(self.context) + self.headers = {api_base.Version.string: str( + versions.MAX_VERSION_STRING)} @mock.patch.object(timeutils, 'utcnow') def test_create_port(self, mock_utcnow): pdict = post_get_test_port() test_time = datetime.datetime(2000, 1, 1, 0, 0) mock_utcnow.return_value = test_time - response = self.post_json('/ports', pdict) + response = self.post_json('/ports', pdict, headers=self.headers) self.assertEqual(http_client.CREATED, response.status_int) - result = self.get_json('/ports/%s' % pdict['uuid']) + result = self.get_json('/ports/%s' % pdict['uuid'], + headers=self.headers) self.assertEqual(pdict['uuid'], result['uuid']) self.assertFalse(result['updated_at']) return_created_at = timeutils.parse_isotime( @@ -691,8 +764,9 @@ class TestPost(test_api_base.BaseApiTest): with mock.patch.object(self.dbapi, 'create_port', wraps=self.dbapi.create_port) as cp_mock: pdict = post_get_test_port(extra={'foo': 123}) - self.post_json('/ports', pdict) - result = self.get_json('/ports/%s' % pdict['uuid']) + self.post_json('/ports', pdict, headers=self.headers) + result = self.get_json('/ports/%s' % pdict['uuid'], + headers=self.headers) self.assertEqual(pdict['extra'], result['extra']) cp_mock.assert_called_once_with(mock.ANY) # Check that 'id' is not in first arg of positional args @@ -701,8 +775,9 @@ class TestPost(test_api_base.BaseApiTest): def test_create_port_generate_uuid(self): pdict = post_get_test_port() del pdict['uuid'] - response = self.post_json('/ports', pdict) - result = self.get_json('/ports/%s' % response.json['uuid']) + response = self.post_json('/ports', pdict, headers=self.headers) + result = self.get_json('/ports/%s' % response.json['uuid'], + headers=self.headers) self.assertEqual(pdict['address'], result['address']) self.assertTrue(uuidutils.is_uuid_like(result['uuid'])) @@ -711,14 +786,16 @@ class TestPost(test_api_base.BaseApiTest): 'float': 0.1, 'bool': True, 'list': [1, 2], 'none': None, 'dict': {'cat': 'meow'}}) - self.post_json('/ports', pdict) - result = self.get_json('/ports/%s' % pdict['uuid']) + self.post_json('/ports', pdict, headers=self.headers) + result = self.get_json('/ports/%s' % pdict['uuid'], + headers=self.headers) self.assertEqual(pdict['extra'], result['extra']) def test_create_port_no_mandatory_field_address(self): pdict = post_get_test_port() del pdict['address'] - response = self.post_json('/ports', pdict, expect_errors=True) + response = self.post_json('/ports', pdict, expect_errors=True, + headers=self.headers) self.assertEqual(http_client.BAD_REQUEST, response.status_int) self.assertEqual('application/json', response.content_type) self.assertTrue(response.json['error_message']) @@ -741,8 +818,9 @@ class TestPost(test_api_base.BaseApiTest): def test_create_port_address_normalized(self): address = 'AA:BB:CC:DD:EE:FF' pdict = post_get_test_port(address=address) - self.post_json('/ports', pdict) - result = self.get_json('/ports/%s' % pdict['uuid']) + self.post_json('/ports', pdict, headers=self.headers) + result = self.get_json('/ports/%s' % pdict['uuid'], + headers=self.headers) self.assertEqual(address.lower(), result['address']) def test_create_port_with_hyphens_delimiter(self): @@ -764,7 +842,7 @@ class TestPost(test_api_base.BaseApiTest): def test_node_uuid_to_node_id_mapping(self): pdict = post_get_test_port(node_uuid=self.node['uuid']) - self.post_json('/ports', pdict) + self.post_json('/ports', pdict, headers=self.headers) # GET doesn't return the node_id it's an internal value port = self.dbapi.get_port_by_uuid(pdict['uuid']) self.assertEqual(self.node['id'], port.node_id) @@ -780,9 +858,10 @@ class TestPost(test_api_base.BaseApiTest): def test_create_port_address_already_exist(self): address = 'AA:AA:AA:11:22:33' pdict = post_get_test_port(address=address) - self.post_json('/ports', pdict) + self.post_json('/ports', pdict, headers=self.headers) pdict['uuid'] = uuidutils.generate_uuid() - response = self.post_json('/ports', pdict, expect_errors=True) + response = self.post_json('/ports', pdict, expect_errors=True, + headers=self.headers) self.assertEqual(http_client.CONFLICT, response.status_int) self.assertEqual('application/json', response.content_type) error_msg = response.json['error_message'] @@ -797,6 +876,74 @@ class TestPost(test_api_base.BaseApiTest): self.assertEqual(http_client.BAD_REQUEST, response.status_int) self.assertTrue(response.json['error_message']) + def test_create_port_some_invalid_local_link_connection_key(self): + pdict = post_get_test_port( + local_link_connection={'switch_id': 'value1', + 'port_id': 'Ethernet1/15', + 'switch_foo': 'value3'}) + response = self.post_json('/ports', pdict, expect_errors=True, + headers=self.headers) + self.assertEqual('application/json', response.content_type) + self.assertEqual(http_client.BAD_REQUEST, response.status_int) + self.assertTrue(response.json['error_message']) + + def test_create_port_local_link_connection_keys(self): + pdict = post_get_test_port( + local_link_connection={'switch_id': '0a:1b:2c:3d:4e:5f', + 'port_id': 'Ethernet1/15', + 'switch_info': 'value3'}) + response = self.post_json('/ports', pdict, headers=self.headers) + self.assertEqual('application/json', response.content_type) + self.assertEqual(http_client.CREATED, response.status_int) + + def test_create_port_local_link_connection_switch_id_bad_mac(self): + pdict = post_get_test_port( + local_link_connection={'switch_id': 'zz:zz:zz:zz:zz:zz', + 'port_id': 'Ethernet1/15', + 'switch_info': 'value3'}) + response = self.post_json('/ports', pdict, expect_errors=True, + headers=self.headers) + self.assertEqual('application/json', response.content_type) + self.assertEqual(http_client.BAD_REQUEST, response.status_int) + self.assertTrue(response.json['error_message']) + + def test_create_port_local_link_connection_missing_mandatory(self): + pdict = post_get_test_port( + local_link_connection={'switch_id': '0a:1b:2c:3d:4e:5f', + 'switch_info': 'fooswitch'}) + response = self.post_json('/ports', pdict, expect_errors=True, + headers=self.headers) + self.assertEqual('application/json', response.content_type) + self.assertEqual(http_client.BAD_REQUEST, response.status_int) + + def test_create_port_local_link_connection_missing_optional(self): + pdict = post_get_test_port( + local_link_connection={'switch_id': '0a:1b:2c:3d:4e:5f', + 'port_id': 'Ethernet1/15'}) + response = self.post_json('/ports', pdict, headers=self.headers) + self.assertEqual('application/json', response.content_type) + self.assertEqual(http_client.CREATED, response.status_int) + + def test_create_port_with_llc_old_api_version(self): + headers = {api_base.Version.string: '1.14'} + pdict = post_get_test_port( + local_link_connection={'switch_id': '0a:1b:2c:3d:4e:5f', + 'port_id': 'Ethernet1/15'}) + 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) + + def test_create_port_with_pxe_enabled_old_api_version(self): + headers = {api_base.Version.string: '1.14'} + pdict = post_get_test_port( + pxe_enabled=False) + del pdict['local_link_connection'] + 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) + @mock.patch.object(rpcapi.ConductorAPI, 'destroy_port') class TestDelete(test_api_base.BaseApiTest): diff --git a/ironic/tests/unit/api/v1/test_types.py b/ironic/tests/unit/api/v1/test_types.py index c05562f0de..ba893517fa 100644 --- a/ironic/tests/unit/api/v1/test_types.py +++ b/ironic/tests/unit/api/v1/test_types.py @@ -287,3 +287,55 @@ class TestListType(base.TestCase): self.assertItemsEqual(['foo', 'bar'], v.validate("foo,foo,foo,bar")) self.assertIsInstance(v.validate('foo,bar'), list) + + +class TestLocalLinkConnectionType(base.TestCase): + + def test_local_link_connection_type(self): + v = types.locallinkconnectiontype + value = {'switch_id': '0a:1b:2c:3d:4e:5f', + 'port_id': 'value2', + 'switch_info': 'value3'} + self.assertItemsEqual(value, v.validate(value)) + + def test_local_link_connection_type_datapath_id(self): + v = types.locallinkconnectiontype + value = {'switch_id': '0000000000000000', + 'port_id': 'value2', + 'switch_info': 'value3'} + self.assertItemsEqual(value, + v.validate(value)) + + def test_local_link_connection_type_not_mac_or_datapath_id(self): + v = types.locallinkconnectiontype + value = {'switch_id': 'badid', + 'port_id': 'value2', + 'switch_info': 'value3'} + self.assertRaises(exception.InvalidSwitchID, v.validate, value) + + def test_local_link_connection_type_invalid_key(self): + v = types.locallinkconnectiontype + value = {'switch_id': '0a:1b:2c:3d:4e:5f', + 'port_id': 'value2', + 'switch_info': 'value3', + 'invalid_key': 'value'} + self.assertRaisesRegex(exception.Invalid, 'are invalid keys', + v.validate, value) + + def test_local_link_connection_type_missing_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_withou_optional_key(self): + v = types.locallinkconnectiontype + value = {'switch_id': '0a:1b:2c:3d:4e:5f', + 'port_id': 'value2'} + self.assertItemsEqual(value, v.validate(value)) + + def test_local_link_connection_type_empty_value(self): + v = types.locallinkconnectiontype + value = {} + self.assertItemsEqual(value, v.validate(value)) diff --git a/ironic/tests/unit/api/v1/test_utils.py b/ironic/tests/unit/api/v1/test_utils.py index 1a897717b0..5d2058d553 100644 --- a/ironic/tests/unit/api/v1/test_utils.py +++ b/ironic/tests/unit/api/v1/test_utils.py @@ -82,6 +82,31 @@ class TestApiUtils(base.TestCase): values = utils.get_patch_values(patch, path) self.assertEqual(['node-x', 'node-y'], values) + def test_is_path_removed_success(self): + patch = [{'path': '/name', 'op': 'remove'}] + path = '/name' + value = utils.is_path_removed(patch, path) + self.assertTrue(value) + + def test_is_path_removed_subpath_success(self): + patch = [{'path': '/local_link_connection/switch_id', 'op': 'remove'}] + path = '/local_link_connection' + value = utils.is_path_removed(patch, path) + self.assertTrue(value) + + def test_is_path_removed_similar_subpath(self): + patch = [{'path': '/local_link_connection_info/switch_id', + 'op': 'remove'}] + path = '/local_link_connection' + value = utils.is_path_removed(patch, path) + self.assertFalse(value) + + def test_is_path_removed_replace(self): + patch = [{'path': '/name', 'op': 'replace', 'value': 'node-x'}] + path = '/name' + value = utils.is_path_removed(patch, path) + self.assertFalse(value) + def test_check_for_invalid_fields(self): requested = ['field_1', 'field_3'] supported = ['field_1', 'field_2', 'field_3'] @@ -200,6 +225,13 @@ class TestApiUtils(base.TestCase): mock_request.version.minor = 17 self.assertFalse(utils.allow_port_internal_info()) + @mock.patch.object(pecan, 'request', spec_set=['version']) + def test_allow_multitenancy_fields(self, mock_request): + mock_request.version.minor = 19 + self.assertTrue(utils.allow_port_advanced_net_fields()) + mock_request.version.minor = 18 + self.assertFalse(utils.allow_port_advanced_net_fields()) + class TestNodeIdent(base.TestCase): diff --git a/ironic/tests/unit/common/test_utils.py b/ironic/tests/unit/common/test_utils.py index 4b372a08e1..ada2cf648a 100644 --- a/ironic/tests/unit/common/test_utils.py +++ b/ironic/tests/unit/common/test_utils.py @@ -385,6 +385,14 @@ class GenericUtilsTestCase(base.TestCase): self.assertFalse(utils.is_valid_mac("AA BB CC DD EE FF")) self.assertFalse(utils.is_valid_mac("AA-BB-CC-DD-EE-FF")) + def test_is_valid_datapath_id(self): + self.assertTrue(utils.is_valid_datapath_id("525400cf2d319fdf")) + self.assertTrue(utils.is_valid_datapath_id("525400CF2D319FDF")) + self.assertFalse(utils.is_valid_datapath_id("52")) + self.assertFalse(utils.is_valid_datapath_id("52:54:00:cf:2d:31")) + self.assertFalse(utils.is_valid_datapath_id("notadatapathid00")) + self.assertFalse(utils.is_valid_datapath_id("5525400CF2D319FDF")) + def test_is_hostname_safe(self): self.assertTrue(utils.is_hostname_safe('spam')) self.assertFalse(utils.is_hostname_safe('spAm')) @@ -456,6 +464,15 @@ class GenericUtilsTestCase(base.TestCase): self.assertEqual(mac.lower(), utils.validate_and_normalize_mac(mac)) + def test_validate_and_normalize_datapath_id(self): + datapath_id = 'AA:BB:CC:DD:EE:FF' + with mock.patch.object(utils, 'is_valid_datapath_id', + autospec=True) as m_mock: + m_mock.return_value = True + self.assertEqual(datapath_id.lower(), + utils.validate_and_normalize_datapath_id( + datapath_id)) + def test_validate_and_normalize_mac_invalid_format(self): with mock.patch.object(utils, 'is_valid_mac', autospec=True) as m_mock: m_mock.return_value = False diff --git a/releasenotes/notes/add-port-advanced-net-fields-55465091f019d962.yaml b/releasenotes/notes/add-port-advanced-net-fields-55465091f019d962.yaml new file mode 100644 index 0000000000..a857de47b3 --- /dev/null +++ b/releasenotes/notes/add-port-advanced-net-fields-55465091f019d962.yaml @@ -0,0 +1,8 @@ +--- +features: + - | + API version is bumped to 1.19, ``local_link_connection`` and + ``pxe_enabled`` fields were added to a Port: + + * ``pxe_enabled`` indicates whether PXE is enabled for the port. + * ``local_link_connection`` contains the port binding profile.