Follow-up fixes to 206244

This commit is a collection of follow-up fixes to the nits/comments on
https://review.openstack.org/#/c/206244/116

Change-Id: I5631f5df3c0b6a3baa386da8b93b1e888c1f8d3c

api-ref/source/baremetal-api-v1-ports.inc

@@ -32,6 +32,11 @@ API microversion 1.8 added the ``fields`` Request parameter. When specified,
 this causes the content of the Response to include only the specified fields,
 rather than the default set.
 
+API microversion 1.19 added the ``pxe_enabled`` and ``local_link_connection``
+fields.
+
+.. TODO: add pxe_enabled and local_link_connection to all sample files
+
 Normal response code: 200
 
 Request

doc/source/webapi/v1.rst

@@ -34,8 +34,7 @@ API Versions History
 
 **1.19**
 
-    This API version adds the multitenancy-related ``local_link_connection``
-    and ``pxe_enabled`` fields to a port.
+    Add ``local_link_connection`` and ``pxe_enabled`` fields to the port object.
 
 **1.18**
 

ironic/api/controllers/v1/port.py

@@ -99,7 +99,7 @@ class Port(base.APIBase):
     """Indicates whether pxe is enabled or disabled on the node."""
 
     local_link_connection = types.locallinkconnectiontype
-    """The port binding profile for each port"""
+    """The port binding profile for the port"""
 
     links = wsme.wsattr([link.Link], readonly=True)
     """A list containing a self link and associated port links"""

ironic/api/controllers/v1/types.py

@@ -309,14 +309,15 @@ class LocalLinkConnectionType(wtypes.UserType):
 
         # Check switch_id is either a valid mac address or
         # OpenFlow datapath_id and normalize it.
-        if utils.is_valid_mac(value['switch_id']):
+        try:
             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'])
+        except exception.InvalidMAC:
+            try:
+                value['switch_id'] = utils.validate_and_normalize_datapath_id(
+                    value['switch_id'])
+            except exception.InvalidDatapathID:
+                raise exception.InvalidSwitchID(switch_id=value['switch_id'])
 
         return value
 

ironic/common/exception.py

@@ -197,7 +197,7 @@ class InvalidSwitchID(Invalid):
                  "received %(switch_id)s.")
 
 
-class InvalidDatapathId(Invalid):
+class InvalidDatapathID(Invalid):
     _msg_fmt = _("Expected an OpenFlow datapath ID but received "
                  "%(datapath_id)s.")
 

ironic/common/utils.py

@@ -308,12 +308,12 @@ def validate_and_normalize_datapath_id(datapath_id):
 
     :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.
+    :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)
+        raise exception.InvalidDatapathID(datapath_id=datapath_id)
     return datapath_id.lower()
 
 

ironic/tests/unit/api/v1/test_ports.py

@@ -64,7 +64,6 @@ 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')
@@ -281,7 +280,8 @@ class TestListPorts(test_api_base.BaseApiTest):
         self.assertEqual(sorted(ports), uuids)
 
     def test_sort_key_invalid(self):
-        invalid_keys_list = ['foo', 'extra', 'internal_info']
+        invalid_keys_list = ['foo', 'extra', 'internal_info',
+                             'local_link_connection']
         for invalid_key in invalid_keys_list:
             response = self.get_json(
                 '/ports?sort_key=%s' % invalid_key, expect_errors=True,
@@ -377,8 +377,6 @@ 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'}
@@ -471,7 +469,7 @@ class TestPatch(test_api_base.BaseApiTest):
                                      '/local_link_connection/switch_id',
                                      'value': switch_id,
                                      'op': 'replace'}],
-                                   headers=self.headers)
+                                   headers={api_base.Version.string: '1.19'})
         self.assertEqual('application/json', response.content_type)
         self.assertEqual(http_client.OK, response.status_code)
         self.assertEqual(switch_id,
@@ -713,7 +711,7 @@ class TestPatch(test_api_base.BaseApiTest):
                                    [{'path': '/pxe_enabled',
                                      'value': pxe_enabled,
                                      'op': 'replace'}],
-                                   headers=self.headers)
+                                   headers={api_base.Version.string: '1.19'})
         self.assertEqual(http_client.OK, response.status_code)
         self.assertEqual(pxe_enabled, response.json['pxe_enabled'])
 
@@ -936,8 +934,7 @@ class TestPost(test_api_base.BaseApiTest):
 
     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)
+        pdict = post_get_test_port(pxe_enabled=False)
         del pdict['local_link_connection']
         response = self.post_json('/ports', pdict, headers=headers,
                                   expect_errors=True)

ironic/tests/unit/api/v1/test_types.py

@@ -329,7 +329,7 @@ class TestLocalLinkConnectionType(base.TestCase):
         self.assertRaisesRegex(exception.Invalid, 'Missing mandatory',
                                v.validate, value)
 
-    def test_local_link_connection_type_withou_optional_key(self):
+    def test_local_link_connection_type_without_optional_key(self):
         v = types.locallinkconnectiontype
         value = {'switch_id': '0a:1b:2c:3d:4e:5f',
                  'port_id': 'value2'}

ironic/tests/unit/api/v1/test_utils.py

@@ -226,7 +226,7 @@ class TestApiUtils(base.TestCase):
         self.assertFalse(utils.allow_port_internal_info())
 
     @mock.patch.object(pecan, 'request', spec_set=['version'])
-    def test_allow_multitenancy_fields(self, mock_request):
+    def test_allow_port_advanced_net_fields(self, mock_request):
         mock_request.version.minor = 19
         self.assertTrue(utils.allow_port_advanced_net_fields())
         mock_request.version.minor = 18

releasenotes/notes/add-port-advanced-net-fields-55465091f019d962.yaml

@@ -1,8 +1,10 @@
 ---
 features:
   - |
-    API version is bumped to 1.19, ``local_link_connection`` and
-    ``pxe_enabled`` fields were added to a Port:
+    Exposes the ``local_link_connection`` and ``pxe_enabled`` properties of the
+    Port resource to the REST API, raising the API maximum version to 1.19.
 
-    * ``pxe_enabled`` indicates whether PXE is enabled for the port.
-    * ``local_link_connection`` contains the port binding profile.
+    * The ``pxe_enabled`` field indicates whether this Port should be used when
+      PXE booting this Node.
+    * The ``local_link_connection`` field may be used to supply the port
+      binding profile.