diff --git a/DESIGN.md b/DESIGN.md index 0c1324f2..232af8c9 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -345,7 +345,10 @@ The server code proceeds in stages: commands. 2. Then it waits for all the model objects that feed into the curtin config to be configured. - 3. It waits for confirmation. + 3. It waits for confirmation. This can in theory be interrupted by a + change in the model objects which are considered 'interesting', + see below, so it is possible to transition from here to the + previous state. 4. It runs "curtin install" and waits for that to finish. 5. It waits for the model objects that feed into the cloud-init config to be configured. @@ -361,6 +364,14 @@ enum, so the client gets notified via long-polling `meta.status.GET()` of progress. In addition, `ApplicationState.ERROR` indicates something has gone wrong. +Originally subiquity was just the server installer and so the set of +models that have to be configured for each step was just static. But +subiquity can also install desktop systems and maybe one day Ubuntu +core, and different information is needed to fully configure each +different variant. This information is handled by the +`subiquity.models.subiquity.SubiquityModel.set_source_variant` method +and surrounding code. + ### Error handling As hard as it is to believe, things do go wrong during diff --git a/subiquity/models/subiquity.py b/subiquity/models/subiquity.py index 2537ec66..93c93470 100644 --- a/subiquity/models/subiquity.py +++ b/subiquity/models/subiquity.py @@ -69,29 +69,20 @@ ff02::1 ip6-allnodes ff02::2 ip6-allrouters """ -# Models that contribute to the curtin config -INSTALL_MODEL_NAMES = [ - "debconf_selections", - "filesystem", - "kernel", - "keyboard", - "mirror", - "network", - "proxy", - ] -# Models that contribute to the cloud-init config (and other postinstall steps) -POSTINSTALL_MODEL_NAMES = [ - "identity", - "locale", - "packages", - "snaplist", - "ssh", - "timezone", - "userdata", - ] +class ModelNames: + def __init__(self, default_names, **per_variant_names): + self.default_names = default_names + self.per_variant_names = per_variant_names -ALL_MODEL_NAMES = INSTALL_MODEL_NAMES + POSTINSTALL_MODEL_NAMES + def for_variant(self, variant): + return self.default_names | self.per_variant_names.get(variant, set()) + + def all(self): + r = set(self.default_names) + for v in self.per_variant_names.values(): + r |= v + return r class DebconfSelectionsModel: @@ -108,7 +99,7 @@ class SubiquityModel: target = '/target' - def __init__(self, root): + def __init__(self, root, install_model_names, postinstall_model_names): self.root = root if root != '/': self.target = root @@ -129,45 +120,77 @@ class SubiquityModel: self.updates = UpdatesModel() self.userdata = {} - self.confirmation = asyncio.Event() + self._confirmation = asyncio.Event() + self._confirmation_task = None - self._events = { - name: asyncio.Event() for name in ALL_MODEL_NAMES - } - self.install_events = { - self._events[name] for name in INSTALL_MODEL_NAMES - } - self.postinstall_events = { - self._events[name] for name in POSTINSTALL_MODEL_NAMES - } + self._configured_names = set() + self._install_model_names = install_model_names + self._postinstall_model_names = postinstall_model_names + self._cur_install_model_names = install_model_names.default_names + self._cur_postinstall_model_names = \ + postinstall_model_names.default_names + self._install_event = asyncio.Event() + self._postinstall_event = asyncio.Event() + + def set_source_variant(self, variant): + self._cur_install_model_names = \ + self._install_model_names.for_variant(variant) + self._cur_postinstall_model_names = \ + self._postinstall_model_names.for_variant(variant) + unconfigured_install_model_names = \ + self._cur_install_model_names - self._configured_names + if unconfigured_install_model_names: + if self._install_event.is_set(): + self._install_event = asyncio.Event() + if self._confirmation_task is not None: + self._confirmation_task.cancel() + else: + self._install_event.set() def configured(self, model_name): - if model_name not in ALL_MODEL_NAMES: - return - self._events[model_name].set() - if model_name in INSTALL_MODEL_NAMES: + self._configured_names.add(model_name) + if model_name in self._cur_install_model_names: stage = 'install' - unconfigured = { - mn for mn in INSTALL_MODEL_NAMES - if not self._events[mn].is_set() - } - elif model_name in POSTINSTALL_MODEL_NAMES: + names = self._cur_install_model_names + event = self._install_event + elif model_name in self._cur_postinstall_model_names: stage = 'postinstall' - unconfigured = { - mn for mn in POSTINSTALL_MODEL_NAMES - if not self._events[mn].is_set() - } + names = self._cur_postinstall_model_names + event = self._postinstall_event + else: + return + unconfigured = names - self._configured_names log.debug( - "model %s for %s is configured, to go %s", + "model %s for %s stage is configured, to go %s", model_name, stage, unconfigured) + if not unconfigured: + event.set() - def needs_configuration(self, model_name): - if model_name is None: + async def wait_install(self): + await self._install_event.wait() + + async def wait_postinstall(self): + await self._postinstall_event.wait() + + async def wait_confirmation(self): + if self._confirmation_task is None: + self._confirmation_task = asyncio.get_event_loop().create_task( + self._confirmation.wait()) + try: + await self._confirmation_task + except asyncio.CancelledError: return False - return not self._events[model_name].is_set() + else: + return True + finally: + self._confirmation_task = None + + def is_postinstall_only(self, model_name): + return model_name in self._cur_postinstall_model_names and \ + model_name not in self._cur_install_model_names def confirm(self): - self.confirmation.set() + self._confirmation.set() def get_target_groups(self): command = ['chroot', self.target, 'getent', 'group'] @@ -214,7 +237,7 @@ class SubiquityModel: config['ssh_authorized_keys'] = self.ssh.authorized_keys if self.ssh.install_server: config['ssh_pwauth'] = self.ssh.pwauth - for model_name in POSTINSTALL_MODEL_NAMES: + for model_name in self._postinstall_model_names.all(): model = getattr(self, model_name) if getattr(model, 'make_cloudconfig', None): merge_config(config, model.make_cloudconfig()) @@ -341,7 +364,7 @@ class SubiquityModel: 'permissions': 0o644, } - for model_name in INSTALL_MODEL_NAMES: + for model_name in self._install_model_names.all(): model = getattr(self, model_name) log.debug("merging config from %s", model) merge_config(config, model.render()) diff --git a/subiquity/models/tests/test_subiquity.py b/subiquity/models/tests/test_subiquity.py index 96839882..f0c79a65 100644 --- a/subiquity/models/tests/test_subiquity.py +++ b/subiquity/models/tests/test_subiquity.py @@ -17,7 +17,26 @@ import fnmatch import unittest import yaml -from subiquity.models.subiquity import SubiquityModel +from subiquity.models.subiquity import ModelNames, SubiquityModel +from subiquity.server.server import ( + INSTALL_MODEL_NAMES, + POSTINSTALL_MODEL_NAMES, + ) + + +class TestModelNames(unittest.TestCase): + + def test_for_known_variant(self): + model_names = ModelNames({'a'}, var1={'b'}, var2={'c'}) + self.assertEqual(model_names.for_variant('var1'), {'a', 'b'}) + + def test_for_unknown_variant(self): + model_names = ModelNames({'a'}, var1={'b'}, var2={'c'}) + self.assertEqual(model_names.for_variant('var3'), {'a'}) + + def test_all(self): + model_names = ModelNames({'a'}, var1={'b'}, var2={'c'}) + self.assertEqual(model_names.all(), {'a', 'b', 'c'}) class TestSubiquityModel(unittest.TestCase): @@ -71,8 +90,21 @@ class TestSubiquityModel(unittest.TestCase): cur = cur[component] self.fail("config has value {} for {}".format(cur, path)) + def test_configure(self): + model = SubiquityModel( + 'test', ModelNames({'a', 'b'}), ModelNames(set())) + model.set_source_variant('var') + model.configured('a') + self.assertFalse(model._install_event.is_set()) + model.configured('b') + self.assertTrue(model._install_event.is_set()) + + def make_model(self): + return SubiquityModel( + 'test', INSTALL_MODEL_NAMES, POSTINSTALL_MODEL_NAMES) + def test_proxy_set(self): - model = SubiquityModel('test') + model = self.make_model() proxy_val = 'http://my-proxy' model.proxy.proxy = proxy_val config = model.render('ident') @@ -87,7 +119,7 @@ class TestSubiquityModel(unittest.TestCase): self.assertTrue(len(confs) > 0) def test_proxy_notset(self): - model = SubiquityModel('test') + model = self.make_model() config = model.render('ident') self.assertConfigDoesNotHaveVal(config, 'proxy.http_proxy') self.assertConfigDoesNotHaveVal(config, 'proxy.https_proxy') @@ -100,13 +132,13 @@ class TestSubiquityModel(unittest.TestCase): self.assertTrue(len(confs) == 0) def test_keyboard(self): - model = SubiquityModel('test') + model = self.make_model() config = model.render('ident') self.assertConfigWritesFile(config, 'etc/default/keyboard') def test_writes_machine_id_media_info(self): - model_no_proxy = SubiquityModel('test') - model_proxy = SubiquityModel('test') + model_no_proxy = self.make_model() + model_proxy = self.make_model() model_proxy.proxy.proxy = 'http://something' for model in model_no_proxy, model_proxy: config = model.render('ident') @@ -114,12 +146,12 @@ class TestSubiquityModel(unittest.TestCase): self.assertConfigWritesFile(config, 'var/log/installer/media-info') def test_storage_version(self): - model = SubiquityModel('test') + model = self.make_model() config = model.render('ident') self.assertConfigHasVal(config, 'storage.version', 1) def test_write_netplan(self): - model = SubiquityModel('test') + model = self.make_model() config = model.render('ident') netplan_content = None for fspec in config['write_files'].values(): @@ -132,12 +164,12 @@ class TestSubiquityModel(unittest.TestCase): self.assertConfigHasVal(netplan, 'network.version', 2) def test_has_sources(self): - model = SubiquityModel('test') + model = self.make_model() config = model.render('ident') self.assertIn('sources', config) def test_mirror(self): - model = SubiquityModel('test') + model = self.make_model() mirror_val = 'http://my-mirror' model.mirror.set_mirror(mirror_val) config = model.render('ident') diff --git a/subiquity/server/controller.py b/subiquity/server/controller.py index 085c2e75..90e9eabf 100644 --- a/subiquity/server/controller.py +++ b/subiquity/server/controller.py @@ -35,7 +35,6 @@ class SubiquityController(BaseController): autoinstall_schema = None autoinstall_default = None endpoint = None - relevant_variants = ('desktop', 'server') def __init__(self, app): super().__init__(app) diff --git a/subiquity/server/controllers/install.py b/subiquity/server/controllers/install.py index 8095b24f..42a0f197 100644 --- a/subiquity/server/controllers/install.py +++ b/subiquity/server/controllers/install.py @@ -199,15 +199,19 @@ class InstallController(SubiquityController): async def install(self, *, context): context.set('is-install-context', True) try: - await asyncio.wait({e.wait() for e in self.model.install_events}) + while True: + self.app.update_state(ApplicationState.WAITING) - if not self.app.interactive: - if 'autoinstall' in self.app.kernel_cmdline: - self.model.confirm() + await self.model.wait_install() - self.app.update_state(ApplicationState.NEEDS_CONFIRMATION) + if not self.app.interactive: + if 'autoinstall' in self.app.kernel_cmdline: + self.model.confirm() - await self.model.confirmation.wait() + self.app.update_state(ApplicationState.NEEDS_CONFIRMATION) + + if await self.model.wait_confirmation(): + break self.app.update_state(ApplicationState.RUNNING) @@ -224,8 +228,7 @@ class InstallController(SubiquityController): self.app.update_state(ApplicationState.POST_WAIT) - await asyncio.wait( - {e.wait() for e in self.model.postinstall_events}) + await self.model.wait_postinstall() self.app.update_state(ApplicationState.POST_RUNNING) diff --git a/subiquity/server/controllers/timezone.py b/subiquity/server/controllers/timezone.py index 64a0a611..b3403003 100644 --- a/subiquity/server/controllers/timezone.py +++ b/subiquity/server/controllers/timezone.py @@ -79,7 +79,6 @@ class TimeZoneController(SubiquityController): } autoinstall_default = '' - relevant_variants = ('desktop', ) def load_autoinstall_data(self, data): self.deserialize(data) diff --git a/subiquity/server/server.py b/subiquity/server/server.py index fabc7dca..e4743e7e 100644 --- a/subiquity/server/server.py +++ b/subiquity/server/server.py @@ -61,7 +61,10 @@ from subiquity.common.types import ( LiveSessionSSHInfo, PasswordKind, ) -from subiquity.models.subiquity import SubiquityModel +from subiquity.models.subiquity import ( + ModelNames, + SubiquityModel, + ) from subiquity.server.controller import SubiquityController from subiquity.server.geoip import GeoIP from subiquity.server.errors import ErrorController @@ -111,9 +114,7 @@ class MetaController: async def client_variant_POST(self, variant: str) -> None: if variant not in ('desktop', 'server'): raise ValueError(f'unrecognized client variant {variant}') - for controller in self.app.controllers.instances: - if variant not in controller.relevant_variants: - controller.configured() + self.app.base_model.set_source_variant(variant) async def ssh_info_GET(self) -> Optional[LiveSessionSSHInfo]: ips = [] @@ -158,6 +159,27 @@ def get_installer_password_from_cloudinit_log(): return None +INSTALL_MODEL_NAMES = ModelNames({ + "debconf_selections", + "filesystem", + "kernel", + "keyboard", + "mirror", + "network", + "proxy", + }) + +POSTINSTALL_MODEL_NAMES = ModelNames({ + "identity", + "locale", + "packages", + "snaplist", + "ssh", + "userdata", + }, + desktop={"timezone"}) + + class SubiquityServer(Application): snapd_socket_path = '/run/snapd.socket' @@ -207,7 +229,8 @@ class SubiquityServer(Application): root = '/' if self.opts.dry_run: root = os.path.abspath('.subiquity') - return SubiquityModel(root) + return SubiquityModel( + root, INSTALL_MODEL_NAMES, POSTINSTALL_MODEL_NAMES) def __init__(self, opts, block_log_dir): super().__init__(opts) @@ -353,7 +376,7 @@ class SubiquityServer(Application): if not controller.interactive(): override_status = 'skip' elif self.state == ApplicationState.NEEDS_CONFIRMATION: - if self.base_model.needs_configuration(controller.model_name): + if self.base_model.is_postinstall_only(controller.model_name): override_status = 'confirm' if override_status is not None: resp = web.Response(headers={'x-status': override_status})