From 7421bf7114bad3762cb1441c5017eefb8be568c8 Mon Sep 17 00:00:00 2001 From: Dan Bungert Date: Mon, 3 Apr 2023 17:30:35 -0600 Subject: [PATCH 1/2] storage/v2: mark can_be_boot_device on disks (cherry picked from commit 3a0e9ed425047b326b1659de409a26c2047df07e) --- subiquity/common/filesystem/labels.py | 1 + subiquity/common/types.py | 1 + .../server/controllers/tests/test_filesystem.py | 16 ++++++++++++++++ 3 files changed, 18 insertions(+) diff --git a/subiquity/common/filesystem/labels.py b/subiquity/common/filesystem/labels.py index f4c0e7e6f..e3d120e16 100644 --- a/subiquity/common/filesystem/labels.py +++ b/subiquity/common/filesystem/labels.py @@ -306,6 +306,7 @@ def _for_client_disk(disk, *, min_size=0): usage_labels=usage_labels(disk), partitions=[for_client(p) for p in gaps.parts_and_gaps(disk)], boot_device=boot.is_boot_device(disk), + can_be_boot_device=boot.can_be_boot_device(disk), ok_for_guided=disk.size >= min_size, model=getattr(disk, 'model', None), vendor=getattr(disk, 'vendor', None)) diff --git a/subiquity/common/types.py b/subiquity/common/types.py index 90c12d81c..f5b872c57 100644 --- a/subiquity/common/types.py +++ b/subiquity/common/types.py @@ -311,6 +311,7 @@ class Disk: preserve: bool path: Optional[str] boot_device: bool + can_be_boot_device: bool model: Optional[str] = None vendor: Optional[str] = None diff --git a/subiquity/server/controllers/tests/test_filesystem.py b/subiquity/server/controllers/tests/test_filesystem.py index b9d311d87..6a5b15217 100644 --- a/subiquity/server/controllers/tests/test_filesystem.py +++ b/subiquity/server/controllers/tests/test_filesystem.py @@ -510,7 +510,12 @@ def _setup(self, bootloader, ptable, **kw): self.app = make_app() self.app.opts.bootloader = bootloader.value self.fsc = FilesystemController(app=self.app) + self.fsc.calculate_suggested_install_min = mock.Mock() + self.fsc.calculate_suggested_install_min.return_value = 10 << 30 self.fsc.model = self.model = make_model(bootloader) + self.model.storage_version = 2 + self.fsc._probe_task.task = mock.Mock() + self.fsc._get_system_task.task = mock.Mock() @parameterized.expand(bootloaders_and_ptables) async def test_get_boot_disks_only(self, bootloader, ptable): @@ -518,6 +523,9 @@ async def test_get_boot_disks_only(self, bootloader, ptable): disk = make_disk(self.model) self.assertEqual([disk.id], await self.fsc.v2_potential_boot_disks_GET()) + resp = await self.fsc.v2_GET() + [d] = resp.disks + self.assertTrue(d.can_be_boot_device) @parameterized.expand(bootloaders_and_ptables) async def test_get_boot_disks_none(self, bootloader, ptable): @@ -531,6 +539,10 @@ async def test_get_boot_disks_all(self, bootloader, ptable): d2 = make_disk(self.model) self.assertEqual(set([d1.id, d2.id]), set(await self.fsc.v2_potential_boot_disks_GET())) + resp = await self.fsc.v2_GET() + [d1, d2] = resp.disks + self.assertTrue(d1.can_be_boot_device) + self.assertTrue(d2.can_be_boot_device) @parameterized.expand(bootloaders_and_ptables) async def test_get_boot_disks_some(self, bootloader, ptable): @@ -548,6 +560,10 @@ async def test_get_boot_disks_some(self, bootloader, ptable): self.assertEqual(expected, set(await self.fsc.v2_potential_boot_disks_GET())) + resp = await self.fsc.v2_GET() + for d in resp.disks: + self.assertEqual(d.id in expected, d.can_be_boot_device) + class TestCoreBootInstallMethods(IsolatedAsyncioTestCase): From ef99122a9c1b107cd8757013b4d001657ead4192 Mon Sep 17 00:00:00 2001 From: Dan Bungert Date: Mon, 3 Apr 2023 17:31:03 -0600 Subject: [PATCH 2/2] storage/v2: remove potential_boot_disks (cherry picked from commit 965a49f28fbc4039f64b9ebb2b206a268989cd07) --- subiquity/common/apidef.py | 8 +------ subiquity/server/controllers/filesystem.py | 5 ---- .../controllers/tests/test_filesystem.py | 24 +++++-------------- 3 files changed, 7 insertions(+), 30 deletions(-) diff --git a/subiquity/common/apidef.py b/subiquity/common/apidef.py index a79a70952..614cdb4a3 100644 --- a/subiquity/common/apidef.py +++ b/subiquity/common/apidef.py @@ -306,16 +306,10 @@ class reformat_disk: def POST(data: Payload[ReformatDisk]) \ -> StorageResponseV2: ... - class potential_boot_disks: - """Obtain the list of disks which can be made bootable. - This list can be empty if there are no disks or if none of them - are plausible sources of a boot disk.""" - def GET() -> List[str]: ... - class add_boot_partition: """Mark a given disk as bootable, which may cause a partition to be added to the disk. It is an error to call this for a - disk not in the list from potential_boot_disks.""" + disk for which can_be_boot_device is False.""" def POST(disk_id: str) -> StorageResponseV2: ... class add_partition: diff --git a/subiquity/server/controllers/filesystem.py b/subiquity/server/controllers/filesystem.py index 1c98aced8..0d6eebb1b 100644 --- a/subiquity/server/controllers/filesystem.py +++ b/subiquity/server/controllers/filesystem.py @@ -710,11 +710,6 @@ async def v2_reformat_disk_POST(self, data: ReformatDisk) \ self.reformat(self.model._one(id=data.disk_id), data.ptable) return await self.v2_GET() - async def v2_potential_boot_disks_GET(self) -> List[str]: - disks = self.potential_boot_disks(check_boot=True, - with_reformatting=False) - return [disk.id for disk in disks] - async def v2_add_boot_partition_POST(self, disk_id: str) \ -> StorageResponseV2: disk = self.model._one(id=disk_id) diff --git a/subiquity/server/controllers/tests/test_filesystem.py b/subiquity/server/controllers/tests/test_filesystem.py index 6a5b15217..f0dc6796a 100644 --- a/subiquity/server/controllers/tests/test_filesystem.py +++ b/subiquity/server/controllers/tests/test_filesystem.py @@ -520,25 +520,16 @@ def _setup(self, bootloader, ptable, **kw): @parameterized.expand(bootloaders_and_ptables) async def test_get_boot_disks_only(self, bootloader, ptable): self._setup(bootloader, ptable) - disk = make_disk(self.model) - self.assertEqual([disk.id], - await self.fsc.v2_potential_boot_disks_GET()) + make_disk(self.model) resp = await self.fsc.v2_GET() [d] = resp.disks self.assertTrue(d.can_be_boot_device) - @parameterized.expand(bootloaders_and_ptables) - async def test_get_boot_disks_none(self, bootloader, ptable): - self._setup(bootloader, ptable) - self.assertEqual([], await self.fsc.v2_potential_boot_disks_GET()) - @parameterized.expand(bootloaders_and_ptables) async def test_get_boot_disks_all(self, bootloader, ptable): self._setup(bootloader, ptable) - d1 = make_disk(self.model) - d2 = make_disk(self.model) - self.assertEqual(set([d1.id, d2.id]), - set(await self.fsc.v2_potential_boot_disks_GET())) + make_disk(self.model) + make_disk(self.model) resp = await self.fsc.v2_GET() [d1, d2] = resp.disks self.assertTrue(d1.can_be_boot_device) @@ -554,15 +545,12 @@ async def test_get_boot_disks_some(self, bootloader, ptable): preserve=True) if bootloader == Bootloader.NONE: # NONE will always pass the boot check, even on a full disk - expected = set([d1.id, d2.id]) + bootable = set([d1.id, d2.id]) else: - expected = set([d2.id]) - self.assertEqual(expected, - set(await self.fsc.v2_potential_boot_disks_GET())) - + bootable = set([d2.id]) resp = await self.fsc.v2_GET() for d in resp.disks: - self.assertEqual(d.id in expected, d.can_be_boot_device) + self.assertEqual(d.id in bootable, d.can_be_boot_device) class TestCoreBootInstallMethods(IsolatedAsyncioTestCase):