LLC Hook: Fix patching Ironic ports

This patch corrects a problem with the LLC hook's use
of the node_info.patch_port function, ensuring the correct data is
passed to the function, not a unicode string.

This patch also ensures that the mac addresses for chassis-id and
port-id are passed to ironic in the expected Unix format. Ironic will
fail to validate the mac addresses without this.

Change-Id: Id5b729cd9e9beeb4f59ba6950162163d2fdf3a3a
Closes-Bug: #1629302
Closes-Bug: #1629303

ironic_inspector/plugins/local_link_connection.py

@@ -68,13 +68,15 @@ class GenericLocalLinkConnectionHook(base.ProcessingHook):
                 value = data[1:].decode()
             if data[0] == PORT_ID_SUBTYPE_MAC:
                 value = str(netaddr.EUI(
-                    binascii.hexlify(data[1:]).decode()))
+                    binascii.hexlify(data[1:]).decode(),
+                    dialect=netaddr.mac_unix_expanded))
         elif tlv_type == LLDP_TLV_TYPE_CHASSIS_ID:
             # Check to ensure the chassis id is the allowed type
             if data[0] == CHASSIS_ID_SUBTYPE_MAC:
                 item = "switch_id"
                 value = str(netaddr.EUI(
-                    binascii.hexlify(data[1:]).decode()))
+                    binascii.hexlify(data[1:]).decode(),
+                    dialect=netaddr.mac_unix_expanded))
 
         if item and value:
             if (not CONF.processing.overwrite_existing and
@@ -93,11 +95,15 @@ class GenericLocalLinkConnectionHook(base.ProcessingHook):
         for iface in inventory['interfaces']:
             if iface['name'] not in introspection_data['all_interfaces']:
                 continue
-            port = ironic_ports[iface['mac_address']]
+
+            mac_address = iface['mac_address']
+            port = ironic_ports[mac_address]
 
             lldp_data = iface.get('lldp')
             if lldp_data is None:
-                LOG.warning(_LW("No LLDP Data found for interface %s"), iface)
+                LOG.warning(_LW("No LLDP Data found for interface %s"),
+                            mac_address, node_info=node_info,
+                            data=introspection_data)
                 continue
 
             patches = []
@@ -111,11 +117,12 @@ class GenericLocalLinkConnectionHook(base.ProcessingHook):
                 # transaction, so create a new ironic client and explicitly
                 # pass it into the function.
                 cli = ironic.get_client(api_version=REQUIRED_IRONIC_VERSION)
-                node_info.patch_port(iface['mac_address'], patches, ironic=cli)
+                node_info.patch_port(port, patches, ironic=cli)
             except client_exc.NotAcceptable:
                 LOG.error(_LE("Unable to set Ironic port local link "
                               "connection information because Ironic does not "
-                              "support the required version"))
+                              "support the required version"),
+                          node_info=node_info, data=introspection_data)
                 # NOTE(sambetts) May as well break out out of the loop here
                 # because Ironic version is not going to change for the other
                 # interfaces.

ironic_inspector/test/unit/test_plugins_local_link_connection.py

@@ -62,7 +62,7 @@ class TestGenericLocalLinkConnectionHook(test_base.NodeTest):
             {'path': '/local_link_connection/port_id',
              'value': 'Ethernet1/18', 'op': 'add'},
             {'path': '/local_link_connection/switch_id',
-             'value': '88-5A-92-EC-54-59', 'op': 'add'},
+             'value': '88:5a:92:ec:54:59', 'op': 'add'},
         ]
         self.hook.before_update(self.data, self.node_info)
         self.assertCalledWithPatch(patches, mock_patch)
@@ -90,7 +90,7 @@ class TestGenericLocalLinkConnectionHook(test_base.NodeTest):
             2, '0645746865726e6574312f3138')
         patches = [
             {'path': '/local_link_connection/switch_id',
-             'value': '88-5A-92-EC-54-59', 'op': 'add'}
+             'value': '88:5a:92:ec:54:59', 'op': 'add'}
         ]
         self.hook.before_update(self.data, self.node_info)
         self.assertCalledWithPatch(patches, mock_patch)
@@ -101,9 +101,9 @@ class TestGenericLocalLinkConnectionHook(test_base.NodeTest):
             2, '03885a92ec5458')
         patches = [
             {'path': '/local_link_connection/port_id',
-             'value': '88-5A-92-EC-54-58', 'op': 'add'},
+             'value': '88:5a:92:ec:54:58', 'op': 'add'},
             {'path': '/local_link_connection/switch_id',
-             'value': '88-5A-92-EC-54-59', 'op': 'add'}
+             'value': '88:5a:92:ec:54:59', 'op': 'add'}
         ]
         self.hook.before_update(self.data, self.node_info)
         self.assertCalledWithPatch(patches, mock_patch)
@@ -132,7 +132,7 @@ class TestGenericLocalLinkConnectionHook(test_base.NodeTest):
         cfg.CONF.set_override('overwrite_existing', False, group='processing')
         patches = [
             {'path': '/local_link_connection/switch_id',
-             'value': '88-5A-92-EC-54-59', 'op': 'add'}
+             'value': '88:5a:92:ec:54:59', 'op': 'add'}
         ]
         self.hook.before_update(self.data, self.node_info)
         self.assertCalledWithPatch(patches, mock_patch)

releasenotes/notes/fix_llc_hook_bugs-efeea008c2f792eb.yaml

@@ -0,0 +1,6 @@
+---
+fixes:
+  - LLC hook now formats the chassis id and port id MAC addresses into Unix
+    format as expected by ironic.
+  - LLC hook ensures that correct port information is passed to patch_port
+    function