Merge pull request #2146 from dbungert/noble-sort-key

noble - filesystem: handle sorting disks without required metadata

subiquity/models/filesystem.py

@@ -646,10 +646,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
 
@@ -776,8 +777,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):
@@ -921,8 +922,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:
@@ -1011,7 +1012,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):
@@ -1110,7 +1111,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
@@ -1277,8 +1278,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

subiquity/models/tests/test_filesystem.py

@@ -1818,6 +1818,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)