openstack
/
ironic-inspector
Code Issues Proposed changes

Use processed lldp data, if available, for local_link_connection plugin 

This change uses the processed lldp stored by the lldp_basic plugin
to populate the local_link_connection port_id and switch_id.

If processed lldp data cannot be found, the raw lldp data is processed by
this plugin as currently implemented.  This patch changes the
processing to use the construct lib as implemented in the common
definitions in lldp_tlvs.py to parse the LLDP TLVs.

A minor change is also made to pass node_info into warning log messages.

Change-Id: Iae3fa41736898df2c73350d8dc61ed24f2963e32
This commit is contained in:
Bob Fournier 2017-02-08 15:09:14 -05:00
parent 328acd4d0a
commit 07c94392e2
4 changed files with 124 additions and 31 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
doc/source/usage.rst
View File

@ -191,7 +191,9 @@ Here are some plugins that can be additionally enabled:
port ID and chassis ID, if found it configures the local link connection
information on the nodes Ironic ports with that data. To enable LLDP in the
inventory from IPA ``ipa-collect-lldp=1`` should be passed as a kernel
parameter to the IPA ramdisk.
parameter to the IPA ramdisk. In order to avoid processing the raw LLDP
data twice, the ``lldp_basic`` plugin should also be installed and run
prior to this plugin.
``lldp_basic``
Processes LLDP data returned from inspection and parses TLVs from the
Basic Management (802.1AB), 802.1Q, and 802.3 sets and stores the

94
ironic_inspector/plugins/local_link_connection.py
View File

@ -15,30 +15,30 @@
import binascii
from construct import core
from ironicclient import exc as client_exc
import netaddr
from oslo_config import cfg
from ironic_inspector.common import ironic
from ironic_inspector.common import lldp_parsers
from ironic_inspector.common import lldp_tlvs as tlv
from ironic_inspector.plugins import base
from ironic_inspector import utils
LOG = utils.getProcessingLogger(__name__)
# NOTE(sambetts) Constants defined according to IEEE standard for LLDP
# http://standards.ieee.org/getieee802/download/802.1AB-2009.pdf
LLDP_TLV_TYPE_CHASSIS_ID = 1
LLDP_TLV_TYPE_PORT_ID = 2
PORT_ID_SUBTYPE_MAC = 3
PORT_ID_SUBTYPE_IFNAME = 5
PORT_ID_SUBTYPE_LOCAL = 7
STRING_PORT_SUBTYPES = [PORT_ID_SUBTYPE_IFNAME, PORT_ID_SUBTYPE_LOCAL]
CHASSIS_ID_SUBTYPE_MAC = 4
CONF = cfg.CONF
REQUIRED_IRONIC_VERSION = '1.19'
PORT_ID_ITEM_NAME = "port_id"
SWITCH_ID_ITEM_NAME = "switch_id"
LLDP_PROC_DATA_MAPPING =\
{lldp_parsers.LLDP_CHASSIS_ID_NM: SWITCH_ID_ITEM_NAME,
lldp_parsers.LLDP_PORT_ID_NM: PORT_ID_ITEM_NAME}
class GenericLocalLinkConnectionHook(base.ProcessingHook):
"""Process mandatory LLDP packet fields
@ -49,33 +49,39 @@ class GenericLocalLinkConnectionHook(base.ProcessingHook):
fields on the Ironic port that represents that NIC.
"""
def _get_local_link_patch(self, tlv_type, tlv_value, port):
def _get_local_link_patch(self, tlv_type, tlv_value, port, node_info):
try:
data = bytearray(binascii.unhexlify(tlv_value))
except TypeError:
LOG.warning("TLV value for TLV type %d not in correct"
"format, ensure TLV value is in "
"hexidecimal format when sent to "
"inspector", tlv_type)
"inspector", tlv_type, node_info=node_info)
return
item = value = None
if tlv_type == LLDP_TLV_TYPE_PORT_ID:
# Check to ensure the port id is an allowed type
item = "port_id"
if data[0] in STRING_PORT_SUBTYPES:
value = data[1:].decode()
if data[0] == PORT_ID_SUBTYPE_MAC:
value = str(netaddr.EUI(
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(),
dialect=netaddr.mac_unix_expanded))
if tlv_type == tlv.LLDP_TLV_PORT_ID:
try:
port_id = tlv.PortId.parse(data)
except (core.MappingError, netaddr.AddrFormatError) as e:
LOG.warning("TLV parse error for Port ID: %s", e,
node_info=node_info)
return
item = PORT_ID_ITEM_NAME
value = port_id.value
elif tlv_type == tlv.LLDP_TLV_CHASSIS_ID:
try:
chassis_id = tlv.ChassisId.parse(data)
except (core.MappingError, netaddr.AddrFormatError) as e:
LOG.warning("TLV parse error for Chassis ID: %s", e,
node_info=node_info)
return
# Only accept mac address for chassis ID
if 'mac_address' in chassis_id.subtype:
item = SWITCH_ID_ITEM_NAME
value = chassis_id.value
if item and value:
if (not CONF.processing.overwrite_existing and
@ -85,6 +91,21 @@ class GenericLocalLinkConnectionHook(base.ProcessingHook):
'path': '/local_link_connection/%s' % item,
'value': value}
def _get_lldp_processed_patch(self, name, item, lldp_proc_data, port):
if 'lldp_processed' not in lldp_proc_data:
return
value = lldp_proc_data['lldp_processed'].get(name)
if value:
if (not CONF.processing.overwrite_existing and
item in port.local_link_connection):
return
return {'op': 'add',
'path': '/local_link_connection/%s' % item,
'value': value}
def before_update(self, introspection_data, node_info, **kwargs):
"""Process LLDP data and patch Ironic port local link connection"""
inventory = utils.get_inventory(introspection_data)
@ -111,11 +132,24 @@ class GenericLocalLinkConnectionHook(base.ProcessingHook):
continue
patches = []
for tlv_type, tlv_value in lldp_data:
patch = self._get_local_link_patch(tlv_type, tlv_value, port)
# First check if lldp data was already processed by lldp_basic
# plugin which stores data in 'all_interfaces'
proc_data = introspection_data['all_interfaces'][iface['name']]
for name, item in LLDP_PROC_DATA_MAPPING.items():
patch = self._get_lldp_processed_patch(name, item,
proc_data, port)
if patch is not None:
patches.append(patch)
# If no processed lldp data was available then parse raw lldp data
if not patches:
for tlv_type, tlv_value in lldp_data:
patch = self._get_local_link_patch(tlv_type, tlv_value,
port, node_info)
if patch is not None:
patches.append(patch)
try:
# NOTE(sambetts) We need a newer version of Ironic API for this
# transaction, so create a new ironic client and explicitly

51
ironic_inspector/test/unit/test_plugins_local_link_connection.py
View File

@ -143,3 +143,54 @@ class TestGenericLocalLinkConnectionHook(test_base.NodeTest):
]
self.hook.before_update(self.data, self.node_info)
self.assertCalledWithPatch(patches, mock_patch)
@mock.patch.object(node_cache.NodeInfo, 'patch_port')
def test_processed_data_available(self, mock_patch):
self.data['all_interfaces'] = {
'em1': {"ip": self.ips[0], "mac": self.macs[0],
"lldp_processed": {
"switch_chassis_id": "11:22:33:aa:bb:dd",
"switch_port_id": "Ethernet2/66"}
}
}
patches = [
{'path': '/local_link_connection/port_id',
'value': 'Ethernet2/66', 'op': 'add'},
{'path': '/local_link_connection/switch_id',
'value': '11:22:33:aa:bb:dd', 'op': 'add'},
]
self.hook.before_update(self.data, self.node_info)
self.assertCalledWithPatch(patches, mock_patch)
@mock.patch.object(node_cache.NodeInfo, 'patch_port')
def test_processed_data_chassis_only(self, mock_patch):
self.data['all_interfaces'] = {
'em1': {"ip": self.ips[0], "mac": self.macs[0],
"lldp_processed": {
"switch_chassis_id": "11:22:33:aa:bb:dd"}
}
}
patches = [
{'path': '/local_link_connection/switch_id',
'value': '11:22:33:aa:bb:dd', 'op': 'add'}
]
self.hook.before_update(self.data, self.node_info)
self.assertCalledWithPatch(patches, mock_patch)
@mock.patch.object(node_cache.NodeInfo, 'patch_port')
def test_processed_data_port_only(self, mock_patch):
self.data['all_interfaces'] = {
'em1': {"ip": self.ips[0], "mac": self.macs[0],
"lldp_processed": {
"switch_port_id": "Ethernet2/66"}
}
}
patches = [
{'path': '/local_link_connection/port_id',
'value': 'Ethernet2/66', 'op': 'add'}
]
self.hook.before_update(self.data, self.node_info)
self.assertCalledWithPatch(patches, mock_patch)

6
releasenotes/notes/add-lldp-plugin-dependency-c323412654f71b3e.yaml Normal file
View File

@ -0,0 +1,6 @@
---
features:
- |
Add a check from the link_local_connection plugin to use data stored by the
lldp_basic plugin to avoid having to parse the LLDP packets twice.