From fbf6de2af9cc56c9ddf7e6e5b68488d5e2a49cb3 Mon Sep 17 00:00:00 2001 From: Robert Malz Date: Tue, 21 Jan 2025 15:57:54 +0100 Subject: [PATCH] filesystem: handle sorting disks without required metadata If there are at least two disks and one of them is missing any metadata used during sorting installer will crash. Issue can be reproduced using already available machine .json file: examples/machines/existing-partitions.json in which /dev/vda does not have serial value. This patch makes sure that all values used in sorting are initialized 2025-01-21 15:41:33,340 ERROR root:38 finish: subiquity/apply_autoinstall_config: FAIL: '<' not supported between instances of 'str' and 'NoneType' 2025-01-21 15:41:33,340 ERROR subiquity.server.server:503 top level error Traceback (most recent call last): File "/home/rmalz/repos/subiquity/subiquity/server/server.py", line 1021, in start await self.apply_autoinstall_config() File "/home/rmalz/repos/subiquity/subiquitycore/context.py", line 165, in decorated_async return await meth(self, **kw) File "/home/rmalz/repos/subiquity/subiquity/server/server.py", line 575, in apply_autoinstall_config await controller.apply_autoinstall_config() File "/home/rmalz/repos/subiquity/subiquitycore/context.py", line 165, in decorated_async return await meth(self, **kw) File "/home/rmalz/repos/subiquity/subiquity/server/controllers/filesystem.py", line 484, in apply_autoinstall_config await self.convert_autoinstall_config(context=context) File "/home/rmalz/repos/subiquity/subiquitycore/context.py", line 165, in decorated_async return await meth(self, **kw) File "/home/rmalz/repos/subiquity/subiquity/server/controllers/filesystem.py", line 1745, in convert_autoinstall_config await self.run_autoinstall_guided(self.ai_data["layout"]) File "/home/rmalz/repos/subiquity/subiquity/server/controllers/filesystem.py", line 1684, in run_autoinstall_guided disk = self.get_bootable_matching_disk(match) File "/home/rmalz/repos/subiquity/subiquity/server/controllers/filesystem.py", line 1594, in get_bootable_matching_disk matching_disks = self.get_bootable_matching_disks(match) File "/home/rmalz/repos/subiquity/subiquity/server/controllers/filesystem.py", line 1582, in get_bootable_matching_disks matching_disks = self.model.disks_for_match(disks, match) File "/home/rmalz/repos/subiquity/subiquity/models/filesystem.py", line 1800, in disks_for_match candidates, _ = self._matching_disks(disks, match) File "/home/rmalz/repos/subiquity/subiquity/models/filesystem.py", line 1791, in _matching_disks candidates = self._sorted_matches(candidates, m) File "/home/rmalz/repos/subiquity/subiquity/models/filesystem.py", line 1769, in _sorted_matches disks.sort(key=lambda d: d.sort_key) TypeError: '<' not supported between instances of 'str' and 'NoneType' Fixes: https://github.com/canonical/subiquity/pull/2050 --- subiquity/models/filesystem.py | 25 ++++++++++++----------- subiquity/models/tests/test_filesystem.py | 10 +++++++++ 2 files changed, 23 insertions(+), 12 deletions(-) diff --git a/subiquity/models/filesystem.py b/subiquity/models/filesystem.py index 9d6b9c116..8a873cc08 100644 --- a/subiquity/models/filesystem.py +++ b/subiquity/models/filesystem.py @@ -653,10 +653,11 @@ def size(self): @property @abstractmethod def sort_key(self) -> Tuple: - """return a value that is usable for sorting. The intent here is - that two runs of a specific version of subiquity must produce an - identical sort result for _Devices, so that while Subiquity might make - an arbitrary choice of disk, it's a consistent arbitrary choice. + """return a value that is usable for sorting and does not contain + any None values. The intent here is that two runs of a specific + version of subiquity must produce an identical sort result for + _Devices, so that while Subiquity might make an arbitrary choice of + disk, it's a consistent arbitrary choice. """ pass @@ -799,8 +800,8 @@ class Disk(_Device): _has_in_use_partition: bool = attributes.for_api(default=False) @property - def sort_key(self) -> int: - return (self.wwn, self.serial, self.path) + def sort_key(self) -> Tuple: + return (self.wwn or "", self.serial or "", self.path or "") @property def available_for_partitions(self): @@ -944,8 +945,8 @@ def __post_init__(self): raise Exception("Exceeded number of available partitions") @property - def sort_key(self) -> int: - return (self.device.sort_key(), self.number) + def sort_key(self) -> Tuple: + return (self.device.sort_key, self.number or 0) def available(self): if self.flag in ["bios_grub", "prep"] or self.grub_device: @@ -1039,7 +1040,7 @@ class Raid(_Device): _has_in_use_partition = False @property - def sort_key(self) -> int: + def sort_key(self) -> Tuple: return tuple([dev.sort_key for dev in raid_device_sort(self.devices)]) def serialize_devices(self): @@ -1138,7 +1139,7 @@ def size(self): return get_lvm_size(self.devices) @property - def sort_key(self) -> int: + def sort_key(self) -> Tuple: return tuple([dev.sort_key for dev in self.devices]) @property @@ -1305,8 +1306,8 @@ def size(self): return 0 @property - def sort_key(self) -> int: - return (self.path,) + def sort_key(self) -> Tuple: + return (self.path or "",) ok_for_raid = False ok_for_lvm_vg = False diff --git a/subiquity/models/tests/test_filesystem.py b/subiquity/models/tests/test_filesystem.py index a69a5bde3..f070e5309 100644 --- a/subiquity/models/tests/test_filesystem.py +++ b/subiquity/models/tests/test_filesystem.py @@ -2075,6 +2075,16 @@ def test_sort_path_wins(self, sort_criteria: str): self.assertEqual(d1, m.disk_for_match([d2, d1], {"size": sort_criteria})) self.assertEqual([d1, d2], m.disks_for_match([d2, d1], {"size": sort_criteria})) + def test_sort_disks_none_values(self): + m = make_model() + d1 = make_disk(m, wwn=None, serial="s", path="/dev/d1") + d2 = make_disk(m, wwn="w", serial=None, path="/dev/d2") + d3 = make_disk(m, wwn="w", serial="s", path=None) + self.assertEqual(d1, m.disk_for_match([d3, d2, d1], {"size": "largest"})) + self.assertEqual( + [d1, d2, d3], m.disks_for_match([d3, d2, d1], {"size": "largest"}) + ) + def test_sort_raid(self): m = make_model() d1_1 = make_disk(m, size=100 << 30)