From b272426562f4a7b123710de2cf1101136ba041d2 Mon Sep 17 00:00:00 2001 From: Lucas Alvares Gomes Date: Wed, 7 Dec 2016 10:50:45 +0000 Subject: [PATCH] Add HCTL to BlockDevices This patch is adding a "hctl" attribute to the BlockDevices. HCTL stands for: Host, Channel, Target and Lun, which is basically the SCSI address. The idea behind this patch is to allow root device hints to find the disk for deployment based on the SCSI address. Partial-Bug: #1648036 Change-Id: If8897c68609e0df0378ee919b803ca5e497def02 --- doc/source/index.rst | 2 +- ironic_python_agent/hardware.py | 15 ++++- .../tests/unit/test_hardware.py | 59 +++++++++++++++---- .../block-device-hctl-e81573812be3d469.yaml | 4 ++ 4 files changed, 65 insertions(+), 15 deletions(-) create mode 100644 releasenotes/notes/block-device-hctl-e81573812be3d469.yaml diff --git a/doc/source/index.rst b/doc/source/index.rst index 133c24a89..d322deda1 100644 --- a/doc/source/index.rst +++ b/doc/source/index.rst @@ -114,7 +114,7 @@ fields: ``disks`` list of disk block devices with fields: ``name``, ``model``, ``size`` (in bytes), ``rotational`` (boolean), ``wwn``, ``serial``, - ``vendor``, ``wwn_with_extension``, ``wwn_vendor_extension``. + ``vendor``, ``wwn_with_extension``, ``wwn_vendor_extension``, ``hctl``. ``interfaces`` list of network interfaces with fields: ``name``, ``mac_address``, diff --git a/ironic_python_agent/hardware.py b/ironic_python_agent/hardware.py index f30594993..cb3eb6a07 100644 --- a/ironic_python_agent/hardware.py +++ b/ironic_python_agent/hardware.py @@ -154,6 +154,16 @@ def list_all_block_devices(block_type='disk'): ('wwn_with_extension', 'WWN_WITH_EXTENSION'), ('wwn_vendor_extension', 'WWN_VENDOR_EXTENSION')]} + # NOTE(lucasagomes): Newer versions of the lsblk tool supports + # HCTL as a parameter but let's get it from sysfs to avoid breaking + # old distros. + try: + extra['hctl'] = os.listdir( + '/sys/block/%s/device/scsi_device' % device['KNAME'])[0] + except (OSError, IndexError): + LOG.warning('Could not find the SCSI address (HCTL) for ' + 'device %s. Skipping', name) + devices.append(BlockDevice(name=name, model=device['MODEL'], size=int(device['SIZE']), @@ -188,11 +198,11 @@ class HardwareType(object): class BlockDevice(encoding.SerializableComparable): serializable_fields = ('name', 'model', 'size', 'rotational', 'wwn', 'serial', 'vendor', 'wwn_with_extension', - 'wwn_vendor_extension') + 'wwn_vendor_extension', 'hctl') def __init__(self, name, model, size, rotational, wwn=None, serial=None, vendor=None, wwn_with_extension=None, - wwn_vendor_extension=None): + wwn_vendor_extension=None, hctl=None): self.name = name self.model = model self.size = size @@ -202,6 +212,7 @@ class BlockDevice(encoding.SerializableComparable): self.vendor = vendor self.wwn_with_extension = wwn_with_extension self.wwn_vendor_extension = wwn_vendor_extension + self.hctl = hctl class NetworkInterface(encoding.SerializableComparable): diff --git a/ironic_python_agent/tests/unit/test_hardware.py b/ironic_python_agent/tests/unit/test_hardware.py index ab1661d4b..9b9fe4573 100644 --- a/ironic_python_agent/tests/unit/test_hardware.py +++ b/ironic_python_agent/tests/unit/test_hardware.py @@ -734,11 +734,13 @@ class TestGenericHardwareManager(test_base.BaseTestCase): list_mock.assert_called_once_with() + @mock.patch.object(os, 'listdir') @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, - mocked_dev_vendor): + mocked_dev_vendor, mock_listdir): + mock_listdir.return_value = ['1:0:0:0'] mocked_execute.return_value = (BLK_DEVICE_TEMPLATE, '') mocked_udev.side_effect = pyudev.DeviceNotFoundError() mocked_dev_vendor.return_value = 'Super Vendor' @@ -748,31 +750,38 @@ class TestGenericHardwareManager(test_base.BaseTestCase): model='TinyUSB Drive', size=3116853504, rotational=False, - vendor='Super Vendor'), + vendor='Super Vendor', + hctl='1:0:0:0'), hardware.BlockDevice(name='/dev/sdb', model='Fastable SD131 7', size=10737418240, rotational=False, - vendor='Super Vendor'), + vendor='Super Vendor', + hctl='1:0:0:0'), hardware.BlockDevice(name='/dev/sdc', model='NWD-BLP4-1600', size=1765517033472, rotational=False, - vendor='Super Vendor'), + vendor='Super Vendor', + hctl='1:0:0:0'), hardware.BlockDevice(name='/dev/sdd', model='NWD-BLP4-1600', size=1765517033472, rotational=False, - vendor='Super Vendor') + vendor='Super Vendor', + hctl='1:0:0:0'), ] self.assertEqual(4, len(devices)) for expected, device in zip(expected_devices, devices): # Compare all attrs of the objects for attr in ['name', 'model', 'size', 'rotational', - 'wwn', 'vendor', 'serial']: + 'wwn', 'vendor', 'serial', 'hctl']: self.assertEqual(getattr(expected, attr), getattr(device, attr)) + expected_calls = [mock.call('/sys/block/%s/device/scsi_device' % dev) + for dev in ('sda', 'sdb', 'sdc', 'sdd')] + mock_listdir.assert_has_calls(expected_calls) @mock.patch.object(hardware, '_get_device_info') @mock.patch.object(pyudev.Device, 'from_device_file') @@ -786,11 +795,29 @@ class TestGenericHardwareManager(test_base.BaseTestCase): devices = hardware.list_all_block_devices() self.assertEqual(4, len(devices)) + @mock.patch.object(os, 'listdir') + @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_hctl_fail(self, mocked_execute, mocked_udev, + mocked_dev_vendor, + mocked_listdir): + mocked_listdir.side_effect = (OSError, IndexError) + mocked_execute.return_value = (BLK_DEVICE_TEMPLATE_SMALL, '') + mocked_dev_vendor.return_value = 'Super Vendor' + devices = hardware.list_all_block_devices() + self.assertEqual(2, len(devices)) + expected_calls = [mock.call('/sys/block/%s/device/scsi_device' % dev) + for dev in ('sda', 'sdb')] + mocked_listdir.assert_has_calls(expected_calls) + + @mock.patch.object(os, 'listdir') @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, - mocked_dev_vendor): + mocked_dev_vendor, mock_listdir): + mock_listdir.return_value = ['1:0:0:0'] mocked_execute.return_value = (BLK_DEVICE_TEMPLATE, '') mocked_udev.side_effect = iter([ {'ID_WWN': 'wwn%d' % i, 'ID_SERIAL_SHORT': 'serial%d' % i, @@ -809,7 +836,8 @@ class TestGenericHardwareManager(test_base.BaseTestCase): wwn='wwn0', wwn_with_extension='wwn-ext0', wwn_vendor_extension='wwn-vendor-ext0', - serial='serial0'), + serial='serial0', + hctl='1:0:0:0'), hardware.BlockDevice(name='/dev/sdb', model='Fastable SD131 7', size=10737418240, @@ -818,7 +846,8 @@ class TestGenericHardwareManager(test_base.BaseTestCase): wwn='wwn1', wwn_with_extension='wwn-ext1', wwn_vendor_extension='wwn-vendor-ext1', - serial='serial1'), + serial='serial1', + hctl='1:0:0:0'), hardware.BlockDevice(name='/dev/sdc', model='NWD-BLP4-1600', size=1765517033472, @@ -827,7 +856,8 @@ class TestGenericHardwareManager(test_base.BaseTestCase): wwn='wwn2', wwn_with_extension='wwn-ext2', wwn_vendor_extension='wwn-vendor-ext2', - serial='serial2'), + serial='serial2', + hctl='1:0:0:0'), hardware.BlockDevice(name='/dev/sdd', model='NWD-BLP4-1600', size=1765517033472, @@ -836,7 +866,8 @@ class TestGenericHardwareManager(test_base.BaseTestCase): wwn='wwn3', wwn_with_extension='wwn-ext3', wwn_vendor_extension='wwn-vendor-ext3', - serial='serial3') + serial='serial3', + hctl='1:0:0:0') ] self.assertEqual(4, len(expected_devices)) @@ -844,9 +875,12 @@ class TestGenericHardwareManager(test_base.BaseTestCase): # Compare all attrs of the objects for attr in ['name', 'model', 'size', 'rotational', 'wwn', 'vendor', 'serial', 'wwn_with_extension', - 'wwn_vendor_extension']: + 'wwn_vendor_extension', 'hctl']: self.assertEqual(getattr(expected, attr), getattr(device, attr)) + expected_calls = [mock.call('/sys/block/%s/device/scsi_device' % dev) + for dev in ('sda', 'sdb', 'sdc', 'sdd')] + mock_listdir.assert_has_calls(expected_calls) @mock.patch.object(hardware, 'dispatch_to_managers') def test_erase_devices(self, mocked_dispatch): @@ -1500,6 +1534,7 @@ class TestGenericHardwareManager(test_base.BaseTestCase): mocked_isdir.assert_called_once_with('/sys/firmware/efi') +@mock.patch.object(os, 'listdir', lambda *_: []) @mock.patch.object(utils, 'execute', autospec=True) class TestModuleFunctions(test_base.BaseTestCase): diff --git a/releasenotes/notes/block-device-hctl-e81573812be3d469.yaml b/releasenotes/notes/block-device-hctl-e81573812be3d469.yaml new file mode 100644 index 000000000..7f33e2b44 --- /dev/null +++ b/releasenotes/notes/block-device-hctl-e81573812be3d469.yaml @@ -0,0 +1,4 @@ +--- +features: + - Add HCTL (Host, Channel, Target and Lun) information to the block + devices.