Obtain autoinstall from combined-cloud-config #1735
Conversation
dbungert
force-pushed
the
consume-combined-cloud-config
branch
from
ebe3a51
to
650effc
Compare
subiquity/cloudinit.py
Outdated
| """Return the host system /run/cloud-init/combined-cloud-config.json""" | ||
| try: | ||
| with open("/run/cloud-init/combined-cloud-config.json") as fp: | ||
| return json.load(fp) |
There was a problem hiding this comment.
Do we want to log success here so we can track any call-sites since models/network.py also re-reads this file. If we grow lots of readers, we may look toward lru_cache of this function in the future even though it's not very expensive.
| return json.load(fp) | |
| config = json.load(fp) | |
| log.debug("Loaded cloud config from /run/cloud-init/combined-cloud-config.json") | |
| return config |
subiquity/server/server.py
Outdated
|
|
||
| cloud_cfg = get_host_combined_cloud_config() | ||
| if len(cloud_cfg) > 0: | ||
| log.debug("loading cloud config from combined-cloud-config") |
There was a problem hiding this comment.
Do we want to push this message into the get_host_combined_cloud_config() so we can track all callsites that successfully pull this data.
| # Ubuntu 22.04.2 LTS and earlier, presumably) | ||
|
|
||
| cloud_cfg = get_host_combined_cloud_config() | ||
| if len(cloud_cfg) > 0: |
There was a problem hiding this comment.
could just use truthy value of dicts to avoid calculating length of keys and then comparing to 0
| if len(cloud_cfg) > 0: | |
| if cloud_cfg: |
There was a problem hiding this comment.
Yes, but we tend to be more explicit on this point in Subiquity, so I'm matching codebase convention.
subiquity/server/server.py
Outdated
| log.debug("loading cloud config from combined-cloud-config") | ||
| system_info = cloud_cfg.get('system_info', {}) | ||
| default_user = system_info.get('default_user', {}) | ||
| self.installer_user_name = default_user.get('name') |
There was a problem hiding this comment.
I think this makes sense, the only thing I can think is a little advertised user-data can manipulate what the default user is using deprecated keys that could override the configuration present in system_info.
Our example doc on this is below and in doc examples for users and groups
user:
name: mynewdefault
sudo: null
I'll think through this path when the PR is up as this #cloud-config override probably also affects the other logical path you have with stages.Init() too.
There was a problem hiding this comment.
If this is the combined-cloud-config version of ug_util.extract_default, then I think we're set.
The None default user one was rough because it could be encountered with certain cloud-config before we even got as far as showing the UI.
My goal isn't some sort of comprehensive support for weird corner cases here - more along the lines of "don't crash".
dbungert
changed the title
dbungert
force-pushed
the
consume-combined-cloud-config
branch
2 times, most recently
from
2b5e1d7
to
5049f92
Compare
| "Loaded cloud config from " | ||
| "/run/cloud-init/combined-cloud-config.json" | ||
| ) | ||
| return config |
There was a problem hiding this comment.
Is it worth logging something in the "we did not use combined cloud config" case?
subiquity/server/server.py
Outdated
| self.installer_user_name = username | ||
|
|
||
| if username is None: | ||
| if self.installer_user_name is None: |
There was a problem hiding this comment.
nitpick level extreme but why change this to refer to self.installer_user_name when all the other references are still to username?
Attempt to load autoinstall from /run/cloud-init/combined-cloud-config.json first, if present. Fallback to existing methods, which requires that cloud-init in the snap is able to unpickle the data created by cloud-init outside the snap. See also LP: #2022102.
Co-authored-by: Chad Smith <chad.smith@canonical.com>
dbungert
force-pushed
the
consume-combined-cloud-config
branch
from
5049f92
to
c1d91d6
Compare
No description provided.