openstack/ironic-python-agent
Code Issues Proposed changes

Fix for matching hints with lists of strings

Browse Source 
Added logic for matching hints with lists of WWN/Serial. These lists
appear when both lsblk and udev are used to fetch the information about
a device. One consequence of this is that it allows a device on the
skip list to be used as root device, thus overwriting the protected
data. This has previously been handled before matching the hints,
e.g. the removed section in hardware.py. This patch aims to fix the
problem globally by handling the issue inside the find_devices_by_hints
function.

Closes-bug: #2130410
Change-Id: I28129f2ededb37474025f35164d5dc9ece21ec8e
Signed-off-by: Morten Stephansen <morten.kaastrup.stephansen@cern.ch>
Signed-off-by: Jakub Jelinek <jakub.jelinek@cern.ch>
(cherry picked from commit bb4b4fdb38)
This commit is contained in:
Morten Stephansen
2025-10-31 13:28:16 +00:00
parent 1492c70563
commit 6cb0d9915f
4 changed files with 94 additions and 29 deletions
Download Patch File Download Diff File Expand all files Collapse all files

63
ironic_python_agent/device_hints.py
View File

@@ -191,11 +191,12 @@ def find_devices_by_hints(devices, root_device_hints):
:size: (Integer) Size of the device in *bytes*
:model: (String) Device model
:vendor: (String) Device vendor name
:serial: (String) Device serial number
:wwn: (String) Unique storage identifier
:wwn_with_extension: (String): Unique storage identifier with
the vendor extension appended
:wwn_vendor_extension: (String): United vendor storage identifier
:serial: (String or List[String]) Device serial number(s)
:wwn: (String or List[String]) Unique storage identifier(s)
:wwn_with_extension: (String or List[String]): Unique storage
identifier(s) with the vendor extension appended
:wwn_vendor_extension: (String or List[String]): United vendor
storage identifier(s)
:rotational: (Boolean) Whether it's a rotational device or
not. Useful to distinguish HDDs (rotational) and SSDs
(not rotational).
@@ -221,6 +222,47 @@ def find_devices_by_hints(devices, root_device_hints):
device_value = dev.get(hint)
hint_value = parsed_hints[hint]
# Handle device attributes that are a list of strings
# (serial, wwn, wwn_with_extension, and wwn_vendor_extension).
if hint_type is str and isinstance(device_value, list):
# NOTE(mostepha): Device value is a list. Consider it matched
# if any value in the list matches the hint.
device_values = [v for v in device_value if v is not None]
if not device_values:
LOG.warning(
'The attribute "%(attr)s" of the device "%(dev)s" '
'has an empty value. Skipping device.',
{'attr': hint, 'dev': device_name})
break
matched = False
for val in device_values:
try:
normalized_val = _normalize_hint_expression(val, hint)
if specs_matcher.match(normalized_val, hint_value):
LOG.info('The attribute "%(attr)s" of device '
'"%(dev)s" matched hint %(hint)s with '
'value "%(value)s"',
{'attr': hint, 'dev': device_name,
'hint': hint_value, 'value': val})
matched = True
break
except ValueError:
# Skip invalid/empty values in the list
continue
# Continue to next hint if any value matched. Otherwise, break
# and try the next device.
if matched:
continue
else:
LOG.info('None of the values %(values)s for attribute '
'"%(attr)s" of device "%(dev)s" match the hint '
'%(hint)s',
{'values': device_values, 'attr': hint,
'dev': device_name, 'hint': hint_value})
break
if hint_type is str:
try:
device_value = _normalize_hint_expression(device_value,
@@ -286,11 +328,12 @@ def match_root_device_hints(devices, root_device_hints):
:size: (Integer) Size of the device in *bytes*
:model: (String) Device model
:vendor: (String) Device vendor name
:serial: (String) Device serial number
:wwn: (String) Unique storage identifier
:wwn_with_extension: (String): Unique storage identifier with
the vendor extension appended
:wwn_vendor_extension: (String): United vendor storage identifier
:serial: (String or List[String]) Device serial number(s)
:wwn: (String or List[String]) Unique storage identifier(s)
:wwn_with_extension: (String or List[String]): Unique storage
identifier(s) with the vendor extension appended
:wwn_vendor_extension: (String or List[String]): United vendor
storage identifier(s)
:rotational: (Boolean) Whether it's a rotational device or
not. Useful to distinguish HDDs (rotational) and SSDs
(not rotational).

16
ironic_python_agent/hardware.py
View File

@@ -1765,22 +1765,6 @@ class GenericHardwareManager(HardwareManager):
dev_name = utils.guess_root_disk(block_devices).name
else:
serialized_devs = [dev.serialize() for dev in block_devices]
orig_size = len(serialized_devs)
for dev_idx in range(orig_size):
ser_dev = serialized_devs.pop(0)
serials = ser_dev.get('serial')
wwns = ser_dev.get('wwn')
# (rozzi) static serial and static wwn are used to avoid
# reundancy in the number of wwns and serials, if the code
# would just loop over both serials and wwns it could be that
# there would be an uncesarry duplication of the first wwn
# number
for serial in serials:
for wwn in wwns:
tmp_ser_dev = ser_dev.copy()
tmp_ser_dev['wwn'] = wwn
tmp_ser_dev['serial'] = serial
serialized_devs.append(tmp_ser_dev)
try:
device = device_hints.match_root_device_hints(
serialized_devs, root_device_hints)

38
ironic_python_agent/tests/unit/test_device_hints.py
View File

@@ -245,11 +245,13 @@ class MatchRootDeviceTestCase(base.IronicAgentTest):
super(MatchRootDeviceTestCase, self).setUp()
self.devices = [
{'name': '/dev/sda', 'size': 64424509440, 'model': 'ok model',
'serial': 'fakeserial'},
'serial': 'fakeserial', 'wwn': 'wwn_1'},
{'name': '/dev/sdb', 'size': 128849018880, 'model': 'big model',
'serial': 'veryfakeserial', 'rotational': 'yes'},
'serial': ['veryfakeserial', 'alsoveryfakeserial'],
'rotational': 'yes', 'wwn': ['wwn_2', 'wwn_2_ext']},
{'name': '/dev/sdc', 'size': 10737418240, 'model': 'small model',
'serial': 'veryveryfakeserial', 'rotational': False},
'serial': 'veryveryfakeserial', 'rotational': False,
'wwn': 'wwn_3'},
]
def test_match_root_device_hints_one_hint(self):
@@ -292,6 +294,12 @@ class MatchRootDeviceTestCase(base.IronicAgentTest):
self.devices, root_device_hints)
self.assertEqual('/dev/sdc', dev['name'])
def test_match_root_device_hints_list_of_wwns(self):
root_device_hints = {'wwn': 'wwn_2_ext'}
dev = device_hints.match_root_device_hints(
self.devices, root_device_hints)
self.assertEqual('/dev/sdb', dev['name'])
def test_match_root_device_hints_no_operators(self):
root_device_hints = {'size': '120', 'model': 'big model',
'serial': 'veryfakeserial'}
@@ -328,3 +336,27 @@ class MatchRootDeviceTestCase(base.IronicAgentTest):
devs = list(device_hints.find_devices_by_hints(self.devices,
root_device_hints))
self.assertEqual([self.devices[0]], devs)
def test_find_devices_single_serial(self):
root_device_hints = {'serial': 's== fakeserial'}
devs = list(device_hints.find_devices_by_hints(self.devices,
root_device_hints))
self.assertEqual([self.devices[0]], devs)
def test_find_devices_multiple_serials(self):
root_device_hints = {'serial': 's== alsoveryfakeserial'}
devs = list(device_hints.find_devices_by_hints(self.devices,
root_device_hints))
self.assertEqual([self.devices[1]], devs)
def test_find_devices_single_wwn(self):
root_device_hints = {'wwn': 's== wwn_1'}
devs = list(device_hints.find_devices_by_hints(self.devices,
root_device_hints))
self.assertEqual([self.devices[0]], devs)
def test_find_devices_multiple_wwns(self):
root_device_hints = {'wwn': 's== wwn_2'}
devs = list(device_hints.find_devices_by_hints(self.devices,
root_device_hints))
self.assertEqual([self.devices[1]], devs)

6
releasenotes/notes/fix-matching-hints-lists-b62254ff8eb4a2c1.yaml Normal file
View File

@@ -0,0 +1,6 @@
---
fixes:
- |
Fixes matching hints with lists of WWN/Serial which was only handled
in some cases - due to the issue, it was possible to choose a device
listed in the skip_block_devices property as a root device.