diff --git a/subiquity/client/controllers/filesystem.py b/subiquity/client/controllers/filesystem.py index 0ce2f620..0072cf6a 100644 --- a/subiquity/client/controllers/filesystem.py +++ b/subiquity/client/controllers/filesystem.py @@ -29,6 +29,7 @@ from subiquity.common.types import ( GuidedCapability, GuidedChoiceV2, GuidedStorageResponseV2, + GuidedStorageTargetManual, GuidedStorageTargetReformat, StorageResponseV2, ) @@ -177,7 +178,10 @@ class FilesystemController(SubiquityTuiController, FilesystemManipulator): await asyncio.sleep(0.1) self.finish() elif self.answers['manual']: - await self._guided_choice(None) + await self._guided_choice( + GuidedChoiceV2( + target=GuidedStorageTargetManual(), + capability=GuidedCapability.MANUAL)) await self._run_actions(self.answers['manual']) self.answers['manual'] = [] @@ -287,21 +291,11 @@ class FilesystemController(SubiquityTuiController, FilesystemManipulator): else: raise Exception("could not process action {}".format(action)) - async def _guided_choice(self, choice: Optional[GuidedChoiceV2]): - if self.core_boot_capability is not None: - self.app.next_screen(self.endpoint.guided.POST(choice)) + async def _guided_choice(self, choice: GuidedChoiceV2): + coro = self.endpoint.guided.POST(choice) + if not choice.capability.supports_manual_customization(): + self.app.next_screen(coro) return - # FIXME It would seem natural here to pass the wait=true flag to the - # below HTTP calls, especially because we wrap the coroutine in - # wait_with_progress. - # Having said that, making the server return a cached result seems like - # the least risky option to address https://launchpad.net/bugs/1993257 - # before the kinetic release. This is also similar to what we did for - # https://launchpad.net/bugs/1962205 - if choice is not None: - coro = self.endpoint.guided.POST(choice) - else: - coro = self.endpoint.GET(use_cached_result=True) status = await self.app.wait_with_progress(coro) self.model = FilesystemModel(status.bootloader) self.model.load_server_data(status) diff --git a/subiquity/common/types.py b/subiquity/common/types.py index aee6b01e..88bf940b 100644 --- a/subiquity/common/types.py +++ b/subiquity/common/types.py @@ -321,6 +321,7 @@ class Disk: class GuidedCapability(enum.Enum): + MANUAL = enum.auto() DIRECT = enum.auto() LVM = enum.auto() LVM_LUKS = enum.auto() @@ -340,6 +341,14 @@ class GuidedCapability(enum.Enum): GuidedCapability.CORE_BOOT_PREFER_ENCRYPTED, GuidedCapability.CORE_BOOT_PREFER_UNENCRYPTED] + def supports_manual_customization(self) -> bool: + # After posting this capability to guided_POST, is it possible + # for the user to customize the layout further? + return self in [GuidedCapability.MANUAL, + GuidedCapability.DIRECT, + GuidedCapability.LVM, + GuidedCapability.LVM_LUKS] + class GuidedDisallowedCapabilityReason(enum.Enum): TOO_SMALL = enum.auto() @@ -437,9 +446,17 @@ class GuidedStorageTargetUseGap: disallowed: List[GuidedDisallowedCapability] = attr.Factory(list) +@attr.s(auto_attribs=True) +class GuidedStorageTargetManual: + allowed: List[GuidedCapability] = attr.Factory( + lambda: [GuidedCapability.MANUAL]) + disallowed: List[GuidedDisallowedCapability] = attr.Factory(list) + + GuidedStorageTarget = Union[GuidedStorageTargetReformat, GuidedStorageTargetResize, - GuidedStorageTargetUseGap] + GuidedStorageTargetUseGap, + GuidedStorageTargetManual] @attr.s(auto_attribs=True) @@ -447,8 +464,7 @@ class GuidedChoiceV2: target: GuidedStorageTarget capability: GuidedCapability password: Optional[str] = attr.ib(default=None, repr=False) - sizing_policy: Optional[SizingPolicy] = \ - attr.ib(default=SizingPolicy.SCALED) + sizing_policy: Optional[SizingPolicy] = SizingPolicy.SCALED reset_partition: bool = False diff --git a/subiquity/server/controllers/filesystem.py b/subiquity/server/controllers/filesystem.py index 968b33ab..555b8769 100644 --- a/subiquity/server/controllers/filesystem.py +++ b/subiquity/server/controllers/filesystem.py @@ -67,6 +67,7 @@ from subiquity.common.types import ( GuidedDisallowedCapabilityReason, GuidedStorageResponseV2, GuidedStorageTarget, + GuidedStorageTargetManual, GuidedStorageTargetReformat, GuidedStorageTargetResize, GuidedStorageTargetUseGap, @@ -544,6 +545,9 @@ class FilesystemController(SubiquityController, FilesystemManipulator): choice: GuidedChoiceV2, reset_partition_only: bool = False ) -> None: + if choice.capability == GuidedCapability.MANUAL: + return + self.model.guided_configuration = choice self.set_info_for_capability(choice.capability) @@ -784,7 +788,7 @@ class FilesystemController(SubiquityController, FilesystemManipulator): async def guided_POST(self, data: GuidedChoiceV2) -> StorageResponse: log.debug(data) await self.guided(data) - if data.capability.is_core_boot(): + if not data.capability.supports_manual_customization(): await self.configured() return self._done_response() @@ -886,18 +890,14 @@ class FilesystemController(SubiquityController, FilesystemManipulator): async def v2_ensure_transaction_POST(self) -> None: self.locked_probe_data = True - def get_available_capabilities(self): + def get_classic_capabilities(self): classic_capabilities = set() - core_boot_capabilities = set() for info in self._variation_info.values(): if not info.is_valid(): continue - if info.is_core_boot_classic(): - core_boot_capabilities.update(info.capability_info.allowed) - else: + if not info.is_core_boot_classic(): classic_capabilities.update(info.capability_info.allowed) - return sorted(classic_capabilities, key=lambda x: x.name), \ - sorted(core_boot_capabilities, key=lambda x: x.name) + return sorted(classic_capabilities, key=lambda x: x.name) async def v2_guided_GET(self, wait: bool = False) \ -> GuidedStorageResponseV2: @@ -911,8 +911,10 @@ class FilesystemController(SubiquityController, FilesystemManipulator): scenarios = [] install_min = self.calculate_suggested_install_min() - classic_capabilities, core_boot_capabilities = \ - self.get_available_capabilities() + classic_capabilities = self.get_classic_capabilities() + + if GuidedCapability.DIRECT in classic_capabilities: + scenarios.append((0, GuidedStorageTargetManual())) for disk in self.potential_boot_disks(with_reformatting=True): capability_info = CapabilityInfo() diff --git a/subiquity/server/controllers/tests/test_filesystem.py b/subiquity/server/controllers/tests/test_filesystem.py index b1b7aaf4..24e3753b 100644 --- a/subiquity/server/controllers/tests/test_filesystem.py +++ b/subiquity/server/controllers/tests/test_filesystem.py @@ -36,6 +36,7 @@ from subiquity.common.types import ( GuidedCapability, GuidedDisallowedCapabilityReason, GuidedChoiceV2, + GuidedStorageTargetManual, GuidedStorageTargetReformat, GuidedStorageTargetResize, GuidedStorageTargetUseGap, @@ -540,6 +541,7 @@ class TestGuidedV2(IsolatedAsyncioTestCase): expected = [ GuidedStorageTargetReformat( disk_id=self.disk.id, allowed=default_capabilities), + GuidedStorageTargetManual(), ] resp = await self.fsc.v2_guided_GET() self.assertEqual(expected, resp.targets) @@ -553,11 +555,23 @@ class TestGuidedV2(IsolatedAsyncioTestCase): self.assertEqual([], resp.targets) self.assertEqual(ProbeStatus.PROBING, resp.status) + async def test_manual(self): + await self._setup(Bootloader.UEFI, 'gpt') + guided_get_resp = await self.fsc.v2_guided_GET() + [reformat, manual] = guided_get_resp.targets + self.assertEqual(manual, GuidedStorageTargetManual()) + data = GuidedChoiceV2(target=reformat, capability=manual.allowed[0]) + # POSTing the manual choice doesn't change anything + await self.fsc.v2_guided_POST(data=data) + guided_get_resp = await self.fsc.v2_guided_GET() + self.assertEqual([reformat, manual], guided_get_resp.targets) + @parameterized.expand(bootloaders_and_ptables) async def test_small_blank_disk(self, bootloader, ptable): await self._setup(bootloader, ptable, size=1 << 30) resp = await self.fsc.v2_guided_GET() - self.assertEqual(1, len(resp.targets)) + self.assertEqual(2, len(resp.targets)) + self.assertEqual(GuidedStorageTargetManual(), resp.targets[1]) self.assertEqual(0, len(resp.targets[0].allowed)) self.assertEqual( { @@ -600,7 +614,7 @@ class TestGuidedV2(IsolatedAsyncioTestCase): self.assertEqual(self.disk.id, resize.disk_id) self.assertEqual(p.number, resize.partition_number) self.assertTrue(isinstance(resize, GuidedStorageTargetResize)) - self.assertEqual(0, len(resp.targets)) + self.assertEqual(1, len(resp.targets)) @parameterized.expand(bootloaders_and_ptables) async def test_used_full_disk(self, bootloader, ptable): @@ -619,7 +633,7 @@ class TestGuidedV2(IsolatedAsyncioTestCase): self.assertEqual(self.disk.id, resize.disk_id) self.assertEqual(p.number, resize.partition_number) self.assertTrue(isinstance(resize, GuidedStorageTargetResize)) - self.assertEqual(0, len(resp.targets)) + self.assertEqual(1, len(resp.targets)) @parameterized.expand(bootloaders_and_ptables) async def test_weighted_split(self, bootloader, ptable): @@ -648,7 +662,7 @@ class TestGuidedV2(IsolatedAsyncioTestCase): maximum=230 << 30, allowed=default_capabilities) self.assertEqual(expected, resize) - self.assertEqual(0, len(possible)) + self.assertEqual(1, len(possible)) @parameterized.expand(bootloaders_and_ptables) async def test_half_disk_reformat(self, bootloader, ptable): @@ -675,7 +689,7 @@ class TestGuidedV2(IsolatedAsyncioTestCase): resp = await self.fsc.v2_GET() self.assertFalse(resp.need_root) self.assertFalse(resp.need_boot) - self.assertEqual(0, len(guided_get_resp.targets)) + self.assertEqual(1, len(guided_get_resp.targets)) @parameterized.expand(bootloaders_and_ptables) async def test_half_disk_use_gap(self, bootloader, ptable): @@ -708,7 +722,7 @@ class TestGuidedV2(IsolatedAsyncioTestCase): self.assertEqual(orig_p, existing_part) self.assertFalse(resp.need_root) self.assertFalse(resp.need_boot) - self.assertEqual(0, len(guided_get_resp.targets)) + self.assertEqual(1, len(guided_get_resp.targets)) @parameterized.expand(bootloaders_and_ptables) async def test_half_disk_resize(self, bootloader, ptable): @@ -743,7 +757,7 @@ class TestGuidedV2(IsolatedAsyncioTestCase): self.assertEqual(p_expected, existing_part) self.assertFalse(resp.need_root) self.assertFalse(resp.need_boot) - self.assertEqual(0, len(guided_get_resp.targets)) + self.assertEqual(1, len(guided_get_resp.targets)) @parameterized.expand([ [10], [20], [25], [30], [50], [100], [250], diff --git a/subiquity/tests/api/test_api.py b/subiquity/tests/api/test_api.py index b9e0f71c..8c00e352 100644 --- a/subiquity/tests/api/test_api.py +++ b/subiquity/tests/api/test_api.py @@ -332,7 +332,7 @@ class TestFlow(TestAPI): {'elected': 'http://us.archive.ubuntu.com/ubuntu'}) resp = await inst.get('/storage/v2/guided?wait=true') - [reformat] = resp['targets'] + [reformat, manual] = resp['targets'] await inst.post( '/storage/v2/guided', { @@ -605,7 +605,7 @@ class TestCore(TestAPI): async with start_server(cfg, **kw) as inst: await inst.post('/source', source_id='ubuntu-desktop') resp = await inst.get('/storage/v2/guided', wait=True) - [reformat] = resp['targets'] + [reformat, manual] = resp['targets'] self.assertIn('CORE_BOOT_PREFER_ENCRYPTED', reformat['allowed']) data = dict(target=reformat, capability='CORE_BOOT_ENCRYPTED') @@ -1042,7 +1042,7 @@ class TestTodos(TestAPI): # server indicators of required client actions self.assertTrue(resp['need_boot']) resp = await inst.get('/storage/v2/guided') - [reformat] = resp['targets'] + [reformat, manual] = resp['targets'] data = { 'target': reformat, 'capability': reformat['allowed'][0], diff --git a/subiquity/ui/views/filesystem/guided.py b/subiquity/ui/views/filesystem/guided.py index 6b8bb781..3ed7c8d4 100644 --- a/subiquity/ui/views/filesystem/guided.py +++ b/subiquity/ui/views/filesystem/guided.py @@ -50,6 +50,7 @@ from subiquity.common.types import ( Gap, GuidedCapability, GuidedChoiceV2, + GuidedStorageTargetManual, GuidedStorageTargetReformat, Partition, ) @@ -313,12 +314,11 @@ class GuidedDiskSelectionView(BaseView): def done(self, sender): results = sender.as_data() password = None + capability = None disk_id = None if self.controller.core_boot_capability is not None: if results.get('use_tpm', sender.tpm_choice.default): capability = GuidedCapability.CORE_BOOT_ENCRYPTED - else: - capability = GuidedCapability.CORE_BOOT_UNENCRYPTED disk_id = results['disk'].id elif results['guided']: if results['guided_choice']['use_lvm']: @@ -341,11 +341,17 @@ class GuidedDiskSelectionView(BaseView): password=password, ) else: - choice = None + choice = GuidedChoiceV2( + target=GuidedStorageTargetManual(), + capability=GuidedCapability.MANUAL, + ) self.controller.guided_choice(choice) def manual(self, sender): - self.controller.guided_choice(None) + self.controller.guided_choice(GuidedChoiceV2( + target=GuidedStorageTargetManual(), + capability=GuidedCapability.MANUAL, + )) def cancel(self, btn=None): self.controller.cancel()