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

[Andrea Bolognani]

On Fri, 2020-04-03 at 09:38 +0200, Erik Skultety wrote:
> On Tue, Mar 31, 2020 at 05:23:32PM +0200, Andrea Bolognani wrote:
> > On Thu, 2020-03-26 at 14:33 +0100, Erik Skultety wrote:
> > > +++ 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.
> 
> This was a deliberate decision and I didn't want to do it the jenkins way, let
> me expand on why. Ultimately, we want other community members to contribute
> their resources to our gitlab so that the test results are centralized, but
> what if they want to setup the same machine for their internal infrastructure
> as well to stay unified with the setup upstream libvirt "mandates". With the
> secret in the vault, we always create machines that can only be plugged into
> *our* gitlab, but why should be that restrictive? Give the consumers the leeway
> to actually have a possibility to plug it into their own instance by supplying
> the token the same way as we supply the vault and root passwords, so I'd rather
> not be tying this to a single use case just like we did with jenkins.

Thanks for spelling out your intentions, it really helps.

I agree that reusing the recipe to build runners that connect to a
local / private GitLab instance is a valid use case that we should
accomodate.

One point that immediately leads to, and which was unintentionally
already addressed above, is that in that case you need not only the
token to be configurable, but also the connection URL.

Other than that, storing the token in ~/.config/lcitool is fine I
guess. The number of files in that directory is getting fairly
ridiculous, though, so Someone™ should really look into throwing
away the current, extremely crude configuration system that's been
inherited from the original shell implementation and replace it
with something more sensible like a config.toml.

> > > +# 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 }}'
> 
> Like I said in the comment above, the reason why I created those 2 tasks to
> create the config file was to ensure idempotency, the registration runs every
> single time you execute the playbook and guess what, it registers a new runner
> which you can see in the gitlab runner pool - we don't want that. I'm not sure
> "creates" actually prevents that behaviour.

That's *exactly* what it does:

  creates     A filename, when it already exists, this step
  (path)      will *not* be run.

https://docs.ansible.com/ansible/latest/modules/shell_module.html#parameter-creates

> >   * 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 get the isolation part, but why exactly is the additional isolation by the
> container deemed so necessary compared to achieving consistency in terms of
> maintenance? I mentioned it earlier, these machines run in a secured
> environment not accessible from the outside. Moreover, these are test machines
> that are designed to come and go, so if the worst comes, they can be torn down
> and replaced with new copies, that's the whole point of CI environments, so
> aren't we trying to be a bit too paranoid?

Consistency is a worthy goal, but it's not the only one. Security is
another very important one.

Anyway, it's simpler than that: all of our Linux jobs currently
expect to run in container images, and we can only have that using
the Docker executor.

We could, I guess, change our configuration so that, instead of
having each job use a specific container image, they would be tagged
with a specific label that would ensure they are scheduled on the
expected target OS, eg. instead of

  dco:
    script:
      - ...
    image: quay.io/libvirt/buildenv-libvirt-fedora-31:latest

we could have

  dco:
    script:
      - ...
    tags:
      - fedora-31

However, due to the restrictions in how GitLab will schedule jobs for
forks, that configuration would only run for libvirt/libvirt branches
and would not run for forks and merge requests, the latter of which
would basically make it pointless.

What I think we should have instead, in addition to the FreeBSD
runner(s) that we need to match Jenkins' coverage, is Linux runner(s)
using the Docker executor that are tagged appropriately and can speed
up post-merge and periodic build tasks by complementing the shared
runners.

> >   * 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).
> 
> Untagged jobs have a problem - you cannot ensure parallelism, so you may end up
> with job queued on a single machine, e.g. we tried to play with different job
> settings with ptoscano and even though there were 3 nodes available and we
> scheduled 3 jobs, 2 jobs were queued on a single machine, so tagged jobs is
> probably what we do want, unless we don't care about parallel execution (but we
> certainly do care about distro coverage).

Were all those runners configured to accept untagged jobs? The
default is not to accept them.

> > [...]
> > > +- 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.
> 
> Have you managed to get it work? I spent like 2 days trying different
> combinations in the name of the service and I just could not make it work, so I
> stopped trying to rub it against the grain and simply followed the FreeBSD
> convention, but if you can make it work, I'm happy to switch, as I really
> wanted to be 99.9% (maybe a little bit less :) ) consistent.

I haven't even tried O:-) If you've already spent that much time on
it without success, let's forget it for the time being. We can always
go back and change it at a later time if we find a way to make it
work with the prettier name :)

-- 
Andrea Bolognani / Red Hat / Virtualization