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. 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``
+
+ * 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.
 
 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=
 
 TESTS ?= %
 IMAGES ?= %
@@ -220,6 +223,7 @@ docker:
 	@echo '    docker-image:        Build all images.'
 	@echo '    docker-image-IMAGE:  Build image "IMAGE".'
 	@echo '    docker-run:          For manually running a "TEST" with "IMAGE".'
+	@echo '    docker-refresh:      Re-generate dockerfiles after updating libvirt-ci'
 	@echo
 	@echo 'Available container images:'
 	@echo '    $(DOCKER_IMAGES)'
@@ -302,3 +306,11 @@ docker-run-%:
 
 docker-clean:
 	$(call quiet-command, $(DOCKER_SCRIPT) clean)
+
+docker-refresh:
+	@if [ -z "$(LCITOOL)" ] ; \
+	then \
+	    echo "No lcitool path specified. Use 'make docker-refresh LCITOOL=/path/to/libvirt-ci.git/lcitool'"; \
+	    exit 1; \
+	fi
+	$(call quiet-command, $(REFRESH_SCRIPT) $(LCITOOL) $(SRC_PATH))
diff --git a/tests/docker/dockerfiles-refresh.py b/tests/docker/dockerfiles-refresh.py
new file mode 100755
index 0000000000..7f59ffbc59
--- /dev/null
+++ b/tests/docker/dockerfiles-refresh.py
@@ -0,0 +1,56 @@
+#!/usr/bin/python3
+#
+# Re-generate container recipes
+#
+# This script uses the "lcitool" available from
+#
+#   https://gitlab.com/libvirt/libvirt-ci
+#
+# Copyright (c) 2020 Red Hat Inc.
+#
+# This work is licensed under the terms of the GNU GPL, version 2
+# or (at your option) any later version. See the COPYING file in
+# the top-level directory.
+
+import sys
+import os
+import os.path
+import subprocess
+
+if len(sys.argv) != 3:
+   print("syntax: %s PATH-TO-LCITOOL SRC-ROOT" % sys.argv[0], file=sys.stderr)
+   sys.exit(1)
+
+lcitool_path=sys.argv[1]
+src_root=sys.argv[2]
+
+def atomic_write(filename, content):
+   dst = os.path.join(src_root, "tests", "docker", "dockerfiles", filename)
+   try:
+      with open(dst + ".tmp", "w") as fp:
+         print(content, file=fp, end="")
+         os.replace(dst + ".tmp", dst)
+   except Exception as ex:
+      os.unlink(dst + ".tmp")
+      raise
+
+def generate_image(filename, host, cross=None, trailer=None):
+   print("Generate %s" % filename)
+   args = [lcitool_path, "dockerfile"]
+   if cross is not None:
+      args.extend(["--cross", cross])
+   args.extend([host, "qemu"])
+   lcitool=subprocess.run(args, capture_output=True)
+
+   if lcitool.returncode != 0:
+      raise Exception("Failed to generate %s: %s" % (filename, lcitool.stderr))
+
+   content = lcitool.stdout.decode("utf8")
+   if trailer is not None:
+      content += trailer
+   atomic_write(filename, content)
+
+try:
+   pass
+except Exception as ex:
+   print(str(ex), file=sys.stderr)
-- 
2.31.1

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

On Mon, Jul 05, 2021 at 02:44:34PM +0100, Alex Bennée wrote:
> 
> 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?

Yes, this is something I've been struggling with, since there
are varying use cases here.

The "lcitool" command was originally written to automate the
provisioning of virtual machines, suitable to act as runners
for a CI tool.

The VMs would be suitable for building a variety of projects,
so would need to be installed with the dependancies of all
projects. It makes sense to have the list of dependancies
in one central place. If you have them kept in each respective
project's git repo, then you have to checkout 20 git repos to
get their dependancies, before you can provision the VM.

It still supports VM provisioning, but now also supports
the Dockerfile generation too in parallel. With the
dockerfiles, you still have a need to access the dependancy
information from outside the main project. For example,
when building libvirt-perl.git, it wants to know the
dependancies needed by libvirt.git, so that it can do
a chained build of the two.

Potentially libvirt would also want to build qemu.git
first, so it can then test libvirt with latest QEMU.

So these things all end up driving towards the idea of
storing the build dependancies separate from the project.

It is definitely sub-optimal though, in that it introduces
a synchronization problem between the 2 respective git
repos for changes.

For libvirt we've mostly just accepted that pain of needing
to merge stuff in lock-step, but I think it is worse when
dealing with QEMU becasue the subsystem maintainer model
means stuff merges in 2 phases, so there's not a ideal
synchronization point.

> > 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?

I did have a submodule in the previous posting, but that creates its
own pain, because there's a chicken and egg problem to updates. Stuff
won't want to be merged into libvirt-ci.git until it is accepted by
a QEMU maintainer, but we need the submodule update ready before
it can be accepted by the QEMU maintainer. There's no nice way to
break that cycle without introducing a bit of manual work and
synchoronization at time of merge to master, which is not desirable
for QEMU IMHO

> > + * 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.


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

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

> On Mon, Jul 05, 2021 at 02:44:34PM +0100, Alex Bennée wrote:
>> 
>> 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?
>
> Yes, this is something I've been struggling with, since there
> are varying use cases here.
>
> The "lcitool" command was originally written to automate the
> provisioning of virtual machines, suitable to act as runners
> for a CI tool.
>
> The VMs would be suitable for building a variety of projects,
> so would need to be installed with the dependancies of all
> projects. It makes sense to have the list of dependancies
> in one central place. If you have them kept in each respective
> project's git repo, then you have to checkout 20 git repos to
> get their dependancies, before you can provision the VM.
>
> It still supports VM provisioning, but now also supports
> the Dockerfile generation too in parallel. With the
> dockerfiles, you still have a need to access the dependancy
> information from outside the main project. For example,
> when building libvirt-perl.git, it wants to know the
> dependancies needed by libvirt.git, so that it can do
> a chained build of the two.
>
> Potentially libvirt would also want to build qemu.git
> first, so it can then test libvirt with latest QEMU.
>
> So these things all end up driving towards the idea of
> storing the build dependancies separate from the project.
>
> It is definitely sub-optimal though, in that it introduces
> a synchronization problem between the 2 respective git
> repos for changes.
>
> For libvirt we've mostly just accepted that pain of needing
> to merge stuff in lock-step, but I think it is worse when
> dealing with QEMU becasue the subsystem maintainer model
> means stuff merges in 2 phases, so there's not a ideal
> synchronization point.
>
>> > 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?
>
> I did have a submodule in the previous posting, but that creates its
> own pain, because there's a chicken and egg problem to updates. Stuff
> won't want to be merged into libvirt-ci.git until it is accepted by
> a QEMU maintainer, but we need the submodule update ready before
> it can be accepted by the QEMU maintainer. There's no nice way to
> break that cycle without introducing a bit of manual work and
> synchoronization at time of merge to master, which is not desirable
> for QEMU IMHO

Can't lcitool be improved to accept out-of-its-tree metadata? 

>> > + * 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.
>
>
> Regards,
> Daniel


-- 
Alex Bennée