Re: [PATCH v5 2/4] Jobs based on custom runners: build environment docs and playbook

[Cleber Rosa]

On Tue, Feb 23, 2021 at 02:01:53PM +0000, Alex Bennée wrote:
> 
> Cleber Rosa <crosa@redhat.com> writes:
> 
> > To run basic jobs on custom runners, the environment needs to be
> > properly set up.  The most common requirement is having the right
> > packages installed.
> >
> > The playbook introduced here covers the QEMU's project s390x and
> > aarch64 machines.  At the time this is being proposed, those machines
> > have already had this playbook applied to them.
> >
> > Signed-off-by: Cleber Rosa <crosa@redhat.com>
> > ---
> >  docs/devel/ci.rst                      | 30 ++++++++++
> >  scripts/ci/setup/build-environment.yml | 76 ++++++++++++++++++++++++++
> >  scripts/ci/setup/inventory             |  1 +
> >  3 files changed, 107 insertions(+)
> >  create mode 100644 scripts/ci/setup/build-environment.yml
> >  create mode 100644 scripts/ci/setup/inventory
> >
> > diff --git a/docs/devel/ci.rst b/docs/devel/ci.rst
> > index 585b7bf4b8..a556558435 100644
> > --- a/docs/devel/ci.rst
> > +++ b/docs/devel/ci.rst
> > @@ -26,3 +26,33 @@ gitlab-runner, is called a "custom runner".
> >  The GitLab CI jobs definition for the custom runners are located under::
> >  
> >    .gitlab-ci.d/custom-runners.yml
> > +
> > +Machine Setup Howto
> > +-------------------
> > +
> > +For all Linux based systems, the setup can be mostly automated by the
> > +execution of two Ansible playbooks.  Start by adding your machines to
> > +the ``inventory`` file under ``scripts/ci/setup``, such as this::
> > +
> > +  fully.qualified.domain
> > +  other.machine.hostname
> 
> Is this really needed? Can't the host list be passed in the command
> line? I find it off to imagine users wanting to configure whole fleets
> of runners.
>

No, it's not needed.

But, in my experience, it's the most common way people use
ansible-playbook.  As with all most tools QEMU relies on, that are
many different ways of using them.  IMO documenting more than one way
to perform the same task makes the documentation unclear.

> > +
> > +You may need to set some variables in the inventory file itself.  One
> > +very common need is to tell Ansible to use a Python 3 interpreter on
> > +those hosts.  This would look like::
> > +
> > +  fully.qualified.domain ansible_python_interpreter=/usr/bin/python3
> > +  other.machine.hostname ansible_python_interpreter=/usr/bin/python3
> > +
> > +Build environment
> > +~~~~~~~~~~~~~~~~~
> > +
> > +The ``scripts/ci/setup/build-environment.yml`` Ansible playbook will
> > +set up machines with the environment needed to perform builds and run
> > +QEMU tests.  It covers a number of different Linux distributions and
> > +FreeBSD.
> > +
> > +To run the playbook, execute::
> > +
> > +  cd scripts/ci/setup
> > +  ansible-playbook -i inventory build-environment.yml
> 
> So I got somewhat there with a direct command line invocation:
> 
>   ansible-playbook -u root -i 192.168.122.24,192.168.122.45 
> scripts/ci/setup/build-environment.yml -e 
> 'ansible_python_interpreter=/usr/bin/python3'
>

Yes, and the "-e" is another example of the multiple ways to achieve
the same task.

> although for some reason a single host -i fails...
> 
> > diff --git a/scripts/ci/setup/build-environment.yml 
> > b/scripts/ci/setup/build-environment.yml

It requires a comma separated list, even if it's a list with a single
item:

   
https://docs.ansible.com/ansible/latest/cli/ansible-playbook.html#cmdoption-ansible-playbook-i

> > new file mode 100644
> > index 0000000000..0197e0a48b
> > --- /dev/null
> > +++ b/scripts/ci/setup/build-environment.yml
> > @@ -0,0 +1,76 @@
> > +---
> > +- name: Installation of basic packages to build QEMU
> > +  hosts: all
> > +  tasks:
> > +    - name: Update apt cache
> > +      apt:
> > +        update_cache: yes
> > +      when:
> > +        - ansible_facts['distribution'] == 'Ubuntu'
> 
> So are we limiting to Ubuntu here rather than say a Debian base?
>

You have a point, because this would certainly work and be applicable
to Debian systems too.  But, this is a new addition on v5, and I'm
limiting this patch to the machines that are available/connected right
now to the QEMU project on GitLab.

I can change that to "distribution_family == Debian" if you think
it's a good idea.  But IMO it'd make more sense for a patch
introducing the package list for Debian systems to change that.

> > +
> > +    - name: Install basic packages to build QEMU on Ubuntu 18.04/20.04
> > +      package:
> > +        name:
> > +        # Originally from tests/docker/dockerfiles/ubuntu1804.docker
> > +          - ccache
> > +          - clang
> > +          - gcc
> > +          - gettext
> > +          - git
> > +          - glusterfs-common
> > +          - libaio-dev
> > +          - libattr1-dev
> > +          - libbrlapi-dev
> > +          - libbz2-dev
> > +          - libcacard-dev
> > +          - libcap-ng-dev
> > +          - libcurl4-gnutls-dev
> > +          - libdrm-dev
> > +          - libepoxy-dev
> > +          - libfdt-dev
> > +          - libgbm-dev
> > +          - libgtk-3-dev
> > +          - libibverbs-dev
> > +          - libiscsi-dev
> > +          - libjemalloc-dev
> > +          - libjpeg-turbo8-dev
> > +          - liblzo2-dev
> > +          - libncurses5-dev
> > +          - libncursesw5-dev
> > +          - libnfs-dev
> > +          - libnss3-dev
> > +          - libnuma-dev
> > +          - libpixman-1-dev
> > +          - librados-dev
> > +          - librbd-dev
> > +          - librdmacm-dev
> > +          - libsasl2-dev
> > +          - libsdl2-dev
> > +          - libseccomp-dev
> > +          - libsnappy-dev
> > +          - libspice-protocol-dev
> > +          - libssh-dev
> > +          - libusb-1.0-0-dev
> > +          - libusbredirhost-dev
> > +          - libvdeplug-dev
> > +          - libvte-2.91-dev
> > +          - libzstd-dev
> > +          - make
> > +          - ninja-build
> > +          - python3-yaml
> > +          - python3-sphinx
> > +          - sparse
> > +          - xfslibs-dev
> > +        state: present
> > +      when:
> > +        - ansible_facts['distribution'] == 'Ubuntu'
> > +
> > +    - name: Install packages to build QEMU on Ubuntu 18.04/20.04 on 
> > non-s390x
> > +      package:
> > +        name:
> > +          - libspice-server-dev
> > +          - libxen-dev
> > +        state: present
> > +      when:
> > +        - ansible_facts['distribution'] == 'Ubuntu'
> > +        - ansible_facts['architecture'] != 's390x'
> > diff --git a/scripts/ci/setup/inventory b/scripts/ci/setup/inventory
> > new file mode 100644
> > index 0000000000..2fbb50c4a8
> > --- /dev/null
> > +++ b/scripts/ci/setup/inventory
> > @@ -0,0 +1 @@
> > +localhost
> 
> I'm not sure we should have a default here because it will inevitably
> cause someone to do something to their machine when trying to setup a
> runner.
>

Fair enough.  Then I see two options:

1) follow the vars.yml.template example and only ship a
   inventory.template file

2) use a placeholder with an impossible hostname such as
   "my-qemu-runner.example.org" or "your-host-name-here"

> -- 
> Alex Bennée
> 

Let me know what you think is more reasonable, and thanks for the
review!

Regards,
- Cleber.

signature.asc
Description: PGP signature