diff options
| -rw-r--r-- | ironic_python_agent/hardware.py | 25 | ||||
| -rw-r--r-- | ironic_python_agent/tests/unit/test_hardware.py | 21 | ||||
| -rw-r--r-- | ironic_python_agent/tests/unit/test_utils.py | 45 | ||||
| -rw-r--r-- | ironic_python_agent/utils.py | 42 | ||||
| -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 | 1 | ||||
| -rw-r--r-- | setup.cfg | 4 | ||||
| -rw-r--r-- | zuul.d/project.yaml | 1 |
10 files changed, 124 insertions, 31 deletions
diff --git a/ironic_python_agent/hardware.py b/ironic_python_agent/hardware.py index a71df126..d32b4b28 100644 --- a/ironic_python_agent/hardware.py +++ b/ironic_python_agent/hardware.py @@ -646,26 +646,34 @@ def list_all_block_devices(block_type='disk', 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 @@ -1800,10 +1808,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/tests/unit/test_hardware.py b/ironic_python_agent/tests/unit/test_hardware.py index 4f45a4a2..c4b1b6fb 100644 --- a/ironic_python_agent/tests/unit/test_hardware.py +++ b/ironic_python_agent/tests/unit/test_hardware.py @@ -1598,13 +1598,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 = [ @@ -1638,19 +1641,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', @@ -1659,7 +1662,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() diff --git a/ironic_python_agent/tests/unit/test_utils.py b/ironic_python_agent/tests/unit/test_utils.py index b975923c..9382a6b4 100644 --- a/ironic_python_agent/tests/unit/test_utils.py +++ b/ironic_python_agent/tests/unit/test_utils.py @@ -17,6 +17,7 @@ import base64 import errno import glob import io +import json import os import shutil import subprocess @@ -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 e6454023..09d726cf 100644 --- a/ironic_python_agent/utils.py +++ b/ironic_python_agent/utils.py @@ -33,6 +33,7 @@ from oslo_concurrency import processutils from oslo_config import cfg from oslo_log import log as logging from oslo_utils import units +import pyudev import requests import tenacity @@ -538,6 +539,42 @@ def gzip_and_b64encode(io_dict=None, file_list=None): 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): """Collect system logs. @@ -576,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) 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 314486ad..18eea691 100644 --- a/requirements.txt +++ b/requirements.txt @@ -2,7 +2,6 @@ # 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 @@ -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 |