On Tue, Mar 20, 2018 at 05:23:58PM +0100, Andrea Bolognani wrote:

We're going to remove the 'jenkins' pseudo-project from the list of per-guest projects soon, so we need another way of deciding whether to install and configure the Jenkins agent.

The availability of the Jenkins secret in the vault is a perfect candidate, and using it improves things in general because we can now store the information about which guests are part of the Jenkins setup in a single place instead of duplicating it.

Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- guests/site.yml | 3 --- guests/tasks/jenkins.yml | 7 +++++++ 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/guests/site.yml b/guests/site.yml index 5f69cfd..8d32561 100644 --- a/guests/site.yml +++ b/guests/site.yml @@ -32,6 +32,3 @@ - include: tasks/jenkins.yml when: - flavor == 'jenkins' - - projects is defined - # jenkins is a pseudo-project - - ( 'jenkins' in projects ) diff --git a/guests/tasks/jenkins.yml b/guests/tasks/jenkins.yml index 94c2404..10aeec7 100644 --- a/guests/tasks/jenkins.yml +++ b/guests/tasks/jenkins.yml @@ -6,6 +6,8 @@ - name: Look up Jenkins secret set_fact: jenkins_secret: '{{ vault.jenkins_secrets[inventory_hostname] }}' + when: + - vault.jenkins_secrets[inventory_hostname] is defined

- name: Download Jenkins agent get_url: @@ -14,6 +16,8 @@ owner: jenkins group: jenkins force: yes + when: + - jenkins_secret is defined

- name: Configure and enable Jenkins agent lineinfile: @@ -24,6 +28,7 @@ line: "nohup {{ su }} - jenkins -c '{{ java }} -jar /home/jenkins/slave.jar -jnlpUrl \"{{ jenkins_url }}\" -secret \"{{ jenkins_secret }}\"' >/var/log/jenkins.log 2>&1 &" insertbefore: '^exit .*$' when: + - jenkins_secret is defined - ansible_service_mgr != 'systemd'

- name: Configure Jenkins agent @@ -31,6 +36,7 @@ src: templates/jenkins.service.j2 dest: /etc/systemd/system/jenkins.service when: + - jenkins_secret is defined - ansible_service_mgr == 'systemd'

- name: Enable Jenkins agent @@ -39,4 +45,5 @@ enabled: yes daemon_reload: yes when: + - jenkins_secret is defined - ansible_service_mgr == 'systemd'

Would it be possible to create a group of tasks that should be run only if "jenkins_secret is defined" and guard the whole group with that check? Pavel

On Wed, Mar 21, 2018 at 04:08:26PM +0100, Andrea Bolognani wrote:

On Wed, 2018-03-21 at 15:52 +0100, Pavel Hrdina wrote:

...

...

- name: Enable Jenkins agent @@ -39,4 +45,5 @@ enabled: yes daemon_reload: yes when: + - jenkins_secret is defined - ansible_service_mgr == 'systemd'

Would it be possible to create a group of tasks that should be run only if "jenkins_secret is defined" and guard the whole group with that check?

We could use blocks:

http://docs.ansible.com/ansible/latest/playbooks_blocks.html

However, we're not using that feature anywhere and we might want to use the 'name' keyword introduced in 2.3 along with it, so I would skip this change for now and apply it more widely as part of the port to Ansible 2.4.

Works for me. Reviewed-by: Pavel Hrdina <phrdina@redhat.com>

On Tue, Mar 20, 2018 at 05:23:59PM +0100, Andrea Bolognani wrote:

The 'jenkins' pseudo-package is an implementation detail, and as such is better not exposed.

Moreover, with this change the JDK will only be installed when the 'jenkins' flavor is used, which means developers will have slightly smaller guests.

Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- guests/host_vars/libvirt-centos-6/main.yml | 1 - guests/host_vars/libvirt-centos-7/main.yml | 1 - guests/host_vars/libvirt-debian-8/main.yml | 1 - guests/host_vars/libvirt-debian-9/main.yml | 1 - guests/host_vars/libvirt-fedora-26/main.yml | 1 - guests/host_vars/libvirt-fedora-27/main.yml | 1 - guests/host_vars/libvirt-fedora-rawhide/main.yml | 1 - guests/host_vars/libvirt-freebsd-10/main.yml | 1 - guests/host_vars/libvirt-freebsd-11/main.yml | 1 - guests/site.yml | 7 +++++++ 10 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/guests/site.yml b/guests/site.yml index 8d32561..509d31a 100644 --- a/guests/site.yml +++ b/guests/site.yml @@ -28,6 +28,13 @@ when: - projects is defined

+ # Install packages needed for the Jenkins agent + - include: tasks/packages.yml + vars: + project: jenkins + when: + - flavor == "jenkins" + # Configure the Jenkins agent - include: tasks/jenkins.yml when:

Would it make sense to move the installation of packages needed for Jenkins agent into the jenkins task to create a single unit out of it? Otherwise looks good. Pavel

On Tue, Mar 20, 2018 at 05:24:00PM +0100, Andrea Bolognani wrote:

The 'base' pseudo-package is an implementation detail, and as such is better not exposed.

Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- guests/host_vars/libvirt-centos-6/main.yml | 1 - guests/host_vars/libvirt-centos-7/main.yml | 1 - guests/host_vars/libvirt-debian-8/main.yml | 1 - guests/host_vars/libvirt-debian-9/main.yml | 1 - guests/host_vars/libvirt-debian-sid/main.yml | 1 - guests/host_vars/libvirt-fedora-26/main.yml | 1 - guests/host_vars/libvirt-fedora-27/main.yml | 1 - guests/host_vars/libvirt-fedora-rawhide/main.yml | 1 - guests/host_vars/libvirt-freebsd-10/main.yml | 1 - guests/host_vars/libvirt-freebsd-11/main.yml | 1 - guests/host_vars/libvirt-freebsd-current/main.yml | 1 - guests/host_vars/libvirt-ubuntu-12/main.yml | 1 - guests/host_vars/libvirt-ubuntu-14/main.yml | 1 - guests/host_vars/libvirt-ubuntu-16/main.yml | 1 - guests/site.yml | 5 +++++ 15 files changed, 5 insertions(+), 14 deletions(-)

diff --git a/guests/site.yml b/guests/site.yml index 509d31a..4207d4e 100644 --- a/guests/site.yml +++ b/guests/site.yml @@ -19,6 +19,11 @@ - include: tasks/compat.yml - include: tasks/user.yml

+ # Install base packages + - include: tasks/packages.yml + vars: + project: base +

The same question as for the jenkins packages, would it make sense to move it into the base.yml task? Otherwise looks good. Pavel

Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- guests/tasks/packages.yml | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/guests/tasks/packages.yml b/guests/tasks/packages.yml index 807b5c4..a26a94d 100644 --- a/guests/tasks/packages.yml +++ b/guests/tasks/packages.yml @@ -1,4 +1,10 @@ --- +# Default to installing packages if state is not passed from the caller +- set_fact: + state: present + when: + - state is undefined + - name: '{{ project }}: Load variables' include_vars: file: 'vars/projects/{{ project }}.yml' @@ -58,9 +64,9 @@ - temp[item] != None - temp[item] not in flattened -- name: '{{ project }}: Install packages' +- name: '{{ project }}: Install/remove packages (state={{ state }})' package: name: '{{ item }}' - state: present + state: '{{ state }}' with_items: '{{ flattened|sort }}' -- 2.14.3

As with ad-hoc installation, we want to get rid of ad-hoc package removal. Add a 'blacklist' pseudo-project which can be used for the purpose. In the future, we might use this facility to keep long-lived guests clean by blacklisting packages as they get dropped from the respective project. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- guests/site.yml | 6 ++++++ guests/tasks/base.yml | 7 ------- guests/vars/mappings.yml | 3 +++ guests/vars/projects/blacklist.yml | 3 +++ 4 files changed, 12 insertions(+), 7 deletions(-) create mode 100644 guests/vars/projects/blacklist.yml diff --git a/guests/site.yml b/guests/site.yml index d057116..a00be86 100644 --- a/guests/site.yml +++ b/guests/site.yml @@ -24,6 +24,12 @@ vars: project: base + # Remove blacklisted packages + - include: tasks/packages.yml + vars: + project: blacklist + state: absent + # Create users. This needs to happen after installing base packages - include: tasks/user.yml diff --git a/guests/tasks/base.yml b/guests/tasks/base.yml index 5379bf6..0c82b58 100644 --- a/guests/tasks/base.yml +++ b/guests/tasks/base.yml @@ -115,13 +115,6 @@ when: - package_format == 'pkg' -- name: Remove unwanted packages - package: - name: '{{ item }}' - state: absent - with_items: - - nano - - name: Configure hostname hostname: name: '{{ inventory_hostname }}' diff --git a/guests/vars/mappings.yml b/guests/vars/mappings.yml index f6ac801..deae69d 100644 --- a/guests/vars/mappings.yml +++ b/guests/vars/mappings.yml @@ -400,6 +400,9 @@ mappings: mingw64-readline: FedoraRawhide: mingw64-readline + nano: + default: nano + netcf: deb: libnetcf-dev rpm: netcf-devel diff --git a/guests/vars/projects/blacklist.yml b/guests/vars/projects/blacklist.yml new file mode 100644 index 0000000..e5f369c --- /dev/null +++ b/guests/vars/projects/blacklist.yml @@ -0,0 +1,3 @@ +--- +packages: + - nano -- 2.14.3

On Wed, 2018-03-21 at 16:11 +0100, Pavel Hrdina wrote:

...

+++ b/guests/vars/projects/blacklist.yml @@ -0,0 +1,3 @@ +--- +packages: + - nano

In the past we've installed some packages, that were removed from ansible and it would make sense to list them in the blacklist in order to cleanup existing installations that someone can already have.

The list of the removed packages is:

polkit-devel glibc-rpcgen on Fedora > 27 (it was glibc-common)

Anyway, this can be a followup patch.

The rpcgen stuff is pretty confusing, even though I'm the one who made the change :) Anyway, glibc-rpcgen no longer seems to exist in rawhide, while glibc-common can't be removed because packages depend on it, so I guess there's no cleaning up required there. As for polkit-devel, I'm trying to figure out a nice way to add it to the blacklist... The current naming convention is along the lines of foo: deb: libfoo-dev rpm: foo-devel so it doesn't fit neatly in there, because we want to blacklist polkit-devel but still install polkit itself. Maybe we could have blacklist-polkit: rpm: polkit-devel or something similar as a workaround? Does that look reasonable? Alternatively we can just leave the package installed, of course :) -- Andrea Bolognani / Red Hat / Virtualization

We only need jessie-backports for the JDK, and we only need the JDK for the Jenkins agent. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- guests/tasks/base.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/guests/tasks/base.yml b/guests/tasks/base.yml index 0c82b58..debf6e4 100644 --- a/guests/tasks/base.yml +++ b/guests/tasks/base.yml @@ -75,6 +75,7 @@ when: - os_name == 'Debian' - os_version == '8' + - flavor == 'jenkins' - name: Configure APT pinning for jessie-backports copy: @@ -85,6 +86,7 @@ when: - os_name == 'Debian' - os_version == '8' + - flavor == 'jenkins' - name: Enable fedora-rawhide-kernel-nodebug repository copy: -- 2.14.3

We already have a separate task for creating the non-root user, so it makes sense to move everything related to the root user to that taks as well. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- guests/site.yml | 2 +- guests/tasks/base.yml | 20 -------------------- guests/tasks/{user.yml => users.yml} | 20 ++++++++++++++++++++ 3 files changed, 21 insertions(+), 21 deletions(-) rename guests/tasks/{user.yml => users.yml} (67%) diff --git a/guests/site.yml b/guests/site.yml index a00be86..351e575 100644 --- a/guests/site.yml +++ b/guests/site.yml @@ -31,7 +31,7 @@ state: absent # Create users. This needs to happen after installing base packages - - include: tasks/user.yml + - include: tasks/users.yml # Install build dependencies for each project - include: tasks/packages.yml diff --git a/guests/tasks/base.yml b/guests/tasks/base.yml index debf6e4..33681b4 100644 --- a/guests/tasks/base.yml +++ b/guests/tasks/base.yml @@ -121,26 +121,6 @@ hostname: name: '{{ inventory_hostname }}' -- name: Configure ssh access for the root user - authorized_key: - user: root - key: '{{ lookup("file", lookup("env", "HOME") + "/.ssh/id_rsa.pub") }}' - state: present - -- name: Configure root password and shell - user: - name: root - password: '{{ lookup("file", root_password_file) }}' - shell: '{{ bash }}' - -- name: Disable password authentication for the root user - lineinfile: - path: /etc/ssh/sshd_config - regexp: '^#*\s*PermitRootLogin\s*.*$' - line: 'PermitRootLogin without-password' - state: present - backup: yes - - name: Look for GRUB2 configuration stat: path: /etc/default/grub diff --git a/guests/tasks/user.yml b/guests/tasks/users.yml similarity index 67% rename from guests/tasks/user.yml rename to guests/tasks/users.yml index 3db5258..6134228 100644 --- a/guests/tasks/user.yml +++ b/guests/tasks/users.yml @@ -1,4 +1,24 @@ --- +- name: 'root: Set password' + user: + name: root + password: '{{ lookup("file", root_password_file) }}' + shell: '{{ bash }}' + +- name: 'root: Configure ssh access' + authorized_key: + user: root + key: '{{ lookup("file", lookup("env", "HOME") + "/.ssh/id_rsa.pub") }}' + state: present + +- name: 'root: Disable ssh password authentication' + lineinfile: + path: /etc/ssh/sshd_config + regexp: '^#*\s*PermitRootLogin\s*.*$' + line: 'PermitRootLogin without-password' + state: present + backup: yes + - name: '{{ flavor }}: Create user account' user: name: '{{ flavor }}' -- 2.14.3

There are enough bootloader tweaks that splitting them off to a separate task makes sense. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- guests/site.yml | 1 + guests/tasks/base.yml | 53 --------------------------------------------- guests/tasks/bootloader.yml | 53 +++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 54 insertions(+), 53 deletions(-) create mode 100644 guests/tasks/bootloader.yml diff --git a/guests/site.yml b/guests/site.yml index 351e575..ceaaf25 100644 --- a/guests/site.yml +++ b/guests/site.yml @@ -17,6 +17,7 @@ # Prepare environment. None of the actions performed here might # depend on packages being installed - include: tasks/base.yml + - include: tasks/bootloader.yml - include: tasks/compat.yml # Install base packages diff --git a/guests/tasks/base.yml b/guests/tasks/base.yml index 33681b4..53cbd65 100644 --- a/guests/tasks/base.yml +++ b/guests/tasks/base.yml @@ -120,56 +120,3 @@ - name: Configure hostname hostname: name: '{{ inventory_hostname }}' - -- name: Look for GRUB2 configuration - stat: - path: /etc/default/grub - register: grubdefault - -- name: Look for GRUB2 configuration - stat: - path: /boot/grub/grub.cfg - register: grubcfg - -- name: Look for GRUB2 configuration - stat: - path: /boot/grub2/grub.cfg - register: grub2cfg - -- name: Configure GRUB2 - lineinfile: - path: /etc/default/grub - regexp: '^{{ item.key }}=.*$' - line: '{{ item.key }}="{{ item.value }}"' - backup: yes - with_items: - - { key: 'GRUB_TIMEOUT', value: '1' } - - { key: 'GRUB_CMDLINE_LINUX_DEFAULT', value: 'console=ttyS0' } - - { key: 'GRUB_CMDLINE_LINUX', value: 'console=ttyS0' } - - { key: 'GRUB_TERMINAL', value: 'serial' } - - { key: 'GRUB_SERIAL_COMMAND', value: 'serial' } - when: - - grubdefault.stat.exists - -- name: Apply GRUB2 configuration - command: 'grub-mkconfig -o /boot/grub/grub.cfg' - when: - - grubcfg.stat.exists - -- name: Apply GRUB2 configuration - command: 'grub2-mkconfig -o /boot/grub2/grub.cfg' - when: - - grub2cfg.stat.exists - -- name: Configure the FreeBSD bootloader - lineinfile: - path: /boot/loader.conf - regexp: '^{{ item.key }}=.*$' - line: '{{ item.key }}="{{ item.value }}"' - create: yes - backup: yes - with_items: - - { key: 'console', value: 'comconsole' } - - { key: 'autoboot_delay', value: '1' } - when: - - os_name == 'FreeBSD' diff --git a/guests/tasks/bootloader.yml b/guests/tasks/bootloader.yml new file mode 100644 index 0000000..face3dd --- /dev/null +++ b/guests/tasks/bootloader.yml @@ -0,0 +1,53 @@ +--- +- name: Look for GRUB2 configuration + stat: + path: /etc/default/grub + register: grubdefault + +- name: Look for GRUB2 configuration + stat: + path: /boot/grub/grub.cfg + register: grubcfg + +- name: Look for GRUB2 configuration + stat: + path: /boot/grub2/grub.cfg + register: grub2cfg + +- name: Configure GRUB2 + lineinfile: + path: /etc/default/grub + regexp: '^{{ item.key }}=.*$' + line: '{{ item.key }}="{{ item.value }}"' + backup: yes + with_items: + - { key: 'GRUB_TIMEOUT', value: '1' } + - { key: 'GRUB_CMDLINE_LINUX_DEFAULT', value: 'console=ttyS0' } + - { key: 'GRUB_CMDLINE_LINUX', value: 'console=ttyS0' } + - { key: 'GRUB_TERMINAL', value: 'serial' } + - { key: 'GRUB_SERIAL_COMMAND', value: 'serial' } + when: + - grubdefault.stat.exists + +- name: Apply GRUB2 configuration + command: 'grub-mkconfig -o /boot/grub/grub.cfg' + when: + - grubcfg.stat.exists + +- name: Apply GRUB2 configuration + command: 'grub2-mkconfig -o /boot/grub2/grub.cfg' + when: + - grub2cfg.stat.exists + +- name: Configure the FreeBSD bootloader + lineinfile: + path: /boot/loader.conf + regexp: '^{{ item.key }}=.*$' + line: '{{ item.key }}="{{ item.value }}"' + create: yes + backup: yes + with_items: + - { key: 'console', value: 'comconsole' } + - { key: 'autoboot_delay', value: '1' } + when: + - os_name == 'FreeBSD' -- 2.14.3

On Wed, Mar 21, 2018 at 01:19:57PM +0100, Andrea Bolognani wrote:

There is a small number of packages that we install as part of the 'base' task with an ad-hoc call to the package module.

Since we have generic facilities for installing packages and a generic 'base' pseudo-project that we use for all packages that are not related to any specific project, we can fold everything into it and have a single source of truth.

The change requires us to delay user creation, though, because as part of that we edit the sudoers file, which doesn't exist until the sudo package has been installed.

Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- guests/site.yml | 7 +++++-- guests/tasks/base.yml | 11 ----------- guests/vars/mappings.yml | 15 +++++++++++++++ guests/vars/projects/base.yml | 5 +++++ 4 files changed, 25 insertions(+), 13 deletions(-)

diff --git a/guests/site.yml b/guests/site.yml index 4207d4e..d057116 100644 --- a/guests/site.yml +++ b/guests/site.yml @@ -14,16 +14,19 @@

tasks:

- # Prepare the base environment + # Prepare environment. None of the actions performed here might + # depend on packages being installed - include: tasks/base.yml - include: tasks/compat.yml - - include: tasks/user.yml

# Install base packages - include: tasks/packages.yml vars: project: base

+ # Create users. This needs to happen after installing base packages + - include: tasks/user.yml +

This would be solved by moving the package installation into the base task. Reviewed-by: Pavel Hrdina <phrdina@redhat.com>