From 59849f7f45bfca599cb80dfb299fec29d37877ff Mon Sep 17 00:00:00 2001
From: Olivier Gayot <olivier.gayot@canonical.com>
Date: Wed, 27 Sep 2023 11:03:47 +0200
Subject: [PATCH] oem: make sure storage is configured before using
 is_core_boot_classic

Before using fs_controller.is_core_boot_classic(), we wait for the call
to /meta/confirmation?tty=xxx. That said, in semi-automated desktop
installs, sometimes the call to /meta/confirmation happens before
marking storage configured. This leads to the following error:

  File "subiquity/server/controllers/oem.py", line 209, in apply_autoinstall_config
    await self.load_metapkgs_task
  File "subiquity/server/controllers/oem.py", line 81, in list_and_mark_configured
    await self.load_metapackages_list()
  File "subiquitycore/context.py", line 149, in decorated_async
    return await meth(self, **kw)
  File "subiquity/server/controllers/oem.py", line 136, in load_metapackages_list
    if fs_controller.is_core_boot_classic():
  File "subiquity/server/controllers/filesystem.py", line 284, in is_core_boot_classic
    return self._info.is_core_boot_classic()
AttributeError: 'NoneType' object has no attribute 'is_core_boot_classic'

Receiving the confirmation before getting the storage configured is
arguably wrong - but let's be prepared for it just in case.

Signed-off-by: Olivier Gayot <olivier.gayot@canonical.com>
---
 subiquity/server/controllers/oem.py | 10 +++++++++
 subiquity/tests/api/test_api.py     | 32 +++++++++++++++++++++++++++++
 2 files changed, 42 insertions(+)

diff --git a/subiquity/server/controllers/oem.py b/subiquity/server/controllers/oem.py
index be21d4f9b..2610f4837 100644
--- a/subiquity/server/controllers/oem.py
+++ b/subiquity/server/controllers/oem.py
@@ -65,6 +65,7 @@ def __init__(self, app) -> None:
 
         self.load_metapkgs_task: Optional[asyncio.Task] = None
         self.kernel_configured_event = asyncio.Event()
+        self.fs_configured_event = asyncio.Event()
 
     def start(self) -> None:
         self._wait_confirmation = asyncio.Event()
@@ -76,6 +77,9 @@ def start(self) -> None:
         self.app.hub.subscribe(
             (InstallerChannels.CONFIGURED, "kernel"), self.kernel_configured_event.set
         )
+        self.app.hub.subscribe(
+            (InstallerChannels.CONFIGURED, "filesystem"), self.fs_configured_event.set
+        )
 
         async def list_and_mark_configured() -> None:
             await self.load_metapackages_list()
@@ -128,6 +132,12 @@ async def wants_oem_kernel(self, pkgname: str, *, context, overlay) -> bool:
     async def load_metapackages_list(self, context) -> None:
         with context.child("wait_confirmation"):
             await self._wait_confirmation.wait()
+        # In normal scenarios, the confirmation event comes after the
+        # storage/filesystem is configured. However, in semi automated desktop
+        # installs (especially in CI), it is possible that the events come in
+        # the reverse order. Let's be prepared for it by also waiting for the
+        # storage configured event.
+        await self.fs_configured_event.wait()
 
         # Only look for OEM meta-packages on supported variants and if we are
         # not running core boot.
diff --git a/subiquity/tests/api/test_api.py b/subiquity/tests/api/test_api.py
index a8d3814c7..c4206e0b9 100644
--- a/subiquity/tests/api/test_api.py
+++ b/subiquity/tests/api/test_api.py
@@ -1820,6 +1820,38 @@ async def test_listing_certified_ubuntu_desktop(self):
             source_id="ubuntu-desktop", expected=["oem-somerville-tentacool-meta"]
         )
 
+    async def test_confirmation_before_storage_configured(self):
+        # On ubuntu-desktop, the confirmation event sometimes comes before the
+        # storage configured event. This was known to cause OEM to fail with
+        # the following error:
+        #   File "server/controllers/oem.py", in load_metapackages_list
+        #     if fs_controller.is_core_boot_classic():
+        #   File "server/controllers/filesystem.py", in is_core_boot_classic
+        #     return self._info.is_core_boot_classic()
+        # AttributeError: 'NoneType' object has no attribute
+        # 'is_core_boot_classic'
+        with patch.dict(os.environ, {"SUBIQUITY_DEBUG": "has-drivers"}):
+            config = "examples/machines/simple.json"
+            args = ["--source-catalog", "examples/sources/mixed.yaml"]
+            async with start_server(config, extra_args=args) as inst:
+                await inst.post("/source", source_id="ubuntu-desktop")
+                names = [
+                    "locale",
+                    "keyboard",
+                    "source",
+                    "network",
+                    "proxy",
+                    "mirror",
+                    "storage",
+                ]
+                await inst.post("/meta/confirm", tty="/dev/tty1")
+                await inst.post("/meta/mark_configured", endpoint_names=names)
+
+                resp = await inst.get("/oem", wait=True)
+                self.assertEqual(
+                    ["oem-somerville-tentacool-meta"], resp["metapackages"]
+                )
+
 
 class TestSource(TestAPI):
     @timeout()