From 20d941eeb15dcf20e3fd6a8eb2edef665211b356 Mon Sep 17 00:00:00 2001 From: Sam Betts Date: Thu, 29 Sep 2016 18:02:25 +0100 Subject: [PATCH] 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 --- .../plugins/local_link_connection.py | 19 +++++++++++++------ .../test_plugins_local_link_connection.py | 10 +++++----- .../fix_llc_hook_bugs-efeea008c2f792eb.yaml | 6 ++++++ 3 files changed, 24 insertions(+), 11 deletions(-) create mode 100644 releasenotes/notes/fix_llc_hook_bugs-efeea008c2f792eb.yaml diff --git a/ironic_inspector/plugins/local_link_connection.py b/ironic_inspector/plugins/local_link_connection.py index 3f5480ddc..1621ca83c 100644 --- a/ironic_inspector/plugins/local_link_connection.py +++ b/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. diff --git a/ironic_inspector/test/unit/test_plugins_local_link_connection.py b/ironic_inspector/test/unit/test_plugins_local_link_connection.py index 759c4a7b0..409cbd181 100644 --- a/ironic_inspector/test/unit/test_plugins_local_link_connection.py +++ b/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) diff --git a/releasenotes/notes/fix_llc_hook_bugs-efeea008c2f792eb.yaml b/releasenotes/notes/fix_llc_hook_bugs-efeea008c2f792eb.yaml new file mode 100644 index 000000000..ed7fc5493 --- /dev/null +++ b/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