[WIP] Refactor cloud init config #364
Conversation
Cristi1324
force-pushed
the
refactor-cloud-init
branch
from
b892b9d to
175c87e
Compare
coriolis/osmorphing/debian.py
Outdated
| @@ -97,6 +97,12 @@ def _compose_netplan_cfg(self, nics_info): | |||
| } | |||
| return yaml.dump(cfg, default_flow_style=False) | |||
|
|
|||
| def _has_systemd_chroot(self): | |||
| if self._test_path("/lib/systemd/system"): | |||
There was a problem hiding this comment.
Why not just return self._test_path('/lib/systemd/system')? :D
coriolis/osmorphing/base.py
Outdated
| cloud_cfg_path = 'etc/cloud/cloud.cfg' | ||
| if not self._test_path(cloud_cfg_path): | ||
| return DEFAULT_CLOUD_USER | ||
| cloud_cfg_content = self._read_file(cloud_cfg_path) |
There was a problem hiding this comment.
Please use _read_file_sudo here.
coriolis/osmorphing/base.py
Outdated
| self._ensure_cloud_init_enabled() | ||
| self._reset_cloud_init_run() | ||
| if self._osmorphing_parameters.get('retain_user_credentials', False): | ||
| self._configure_cloud_init_user_retention() |
There was a problem hiding this comment.
This method overwrites cloud.cfg. I'd like to add those mods into a separate file called 99_coriolis.cfg inside cloud.cfg.d. Save them inside a cloud_cfg_mods and only at the end of this _configure_cloud_init method you'd write that file.
Also, here is what the set_dhcp mods should also be included.
Cristi1324
force-pushed
the
refactor-cloud-init
branch
3 times, most recently
from
c698ff2 to
9219663
Compare
coriolis/osmorphing/base.py
Outdated
| 'ssh_pwauth': True, | ||
| 'users': None | ||
| } | ||
| cloud_cfg_mods = {**cloud_cfg_mods, |
There was a problem hiding this comment.
NIT: I think it's cleaner if you use update()
| cloud_cfg_mods = {**cloud_cfg_mods, | |
| cloud_cfg_mods.update(cloud_cfg_user_retention) |
coriolis/osmorphing/base.py
Outdated
|
|
||
| if not self._osmorphing_parameters.get('set_dhcp', False): | ||
| disabled_network_config = {"network": {"config": "disabled"}} | ||
| cloud_cfg_mods = {**cloud_cfg_mods, **disabled_network_config} |
Cristi1324
force-pushed
the
refactor-cloud-init
branch
from
9219663 to
07eba4e
Compare
|
LGTM. Please update tests as well. Feel free to open up the PR as well |
coriolis/osmorphing/base.py
Outdated
| cloud_cfg_path)) | ||
| continue | ||
| "Configuring cloud-init additional options") | ||
| new_cloud_cfg = yaml.dump(cloud_cfg) |
There was a problem hiding this comment.
Please always use safe loaders and dumpers when using yaml module.
I.E.: yaml.load(content, Loader=yaml.SafeLoader)
| new_cloud_cfg = yaml.dump(cloud_cfg) | |
| new_cloud_cfg = yaml.dump(cloud_cfg. Dumper=yaml.SafeDumper) |
coriolis/osmorphing/base.py
Outdated
| LOG.warn("Could not find %s. Skipping reconfiguration." % ( | ||
| cloud_cfg_path)) | ||
| continue | ||
| "Configuring cloud-init additional options") |
There was a problem hiding this comment.
NIT: I think this sounds better
| "Configuring cloud-init additional options") | |
| "Customizing cloud-init configuration") |
coriolis/osmorphing/base.py
Outdated
| cloud_cfg_paths.append("%s/%s" % (cloud_cfgs_dir, path)) | ||
|
|
||
| if not self._test_path(cloud_cfgs_dir): | ||
| exception.CoriolisException("Could not find %s" % cloud_cfgs_dir) |
There was a problem hiding this comment.
I think it'd be better to just create this directory instead of straight out erroring out. Cloud-init will look for this location regardless, it's not customizable afaik. (if it is, find a way to locate it)
At least execute: mkdir -p /etc/cloud/cloud.cfg.d
coriolis/osmorphing/base.py
Outdated
| installer_config_path = "/etc/cloud/cloud.cfg.d/99-installer.cfg" | ||
| if self._test_path(installer_config_path): | ||
| self._event_manager.progress_update( | ||
| "Disabling Ubuntu installer-generated cloud-config") |
There was a problem hiding this comment.
Since this is in base now, I think you should drop the Ubuntu
| self._exec_cmd_chroot(f"rm {disabler_file}") | ||
| if self._test_path(system_conf_disabler): | ||
| self._exec_cmd_chroot( | ||
| "sed -i '/cloud-init=disabled/d' %s" % system_conf_disabler) |
There was a problem hiding this comment.
I'm not sure this will work as you expect it, it could just leave an empty variable and maybe even bork the system.conf file. If you haven't already, please do a test and see if it gets enabled and systemd doesn't just fall on its head.
Let me know if you already tested this and it works.
| "sed -i '/cloud-init=disabled/d' %s" % system_conf_disabler) | ||
| if self._test_path(grub_conf_disabler): | ||
| self._exec_cmd_chroot( | ||
| "sed -i '/cloud-init=disabled/d' %s" % grub_conf_disabler) |
There was a problem hiding this comment.
This I know for sure will not work, because any grub update needs an update-grub or equivalent (redhat-derivates have a different command).
In some cases, this grub update may not happen, so we need to also run the grub update command here.
I think for both cases (grub and system.conf), you'll need to first grep and then do sed and/or update commands.
coriolis/osmorphing/base.py
Outdated
| else: | ||
| self._create_cloudinit_user() | ||
|
|
||
| if not self._osmorphing_parameters.get('set_dhcp', False): |
There was a problem hiding this comment.
Please set set_dhcp's default to True. Otherwise, in providers where we do not support this option, it will always disable cloud-init networking.
| if not self._osmorphing_parameters.get('set_dhcp', False): | |
| if not self._osmorphing_parameters.get('set_dhcp', True): |
coriolis/osmorphing/redhat.py
Outdated
| package_names | ||
| ) |
There was a problem hiding this comment.
NIT:
| package_names | |
| ) | |
| package_names) |
coriolis/osmorphing/suse.py
Outdated
| self._run_dracut() | ||
| self._configure_cloud_init() |
There was a problem hiding this comment.
NIT for consistency's sake:
| self._run_dracut() | |
| self._configure_cloud_init() | |
| self._configure_cloud_init() | |
| self._run_dracut() |
Cristi1324
force-pushed
the
refactor-cloud-init
branch
from
07eba4e to
0cc1819
Compare
|
LGTM. Please update tests. Feel free to open up the PR as well |
No description provided.