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

[Cleber Rosa]

On Tue, Jun 8, 2021 at 2:48 PM Wainer dos Santos Moschetta
<wainersm@redhat.com> wrote:
>
> Hi,
>
> On 6/8/21 12:14 AM, Cleber Rosa wrote:
> > 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 | 98 ++++++++++++++++++++++++++
> >   scripts/ci/setup/inventory.template    |  1 +
> >   3 files changed, 129 insertions(+)
> >   create mode 100644 scripts/ci/setup/build-environment.yml
> >   create mode 100644 scripts/ci/setup/inventory.template
> >
> > diff --git a/docs/devel/ci.rst b/docs/devel/ci.rst
> > index 585b7bf4b8..35c6b5e269 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.  Create an ``inventory`` file
> > +under ``scripts/ci/setup``, such as this::
> Missing to mention the template file.
> > +
> > +  fully.qualified.domain
> > +  other.machine.hostname
> > +
> > +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
> > diff --git a/scripts/ci/setup/build-environment.yml 
> > b/scripts/ci/setup/build-environment.yml
> > new file mode 100644
> > index 0000000000..664f2f0519
> > --- /dev/null
> > +++ b/scripts/ci/setup/build-environment.yml
> > @@ -0,0 +1,98 @@
> > +---
> > +- name: Installation of basic packages to build QEMU
> > +  hosts: all
>
> You will need to "become: yes" if the login user is not privileged, right?
>

Hi Wainer,

Yes, pretty much all of the actions here need a privileged user.  I'm
not sure how much of Ansible basics we should be replicating here.  In
my experience, users running "ansible-playbook" will resort to a
combination of --become, --become-method, --ask-become-pass, etc, if
needed.

> Or mention on the documentation how the user should configure the
> connection for privileged login.
>

Yes, I think it's better to just put a note/reference, instead of
inundating the user and the playbook with possibly unnecessary
configurations and details.  I've added the following to the
documentation:

Please note that most of the tasks in the playbook require superuser
privileges, such as those from the ``root`` account or those obtained
by ``sudo``.  If necessary, please refer to ``ansible-playbook`` options
such as ``--become``, ``--become-method``, ``--become-user`` and
``--ask-become-pass``.

Let me know if that sounds good to you.

> About privilege escalation with Ansible:
> https://docs.ansible.com/ansible/latest/user_guide/become.html
>
> > +  tasks:
>
> Just a tip: you can put all those task under a block
> (https://docs.ansible.com/ansible/latest/user_guide/playbooks_blocks.html)
> so check if "ansible_facts['distribution'] == 'Ubuntu'" only once.
>

There will be other non-Ubuntu tasks added later (such as in the
CentOS Stream 8 series), so I think it's better to keep those
separate.  Also, the Ansible version check which Alex suggested is not
limited to Ubuntu.

> I reviewed the remain of the playbook; it looks good to me.
>

Thanks!
- Cleber.