filesystem: arbitrary disk select more consistent

If two disks of the same size went to the disk matchers with the default
"largest" match, the one we would choose would be based on whatever
order in python they ended up, and across installs this means that we
could select arbitrarily among disks of a matching size.

Add sort criteria based on various unique identifiers.  Make it work for
other disk-like devices, though the main target is disks and raids
because that's what can go to the matcher system.

subiquity/models/filesystem.py

@@ -617,6 +617,16 @@ class _Device(_Formattable, ABC):
     def size(self):
         pass
 
+    @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.
+        """
+        pass
+
     # [Partition]
     _partitions: List["Partition"] = attributes.backlink(default=attr.Factory(list))
 
@@ -739,6 +749,10 @@ class Disk(_Device):
     _info: StorageInfo = attributes.for_api()
     _has_in_use_partition: bool = attributes.for_api(default=False)
 
+    @property
+    def sort_key(self) -> int:
+        return (self.wwn, self.serial, self.path)
+
     @property
     def available_for_partitions(self):
         margin_before = self.alignment_data().min_start_offset
@@ -880,6 +894,10 @@ def __post_init__(self):
                 return
         raise Exception("Exceeded number of available partitions")
 
+    @property
+    def sort_key(self) -> int:
+        return (self.device.sort_key(), self.number)
+
     def available(self):
         if self.flag in ["bios_grub", "prep"] or self.grub_device:
             return False
@@ -966,6 +984,10 @@ class Raid(_Device):
     _info: Optional[StorageInfo] = attributes.for_api(default=None)
     _has_in_use_partition = False
 
+    @property
+    def sort_key(self) -> int:
+        return tuple([dev.sort_key for dev in raid_device_sort(self.devices)])
+
     def serialize_devices(self):
         # Surprisingly, the order of devices passed to mdadm --create
         # matters (see get_raid_size) so we sort devices here the same
@@ -1061,6 +1083,10 @@ def size(self):
         # Should probably query actual size somehow for an existing VG!
         return get_lvm_size(self.devices)
 
+    @property
+    def sort_key(self) -> int:
+        return tuple([dev.sort_key for dev in self.devices])
+
     @property
     def available_for_partitions(self):
         return self.size
@@ -1224,6 +1250,10 @@ class ArbitraryDevice(_Device):
     def size(self):
         return 0
 
+    @property
+    def sort_key(self) -> int:
+        return (self.path,)
+
     ok_for_raid = False
     ok_for_lvm_vg = False
 
@@ -1680,6 +1710,13 @@ def match_nonzero_size(disk):
         return matchers
 
     def _sorted_matches(self, disks: Sequence[_Device], match: dict):
+        # sort first on the sort_key.  Objective here is that if we are falling
+        # back to arbitrary disk selection, we're at least consistent in what
+        # disk we arbitrarily select across runs
+        disks.sort(key=lambda d: d.sort_key)
+
+        # then sort on size, if requested.  Thanks to stable sort, if disks
+        # (or raids) have the same size, the sort_key will tiebreak.
         if match.get("size") == "smallest":
             disks.sort(key=lambda d: d.size)
         elif match.get("size") == "largest":

subiquity/models/tests/test_filesystem.py

@@ -1862,17 +1862,23 @@ def test_lvm_logical_volume(self):
 
 
 class TestDiskForMatch(SubiTestCase):
+    match_sort_criteria = (["smallest"], ["largest"])
+
     def test_empty_match_directive(self):
         m = make_model()
         d1 = make_disk(m)
         d2 = make_disk(m)
+
+        # this test relies heavily on the assumptions in make_disk
         actual = m.disk_for_match([d1, d2], {})
         self.assertEqual(d1, actual)
+        actual = m.disk_for_match([d2, d1], {})
+        self.assertEqual(d1, actual)
 
     def test_sort_largest(self):
         m = make_model()
-        d100 = make_disk(m, size=100 << 30)
-        d200 = make_disk(m, size=200 << 30)
+        d100 = make_disk(m, size=100 << 30, serial="s1", path="/dev/d1")
+        d200 = make_disk(m, size=200 << 30, serial="s2", path="/dev/d2")
         actual = m.disk_for_match([d100, d200], {"size": "largest"})
         self.assertEqual(d200, actual)
 
@@ -1883,6 +1889,79 @@ def test_sort_smallest(self):
         actual = m.disk_for_match([d200, d100], {"size": "smallest"})
         self.assertEqual(d100, actual)
 
+    @parameterized.expand(match_sort_criteria)
+    def test_sort_serial(self, sort_criteria: str):
+        m = make_model()
+        d1 = make_disk(m, serial="s1", path=None, wwn=None)
+        d2 = make_disk(m, serial="s2", path=None, wwn=None)
+        # while the size sort is reversed when doing largest,
+        # we pre-sort on the other criteria, and stable sort helps out
+        actual = m.disk_for_match([d2, d1], {"size": sort_criteria})
+        self.assertEqual(d1, actual)
+
+    @parameterized.expand(match_sort_criteria)
+    def test_sort_path(self, sort_criteria: str):
+        m = make_model()
+        d1 = make_disk(m, serial=None, path="/dev/d1", wwn=None)
+        d2 = make_disk(m, serial=None, path="/dev/d2", wwn=None)
+        actual = m.disk_for_match([d2, d1], {"size": sort_criteria})
+        self.assertEqual(d1, actual)
+
+    @parameterized.expand(match_sort_criteria)
+    def test_sort_wwn(self, sort_criteria: str):
+        m = make_model()
+        d1 = make_disk(m, serial=None, path=None, wwn="w1")
+        d2 = make_disk(m, serial=None, path=None, wwn="w2")
+        actual = m.disk_for_match([d2, d1], {"size": sort_criteria})
+        self.assertEqual(d1, actual)
+
+    @parameterized.expand(match_sort_criteria)
+    def test_sort_wwn_wins(self, sort_criteria: str):
+        m = make_model()
+        d1 = make_disk(m, serial="s2", path="/dev/d2", wwn="w1")
+        d2 = make_disk(m, serial="s1", path="/dev/d1", wwn="w2")
+        actual = m.disk_for_match([d2, d1], {"size": sort_criteria})
+        self.assertEqual(d1, actual)
+
+    @parameterized.expand(match_sort_criteria)
+    def test_sort_serial_wins(self, sort_criteria: str):
+        m = make_model()
+        d1 = make_disk(m, serial="s1", path="/dev/d2", wwn="w")
+        d2 = make_disk(m, serial="s2", path="/dev/d1", wwn="w")
+        actual = m.disk_for_match([d2, d1], {"size": sort_criteria})
+        self.assertEqual(d1, actual)
+
+    @parameterized.expand(match_sort_criteria)
+    def test_sort_path_wins(self, sort_criteria: str):
+        m = make_model()
+        d1 = make_disk(m, serial="s", path="/dev/d1", wwn="w")
+        d2 = make_disk(m, serial="s", path="/dev/d2", wwn="w")
+        actual = m.disk_for_match([d2, d1], {"size": sort_criteria})
+        self.assertEqual(d1, actual)
+
+    def test_sort_raid(self):
+        m = make_model()
+        d1_1 = make_disk(m, size=100 << 30)
+        d1_2 = make_disk(m, size=100 << 30)
+        d2_1 = make_disk(m, size=200 << 30)
+        d2_2 = make_disk(m, size=200 << 30)
+        r1 = make_raid(m, disks={d1_1, d1_2})
+        r2 = make_raid(m, disks={d2_1, d2_2})
+        actual = m.disk_for_match([r1, r2], {"size": "largest"})
+        self.assertEqual(r2, actual)
+
+    @parameterized.expand(match_sort_criteria)
+    def test_sort_raid_on_disks(self, sort_criteria: str):
+        m = make_model()
+        d1_1 = make_disk(m, serial=None, path=None, wwn="w1_1")
+        d1_2 = make_disk(m, serial=None, path=None, wwn="w1_2")
+        d2_1 = make_disk(m, serial=None, path=None, wwn="w2_1")
+        d2_2 = make_disk(m, serial=None, path=None, wwn="w2_2")
+        r1 = make_raid(m, disks={d1_1, d1_2})
+        r2 = make_raid(m, disks={d2_1, d2_2})
+        actual = m.disk_for_match([r1, r2], {"size": sort_criteria})
+        self.assertEqual(r1, actual)
+
     def test_skip_empty(self):
         m = make_model()
         d0 = make_disk(m, size=0)