Merge "Deprecate LLDP in inventory in favour of a new collector"

7 files changed, 72 insertions, 32 deletions

diff --git a/doc/source/admin/how_it_works.rst b/doc/source/admin/how_it_works.rst
index 803ac7a8..5f4a6773 100644
--- a/doc/source/admin/how_it_works.rst
+++ b/doc/source/admin/how_it_works.rst
@@ -145,8 +145,16 @@ collectors are:
       * ``nics`` - list of objects with keys ``name`` (NIC name) and
         ``numa_node`` (node ID).
 
+``lldp``
+    Collects information about the network connectivity using LLDP_. Provides
+    one key:
+
+    * ``lldp_raw`` - mapping of interface names to lists of raw
+      type-length-value (TLV) records.
+
 .. _hardware: https://pypi.org/project/hardware/
 .. _NUMA: https://en.wikipedia.org/wiki/Non-uniform_memory_access
+.. _LLDP: https://en.wikipedia.org/wiki/Link_Layer_Discovery_Protocol
 
 .. _hardware-inventory:
 
@@ -191,10 +199,12 @@ fields:
 ``interfaces``
     list of network interfaces with fields: ``name``, ``mac_address``,
     ``ipv4_address``, ``lldp``, ``vendor``, ``product``, and optionally
-    ``biosdevname`` (BIOS given NIC name). If configuration option
-    ``collect_lldp`` is set to True the ``lldp`` field will be populated
-    by a list of type-length-value(TLV) fields retrieved using the
-    Link Layer Discovery Protocol (LLDP).
+    ``biosdevname`` (BIOS given NIC name).
+
+    .. note::
+       For backward compatibility, interfaces may contain ``lldp`` fields.
+       They are deprecated, consumers should rely on the ``lldp`` inspection
+       collector instead.
 
 ``system_vendor``
     system vendor information from SMBIOS as reported by ``dmidecode``:
diff --git a/ironic_python_agent/config.py b/ironic_python_agent/config.py
index 9251a3e3..056cab85 100644
--- a/ironic_python_agent/config.py
+++ b/ironic_python_agent/config.py
@@ -149,7 +149,9 @@ cli_opts = [
                 help='Whether IPA should attempt to receive LLDP packets for '
                      'each network interface it discovers in the inventory. '
                      'Can be supplied as "ipa-collect-lldp" '
-                     'kernel parameter.'),
+                     'kernel parameter.',
+                deprecated_for_removal=True,
+                deprecated_reason="Use the lldp collector instead"),
 
     cfg.StrOpt('inspection_callback_url',
                default=APARAMS.get('ipa-inspection-callback-url'),
diff --git a/ironic_python_agent/hardware.py b/ironic_python_agent/hardware.py
index 8dd3a961..badb07db 100644
--- a/ironic_python_agent/hardware.py
+++ b/ironic_python_agent/hardware.py
@@ -865,6 +865,9 @@ class HardwareManager(object, metaclass=abc.ABCMeta):
     def list_network_interfaces(self):
         raise errors.IncompatibleHardwareMethodError
 
+    def collect_lldp_data(self, interface_names=None):
+        raise errors.IncompatibleHardwareMethodError
+
     def get_cpus(self):
         raise errors.IncompatibleHardwareMethodError
 
@@ -1198,7 +1201,6 @@ class GenericHardwareManager(HardwareManager):
     HARDWARE_MANAGER_VERSION = '1.1'
 
     def __init__(self):
-        self.sys_path = '/sys'
         self.lldp_data = {}
 
     def evaluate_hardware_support(self):
@@ -1213,7 +1215,7 @@ class GenericHardwareManager(HardwareManager):
         self.wait_for_disks()
         return HardwareSupport.GENERIC
 
-    def collect_lldp_data(self, interface_names):
+    def collect_lldp_data(self, interface_names=None):
         """Collect and convert LLDP info from the node.
 
         In order to process the LLDP information later, the raw data needs to
@@ -1222,7 +1224,8 @@ class GenericHardwareManager(HardwareManager):
         :param interface_names: list of names of node's interfaces.
         :return: a dict, containing the lldp data from every interface.
         """
-
+        if interface_names is None:
+            interface_names = netutils.list_interfaces()
         interface_names = [name for name in interface_names if name != 'lo']
         lldp_data = {}
         try:
@@ -1292,7 +1295,7 @@ class GenericHardwareManager(HardwareManager):
         """
         global WARN_BIOSDEVNAME_NOT_FOUND
 
-        if self._is_vlan(interface_name):
+        if netutils.is_vlan(interface_name):
             LOG.debug('Interface %s is a VLAN, biosdevname not called',
                       interface_name)
             return
@@ -1313,35 +1316,14 @@ class GenericHardwareManager(HardwareManager):
             else:
                 LOG.warning('Biosdevname returned exit code %s', e.exit_code)
 
-    def _is_device(self, interface_name):
-        device_path = '{}/class/net/{}/device'.format(self.sys_path,
-                                                      interface_name)
-        return os.path.exists(device_path)
-
-    def _is_vlan(self, interface_name):
-        # A VLAN interface does not have /device, check naming convention
-        # used when adding VLAN interface
-
-        interface, sep, vlan = interface_name.partition('.')
-
-        return vlan.isdigit()
-
-    def _is_bond(self, interface_name):
-        device_path = '{}/class/net/{}/bonding'.format(self.sys_path,
-                                                       interface_name)
-        return os.path.exists(device_path)
-
     def list_network_interfaces(self):
-        network_interfaces_list = []
-        iface_names = os.listdir('{}/class/net'.format(self.sys_path))
-        iface_names = [name for name in iface_names
-                       if self._is_vlan(name) or self._is_device(name)
-                       or self._is_bond(name)]
+        iface_names = netutils.list_interfaces()
 
         if CONF.collect_lldp:
             self.lldp_data = dispatch_to_managers('collect_lldp_data',
                                                   interface_names=iface_names)
 
+        network_interfaces_list = []
         for iface_name in iface_names:
             try:
                 result = dispatch_to_managers(
diff --git a/ironic_python_agent/inspector.py b/ironic_python_agent/inspector.py
index 8e570c3b..c2d98d5d 100644
--- a/ironic_python_agent/inspector.py
+++ b/ironic_python_agent/inspector.py
@@ -364,3 +364,12 @@ def collect_pci_devices_info(data, failures):
                                  'revision': pci_revision,
                                  'bus': subdir})
     data['pci_devices'] = pci_devices_info
+
+
+def collect_lldp(data, failures):
+    """Collect LLDP information for network interfaces.
+
+    :param data: mutable data that we'll send to inspector
+    :param failures: AccumulatedFailures object
+    """
+    data['lldp_raw'] = hardware.dispatch_to_managers('collect_lldp_data')
diff --git a/ironic_python_agent/netutils.py b/ironic_python_agent/netutils.py
index 0b595d96..2e289808 100644
--- a/ironic_python_agent/netutils.py
+++ b/ironic_python_agent/netutils.py
@@ -14,6 +14,7 @@
 
 import ctypes
 import fcntl
+import os
 import select
 import socket
 import struct
@@ -247,6 +248,29 @@ def get_hostname():
     return socket.gethostname()
 
 
+def is_network_device(interface_name):
+    device_path = f'/sys/class/net/{interface_name}/device'
+    return os.path.exists(device_path)
+
+
+def is_vlan(interface_name):
+    # A VLAN interface does not have /device, check naming convention
+    # used when adding VLAN interface
+    interface, sep, vlan = interface_name.partition('.')
+    return vlan.isdigit()
+
+
+def is_bond(interface_name):
+    device_path = f'/sys/class/net/{interface_name}/bonding'
+    return os.path.exists(device_path)
+
+
+def list_interfaces():
+    iface_names = os.listdir('/sys/class/net')
+    return [name for name in iface_names
+            if is_vlan(name) or is_network_device(name) or is_bond(name)]
+
+
 def interface_has_carrier(interface_name):
     path = '/sys/class/net/{}/carrier'.format(interface_name)
     try:
diff --git a/releasenotes/notes/lldp-raw-a09174cb930bca97.yaml b/releasenotes/notes/lldp-raw-a09174cb930bca97.yaml
new file mode 100644
index 00000000..19d668bc
--- /dev/null
+++ b/releasenotes/notes/lldp-raw-a09174cb930bca97.yaml
@@ -0,0 +1,12 @@
+---
+features:
+  - |
+    Adds a new inspection collector ``lldp`` that collects LLDP information
+    into the ``lldp_raw`` field.
+deprecations:
+  - |
+    The LLDP information as part of the general inventory is deprecated.
+    Use the new ``lldp`` inspection collector to retrieve it.
+  - |
+    The ``ipa-collect-lldp`` kernel parameter and the corresponding option are
+    now deprecated.
diff --git a/setup.cfg b/setup.cfg
index 8e9e95f3..e2a28664 100644
--- a/setup.cfg
+++ b/setup.cfg
@@ -55,6 +55,7 @@ ironic_python_agent.inspector.collectors =
     pci-devices = ironic_python_agent.inspector:collect_pci_devices_info
     numa-topology = ironic_python_agent.numa_inspector:collect_numa_topology_info
     dmi-decode = ironic_python_agent.dmi_inspector:collect_dmidecode_info
+    lldp = ironic_python_agent.inspector:collect_lldp
 
 [pbr]
 autodoc_index_modules = True