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

[Cleber Rosa]

On 6/9/21 1:16 PM, Willian Rampazzo wrote:
> On Tue, Jun 8, 2021 at 12:14 AM Cleber Rosa <crosa@redhat.com> 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::
> > +
> > +  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
> > +
> As I mentioned to Wainer, my suggestion is to include a comment about
> ansible_become=yes and
> ansible_become_password= variables as some systems may need it.


OK.  I mentioned in a previous reply that I'll plan to do this slightly 
different, that is, to mention the ansible-playbook command line 
options.  Let me know if that works for you.


> > +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
> > +  tasks:
> > +    - name: Update apt cache
> > +      apt:
> > +        update_cache: yes
> On a freshly installed Ubuntu 20.04, the script failed for me with
> dependency messages on the apt side. After I updated the packages on
> the system, the playbook worked without problems.


I know it's too late now, but it would have been helpful to document the 
exact error, at the very least here in this thread. I tried to replicate 
with an also fresh install, but resulted in:


PLAY RECAP 
**************************************************************************************************************************************
192.168.122.19             : ok=6    changed=4    unreachable=0 
failed=0    skipped=1    rescued=0    ignored=0


A second run resulted in:

PLAY [Installation of basic packages to build QEMU] 
*********************************************************************************************

TASK [Gathering Facts] 
**************************************************************************************************************************
ok: [192.168.122.19]

TASK [Check for suitable ansible version] 
*******************************************************************************************************
ok: [192.168.122.19] => {
    "changed": false,
    "msg": "All assertions passed"
}

TASK [Update apt cache] 
*************************************************************************************************************************
ok: [192.168.122.19]

TASK [Install basic packages to build QEMU on Ubuntu 18.04/20.04] 
*******************************************************************************
ok: [192.168.122.19]

TASK [Install packages to build QEMU on Ubuntu 18.04/20.04 on non-s390x] 
************************************************************************
ok: [192.168.122.19]

TASK [Install basic packages to build QEMU on Ubuntu 18.04] 
*************************************************************************************
skipping: [192.168.122.19]

TASK [Install basic packages to build QEMU on Ubuntu 20.04] 
*************************************************************************************
ok: [192.168.122.19]

PLAY RECAP 
**************************************************************************************************************************************
192.168.122.19             : ok=6    changed=0    unreachable=0 
failed=0    skipped=1    rescued=0    ignored=0


> So, my suggestion is to add the "update = yes" here, or add a note in
> the documentation asking the user to update the system before running
> the playbook.


IIUC, you mean an "upgrade: yes" on the "Update apt cache" task, right?  
Something like:


-    - name: Update apt cache
+    - name: Update apt cache / upgrade packages via apt
       apt:
         update_cache: yes
+        upgrade: yes

I have mixed feelings about going beyond installing/updating the 
packages we care about.  On one hand, it's a good sysadmin practice, but 
on the other hand, development/test machines are sometimes peculiar.  
Since you and Erik are in favor of the updates, I'll change that and 
wait for feedback on v7.


So, besides the change to the playbook, I'm adding to the docs:


The ``scripts/ci/setup/build-environment.yml`` Ansible playbook will
set up machines with the environment needed to perform builds and run
QEMU tests.  This playbook consists on the installation of various
required packages (and a general package update while at it).  It
currently covers a number of different Linux distributions, but it can
be expanded to cover other systems.


> Except for the above comment and Wainer's comments, it looks good to
> me. With these changes:
>
> Reviewed-by: Willian Rampazzo <willianr@redhat.com>
> Tested-by: Willian Rampazzo <willianr@redhat.com>

Thanks for the review and test!  Given the amount of changes, I'll hold 
a bit to take you Rb/Tb until you see/test v7.

Thanks again!

- Cleber