From 996a2125f2e4e83caaa4100912dfd9da41bce11f Mon Sep 17 00:00:00 2001
From: Michael Hudson-Doyle <michael.hudson@canonical.com>
Date: Thu, 4 Nov 2021 09:47:45 +1300
Subject: [PATCH 1/3] pass install source as an argument to curtin install, not
 via config

---
 subiquity/models/source.py               | 11 +++++------
 subiquity/models/tests/test_source.py    | 22 +++++++++++++---------
 subiquity/models/tests/test_subiquity.py |  4 ++--
 subiquity/server/controllers/install.py  | 17 +++++++++++++----
 4 files changed, 33 insertions(+), 21 deletions(-)

diff --git a/subiquity/models/source.py b/subiquity/models/source.py
index de537503..1d766dbc 100644
--- a/subiquity/models/source.py
+++ b/subiquity/models/source.py
@@ -87,7 +87,7 @@ class SourceModel:
         if self.current is None:
             self.current = self.sources[0]
 
-    def render(self):
+    def get_source(self):
         path = os.path.join(self._dir, self.current.path)
         if self.current.preinstalled_langs:
             base, ext = os.path.splitext(path)
@@ -97,8 +97,7 @@ class SourceModel:
                 suffix = 'no-languages'
             path = base + '.' + suffix + ext
         scheme = self.current.type
-        return {
-            'sources': {
-                'ubuntu00': f'{scheme}://{path}'
-                },
-            }
+        return f'{scheme}://{path}'
+
+    def render(self):
+        return {}
diff --git a/subiquity/models/tests/test_source.py b/subiquity/models/tests/test_source.py
index 1663fb52..f74471cd 100644
--- a/subiquity/models/tests/test_source.py
+++ b/subiquity/models/tests/test_source.py
@@ -46,7 +46,7 @@ def make_entry(**fields):
     return raw
 
 
-class TestMirrorModel(unittest.TestCase):
+class TestSourceModel(unittest.TestCase):
 
     def tdir(self):
         tdir = tempfile.mkdtemp()
@@ -88,7 +88,7 @@ class TestMirrorModel(unittest.TestCase):
         self.write_and_load_entries(model, entries)
         self.assertEqual(model.current.id, 'id2')
 
-    def test_render_absolute(self):
+    def test_get_source_absolute(self):
         entry = make_entry(
             type='scheme',
             path='/foo/bar/baz',
@@ -96,9 +96,9 @@ class TestMirrorModel(unittest.TestCase):
         model = SourceModel()
         self.write_and_load_entries(model, [entry])
         self.assertEqual(
-            model.render(), {'sources': {'ubuntu00': 'scheme:///foo/bar/baz'}})
+            model.get_source(), 'scheme:///foo/bar/baz')
 
-    def test_render_relative(self):
+    def test_get_source_relative(self):
         dir = self.tdir()
         entry = make_entry(
             type='scheme',
@@ -107,11 +107,15 @@ class TestMirrorModel(unittest.TestCase):
         model = SourceModel()
         self.write_and_load_entries(model, [entry], dir)
         self.assertEqual(
-            model.render(),
-            {'sources': {'ubuntu00': f'scheme://{dir}/foo/bar/baz'}})
+            model.get_source(),
+            f'scheme://{dir}/foo/bar/baz')
 
-    def test_render_initial(self):
+    def test_get_source_initial(self):
         model = SourceModel()
         self.assertEqual(
-            model.render(),
-            {'sources': {'ubuntu00': 'cp:///media/filesystem'}})
+            model.get_source(),
+            'cp:///media/filesystem')
+
+    def test_render(self):
+        model = SourceModel()
+        self.assertEqual(model.render(), {})
diff --git a/subiquity/models/tests/test_subiquity.py b/subiquity/models/tests/test_subiquity.py
index a4e6b7a2..99cd45cb 100644
--- a/subiquity/models/tests/test_subiquity.py
+++ b/subiquity/models/tests/test_subiquity.py
@@ -172,10 +172,10 @@ class TestSubiquityModel(unittest.TestCase):
         netplan = yaml.safe_load(netplan_content)
         self.assertConfigHasVal(netplan, 'network.version', 2)
 
-    def test_has_sources(self):
+    def test_sources(self):
         model = self.make_model()
         config = model.render()
-        self.assertIn('sources', config)
+        self.assertNotIn('sources', config)
 
     def test_mirror(self):
         model = self.make_model()
diff --git a/subiquity/server/controllers/install.py b/subiquity/server/controllers/install.py
index 070122b9..d2955e7b 100644
--- a/subiquity/server/controllers/install.py
+++ b/subiquity/server/controllers/install.py
@@ -19,11 +19,12 @@ import os
 import re
 import shutil
 
+from curtin.commands.extract import get_handler_for_source
 from curtin.commands.install import (
     ERROR_TARFILE,
     INSTALL_LOG,
     )
-from curtin.util import write_file
+from curtin.util import sanitize_source, write_file
 
 import yaml
 
@@ -130,9 +131,9 @@ class InstallController(SubiquityController):
 
     @with_context(
         description="installing system", level="INFO", childlevel="DEBUG")
-    async def curtin_install(self, *, context):
+    async def curtin_install(self, *, context, source):
         await run_curtin_command(
-            self.app, context, 'install', config=self.write_config())
+            self.app, context, 'install', source, config=self.write_config())
 
     @with_context()
     async def install(self, *, context):
@@ -154,11 +155,19 @@ class InstallController(SubiquityController):
 
             self.app.update_state(ApplicationState.RUNNING)
 
+            handler = get_handler_for_source(
+                sanitize_source(self.model.source.get_source()))
+            if self.app.opts.dry_run:
+                path = '/'
+            else:
+                path = handler.setup()
+
             if os.path.exists(self.model.target):
                 await self.unmount_target(
                     context=context, target=self.model.target)
 
-            await self.curtin_install(context=context)
+            await self.curtin_install(
+                context=context, source='cp://' + path)
 
             self.app.update_state(ApplicationState.POST_WAIT)
 

From be57dc9da03ea91651418c2d2aae7a6305bccf13 Mon Sep 17 00:00:00 2001
From: Michael Hudson-Doyle <michael.hudson@canonical.com>
Date: Wed, 17 Nov 2021 11:20:18 +1300
Subject: [PATCH 2/3] fix offline installs

---
 snapcraft.yaml          | 2 +-
 subiquity/server/apt.py | 4 +++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/snapcraft.yaml b/snapcraft.yaml
index 71e8ccdf..3388c16c 100644
--- a/snapcraft.yaml
+++ b/snapcraft.yaml
@@ -42,7 +42,7 @@ parts:
     plugin: python
     source-type: git
     source: https://git.launchpad.net/curtin
-    source-commit: 809817fdac257de1769e385a42aeaf3d5bcc2c60
+    source-commit: f13edc1ac71dde678149b4918ee923b25a1b7039
     build-packages:
       - shared-mime-info
       - zlib1g-dev
diff --git a/subiquity/server/apt.py b/subiquity/server/apt.py
index 88ca5030..ef6564f8 100644
--- a/subiquity/server/apt.py
+++ b/subiquity/server/apt.py
@@ -126,7 +126,9 @@ class AptConfigurer:
                 self.tpath('etc/apt/sources.list'),
                 self.tpath('etc/apt/sources.list.d/original.list'))
         else:
-            os.unlink(self.tpath('etc/apt/apt.conf.d/90curtin-aptproxy'))
+            proxy_path = self.tpath('etc/apt/apt.conf.d/90curtin-aptproxy')
+            if os.path.exists(proxy_path):
+                os.unlink(proxy_path)
             await self.setup_overlay(self.tpath('var/lib/apt/lists'))
 
         codename = lsb_release()['codename']

From f2e05f993dcc51f5a96ac61acdda28b5726fae79 Mon Sep 17 00:00:00 2001
From: Michael Hudson-Doyle <michael.hudson@canonical.com>
Date: Thu, 4 Nov 2021 10:08:24 +1300
Subject: [PATCH 3/3] do apt configuration before install starts

via lots of overlays
---
 bin/subiquity-configure-apt             |   3 -
 setup.py                                |   1 -
 subiquity/common/apidef.py              |   5 -
 subiquity/models/subiquity.py           |   6 +-
 subiquity/server/apt.py                 | 144 ++++++++++++++++--------
 subiquity/server/controllers/install.py |  24 ++--
 6 files changed, 112 insertions(+), 71 deletions(-)
 delete mode 100755 bin/subiquity-configure-apt

diff --git a/bin/subiquity-configure-apt b/bin/subiquity-configure-apt
deleted file mode 100755
index 3814712e..00000000
--- a/bin/subiquity-configure-apt
+++ /dev/null
@@ -1,3 +0,0 @@
-#!/bin/bash -eux
-
-curl --fail --unix-socket /run/subiquity/socket --data '' a/install/configure_apt
diff --git a/setup.py b/setup.py
index f8b5cebc..94354426 100644
--- a/setup.py
+++ b/setup.py
@@ -108,7 +108,6 @@ setup(name='subiquity',
           'bin/console-conf-wait',
           'bin/console-conf-wrapper',
           'bin/subiquity-debug',
-          'bin/subiquity-configure-apt',
           'bin/subiquity-loadkeys',
           'bin/subiquity-service',
           'bin/subiquity-server',
diff --git a/subiquity/common/apidef.py b/subiquity/common/apidef.py
index b3a5be50..de9c932c 100644
--- a/subiquity/common/apidef.py
+++ b/subiquity/common/apidef.py
@@ -294,11 +294,6 @@ class API:
         def GET() -> TimeZoneInfo: ...
         def POST(tz: str): ...
 
-    class install:
-        class configure_apt:
-            def POST():
-                "Not for client use."
-
     class shutdown:
         def POST(mode: ShutdownMode, immediate: bool = False): ...
 
diff --git a/subiquity/models/subiquity.py b/subiquity/models/subiquity.py
index 0e5dce12..d41d6088 100644
--- a/subiquity/models/subiquity.py
+++ b/subiquity/models/subiquity.py
@@ -26,7 +26,7 @@ from curtin.config import merge_config
 
 from subiquitycore.file_util import write_file
 
-from subiquity.common.resources import resource_path, get_users_and_groups
+from subiquity.common.resources import get_users_and_groups
 from subiquity.server.types import InstallerChannels
 
 from .filesystem import FilesystemModel
@@ -335,8 +335,8 @@ class SubiquityModel:
             'stages': stages,
 
             'curthooks_commands': {
-                '001-configure-apt': [
-                    resource_path('bin/subiquity-configure-apt'),
+                '001-mount-cdrom': [
+                    'mount', '--bind', '/cdrom', '/target/cdrom',
                     ],
                 },
 
diff --git a/subiquity/server/apt.py b/subiquity/server/apt.py
index ef6564f8..2f9536aa 100644
--- a/subiquity/server/apt.py
+++ b/subiquity/server/apt.py
@@ -23,15 +23,17 @@ from curtin.util import write_file
 import yaml
 
 from subiquitycore.lsb_release import lsb_release
+from subiquitycore.utils import arun_command
 
 from subiquity.server.curtin import run_curtin_command
 
 
 class AptConfigurer:
 
-    def __init__(self, app, target):
+    def __init__(self, app, source):
         self.app = app
-        self.target = target
+        self.source = source
+        self.configured = None
         self._mounts = []
         self._tdirs = []
 
@@ -40,9 +42,6 @@ class AptConfigurer:
         self._tdirs.append(d)
         return d
 
-    def tpath(self, *args):
-        return os.path.join(self.target, *args)
-
     async def mount(self, device, mountpoint, options=None, type=None):
         opts = []
         if options is not None:
@@ -56,47 +55,53 @@ class AptConfigurer:
     async def unmount(self, mountpoint):
         await self.app.command_runner.run(['umount', mountpoint])
 
-    async def setup_overlay(self, dir):
+    async def setup_overlay(self, source, target):
         tdir = self.tdir()
         w = f'{tdir}/work'
         u = f'{tdir}/upper'
         for d in w, u:
             os.mkdir(d)
         await self.mount(
-            'overlay', dir, type='overlay',
-            options=f'lowerdir={dir},upperdir={u},workdir={w}')
+            'overlay', target, type='overlay',
+            options=f'lowerdir={source},upperdir={u},workdir={w}')
+        return u
 
     async def configure(self, context):
         # Configure apt so that installs from the pool on the cdrom are
         # preferred during installation but not in the installed system.
         #
-        # This has a few steps.
+        # First we create an overlay ('configured') over the installation
+        # source and configure that overlay as we want the target system to
+        # end up by running curtin's apt-config subcommand.
         #
-        # 1. As the remaining steps mean that any changes to apt configuration
-        #    are do not persist into the installed system, we get curtin to
-        #    configure apt a bit earlier than it would by default.
+        # Then we create a fresh overlay ('for_install') over the first one
+        # and configure it for the installation. This means:
         #
-        # 2. Bind-mount the cdrom into the installed system as /cdrom.
+        # 1. Bind-mounting /cdrom into this new overlay.
         #
-        # 3. Set up an overlay over /target/etc/apt. This means that any
-        #    changes we make will not persist into the installed system and we
-        #    do not have to worry about cleaning up after ourselves.
+        # 2. When the network is expected to be working, copying the original
+        #    /etc/apt/sources.list to /etc/apt/sources.list.d/original.list.
         #
-        # 4. Configure apt in /target to look at the pool on the cdrom.  This
-        #    has two subcases:
+        # 3. writing "deb file:///cdrom $(lsb_release -sc) main restricted"
+        #    to /etc/apt/sources.list.
         #
-        #     a. if we expect the network to be working, this basically means
-        #        prepending
-        #        "deb file:///run/cdrom $(lsb_release -sc) main restricted"
-        #        to the sources.list file.
+        # 4. running "apt-get update" in the new overlay.
         #
-        #     b. if the network is not expected to be working, we replace the
-        #        sources.list with a file just referencing the cdrom.
+        # When the install is done we try to make the installed system's apt
+        # state look as if the pool had never been configured. So this means:
         #
-        # 5. If the network is not expected to be working, we also set up an
-        #    overlay over /target/var/lib/apt/lists (if the network is working,
-        #    we'll run "apt update" after the /target/etc/apt overlay has been
-        #    cleared).
+        # 1. Removing /cdrom from the installed system.
+        #
+        # 2. Copying /etc/apt from the 'configured' overlay to the installed
+        #    system.
+        #
+        # 3. If the network is working, run apt-get update in the installed
+        #    system, or if it is not, just copy /var/lib/apt/lists from the
+        #    'configured' overlay.
+
+        self.configured = self.tdir()
+
+        config_upper = await self.setup_overlay(self.source, self.configured)
 
         config = {
             'apt': self.app.base_model.mirror.config,
@@ -104,52 +109,93 @@ class AptConfigurer:
         config_location = os.path.join(
             self.app.root, 'var/log/installer/subiquity-curtin-apt.conf')
 
-        with open(config_location, 'w') as conf:
-            datestr = '# Autogenerated by Subiquity: {} UTC\n'.format(
-                str(datetime.datetime.utcnow()))
-            conf.write(datestr)
-            conf.write(yaml.dump(config))
+        datestr = '# Autogenerated by Subiquity: {} UTC\n'.format(
+            str(datetime.datetime.utcnow()))
+        write_file(config_location, datestr + yaml.dump(config))
 
         self.app.note_data_for_apport("CurtinAptConfig", config_location)
 
         await run_curtin_command(
-            self.app, context, 'apt-config', '-t', self.tpath(),
+            self.app, context, 'apt-config', '-t', self.configured,
             config=config_location)
 
-        await self.setup_overlay(self.tpath('etc/apt'))
+        for_install = self.tdir()
+        await self.setup_overlay(config_upper + ':' + self.source, for_install)
 
-        os.mkdir(self.tpath('cdrom'))
-        await self.mount('/cdrom', self.tpath('cdrom'), options='bind')
+        os.mkdir(f'{for_install}/cdrom')
+        await self.mount('/cdrom', f'{for_install}/cdrom', options='bind')
 
         if self.app.base_model.network.has_network:
             os.rename(
-                self.tpath('etc/apt/sources.list'),
-                self.tpath('etc/apt/sources.list.d/original.list'))
+                f'{for_install}/etc/apt/sources.list',
+                f'{for_install}/etc/apt/sources.list.d/original.list')
         else:
-            proxy_path = self.tpath('etc/apt/apt.conf.d/90curtin-aptproxy')
+            proxy_path = f'{for_install}/etc/apt/apt.conf.d/90curtin-aptproxy'
             if os.path.exists(proxy_path):
                 os.unlink(proxy_path)
-            await self.setup_overlay(self.tpath('var/lib/apt/lists'))
 
         codename = lsb_release()['codename']
 
         write_file(
-            self.tpath('etc/apt/sources.list'),
-            f'deb [check-date=no] file:///cdrom {codename} main restricted\n',
-            )
+            f'{for_install}/etc/apt/sources.list',
+            f'deb [check-date=no] file:///cdrom {codename} main restricted\n')
 
         await run_curtin_command(
-            self.app, context, "in-target", "-t", self.tpath(),
+            self.app, context, "in-target", "-t", for_install,
             "--", "apt-get", "update")
 
-    async def deconfigure(self, context):
+        return for_install
+
+    async def deconfigure(self, context, target):
+        await self.unmount(f'{target}/cdrom')
+        os.rmdir(f'{target}/cdrom')
+
+        restore_dirs = ['etc/apt']
+        if not self.app.base_model.network.has_network:
+            restore_dirs.append('var/lib/apt/lists')
+        for dir in restore_dirs:
+            shutil.rmtree(f'{target}/{dir}')
+            await self.app.command_runner.run([
+                'cp', '-aT', f'{self.configured}/{dir}', f'{target}/{dir}',
+                ])
+
+        if self.app.base_model.network.has_network:
+            await run_curtin_command(
+                self.app, context, "in-target", "-t", target,
+                "--", "apt-get", "update")
+
         for m in reversed(self._mounts):
             await self.unmount(m)
         for d in self._tdirs:
             shutil.rmtree(d)
-        if not self.app.opts.dry_run:
-            os.rmdir(self.tpath('cdrom'))
         if self.app.base_model.network.has_network:
             await run_curtin_command(
-                self.app, context, "in-target", "-t", self.tpath(),
+                self.app, context, "in-target", "-t", target,
                 "--", "apt-get", "update")
+
+
+class DryRunAptConfigurer(AptConfigurer):
+
+    async def setup_overlay(self, source, target):
+        if source.startswith('u+'):
+            # Please excuse the obscure way the path is transmitted
+            # from the first invocation of this method to the second :/
+            source = source.split(':')[0][2:]
+        os.mkdir(f'{target}/etc')
+        await arun_command([
+            'cp', '-aT', f'{source}/etc/apt', f'{target}/etc/apt',
+            ], check=True)
+        if os.path.isdir(f'{target}/etc/apt/sources.list.d'):
+            shutil.rmtree(f'{target}/etc/apt/sources.list.d')
+        os.mkdir(f'{target}/etc/apt/sources.list.d')
+        return 'u+' + target
+
+    async def deconfigure(self, context, target):
+        return
+
+
+def get_apt_configurer(app, source):
+    if app.opts.dry_run:
+        return DryRunAptConfigurer(app, source)
+    else:
+        return AptConfigurer(app, source)
diff --git a/subiquity/server/controllers/install.py b/subiquity/server/controllers/install.py
index d2955e7b..11b37653 100644
--- a/subiquity/server/controllers/install.py
+++ b/subiquity/server/controllers/install.py
@@ -33,7 +33,6 @@ from subiquitycore.async_helpers import (
     )
 from subiquitycore.context import with_context
 
-from subiquity.common.apidef import API
 from subiquity.common.errorreport import ErrorReportKind
 from subiquity.common.types import (
     ApplicationState,
@@ -41,7 +40,7 @@ from subiquity.common.types import (
 from subiquity.journald import (
     journald_listen,
     )
-from subiquity.server.apt import AptConfigurer
+from subiquity.server.apt import get_apt_configurer
 from subiquity.server.controller import (
     SubiquityController,
     )
@@ -74,8 +73,6 @@ class TracebackExtractor:
 
 class InstallController(SubiquityController):
 
-    endpoint = API.install
-
     def __init__(self, app):
         super().__init__(app)
         self.model = app.base_model
@@ -83,7 +80,7 @@ class InstallController(SubiquityController):
         self.unattended_upgrades_cmd = None
         self.unattended_upgrades_ctx = None
         self.tb_extractor = TracebackExtractor()
-        self.apt_configurer = AptConfigurer(self.app, self.tpath())
+        self.apt_configurer = None
 
     def interactive(self):
         return True
@@ -129,6 +126,11 @@ class InstallController(SubiquityController):
         if not self.app.opts.dry_run:
             shutil.rmtree(target)
 
+    @with_context(
+        description="configuring apt", level="INFO", childlevel="DEBUG")
+    async def configure_apt(self, *, context):
+        return await self.apt_configurer.configure(context)
+
     @with_context(
         description="installing system", level="INFO", childlevel="DEBUG")
     async def curtin_install(self, *, context, source):
@@ -157,17 +159,22 @@ class InstallController(SubiquityController):
 
             handler = get_handler_for_source(
                 sanitize_source(self.model.source.get_source()))
+
             if self.app.opts.dry_run:
                 path = '/'
             else:
                 path = handler.setup()
 
+            self.apt_configurer = get_apt_configurer(self.app, path)
+
+            for_install_path = await self.configure_apt()
+
             if os.path.exists(self.model.target):
                 await self.unmount_target(
                     context=context, target=self.model.target)
 
             await self.curtin_install(
-                context=context, source='cp://' + path)
+                context=context, source='cp://' + for_install_path)
 
             self.app.update_state(ApplicationState.POST_WAIT)
 
@@ -223,7 +230,7 @@ class InstallController(SubiquityController):
 
     @with_context(description="restoring apt configuration")
     async def restore_apt_config(self, context):
-        await self.apt_configurer.deconfigure(context)
+        await self.apt_configurer.deconfigure(context, self.tpath())
 
     @with_context(description="downloading and installing {policy} updates")
     async def run_unattended_upgrades(self, context, policy):
@@ -263,9 +270,6 @@ class InstallController(SubiquityController):
                self.unattended_upgrades_cmd is not None:
                 self.unattended_upgrades_cmd.proc.terminate()
 
-    async def configure_apt_POST(self, context):
-        await self.apt_configurer.configure(context)
-
 
 uu_apt_conf = b"""\
 # Config for the unattended-upgrades run to avoid failing on battery power or