From 830fdfa4c6ec62fdd1f1ec761c6f92b0f218d252 Mon Sep 17 00:00:00 2001 From: Rozzii Date: Tue, 16 Aug 2022 11:11:16 +0300 Subject: [PATCH] prioritize lsblk as a source of device serials The current way of prioritizing ID/DM_SERIAL_SHORT or ID/DM_SERIAL works in most cases but the udev values seem to be unreliable. Based on experience it looks like lsblk might be a better source of truth than udev in regerards to serial number information. This commit makes lsblk the default provider of block device serial number information. Story: 2010263 Task: 46161 Change-Id: I16039b46676f1a61b32ee7ca7e6d526e65829113 --- ironic_python_agent/hardware.py | 23 ++-- .../tests/unit/extensions/test_image.py | 6 +- .../tests/unit/samples/hardware_samples.py | 100 ++++++++------ .../tests/unit/test_hardware.py | 127 ++++++++++++++---- ironic_python_agent/utils.py | 3 +- ...lsblk-device-serials-8cae406ca5164a01.yaml | 8 ++ 6 files changed, 190 insertions(+), 77 deletions(-) create mode 100644 releasenotes/notes/prioritize-lsblk-device-serials-8cae406ca5164a01.yaml diff --git a/ironic_python_agent/hardware.py b/ironic_python_agent/hardware.py index fe74aba18..0fbdab3be 100644 --- a/ironic_python_agent/hardware.py +++ b/ironic_python_agent/hardware.py @@ -648,6 +648,9 @@ def list_all_block_devices(block_type='disk', name = os.path.join('/dev', device_raw['kname']) extra = {} + lsblk_serial = device_raw.get('serial') + if lsblk_serial: + extra['serial'] = lsblk_serial try: udev = pyudev.Devices.from_device_file(context, name) except pyudev.DeviceNotFoundByFileError as e: @@ -658,17 +661,21 @@ def list_all_block_devices(block_type='disk', "skipping... Error: %(error)s", {'dev': name, 'error': e}) else: - # TODO(lucasagomes): Since lsblk only supports - # returning the short serial we are using - # ID_SERIAL_SHORT first to keep compatibility with the - # bash deploy ramdisk - for key, udev_key in [ - ('serial', 'SERIAL_SHORT'), - ('serial', 'SERIAL'), + # lsblk serial information is prioritized over + # udev serial information + udev_property_mappings = [ ('wwn', 'WWN'), ('wwn_with_extension', 'WWN_WITH_EXTENSION'), ('wwn_vendor_extension', 'WWN_VENDOR_EXTENSION') - ]: + ] + # Only check device serial information from udev + # when lsblk returned None + if not lsblk_serial: + udev_property_mappings += [ + ('serial', 'SERIAL_SHORT'), + ('serial', 'SERIAL') + ] + for key, udev_key in udev_property_mappings: if key in extra: continue value = (udev.get(f'ID_{udev_key}') diff --git a/ironic_python_agent/tests/unit/extensions/test_image.py b/ironic_python_agent/tests/unit/extensions/test_image.py index e488b74f7..d35a97c0f 100644 --- a/ironic_python_agent/tests/unit/extensions/test_image.py +++ b/ironic_python_agent/tests/unit/extensions/test_image.py @@ -815,7 +815,8 @@ Boot0004* ironic1 HD(1,GPT,55db8d03-c8f6-4a5b-9155-790dddc348fa,0x800,0x640 use_standard_locale=True), mock.call('udevadm', 'settle'), mock.call('lsblk', '-bia', '--json', - '-oKNAME,MODEL,SIZE,ROTA,TYPE,UUID,PARTUUID', + '-oKNAME,MODEL,SIZE,ROTA,' + + 'TYPE,UUID,PARTUUID,SERIAL', check_exit_code=[0]), mock.call('umount', self.fake_dir + '/boot/efi', attempts=3, delay_on_retry=True), @@ -934,7 +935,8 @@ Boot0004* ironic1 HD(1,GPT,55db8d03-c8f6-4a5b-9155-790dddc348fa,0x800,0x640 use_standard_locale=True), mock.call('udevadm', 'settle'), mock.call('lsblk', '-bia', '--json', - '-oKNAME,MODEL,SIZE,ROTA,TYPE,UUID,PARTUUID', + '-oKNAME,MODEL,SIZE,ROTA,' + + 'TYPE,UUID,PARTUUID,SERIAL', check_exit_code=[0]), mock.call('umount', self.fake_dir + '/boot/efi', attempts=3, delay_on_retry=True), diff --git a/ironic_python_agent/tests/unit/samples/hardware_samples.py b/ironic_python_agent/tests/unit/samples/hardware_samples.py index c9e597a94..c00d637b0 100644 --- a/ironic_python_agent/tests/unit/samples/hardware_samples.py +++ b/ironic_python_agent/tests/unit/samples/hardware_samples.py @@ -101,33 +101,36 @@ BLK_DEVICE_TEMPLATE = """ { "blockdevices": [ {"kname":"sda", "model":"TinyUSB Drive", "size":3116853504, - "rota":false, "type":"disk", "serial":123, "uuid":"F531-BDC3", + "rota":false, "type":"disk", "serial":"sda123", "uuid":"F531-BDC3", "partuuid":null}, {"kname":"sdb", "model":"Fastable SD131 7", "size":10737418240, - "rota":false, "type":"disk", + "rota":false, "type":"disk", "serial":"sdb123", "uuid":"9a5e5cca-e03d-4cbd-9054-9e6ca9048222", "partuuid":null}, {"kname":"sdc", "model":"NWD-BLP4-1600", "size":1765517033472, - "rota":false, "type":"disk", "uuid":null, "partuuid":null}, + "rota":false, "type":"disk", "serial":"sdc123", "uuid":null, + "partuuid":null}, {"kname":"sdd", "model":"NWD-BLP4-1600", "size":1765517033472, - "rota":false, "type":"disk", "uuid":null, "partuuid":null}, + "rota":false, "type":"disk", "serial":"sdd123", "uuid":null, + "partuuid":null}, {"kname":"loop0", "model":null, "size":109109248, "rota":true, - "type":"loop", "uuid":null, "partuuid": null}, + "type":"loop", "serial":null, "uuid":null, "partuuid": null}, {"kname":"zram0", "model":null, "size":0, "rota":false, "type":"disk", - "uuid":null, "partuuid":null}, + "serial":null, "uuid":null, "partuuid":null}, {"kname":"ram0", "model":null, "size":8388608, "rota":false, - "type":"disk", "uuid":null, "partuuid":null}, + "type":"disk", "serial":null, "uuid":null, "partuuid":null}, {"kname":"ram1", "model":null, "size":8388608, "rota":false, - "type":"disk", "uuid":null, "partuuid":null}, + "type":"disk", "serial":null, "uuid":null, "partuuid":null}, {"kname":"ram2", "model":null, "size":8388608, "rota":false, - "type":"disk", "uuid":null, "partuuid":null}, + "type":"disk", "serial":null, "uuid":null, "partuuid":null}, {"kname":"ram3", "model":null, "size":8388608, "rota":false, - "type":"disk", "uuid":null, "partuuid":null}, + "type":"disk", "serial":null, "uuid":null, "partuuid":null}, {"kname":"fd1", "model":"magic", "size":4096, "rota":true, - "type":"disk", "uuid":null, "partuuid":null}, + "type":"disk", "serial":null, "uuid":null, "partuuid":null}, {"kname":"sdf", "model":"virtual floppy", "size":0, "rota":true, - "type":"disk", "uuid":null, "partuuid":null}, + "type":"disk", "serial":null, "uuid":null, "partuuid":null}, {"kname":"dm-0", "model":"NWD-BLP4-1600", "size":"1765517033472", - "rota":false, "type":"mpath", "uuid":null, "partuuid":null} + "rota":false, "type":"mpath", "serial":null, "uuid":null, + "partuuid":null} ] } """ @@ -137,9 +140,22 @@ BLK_DEVICE_TEMPLATE_SMALL = """ { "blockdevices": [ {"kname":"sda", "model":"TinyUSB Drive", "size":3116853504, "rota":false, - "type":"disk", "uuid":"F531-BDC", "partuuid":null}, + "type":"disk", "serial":"123", "uuid":"F531-BDC", "partuuid":null}, {"kname":"sdb", "model":"AlmostBigEnough Drive", "size":"4294967295", - "rota":false, "type":"disk", "uuid":null, "partuuid":null} + "rota":false, "type":"disk", "serial":"456", "uuid":null, "partuuid":null} + ] +} +""" + + +# NOTE This is intentionally have serials removed +BLK_INCOMPLETE_DEVICE_TEMPLATE_SMALL = """ +{ + "blockdevices": [ + {"kname":"sda", "model":"TinyUSB Drive", "size":3116853504, "rota":false, + "type":"disk", "serial":"", "uuid":"F531-BDC", "partuuid":null}, + {"kname":"sdb", "model":"AlmostBigEnough Drive", "size":"4294967295", + "rota":false, "type":"disk", "serial":"", "uuid":null, "partuuid":null} ] } """ @@ -155,23 +171,23 @@ RAID_BLK_DEVICE_TEMPLATE = (""" { "blockdevices": [ {"kname":"sda", "model":"DRIVE 0", "size":1765517033472, "rota":true, - "type":"disk", "uuid":null, "partuuid":null}, + "type":"disk", "serial":"sda123", "uuid":null, "partuuid":null}, {"kname":"sda1", "model":"DRIVE 0", "size":107373133824, "rota":true, - "type":"part", "uuid":null, "partuuid":null}, + "type":"part", "serial":"sda1123", "uuid":null, "partuuid":null}, {"kname":"sdb", "model":"DRIVE 1", "size":1765517033472, "rota":true, - "type":"disk", "uuid":null, "partuuid":null}, + "type":"disk", "serial":"sdb123", "uuid":null, "partuuid":null}, {"kname":"sdb", "model":"DRIVE 1", "size":1765517033472, "rota":true, "type":"disk", "uuid":null, "partuuid":null}, {"kname":"sdb1", "model":"DRIVE 1", "size":107373133824, "rota":true, - "type":"part", "uuid":null, "partuuid":null}, + "type":"part", "serial":"sdb1123", "uuid":null, "partuuid":null}, {"kname":"md0p1", "model":"RAID", "size":107236818944, "rota":false, - "type":"md", "uuid":null, "partuuid":null}, + "type":"md", "serial":null, "uuid":null, "partuuid":null}, {"kname":"md0", "model":"RAID", "size":1765517033470, "rota":false, - "type":"raid1", "uuid":null, "partuuid":null}, + "type":"raid1", "serial":null, "uuid":null, "partuuid":null}, {"kname":"md0", "model":"RAID", "size":1765517033470, "rota":false, - "type":"raid1", "uuid":null, "partuuid":null}, + "type":"raid1", "serial":null, "uuid":null, "partuuid":null}, {"kname":"md1", "model":"RAID", "size":0, "rota":false, "type":"raid1", - "uuid":null, "partuuid":null} + "serial":null, "uuid":null, "partuuid":null} ] } """) @@ -180,49 +196,52 @@ MULTIPATH_BLK_DEVICE_TEMPLATE = (""" { "blockdevices": [ {"kname":"sda", "model":"INTEL_SSDSC2CT060A3", "size":"60022480896", - "rota":false, "type":"disk", "uuid":null, "partuuid":null}, + "rota":false, "type":"disk", "serial":"sda123", "uuid":null, + "partuuid":null}, {"kname":"sda2", "model":null, "size":"59162722304", "rota":false, "type":"part", "uuid":"f8b55d59-96c3-3982-b129-1b6b2ee8da86", - "partuuid":"c97c8aac-7796-4433-b1fc-9b5fac43edf3"}, + "partuuid":"c97c8aac-7796-4433-b1fc-9b5fac43edf3", "serial":"sda2123"}, {"kname":"sda3", "model":null, "size":"650002432", "rota":false, "type":"part", "uuid":"b3b03565-5f13-3c93-b2a6-6d90e25be926", - "partuuid":"6c85beff-b2bd-4a1c-91b7-8abb5256459d"}, + "partuuid":"6c85beff-b2bd-4a1c-91b7-8abb5256459d", "serial":"sda3123"}, {"kname":"sda1", "model":null, "size":"209715200", "rota":false, "type":"part", "uuid":"0a83355d-7500-3f5f-9abd-66f6fd03714c", - "partuuid":"eba28b26-b76a-402c-94dd-0b66a523a485"}, + "partuuid":"eba28b26-b76a-402c-94dd-0b66a523a485", "serial":"sda1123"}, {"kname":"dm-0", "model":null, "size":"60022480896", "rota":false, - "type":"mpath", "uuid":null, "partuuid":null}, + "type":"mpath", "serial":null, "uuid":null, "partuuid":null}, {"kname":"dm-4", "model":null, "size":"650002432", "rota":false, "type":"part", "uuid":"b3b03565-5f13-3c93-b2a6-6d90e25be926", - "partuuid":"6c85beff-b2bd-4a1c-91b7-8abb5256459d"}, + "partuuid":"6c85beff-b2bd-4a1c-91b7-8abb5256459d", "serial":null}, {"kname":"dm-2", "model":null, "size":"209715200", "rota":false, "type":"part", "uuid":"0a83355d-7500-3f5f-9abd-66f6fd03714c", - "partuuid":"eba28b26-b76a-402c-94dd-0b66a523a485"}, + "partuuid":"eba28b26-b76a-402c-94dd-0b66a523a485", "serial":null}, {"kname":"dm-3", "model":null, "size":"59162722304", "rota":false, "type":"part", "uuid":"f8b55d59-96c3-3982-b129-1b6b2ee8da86", - "partuuid":"c97c8aac-7796-4433-b1fc-9b5fac43edf3"}, + "partuuid":"c97c8aac-7796-4433-b1fc-9b5fac43edf3", "serial":null}, {"kname":"sdb", "model":"INTEL_SSDSC2CT060A3", "size":"60022480896", - "rota":false, "type":"disk", "uuid":null, "partuuid":null}, + "rota":false, "type":"disk", "serial":"sdb123", "uuid":null, + "partuuid":null}, {"kname":"sdb2", "model":null, "size":"59162722304", - "rota":false, "type":"part", + "rota":false, "type":"part", "serial":"sdb2123", "uuid":"f8b55d59-96c3-3982-b129-1b6b2ee8da86", "partuuid":"c97c8aac-7796-4433-b1fc-9b5fac43edf3"}, {"kname":"sdb3", "model":null, "size":"650002432", - "rota":false, "type":"part", + "rota":false, "type":"part", "serial":"sdv3123", "uuid":"b3b03565-5f13-3c93-b2a6-6d90e25be926", "partuuid":"6c85beff-b2bd-4a1c-91b7-8abb5256459d"}, {"kname":"sdb1", "model":null, "size":"209715200", - "rota":false, "type":"part", + "rota":false, "type":"part", "serial":"sdb1123", "uuid":"0a83355d-7500-3f5f-9abd-66f6fd03714c", "partuuid":"eba28b26-b76a-402c-94dd-0b66a523a485"}, {"kname":"sdc", "model":"ST1000DM003-1CH162", "size":"1000204886016", - "rota":true, "type":"disk", "uuid":null, "partuuid":null}, + "rota":true, "type":"disk", "serial":"sdc123", "uuid":null, + "partuuid":null}, {"kname":"sdc1", "model":null, "size":"899999072256", - "rota":true, "type":"part", + "rota":true, "type":"part", "serial":"sdc1123", "uuid":"457f7d3c-9376-4997-89bd-d1a7c8b04060", "partuuid":"c9433d2e-3bbc-47b4-92bf-43c1d80f06e0"}, {"kname":"dm-1", "model":null, "size":"1000204886016", "rota":false, - "type":"mpath", "uuid":null, "partuuid":null} + "type":"mpath", "serial":null, "uuid":null, "partuuid":null} ] } """) @@ -231,9 +250,10 @@ PARTUUID_DEVICE_TEMPLATE = (""" { "blockdevices": [ {"kname":"sda", "model":"DRIVE 0", "size":1765517033472, "rota":true, - "type":"disk", "uuid":null, "partuuid":null}, + "type":"disk", "serial":"sda123", "uuid":null, "partuuid":null}, {"kname":"sda1", "model":"DRIVE 0", "size":107373133824, "rota":true, - "type":"part", "uuid":"987654-3210", "partuuid":"1234-5678"} + "type":"part", "serial":"sda1123", "uuid":"987654-3210", + "partuuid":"1234-5678"} ] } """) diff --git a/ironic_python_agent/tests/unit/test_hardware.py b/ironic_python_agent/tests/unit/test_hardware.py index cc39d8339..9fcf775c4 100644 --- a/ironic_python_agent/tests/unit/test_hardware.py +++ b/ironic_python_agent/tests/unit/test_hardware.py @@ -45,32 +45,38 @@ CONF.import_opt('disk_wait_delay', 'ironic_python_agent.config') BLK_DEVICE_TEMPLATE_SMALL_DEVICES = [ hardware.BlockDevice(name='/dev/sda', model='TinyUSB Drive', size=3116853504, rotational=False, - vendor="FooTastic", uuid="F531-BDC3"), + vendor="FooTastic", uuid="F531-BDC3", + serial="123"), hardware.BlockDevice(name='/dev/sdb', model='AlmostBigEnough Drive', size=4294967295, rotational=False, - vendor="FooTastic", uuid=""), + vendor="FooTastic", uuid="", + serial="456"), ] RAID_BLK_DEVICE_TEMPLATE_DEVICES = [ hardware.BlockDevice(name='/dev/sda', model='DRIVE 0', size=1765517033472, rotational=True, - vendor="FooTastic", uuid=""), + vendor="FooTastic", uuid="", + serial="sda123"), hardware.BlockDevice(name='/dev/sdb', model='DRIVE 1', size=1765517033472, rotational=True, - vendor="FooTastic", uuid=""), + vendor="FooTastic", uuid="", + serial="sdb123"), hardware.BlockDevice(name='/dev/md0', model='RAID', size=1765517033470, rotational=False, - vendor="FooTastic", uuid=""), + vendor="FooTastic", uuid="", + serial=None), hardware.BlockDevice(name='/dev/md1', model='RAID', size=0, rotational=False, - vendor="FooTastic", uuid=""), + vendor="FooTastic", uuid="", + serial=None), ] BLK_DEVICE_TEMPLATE_PARTUUID_DEVICE = [ hardware.BlockDevice(name='/dev/sda1', model='DRIVE 0', size=107373133824, rotational=True, vendor="FooTastic", uuid="987654-3210", - partuuid="1234-5678"), + partuuid="1234-5678", serial="sda1123"), ] @@ -869,7 +875,7 @@ class TestGenericHardwareManager(base.IronicAgentTest): ] expected = [ mock.call('lsblk', '-bia', '--json', - '-oKNAME,MODEL,SIZE,ROTA,TYPE,UUID,PARTUUID', + '-oKNAME,MODEL,SIZE,ROTA,TYPE,UUID,PARTUUID,SERIAL', check_exit_code=[0]), mock.call('multipath', '-c', '/dev/sda'), mock.call('multipath', '-ll', '/dev/sda'), @@ -949,7 +955,7 @@ class TestGenericHardwareManager(base.IronicAgentTest): ] expected = [ mock.call('lsblk', '-bia', '--json', - '-oKNAME,MODEL,SIZE,ROTA,TYPE,UUID,PARTUUID', + '-oKNAME,MODEL,SIZE,ROTA,TYPE,UUID,PARTUUID,SERIAL', check_exit_code=[0]), mock.call('multipath', '-c', '/dev/sda'), mock.call('multipath', '-ll', '/dev/sda'), @@ -1004,7 +1010,7 @@ class TestGenericHardwareManager(base.IronicAgentTest): self.assertEqual('/dev/md0', self.hardware.get_os_install_device()) expected = [ mock.call('lsblk', '-bia', '--json', - '-oKNAME,MODEL,SIZE,ROTA,TYPE,UUID,PARTUUID', + '-oKNAME,MODEL,SIZE,ROTA,TYPE,UUID,PARTUUID,SERIAL', check_exit_code=[0]), ] @@ -1030,14 +1036,14 @@ class TestGenericHardwareManager(base.IronicAgentTest): self.hardware.get_os_install_device) expected = [ mock.call('lsblk', '-bia', '--json', - '-oKNAME,MODEL,SIZE,ROTA,TYPE,UUID,PARTUUID', + '-oKNAME,MODEL,SIZE,ROTA,TYPE,UUID,PARTUUID,SERIAL', check_exit_code=[0]), ] mocked_execute.assert_has_calls(expected) mocked_execute.assert_called_once_with( 'lsblk', '-bia', '--json', - '-oKNAME,MODEL,SIZE,ROTA,TYPE,UUID,PARTUUID', + '-oKNAME,MODEL,SIZE,ROTA,TYPE,UUID,PARTUUID,SERIAL', check_exit_code=[0]) self.assertIn(str(4 * units.Gi), ex.details) mock_cached_node.assert_called_once_with() @@ -1748,21 +1754,24 @@ class TestGenericHardwareManager(base.IronicAgentTest): rotational=False, vendor='Super Vendor', hctl='1:0:0:0', - by_path='/dev/disk/by-path/1:0:0:0'), + by_path='/dev/disk/by-path/1:0:0:0', + serial='sda123'), hardware.BlockDevice(name='/dev/sdb', model='Fastable SD131 7', size=10737418240, rotational=False, vendor='Super Vendor', hctl='1:0:0:0', - by_path='/dev/disk/by-path/1:0:0:1'), + by_path='/dev/disk/by-path/1:0:0:1', + serial='sdb123'), hardware.BlockDevice(name='/dev/sdc', model='NWD-BLP4-1600', size=1765517033472, rotational=False, vendor='Super Vendor', hctl='1:0:0:0', - by_path='/dev/disk/by-path/1:0:0:2'), + by_path='/dev/disk/by-path/1:0:0:2', + serial='sdc123'), hardware.BlockDevice(name='/dev/dm-0', model='NWD-BLP4-1600', size=1765517033472, @@ -1787,7 +1796,7 @@ class TestGenericHardwareManager(base.IronicAgentTest): mock_readlink.assert_has_calls(expected_calls) expected_calls = [ mock.call('lsblk', '-bia', '--json', - '-oKNAME,MODEL,SIZE,ROTA,TYPE,UUID,PARTUUID', + '-oKNAME,MODEL,SIZE,ROTA,TYPE,UUID,PARTUUID,SERIAL', check_exit_code=[0]), mock.call('multipath', '-c', '/dev/sda'), mock.call('multipath', '-c', '/dev/sdb'), @@ -1893,7 +1902,7 @@ class TestGenericHardwareManager(base.IronicAgentTest): 'ID_WWN_VENDOR_EXTENSION': 'wwn-vendor-ext%d' % i} for i in range(3) ] + [ - {'DM_WWN': 'wwn3', 'DM_SERIAL': 'serial3'}, + {'DM_WWN': 'wwn3', 'DM_SERIAL': 'serial3'} ] mocked_dev_vendor.return_value = 'Super Vendor' devices = hardware.list_all_block_devices() @@ -1906,7 +1915,7 @@ class TestGenericHardwareManager(base.IronicAgentTest): wwn='wwn0', wwn_with_extension='wwn-ext0', wwn_vendor_extension='wwn-vendor-ext0', - serial='serial0', + serial='sda123', hctl='1:0:0:0'), hardware.BlockDevice(name='/dev/sdb', model='Fastable SD131 7', @@ -1916,7 +1925,7 @@ class TestGenericHardwareManager(base.IronicAgentTest): wwn='wwn1', wwn_with_extension='wwn-ext1', wwn_vendor_extension='wwn-vendor-ext1', - serial='serial1', + serial='sdb123', hctl='1:0:0:0'), hardware.BlockDevice(name='/dev/sdc', model='NWD-BLP4-1600', @@ -1926,7 +1935,7 @@ class TestGenericHardwareManager(base.IronicAgentTest): wwn='wwn2', wwn_with_extension='wwn-ext2', wwn_vendor_extension='wwn-vendor-ext2', - serial='serial2', + serial='sdc123', hctl='1:0:0:0'), hardware.BlockDevice(name='/dev/dm-0', model='NWD-BLP4-1600', @@ -1953,6 +1962,72 @@ class TestGenericHardwareManager(base.IronicAgentTest): mocked_listdir.assert_has_calls(expected_calls) mocked_mpath.assert_called_once_with() + @mock.patch.object(hardware, 'get_multipath_status', autospec=True) + @mock.patch.object(os, 'readlink', autospec=True) + @mock.patch.object(os, 'listdir', autospec=True) + @mock.patch.object(hardware, '_get_device_info', autospec=True) + @mock.patch.object(pyudev.Devices, 'from_device_file', autospec=False) + @mock.patch.object(il_utils, 'execute', autospec=True) + def test_list_all_block_device_with_only_udev(self, + mocked_execute, + mocked_udev, + mocked_dev_vendor, + mocked_listdir, + mocked_readlink, + mocked_mpath): + mocked_readlink.return_value = '../../sda' + mocked_listdir.return_value = ['1:0:0:0'] + mocked_execute.side_effect = [ + (hws.BLK_INCOMPLETE_DEVICE_TEMPLATE_SMALL, ''), + processutils.ProcessExecutionError( + stderr=hws.MULTIPATH_INVALID_PATH % '/dev/sda'), + processutils.ProcessExecutionError( + stderr=hws.MULTIPATH_INVALID_PATH % '/dev/sdb'), + ] + + mocked_mpath.return_value = True + mocked_udev.side_effect = [ + {'ID_WWN': 'wwn%d' % i, 'ID_SERIAL_SHORT': 'serial%d' % i, + 'ID_SERIAL': 'do not use me', + 'ID_WWN_WITH_EXTENSION': 'wwn-ext%d' % i, + 'ID_WWN_VENDOR_EXTENSION': 'wwn-vendor-ext%d' % i} + for i in range(2) + ] + devices = hardware.list_all_block_devices() + expected_devices = [ + hardware.BlockDevice(name='/dev/sda', + model='TinyUSB Drive', + size=3116853504, + rotational=False, + wwn='wwn0', + wwn_with_extension='wwn-ext0', + wwn_vendor_extension='wwn-vendor-ext0', + serial='serial0', + hctl='1:0:0:0'), + hardware.BlockDevice(name='/dev/sdb', + model='AlmostBigEnough Drive', + size=4294967295, + rotational=False, + wwn='wwn1', + wwn_with_extension='wwn-ext1', + wwn_vendor_extension='wwn-vendor-ext1', + serial='serial1', + hctl='1:0:0:0') + ] + + self.assertEqual(2, len(devices)) + for expected, device in zip(expected_devices, devices): + # Compare all attrs of the objects + for attr in ['name', 'model', 'size', 'rotational', + 'wwn', 'serial', 'wwn_with_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')] + mocked_listdir.assert_has_calls(expected_calls) + mocked_mpath.assert_called_once_with() + @mock.patch.object(hardware, 'safety_check_block_device', autospec=True) @mock.patch.object(hardware, 'ThreadPool', autospec=True) @mock.patch.object(hardware, 'dispatch_to_managers', autospec=True) @@ -5457,7 +5532,7 @@ class TestModuleFunctions(base.IronicAgentTest): result = hardware.list_all_block_devices() expected_calls = [ mock.call('lsblk', '-bia', '--json', - '-oKNAME,MODEL,SIZE,ROTA,TYPE,UUID,PARTUUID', + '-oKNAME,MODEL,SIZE,ROTA,TYPE,UUID,PARTUUID,SERIAL', check_exit_code=[0]), mock.call('multipath', '-c', '/dev/sda'), mock.call('multipath', '-c', '/dev/sdb') @@ -5504,7 +5579,7 @@ class TestModuleFunctions(base.IronicAgentTest): ] expected_calls = [ mock.call('lsblk', '-bia', '--json', - '-oKNAME,MODEL,SIZE,ROTA,TYPE,UUID,PARTUUID', + '-oKNAME,MODEL,SIZE,ROTA,TYPE,UUID,PARTUUID,SERIAL', check_exit_code=[0]), mock.call('multipath', '-c', '/dev/sda'), mock.call('multipath', '-c', '/dev/sda1'), @@ -5543,7 +5618,7 @@ class TestModuleFunctions(base.IronicAgentTest): result = hardware.list_all_block_devices(block_type='part') expected_calls = [ mock.call('lsblk', '-bia', '--json', - '-oKNAME,MODEL,SIZE,ROTA,TYPE,UUID,PARTUUID', + '-oKNAME,MODEL,SIZE,ROTA,TYPE,UUID,PARTUUID,SERIAL', check_exit_code=[0]), mock.call('multipath', '-c', '/dev/sda'), mock.call('multipath', '-c', '/dev/sda1'), @@ -5566,7 +5641,7 @@ class TestModuleFunctions(base.IronicAgentTest): result = hardware.list_all_block_devices() mocked_execute.assert_called_once_with( 'lsblk', '-bia', '--json', - '-oKNAME,MODEL,SIZE,ROTA,TYPE,UUID,PARTUUID', + '-oKNAME,MODEL,SIZE,ROTA,TYPE,UUID,PARTUUID,SERIAL', check_exit_code=[0]) self.assertEqual([], result) mocked_udev.assert_called_once_with() @@ -5580,7 +5655,7 @@ class TestModuleFunctions(base.IronicAgentTest): mocked_mpath.return_value = False expected_calls = [ mock.call('lsblk', '-bia', '--json', - '-oKNAME,MODEL,SIZE,ROTA,TYPE,UUID,PARTUUID', + '-oKNAME,MODEL,SIZE,ROTA,TYPE,UUID,PARTUUID,SERIAL', check_exit_code=[0]), ] mocked_execute.return_value = ( @@ -5588,7 +5663,7 @@ class TestModuleFunctions(base.IronicAgentTest): self.assertRaisesRegex( errors.BlockDeviceError, r'^Block device caused unknown error: kname, partuuid, rota, ' - r'size, uuid must be returned by lsblk.$', + r'serial, size, uuid must be returned by lsblk.$', hardware.list_all_block_devices) mocked_udev.assert_called_once_with() mocked_execute.assert_has_calls(expected_calls) diff --git a/ironic_python_agent/utils.py b/ironic_python_agent/utils.py index 0da737a81..41daaf975 100644 --- a/ironic_python_agent/utils.py +++ b/ironic_python_agent/utils.py @@ -59,7 +59,8 @@ CONF = cfg.CONF AGENT_PARAMS_CACHED = dict() -LSBLK_COLUMNS = ['KNAME', 'MODEL', 'SIZE', 'ROTA', 'TYPE', 'UUID', 'PARTUUID'] +LSBLK_COLUMNS = ['KNAME', 'MODEL', 'SIZE', 'ROTA', + 'TYPE', 'UUID', 'PARTUUID', 'SERIAL'] COLLECT_LOGS_COMMANDS = { diff --git a/releasenotes/notes/prioritize-lsblk-device-serials-8cae406ca5164a01.yaml b/releasenotes/notes/prioritize-lsblk-device-serials-8cae406ca5164a01.yaml new file mode 100644 index 000000000..3c0b26b14 --- /dev/null +++ b/releasenotes/notes/prioritize-lsblk-device-serials-8cae406ca5164a01.yaml @@ -0,0 +1,8 @@ +--- +features: + - | + When detecting a serial number of a block device, the agent now tries + to use lsblk first and only falls back to udev if lsblk does not return + a serial number. Based on experience it looks like lsblk might be a better + source of truth than udev in regerard to serial number information. +