Collect NIC name given by BIOS

Adds an extra field ``biosdevname`` to network interface inventory
collected by ``default`` inspection collector (which collects the whole
inventory returned by hardware manager) of ironic-python-agent.

This feature requires biosdevname utility to collect the bios given NIC
names. The tooling module for tinyIPA is created for the same purpose.
For CoreOS IPA pxe images, biosdevname tooling module is limited,
because Docker repository is created and embedded into CoreOS pxe
images. The Docker repository uses debian to download the packages.
Debian does not have biosdevname package.

Adds an export variable TINYIPA_REQUIRE_BIOSDEVNAME. Set this
variable to ``true`` in your shell before building tinyIPA.

Closes-Bug: #1635351
Change-Id: Ia96af59e2a74868cac59e5a88cfbb3be60d85687

doc/source/index.rst

@@ -118,11 +118,11 @@ fields:
 
 ``interfaces``
     list of network interfaces with fields: ``name``, ``mac_address``,
-    ``ipv4_address``, ``lldp``, ``vendor`` and ``product``.
-    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).
-
+    ``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).
 
 ``system_vendor``
     system vendor information from SMBIOS as reported by ``dmidecode``:

imagebuild/tinyipa/README.rst

@@ -105,3 +105,13 @@ To provide other public SSH key, export path to it in your shell before
 building tinyipa as follows::
 
   export SSH_PUBLIC_KEY=<full-path-to-public-key>
+
+
+Enabling biosdevname in the ramdisk
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+If you want to collect BIOS given names of NICs in the inventory, set
+``TINYIPA_REQUIRE_BIOSDEVNAME`` variable in your shell before building the
+tinyipa::
+
+  export TINYIPA_REQUIRE_BIOSDEVNAME=true

imagebuild/tinyipa/build-tinyipa.sh

@@ -6,6 +6,7 @@ source ${WORKDIR}/tc-mirror.sh
 BUILDDIR="$WORKDIR/tinyipabuild"
 BUILD_AND_INSTALL_TINYIPA=${BUILD_AND_INSTALL_TINYIPA:-false}
 TINYCORE_MIRROR_URL=${TINYCORE_MIRROR_URL:-}
+TINYIPA_REQUIRE_BIOSDEVNAME=${TINYIPA_REQUIRE_BIOSDEVNAME:-false}
 
 CHROOT_PATH="/tmp/overides:/usr/local/sbin:/usr/local/bin:/apps/bin:/usr/sbin:/usr/bin:/sbin:/bin"
 CHROOT_CMD="sudo chroot $BUILDDIR /usr/bin/env -i PATH=$CHROOT_PATH http_proxy=$http_proxy https_proxy=$https_proxy no_proxy=$no_proxy"
@@ -57,9 +58,12 @@ sudo sh -c "echo $TINYCORE_MIRROR_URL > $BUILDDIR/opt/tcemirror"
 # Download get-pip into ramdisk
 ( cd "$BUILDDIR/tmp" && wget https://bootstrap.pypa.io/get-pip.py )
 
-# Download TGT and Qemu-utils source
+# Download TGT, Qemu-utils, and Biosdevname source
 clone_and_checkout "https://github.com/fujita/tgt.git" "${BUILDDIR}/tmp/tgt" "v1.0.62"
 clone_and_checkout "https://github.com/qemu/qemu.git" "${BUILDDIR}/tmp/qemu" "v2.5.0"
+if $TINYIPA_REQUIRE_BIOSDEVNAME; then
+    wget -N -O - https://linux.dell.com/biosdevname/biosdevname-0.7.2/biosdevname-0.7.2.tar.gz | tar -xz -C "${BUILDDIR}/tmp" -f -
+fi
 
 # Create directory for python local mirror
 mkdir -p "$BUILDDIR/tmp/localpip"
@@ -114,3 +118,11 @@ cd $WORKDIR/build_files && mksquashfs $BUILDDIR/tmp/qemu-utils qemu-utils.tcz &&
 
 # Create qemu-utils.tcz.dep
 echo "glib2.tcz" > qemu-utils.tcz.dep
+
+# Build biosdevname
+if $TINYIPA_REQUIRE_BIOSDEVNAME; then
+    rm -rf $WORKDIR/build_files/biosdevname.tcz
+    $CHROOT_CMD /bin/sh -c "cd /tmp/biosdevname-* && ./configure && make && make install DESTDIR=/tmp/biosdevname-installed"
+    find $BUILDDIR/tmp/biosdevname-installed/ -type f -executable | xargs file | awk -F ':' '/ELF/ {print $1}' | sudo xargs strip
+    cd $WORKDIR/build_files && mksquashfs $BUILDDIR/tmp/biosdevname-installed biosdevname.tcz && md5sum biosdevname.tcz > biosdevname.tcz.md5.txt
+fi

imagebuild/tinyipa/build_files/buildreqs.lst

@@ -7,6 +7,8 @@ hdparm.tcz
 parted.tcz
 python.tcz
 python-dev.tcz
+pciutils.tcz
+libpci-dev.tcz
 raid-dm-4.2.9-tinycore64.tcz
 scsi-4.2.9-tinycore64.tcz
 udev-lib.tcz

imagebuild/tinyipa/build_files/finalreqs.lst

@@ -7,6 +7,7 @@ iproute2.tcz
 parted.tcz
 popt.tcz
 python.tcz
+pciutils.tcz
 raid-dm-4.2.9-tinycore64.tcz
 scsi-4.2.9-tinycore64.tcz
 udev-lib.tcz

imagebuild/tinyipa/finalise-tinyipa.sh

@@ -10,6 +10,7 @@ TINYCORE_MIRROR_URL=${TINYCORE_MIRROR_URL:-}
 ENABLE_SSH=${ENABLE_SSH:-false}
 SSH_PUBLIC_KEY=${SSH_PUBLIC_KEY:-}
 PYOPTIMIZE_TINYIPA=${PYOPTIMIZE_TINYIPA:-true}
+TINYIPA_REQUIRE_BIOSDEVNAME=${TINYIPA_REQUIRE_BIOSDEVNAME:-false}
 
 TC=1001
 STAFF=50
@@ -86,6 +87,9 @@ echo "tc" | $CHROOT_CMD tee -a /etc/sysconfig/tcuser
 
 cp $WORKDIR/build_files/tgt.* $FINALDIR/tmp/builtin/optional
 cp $WORKDIR/build_files/qemu-utils.* $FINALDIR/tmp/builtin/optional
+if $TINYIPA_REQUIRE_BIOSDEVNAME; then
+    cp $WORKDIR/build_files/biosdevname.* $FINALDIR/tmp/builtin/optional
+fi
 
 # Mount /proc for chroot commands
 sudo mount --bind /proc $FINALDIR/proc
@@ -123,6 +127,9 @@ fi
 
 $TC_CHROOT_CMD tce-load -ic /tmp/builtin/optional/tgt.tcz
 $TC_CHROOT_CMD tce-load -ic /tmp/builtin/optional/qemu-utils.tcz
+if $TINYIPA_REQUIRE_BIOSDEVNAME; then
+    $TC_CHROOT_CMD tce-load -ic /tmp/builtin/optional/biosdevname.tcz
+fi
 
 # Ensure tinyipa picks up installed kernel modules
 $CHROOT_CMD depmod -a `$WORKDIR/build_files/fakeuname -r`

ironic_python_agent/hardware.py

@@ -218,10 +218,11 @@ class BlockDevice(encoding.SerializableComparable):
 class NetworkInterface(encoding.SerializableComparable):
     serializable_fields = ('name', 'mac_address', 'ipv4_address',
                            'has_carrier', 'lldp', 'vendor', 'product',
-                           'client_id')
+                           'client_id', 'biosdevname')
 
     def __init__(self, name, mac_addr, ipv4_address=None, has_carrier=True,
-                 lldp=None, vendor=None, product=None, client_id=None):
+                 lldp=None, vendor=None, product=None, client_id=None,
+                 biosdevname=None):
         self.name = name
         self.mac_address = mac_addr
         self.ipv4_address = ipv4_address
@@ -229,6 +230,7 @@ class NetworkInterface(encoding.SerializableComparable):
         self.lldp = lldp
         self.vendor = vendor
         self.product = product
+        self.biosdevname = biosdevname
         # client_id is used for InfiniBand only. we calculate the DHCP
         # client identifier Option to allow DHCP to work over InfiniBand.
         # see https://tools.ietf.org/html/rfc4390
@@ -531,11 +533,41 @@ class GenericHardwareManager(HardwareManager):
             ipv4_address=self.get_ipv4_addr(interface_name),
             has_carrier=netutils.interface_has_carrier(interface_name),
             vendor=_get_device_info(interface_name, 'net', 'vendor'),
-            product=_get_device_info(interface_name, 'net', 'device'))
+            product=_get_device_info(interface_name, 'net', 'device'),
+            biosdevname=self.get_bios_given_nic_name(interface_name))
 
     def get_ipv4_addr(self, interface_id):
         return netutils.get_ipv4_addr(interface_id)
 
+    def get_bios_given_nic_name(self, interface_name):
+        """Collect the BIOS given NICs name.
+
+        This function uses the biosdevname utility to collect the BIOS given
+        name of network interfaces.
+
+        The collected data is added to the network interface inventory with an
+        extra field named ``biosdevname``.
+
+        :param interface_name: list of names of node's interfaces.
+        :return: the BIOS given NIC name of node's interfaces or default
+                 as None.
+        """
+        try:
+            stdout, _ = utils.execute('biosdevname', '-i',
+                                      interface_name)
+            return stdout.rstrip('\n')
+        except OSError:
+            LOG.warning("Executable 'biosdevname' not found")
+            return
+        except processutils.ProcessExecutionError as e:
+            # NOTE(alezil) biosdevname returns 4 if running in a
+            # virtual machine.
+            if e.exit_code == 4:
+                LOG.info('The system is a virtual machine, so biosdevname '
+                         'utility does not provide names for virtual NICs.')
+            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)

ironic_python_agent/tests/unit/test_agent.py

@@ -385,15 +385,14 @@ class TestBaseAgent(ironic_agent_base.IronicAgentTest):
         mock_dispatch.assert_has_calls(expected_dispatch_calls)
         mock_sleep.assert_has_calls(expected_sleep_calls)
 
-    @mock.patch('ironic_python_agent.hardware_managers.cna._detect_cna_card',
-                autospec=True)
+    @mock.patch.object(hardware, 'load_managers', autospec=True)
     @mock.patch.object(time, 'sleep', autospec=True)
-    @mock.patch('wsgiref.simple_server.make_server', autospec=True)
-    @mock.patch.object(hardware, '_check_for_iscsi', autospec=True)
-    @mock.patch.object(hardware.HardwareManager, 'list_hardware_info',
+    @mock.patch.object(agent.IronicPythonAgent, '_wait_for_interface',
                        autospec=True)
-    def test_run_with_sleep(self, mock_check_for_iscsi, mock_list_hardware,
-                            mock_make_server, mock_sleep, mock_cna):
+    @mock.patch.object(hardware, 'dispatch_to_managers', autospec=True)
+    @mock.patch('wsgiref.simple_server.make_server', autospec=True)
+    def test_run_with_sleep(self, mock_make_server, mock_dispatch,
+                            mock_load_managers, mock_sleep, mock_wait):
         CONF.set_override('inspection_callback_url', '', enforce_type=True)
         wsgi_server = mock_make_server.return_value
         wsgi_server.start.side_effect = KeyboardInterrupt()
@@ -409,7 +408,6 @@ class TestBaseAgent(ironic_agent_base.IronicAgentTest):
                 'heartbeat_timeout': 300
             }
         }
-        mock_cna.return_value = False
         self.agent.run()
 
         listen_addr = agent.Host('192.0.2.1', 9999)
@@ -422,7 +420,9 @@ class TestBaseAgent(ironic_agent_base.IronicAgentTest):
 
         self.agent.heartbeater.start.assert_called_once_with()
         mock_sleep.assert_called_once_with(10)
-        self.assertTrue(mock_check_for_iscsi.called)
+        self.assertTrue(mock_load_managers.called)
+        self.assertTrue(mock_wait.called)
+        mock_dispatch.assert_called_once_with('list_hardware_info')
 
     def test_async_command_success(self):
         result = base.AsyncCommandResult('foo_command', {'fail': False},

ironic_python_agent/tests/unit/test_hardware.py

@@ -378,7 +378,9 @@ class TestGenericHardwareManager(base.IronicAgentTest):
     @mock.patch('os.listdir', autospec=True)
     @mock.patch('os.path.exists', autospec=True)
     @mock.patch('six.moves.builtins.open', autospec=True)
+    @mock.patch.object(utils, 'execute', autospec=True)
     def test_list_network_interfaces(self,
+                                     mocked_execute,
                                      mocked_open,
                                      mocked_exists,
                                      mocked_listdir,
@@ -394,6 +396,7 @@ class TestGenericHardwareManager(base.IronicAgentTest):
         mocked_ifaddresses.return_value = {
             netifaces.AF_INET: [{'addr': '192.168.1.2'}]
         }
+        mocked_execute.return_value = ('em0\n', '')
         interfaces = self.hardware.list_network_interfaces()
         self.assertEqual(1, len(interfaces))
         self.assertEqual('eth0', interfaces[0].name)
@@ -401,6 +404,92 @@ class TestGenericHardwareManager(base.IronicAgentTest):
         self.assertEqual('192.168.1.2', interfaces[0].ipv4_address)
         self.assertIsNone(interfaces[0].lldp)
         self.assertTrue(interfaces[0].has_carrier)
+        self.assertEqual('em0', interfaces[0].biosdevname)
+
+    @mock.patch('ironic_python_agent.hardware._get_managers', autospec=True)
+    @mock.patch('netifaces.ifaddresses', autospec=True)
+    @mock.patch('os.listdir', autospec=True)
+    @mock.patch('os.path.exists', autospec=True)
+    @mock.patch('six.moves.builtins.open', autospec=True)
+    @mock.patch.object(utils, 'execute', autospec=True)
+    def test_list_network_interfaces_with_biosdevname(self,
+                                                      mocked_execute,
+                                                      mocked_open,
+                                                      mocked_exists,
+                                                      mocked_listdir,
+                                                      mocked_ifaddresses,
+                                                      mocked_get_managers):
+        mocked_get_managers.return_value = [hardware.GenericHardwareManager()]
+        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_execute.return_value = ('em0\n', '')
+
+        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.assertIsNone(interfaces[0].lldp)
+        self.assertTrue(interfaces[0].has_carrier)
+        self.assertEqual('em0', interfaces[0].biosdevname)
+
+    @mock.patch.object(utils, 'execute', autospec=True)
+    def test_get_bios_given_nic_name_ok(self, mock_execute):
+        interface_name = 'eth0'
+        mock_execute.return_value = ('em0\n', '')
+        result = self.hardware.get_bios_given_nic_name(interface_name)
+        self.assertEqual('em0', result)
+        mock_execute.assert_called_once_with('biosdevname', '-i',
+                                             interface_name)
+
+    @mock.patch.object(utils, 'execute', autospec=True)
+    def test_get_bios_given_nic_name_oserror(self, mock_execute):
+        interface_name = 'eth0'
+        mock_execute.side_effect = OSError()
+        result = self.hardware.get_bios_given_nic_name(interface_name)
+        self.assertIsNone(result)
+        mock_execute.assert_called_once_with('biosdevname', '-i',
+                                             interface_name)
+
+    @mock.patch.object(utils, 'execute', autospec=True)
+    @mock.patch.object(hardware, 'LOG', autospec=True)
+    def test_get_bios_given_nic_name_process_exec_err4(self, mock_log,
+                                                       mock_execute):
+        interface_name = 'eth0'
+        mock_execute.side_effect = [
+            processutils.ProcessExecutionError(exit_code=4)]
+
+        result = self.hardware.get_bios_given_nic_name(interface_name)
+
+        mock_log.info.assert_called_once_with(
+            'The system is a virtual machine, so biosdevname utility does '
+            'not provide names for virtual NICs.')
+        self.assertIsNone(result)
+        mock_execute.assert_called_once_with('biosdevname', '-i',
+                                             interface_name)
+
+    @mock.patch.object(utils, 'execute', autospec=True)
+    @mock.patch.object(hardware, 'LOG', autospec=True)
+    def test_get_bios_given_nic_name_process_exec_err3(self, mock_log,
+                                                       mock_execute):
+        interface_name = 'eth0'
+        mock_execute.side_effect = [
+            processutils.ProcessExecutionError(exit_code=3)]
+
+        result = self.hardware.get_bios_given_nic_name(interface_name)
+
+        mock_log.warning.assert_called_once_with(
+            'Biosdevname returned exit code %s', 3)
+        self.assertIsNone(result)
+        mock_execute.assert_called_once_with('biosdevname', '-i',
+                                             interface_name)
 
     @mock.patch('ironic_python_agent.hardware._get_managers', autospec=True)
     @mock.patch('ironic_python_agent.netutils.get_lldp_info', autospec=True)
@@ -408,7 +497,9 @@ class TestGenericHardwareManager(base.IronicAgentTest):
     @mock.patch('os.listdir', autospec=True)
     @mock.patch('os.path.exists', autospec=True)
     @mock.patch('six.moves.builtins.open', autospec=True)
+    @mock.patch.object(utils, 'execute', autospec=True)
     def test_list_network_interfaces_with_lldp(self,
+                                               mocked_execute,
                                                mocked_open,
                                                mocked_exists,
                                                mocked_listdir,
@@ -432,6 +523,7 @@ class TestGenericHardwareManager(base.IronicAgentTest):
             (2, b'\x05Ethernet1/18'),
             (3, b'\x00x')]
         }
+        mocked_execute.return_value = ('em0\n', '')
         interfaces = self.hardware.list_network_interfaces()
         self.assertEqual(1, len(interfaces))
         self.assertEqual('eth0', interfaces[0].name)
@@ -445,6 +537,7 @@ class TestGenericHardwareManager(base.IronicAgentTest):
         ]
         self.assertEqual(expected_lldp_info, interfaces[0].lldp)
         self.assertTrue(interfaces[0].has_carrier)
+        self.assertEqual('em0', interfaces[0].biosdevname)
 
     @mock.patch('ironic_python_agent.hardware._get_managers', autospec=True)
     @mock.patch('ironic_python_agent.netutils.get_lldp_info', autospec=True)
@@ -452,8 +545,9 @@ class TestGenericHardwareManager(base.IronicAgentTest):
     @mock.patch('os.listdir', autospec=True)
     @mock.patch('os.path.exists', autospec=True)
     @mock.patch('six.moves.builtins.open', autospec=True)
+    @mock.patch.object(utils, 'execute', autospec=True)
     def test_list_network_interfaces_with_lldp_error(
-            self, mocked_open, mocked_exists, mocked_listdir,
+            self, mocked_execute, mocked_open, mocked_exists, mocked_listdir,
             mocked_ifaddresses, mocked_lldp_info, mocked_get_managers):
         mocked_get_managers.return_value = [hardware.GenericHardwareManager()]
         CONF.set_override('collect_lldp', True)
@@ -467,6 +561,7 @@ class TestGenericHardwareManager(base.IronicAgentTest):
             netifaces.AF_INET: [{'addr': '192.168.1.2'}]
         }
         mocked_lldp_info.side_effect = Exception('Boom!')
+        mocked_execute.return_value = ('em0\n', '')
         interfaces = self.hardware.list_network_interfaces()
         self.assertEqual(1, len(interfaces))
         self.assertEqual('eth0', interfaces[0].name)
@@ -474,13 +569,16 @@ class TestGenericHardwareManager(base.IronicAgentTest):
         self.assertEqual('192.168.1.2', interfaces[0].ipv4_address)
         self.assertIsNone(interfaces[0].lldp)
         self.assertTrue(interfaces[0].has_carrier)
+        self.assertEqual('em0', interfaces[0].biosdevname)
 
     @mock.patch('ironic_python_agent.hardware._get_managers', autospec=True)
     @mock.patch('netifaces.ifaddresses', autospec=True)
     @mock.patch('os.listdir', autospec=True)
     @mock.patch('os.path.exists', autospec=True)
     @mock.patch('six.moves.builtins.open', autospec=True)
+    @mock.patch.object(utils, 'execute', autospec=True)
     def test_list_network_interfaces_no_carrier(self,
+                                                mocked_execute,
                                                 mocked_open,
                                                 mocked_exists,
                                                 mocked_listdir,
@@ -497,6 +595,7 @@ class TestGenericHardwareManager(base.IronicAgentTest):
         mocked_ifaddresses.return_value = {
             netifaces.AF_INET: [{'addr': '192.168.1.2'}]
         }
+        mocked_execute.return_value = ('em0\n', '')
         interfaces = self.hardware.list_network_interfaces()
         self.assertEqual(1, len(interfaces))
         self.assertEqual('eth0', interfaces[0].name)
@@ -504,13 +603,16 @@ class TestGenericHardwareManager(base.IronicAgentTest):
         self.assertEqual('192.168.1.2', interfaces[0].ipv4_address)
         self.assertFalse(interfaces[0].has_carrier)
         self.assertIsNone(interfaces[0].vendor)
+        self.assertEqual('em0', interfaces[0].biosdevname)
 
     @mock.patch('ironic_python_agent.hardware._get_managers', autospec=True)
     @mock.patch('netifaces.ifaddresses', autospec=True)
     @mock.patch('os.listdir', autospec=True)
     @mock.patch('os.path.exists', autospec=True)
     @mock.patch('six.moves.builtins.open', autospec=True)
+    @mock.patch.object(utils, 'execute', autospec=True)
     def test_list_network_interfaces_with_vendor_info(self,
+                                                      mocked_execute,
                                                       mocked_open,
                                                       mocked_exists,
                                                       mocked_listdir,
@@ -527,6 +629,7 @@ class TestGenericHardwareManager(base.IronicAgentTest):
         mocked_ifaddresses.return_value = {
             netifaces.AF_INET: [{'addr': '192.168.1.2'}]
         }
+        mocked_execute.return_value = ('em0\n', '')
         interfaces = self.hardware.list_network_interfaces()
         self.assertEqual(1, len(interfaces))
         self.assertEqual('eth0', interfaces[0].name)
@@ -535,6 +638,7 @@ class TestGenericHardwareManager(base.IronicAgentTest):
         self.assertTrue(interfaces[0].has_carrier)
         self.assertEqual('0x15b3', interfaces[0].vendor)
         self.assertEqual('0x1014', interfaces[0].product)
+        self.assertEqual('em0', interfaces[0].biosdevname)
 
     @mock.patch.object(hardware, 'get_cached_node', autospec=True)
     @mock.patch.object(utils, 'execute', autospec=True)

releasenotes/notes/Collect_NIC_name_given_by_BIOS-657c68c0ae16365b.yaml

@@ -0,0 +1,11 @@
+---
+features:
+  - Adds an extra field ``biosdevname`` (BIOS given NICs name) to network
+    interface inventory collected by ``default`` collector of
+    ironic-python-agent. Biosdevname utility is used for collecting bios given
+    NICs name.
+
+issues:
+  - Collecting the 'biosdevname' field on network interfaces is impossible on any
+    Debian-based images due to the missing 'biosdevname' utility. This includes
+    the CoreOS image, as the CoreOS image utilizes a Debian-based chroot.