Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

filesystem: handle sorting disks without required metadata #2143

Merged
merged 1 commit into from
Jan 24, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 13 additions & 12 deletions subiquity/models/filesystem.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
10 changes: 10 additions & 0 deletions subiquity/models/tests/test_filesystem.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Loading