With the recent efforts in upstream libvirt to centralize our CI on
gitlab, let's add a new gitlab-specific flavor along with related
playbook tasks. This flavour revolves around installing and configuring
the gitlab-runner agent binary which requires the per-project
registration token to be specified in order for the runner to be
successfully registered with the gitlab server.

Note that as part of the registration process each runner acquires a new
unique access token. This means that we must ensure that the
registration is run only on the first update, otherwise a new runner
with a new access token is registered with the gitlab project.

Signed-off-by: Erik Skultety <eskultet@redhat.com>
---
 guests/playbooks/update/main.yml         |  5 ++
 guests/playbooks/update/tasks/gitlab.yml | 58 ++++++++++++++++++++++++
 2 files changed, 63 insertions(+)
 create mode 100644 guests/playbooks/update/tasks/gitlab.yml

diff --git a/guests/playbooks/update/main.yml b/guests/playbooks/update/main.yml
index a5a4de8..371e53d 100644
--- a/guests/playbooks/update/main.yml
+++ b/guests/playbooks/update/main.yml
@@ -58,3 +58,8 @@
     - include: '{{ playbook_base }}/tasks/jenkins.yml'
       when:
         - flavor == 'jenkins'
+
+    # Install the Gitlab runner agent
+    - include: '{{ playbook_base }}/tasks/gitlab.yml'
+      when:
+        - flavor == 'gitlab'
diff --git a/guests/playbooks/update/tasks/gitlab.yml b/guests/playbooks/update/tasks/gitlab.yml
new file mode 100644
index 0000000..1f75d98
--- /dev/null
+++ b/guests/playbooks/update/tasks/gitlab.yml
@@ -0,0 +1,58 @@
+---
+- name: Define gitlab-related facts
+  set_fact:
+    gitlab_url: '{{ lookup("file", gitlab_url_file) }}'
+    gitlab_runner_secret: '{{ lookup("file", gitlab_runner_token_file) }}'
+    gitlab_runner_download_url: https://gitlab-runner-downloads.s3.amazonaws.com/latest/binaries/gitlab-runner-{{ ansible_system|lower }}-amd64
+    gitlab_runner_config_dir: '/etc/gitlab-runner'
+
+- name: Download gitlab-runner agent
+  get_url:
+    url: '{{ gitlab_runner_download_url }}'
+    dest: /usr/local/bin/gitlab-runner
+    mode: '0755'
+    force: yes
+
+- name: Register the gitlab-runner agent
+  shell: 'gitlab-runner register --non-interactive --config "{{ gitlab_runner_config_dir }}/config.toml" --registration-token "{{ gitlab_runner_secret }}" --url "{{ gitlab_url }}" --executor shell --tag-list "{{ os_name|lower }}-{{ os_version }}"'
+  args:
+    creates: '{{ gitlab_runner_config_dir }}/config.toml'
+
+- name: Make {{ gitlab_runner_config_dir }} world readable
+  file:
+    path: '{{ gitlab_runner_config_dir }}'
+    mode: '0755'
+
+- name: Make {{ gitlab_runner_config_dir }}/config.toml world readable
+  file:
+    path: '{{ gitlab_runner_config_dir }}/config.toml'
+    mode: '0644'
+
+- block:
+    - name: Install the gitlab-runner service unit
+      template:
+        src: '{{ playbook_base }}/templates/gitlab-runner.service.j2'
+        dest: /etc/systemd/system/gitlab-runner.service
+
+    - name: Enable the gitlab-runner service
+      systemd:
+        name: gitlab-runner
+        state: started
+        enabled: yes
+        daemon_reload: yes
+  when: ansible_service_mgr == 'systemd'
+
+- 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
+      service:
+        name: gitlab_runner
+        state: started
+        enabled: yes
+  when: ansible_service_mgr != 'systemd'
+
-- 
2.25.1

On Thu, 2020-04-09 at 06:23 +0200, Erik Skultety wrote:
> +++ b/guests/playbooks/update/tasks/gitlab.yml
> +- name: Make {{ gitlab_runner_config_dir }} world readable
> +  file:
> +    path: '{{ gitlab_runner_config_dir }}'
> +    mode: '0755'
> +
> +- name: Make {{ gitlab_runner_config_dir }}/config.toml world readable
> +  file:
> +    path: '{{ gitlab_runner_config_dir }}/config.toml'
> +    mode: '0644'

The message for these tasks is unnecessarily detailed: I'd just use
something like

  Make gitlab-runner configuration readable

for both.

Additionally, even though the gitlab user is going to be the only one
on the system so it doesn't make much of a difference in practice, I
think we should have config.toml

  owner: root
  group: gitlab
  mode: '0640'

> +- 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
> +      service:
> +        name: gitlab_runner
> +        state: started
> +        enabled: yes
> +  when: ansible_service_mgr != 'systemd'
> +

This blank line is unnecessary and 'git am' complains about it:

  .git/rebase-apply/patch:83: new blank line at EOF.
  +

With that taken care of and config.toml's permissions adjusted,

  Reviewed-by: Andrea Bolognani <abologna@redhat.com>

-- 
Andrea Bolognani / Red Hat / Virtualization

On Thu, Apr 09, 2020 at 12:28:50PM +0200, Andrea Bolognani wrote:
> On Thu, 2020-04-09 at 06:23 +0200, Erik Skultety wrote:
> > +++ b/guests/playbooks/update/tasks/gitlab.yml
> > +- name: Make {{ gitlab_runner_config_dir }} world readable
> > +  file:
> > +    path: '{{ gitlab_runner_config_dir }}'
> > +    mode: '0755'
> > +
> > +- name: Make {{ gitlab_runner_config_dir }}/config.toml world readable
> > +  file:
> > +    path: '{{ gitlab_runner_config_dir }}/config.toml'
> > +    mode: '0644'
>
> The message for these tasks is unnecessarily detailed: I'd just use
> something like
>
>   Make gitlab-runner configuration readable

Okay, however...

>
> for both.
>
> Additionally, even though the gitlab user is going to be the only one
> on the system so it doesn't make much of a difference in practice, I
> think we should have config.toml
>

...here you suggest the following adjustment. I feel like the messages above
will then become confusing and misleading, since who are we making it readable
for? Well, only for the gitlab user, so I think a little more detail in them is
justifiable.

>   owner: root
>   group: gitlab
>   mode: '0640'

So how about:
"Make gitlab-runner config dir readable" for the former and
"Make gitlab-runner config.toml owned by the gitlab group" for the latter

--
Erik Skultety

On Tue, 2020-04-14 at 10:17 +0200, Erik Skultety wrote:
> On Thu, Apr 09, 2020 at 12:28:50PM +0200, Andrea Bolognani wrote:
> > On Thu, 2020-04-09 at 06:23 +0200, Erik Skultety wrote:
> > > +++ b/guests/playbooks/update/tasks/gitlab.yml
> > > +- name: Make {{ gitlab_runner_config_dir }} world readable
> > > +  file:
> > > +    path: '{{ gitlab_runner_config_dir }}'
> > > +    mode: '0755'
> > > +
> > > +- name: Make {{ gitlab_runner_config_dir }}/config.toml world readable
> > > +  file:
> > > +    path: '{{ gitlab_runner_config_dir }}/config.toml'
> > > +    mode: '0644'
> > 
> > The message for these tasks is unnecessarily detailed: I'd just use
> > something like
> > 
> >   Make gitlab-runner configuration readable
> 
> Okay, however...
> 
> > for both.
> > 
> > Additionally, even though the gitlab user is going to be the only one
> > on the system so it doesn't make much of a difference in practice, I
> > think we should have config.toml
> > 
> 
> ...here you suggest the following adjustment. I feel like the messages above
> will then become confusing and misleading, since who are we making it readable
> for? Well, only for the gitlab user, so I think a little more detail in them is
> justifiable.
> 
> >   owner: root
> >   group: gitlab
> >   mode: '0640'
> 
> So how about:
> "Make gitlab-runner config dir readable" for the former and
> "Make gitlab-runner config.toml owned by the gitlab group" for the latter

I still think that's an unnecessary amount of detail, and we have
plenty of existing examples in the repository such as

  - name: Update installed packages
    package:
      name: fedora-gpg-keys
      state: latest
      disable_gpg_check: yes
    when:
      - os_name == 'Fedora'
      - os_version == 'Rawhide'

  - name: Update installed packages
    command: '{{ package_manager }} update --refresh --exclude "kernel*" -y'
    args:
      warn: no
    when:
      - os_name == 'Fedora'
      - os_version == 'Rawhide'

  - name: Update installed packages
    command: '{{ package_manager }} update --disablerepo="*" --enablerepo=fedora-rawhide-kernel-nodebug "kernel*" -y'
    args:
      warn: no
    when:
      - os_name == 'Fedora'
      - os_version == 'Rawhide'

where we provide the high-level information as feedback to the user,
without going too much into detail - in this case, that we're
updating the system in three steps instead of a single one because
some packages require special handling.

But I don't want to hold up the series because of bikeshedding, so
if you are very keen on having the extra detail I'll take it as-is :)

-- 
Andrea Bolognani / Red Hat / Virtualization

On Tue, Apr 14, 2020 at 12:31:26PM +0200, Andrea Bolognani wrote:
> On Tue, 2020-04-14 at 10:17 +0200, Erik Skultety wrote:
> > On Thu, Apr 09, 2020 at 12:28:50PM +0200, Andrea Bolognani wrote:
> > > On Thu, 2020-04-09 at 06:23 +0200, Erik Skultety wrote:
> > > > +++ b/guests/playbooks/update/tasks/gitlab.yml
> > > > +- name: Make {{ gitlab_runner_config_dir }} world readable
> > > > +  file:
> > > > +    path: '{{ gitlab_runner_config_dir }}'
> > > > +    mode: '0755'
> > > > +
> > > > +- name: Make {{ gitlab_runner_config_dir }}/config.toml world readable
> > > > +  file:
> > > > +    path: '{{ gitlab_runner_config_dir }}/config.toml'
> > > > +    mode: '0644'
> > >
> > > The message for these tasks is unnecessarily detailed: I'd just use
> > > something like
> > >
> > >   Make gitlab-runner configuration readable
> >
> > Okay, however...
> >
> > > for both.
> > >
> > > Additionally, even though the gitlab user is going to be the only one
> > > on the system so it doesn't make much of a difference in practice, I
> > > think we should have config.toml
> > >
> >
> > ...here you suggest the following adjustment. I feel like the messages above
> > will then become confusing and misleading, since who are we making it readable
> > for? Well, only for the gitlab user, so I think a little more detail in them is
> > justifiable.
> >
> > >   owner: root
> > >   group: gitlab
> > >   mode: '0640'
> >
> > So how about:
> > "Make gitlab-runner config dir readable" for the former and
> > "Make gitlab-runner config.toml owned by the gitlab group" for the latter
>
> I still think that's an unnecessary amount of detail, and we have
> plenty of existing examples in the repository such as
>
>   - name: Update installed packages
>     package:
>       name: fedora-gpg-keys
>       state: latest
>       disable_gpg_check: yes
>     when:
>       - os_name == 'Fedora'
>       - os_version == 'Rawhide'
>
>   - name: Update installed packages
>     command: '{{ package_manager }} update --refresh --exclude "kernel*" -y'
>     args:
>       warn: no
>     when:
>       - os_name == 'Fedora'
>       - os_version == 'Rawhide'
>
>   - name: Update installed packages
>     command: '{{ package_manager }} update --disablerepo="*" --enablerepo=fedora-rawhide-kernel-nodebug "kernel*" -y'
>     args:
>       warn: no
>     when:
>       - os_name == 'Fedora'
>       - os_version == 'Rawhide'

Fair enough, ^this is a compelling counter-argument, I proceeded with the
original suggestion and merged the series, thanks for the review.

--
Erik Skultety