diff --git a/ironic_python_agent/device_hints.py b/ironic_python_agent/device_hints.py index 5d6160409..47d70842c 100644 --- a/ironic_python_agent/device_hints.py +++ b/ironic_python_agent/device_hints.py @@ -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). diff --git a/ironic_python_agent/hardware.py b/ironic_python_agent/hardware.py index 36e7178f4..01cc43c8e 100644 --- a/ironic_python_agent/hardware.py +++ b/ironic_python_agent/hardware.py @@ -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) diff --git a/ironic_python_agent/tests/unit/test_device_hints.py b/ironic_python_agent/tests/unit/test_device_hints.py index 50c67e0b7..7595182e8 100644 --- a/ironic_python_agent/tests/unit/test_device_hints.py +++ b/ironic_python_agent/tests/unit/test_device_hints.py @@ -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) diff --git a/releasenotes/notes/fix-matching-hints-lists-b62254ff8eb4a2c1.yaml b/releasenotes/notes/fix-matching-hints-lists-b62254ff8eb4a2c1.yaml new file mode 100644 index 000000000..b5c3f8263 --- /dev/null +++ b/releasenotes/notes/fix-matching-hints-lists-b62254ff8eb4a2c1.yaml @@ -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.