Re: [PATCH v4 16/22] tests/docker: add script for automating container refresh

["Alex Bennée" <alex.bennee@linaro.org>]

Daniel P. Berrangé <berrange@redhat.com> writes:

> This introduces
>
>   https://gitlab.com/libvirt/libvirt-ci
>
> as a git submodule at tests/docker/libvirt-ci
>
> This submodule only needs to be checked out when needing to re-generate
> the files in tests/docker/dockerfiles.
>
> When a new build pre-requisite is needed for QEMU, it should be added to
> the libvirt-ci project 'qemu.yml' file, and the submodule updated to the
> new commit.

It seems a bit weird to have the canonical description of QEMU
dependencies live in another project does it not?

> The 'make docker-refresh' target will then re-create all the
> dockerfiles with updated package lists. This ensures that all the
> containers get exactly the same build pre-requisite packages installed.
>
> It also facilitates the addition of containers targetting new distros
> or updating existing containers to new versions of the same distro,
> where packages might have been renamed.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  docs/devel/testing.rst              | 34 ++++++++++++++++--
>  tests/docker/Makefile.include       | 12 +++++++
>  tests/docker/dockerfiles-refresh.py | 56 +++++++++++++++++++++++++++++
>  3 files changed, 100 insertions(+), 2 deletions(-)
>  create mode 100755 tests/docker/dockerfiles-refresh.py
>
> diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst
> index 4e42392810..7882db85d4 100644
> --- a/docs/devel/testing.rst
> +++ b/docs/devel/testing.rst
> @@ -372,8 +372,38 @@ Along with many other images, the ``centos8`` image is defined in a Dockerfile
>  in ``tests/docker/dockerfiles/``, called ``centos8.docker``. ``make docker-help``
>  command will list all the available images.
>  
> -To add a new image, simply create a new ``.docker`` file under the
> -``tests/docker/dockerfiles/`` directory.
> +Most of the existing Dockerfiles were written by hand, simply by creating a
> +a new ``.docker`` file under the ``tests/docker/dockerfiles/`` directory.
> +This has led to an inconsistent set of packages being present across the
> +different containers.
> +
> +Thus going forward, QEMU is aiming to automatically generate the Dockerfiles
> +using the ``lcitool`` program provided by the ``libvirt-ci`` project:
> +
> +  https://gitlab.com/libvirt/libvirt-ci
> +
> +In that project, there is a ``qemu.yml`` file defining the list of build
> +pre-requisites needed by QEMU. This is processed together with the
> +``mappings.yml`` file to compute the distro specific list of package names.
> +The package names are then fed into a generator which emits a well structured
> +dockerfile. The set of dockerfiles which are auto-generated is defined in
> +the ``tests/docker/dockerfiles-refresh.py`` script.
> +
> +When preparing a patch series that changes dockerfiles managed by ``libvirt-ci``
> +tools, the following steps should be takenL
> +
> + * Fork the ``libvirt-ci`` project on gitlab
> +
> + * Prepare changes to its ``qemu.yml`` file and optionally ``mappings.yml``
> +   to define the packages to be added to QEMU's dockerfiles.
> +
> + * In QEMU run ``make docker-refresh LCITOOL=/path/to/libvirt-ci/lcitool``
> +   to re-create the dockerfiles in ``tests/docker/dockerfiles``

If lcitool could be a pre-requisite (even as a developer only one)
should we consider having a submodule and QEMU mirror of it?

> +
> + * Submit your changes to QEMU in the normal manner
> +
> + * Submit ``libvirt-ci`` changes as a merge request, linking to the
> +   QEMU patch series that uses them.

This just seems clunky and likely to therefor not get used. I would
prefer keeping the meta-data within the project, maybe with a check that
ensures the dockerfiles have not gone out of sync with their "idealised"
form.

>  
>  A ``.pre`` script can be added beside the ``.docker`` file, which will be
>  executed before building the image under the build context directory. This is
> diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include
> index ff5d732889..e8e5f497cc 100644
> --- a/tests/docker/Makefile.include
> +++ b/tests/docker/Makefile.include
> @@ -27,6 +27,9 @@ DOCKER_TESTS := $(notdir $(shell \
>  ENGINE := auto
>  
>  DOCKER_SCRIPT=$(SRC_PATH)/tests/docker/docker.py --engine $(ENGINE)
> +REFRESH_SCRIPT=$(SRC_PATH)/tests/docker/dockerfiles-refresh.py
> +# To be set to /path/to/checkout/of/libvirt-ci.git/lcitool
> +LCITOOL=

Another argument for a sub-module?

-- 
Alex Bennée