[libvirt-jenkins-ci PATCH v3 0/5] Introduce the new 'gitlab' flavor
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
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@redhat.com> Reviewed-by: Andrea Bolognani <abologna@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
Signed-off-by: Erik Skultety <eskultet@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
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@redhat.com> -- Andrea Bolognani / Red Hat / Virtualization
Based on https://docs.gitlab.com/runner/install/freebsd.html#installing-gitlab-runner Signed-off-by: Erik Skultety <eskultet@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
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@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@redhat.com> -- Andrea Bolognani / Red Hat / Virtualization
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
Signed-off-by: Erik Skultety <eskultet@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
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@redhat.com> -- Andrea Bolognani / Red Hat / Virtualization
participants (2)
-
Andrea Bolognani -
Erik Skultety