Re: [libvirt-ci PATCH 10/13] lcitool: Use d.update() on extra_vars for options coming from the config
On Wed, 2020-04-22 at 15:28 +0200, Erik Skultety wrote:
+ # start with generic items not coming from the config
extra_vars = {
"base": base,
"playbook_base": playbook_base,
- "root_password":
self._config.dict["install"]["root_password"],
- "flavor":
self._config.dict["install"]["flavor"],
"selected_projects": selected_projects,
"git_remote": git_remote,
"git_branch": git_branch,
- "gitlab_url":
self._config.dict["gitlab"]["url"],
- "gitlab_runner_secret":
self._config.dict["gitlab"]["token"],
}
+
+ # now add the config vars
+ extra_vars.update(self._config.dict["install"])
+
+ if extra_vars["flavor"] == "gitlab":
+ extra_vars.update(self._config.dict["gitlab"])
+
Mh, I don't think I like this.
First of all, it forces you to replicate the "if gitlab" conditional
in one additional place, and most importantly it completely flattens
the resulting dictionary, eg.
config.dict["gitlab"]["url"] -> extra_vars["url"]
Both of these issues will disappear if you follow the suggestion I
made in 6/13 and load the default config.toml first: at that point,
you can unconditionally copy entire sections from the TOML into
extra_vars, and later in the playbook you can simply refer to them
along the lines of
- shell: gitlab-runner {{ gitlab.url }} {{ gitlab.runner_secret }}
when:
- install.flavor == "gitlab"
which is much nicer.
In fact, we should use the same approach for values in the
inventory, eg.
os_name: "Debian"
os_version: "10"
should become
os:
name: "Debian"
version: "10"
Given that both Python and Ansible support proper dictionaries, we
might very well use them instead of using prefix notation to emulate
a worse version of them :)
--
Andrea Bolognani / Red Hat / Virtualization