From 9442c4ad07426a844b4731c6ed9dd8ebfca5d107 Mon Sep 17 00:00:00 2001 From: Michael Hudson-Doyle Date: Thu, 4 Nov 2021 22:04:41 +1300 Subject: [PATCH] make CurtinCommandRunner a bit more self-contained --- subiquity/models/subiquity.py | 11 +--------- subiquity/models/tests/test_subiquity.py | 16 +++++++------- subiquity/server/controllers/install.py | 4 ++-- subiquity/server/curtin.py | 27 ++++++++++++++++++------ 4 files changed, 31 insertions(+), 27 deletions(-) diff --git a/subiquity/models/subiquity.py b/subiquity/models/subiquity.py index ac06e77b..768b4eba 100644 --- a/subiquity/models/subiquity.py +++ b/subiquity/models/subiquity.py @@ -325,7 +325,7 @@ class SubiquityModel: with open('/etc/machine-id') as fp: return fp.read() - def render(self, syslog_identifier): + def render(self): # Until https://bugs.launchpad.net/curtin/+bug/1876984 gets # fixed, the only way to get curtin to leave the network # config entirely alone is to omit the 'network' stage. @@ -356,8 +356,6 @@ class SubiquityModel: '/var/log/installer/curtin-install.log', }, - 'verbosity': 3, - 'pollinate': { 'user_agent': { 'subiquity': "%s_%s" % (os.environ.get("SNAP_VERSION", @@ -367,13 +365,6 @@ class SubiquityModel: }, }, - 'reporting': { - 'subiquity': { - 'type': 'journald', - 'identifier': syslog_identifier, - }, - }, - 'write_files': { 'etc_machine_id': { 'path': 'etc/machine-id', diff --git a/subiquity/models/tests/test_subiquity.py b/subiquity/models/tests/test_subiquity.py index 083ec118..43ba68f8 100644 --- a/subiquity/models/tests/test_subiquity.py +++ b/subiquity/models/tests/test_subiquity.py @@ -116,7 +116,7 @@ class TestSubiquityModel(unittest.TestCase): model = self.make_model() proxy_val = 'http://my-proxy' model.proxy.proxy = proxy_val - config = model.render('ident') + config = model.render() self.assertConfigHasVal(config, 'proxy.http_proxy', proxy_val) self.assertConfigHasVal(config, 'proxy.https_proxy', proxy_val) self.assertConfigHasVal(config, 'apt.http_proxy', proxy_val) @@ -129,7 +129,7 @@ class TestSubiquityModel(unittest.TestCase): def test_proxy_notset(self): model = self.make_model() - config = model.render('ident') + config = model.render() self.assertConfigDoesNotHaveVal(config, 'proxy.http_proxy') self.assertConfigDoesNotHaveVal(config, 'proxy.https_proxy') self.assertConfigDoesNotHaveVal(config, 'apt.http_proxy') @@ -142,7 +142,7 @@ class TestSubiquityModel(unittest.TestCase): def test_keyboard(self): model = self.make_model() - config = model.render('ident') + config = model.render() self.assertConfigWritesFile(config, 'etc/default/keyboard') def test_writes_machine_id_media_info(self): @@ -150,18 +150,18 @@ class TestSubiquityModel(unittest.TestCase): model_proxy = self.make_model() model_proxy.proxy.proxy = 'http://something' for model in model_no_proxy, model_proxy: - config = model.render('ident') + config = model.render() self.assertConfigWritesFile(config, 'etc/machine-id') self.assertConfigWritesFile(config, 'var/log/installer/media-info') def test_storage_version(self): model = self.make_model() - config = model.render('ident') + config = model.render() self.assertConfigHasVal(config, 'storage.version', 1) def test_write_netplan(self): model = self.make_model() - config = model.render('ident') + config = model.render() netplan_content = None for fspec in config['write_files'].values(): if fspec['path'].startswith('etc/netplan'): @@ -174,14 +174,14 @@ class TestSubiquityModel(unittest.TestCase): def test_has_sources(self): model = self.make_model() - config = model.render('ident') + config = model.render() self.assertIn('sources', config) def test_mirror(self): model = self.make_model() mirror_val = 'http://my-mirror' model.mirror.set_mirror(mirror_val) - config = model.render('ident') + config = model.render() from curtin.commands.apt_config import get_mirror try: from curtin.distro import get_architecture diff --git a/subiquity/server/controllers/install.py b/subiquity/server/controllers/install.py index 8ce7b5be..707950dd 100644 --- a/subiquity/server/controllers/install.py +++ b/subiquity/server/controllers/install.py @@ -98,7 +98,7 @@ class InstallController(SubiquityController): self.tb_extractor.feed(event['MESSAGE']) def make_curtin_command_runner(self): - config = self.model.render(syslog_identifier=self._event_syslog_id) + config = self.model.render() config_location = '/var/log/installer/subiquity-curtin-install.conf' log_location = INSTALL_LOG if self.app.opts.dry_run: @@ -115,7 +115,7 @@ class InstallController(SubiquityController): self.app.note_file_for_apport("CurtinErrors", ERROR_TARFILE) self.app.note_file_for_apport("CurtinLog", log_location) self.curtin_runner = get_curtin_command_runner( - self.app, self._event_syslog_id, config_location) + self.app, config_location) @with_context(description="umounting /target dir") async def unmount_target(self, *, context, target): diff --git a/subiquity/server/curtin.py b/subiquity/server/curtin.py index 38606e64..9c1ebd25 100644 --- a/subiquity/server/curtin.py +++ b/subiquity/server/curtin.py @@ -17,6 +17,7 @@ import asyncio import json import logging +import os import sys from curtin.commands.install import ( @@ -34,13 +35,17 @@ log = logging.getLogger('subiquity.server.curtin') class CurtinCommandRunner: - def __init__(self, runner, event_syslog_id, config_location): + _count = 0 + + def __init__(self, runner, config_location=None): self.runner = runner - self.event_syslog_id = event_syslog_id + self._event_syslog_id = 'curtin_event.%s.%s' % ( + os.getpid(), CurtinCommandRunner._count) + CurtinCommandRunner._count += 1 self.config_location = config_location self._event_contexts = {} journald_listen( - asyncio.get_event_loop(), [event_syslog_id], self._event) + asyncio.get_event_loop(), [self._event_syslog_id], self._event) def _event(self, event): e = { @@ -78,8 +83,16 @@ class CurtinCommandRunner: def make_command(self, command, *args, **conf): cmd = [ sys.executable, '-m', 'curtin', '--showtrace', - '-c', self.config_location, ] + if self.config_location is not None: + cmd.extend([ + '-c', self.config_location, + ]) + conf.setdefault('reporting', {})['subiquity'] = { + 'type': 'journald', + 'identifier': self._event_syslog_id, + } + conf['verbosity'] = 3 for k, v in conf.items(): cmd.extend(['--set', 'json:' + k + '=' + json.dumps(v)]) cmd.append(command) @@ -105,7 +118,7 @@ class DryRunCurtinCommandRunner(CurtinCommandRunner): if command == 'install': return [ sys.executable, "scripts/replay-curtin-log.py", - self.event_file, self.event_syslog_id, + self.event_file, self._event_syslog_id, '.subiquity' + INSTALL_LOG, ] else: @@ -117,7 +130,7 @@ class FailingDryRunCurtinCommandRunner(DryRunCurtinCommandRunner): event_file = 'examples/curtin-events-fail.json' -def get_curtin_command_runner(app, event_syslog_id, config_location=None): +def get_curtin_command_runner(app, config_location=None): if app.opts.dry_run: if 'install-fail' in app.debug_flags: cls = FailingDryRunCurtinCommandRunner @@ -125,4 +138,4 @@ def get_curtin_command_runner(app, event_syslog_id, config_location=None): cls = DryRunCurtinCommandRunner else: cls = CurtinCommandRunner - return cls(app.command_runner, event_syslog_id, config_location) + return cls(app.command_runner, config_location)