From 3b3e3310d607e13d9037222d936f339f732640d8 Mon Sep 17 00:00:00 2001 From: Chris Peterson Date: Wed, 8 May 2024 15:57:50 -0700 Subject: [PATCH] filesystem: look-ahead on reset-partition-only The identity controller shouldn't own the logic for determining if the filesystem controller will only install the reset partition. Creates a utility function that can be called by the identity controller to determine if only installing the reset partition. --- subiquity/server/controllers/filesystem.py | 10 ++++ subiquity/server/controllers/identity.py | 9 +--- .../controllers/tests/test_filesystem.py | 47 +++++++++++++++++++ .../server/controllers/tests/test_identity.py | 7 ++- 4 files changed, 63 insertions(+), 10 deletions(-) diff --git a/subiquity/server/controllers/filesystem.py b/subiquity/server/controllers/filesystem.py index 1a58c0c0..e8bbd2e9 100644 --- a/subiquity/server/controllers/filesystem.py +++ b/subiquity/server/controllers/filesystem.py @@ -294,6 +294,16 @@ class FilesystemController(SubiquityController, FilesystemManipulator): # log.debug("self.ai_data = %s", data) self.ai_data = data + # The identity and user-data section are optional if we are only installing + # the reset partition, however the identity controller needs to know this + # before the filesystem controller naturally sets this. So this is a + # function to inspect the outcome early. + # See: https://github.com/canonical/subiquity/pull/1965 + def is_reset_partition_only(self): + storage_config = self.app.autoinstall_config.get(self.autoinstall_key, {}) + layout = storage_config.get("layout", {}) + return layout.get("reset-partition-only", False) + async def configured(self): self._configured = True if self._info is None: diff --git a/subiquity/server/controllers/identity.py b/subiquity/server/controllers/identity.py index 47981a75..726cf08a 100644 --- a/subiquity/server/controllers/identity.py +++ b/subiquity/server/controllers/identity.py @@ -89,14 +89,7 @@ class IdentityController(SubiquityController): if self.app.base_model.source.current.variant != "server": return # 3. we are only refreshing the reset partition - # (The identity controller doesn't figure this out until the apply - # step, so we are going to cheat and inspect the situation here) - storage_config = self.app.autoinstall_config.get("storage") - if ( - storage_config is not None - and storage_config.get("layout") is not None - and storage_config["layout"].get("reset-partition-only") - ): + if self.app.controllers.Filesystem.is_reset_partition_only(): return # 4. identity section is interactive if self.interactive(): diff --git a/subiquity/server/controllers/tests/test_filesystem.py b/subiquity/server/controllers/tests/test_filesystem.py index bbad135f..fdfce6bf 100644 --- a/subiquity/server/controllers/tests/test_filesystem.py +++ b/subiquity/server/controllers/tests/test_filesystem.py @@ -1660,3 +1660,50 @@ class TestMatchingDisks(IsolatedAsyncioTestCase): # overhead actual = self.fsc.get_bootable_matching_disk({"path": "/dev/md/*"}) self.assertEqual(r1, actual) + + +class TestResetPartitionLookAhead(IsolatedAsyncioTestCase): + def setUp(self): + self.app = make_app() + self.app.opts.bootloader = None + self.fsc = FilesystemController(app=self.app) + + @parameterized.expand( + # (config, is reset only) + ( + ({}, False), + ( + { + "storage": {}, + }, + False, + ), + ( + { + "storage": { + "layout": { + "name": "direct", + "reset-partition": True, + }, + }, + }, + False, + ), + ( + { + "storage": { + "layout": { + "reset-partition-only": True, + }, + }, + }, + True, + ), + ) + ) + def test_is_reset_partition_only_utility(self, config, expected): + """Test is_reset_partition_only utility""" + + self.app.autoinstall_config = config + + self.assertEqual(self.fsc.is_reset_partition_only(), expected) diff --git a/subiquity/server/controllers/tests/test_identity.py b/subiquity/server/controllers/tests/test_identity.py index fb4f80a8..8f72071f 100644 --- a/subiquity/server/controllers/tests/test_identity.py +++ b/subiquity/server/controllers/tests/test_identity.py @@ -17,6 +17,7 @@ import jsonschema from jsonschema.validators import validator_for from subiquity.server.autoinstall import AutoinstallError +from subiquity.server.controllers.filesystem import FilesystemController from subiquity.server.controllers.identity import IdentityController from subiquitycore.tests import SubiTestCase from subiquitycore.tests.mocks import make_app @@ -39,6 +40,8 @@ class TestControllerUserCreationFlows(SubiTestCase): # See subiquity/models/tests/test_subiquity.py for details. def setUp(self): self.app = make_app() + self.app.opts.bootloader = False + self.app.controllers.Filesystem = FilesystemController(self.app) self.ic = IdentityController(self.app) self.ic.model.user = None @@ -54,9 +57,9 @@ class TestControllerUserCreationFlows(SubiTestCase): ({"interactive-sections": ["*"]}, True), # No Autoinstall => interactive ({}, True), - # Can be missing if reset-parition-only specified + # Can be missing if reset-partition-only specified ({"storage": {"layout": {"reset-partition-only": True}}}, True), - # Can't be missing if reset-parition-only is not specified + # Can't be missing if reset-partition-only is not specified ({"storage": {"layout": {}}}, False), # user-data passed instead ({"user-data": "..."}, True),