11:23 p.m.
since v1:
- .bash_logout is removed from the user home environment (causes builds to fail
on Debian)
- gitlab-runner is now downloaded to and executed from user's bin/ directory
instead of using /usr/local/bin
- both RC and systemd service files now define the user explicitly that the init
system changes to when spawning the gitlab-runner service
- the gitlab flavour tasks file now relies on shell's 'creates' module
argument
so that the agent registration is idempotent rather than touching the
dirs/files explicitly
- both gitlab URL and runner token are now files editable under
~/.config/lcitool
since v2:
- download location changed to /usr/local/bin
- config location changed to /etc/gitlab-runner (readable to the gitlab user)
- machine tagging changed to 'os_version-distro_major_number'
- fixed the regression caused by the gitlab code which was checking the gitlab
config files unconditionally for all flavours
- cosmetic syntactic changes to the Python code as requested by the reviewer
Both the config.toml improvement in terms of replacing the ~/.config/lcitool
files as discussed during the v1 review process as well as cloudinit usage is
planned for a separate respective series, so for now, we'll have to live with 2
new config files and cloud images will need to be built manually.
Erik Skultety (5):
guests: users: Discard the .bash_logout skeleton file
guests: templates: Introduce a gitlab-runner systemd service template
guests: templates: Introduce a gitlab-runner RC init service template
guests: Introduce the new 'gitlab' flavor
guests: lcitool: Enable the new 'gitlab' flavor in the lcitool script
guests/lcitool | 46 ++++++++++++++-
guests/playbooks/update/main.yml | 5 ++
guests/playbooks/update/tasks/gitlab.yml | 58 +++++++++++++++++++
guests/playbooks/update/tasks/users.yml | 3 +-
.../update/templates/gitlab-runner.j2 | 31 ++++++++++
.../update/templates/gitlab-runner.service.j2 | 14 +++++
6 files changed, 154 insertions(+), 3 deletions(-)
create mode 100644 guests/playbooks/update/tasks/gitlab.yml
create mode 100644 guests/playbooks/update/templates/gitlab-runner.j2
create mode 100644 guests/playbooks/update/templates/gitlab-runner.service.j2
--
2.25.1
11:23 p.m.
New subject: [libvirt-jenkins-ci PATCH v3 1/5] guests: users: Discard the .bash_logout skeleton file
As part of the users tasks, we remove the .profile file after we create
our own .bash_profile. As part of that effort, let's remove the .bash_logout
skeleton file as well, the reason being gitlab's runner agent which
has a hard time initializing a build environment on Debian where the
system populates the .bash_logout with console cleaning code which
messes up the runner and causes the build to fail instantly [1].
Since Debian is the only distro actually populating the file and the
machines are assumed to be accessed over SSH, we can safely drop it.
[1] https://gitlab.com/gitlab-org/gitlab-runner/issues/4449
Signed-off-by: Erik Skultety <eskultet(a)redhat.com>
Reviewed-by: Andrea Bolognani <abologna(a)redhat.com>
---
guests/playbooks/update/tasks/users.yml | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/guests/playbooks/update/tasks/users.yml
b/guests/playbooks/update/tasks/users.yml
index 30e5fca..a07349f 100644
--- a/guests/playbooks/update/tasks/users.yml
+++ b/guests/playbooks/update/tasks/users.yml
@@ -63,9 +63,10 @@
- bash_profile
- bashrc
-- name: '{{ flavor }}: Remove existing shell profile'
+- name: '{{ flavor }}: Remove unnecessary home skeleton files'
file:
path: /home/{{ flavor }}/.{{ item }}
state: absent
with_items:
- profile
+ - bash_logout
--
2.25.1
11:23 p.m.
New subject: [libvirt-jenkins-ci PATCH v3 2/5] guests: templates: Introduce a gitlab-runner systemd service template
Signed-off-by: Erik Skultety <eskultet(a)redhat.com>
---
.../update/templates/gitlab-runner.service.j2 | 14 ++++++++++++++
1 file changed, 14 insertions(+)
create mode 100644 guests/playbooks/update/templates/gitlab-runner.service.j2
diff --git a/guests/playbooks/update/templates/gitlab-runner.service.j2
b/guests/playbooks/update/templates/gitlab-runner.service.j2
new file mode 100644
index 0000000..9c3adc0
--- /dev/null
+++ b/guests/playbooks/update/templates/gitlab-runner.service.j2
@@ -0,0 +1,14 @@
+[Unit]
+Description=GitLab Runner
+After=network.target
+ConditionFileIsExecutable=/usr/local/bin/gitlab-runner
+
+[Service]
+User=gitlab
+WorkingDirectory=/home/gitlab
+ExecStart=/usr/local/bin/gitlab-runner run --config /etc/gitlab-runner/config.toml
+Restart=always
+
+[Install]
+WantedBy=multi-user.target
+
--
2.25.1
5:16 a.m.
New subject: [libvirt-jenkins-ci PATCH v3 2/5] guests: templates: Introduce a gitlab-runner systemd service template
On Thu, 2020-04-09 at 06:23 +0200, Erik Skultety wrote:
+++ b/guests/playbooks/update/templates/gitlab-runner.service.j2
+[Install]
+WantedBy=multi-user.target
+
This blank line is unnecessary and 'git am' complains about it:
.git/rebase-apply/patch:25: new blank line at EOF.
+
Once you've gotten rid of it,
Reviewed-by: Andrea Bolognani <abologna(a)redhat.com>
--
Andrea Bolognani / Red Hat / Virtualization
11:23 p.m.
New subject: [libvirt-jenkins-ci PATCH v3 3/5] guests: templates: Introduce a gitlab-runner RC init service template
Based on https://docs.gitlab.com/runner/install/freebsd.html#installing-gitlab-runner
Signed-off-by: Erik Skultety <eskultet(a)redhat.com>
---
.../update/templates/gitlab-runner.j2 | 31 +++++++++++++++++++
1 file changed, 31 insertions(+)
create mode 100644 guests/playbooks/update/templates/gitlab-runner.j2
diff --git a/guests/playbooks/update/templates/gitlab-runner.j2
b/guests/playbooks/update/templates/gitlab-runner.j2
new file mode 100644
index 0000000..4c40c23
--- /dev/null
+++ b/guests/playbooks/update/templates/gitlab-runner.j2
@@ -0,0 +1,31 @@
+#!/bin/sh
+# PROVIDE: gitlab_runner
+# REQUIRE: DAEMON NETWORKING
+# BEFORE:
+# KEYWORD:
+
+. /etc/rc.subr
+
+name="gitlab_runner"
+rcvar="gitlab_runner_enable"
+
+user="gitlab"
+user_home="/home/gitlab"
+command="/usr/local/bin/gitlab-runner"
+command_args="run --config /etc/gitlab-runner/config.toml"
+pidfile="/var/run/${name}.pid"
+
+start_cmd="gitlab_runner_start"
+
+gitlab_runner_start()
+{
+ export USER=${user}
+ export HOME=${user_home}
+ if checkyesno ${rcvar}; then
+ cd ${user_home}
+ /usr/sbin/daemon -u ${user} -p ${pidfile} ${command} ${command_args} >
/var/log/gitlab-runner.log 2>&1
+ fi
+}
+
+load_rc_config $name
+run_rc_command $1
--
2.25.1
5:17 a.m.
New subject: [libvirt-jenkins-ci PATCH v3 3/5] guests: templates: Introduce a gitlab-runner RC init service template
On Thu, 2020-04-09 at 06:23 +0200, Erik Skultety wrote:
Based on
https://docs.gitlab.com/runner/install/freebsd.html#installing-gitlab-runner
Signed-off-by: Erik Skultety <eskultet(a)redhat.com>
---
.../update/templates/gitlab-runner.j2 | 31 +++++++++++++++++++
1 file changed, 31 insertions(+)
create mode 100644 guests/playbooks/update/templates/gitlab-runner.j2
Reviewed-by: Andrea Bolognani <abologna(a)redhat.com>
--
Andrea Bolognani / Red Hat / Virtualization
11:23 p.m.
New subject: [libvirt-jenkins-ci PATCH v3 4/5] guests: Introduce the new 'gitlab' flavor
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(a)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-r...
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
5:28 a.m.
New subject: [libvirt-jenkins-ci PATCH v3 4/5] guests: Introduce the new 'gitlab' flavor
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(a)redhat.com>
--
Andrea Bolognani / Red Hat / Virtualization
3:17 a.m.
New subject: [libvirt-jenkins-ci PATCH v3 4/5] guests: Introduce the new 'gitlab' flavor
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
5:31 a.m.
New subject: [libvirt-jenkins-ci PATCH v3 4/5] guests: Introduce the new 'gitlab' flavor
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
5:46 a.m.
New subject: [libvirt-jenkins-ci PATCH v3 4/5] guests: Introduce the new 'gitlab' flavor
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
11:23 p.m.
New subject: [libvirt-jenkins-ci PATCH v3 5/5] guests: lcitool: Enable the new 'gitlab' flavor in the lcitool script
Signed-off-by: Erik Skultety <eskultet(a)redhat.com>
---
guests/lcitool | 46 ++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 44 insertions(+), 2 deletions(-)
diff --git a/guests/lcitool b/guests/lcitool
index 689a8cf..975a811 100755
--- a/guests/lcitool
+++ b/guests/lcitool
@@ -175,7 +175,7 @@ class Config:
)
)
- if flavor not in ["test", "jenkins"]:
+ if flavor not in ["test", "jenkins", "gitlab"]:
raise Exception("Invalid flavor '{}'".format(flavor))
return flavor
@@ -185,7 +185,7 @@ class Config:
# 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")
try:
@@ -217,6 +217,44 @@ class Config:
return root_pass_file
+ def get_gitlab_runner_token_file(self):
+ if self.get_flavor() != "gitlab":
+ return None
+
+ 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(
+ gitlab_runner_token_file, ex
+ )
+ )
+
+ return gitlab_runner_token_file
+
+ def get_gitlab_url_file(self):
+ if self.get_flavor() != "gitlab":
+ return None
+
+ gitlab_url_file = self._get_config_file("gitlab-url")
+
+ try:
+ with open(gitlab_url_file, "r") as infile:
+ if not infile.readline().strip():
+ raise ValueError
+ except Exception as ex:
+ raise Exception(
+ "Missing or invalid GitLab url file ({}): {}".format(
+ gitlab_url_file, ex
+ )
+ )
+
+ return gitlab_url_file
+
class Inventory:
@@ -449,6 +487,8 @@ class Application:
flavor = self._config.get_flavor()
vault_pass_file = self._config.get_vault_password_file()
root_pass_file = self._config.get_root_password_file()
+ gitlab_url_file = self._config.get_gitlab_url_file()
+ gitlab_runner_token_file = self._config.get_gitlab_runner_token_file()
ansible_hosts = ",".join(self._inventory.expand_pattern(hosts))
selected_projects = self._projects.expand_pattern(projects)
@@ -477,6 +517,8 @@ class Application:
"selected_projects": selected_projects,
"git_remote": git_remote,
"git_branch": git_branch,
+ "gitlab_url_file": gitlab_url_file,
+ "gitlab_runner_token_file": gitlab_runner_token_file
})
ansible_playbook = distutils.spawn.find_executable("ansible-playbook")
--
2.25.1
5:31 a.m.
New subject: [libvirt-jenkins-ci PATCH v3 5/5] guests: lcitool: Enable the new 'gitlab' flavor in the lcitool script
On Thu, 2020-04-09 at 06:23 +0200, Erik Skultety wrote:
+++ b/guests/lcitool
@@ -477,6 +517,8 @@ class Application:
"selected_projects": selected_projects,
"git_remote": git_remote,
"git_branch": git_branch,
+ "gitlab_url_file": gitlab_url_file,
+ "gitlab_runner_token_file": gitlab_runner_token_file
Missing comma at the end of the last line.
With that fixed,
Reviewed-by: Andrea Bolognani <abologna(a)redhat.com>
--
Andrea Bolognani / Red Hat / Virtualization
1685
days inactive
1690
days old
12 comments
2 participants
participants (2)
-
Andrea Bolognani -
Erik Skultety