Merge pull request #1705 from mwhudson/bye-v1-guided

move server client (mostly) to v2 guided API

subiquity/client/controllers/filesystem.py

@@ -27,6 +27,10 @@
 from subiquity.common.types import (
     ProbeStatus,
     GuidedCapability,
+    GuidedChoiceV2,
+    GuidedStorageResponseV2,
+    GuidedStorageTargetReformat,
+    StorageResponseV2,
     )
 from subiquity.models.filesystem import (
     Bootloader,
@@ -66,12 +70,12 @@ def get_current_view() -> BaseView:
             assert self.current_view is not None
             return self.current_view
 
-        status = await self.endpoint.guided.GET()
+        status: GuidedStorageResponseV2 = await self.endpoint.v2.guided.GET()
         if status.status == ProbeStatus.PROBING:
             run_bg_task(self._wait_for_probing())
             self.current_view = SlowProbing(self)
         else:
-            self.current_view = self.make_guided_ui(status)
+            self.current_view = await self.make_guided_ui(status)
         # NOTE: If we return a BaseView instance directly here, we have no
         # guarantee that it will be displayed on the screen by the time the
         # probing operation finishes. Therefore, to allow us to reliably
@@ -83,34 +87,59 @@ def get_current_view() -> BaseView:
         return get_current_view
 
     async def _wait_for_probing(self):
-        status = await self.endpoint.guided.GET(wait=True)
-        self.current_view = self.make_guided_ui(status)
+        status = await self.endpoint.v2.guided.GET(wait=True)
+        self.current_view = await self.make_guided_ui(status)
         if isinstance(self.ui.body, SlowProbing):
             self.ui.set_body(self.current_view)
         else:
             log.debug("not refreshing the display. Current display is %r",
                       self.ui.body)
 
-    def make_guided_ui(self, status):
-        if status.core_boot_classic_error != '':
-            return CoreBootClassicError(self, status.core_boot_classic_error)
+    async def make_guided_ui(
+            self,
+            status: GuidedStorageResponseV2,
+            ) -> GuidedDiskSelectionView:
         if status.status == ProbeStatus.FAILED:
             self.app.show_error_report(status.error_report)
             return ProbingFailed(self, status.error_report)
 
-        for capability in status.capabilities:
-            if capability.is_core_boot():
-                assert len(status.capabilities) == 1
-                self.core_boot_capability = status.capabilities[0]
-                break
-        else:
-            self.core_boot_capability = None
+        reformat_targets = [
+            target
+            for target in status.targets
+            if isinstance(target, GuidedStorageTargetReformat)
+            ]
+
+        self.core_boot_capability = None
+        self.encryption_unavailable_reason = ''
+
+        response: StorageResponseV2 = await self.endpoint.v2.GET(
+            include_raid=True)
+
+        disk_by_id = {
+            disk.id: disk for disk in response.disks
+            }
+
+        disks = []
+
+        for target in reformat_targets:
+            if target.allowed:
+                disks.append(disk_by_id[target.disk_id])
+                for capability in target.allowed:
+                    if capability.is_core_boot():
+                        assert len(target.allowed) == 1
+                        self.core_boot_capability = capability
+            for disallowed in target.disallowed:
+                if disallowed.capability.is_core_boot():
+                    self.encryption_unavailable_reason = disallowed.message
+
+        if not disks and self.encryption_unavailable_reason:
+            return CoreBootClassicError(
+                self, self.encryption_unavailable_reason)
 
-        self.encryption_unavailable_reason = \
-            status.encryption_unavailable_reason
         if status.error_report:
             self.app.show_error_report(status.error_report)
-        return GuidedDiskSelectionView(self, status.disks)
+
+        return GuidedDiskSelectionView(self, disks)
 
     async def run_answers(self):
         # Wait for probing to finish.
@@ -258,7 +287,7 @@ async def _answers_action(self, action):
         else:
             raise Exception("could not process action {}".format(action))
 
-    async def _guided_choice(self, choice):
+    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))
             return

subiquity/common/apidef.py

@@ -43,9 +43,7 @@
     Change,
     Disk,
     ErrorReportRef,
-    GuidedChoice,
     GuidedChoiceV2,
-    GuidedStorageResponse,
     GuidedStorageResponseV2,
     KeyboardSetting,
     KeyboardSetup,
@@ -269,10 +267,7 @@ def GET(dev_name: str) -> str: ...
 
     class storage:
         class guided:
-            def GET(wait: bool = False) -> GuidedStorageResponse:
-                pass
-
-            def POST(data: Payload[GuidedChoice]) \
+            def POST(data: Payload[GuidedChoiceV2]) \
                     -> StorageResponse:
                 pass
 
@@ -296,7 +291,11 @@ class has_bitlocker:
             def GET() -> List[Disk]: ...
 
         class v2:
-            def GET(wait: bool = False) -> StorageResponseV2: ...
+            def GET(
+                    wait: bool = False,
+                    include_raid: bool = False,
+                    ) -> StorageResponseV2: ...
+
             def POST() -> StorageResponseV2: ...
 
             class orig_config:

subiquity/common/types.py

@@ -353,23 +353,6 @@ class GuidedDisallowedCapability:
     message: Optional[str] = None
 
 
-@attr.s(auto_attribs=True)
-class GuidedChoice:
-    disk_id: str
-    capability: GuidedCapability = GuidedCapability.DIRECT
-    password: Optional[str] = attr.ib(default=None, repr=False)
-
-
-@attr.s(auto_attribs=True)
-class GuidedStorageResponse:
-    status: ProbeStatus
-    error_report: Optional[ErrorReportRef] = None
-    disks: Optional[List[Disk]] = None
-    core_boot_classic_error: str = ''
-    encryption_unavailable_reason: str = ''
-    capabilities: List[GuidedCapability] = attr.Factory(list)
-
-
 @attr.s(auto_attribs=True)
 class StorageResponse:
     status: ProbeStatus
@@ -468,16 +451,6 @@ class GuidedChoiceV2:
         attr.ib(default=SizingPolicy.SCALED)
     reset_partition: bool = False
 
-    @staticmethod
-    def from_guided_choice(choice: GuidedChoice):
-        return GuidedChoiceV2(
-                target=GuidedStorageTargetReformat(
-                    disk_id=choice.disk_id, allowed=[choice.capability]),
-                capability=choice.capability,
-                password=choice.password,
-                sizing_policy=SizingPolicy.SCALED,
-                )
-
 
 @attr.s(auto_attribs=True)
 class GuidedStorageResponseV2:

subiquity/server/controllers/filesystem.py

@@ -62,11 +62,9 @@
     Bootloader,
     Disk,
     GuidedCapability,
-    GuidedChoice,
     GuidedChoiceV2,
     GuidedDisallowedCapability,
     GuidedDisallowedCapabilityReason,
-    GuidedStorageResponse,
     GuidedStorageResponseV2,
     GuidedStorageTarget,
     GuidedStorageTargetReformat,
@@ -179,7 +177,7 @@ def capability_info_for_disk(
             install_min = self.min_size
         r = CapabilityInfo()
         r.disallowed = list(self.capability_info.disallowed)
-        if size < install_min:
+        if self.capability_info.allowed and size < install_min:
             for capability in self.capability_info.allowed:
                 r.disallowed.append(GuidedDisallowedCapability(
                     capability=capability,
@@ -666,46 +664,6 @@ def potential_boot_disks(self, check_boot=True, with_reformatting=False):
             disks.append(disk)
         return [d for d in disks if d not in self.model._exclusions]
 
-    async def guided_GET(self, wait: bool = False) -> GuidedStorageResponse:
-        probe_resp = await self._probe_response(wait, GuidedStorageResponse)
-        if probe_resp is not None:
-            return probe_resp
-        disks = self.potential_boot_disks(with_reformatting=True)
-
-        # Choose the first non-core-boot one offered.  If we only have
-        # core-boot choices, choose the first of those.
-        core_boot_info = None
-        for info in self._variation_info.values():
-            if not info.is_core_boot_classic():
-                break
-            if core_boot_info is None:
-                core_boot_info = info
-        else:
-            info = core_boot_info
-
-        if info.capability_info.allowed:
-            disks = [
-                labels.for_client(d, min_size=info.min_size) for d in disks
-                ]
-            error = ''
-        else:
-            disks = []
-            error = info.capability_info.disallowed[0].message
-
-        encryption_unavailable_reason = ''
-        for disallowed_cap in info.capability_info.disallowed:
-            if disallowed_cap.capability == \
-               GuidedCapability.CORE_BOOT_ENCRYPTED:
-                encryption_unavailable_reason = disallowed_cap.message
-
-        return GuidedStorageResponse(
-            status=ProbeStatus.DONE,
-            error_report=self.full_probe_error(),
-            disks=disks,
-            core_boot_classic_error=error,
-            encryption_unavailable_reason=encryption_unavailable_reason,
-            capabilities=list(info.capability_info.allowed))
-
     def _offsets_and_sizes_for_volume(self, volume):
         offset = self.model._partition_alignment_data['gpt'].min_start_offset
         for structure in volume.structure:
@@ -823,9 +781,9 @@ async def finish_install(self, context):
                 step=snapdapi.SystemActionStep.FINISH,
                 on_volumes=self._on_volumes()))
 
-    async def guided_POST(self, data: GuidedChoice) -> StorageResponse:
+    async def guided_POST(self, data: GuidedChoiceV2) -> StorageResponse:
         log.debug(data)
-        await self.guided(GuidedChoiceV2.from_guided_choice(data))
+        await self.guided(data)
         if data.capability.is_core_boot():
             await self.configured()
         return self._done_response()
@@ -873,13 +831,18 @@ def calculate_suggested_install_min(self):
                      for pa in self.model._partition_alignment_data.values()))
         return sizes.calculate_suggested_install_min(source_min, align)
 
-    async def get_v2_storage_response(self, model, wait):
+    async def get_v2_storage_response(self, model, wait, include_raid):
         probe_resp = await self._probe_response(wait, StorageResponseV2)
         if probe_resp is not None:
             return probe_resp
-        disks = [
-            d for d in model._all(type='disk') if d not in model._exclusions
-            ]
+        if include_raid:
+            disks = self.potential_boot_disks(with_reformatting=True)
+        else:
+            disks = [
+                d
+                for d in model._all(type='disk')
+                if d not in model._exclusions
+                ]
         minsize = self.calculate_suggested_install_min()
         return StorageResponseV2(
                 status=ProbeStatus.DONE,
@@ -889,16 +852,21 @@ async def get_v2_storage_response(self, model, wait):
                 install_minimum_size=minsize,
                 )
 
-    async def v2_GET(self, wait: bool = False) -> StorageResponseV2:
-        return await self.get_v2_storage_response(self.model, wait)
+    async def v2_GET(
+            self,
+            wait: bool = False,
+            include_raid: bool = False,
+            ) -> StorageResponseV2:
+        return await self.get_v2_storage_response(
+            self.model, wait, include_raid)
 
     async def v2_POST(self) -> StorageResponseV2:
         await self.configured()
         return await self.v2_GET()
 
     async def v2_orig_config_GET(self) -> StorageResponseV2:
         model = self.model.get_orig_model()
-        return await self.get_v2_storage_response(model, False)
+        return await self.get_v2_storage_response(model, False, False)
 
     async def v2_reset_POST(self) -> StorageResponseV2:
         log.info("Resetting Filesystem model")

subiquity/server/controllers/tests/test_filesystem.py

@@ -1058,3 +1058,22 @@ async def test_from_sample_data(self):
         request = call.args[2]
         self.assertEqual(request.action, snapdapi.SystemAction.INSTALL)
         self.assertEqual(request.step, snapdapi.SystemActionStep.FINISH)
+
+    async def test_from_sample_data_defective(self):
+        self.fsc.model = model = make_model(Bootloader.UEFI)
+        make_disk(model)
+        self.app.base_model.source.current.variations = {
+            'default': CatalogEntryVariation(
+                path='', size=1, snapd_system_label='defective'),
+            }
+        self.app.dr_cfg.systems_dir_exists = True
+        await self.fsc._examine_systems_task.start()
+        self.fsc.start()
+        response = await self.fsc.v2_guided_GET(wait=True)
+        self.assertEqual(len(response.targets), 1)
+        self.assertEqual(len(response.targets[0].allowed), 0)
+        self.assertEqual(len(response.targets[0].disallowed), 1)
+        disallowed = response.targets[0].disallowed[0]
+        self.assertEqual(
+            disallowed.reason,
+            GuidedDisallowedCapabilityReason.CORE_BOOT_ENCRYPTION_UNAVAILABLE)

subiquity/ui/views/filesystem/guided.py

@@ -49,7 +49,8 @@
 from subiquity.common.types import (
     Gap,
     GuidedCapability,
-    GuidedChoice,
+    GuidedChoiceV2,
+    GuidedStorageTargetReformat,
     Partition,
 )
 from subiquity.models.filesystem import humanize_size
@@ -311,16 +312,15 @@ def local_help(self):
 
     def done(self, sender):
         results = sender.as_data()
-        choice = None
+        password = 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
-            choice = GuidedChoice(
-                disk_id=results['disk'].id, capability=capability)
+            disk_id = results['disk'].id
         elif results['guided']:
-            password = None
             if results['guided_choice']['use_lvm']:
                 opts = results['guided_choice'].get('lvm_options', {})
                 if opts.get('encrypt', False):
@@ -330,9 +330,18 @@ def done(self, sender):
                     capability = GuidedCapability.LVM
             else:
                 capability = GuidedCapability.DIRECT
-            choice = GuidedChoice(
-                disk_id=results['guided_choice']['disk'].id,
-                capability=capability, password=password)
+            disk_id = results['guided_choice']['disk'].id
+        else:
+            disk_id = None
+        if disk_id is not None:
+            choice = GuidedChoiceV2(
+                target=GuidedStorageTargetReformat(
+                    disk_id=disk_id, allowed=[capability]),
+                capability=capability,
+                password=password,
+                )
+        else:
+            choice = None
         self.controller.guided_choice(choice)
 
     def manual(self, sender):