I’ve looked at the source code, and I see that you’re using curtin’s merge_config to merge the configuration generated by subiquity with supplied user-data (if present).
merge_config is smart about merging dictionaries, but doesn’t do anything special about the lists. What if we merge lists as well, instead of overwriting them?
Please note that the patch below is just a suggestion, I haven’t tested it yet. I have a couple of concerns too:
- Could we break something by merging lists? While beneficial for stuff like users and groups, it may perhaps corrupt other configuration items? This is curtin territory after all.
- While subiquity can update itself during the installation, I imagine curtin can’t?
-
In curtin, we add support for lists by extending them:
diff --git a/curtin/config.py b/curtin/config.py index 2106b239..93695c87 100644 --- a/curtin/config.py +++ b/curtin/config.py @@ -33,6 +33,8 @@ def merge_config(cfg, cfg2): for k, v in cfg2.items(): if isinstance(v, dict) and isinstance(cfg.get(k, None), dict): merge_config(cfg[k], v) + elif isinstance(v, list) and isinstance(cfg.get(k, None), list): + cfg[k].extend(v) else: cfg[k] = v -
In subiquity, we merge user-data into subiquity’s config, not vice versa. This will ensure, for instance, that the default system user (if defined) will come first and have UID 1000. And minus one deep copy, which is also nice.
diff --git a/subiquity/models/subiquity.py b/subiquity/models/subiquity.py index 43814794..ff6d5d7c 100644 --- a/subiquity/models/subiquity.py +++ b/subiquity/models/subiquity.py @@ -193,9 +193,8 @@ class SubiquityModel: config['snap'] = { 'commands': cmds, } - userdata = copy.deepcopy(self.userdata) - merge_config(userdata, config) - return userdata + merge_config(config, self.userdata) + return config def _cloud_init_files(self): # TODO, this should be moved to the in-target cloud-config seed so on