diff --git a/subiquity/models/filesystem.py b/subiquity/models/filesystem.py index 35cbb00b..746bf7ee 100644 --- a/subiquity/models/filesystem.py +++ b/subiquity/models/filesystem.py @@ -689,10 +689,16 @@ class Partition(_Formattable): @property def _number(self): - if self.preserve: + if self.number is not None: return self.number - else: - return self.device._partitions.index(self) + 1 + used_nums = {part.number for part in self.device._partitions + if part.number is not None} + possible_nums = {i for i in range(1, len(self.device._partitions) + 1)} + unused_nums = sorted(list(possible_nums - used_nums)) + for part in self.device._partitions: + if part.number is None: + part.number = unused_nums.pop(0) + return self.number def _path(self): return partition_kname(self.device.path, self._number) diff --git a/subiquity/models/tests/test_filesystem.py b/subiquity/models/tests/test_filesystem.py index 1d29576e..50bf248b 100644 --- a/subiquity/models/tests/test_filesystem.py +++ b/subiquity/models/tests/test_filesystem.py @@ -177,7 +177,11 @@ def make_partition(model, device=None, *, preserve=False, size=None, partition = Partition(m=model, device=device, size=size, offset=offset, preserve=preserve, **kw) if preserve: - partition.number = len(device._partitions) + number = kw.get('number') + if number is not None: + partition.number = number + else: + partition.number = len(device._partitions) model._actions.append(partition) return partition @@ -692,3 +696,56 @@ class TestAlignmentData(unittest.TestCase): # information, so gaps produces numbers that are too small by 1MiB # for ptable != 'gpt' self.assertTrue(gaps_max <= align_max, f'ptable={ptable}') + + +class TestPartitionNumbering(unittest.TestCase): + def test_basic(self): + m, d1 = make_model_and_disk(ptable='gpt') + p1 = make_partition(m, d1) + p2 = make_partition(m, d1) + p3 = make_partition(m, d1) + self.assertEqual(1, p1._number) + self.assertEqual(2, p2._number) + self.assertEqual(3, p3._number) + + def test_p1_preserved(self): + m = make_model() + m.storage_version = 2 + d1 = make_disk(m, ptable='gpt') + p1 = make_partition(m, d1, preserve=True, number=1) + p2 = make_partition(m, d1) + p3 = make_partition(m, d1) + self.assertEqual(1, p1._number) + self.assertEqual(True, p1.preserve) + self.assertEqual(2, p2._number) + self.assertEqual(False, p2.preserve) + self.assertEqual(3, p3._number) + self.assertEqual(False, p3.preserve) + + def test_p2_preserved(self): + m = make_model() + m.storage_version = 2 + d1 = make_disk(m, ptable='gpt') + p2 = make_partition(m, d1, preserve=True, number=2) + p1 = make_partition(m, d1) + p3 = make_partition(m, d1) + self.assertEqual(1, p1._number) + self.assertEqual(False, p1.preserve) + self.assertEqual(2, p2._number) + self.assertEqual(True, p2.preserve) + self.assertEqual(3, p3._number) + self.assertEqual(False, p3.preserve) + + def test_trigger_part_num_allocation_for_disk(self): + m = make_model() + d1 = make_disk(m, ptable='gpt') + p1 = make_partition(m, d1) + p2 = make_partition(m, d1) + p3 = make_partition(m, d1) + self.assertIsNone(p1.number) + self.assertIsNone(p2.number) + self.assertIsNone(p3.number) + self.assertEqual(3, p3._number) + self.assertEqual(1, p1.number) + self.assertEqual(2, p2.number) + self.assertEqual(3, p3.number) diff --git a/subiquity/tests/api/test_api.py b/subiquity/tests/api/test_api.py index 911d6756..2ce8aa95 100755 --- a/subiquity/tests/api/test_api.py +++ b/subiquity/tests/api/test_api.py @@ -299,36 +299,34 @@ class TestFlow(TestAPI): } } add_resp = await inst.post('/storage/v2/add_partition', data) - sda = first(add_resp['disks'], 'id', disk_id) - sda2 = first(sda['partitions'], 'number', 2) - self.assertEqual('ext3', sda2['format']) + [sda] = add_resp['disks'] + [root] = match(sda['partitions'], mount='/') + self.assertEqual('ext3', root['format']) data = { 'disk_id': disk_id, 'partition': { - 'number': 2, + 'number': root['number'], 'format': 'ext4', } } edit_resp = await inst.post('/storage/v2/edit_partition', data) - add_sda = first(add_resp['disks'], 'id', disk_id) - add_sda2 = first(add_sda['partitions'], 'number', 2) + [add_sda] = add_resp['disks'] + [add_root] = match(add_sda['partitions'], mount='/') - edit_sda = first(edit_resp['disks'], 'id', disk_id) - edit_sda2 = first(edit_sda['partitions'], 'number', 2) + [edit_sda] = edit_resp['disks'] + [edit_root] = match(edit_sda['partitions'], mount='/') for key in 'size', 'number', 'mount', 'boot': - self.assertEqual(add_sda2[key], edit_sda2[key], key) - self.assertEqual('ext4', edit_sda2['format']) + self.assertEqual(add_root[key], edit_root[key], key) + self.assertEqual('ext4', edit_root['format']) del_resp = await inst.post('/storage/v2/delete_partition', data) - sda = first(del_resp['disks'], 'id', disk_id) - self.assertEqual(2, len(sda['partitions'])) - - for type in 'Partition', 'Gap': - pgs = [pg for pg in sda['partitions'] if pg['$type'] == type] - self.assertEqual(len(pgs), 1) + [sda] = del_resp['disks'] + [p, g] = sda['partitions'] + self.assertEqual('Partition', p['$type']) + self.assertEqual('Gap', g['$type']) reset_resp = await inst.post('/storage/v2/reset') self.assertEqual(orig_resp, reset_resp) @@ -374,8 +372,9 @@ class TestAdd(TestAPI): await inst.post('/storage/v2/reset') - # these manual steps are expected to be equivalent to just adding - # the single partition and getting the automatic boot partition + # these manual steps are expected to be mostly equivalent to just + # adding the single partition and getting the automatic boot + # partition resp = await inst.post( '/storage/v2/add_boot_partition', disk_id=disk_id) sda = first(resp['disks'], 'id', disk_id) @@ -390,6 +389,15 @@ class TestAdd(TestAPI): } manual_add = await inst.post('/storage/v2/add_partition', data) + # the only difference is the partition number assigned - when we + # explicitly add_boot_partition, that is the first partition + # created, versus when we add_partition and get a boot partition + # implicitly + for resp in single_add, manual_add: + for part in resp['disks'][0]['partitions']: + part.pop('number') + part.pop('path') + self.assertEqual(single_add, manual_add) @timeout() @@ -978,10 +986,11 @@ class TestGap(TestAPI): } } resp = await inst.post('/storage/v2/add_partition', data) - sda = first(resp['disks'], 'id', 'disk-sda') - boot = first(sda['partitions'], 'number', 1) - gap = sda['partitions'][2] - expected = (10 << 30) - boot['size'] - (4 << 30) - (2 << 20) + [sda] = resp['disks'] + [boot] = match(sda['partitions'], mount='/boot/efi') + [p1, p2, gap] = sda['partitions'] + self.assertEqual('Gap', gap['$type']) + expected = (10 << 30) - p1['size'] - p2['size'] - (2 << 20) self.assertEqual(expected, gap['size']) async def SKIP_test_two_gaps(self): @@ -1040,10 +1049,10 @@ class TestRegression(TestAPI): } } resp = await inst.post('/storage/v2/add_partition', data) - sda = first(resp['disks'], 'id', disk_id) - sda2 = first(sda['partitions'], 'number', 2) - sda2.update({'format': 'ext3', 'mount': '/bar'}) - data['partition'] = sda2 + [sda] = resp['disks'] + [part] = match(sda['partitions'], mount='/foo') + part.update({'format': 'ext3', 'mount': '/bar'}) + data['partition'] = part data.pop('gap') await inst.post('/storage/v2/edit_partition', data) # should not throw an exception complaining about boot