Support LLDP data as part of interfaces in inventory

To support multi-tenant networking in Ironic we need to be able to
discover not just the NICs a baremetal machine has but also the physical
connectivity to switches in the network.

This patch collects LLDP (Link Layer Discovery Protocol) data as part of
the list interfaces stage of the generic hardware manager. This
information can then be processed by the ironic inspector to populate
the local link information on each ironic port.

The processing done on this data in ironic python agent is limited, this
is to allow for server side processing hooks to process as much or as
little of the data as they want. This is to allow for multi-vendor
environments that might use different parts of the LLDP packet to use a
generic ramdisk and configure the processing server side using inspector
plugins.

Reserved fields switch_port_descr and switch_chassis_descr have been
deprecated for removal in Ocata in favor of passing the whole packet.

Change-Id: Idae9b1ede1797029da1bd521501b121957ca1f1a
Partial-Bug: #1526403

doc/source/index.rst

@@ -110,8 +110,12 @@ fields:
 
 ``interfaces``
     list of network interfaces with fields: ``name``, ``mac_address``,
-    ``ipv4_address``. Currently IPA also returns 2 fields ``switch_port_descr``
+    ``ipv4_address``, ``lldp``. If configuration option ``collect_lldp`` is
-    and ``switch_chassis_descr`` which are reserved for future use.
+    set to True the ``lldp`` field will be populated by a list of TLVs pulled
+    from LLDP. Currently IPA also returns 2 fields ``switch_port_descr`` and
+    ``switch_chassis_descr`` which were reserved for future use, these are now
+    deprecated to be removed in Ocata in favor of including all LLDP data in
+    the ``lddp`` field.
 
 ``system_vendor``
     system vendor information from SMBIOS as reported by ``dmidecode``:

ironic_python_agent/config.py

@@ -92,6 +92,11 @@ cli_opts = [
                                      APARAMS.get('lldp-timeout', 30.0)),
                  help='The amount of seconds to wait for LLDP packets.'),
 
+    cfg.BoolOpt('collect_lldp',
+                default=APARAMS.get('ipa-collect-lldp', False),
+                help='Whether IPA should attempt to receive LLDP packets for '
+                     'each network interface it discovers in the inventory.'),
+
     cfg.BoolOpt('standalone',
                 default=APARAMS.get('ipa-standalone', False),
                 help='Note: for debugging only. Start the Agent but suppress '

ironic_python_agent/hardware.py

@@ -13,6 +13,7 @@
 # limitations under the License.
 
 import abc
+import binascii
 import functools
 import os
 import shlex
@@ -31,6 +32,7 @@ import stevedore
 
 from ironic_python_agent import encoding
 from ironic_python_agent import errors
+from ironic_python_agent import netutils
 from ironic_python_agent import utils
 
 _global_managers = None
@@ -181,14 +183,17 @@ class BlockDevice(encoding.SerializableComparable):
 class NetworkInterface(encoding.SerializableComparable):
     serializable_fields = ('name', 'mac_address', 'switch_port_descr',
                            'switch_chassis_descr', 'ipv4_address',
-                           'has_carrier')
+                           'has_carrier', 'lldp')
 
-    def __init__(self, name, mac_addr, ipv4_address=None, has_carrier=True):
+    def __init__(self, name, mac_addr, ipv4_address=None, has_carrier=True,
+                 lldp=None):
         self.name = name
         self.mac_address = mac_addr
         self.ipv4_address = ipv4_address
         self.has_carrier = has_carrier
-        # TODO(russellhaering): Pull these from LLDP
+        self.lldp = lldp
+        # TODO(sambetts) Remove these fields in Ocata, they have been
+        # superseded by self.lldp
         self.switch_port_descr = None
         self.switch_chassis_descr = None
 
@@ -412,6 +417,7 @@ class GenericHardwareManager(HardwareManager):
 
     def __init__(self):
         self.sys_path = '/sys'
+        self.lldp_data = {}
 
     def evaluate_hardware_support(self):
         # Do some initialization before we declare ourself ready
@@ -441,6 +447,32 @@ class GenericHardwareManager(HardwareManager):
             LOG.warning('No disks detected in %d seconds',
                         CONF.disk_wait_delay * CONF.disk_wait_attempts)
 
+    def _cache_lldp_data(self, interface_names):
+        interface_names = [name for name in interface_names if name != 'lo']
+        try:
+            raw_lldp_data = netutils.get_lldp_info(interface_names)
+        except Exception:
+            # NOTE(sambetts) The get_lldp_info function will log this exception
+            # and we don't invalidate any existing data in the cache if we fail
+            # to get data to replace it so just return.
+            return
+        for ifname, tlvs in raw_lldp_data.items():
+            # NOTE(sambetts) Convert each tlv value to hex so that it can be
+            # serialised safely
+            processed_tlvs = []
+            for typ, data in tlvs:
+                try:
+                    processed_tlvs.append((typ,
+                                           binascii.hexlify(data).decode()))
+                except (binascii.Error, binascii.Incomplete) as e:
+                    LOG.warning('An error occurred while processing TLV type '
+                                '%s for interface %s: %s', (typ, ifname, e))
+            self.lldp_data[ifname] = processed_tlvs
+
+    def _get_lldp_data(self, interface_name):
+        if self.lldp_data:
+            return self.lldp_data.get(interface_name)
+
     def _get_interface_info(self, interface_name):
         addr_path = '{0}/class/net/{1}/address'.format(self.sys_path,
                                                        interface_name)
@@ -450,7 +482,8 @@ class GenericHardwareManager(HardwareManager):
         return NetworkInterface(
             interface_name, mac_addr,
             ipv4_address=self.get_ipv4_addr(interface_name),
-            has_carrier=self._interface_has_carrier(interface_name))
+            has_carrier=self._interface_has_carrier(interface_name),
+            lldp=self._get_lldp_data(interface_name))
 
     def get_ipv4_addr(self, interface_id):
         try:
@@ -478,9 +511,12 @@ class GenericHardwareManager(HardwareManager):
 
     def list_network_interfaces(self):
         iface_names = os.listdir('{0}/class/net'.format(self.sys_path))
-        return [self._get_interface_info(name)
+        iface_names = [name for name in iface_names if self._is_device(name)]
-                for name in iface_names
+
-                if self._is_device(name)]
+        if CONF.collect_lldp:
+            self._cache_lldp_data(iface_names)
+
+        return [self._get_interface_info(name) for name in iface_names]
 
     def get_cpus(self):
         lines = utils.execute('lscpu')[0]

ironic_python_agent/tests/unit/test_hardware.py

@@ -307,6 +307,75 @@ class TestGenericHardwareManager(test_base.BaseTestCase):
         self.assertEqual('eth0', interfaces[0].name)
         self.assertEqual('00:0c:29:8c:11:b1', interfaces[0].mac_address)
         self.assertEqual('192.168.1.2', interfaces[0].ipv4_address)
+        self.assertEqual(None, interfaces[0].lldp)
+        self.assertTrue(interfaces[0].has_carrier)
+
+    @mock.patch('ironic_python_agent.netutils.get_lldp_info')
+    @mock.patch('netifaces.ifaddresses')
+    @mock.patch('os.listdir')
+    @mock.patch('os.path.exists')
+    @mock.patch('six.moves.builtins.open')
+    def test_list_network_interfaces_with_lldp(self,
+                                               mocked_open,
+                                               mocked_exists,
+                                               mocked_listdir,
+                                               mocked_ifaddresses,
+                                               mocked_lldp_info):
+        CONF.set_override('collect_lldp', True)
+        mocked_listdir.return_value = ['lo', 'eth0']
+        mocked_exists.side_effect = [False, True]
+        mocked_open.return_value.__enter__ = lambda s: s
+        mocked_open.return_value.__exit__ = mock.Mock()
+        read_mock = mocked_open.return_value.read
+        read_mock.side_effect = ['00:0c:29:8c:11:b1\n', '1']
+        mocked_ifaddresses.return_value = {
+            netifaces.AF_INET: [{'addr': '192.168.1.2'}]
+        }
+        mocked_lldp_info.return_value = {'eth0': [
+            (0, b''),
+            (1, b'\x04\x88Z\x92\xecTY'),
+            (2, b'\x05Ethernet1/18'),
+            (3, b'\x00x')]
+        }
+        interfaces = self.hardware.list_network_interfaces()
+        self.assertEqual(1, len(interfaces))
+        self.assertEqual('eth0', interfaces[0].name)
+        self.assertEqual('00:0c:29:8c:11:b1', interfaces[0].mac_address)
+        self.assertEqual('192.168.1.2', interfaces[0].ipv4_address)
+        expected_lldp_info = [
+            (0, ''),
+            (1, '04885a92ec5459'),
+            (2, '0545746865726e6574312f3138'),
+            (3, '0078'),
+        ]
+        self.assertEqual(expected_lldp_info, interfaces[0].lldp)
+        self.assertTrue(interfaces[0].has_carrier)
+
+    @mock.patch('ironic_python_agent.netutils.get_lldp_info')
+    @mock.patch('netifaces.ifaddresses')
+    @mock.patch('os.listdir')
+    @mock.patch('os.path.exists')
+    @mock.patch('six.moves.builtins.open')
+    def test_list_network_interfaces_with_lldp_error(
+            self, mocked_open, mocked_exists, mocked_listdir,
+            mocked_ifaddresses, mocked_lldp_info):
+        CONF.set_override('collect_lldp', True)
+        mocked_listdir.return_value = ['lo', 'eth0']
+        mocked_exists.side_effect = [False, True]
+        mocked_open.return_value.__enter__ = lambda s: s
+        mocked_open.return_value.__exit__ = mock.Mock()
+        read_mock = mocked_open.return_value.read
+        read_mock.side_effect = ['00:0c:29:8c:11:b1\n', '1']
+        mocked_ifaddresses.return_value = {
+            netifaces.AF_INET: [{'addr': '192.168.1.2'}]
+        }
+        mocked_lldp_info.side_effect = Exception('Boom!')
+        interfaces = self.hardware.list_network_interfaces()
+        self.assertEqual(1, len(interfaces))
+        self.assertEqual('eth0', interfaces[0].name)
+        self.assertEqual('00:0c:29:8c:11:b1', interfaces[0].mac_address)
+        self.assertEqual('192.168.1.2', interfaces[0].ipv4_address)
+        self.assertEqual(None, interfaces[0].lldp)
         self.assertTrue(interfaces[0].has_carrier)
 
     @mock.patch('netifaces.ifaddresses')

ironic_python_agent/tests/unit/test_ironic_api_client.py

@@ -41,7 +41,10 @@ class TestBaseIronicPythonAgent(test_base.BaseTestCase):
         self.hardware_info = {
             'interfaces': [
                 hardware.NetworkInterface('eth0', '00:0c:29:8c:11:b1'),
-                hardware.NetworkInterface('eth1', '00:0c:29:8c:11:b2'),
+                hardware.NetworkInterface(
+                    'eth1', '00:0c:29:8c:11:b2',
+                    lldp=[(1, '04885a92ec5459'),
+                          (2, '0545746865726e6574312f3138')]),
             ],
             'cpu': hardware.CPU('Awesome Jay CPU x10 9001', '9001', '10',
                                 'ARMv9'),
@@ -162,6 +165,7 @@ class TestBaseIronicPythonAgent(test_base.BaseTestCase):
                     u'switch_chassis_descr': None,
                     u'switch_port_descr': None,
                     u'has_carrier': True,
+                    u'lldp': None,
                 },
                 {
                     u'mac_address': u'00:0c:29:8c:11:b2',
@@ -170,6 +174,8 @@ class TestBaseIronicPythonAgent(test_base.BaseTestCase):
                     u'switch_chassis_descr': None,
                     u'switch_port_descr': None,
                     u'has_carrier': True,
+                    u'lldp': [[1, u'04885a92ec5459'],
+                              [2, u'0545746865726e6574312f3138']],
                 }
             ],
             u'cpu': {
@@ -300,6 +306,7 @@ class TestBaseIronicPythonAgent(test_base.BaseTestCase):
                     u'switch_chassis_descr': None,
                     u'switch_port_descr': None,
                     u'has_carrier': True,
+                    u'lldp': None,
                 },
                 {
                     u'mac_address': u'00:0c:29:8c:11:b2',
@@ -308,6 +315,8 @@ class TestBaseIronicPythonAgent(test_base.BaseTestCase):
                     u'switch_chassis_descr': None,
                     u'switch_port_descr': None,
                     u'has_carrier': True,
+                    u'lldp': [[1, u'04885a92ec5459'],
+                              [2, u'0545746865726e6574312f3138']],
                 }
             ],
             u'cpu': {

releasenotes/notes/support-lldp-in-inventory-4ab6e45ccd35dace.yaml

@@ -0,0 +1,12 @@
+---
+features:
+  - Add support for LLDP data in the returned inventory when option
+    collect_lldp is set to True. Each interface returned in the inventory that
+    receives an LLDP packet will contain the whole LLDP packet represented as a
+    list of TLVs in the field 'lldp'.
+upgrade:
+  - Deprecated reserved fields switch_port_descr and switch_chassis_descr in
+    favor of returning the whole LLDP packet for each interface so that IPA
+    processing of this data remains generic and full processing can be
+    customised server side instead of having to create custom ramdisks. These
+    fields will be removed in Ocata.