diff options
| -rw-r--r-- | ironic_python_agent/hardware.py | 101 | ||||
| -rw-r--r-- | ironic_python_agent/inspector.py | 8 | ||||
| -rw-r--r-- | ironic_python_agent/ironic_api_client.py | 13 | ||||
| -rw-r--r-- | ironic_python_agent/tests/unit/extensions/test_image.py | 39 | ||||
| -rw-r--r-- | ironic_python_agent/tests/unit/samples/hardware_samples.py | 238 | ||||
| -rw-r--r-- | ironic_python_agent/tests/unit/test_agent.py | 6 | ||||
| -rw-r--r-- | ironic_python_agent/tests/unit/test_hardware.py | 95 | ||||
| -rw-r--r-- | ironic_python_agent/tests/unit/test_ironic_api_client.py | 11 | ||||
| -rw-r--r-- | ironic_python_agent/tests/unit/test_utils.py | 49 | ||||
| -rw-r--r-- | ironic_python_agent/utils.py | 60 | ||||
| -rw-r--r-- | ironic_python_agent/version.py | 7 | ||||
| -rw-r--r-- | releasenotes/notes/collect-udev-f6ada5163cf4a26c.yaml | 5 | ||||
| -rw-r--r-- | releasenotes/notes/multipath-serial-615fc925984abbf7.yaml | 4 | ||||
| -rw-r--r-- | requirements.txt | 2 | ||||
| -rw-r--r-- | setup.cfg | 4 | ||||
| -rw-r--r-- | zuul.d/project.yaml | 1 |
16 files changed, 384 insertions, 259 deletions
diff --git a/ironic_python_agent/hardware.py b/ironic_python_agent/hardware.py index a7a6fb20..0f7e4f82 100644 --- a/ironic_python_agent/hardware.py +++ b/ironic_python_agent/hardware.py @@ -540,32 +540,36 @@ def list_all_block_devices(block_type='disk', "Cause: %(error)s", {'path': disk_by_path_dir, 'error': e}) columns = utils.LSBLK_COLUMNS - report = il_utils.execute('lsblk', '-Pbia', - '-o{}'.format(','.join(columns)))[0] - lines = report.splitlines() + report = il_utils.execute('lsblk', '-bia', '--json', + '-o{}'.format(','.join(columns)), + check_exit_code=[0])[0] + + try: + report_json = json.loads(report) + except json.decoder.JSONDecodeError as ex: + LOG.error("Unable to decode lsblk output, invalid JSON: %s", ex) + context = pyudev.Context() + devices_raw = report_json['blockdevices'] + # Convert raw json output to something useful for us devices = [] - for line in lines: - device = {} - # Split into KEY=VAL pairs - vals = shlex.split(line) - for key, val in (v.split('=', 1) for v in vals): - device[key] = val.strip() + for device_raw in devices_raw: # Ignore block types not specified - devtype = device.get('TYPE') + devtype = device_raw.get('type') # We already have devices, we should ensure we don't store duplicates. - if _is_known_device(devices, device.get('KNAME')): + if _is_known_device(devices, device_raw.get('kname')): + LOG.debug('Ignoring already known device %s', device_raw) continue # If we collected the RM column, we could consult it for removable # media, however USB devices are also flagged as removable media. # we have to explicitly do this as floppy disks are type disk. - if ignore_floppy and str(device.get('KNAME')).startswith('fd'): - LOG.debug('Ignoring floppy disk device: %s', line) + if ignore_floppy and str(device_raw.get('kname')).startswith('fd'): + LOG.debug('Ignoring floppy disk device %s', device_raw) continue - dev_kname = device.get('KNAME') + dev_kname = device_raw.get('kname') if check_multipath: # Net effect is we ignore base devices, and their base devices # to what would be the mapped device name which would not pass the @@ -588,14 +592,15 @@ def list_all_block_devices(block_type='disk', if devtype is None or ignore_raid: LOG.debug( "TYPE did not match. Wanted: %(block_type)s but found: " - "%(line)s (RAID devices are ignored)", - {'block_type': block_type, 'line': line}) + "%(devtype)s (RAID devices are ignored)", + {'block_type': block_type, 'devtype': devtype}) continue elif ('raid' in devtype and block_type in ['raid', 'disk', 'mpath']): LOG.debug( "TYPE detected to contain 'raid', signifying a " - "RAID volume. Found: %s", line) + "RAID volume. Found: %(device_raw)s", + {'device_raw': device_raw}) elif (devtype == 'md' and (block_type == 'part' or block_type == 'md')): @@ -605,66 +610,77 @@ def list_all_block_devices(block_type='disk', # more detail. LOG.debug( "TYPE detected to contain 'md', signifying a " - "RAID partition. Found: %s", line) + "RAID partition. Found: %(device_raw)s", + {'device_raw': device_raw}) elif devtype == 'mpath' and block_type == 'disk': LOG.debug( "TYPE detected to contain 'mpath', " "signifing a device mapper multipath device. " - "Found: %s", line) + "Found: %(device_raw)s", + {'device_raw': device_raw}) else: LOG.debug( "TYPE did not match. Wanted: %(block_type)s but found: " - "%(line)s", {'block_type': block_type, 'line': line}) + "%(device_raw)s (RAID devices are ignored)", + {'block_type': block_type, 'device_raw': device_raw}) continue # Ensure all required columns are at least present, even if blank - missing = set(columns) - set(device) + missing = set(map(str.lower, columns)) - set(device_raw) if missing: raise errors.BlockDeviceError( '%s must be returned by lsblk.' % ', '.join(sorted(missing))) # NOTE(dtantsur): RAM disks and zRAM devices appear in the output of # lsblk as disks, but we cannot do anything useful with them. - if (device['KNAME'].startswith('ram') - or device['KNAME'].startswith('zram')): - LOG.debug('Skipping RAM device %s', device) + if (device_raw['kname'].startswith('ram') + or device_raw['kname'].startswith('zram')): + LOG.debug('Skipping RAM device %s', device_raw) continue # NOTE(dtantsur): some hardware represents virtual floppy devices as # normal block devices with size 0. Filter them out. - if ignore_empty and not int(device['SIZE'] or 0): - LOG.debug('Skipping device %s with zero size', device) + if ignore_empty and not int(device_raw['size'] or 0): + LOG.debug('Skipping device %s with zero size', device_raw) continue - name = os.path.join('/dev', device['KNAME']) + name = os.path.join('/dev', device_raw['kname']) + extra = {} try: udev = pyudev.Devices.from_device_file(context, name) except pyudev.DeviceNotFoundByFileError as e: LOG.warning("Device %(dev)s is inaccessible, skipping... " "Error: %(error)s", {'dev': name, 'error': e}) - extra = {} except pyudev.DeviceNotFoundByNumberError as e: LOG.warning("Device %(dev)s is not supported by pyudev, " "skipping... Error: %(error)s", {'dev': name, 'error': e}) - extra = {} else: # TODO(lucasagomes): Since lsblk only supports # returning the short serial we are using - # ID_SERIAL_SHORT here to keep compatibility with the + # ID_SERIAL_SHORT first to keep compatibility with the # bash deploy ramdisk - extra = {key: udev.get('ID_%s' % udev_key) for key, udev_key in - [('wwn', 'WWN'), ('serial', 'SERIAL_SHORT'), - ('wwn_with_extension', 'WWN_WITH_EXTENSION'), - ('wwn_vendor_extension', 'WWN_VENDOR_EXTENSION')]} + for key, udev_key in [ + ('serial', 'SERIAL_SHORT'), + ('serial', 'SERIAL'), + ('wwn', 'WWN'), + ('wwn_with_extension', 'WWN_WITH_EXTENSION'), + ('wwn_vendor_extension', 'WWN_VENDOR_EXTENSION') + ]: + if key in extra: + continue + value = (udev.get(f'ID_{udev_key}') + or udev.get(f'DM_{udev_key}')) # devicemapper + if value: + extra[key] = value # 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] + '/sys/block/%s/device/scsi_device' % device_raw['kname'])[0] except (OSError, IndexError): LOG.warning('Could not find the SCSI address (HCTL) for ' 'device %s. Skipping', name) @@ -673,14 +689,14 @@ def list_all_block_devices(block_type='disk', by_path_name = by_path_mapping.get(name) devices.append(BlockDevice(name=name, - model=device['MODEL'], - size=int(device['SIZE'] or 0), - rotational=bool(int(device['ROTA'])), - vendor=_get_device_info(device['KNAME'], + model=device_raw['model'], + size=int(device_raw['size'] or 0), + rotational=bool(int(device_raw['rota'])), + vendor=_get_device_info(device_raw['kname'], 'block', 'vendor'), by_path=by_path_name, - uuid=device['UUID'], - partuuid=device['PARTUUID'], + uuid=device_raw['uuid'], + partuuid=device_raw['partuuid'], **extra)) return devices @@ -1798,10 +1814,11 @@ class GenericHardwareManager(HardwareManager): 'ATA commands via the `smartctl` utility with device ' '%s do not succeed.', block_device.name) return False - except OSError: + except OSError as e: # Processutils can raise OSError if a path is not found, # and it is okay that we tollerate that since it was the # prior behavior. + LOG.warning('Unable to execute `smartctl` utility: %s', e) return True def _ata_erase(self, block_device): diff --git a/ironic_python_agent/inspector.py b/ironic_python_agent/inspector.py index 31ca0a92..8e570c3b 100644 --- a/ironic_python_agent/inspector.py +++ b/ironic_python_agent/inspector.py @@ -13,6 +13,7 @@ # See the License for the specific language governing permissions and # limitations under the License. +import json import os import time @@ -20,7 +21,6 @@ from ironic_lib import mdns from oslo_concurrency import processutils from oslo_config import cfg from oslo_log import log as logging -from oslo_serialization import jsonutils from oslo_utils import excutils import requests import stevedore @@ -290,10 +290,10 @@ def collect_extra_hardware(data, failures): return try: - data['data'] = jsonutils.loads(out) - except ValueError as exc: + data['data'] = json.loads(out) + except json.decoder.JSONDecodeError as ex: msg = 'JSON returned from hardware-detect cannot be decoded: %s' - failures.add(msg, exc) + failures.add(msg, ex) def collect_pci_devices_info(data, failures): diff --git a/ironic_python_agent/ironic_api_client.py b/ironic_python_agent/ironic_api_client.py index dc6f0cf7..a2e7e1d0 100644 --- a/ironic_python_agent/ironic_api_client.py +++ b/ironic_python_agent/ironic_api_client.py @@ -12,9 +12,10 @@ # See the License for the specific language governing permissions and # limitations under the License. +import json + from oslo_config import cfg from oslo_log import log -from oslo_serialization import jsonutils import requests import tenacity @@ -103,7 +104,7 @@ class APIClient(object): try: response = self._request('GET', '/') - data = jsonutils.loads(response.content) + data = json.loads(response.content) version = data['default_version']['version'].split('.') self._ironic_api_version = (int(version[0]), int(version[1])) return self._ironic_api_version @@ -127,8 +128,8 @@ class APIClient(object): if not isinstance(body, dict): # Old ironic format try: - body = jsonutils.loads(body) - except ValueError: + body = json.loads(body) + except json.decoder.JSONDecodeError: body = {} text = (body.get('faultstring') @@ -253,8 +254,8 @@ class APIClient(object): return False try: - content = jsonutils.loads(response.content) - except Exception as e: + content = json.loads(response.content) + except json.decoder.JSONDecodeError as e: LOG.warning('Error decoding response: %s', e) return False diff --git a/ironic_python_agent/tests/unit/extensions/test_image.py b/ironic_python_agent/tests/unit/extensions/test_image.py index 2444e30d..e488b74f 100644 --- a/ironic_python_agent/tests/unit/extensions/test_image.py +++ b/ironic_python_agent/tests/unit/extensions/test_image.py @@ -29,6 +29,7 @@ from ironic_python_agent import hardware from ironic_python_agent import partition_utils from ironic_python_agent import raid_utils from ironic_python_agent.tests.unit import base +from ironic_python_agent.tests.unit.samples import hardware_samples as hws @mock.patch.object(hardware, 'dispatch_to_managers', autospec=True) @@ -756,17 +757,7 @@ Boot0004* ironic1 HD(1,GPT,55db8d03-c8f6-4a5b-9155-790dddc348fa,0x800,0x640 mock_is_md_device.return_value = False mock_md_get_raid_devices.return_value = {} mock_exists.side_effect = iter([False, True, False, True, True]) - partuuid_device = ('KNAME="sda" MODEL="DRIVE 0" SIZE="10240000" ' - 'ROTA="1" TYPE="disk" UUID="987654-3210" ' - 'PARTUUID=""\n' - 'KNAME="sda0" MODEL="DRIVE 0" SIZE="102400" ' - 'ROTA="1" TYPE="part" ' - 'UUID="' + self.fake_efi_system_part_uuid + '" ' - 'PARTUUID="1234-2918"\n') - exec_side_effect = [('', '')] * 16 - exec_side_effect.append((partuuid_device, '')) - exec_side_effect.extend([('', '')] * 8) - mock_execute.side_effect = exec_side_effect + mock_execute.return_value = (hws.PARTUUID_DEVICE_TEMPLATE, '') with mock.patch('builtins.open', mock.mock_open()) as mock_open: image._install_grub2( @@ -823,8 +814,9 @@ Boot0004* ironic1 HD(1,GPT,55db8d03-c8f6-4a5b-9155-790dddc348fa,0x800,0x640 'GRUB_SAVEDEFAULT': 'true'}, use_standard_locale=True), mock.call('udevadm', 'settle'), - mock.call('lsblk', '-Pbia', - '-oKNAME,MODEL,SIZE,ROTA,TYPE,UUID,PARTUUID'), + mock.call('lsblk', '-bia', '--json', + '-oKNAME,MODEL,SIZE,ROTA,TYPE,UUID,PARTUUID', + check_exit_code=[0]), mock.call('umount', self.fake_dir + '/boot/efi', attempts=3, delay_on_retry=True), mock.call(('chroot %s /bin/sh -c "umount -a -t vfat"' % @@ -870,17 +862,7 @@ Boot0004* ironic1 HD(1,GPT,55db8d03-c8f6-4a5b-9155-790dddc348fa,0x800,0x640 environ_mock.get.return_value = '/sbin' mock_is_md_device.return_value = False mock_md_get_raid_devices.return_value = {} - partuuid_device = ('KNAME="sda" MODEL="DRIVE 0" SIZE="10240000" ' - 'ROTA="1" TYPE="disk" UUID="987654-3210" ' - 'PARTUUID=""\n' - 'KNAME="sda0" MODEL="DRIVE 0" SIZE="102400" ' - 'ROTA="1" TYPE="part" UUID="987654-3210" ' - 'PARTUUID="' + self.fake_efi_system_part_uuid - + '"\n') - exec_side_effect = [('', '')] * 16 - exec_side_effect.append((partuuid_device, '')) - exec_side_effect.extend([('', '')] * 8) - mock_execute.side_effect = exec_side_effect + mock_execute.return_value = (hws.PARTUUID_DEVICE_TEMPLATE, '') # Validates the complete opposite path *and* no-write behavior # occurs if the entry already exists. fstab_data = ( @@ -951,8 +933,9 @@ Boot0004* ironic1 HD(1,GPT,55db8d03-c8f6-4a5b-9155-790dddc348fa,0x800,0x640 'GRUB_SAVEDEFAULT': 'true'}, use_standard_locale=True), mock.call('udevadm', 'settle'), - mock.call('lsblk', '-Pbia', - '-oKNAME,MODEL,SIZE,ROTA,TYPE,UUID,PARTUUID'), + mock.call('lsblk', '-bia', '--json', + '-oKNAME,MODEL,SIZE,ROTA,TYPE,UUID,PARTUUID', + check_exit_code=[0]), mock.call('umount', self.fake_dir + '/boot/efi', attempts=3, delay_on_retry=True), mock.call(('chroot %s /bin/sh -c "umount -a -t vfat"' % @@ -1977,9 +1960,11 @@ Boot0004* ironic1 HD(1,GPT,55db8d03-c8f6-4a5b-9155-790dddc348fa,0x800,0x640 mock_dispatch.assert_any_call('get_boot_info') self.assertEqual(0, mock_execute.call_count) + @mock.patch.object(hardware, 'list_all_block_devices', autospec=True) @mock.patch.object(os.path, 'exists', lambda *_: True) def test__append_uefi_to_fstab_handles_error(self, mock_execute, - mock_dispatch): + mock_dispatch, + mock_list_blk_devs): with mock.patch('builtins.open', mock.mock_open()) as mock_open: mock_open.side_effect = OSError('boom') image._append_uefi_to_fstab( diff --git a/ironic_python_agent/tests/unit/samples/hardware_samples.py b/ironic_python_agent/tests/unit/samples/hardware_samples.py index e402af45..82f29eb7 100644 --- a/ironic_python_agent/tests/unit/samples/hardware_samples.py +++ b/ironic_python_agent/tests/unit/samples/hardware_samples.py @@ -97,43 +97,52 @@ HDPARM_INFO_TEMPLATE = ( 'Checksum: correct\n' ) -BLK_DEVICE_TEMPLATE = ( - 'KNAME="sda" MODEL="TinyUSB Drive" SIZE="3116853504" ' - 'ROTA="0" TYPE="disk" SERIAL="123" UUID="F531-BDC3" PARTUUID=""\n' - 'KNAME="sdb" MODEL="Fastable SD131 7" SIZE="10737418240" ' - 'ROTA="0" TYPE="disk" UUID="9a5e5cca-e03d-4cbd-9054-9e6ca9048222" ' - 'PARTUUID=""\n' - 'KNAME="sdc" MODEL="NWD-BLP4-1600 " SIZE="1765517033472" ' - ' ROTA="0" TYPE="disk" UUID="" PARTUUID=""\n' - 'KNAME="sdd" MODEL="NWD-BLP4-1600 " SIZE="1765517033472" ' - ' ROTA="0" TYPE="disk" UUID="" PARTUUID=""\n' - 'KNAME="loop0" MODEL="" SIZE="109109248" ROTA="1" TYPE="loop" UUID="" ' - 'PARTUUID=""\n' - 'KNAME="zram0" MODEL="" SIZE="" ROTA="0" TYPE="disk" UUID="" PARTUUID=""\n' - 'KNAME="ram0" MODEL="" SIZE="8388608" ROTA="0" TYPE="disk" UUID="" ' - 'PARTUUID=""\n' - 'KNAME="ram1" MODEL="" SIZE="8388608" ROTA="0" TYPE="disk" UUID="" ' - 'PARTUUID=""\n' - 'KNAME="ram2" MODEL="" SIZE="8388608" ROTA="0" TYPE="disk" UUID="" ' - 'PARTUUID=""\n' - 'KNAME="ram3" MODEL="" SIZE="8388608" ROTA="0" TYPE="disk" UUID="" ' - 'PARTUUID=""\n' - 'KNAME="fd1" MODEL="magic" SIZE="4096" ROTA="1" TYPE="disk" UUID="" ' - 'PARTUUID=""\n' - 'KNAME="sdf" MODEL="virtual floppy" SIZE="0" ROTA="1" TYPE="disk" UUID="" ' - 'PARTUUID=""\n' - 'KNAME="dm-0" MODEL="NWD-BLP4-1600 " SIZE="1765517033472" ' - ' ROTA="0" TYPE="mpath" UUID="" PARTUUID=""\n' - -) +BLK_DEVICE_TEMPLATE = """ +{ + "blockdevices": [ + {"kname":"sda", "model":"TinyUSB Drive", "size":3116853504, + "rota":false, "type":"disk", "serial":123, "uuid":"F531-BDC3", + "partuuid":null}, + {"kname":"sdb", "model":"Fastable SD131 7", "size":10737418240, + "rota":false, "type":"disk", + "uuid":"9a5e5cca-e03d-4cbd-9054-9e6ca9048222", "partuuid":null}, + {"kname":"sdc", "model":"NWD-BLP4-1600", "size":1765517033472, + "rota":false, "type":"disk", "uuid":null, "partuuid":null}, + {"kname":"sdd", "model":"NWD-BLP4-1600", "size":1765517033472, + "rota":false, "type":"disk", "uuid":null, "partuuid":null}, + {"kname":"loop0", "model":null, "size":109109248, "rota":true, + "type":"loop", "uuid":null, "partuuid": null}, + {"kname":"zram0", "model":null, "size":0, "rota":false, "type":"disk", + "uuid":null, "partuuid":null}, + {"kname":"ram0", "model":null, "size":8388608, "rota":false, + "type":"disk", "uuid":null, "partuuid":null}, + {"kname":"ram1", "model":null, "size":8388608, "rota":false, + "type":"disk", "uuid":null, "partuuid":null}, + {"kname":"ram2", "model":null, "size":8388608, "rota":false, + "type":"disk", "uuid":null, "partuuid":null}, + {"kname":"ram3", "model":null, "size":8388608, "rota":false, + "type":"disk", "uuid":null, "partuuid":null}, + {"kname":"fd1", "model":"magic", "size":4096, "rota":true, + "type":"disk", "uuid":null, "partuuid":null}, + {"kname":"sdf", "model":"virtual floppy", "size":0, "rota":true, + "type":"disk", "uuid":null, "partuuid":null}, + {"kname":"dm-0", "model":"NWD-BLP4-1600", "size":"1765517033472", + "rota":false, "type":"mpath", "uuid":null, "partuuid":null} + ] +} +""" # NOTE(pas-ha) largest device is 1 byte smaller than 4GiB -BLK_DEVICE_TEMPLATE_SMALL = ( - 'KNAME="sda" MODEL="TinyUSB Drive" SIZE="3116853504" ' - 'ROTA="0" TYPE="disk" UUID="F531-BDC3" PARTUUID=""\n' - 'KNAME="sdb" MODEL="AlmostBigEnough Drive" SIZE="4294967295" ' - 'ROTA="0" TYPE="disk" UUID="" PARTUUID=""' -) +BLK_DEVICE_TEMPLATE_SMALL = """ +{ + "blockdevices": [ + {"kname":"sda", "model":"TinyUSB Drive", "size":3116853504, "rota":false, + "type":"disk", "uuid":"F531-BDC", "partuuid":null}, + {"kname":"sdb", "model":"AlmostBigEnough Drive", "size":"4294967295", + "rota":false, "type":"disk", "uuid":null, "partuuid":null} + ] +} +""" # NOTE(TheJulia): This list intentionally contains duplicates # as the code filters them out by kernel device name. @@ -142,76 +151,92 @@ BLK_DEVICE_TEMPLATE_SMALL = ( # ROTA has been set to 0 on some software RAID devices for testing # purposes. In practice is appears to inherit from the underyling # devices, so in this example it would normally be 1. -RAID_BLK_DEVICE_TEMPLATE = ( - 'KNAME="sda" MODEL="DRIVE 0" SIZE="1765517033472" ' - 'ROTA="1" TYPE="disk" UUID="" PARTUUID=""\n' - 'KNAME="sda1" MODEL="DRIVE 0" SIZE="107373133824" ' - 'ROTA="1" TYPE="part" UUID="" PARTUUID=""\n' - 'KNAME="sdb" MODEL="DRIVE 1" SIZE="1765517033472" ' - 'ROTA="1" TYPE="disk" UUID="" PARTUUID=""\n' - 'KNAME="sdb" MODEL="DRIVE 1" SIZE="1765517033472" ' - 'ROTA="1" TYPE="disk" UUID="" PARTUUID=""\n' - 'KNAME="sdb1" MODEL="DRIVE 1" SIZE="107373133824" ' - 'ROTA="1" TYPE="part" UUID="" PARTUUID=""\n' - 'KNAME="md0p1" MODEL="RAID" SIZE="107236818944" ' - 'ROTA="0" TYPE="md" UUID="" PARTUUID=""\n' - 'KNAME="md0" MODEL="RAID" SIZE="1765517033470" ' - 'ROTA="0" TYPE="raid1" UUID="" PARTUUID=""\n' - 'KNAME="md0" MODEL="RAID" SIZE="1765517033470" ' - 'ROTA="0" TYPE="raid1" UUID="" PARTUUID=""\n' - 'KNAME="md1" MODEL="RAID" SIZE="" ROTA="0" TYPE="raid1" UUID="" ' - 'PARTUUID=""' -) +RAID_BLK_DEVICE_TEMPLATE = (""" +{ + "blockdevices": [ + {"kname":"sda", "model":"DRIVE 0", "size":1765517033472, "rota":true, + "type":"disk", "uuid":null, "partuuid":null}, + {"kname":"sda1", "model":"DRIVE 0", "size":107373133824, "rota":true, + "type":"part", "uuid":null, "partuuid":null}, + {"kname":"sdb", "model":"DRIVE 1", "size":1765517033472, "rota":true, + "type":"disk", "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}, + {"kname":"md0p1", "model":"RAID", "size":107236818944, "rota":false, + "type":"md", "uuid":null, "partuuid":null}, + {"kname":"md0", "model":"RAID", "size":1765517033470, "rota":false, + "type":"raid1", "uuid":null, "partuuid":null}, + {"kname":"md0", "model":"RAID", "size":1765517033470, "rota":false, + "type":"raid1", "uuid":null, "partuuid":null}, + {"kname":"md1", "model":"RAID", "size":0, "rota":false, "type":"raid1", + "uuid":null, "partuuid":null} + ] +} +""") -MULTIPATH_BLK_DEVICE_TEMPLATE = ( - 'KNAME="sda" MODEL="INTEL_SSDSC2CT060A3" SIZE="60022480896" ROTA="0" ' - 'TYPE="disk" UUID="" PARTUUID=""\n' - 'KNAME="sda2" MODEL="" SIZE="59162722304" ROTA="0" TYPE="part" ' - 'UUID="f8b55d59-96c3-3982-b129-1b6b2ee8da86" ' - 'PARTUUID="c97c8aac-7796-4433-b1fc-9b5fac43edf3"\n' - 'KNAME="sda3" MODEL="" SIZE="650002432" ROTA="0" TYPE="part" ' - 'UUID="b3b03565-5f13-3c93-b2a6-6d90e25be926" ' - 'PARTUUID="6c85beff-b2bd-4a1c-91b7-8abb5256459d"\n' - 'KNAME="sda1" MODEL="" SIZE="209715200" ROTA="0" TYPE="part" ' - 'UUID="0a83355d-7500-3f5f-9abd-66f6fd03714c" ' - 'PARTUUID="eba28b26-b76a-402c-94dd-0b66a523a485"\n' - 'KNAME="dm-0" MODEL="" SIZE="60022480896" ROTA="0" TYPE="mpath" ' - 'UUID="" PARTUUID=""\n' - 'KNAME="dm-4" MODEL="" SIZE="650002432" ROTA="0" TYPE="part" ' - 'UUID="b3b03565-5f13-3c93-b2a6-6d90e25be926" ' - 'PARTUUID="6c85beff-b2bd-4a1c-91b7-8abb5256459d"\n' - 'KNAME="dm-2" MODEL="" SIZE="209715200" ROTA="0" TYPE="part" ' - 'UUID="0a83355d-7500-3f5f-9abd-66f6fd03714c" ' - 'PARTUUID="eba28b26-b76a-402c-94dd-0b66a523a485"\n' - 'KNAME="dm-3" MODEL="" SIZE="59162722304" ROTA="0" TYPE="part" ' - 'UUID="f8b55d59-96c3-3982-b129-1b6b2ee8da86" ' - 'PARTUUID="c97c8aac-7796-4433-b1fc-9b5fac43edf3"\n' - 'KNAME="sdb" MODEL="INTEL_SSDSC2CT060A3" SIZE="60022480896" ' - 'ROTA="0" TYPE="disk" UUID="" PARTUUID=""\n' - 'KNAME="sdb2" MODEL="" SIZE="59162722304" ROTA="0" TYPE="part" ' - 'UUID="f8b55d59-96c3-3982-b129-1b6b2ee8da86" ' - 'PARTUUID="c97c8aac-7796-4433-b1fc-9b5fac43edf3"\n' - 'KNAME="sdb3" MODEL="" SIZE="650002432" ROTA="0" TYPE="part" ' - 'UUID="b3b03565-5f13-3c93-b2a6-6d90e25be926" ' - 'PARTUUID="6c85beff-b2bd-4a1c-91b7-8abb5256459d"\n' - 'KNAME="sdb1" MODEL="" SIZE="209715200" ROTA="0" TYPE="part" ' - 'UUID="0a83355d-7500-3f5f-9abd-66f6fd03714c" ' - 'PARTUUID="eba28b26-b76a-402c-94dd-0b66a523a485"\n' - 'KNAME="sdc" MODEL="ST1000DM003-1CH162" SIZE="1000204886016" ' - 'ROTA="1" TYPE="disk" UUID="" PARTUUID=""\n' - 'KNAME="sdc1" MODEL="" SIZE="899999072256" ROTA="1" TYPE="part" ' - 'UUID="457f7d3c-9376-4997-89bd-d1a7c8b04060" ' - 'PARTUUID="c9433d2e-3bbc-47b4-92bf-43c1d80f06e0"\n' - 'KNAME="dm-1" MODEL="" SIZE="1000204886016" ROTA="0" TYPE="mpath" ' - 'UUID="" PARTUUID=""\n' -) +MULTIPATH_BLK_DEVICE_TEMPLATE = (""" +{ + "blockdevices": [ + {"kname":"sda", "model":"INTEL_SSDSC2CT060A3", "size":"60022480896", + "rota":false, "type":"disk", "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"}, + {"kname":"sda3", "model":null, "size":"650002432", "rota":false, + "type":"part", "uuid":"b3b03565-5f13-3c93-b2a6-6d90e25be926", + "partuuid":"6c85beff-b2bd-4a1c-91b7-8abb5256459d"}, + {"kname":"sda1", "model":null, "size":"209715200", "rota":false, + "type":"part", "uuid":"0a83355d-7500-3f5f-9abd-66f6fd03714c", + "partuuid":"eba28b26-b76a-402c-94dd-0b66a523a485"}, + {"kname":"dm-0", "model":null, "size":"60022480896", "rota":false, + "type":"mpath", "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"}, + {"kname":"dm-2", "model":null, "size":"209715200", "rota":false, + "type":"part", "uuid":"0a83355d-7500-3f5f-9abd-66f6fd03714c", + "partuuid":"eba28b26-b76a-402c-94dd-0b66a523a485"}, + {"kname":"dm-3", "model":null, "size":"59162722304", "rota":false, + "type":"part", "uuid":"f8b55d59-96c3-3982-b129-1b6b2ee8da86", + "partuuid":"c97c8aac-7796-4433-b1fc-9b5fac43edf3"}, + {"kname":"sdb", "model":"INTEL_SSDSC2CT060A3", "size":"60022480896", + "rota":false, "type":"disk", "uuid":null, "partuuid":null}, + {"kname":"sdb2", "model":null, "size":"59162722304", + "rota":false, "type":"part", + "uuid":"f8b55d59-96c3-3982-b129-1b6b2ee8da86", + "partuuid":"c97c8aac-7796-4433-b1fc-9b5fac43edf3"}, + {"kname":"sdb3", "model":null, "size":"650002432", + "rota":false, "type":"part", + "uuid":"b3b03565-5f13-3c93-b2a6-6d90e25be926", + "partuuid":"6c85beff-b2bd-4a1c-91b7-8abb5256459d"}, + {"kname":"sdb1", "model":null, "size":"209715200", + "rota":false, "type":"part", + "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}, + {"kname":"sdc1", "model":null, "size":"899999072256", + "rota":true, "type":"part", + "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} + ] +} +""") -PARTUUID_DEVICE_TEMPLATE = ( - 'KNAME="sda" MODEL="DRIVE 0" SIZE="1765517033472" ' - 'ROTA="1" TYPE="disk" UUID="" PARTUUID=""\n' - 'KNAME="sda1" MODEL="DRIVE 0" SIZE="107373133824" ' - 'ROTA="1" TYPE="part" UUID="987654-3210" PARTUUID="1234-5678"\n' -) +PARTUUID_DEVICE_TEMPLATE = (""" +{ + "blockdevices": [ + {"kname":"sda", "model":"DRIVE 0", "size":1765517033472, "rota":true, + "type":"disk", "uuid":null, "partuuid":null}, + {"kname":"sda1", "model":"DRIVE 0", "size":107373133824, "rota":true, + "type":"part", "uuid":"987654-3210", "partuuid":"1234-5678"} + ] +} +""") SHRED_OUTPUT_0_ITERATIONS_ZERO_FALSE = () @@ -241,7 +266,6 @@ SHRED_OUTPUT_2_ITERATIONS_ZERO_FALSE = ( 'shred: /dev/sda: pass 2/2 (random)...29GiB/29GiB 100%\n' ) - LSCPU_OUTPUT = """ Architecture: x86_64 CPU op-mode(s): 32-bit, 64-bit @@ -893,7 +917,6 @@ Copyright (C) 2002-13, Bruce Allen, Christian Franke, www.smartmontools.org ATA Security is: Unavailable """) # noqa - IPMITOOL_LAN6_PRINT_DYNAMIC_ADDR = """ IPv6 Dynamic Address 0: Source/Type: DHCPv6 @@ -1008,7 +1031,6 @@ Working Devices : 2 1 259 3 1 active sync /dev/nvme1n1p1 """) - MDADM_DETAIL_OUTPUT_BROKEN_RAID0 = ("""/dev/md126: Version : 1.2 Raid Level : raid0 @@ -1027,7 +1049,6 @@ MDADM_DETAIL_OUTPUT_BROKEN_RAID0 = ("""/dev/md126: - 8 2 - /dev/sda2 """) - MDADM_EXAMINE_OUTPUT_MEMBER = ("""/dev/sda1: Magic : a92b4efc Version : 1.2 @@ -1056,7 +1077,6 @@ MDADM_EXAMINE_OUTPUT_MEMBER = ("""/dev/sda1: Array State : A. ('A' == active, '.' == missing, 'R' == replacing) """) - MDADM_EXAMINE_OUTPUT_NON_MEMBER = ("""/dev/sdz1: Magic : a92b4efc Version : 1.2 @@ -1085,7 +1105,6 @@ MDADM_EXAMINE_OUTPUT_NON_MEMBER = ("""/dev/sdz1: Array State : A. ('A' == active, '.' == missing, 'R' == replacing) """) - PROC_MOUNTS_OUTPUT = (""" debugfs /sys/kernel/debug debugfs rw,relatime 0 0 /dev/sda2 / ext4 rw,relatime,errors=remount-ro 0 0 @@ -1094,7 +1113,6 @@ pstore /sys/fs/pstore pstore rw,nosuid,nodev,noexec,relatime 0 0 /dev/loop19 /snap/core/10126 squashfs ro,nodev,relatime 0 0 """) - PROC_MOUNTS_OUTPUT_NO_PSTORE = (""" debugfs /sys/kernel/debug debugfs rw,relatime 0 0 /dev/sda2 / ext4 rw,relatime,errors=remount-ro 0 0 diff --git a/ironic_python_agent/tests/unit/test_agent.py b/ironic_python_agent/tests/unit/test_agent.py index d90b0414..b5e40bf6 100644 --- a/ironic_python_agent/tests/unit/test_agent.py +++ b/ironic_python_agent/tests/unit/test_agent.py @@ -12,6 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. +import json import socket import time from unittest import mock @@ -19,7 +20,6 @@ from unittest import mock from ironic_lib import exception as lib_exc from oslo_concurrency import processutils from oslo_config import cfg -from oslo_serialization import jsonutils import pkg_resources from stevedore import extension @@ -192,8 +192,8 @@ class TestBaseAgent(ironic_agent_base.IronicAgentTest): # object. a_encoded = self.encoder.encode(a) b_encoded = self.encoder.encode(b) - self.assertEqual(jsonutils.loads(a_encoded), - jsonutils.loads(b_encoded)) + self.assertEqual(json.loads(a_encoded), + json.loads(b_encoded)) def test_get_status(self): started_at = time.time() diff --git a/ironic_python_agent/tests/unit/test_hardware.py b/ironic_python_agent/tests/unit/test_hardware.py index f6b7ccc1..a610eb2f 100644 --- a/ironic_python_agent/tests/unit/test_hardware.py +++ b/ironic_python_agent/tests/unit/test_hardware.py @@ -868,8 +868,9 @@ class TestGenericHardwareManager(base.IronicAgentTest): stderr='the -c option requires a path to check'), # dm-1 ] expected = [ - mock.call('lsblk', '-Pbia', - '-oKNAME,MODEL,SIZE,ROTA,TYPE,UUID,PARTUUID'), + mock.call('lsblk', '-bia', '--json', + '-oKNAME,MODEL,SIZE,ROTA,TYPE,UUID,PARTUUID', + check_exit_code=[0]), mock.call('multipath', '-c', '/dev/sda'), mock.call('multipath', '-ll', '/dev/sda'), mock.call('multipath', '-c', '/dev/sda2'), @@ -947,8 +948,9 @@ class TestGenericHardwareManager(base.IronicAgentTest): stderr='the -c option requires a path to check'), # dm-1 ] expected = [ - mock.call('lsblk', '-Pbia', - '-oKNAME,MODEL,SIZE,ROTA,TYPE,UUID,PARTUUID'), + mock.call('lsblk', '-bia', '--json', + '-oKNAME,MODEL,SIZE,ROTA,TYPE,UUID,PARTUUID', + check_exit_code=[0]), mock.call('multipath', '-c', '/dev/sda'), mock.call('multipath', '-ll', '/dev/sda'), mock.call('multipath', '-c', '/dev/sda2'), @@ -1001,8 +1003,9 @@ class TestGenericHardwareManager(base.IronicAgentTest): # should always be smaller self.assertEqual('/dev/md0', self.hardware.get_os_install_device()) expected = [ - mock.call('lsblk', '-Pbia', - '-oKNAME,MODEL,SIZE,ROTA,TYPE,UUID,PARTUUID'), + mock.call('lsblk', '-bia', '--json', + '-oKNAME,MODEL,SIZE,ROTA,TYPE,UUID,PARTUUID', + check_exit_code=[0]), ] mocked_execute.assert_has_calls(expected) @@ -1026,11 +1029,16 @@ class TestGenericHardwareManager(base.IronicAgentTest): ex = self.assertRaises(errors.DeviceNotFound, self.hardware.get_os_install_device) expected = [ - mock.call('lsblk', '-Pbia', - '-oKNAME,MODEL,SIZE,ROTA,TYPE,UUID,PARTUUID'), + mock.call('lsblk', '-bia', '--json', + '-oKNAME,MODEL,SIZE,ROTA,TYPE,UUID,PARTUUID', + 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', + check_exit_code=[0]) self.assertIn(str(4 * units.Gi), ex.details) mock_cached_node.assert_called_once_with() self.assertEqual(1, mocked_mpath.call_count) @@ -1535,8 +1543,9 @@ class TestGenericHardwareManager(base.IronicAgentTest): for dev in range(3)] mock_readlink.assert_has_calls(expected_calls) expected_calls = [ - mock.call('lsblk', '-Pbia', - '-oKNAME,MODEL,SIZE,ROTA,TYPE,UUID,PARTUUID'), + mock.call('lsblk', '-bia', '--json', + '-oKNAME,MODEL,SIZE,ROTA,TYPE,UUID,PARTUUID', + check_exit_code=[0]), mock.call('multipath', '-c', '/dev/sda'), mock.call('multipath', '-c', '/dev/sdb'), mock.call('multipath', '-c', '/dev/sdc'), @@ -1633,13 +1642,16 @@ class TestGenericHardwareManager(base.IronicAgentTest): ('', ''), ] - mocked_mpath.return_value = False - mocked_udev.side_effect = iter([ + 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(5) - ]) + for i in range(3) + ] + [ + {'DM_WWN': 'wwn3', 'DM_SERIAL': 'serial3'}, + ] mocked_dev_vendor.return_value = 'Super Vendor' devices = hardware.list_all_block_devices() expected_devices = [ @@ -1673,19 +1685,19 @@ class TestGenericHardwareManager(base.IronicAgentTest): wwn_vendor_extension='wwn-vendor-ext2', serial='serial2', hctl='1:0:0:0'), - hardware.BlockDevice(name='/dev/sdd', + hardware.BlockDevice(name='/dev/dm-0', model='NWD-BLP4-1600', size=1765517033472, rotational=False, vendor='Super Vendor', wwn='wwn3', - wwn_with_extension='wwn-ext3', - wwn_vendor_extension='wwn-vendor-ext3', + wwn_with_extension=None, + wwn_vendor_extension=None, serial='serial3', hctl='1:0:0:0') ] - self.assertEqual(4, len(expected_devices)) + 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', @@ -1694,7 +1706,7 @@ class TestGenericHardwareManager(base.IronicAgentTest): 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')] + for dev in ('sda', 'sdb', 'sdc', 'dm-0')] mocked_listdir.assert_has_calls(expected_calls) mocked_mpath.assert_called_once_with() @@ -3622,6 +3634,7 @@ class TestGenericHardwareManager(base.IronicAgentTest): ] } self.node['target_raid_config'] = raid_config + mocked_execute.return_value = (hws.RAID_BLK_DEVICE_TEMPLATE, '') self.assertRaises(errors.SoftwareRAIDError, self.hardware.create_configuration, self.node, []) @@ -4280,6 +4293,7 @@ class TestGenericHardwareManager(base.IronicAgentTest): }, ] } + mocked_execute.return_value = (hws.RAID_BLK_DEVICE_TEMPLATE, '') self.assertIsNone(self.hardware.validate_configuration(raid_config, self.node)) @@ -4299,6 +4313,7 @@ class TestGenericHardwareManager(base.IronicAgentTest): }, ] } + mocked_execute.return_value = (hws.RAID_BLK_DEVICE_TEMPLATE, '') self.assertRaises(errors.SoftwareRAIDError, self.hardware.validate_configuration, raid_config, self.node) @@ -4319,6 +4334,7 @@ class TestGenericHardwareManager(base.IronicAgentTest): }, ] } + mocked_execute.return_value = (hws.RAID_BLK_DEVICE_TEMPLATE, '') self.assertRaises(errors.SoftwareRAIDError, self.hardware.validate_configuration, raid_config, self.node) @@ -4642,8 +4658,9 @@ class TestModuleFunctions(base.IronicAgentTest): ] result = hardware.list_all_block_devices() expected_calls = [ - mock.call('lsblk', '-Pbia', - '-oKNAME,MODEL,SIZE,ROTA,TYPE,UUID,PARTUUID'), + mock.call('lsblk', '-bia', '--json', + '-oKNAME,MODEL,SIZE,ROTA,TYPE,UUID,PARTUUID', + check_exit_code=[0]), mock.call('multipath', '-c', '/dev/sda'), mock.call('multipath', '-c', '/dev/sdb') ] @@ -4688,8 +4705,9 @@ class TestModuleFunctions(base.IronicAgentTest): stderr='the -c option requires a path to check'), # md1 ] expected_calls = [ - mock.call('lsblk', '-Pbia', - '-oKNAME,MODEL,SIZE,ROTA,TYPE,UUID,PARTUUID'), + mock.call('lsblk', '-bia', '--json', + '-oKNAME,MODEL,SIZE,ROTA,TYPE,UUID,PARTUUID', + check_exit_code=[0]), mock.call('multipath', '-c', '/dev/sda'), mock.call('multipath', '-c', '/dev/sda1'), mock.call('multipath', '-c', '/dev/sdb'), @@ -4726,8 +4744,9 @@ class TestModuleFunctions(base.IronicAgentTest): ] result = hardware.list_all_block_devices(block_type='part') expected_calls = [ - mock.call('lsblk', '-Pbia', - '-oKNAME,MODEL,SIZE,ROTA,TYPE,UUID,PARTUUID'), + mock.call('lsblk', '-bia', '--json', + '-oKNAME,MODEL,SIZE,ROTA,TYPE,UUID,PARTUUID', + check_exit_code=[0]), mock.call('multipath', '-c', '/dev/sda'), mock.call('multipath', '-c', '/dev/sda1'), ] @@ -4744,10 +4763,13 @@ class TestModuleFunctions(base.IronicAgentTest): mock_mpath_enabled, mocked_execute): mock_mpath_enabled.return_value = False - mocked_execute.return_value = ('TYPE="foo" MODEL="model"', '') + mocked_execute.return_value = ( + '{"blockdevices": [{"type":"foo", "model":"model"}]}', '') result = hardware.list_all_block_devices() mocked_execute.assert_called_once_with( - 'lsblk', '-Pbia', '-oKNAME,MODEL,SIZE,ROTA,TYPE,UUID,PARTUUID') + 'lsblk', '-bia', '--json', + '-oKNAME,MODEL,SIZE,ROTA,TYPE,UUID,PARTUUID', + check_exit_code=[0]) self.assertEqual([], result) mocked_udev.assert_called_once_with() @@ -4758,22 +4780,17 @@ class TestModuleFunctions(base.IronicAgentTest): mocked_execute): """Test for missing values returned from lsblk""" mocked_mpath.return_value = False - mocked_execute.side_effect = [ - ('TYPE="disk" MODEL="model"', ''), - ('', ''), - ('', ''), - ('', ''), - ('', ''), - ] expected_calls = [ - mock.call('lsblk', '-Pbia', - '-oKNAME,MODEL,SIZE,ROTA,TYPE,UUID,PARTUUID'), + mock.call('lsblk', '-bia', '--json', + '-oKNAME,MODEL,SIZE,ROTA,TYPE,UUID,PARTUUID', + check_exit_code=[0]), ] - + mocked_execute.return_value = ( + '{"blockdevices": [{"type":"disk", "model":"model"}]}', '') self.assertRaisesRegex( errors.BlockDeviceError, - r'^Block device caused unknown error: KNAME, PARTUUID, ROTA, ' - r'SIZE, UUID must be returned by lsblk.$', + r'^Block device caused unknown error: kname, partuuid, rota, ' + r'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/tests/unit/test_ironic_api_client.py b/ironic_python_agent/tests/unit/test_ironic_api_client.py index ba603651..8c05652c 100644 --- a/ironic_python_agent/tests/unit/test_ironic_api_client.py +++ b/ironic_python_agent/tests/unit/test_ironic_api_client.py @@ -16,7 +16,6 @@ import json from unittest import mock from oslo_config import cfg -from oslo_serialization import jsonutils import requests from ironic_python_agent import errors @@ -149,7 +148,7 @@ class TestBaseIronicPythonAgent(base.IronicAgentTest): expected_data = { 'callback_url': 'http://192.0.2.1:9999', 'agent_version': version.__version__} - self.assertEqual(jsonutils.dumps(expected_data), data) + self.assertEqual(json.dumps(expected_data), data) def test_successful_heartbeat_ip6(self): response = FakeResponse(status_code=202) @@ -172,7 +171,7 @@ class TestBaseIronicPythonAgent(base.IronicAgentTest): expected_data = { 'callback_url': 'http://[fc00:1111::4]:9999', 'agent_version': version.__version__} - self.assertEqual(jsonutils.dumps(expected_data), data) + self.assertEqual(json.dumps(expected_data), data) def test_successful_heartbeat_with_token(self): response = FakeResponse(status_code=202) @@ -197,7 +196,7 @@ class TestBaseIronicPythonAgent(base.IronicAgentTest): 'callback_url': 'http://192.0.2.1:9999', 'agent_token': 'magical', 'agent_version': version.__version__} - self.assertEqual(jsonutils.dumps(expected_data), data) + self.assertEqual(json.dumps(expected_data), data) def test_heartbeat_agent_version_unsupported(self): response = FakeResponse(status_code=202) @@ -218,7 +217,7 @@ class TestBaseIronicPythonAgent(base.IronicAgentTest): self.assertEqual(API_URL + heartbeat_path, request_args[1]) expected_data = { 'callback_url': 'http://[fc00:1111::4]:9999'} - self.assertEqual(jsonutils.dumps(expected_data), data) + self.assertEqual(json.dumps(expected_data), data) def test_successful_heartbeat_with_verify_ca(self): response = FakeResponse(status_code=202) @@ -246,7 +245,7 @@ class TestBaseIronicPythonAgent(base.IronicAgentTest): 'agent_token': 'magical', 'agent_version': version.__version__, 'agent_verify_ca': 'I am a cert'} - self.assertEqual(jsonutils.dumps(expected_data), data) + self.assertEqual(json.dumps(expected_data), data) headers = self.api_client.session.request.call_args[1]['headers'] self.assertEqual( '%d.%d' % ironic_api_client.AGENT_VERIFY_CA_IRONIC_VERSION, diff --git a/ironic_python_agent/tests/unit/test_utils.py b/ironic_python_agent/tests/unit/test_utils.py index c99f7dca..9382a6b4 100644 --- a/ironic_python_agent/tests/unit/test_utils.py +++ b/ironic_python_agent/tests/unit/test_utils.py @@ -13,9 +13,11 @@ # License for the specific language governing permissions and limitations # under the License. +import base64 import errno import glob import io +import json import os import shutil import subprocess @@ -25,7 +27,6 @@ from unittest import mock from ironic_lib import utils as ironic_utils from oslo_concurrency import processutils -from oslo_serialization import base64 import requests import testtools @@ -320,7 +321,7 @@ class TestUtils(ironic_agent_base.IronicAgentTest): data = utils.gzip_and_b64encode(io_dict=io_dict) self.assertIsInstance(data, str) - res = io.BytesIO(base64.decode_as_bytes(data)) + res = io.BytesIO(base64.b64decode(data)) with tarfile.open(fileobj=res) as tar: members = [(m.name, m.size) for m in tar] self.assertEqual([('fake-name', len(contents))], members) @@ -410,12 +411,14 @@ class TestUtils(ironic_agent_base.IronicAgentTest): mock_call.side_effect = os_error self.assertFalse(utils.is_journalctl_present()) + @mock.patch.object(utils, '_collect_udev', autospec=True) @mock.patch.object(utils, 'gzip_and_b64encode', autospec=True) @mock.patch.object(utils, 'is_journalctl_present', autospec=True) @mock.patch.object(utils, 'get_command_output', autospec=True) @mock.patch.object(utils, 'get_journalctl_output', autospec=True) def test_collect_system_logs_journald( - self, mock_logs, mock_outputs, mock_journalctl, mock_gzip_b64): + self, mock_logs, mock_outputs, mock_journalctl, mock_gzip_b64, + mock_udev): mock_journalctl.return_value = True ret = 'Patrick Star' mock_gzip_b64.return_value = ret @@ -435,13 +438,16 @@ class TestUtils(ironic_agent_base.IronicAgentTest): 'mount': mock.ANY, 'parted': mock.ANY, 'multipath': mock.ANY}, file_list=[]) + mock_udev.assert_called_once_with(mock.ANY) + @mock.patch.object(utils, '_collect_udev', autospec=True) @mock.patch.object(utils, 'gzip_and_b64encode', autospec=True) @mock.patch.object(utils, 'is_journalctl_present', autospec=True) @mock.patch.object(utils, 'get_command_output', autospec=True) @mock.patch.object(utils, 'get_journalctl_output', autospec=True) def test_collect_system_logs_journald_with_logfile( - self, mock_logs, mock_outputs, mock_journalctl, mock_gzip_b64): + self, mock_logs, mock_outputs, mock_journalctl, mock_gzip_b64, + mock_udev): tmp = tempfile.NamedTemporaryFile() self.addCleanup(lambda: tmp.close()) @@ -465,12 +471,15 @@ class TestUtils(ironic_agent_base.IronicAgentTest): 'mount': mock.ANY, 'parted': mock.ANY, 'multipath': mock.ANY}, file_list=[tmp.name]) + mock_udev.assert_called_once_with(mock.ANY) + @mock.patch.object(utils, '_collect_udev', autospec=True) @mock.patch.object(utils, 'gzip_and_b64encode', autospec=True) @mock.patch.object(utils, 'is_journalctl_present', autospec=True) @mock.patch.object(utils, 'get_command_output', autospec=True) def test_collect_system_logs_non_journald( - self, mock_outputs, mock_journalctl, mock_gzip_b64): + self, mock_outputs, mock_journalctl, mock_gzip_b64, + mock_udev): mock_journalctl.return_value = False ret = 'SpongeBob SquarePants' mock_gzip_b64.return_value = ret @@ -490,12 +499,15 @@ class TestUtils(ironic_agent_base.IronicAgentTest): 'mount': mock.ANY, 'parted': mock.ANY, 'multipath': mock.ANY}, file_list=['/var/log']) + mock_udev.assert_called_once_with(mock.ANY) + @mock.patch.object(utils, '_collect_udev', autospec=True) @mock.patch.object(utils, 'gzip_and_b64encode', autospec=True) @mock.patch.object(utils, 'is_journalctl_present', autospec=True) @mock.patch.object(utils, 'get_command_output', autospec=True) def test_collect_system_logs_non_journald_with_logfile( - self, mock_outputs, mock_journalctl, mock_gzip_b64): + self, mock_outputs, mock_journalctl, mock_gzip_b64, + mock_udev): tmp = tempfile.NamedTemporaryFile() self.addCleanup(lambda: tmp.close()) @@ -519,6 +531,31 @@ class TestUtils(ironic_agent_base.IronicAgentTest): 'mount': mock.ANY, 'parted': mock.ANY, 'multipath': mock.ANY}, file_list=['/var/log', tmp.name]) + mock_udev.assert_called_once_with(mock.ANY) + + @mock.patch('pyudev.Context', lambda: mock.sentinel.context) + @mock.patch('pyudev.Devices.from_device_file', autospec=True) + @mock.patch.object(ironic_utils, 'execute', autospec=True) + def test_collect_udev(self, mock_execute, mock_from_dev): + mock_execute.return_value = """ + fake0 + fake1 + fake42 + """, "" + mock_from_dev.side_effect = [ + mock.Mock(properties={'ID_UUID': '0'}), + RuntimeError('nope'), + {'ID_UUID': '42'} + ] + + result = {} + utils._collect_udev(result) + self.assertEqual({'udev/fake0', 'udev/fake42'}, set(result)) + for i in ('0', '42'): + buf = result[f'udev/fake{i}'] + # Avoiding getvalue on purpose - checking that the IO is not closed + val = json.loads(buf.read().decode('utf-8')) + self.assertEqual({'ID_UUID': i}, val) def test_get_ssl_client_options(self): # defaults diff --git a/ironic_python_agent/utils.py b/ironic_python_agent/utils.py index 66f6819f..09d726cf 100644 --- a/ironic_python_agent/utils.py +++ b/ironic_python_agent/utils.py @@ -12,12 +12,14 @@ # See the License for the specific language governing permissions and # limitations under the License. +import base64 from collections import abc import contextlib import copy import errno import glob import io +import json import os import re import shutil @@ -30,9 +32,8 @@ from ironic_lib import utils as ironic_utils from oslo_concurrency import processutils from oslo_config import cfg from oslo_log import log as logging -from oslo_serialization import base64 -from oslo_serialization import jsonutils from oslo_utils import units +import pyudev import requests import tenacity @@ -502,6 +503,13 @@ def get_journalctl_output(lines=None, units=None): return get_command_output(cmd) +def _encode_as_text(s): + if isinstance(s, str): + s = s.encode('utf-8') + s = base64.b64encode(s) + return s.decode('ascii') + + def gzip_and_b64encode(io_dict=None, file_list=None): """Gzip and base64 encode files and BytesIO buffers. @@ -527,7 +535,44 @@ def gzip_and_b64encode(io_dict=None, file_list=None): tar.add(f) fp.seek(0) - return base64.encode_as_text(fp.getvalue()) + + return _encode_as_text(fp.getvalue()) + + +def _collect_udev(io_dict): + """Collect device properties from udev.""" + try: + out, _e = ironic_utils.execute('lsblk', '-no', 'KNAME') + except processutils.ProcessExecutionError as exc: + LOG.warning('Could not list block devices: %s', exc) + return + + context = pyudev.Context() + + for kname in out.splitlines(): + kname = kname.strip() + if not kname: + continue + + name = os.path.join('/dev', kname) + + try: + udev = pyudev.Devices.from_device_file(context, name) + except Exception as e: + LOG.warning("Device %(dev)s is inaccessible, skipping... " + "Error: %(error)s", {'dev': name, 'error': e}) + continue + + try: + props = dict(udev.properties) + except AttributeError: # pyudev < 0.20 + props = dict(udev) + + fp = io.TextIOWrapper(io.BytesIO(), encoding='utf-8') + json.dump(props, fp) + buf = fp.detach() + buf.seek(0) + io_dict[f'udev/{kname}'] = buf def collect_system_logs(journald_max_lines=None): @@ -568,6 +613,11 @@ def collect_system_logs(journald_max_lines=None): for name, cmd in COLLECT_LOGS_COMMANDS.items(): try_get_command_output(io_dict, name, cmd) + try: + _collect_udev(io_dict) + except Exception: + LOG.exception('Unexpected error when collecting udev properties') + return gzip_and_b64encode(io_dict=io_dict, file_list=file_list) @@ -643,8 +693,8 @@ def parse_capabilities(root): capabilities = root.get('capabilities', {}) if isinstance(capabilities, str): try: - capabilities = jsonutils.loads(capabilities) - except (ValueError, TypeError): + capabilities = json.loads(capabilities) + except json.decoder.JSONDecodeError: capabilities = _parse_capabilities_str(capabilities) if not isinstance(capabilities, dict): diff --git a/ironic_python_agent/version.py b/ironic_python_agent/version.py index 3c0bdb35..a7bf3f3f 100644 --- a/ironic_python_agent/version.py +++ b/ironic_python_agent/version.py @@ -13,11 +13,6 @@ # License for the specific language governing permissions and limitations # under the License. -try: - # For Python 3.8 and later - import importlib.metadata as importlib_metadata -except ImportError: - # For everyone else - import importlib_metadata +from importlib import metadata as importlib_metadata __version__ = importlib_metadata.version("ironic_python_agent") diff --git a/releasenotes/notes/collect-udev-f6ada5163cf4a26c.yaml b/releasenotes/notes/collect-udev-f6ada5163cf4a26c.yaml new file mode 100644 index 00000000..24437c3b --- /dev/null +++ b/releasenotes/notes/collect-udev-f6ada5163cf4a26c.yaml @@ -0,0 +1,5 @@ +--- +other: + - | + Block devices properties reported by udev are now collected with the + ramdisk logs. diff --git a/releasenotes/notes/multipath-serial-615fc925984abbf7.yaml b/releasenotes/notes/multipath-serial-615fc925984abbf7.yaml new file mode 100644 index 00000000..4a13352b --- /dev/null +++ b/releasenotes/notes/multipath-serial-615fc925984abbf7.yaml @@ -0,0 +1,4 @@ +--- +fixes: + - | + Fixes discovering WWN/serial numbers for devicemapper devices. diff --git a/requirements.txt b/requirements.txt index d84bb89d..18eea691 100644 --- a/requirements.txt +++ b/requirements.txt @@ -2,13 +2,11 @@ # of appearance. Changing the order has an impact on the overall integration # process, which may cause wedges in the gate later. pbr!=2.1.0,>=2.0.0 # Apache-2.0 -importlib_metadata>=1.7.0;python_version<'3.8' # Apache-2.0 eventlet!=0.18.3,!=0.20.1,>=0.18.2 # MIT netifaces>=0.10.4 # MIT oslo.config>=5.2.0 # Apache-2.0 oslo.concurrency>=3.26.0 # Apache-2.0 oslo.log>=4.6.1 # Apache-2.0 -oslo.serialization!=2.19.1,>=2.18.0 # Apache-2.0 oslo.service!=1.28.1,>=1.24.0 # Apache-2.0 oslo.utils>=3.34.0 # Apache-2.0 Pint>=0.5 # BSD @@ -7,7 +7,7 @@ author_email = openstack-discuss@lists.openstack.org home_page = https://docs.openstack.org/ironic-python-agent/ summary = Ironic Python Agent Ramdisk license = Apache-2 -python_requires = >=3.6 +python_requires = >=3.8 classifier = Environment :: OpenStack Intended Audience :: System Administrators @@ -18,8 +18,6 @@ classifier = Programming Language :: Python :: Implementation :: CPython Programming Language :: Python :: 3 :: Only Programming Language :: Python :: 3 - Programming Language :: Python :: 3.6 - Programming Language :: Python :: 3.7 Programming Language :: Python :: 3.8 Programming Language :: Python :: 3.9 diff --git a/zuul.d/project.yaml b/zuul.d/project.yaml index 429641c6..f3769f42 100644 --- a/zuul.d/project.yaml +++ b/zuul.d/project.yaml @@ -31,6 +31,7 @@ queue: ironic jobs: - openstack-tox-functional + - ipa-tox-examples - ipa-tempest-bios-ipmi-direct-src - ipa-tempest-uefi-redfish-vmedia-src - metalsmith-integration-ipa-src-uefi |