filesystem: handle sorting disks without required metadata

[rmalz-c]

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: #2050

LP:#2095295

[rmalz-c]

There is no LP bug for this issue, let me know if I should create one.

[yolkispalkis]

seems related
https://bugs.launchpad.net/subiquity/+bug/2095295

[dbungert]

@yolkispalkis - yes, I believe that LP:#2095295 is the same root cause issue, thanks for pointing that out. It specifically looks to be failing on wwn instead of serial but I'm pretty certain it's the same problem.

@rmalz-c - thanks very much for this, I think you're on the right track.

So while we could fix the users of sort_key, I don't like that as much as it becomes a hidden requirement on theoretical future users of sort_key, should they emerge. Instead I want to see the existing providers of sort_key return a valid answer. Also, note that Raid and LVM_VolGroup objects provide a nested answer, which I think will still fail to sort but not be adequately fixed up by the current PR since they have a second level of tuple going on before the actual values, so sort_key itself must provide a valid answer.

I think the providers of sort_key that interact with the values should be handling this, so Disk, Partition, and ArbitraryDevice, need to None check and substitute the empty string or zero value as matches the type of the field.

Also, I would appreciate unit tests here so that we don't regress that in the future, and an addition to the docstring in _Device.sort_key acknowledging this requirement that the tuple values must never be None.

[rmalz-c]

pretty sure self.number cannot be None but left it either way

[ogayot]

In theory it shouldn't .... but in practice it has at least happened recently

https://bugs.launchpad.net/subiquity/+bug/2095159

[rmalz-c]

@dbungert thanks for the review, I have moved a logic as proposed and included a unit test for it.
Also updated, commit message to properly reflect on the changes. As commit message was changed I have also changed PR description.

[ogayot]

In theory it shouldn't .... but in practice it has at least happened recently

https://bugs.launchpad.net/subiquity/+bug/2095159

[dbungert]

Almost ready, with a fix for Partition.sort_key we can merge this.

[ogayot]

Thank you, LGTM!

for future PRs, I'd prefer to see separate patches for the different fixes. Typically one for the type hint fixes, one for the None keys and one for the sort_keys() -> sort_keys.

[Chris-Peterson444]

Thanks!