autoinstall! #625

mwhudson · 2019-12-20T02:25:16Z

The branch implements the first cut of autoinstall for subiquity. What is there:

support for fully non-interactive and partially interactive installs
support for early and late commands

What is not there:

any sort of decent error handling
- this includes any sort of sensible data validation. Get your autoinstall.yaml wrong and things will just blow up in a mess
support for the packages, debconf-selections, snaplist, user-data sections of the config file
sensible behaviour when more than one subiquity is running (e.g. on tty1 and serial)
generation of an autoinstall file at the end of install
any kind of sensible storage configuration, it just supports lvm or direct installation on the largest disk in the system

But I have plans for all of these things :)

mwhudson · 2019-12-20T02:37:55Z

To test this, try something like:

$ wget https://people.canonical.com/~mwh/subiquity_autoinstall.snap
$ sudo ~/src/subiquity/scripts/inject-subiquity-snap.sh \
    -c 'cp /home/mwhudson/src/subiquity/examples/autoinstall.yaml new_installer/' \
    ~/isos/focal-live-server-amd64.iso  subiquity_autoinstall.snap ~/isos/custom.iso

raharper

I don't see anything that would affect existing interactive installs. I've left some review comments for the autoinstall bits.

raharper · 2020-01-29T17:04:23Z

subiquity/autoinstall.py

+from curtin.config import merge_config
+
+
+def merge_autoinstall_configs(source_paths, target_path):


The order of the source_paths controls what the final config will look like due to how merge_config works with dictionaries and lists. I suggest documenting that behavior. Additionally it's important to retain the order of the configs, or the order of the expected precedence of autoinstall file locations. For example, would you expect the initrd-autoinstall.yaml to override other paths?

Yes, we need to make some decisions and document how all this works. I'm going to post to discource, probably tomorrow, about this.

raharper · 2020-01-29T17:05:42Z

subiquity/autoinstall.py

+    for path in source_paths:
+        with open(path) as fp:
+            c = yaml.safe_load(fp)
+        merge_config(config, c)


I would include a comment or a value in the merged config indicating the source yamls and the order of their merging so that when debugging a resulting autoinstall we can sort out how a value was modified after merging.

raharper · 2020-01-29T17:09:01Z

subiquity/controller.py

+            self.load_autoinstall_data(
+                app.autoinstall_config.get(
+                    self.autoinstall_key,
+                    self.autoinstall_default))


Pre-populating the autoinstall_config dictionary with keys which may be None (due to the default value being set) means that checking for presence of a key w.r.t to whether subiquity uses a default value or the user provided can be difficult.

Yes, there are a few different ways of looking at this. This API seems reasonably ok. load_install_data has access to self.app.autoinstall_config if it needs it...

raharper · 2020-01-29T17:11:11Z

subiquity/controllers/cmdlist.py

+        self.cmds = data
+
+    async def run(self):
+        for i, cmd in enumerate(self.cmds):


This seems to expect that self.cmds will return a list like object; if an autoinstall config does not have any early commands, wont the load_autoinstall_data from the base controller set this to a None which would fail to enumerate?

"autoinstall_default = []" handles this

raharper · 2020-01-29T17:12:06Z

subiquity/controllers/cmdlist.py

+class CmdListController(NoUIController):
+
+    autoinstall_default = []
+    cmds = ()


Should this be a list instead of tuple?

perhaps autoinstall_default should be ()? I don't think it matters a great deal although having them different is indeed odd.

raharper · 2020-01-29T17:17:30Z

subiquity/controllers/mirror.py

+        if data is None:
+            return
+        self.ai_data = data
+        if 'mirror' in data:


This check fails if users don't provide 'mirror' info in the autoinstall, as the loader sets data['mirror'] = None.
Either the loader shouldn't set default value to None so you can do key checks like this, or this needs to be
if data['mirror'] is not None:

I don't understand your comment but I don't think it is correct.

This all needs to change anyway because I want to redo how mirrors are described, see #626 and https://wiki.ubuntu.com/FoundationsTeam/AutomatedServerInstalls/ConfigReference?action=diff&rev2=40&rev1=38

raharper · 2020-01-29T17:19:21Z

subiquity/controllers/refresh.py

+        self.active = self.interactive()
+
+    def load_autoinstall_data(self, data):
+        if data is not None and data.get('refresh'):


data.get('refresh-installer') or 'refresh' ?

'refresh'. The format of the yaml is:

refresh-installer: refresh: yes

raharper · 2020-01-29T17:21:56Z

subiquity/controllers/ssh.py

+        self.model.authorized_keys = self.autoinstall_data.get(
+            'authorized-keys', [])
+        self.model.pwauth = self.autoinstall_data.get(
+            'allow-pw', not self.model.authorized_keys)


Policy decision? Is this documented that one cannot set password and import ssh keys?

Uh, the idea is only that the default of allow-pw is False iff a key is supplied. And I think that's what this code does but I've been thinking about udev too much to be sure.

raharper · 2020-01-29T17:26:51Z

subiquity/controllers/welcome.py

@@ -25,7 +25,11 @@

 class WelcomeController(SubiquityController):

-    model_name = "locale"
+    autoinstall_key = model_name = "locale"
+    autoinstall_default = 'en_US.UTF-8'


C.UTF-8 is the default in cloud-images;

Hmm. subiquity is more targeted towards humans, but maybe not for autoinstall cases. Easy to change later in any case.

raharper · 2020-01-29T17:28:57Z

examples/autoinstall.yaml

+        realname: ''
+        username: ubuntu
+        password: '$6$wdAcoXrU039hKYPd$508Qvbe7ObUnxoj15DRCkzC3qO7edjH0VV7BPNRDYK4QR8ofJaEEF2heacn0QgD.f8pO8SNp83XNdWG6tocBM1'
+        hostname: ubuntu


Does this example include all possible keys? We might want to expand this with comments or examples of other accepted autoinstall keys.

No it certainly does not, and yes providing a more complete example would make sense.

mwhudson · 2020-01-30T03:02:58Z

I don't see anything that would affect existing interactive installs. I've left some review comments for the autoinstall bits.

Thanks for the review. I don't plan to make many changes to this branch in response to them, but I'll keep them in mind for later. I'll probably make some tweaks and merge this tomorrow.

…config

ymmt2005 mentioned this pull request Jan 19, 2020

Ubuntu 20.04 cybozu-go/neco#741

Closed

5 tasks

mwhudson force-pushed the autoinstall-2 branch from 6dd86d1 to 8bb18a1 Compare January 24, 2020 00:16

raharper reviewed Jan 29, 2020

View reviewed changes

mwhudson force-pushed the autoinstall-2 branch from 8bb18a1 to b9c56c7 Compare February 3, 2020 02:35

mwhudson added 12 commits February 4, 2020 13:49

add some example autoinstall files

9db0c18

add --autoinstall command line option

3e729bd

load, merge and write out again the autoinstall configs

e975d18

add controller and application .interactive() methods

671febc

general machinery for autoinstalls

9c59fa5

leave the terminal in text mode and print to it when not interactive

1f4c05a

support early and late commands

99218fb

fill out autoinstall_key everywhere

ce75210

first stab at implementing load_autoinstall_data & apply_autoinstall_…

53e899c

…config

do not listen to udev block events when not interactive

d485a72

reboot after successful autoinstall

e382954

update apt/mirror handling to new schema

db9762b

mwhudson force-pushed the autoinstall-2 branch from b9c56c7 to db9762b Compare February 4, 2020 01:11

mwhudson merged commit e2410a0 into canonical:master Feb 4, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

autoinstall! #625

autoinstall! #625

mwhudson commented Dec 20, 2019

mwhudson commented Dec 20, 2019

raharper left a comment

raharper Jan 29, 2020

mwhudson Jan 30, 2020

raharper Jan 29, 2020

mwhudson Jan 30, 2020

raharper Jan 29, 2020

mwhudson Jan 30, 2020

raharper Jan 29, 2020

mwhudson Jan 30, 2020

raharper Jan 29, 2020

mwhudson Jan 30, 2020

raharper Jan 29, 2020

mwhudson Jan 30, 2020

raharper Jan 29, 2020

mwhudson Jan 30, 2020 •

edited

raharper Jan 29, 2020

mwhudson Jan 30, 2020

raharper Jan 29, 2020

mwhudson Jan 30, 2020

raharper Jan 29, 2020

mwhudson Jan 30, 2020

mwhudson commented Jan 30, 2020

		from curtin.config import merge_config


		def merge_autoinstall_configs(source_paths, target_path):

autoinstall! #625

autoinstall! #625

Conversation

mwhudson commented Dec 20, 2019

mwhudson commented Dec 20, 2019

raharper left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mwhudson Jan 30, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mwhudson commented Jan 30, 2020

mwhudson Jan 30, 2020 •

edited