From fe3b63036095469fa0790202f2927f328a8db97b Mon Sep 17 00:00:00 2001 From: Moshe Levi Date: Thu, 11 Feb 2016 14:00:01 +0200 Subject: [PATCH] Add vendor, product to interface information This patch updates the interfaces JSON to include vendor and product u'interfaces': [ { u'mac_address': u'00:0c:29:8c:11:b1', u'name': u'eth0', u'ipv4_address': None, u'switch_chassis_descr': None, u'switch_port_descr': None, u'lldp': None, u'vendor': u'0x15b3', u'product': u'0x1014, }] Co-Authored-By: yogananth subramanian Closes-Bug: #1611856 Change-Id: I8fa58dde29601abee959f74b69b692ed9eaffb94 --- doc/source/index.rst | 12 +++-- ironic_python_agent/hardware.py | 24 ++++++---- .../tests/unit/test_hardware.py | 46 ++++++++++++++++--- .../tests/unit/test_ironic_api_client.py | 11 ++++- ...e_vendor_and_product-74e9815f20ee0cac.yaml | 4 ++ 5 files changed, 75 insertions(+), 22 deletions(-) create mode 100644 releasenotes/notes/add_interface_vendor_and_product-74e9815f20ee0cac.yaml diff --git a/doc/source/index.rst b/doc/source/index.rst index 241806f37..0062380d3 100644 --- a/doc/source/index.rst +++ b/doc/source/index.rst @@ -118,13 +118,15 @@ fields: ``interfaces`` list of network interfaces with fields: ``name``, ``mac_address``, - ``ipv4_address``, ``lldp``. 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). Currently IPA also returns 2 fields ``switch_port_descr`` + ``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). + 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. + in the ``lldp`` field. + ``system_vendor`` system vendor information from SMBIOS as reported by ``dmidecode``: diff --git a/ironic_python_agent/hardware.py b/ironic_python_agent/hardware.py index bb5cf5176..aad477712 100644 --- a/ironic_python_agent/hardware.py +++ b/ironic_python_agent/hardware.py @@ -48,14 +48,17 @@ UNIT_CONVERTER.define('GB = 1024 MB') NODE = None -def _get_device_vendor(dev): - """Get the vendor name of a given device.""" +def _get_device_info(dev, devclass, field): + """Get the device info according to device class and field.""" try: devname = os.path.basename(dev) - with open('/sys/class/block/%s/device/vendor' % devname, 'r') as f: + with open('/sys/class/%s/%s/device/%s' % (devclass, devname, field), + 'r') as f: return f.read().strip() except IOError: - LOG.warning("Can't find the device vendor for device %s", dev) + LOG.warning( + "Can't find field {0} for device {1} in device class {2}".format( + field, dev, devclass)) def _udev_settle(): @@ -157,7 +160,8 @@ def list_all_block_devices(block_type='disk'): model=device['MODEL'], size=int(device['SIZE']), rotational=bool(int(device['ROTA'])), - vendor=_get_device_vendor(device['KNAME']), + vendor=_get_device_info(device['KNAME'], + 'block', 'vendor'), **extra)) return devices @@ -205,15 +209,17 @@ class BlockDevice(encoding.SerializableComparable): class NetworkInterface(encoding.SerializableComparable): serializable_fields = ('name', 'mac_address', 'switch_port_descr', 'switch_chassis_descr', 'ipv4_address', - 'has_carrier', 'lldp') + 'has_carrier', 'lldp', 'vendor', 'product') def __init__(self, name, mac_addr, ipv4_address=None, has_carrier=True, - lldp=None): + lldp=None, vendor=None, product=None): self.name = name self.mac_address = mac_addr self.ipv4_address = ipv4_address self.has_carrier = has_carrier self.lldp = lldp + self.vendor = vendor + self.product = product # TODO(sambetts) Remove these fields in Ocata, they have been # superseded by self.lldp self.switch_port_descr = None @@ -501,7 +507,9 @@ class GenericHardwareManager(HardwareManager): interface_name, mac_addr, ipv4_address=self.get_ipv4_addr(interface_name), has_carrier=self._interface_has_carrier(interface_name), - lldp=self._get_lldp_data(interface_name)) + lldp=self._get_lldp_data(interface_name), + vendor=_get_device_info(interface_name, 'net', 'vendor'), + product=_get_device_info(interface_name, 'net', 'device')) def get_ipv4_addr(self, interface_id): try: diff --git a/ironic_python_agent/tests/unit/test_hardware.py b/ironic_python_agent/tests/unit/test_hardware.py index a429c7589..548c850ce 100644 --- a/ironic_python_agent/tests/unit/test_hardware.py +++ b/ironic_python_agent/tests/unit/test_hardware.py @@ -423,6 +423,35 @@ class TestGenericHardwareManager(test_base.BaseTestCase): self.assertEqual('00:0c:29:8c:11:b1', interfaces[0].mac_address) self.assertEqual('192.168.1.2', interfaces[0].ipv4_address) self.assertFalse(interfaces[0].has_carrier) + self.assertIsNone(interfaces[0].vendor) + + @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_vendor_info(self, + mocked_open, + mocked_exists, + mocked_listdir, + mocked_ifaddresses): + 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 + mac = '00:0c:29:8c:11:b1' + read_mock.side_effect = [mac + '\n', '1', '0x15b3\n', '0x1014\n'] + mocked_ifaddresses.return_value = { + netifaces.AF_INET: [{'addr': '192.168.1.2'}] + } + interfaces = self.hardware.list_network_interfaces() + self.assertEqual(1, len(interfaces)) + self.assertEqual('eth0', interfaces[0].name) + self.assertEqual(mac, interfaces[0].mac_address) + self.assertEqual('192.168.1.2', interfaces[0].ipv4_address) + self.assertTrue(interfaces[0].has_carrier) + self.assertEqual('0x15b3', interfaces[0].vendor) + self.assertEqual('0x1014', interfaces[0].product) @mock.patch.object(hardware, 'get_cached_node') @mock.patch.object(utils, 'execute') @@ -558,11 +587,12 @@ class TestGenericHardwareManager(test_base.BaseTestCase): mock_cached_node.assert_called_once_with() mock_dev.assert_called_once_with() - def test__get_device_vendor(self): + def test__get_device_info(self): fileobj = mock.mock_open(read_data='fake-vendor') with mock.patch( 'six.moves.builtins.open', fileobj, create=True) as mock_open: - vendor = hardware._get_device_vendor('/dev/sdfake') + vendor = hardware._get_device_info( + '/dev/sdfake', 'block', 'vendor') mock_open.assert_called_once_with( '/sys/class/block/sdfake/device/vendor', 'r') self.assertEqual('fake-vendor', vendor) @@ -692,7 +722,7 @@ class TestGenericHardwareManager(test_base.BaseTestCase): list_mock.assert_called_once_with() - @mock.patch.object(hardware, '_get_device_vendor') + @mock.patch.object(hardware, '_get_device_info') @mock.patch.object(pyudev.Device, 'from_device_file') @mock.patch.object(utils, 'execute') def test_list_all_block_device(self, mocked_execute, mocked_udev, @@ -732,7 +762,7 @@ class TestGenericHardwareManager(test_base.BaseTestCase): self.assertEqual(getattr(expected, attr), getattr(device, attr)) - @mock.patch.object(hardware, '_get_device_vendor') + @mock.patch.object(hardware, '_get_device_info') @mock.patch.object(pyudev.Device, 'from_device_file') @mock.patch.object(utils, 'execute') def test_list_all_block_device_udev_17(self, mocked_execute, mocked_udev, @@ -744,7 +774,7 @@ class TestGenericHardwareManager(test_base.BaseTestCase): devices = hardware.list_all_block_devices() self.assertEqual(4, len(devices)) - @mock.patch.object(hardware, '_get_device_vendor') + @mock.patch.object(hardware, '_get_device_info') @mock.patch.object(pyudev.Device, 'from_device_file') @mock.patch.object(utils, 'execute') def test_list_all_block_device_with_udev(self, mocked_execute, mocked_udev, @@ -1461,7 +1491,8 @@ class TestGenericHardwareManager(test_base.BaseTestCase): @mock.patch.object(utils, 'execute', autospec=True) class TestModuleFunctions(test_base.BaseTestCase): - @mock.patch.object(hardware, '_get_device_vendor', lambda x: "FooTastic") + @mock.patch.object(hardware, '_get_device_info', + lambda x, y, z: 'FooTastic') @mock.patch.object(hardware, '_udev_settle', autospec=True) @mock.patch.object(hardware.pyudev.Device, "from_device_file") def test_list_all_block_devices_success(self, mocked_fromdevfile, @@ -1475,7 +1506,8 @@ class TestModuleFunctions(test_base.BaseTestCase): self.assertEqual(BLK_DEVICE_TEMPLATE_SMALL_DEVICES, result) mocked_udev.assert_called_once_with() - @mock.patch.object(hardware, '_get_device_vendor', lambda x: "FooTastic") + @mock.patch.object(hardware, '_get_device_info', + lambda x, y: "FooTastic") @mock.patch.object(hardware, '_udev_settle', autospec=True) def test_list_all_block_devices_wrong_block_type(self, mocked_udev, mocked_execute): diff --git a/ironic_python_agent/tests/unit/test_ironic_api_client.py b/ironic_python_agent/tests/unit/test_ironic_api_client.py index bc674016e..703ae9493 100644 --- a/ironic_python_agent/tests/unit/test_ironic_api_client.py +++ b/ironic_python_agent/tests/unit/test_ironic_api_client.py @@ -41,11 +41,14 @@ class TestBaseIronicPythonAgent(test_base.BaseTestCase): self.api_client = ironic_api_client.APIClient(API_URL, DRIVER) self.hardware_info = { 'interfaces': [ - hardware.NetworkInterface('eth0', '00:0c:29:8c:11:b1'), + hardware.NetworkInterface( + 'eth0', '00:0c:29:8c:11:b1', vendor='0x15b3', + product='0x1014'), hardware.NetworkInterface( 'eth1', '00:0c:29:8c:11:b2', lldp=[(1, '04885a92ec5459'), - (2, '0545746865726e6574312f3138')]), + (2, '0545746865726e6574312f3138')], + vendor='0x15b3', product='0x1014'), ], 'cpu': hardware.CPU('Awesome Jay CPU x10 9001', '9001', '10', 'ARMv9'), @@ -298,6 +301,8 @@ class TestBaseIronicPythonAgent(test_base.BaseTestCase): u'switch_port_descr': None, u'has_carrier': True, u'lldp': None, + u'vendor': u'0x15b3', + u'product': u'0x1014' }, { u'mac_address': u'00:0c:29:8c:11:b2', @@ -308,6 +313,8 @@ class TestBaseIronicPythonAgent(test_base.BaseTestCase): u'has_carrier': True, u'lldp': [[1, u'04885a92ec5459'], [2, u'0545746865726e6574312f3138']], + u'vendor': u'0x15b3', + u'product': u'0x1014' } ], u'cpu': { diff --git a/releasenotes/notes/add_interface_vendor_and_product-74e9815f20ee0cac.yaml b/releasenotes/notes/add_interface_vendor_and_product-74e9815f20ee0cac.yaml new file mode 100644 index 000000000..8da1a41d9 --- /dev/null +++ b/releasenotes/notes/add_interface_vendor_and_product-74e9815f20ee0cac.yaml @@ -0,0 +1,4 @@ +--- +features: + - Add 'vendor' and 'product' fields to + interfaces data for future use in Ironic Inspector. \ No newline at end of file