Merge pull request #1706 from mwhudson/manual-capability

add a target/capability for manual partitioning

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]):
+    async def _guided_choice(self, choice: GuidedChoiceV2):
-        if self.core_boot_capability is not None:
+        coro = self.endpoint.guided.POST(choice)
-            self.app.next_screen(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)

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] = \
+    sizing_policy: Optional[SizingPolicy] = SizingPolicy.SCALED
-        attr.ib(default=SizingPolicy.SCALED)
     reset_partition: bool = False
 
 

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():
+            if not info.is_core_boot_classic():
-                core_boot_capabilities.update(info.capability_info.allowed)
-            else:
                 classic_capabilities.update(info.capability_info.allowed)
-        return sorted(classic_capabilities, key=lambda x: x.name), \
+        return sorted(classic_capabilities, key=lambda x: x.name)
-            sorted(core_boot_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 = \
+        classic_capabilities = self.get_classic_capabilities()
-            self.get_available_capabilities()
+
+        if GuidedCapability.DIRECT in classic_capabilities:
+            scenarios.append((0, GuidedStorageTargetManual()))
 
         for disk in self.potential_boot_disks(with_reformatting=True):
             capability_info = CapabilityInfo()

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],

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],

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()