[libvirt-jenkins-ci PATCH 3/5] guests: Introduce the new 'gitlab' flavor

[Andrea Bolognani]

On Thu, 2020-03-26 at 14:33 +0100, Erik Skultety wrote:
> +++ b/guests/group_vars/all/main.yml
> @@ -5,3 +5,6 @@
>  ansible_ssh_pass: root
>  
>  jenkins_url: https://ci.centos.org/computer/{{ inventory_hostname }}/slave-agent.jnlp
> +
> +# In our case, ansible_system is either Linux or FreeBSD
> +gitlab_runner_url: https://gitlab-runner-downloads.s3.amazonaws.com/latest/binaries/gitlab-runner-{{ ansible_system|lower }}-amd64

Note that these two URLs are semantically quite different:

  * jenkins_url is the endpoint to which the Jenkins agent will
    connect to;

  * gitlab_runner_url is the location from where the gitlab-runner
    binary can be downloaded.

To keep things consistent, we could have

  jenkins_url: https://ci.centos.org/computer/{{ inventory_hostname }}/slave-agent.jnlp
  jenkins_agent_url: https://ci.centos.org/jnlpJars/slave.jar

  gitlab_url: https://gitlab.com
  gitlab_runner_url: https://gitlab-runner-downloads.s3.amazonaws.com/latest/binaries/gitlab-runner-{{ ansible_system|lower }}-amd64

although I'm starting to question why these URLs should even be part
of the inventory when they could just as easily be hardcoded into the
corresponding playbook...

> +++ b/guests/playbooks/update/tasks/gitlab.yml
> @@ -0,0 +1,64 @@
> +---
> +- name: Look up Gitlab runner secret
> +  set_fact:
> +    gitlab_runner_secret: '{{ lookup("file", gitlab_runner_token_file) }}'

The GitLab token should be extracted from the vault, not from a
regular file.

[...]
> +- name: Make sure the gitlab-runner config dir exists exists
> +  file:
> +    path: '{{ gitlab_runner_config_path | dirname }}'
> +    owner: gitlab
> +    group: gitlab
> +    state: directory
> +  register: rc_gitlab_runner_config_dir
> +
> +- name: Create and empty gitlab-runner config
> +  file:
> +    path: '{{ gitlab_runner_config_path }}'
> +    owner: gitlab
> +    group: gitlab
> +    state: touch
> +  when: rc_gitlab_runner_config_dir.changed

Running 'gitlab-runner register' will create the configuration file,
so touching it in advance is unnecessary and...

> +# To ensure idempotency, we must run the registration only when we first
> +# created the config dir, otherwise we'll register a new runner on every
> +# update
> +- name: Register the gitlab-runner agent
> +  shell: 'gitlab-runner register --non-interactive --config /home/gitlab/.gitlab-runner/config.toml --registration-token {{ gitlab_runner_secret }} --url https://gitlab.com --executor shell --tag-list {{ inventory_hostname }}'
> +  when: rc_gitlab_runner_config_dir.changed

... this check could be replaced with

  creates: '{{ gitlab_runner_config_path }}'

Additional comments on this command:

  * you have defined gitlab_runner_config_path earlier, might as well
    use it here;

  * the 'shell' executor is I think our only option when it comes to
    FreeBSD (and we should make sure we fully understand the security
    implications of using it before exposing any of this on the
    internet), but for Linux we should be able to use the 'docker'
    executor instead, which should provide additional isolation;

  * I haven't played around with tags much, but from what I think I
    have understood we will want to have something more generic here,
    such as "freebsd" and "linux", so that we can for example tag
    some of the jobs as "freebsd" and be sure they will run on the
    expected OS;

  * I think we'll want to have the Linux runners configured to accept
    untagged jobs, but the FreeBSD ones only accepting tagged ones
    (again, if I have understood how job tagging works).

[...]
> +- block:
> +    - name: Install the gitlab_runner rc service script
> +      template:
> +        src: '{{ playbook_base }}/templates/gitlab-runner.j2'
> +        dest: '/usr/local/etc/rc.d/gitlab_runner'
> +        mode: '0755'
> +
> +    - name: Enable the gitlab-runner rc service

s/gitlab-runner/gitlab_runner/

though I wonder if we can't just use the same name on both Linux and
FreeBSD? On my FreeBSD machine, while the use of underscore in
service names is clearly the default, there's at least one service
(ftp-proxy) which doesn't follow that convention with (I assume) no
ill effect.

-- 
Andrea Bolognani / Red Hat / Virtualization