[libvirt-jenkins-ci PATCH v2 6/6] guests: lcitool: Enable the new 'gitlab' flavor in the lcitool script

[Andrea Bolognani]

On Tue, 2020-04-07 at 13:31 +0200, Erik Skultety wrote:
>          # The vault password is only needed for the jenkins flavor, but in
>          # that case we want to make sure there's *something* in there
> -        if self.get_flavor() != "test":
> +        if self.get_flavor() == "jenkins":
>              vault_pass_file = self._get_config_file("vault-password")

You need something similar to this in your new functions, otherwise
trying even if you've configured lcitool to use the test flavor
you'll still get

  $ ./lcitool update all all
  ./lcitool: Missing or invalid gitlab url file ...

> +    def get_gitlab_runner_token_file(self):
> +        gitlab_runner_token_file = self._get_config_file("gitlab-runner-token")
> +
> +        try:
> +            with open(gitlab_runner_token_file, "r") as infile:
> +                if not infile.readline().strip():
> +                    raise ValueError
> +        except Exception as ex:
> +            raise Exception(
> +                "Missing or invalid gitlab runner token file ({}): {}".format(

Please capitalize GitLab properly in user-visible strings.

> -        extra_vars = json.dumps({
> +        extra_vars_d = {

You don't really need to change the name of the variable.

> +        if flavor == "gitlab":
> +            extra_vars_d.update([
> +                ("gitlab_url_file", gitlab_url_file),
> +                ("gitlab_runner_token_file", gitlab_runner_token_file),
> +            ])

You can use a dict as argument to update(), which is more natural
than a list of tuples.

But in reality I think you can skip the check entirely and add both
keys to extra_vars unconditionally: after you've fixed the issue I
pointed out above, they will either be set (gitlab flavor) or None
(any other flavor), and even in the latter case it's fine to pass
them to Ansible as they will simply not be used.

Everything else looks good.

-- 
Andrea Bolognani / Red Hat / Virtualization