Merge "Collect NIC name given by BIOS"

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.