[libvirt-jenkins-ci PATCH] guests: allow for container image inheritance

Daniel P. Berrangé posted 1 patch 4 years ago
Failed in applying to current master (apply log)
guests/lcitool | 108 ++++++++++++++++++++++++++++++++++++++++++++-----
1 file changed, 98 insertions(+), 10 deletions(-)
[libvirt-jenkins-ci PATCH] guests: allow for container image inheritance
Posted by Daniel P. Berrangé 4 years ago
Currently when creating a Dockerfile for a container, we include the
full set of base packages, along with the packages for the project
itself. If building a Perl binding, this would require us to install
the base package, libvirt packages and Perl packages. With the use
of the "--inherit libvirt-fedora-30" arg, it is possible to have a
dockerfile that only adds the Perl packages, getting everything
else from the parent image.

For example, a full Dockerfile for libvirt-go would thus be:

  FROM libvirt-centos-7:latest

  RUN yum install -y \
          golang && \
      yum autoremove -y && \
      yum clean all -y

Note there is no need to set ENV either, as that's inherited from the
parent container.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 guests/lcitool | 108 ++++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 98 insertions(+), 10 deletions(-)

diff --git a/guests/lcitool b/guests/lcitool
index 689a8cf..b3afb6a 100755
--- a/guests/lcitool
+++ b/guests/lcitool
@@ -396,6 +396,12 @@ class Application:
                 help="target architecture for cross compiler",
             )
 
+        def add_inherit_arg(parser):
+            parser.add_argument(
+                "-i", "--inherit",
+                help="inherit from an intermediate container image",
+            )
+
         def add_wait_arg(parser):
             parser.add_argument(
                 "-w", "--wait",
@@ -442,6 +448,7 @@ class Application:
         add_hosts_arg(dockerfileparser)
         add_projects_arg(dockerfileparser)
         add_cross_arch_arg(dockerfileparser)
+        add_inherit_arg(dockerfileparser)
 
     def _execute_playbook(self, playbook, hosts, projects, git_revision):
         base = Util.get_base()
@@ -644,11 +651,11 @@ class Application:
         with open(keyfile, "r") as r:
             return r.read().rstrip()
 
-    def _dockerfile_build_varmap(self, facts, mappings, pip_mappings, projects, cross_arch):
+    def _dockerfile_build_varmap(self, facts, mappings, pip_mappings, projects, cross_arch, needbase):
         if facts["package_format"] == "deb":
-            varmap = self._dockerfile_build_varmap_deb(facts, mappings, pip_mappings, projects, cross_arch)
+            varmap = self._dockerfile_build_varmap_deb(facts, mappings, pip_mappings, projects, cross_arch, needbase)
         if facts["package_format"] == "rpm":
-            varmap = self._dockerfile_build_varmap_rpm(facts, mappings, pip_mappings, projects, cross_arch)
+            varmap = self._dockerfile_build_varmap_rpm(facts, mappings, pip_mappings, projects, cross_arch, needbase)
 
         varmap["package_manager"] = facts["package_manager"]
         varmap["cc"] = facts["cc"]
@@ -662,7 +669,7 @@ class Application:
 
         return varmap
 
-    def _dockerfile_build_varmap_deb(self, facts, mappings, pip_mappings, projects, cross_arch):
+    def _dockerfile_build_varmap_deb(self, facts, mappings, pip_mappings, projects, cross_arch, needbase):
         package_format = facts["package_format"]
         package_manager = facts["package_manager"]
         os_name = facts["os_name"]
@@ -682,7 +689,10 @@ class Application:
 
         # We need to add the base project manually here: the standard
         # machinery hides it because it's an implementation detail
-        for project in projects + ["base"]:
+        pkgprojects = projects
+        if needbase:
+            pkgprojects = projects + ["base"]
+        for project in pkgprojects:
             for package in self._projects.get_packages(project):
                 cross_policy = "native"
                 for key in cross_keys:
@@ -730,7 +740,7 @@ class Application:
 
         return varmap
 
-    def _dockerfile_build_varmap_rpm(self, facts, mappings, pip_mappings, projects, cross_arch):
+    def _dockerfile_build_varmap_rpm(self, facts, mappings, pip_mappings, projects, cross_arch, needbase):
         package_format = facts["package_format"]
         package_manager = facts["package_manager"]
         os_name = facts["os_name"]
@@ -744,7 +754,10 @@ class Application:
 
         # We need to add the base project manually here: the standard
         # machinery hides it because it's an implementation detail
-        for project in projects + ["base"]:
+        pkgprojects = projects
+        if needbase:
+            pkgprojects = projects + ["base"]
+        for project in pkgprojects:
             for package in self._projects.get_packages(project):
                 for key in keys:
                     if key in mappings[package]:
@@ -791,7 +804,7 @@ class Application:
 
         return varmap
 
-    def _dockerfile_format(self, facts, cross_arch, varmap):
+    def _dockerfile_base_format(self, facts, cross_arch, varmap):
         package_format = facts["package_format"]
         package_manager = facts["package_manager"]
         os_name = facts["os_name"]
@@ -931,6 +944,74 @@ class Application:
                 ENV CONFIGURE_OPTS "--host={cross_abi}"
             """).format(**varmap))
 
+    def _dockerfile_child_format(self, facts, cross_arch, inherit, varmap):
+        package_format = facts["package_format"]
+        package_manager = facts["package_manager"]
+        os_name = facts["os_name"]
+        os_version = facts["os_version"]
+
+        print("FROM {}".format(inherit))
+
+        commands = []
+
+        if package_format == "deb":
+            commands.extend([
+                "export DEBIAN_FRONTEND=noninteractive",
+                "{package_manager} update",
+                "{package_manager} install --no-install-recommends -y {pkgs}",
+                "{package_manager} autoremove -y",
+                "{package_manager} autoclean -y",
+            ])
+        elif package_format == "rpm":
+            commands.extend([
+                "{package_manager} install -y {pkgs}",
+            ])
+
+            # openSUSE doesn't seem to have a convenient way to remove all
+            # unnecessary packages, but CentOS and Fedora do
+            if os_name == "OpenSUSE":
+                commands.extend([
+                    "{package_manager} clean --all",
+                ])
+            else:
+                commands.extend([
+                    "{package_manager} autoremove -y",
+                    "{package_manager} clean all -y",
+                ])
+
+
+        script = "\nRUN " + (" && \\\n    ".join(commands)) + "\n"
+        sys.stdout.write(script.format(**varmap))
+
+        if cross_arch:
+            cross_commands = []
+
+            # Intentionally a separate RUN command from the above
+            # so that the common packages of all cross-built images
+            # share a Docker image layer.
+            if package_format == "deb":
+                cross_commands.extend([
+                    "export DEBIAN_FRONTEND=noninteractive",
+                    "{package_manager} update",
+                    "{package_manager} install --no-install-recommends -y {cross_pkgs}",
+                    "{package_manager} autoremove -y",
+                    "{package_manager} autoclean -y",
+                ])
+            elif package_format == "rpm":
+                cross_commands.extend([
+                    "{package_manager} install -y {cross_pkgs}",
+                    "{package_manager} clean all -y",
+                ])
+
+            cross_script = "\nRUN " + (" && \\\n    ".join(cross_commands)) + "\n"
+            sys.stdout.write(cross_script.format(**varmap))
+
+        if "pip_pkgs" in varmap:
+            sys.stdout.write(textwrap.dedent("""
+                RUN pip3 install {pip_pkgs}
+            """).format(**varmap))
+
+
     def _action_dockerfile(self, args):
         mappings = self._projects.get_mappings()
         pip_mappings = self._projects.get_pip_mappings()
@@ -947,6 +1028,7 @@ class Application:
         os_version = facts["os_version"]
         os_full = os_name + os_version
         cross_arch = args.cross_arch
+        inherit = args.inherit
 
         if package_format not in ["deb", "rpm"]:
             raise Exception("Host {} doesn't support Dockerfiles".format(host))
@@ -983,8 +1065,14 @@ class Application:
                     )
                 )
 
-        varmap = self._dockerfile_build_varmap(facts, mappings, pip_mappings, projects, cross_arch)
-        self._dockerfile_format(facts, cross_arch, varmap)
+        needbase = True
+        if inherit is not None:
+            needbase = False
+        varmap = self._dockerfile_build_varmap(facts, mappings, pip_mappings, projects, cross_arch, needbase)
+        if inherit is None:
+            self._dockerfile_base_format(facts, cross_arch, varmap)
+        else:
+            self._dockerfile_child_format(facts, cross_arch, inherit, varmap)
 
     def run(self):
         args = self._parser.parse_args()
-- 
2.24.1

Re: [libvirt-jenkins-ci PATCH] guests: allow for container image inheritance
Posted by Andrea Bolognani 3 years, 12 months ago
On Tue, 2020-03-31 at 15:28 +0100, Daniel P. Berrangé wrote:
> Currently when creating a Dockerfile for a container, we include the
> full set of base packages, along with the packages for the project
> itself. If building a Perl binding, this would require us to install
> the base package, libvirt packages and Perl packages. With the use
> of the "--inherit libvirt-fedora-30" arg, it is possible to have a
> dockerfile that only adds the Perl packages, getting everything
> else from the parent image.
> 
> For example, a full Dockerfile for libvirt-go would thus be:
> 
>   FROM libvirt-centos-7:latest
> 
>   RUN yum install -y \
>           golang && \
>       yum autoremove -y && \
>       yum clean all -y
> 
> Note there is no need to set ENV either, as that's inherited from the
> parent container.

I marked this for review and then kept forgetting to get around to
it, sorry!

I like the idea of reusing existing images, as the layering would
result in significant savings when it comes to disk space and fetch
times.

However, as we discussed in the past, there are two possible
scenarios for subprojects such as libvirt-go:

  * include dependencies for both libvirt and libvirt-go, then build
    both projects in the resulting container;

  * include dependencies for libvirt-go along with distro-provided
    packages for libvirt, and only build libvirt-go.

This would cover the former case, but not the latter one. And, if I
remember correctly, libvirt-go was one of the projects that would
benefit more from the latter, because it's supposed to be buildable
against various versions of libvirt.

So what I think we need is an additional flag that can be used to
choose one of the two possible behaviors. This wouldn't be limited
to the Dockerfile generator, since (unlike inheritance) it can apply
also to VM management.

As an additional point, we really need to figure out a good way to
store dependencies between projects into lcitool itself, so that you
can tell it that you're interested in building eg. libosinfo and it
will automatically take care of making osinfo-db-tools and osinfo-db
available to you, either by installing the binary packages or their
build dependencies. This is not a strict requirement for container
inheritance, I think, but the more we go on the more this limitation
is becoming painful.

-- 
Andrea Bolognani / Red Hat / Virtualization

Re: [libvirt-jenkins-ci PATCH] guests: allow for container image inheritance
Posted by Daniel P. Berrangé 3 years, 12 months ago
On Wed, Apr 29, 2020 at 11:10:41AM +0200, Andrea Bolognani wrote:
> On Tue, 2020-03-31 at 15:28 +0100, Daniel P. Berrangé wrote:
> > Currently when creating a Dockerfile for a container, we include the
> > full set of base packages, along with the packages for the project
> > itself. If building a Perl binding, this would require us to install
> > the base package, libvirt packages and Perl packages. With the use
> > of the "--inherit libvirt-fedora-30" arg, it is possible to have a
> > dockerfile that only adds the Perl packages, getting everything
> > else from the parent image.
> > 
> > For example, a full Dockerfile for libvirt-go would thus be:
> > 
> >   FROM libvirt-centos-7:latest
> > 
> >   RUN yum install -y \
> >           golang && \
> >       yum autoremove -y && \
> >       yum clean all -y
> > 
> > Note there is no need to set ENV either, as that's inherited from the
> > parent container.
> 
> I marked this for review and then kept forgetting to get around to
> it, sorry!
> 
> I like the idea of reusing existing images, as the layering would
> result in significant savings when it comes to disk space and fetch
> times.
> 
> However, as we discussed in the past, there are two possible
> scenarios for subprojects such as libvirt-go:
> 
>   * include dependencies for both libvirt and libvirt-go, then build
>     both projects in the resulting container;
> 
>   * include dependencies for libvirt-go along with distro-provided
>     packages for libvirt, and only build libvirt-go.
> 
> This would cover the former case, but not the latter one. And, if I
> remember correctly, libvirt-go was one of the projects that would
> benefit more from the latter, because it's supposed to be buildable
> against various versions of libvirt.
> 
> So what I think we need is an additional flag that can be used to
> choose one of the two possible behaviors. This wouldn't be limited
> to the Dockerfile generator, since (unlike inheritance) it can apply
> also to VM management.

I think this problem is tangential to container inheritance and so
doesn't need to be dealt with here.

Instead, it should be solved by simply defining another project
"libvirt-devel", or "libvirt-dist" which pulls in the pre-built
distro packages for libvirt.

> As an additional point, we really need to figure out a good way to
> store dependencies between projects into lcitool itself, so that you
> can tell it that you're interested in building eg. libosinfo and it
> will automatically take care of making osinfo-db-tools and osinfo-db
> available to you, either by installing the binary packages or their
> build dependencies. This is not a strict requirement for container
> inheritance, I think, but the more we go on the more this limitation
> is becoming painful.

I'm not really experiencing this as a painpoint from the container CI
side.

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

Re: [libvirt-jenkins-ci PATCH] guests: allow for container image inheritance
Posted by Andrea Bolognani 3 years, 12 months ago
On Wed, 2020-04-29 at 10:21 +0100, Daniel P. Berrangé wrote:
> On Wed, Apr 29, 2020 at 11:10:41AM +0200, Andrea Bolognani wrote:
> > So what I think we need is an additional flag that can be used to
> > choose one of the two possible behaviors. This wouldn't be limited
> > to the Dockerfile generator, since (unlike inheritance) it can apply
> > also to VM management.
> 
> I think this problem is tangential to container inheritance and so
> doesn't need to be dealt with here.
> 
> Instead, it should be solved by simply defining another project
> "libvirt-devel", or "libvirt-dist" which pulls in the pre-built
> distro packages for libvirt.

Yeah, the additional projects introduced in the patch you posted
yesterday cover this use case quite nicely without having to
introduce any additional behaviors in lcitool.

> > As an additional point, we really need to figure out a good way to
> > store dependencies between projects into lcitool itself, so that you
> > can tell it that you're interested in building eg. libosinfo and it
> > will automatically take care of making osinfo-db-tools and osinfo-db
> > available to you, either by installing the binary packages or their
> > build dependencies. This is not a strict requirement for container
> > inheritance, I think, but the more we go on the more this limitation
> > is becoming painful.
> 
> I'm not really experiencing this as a painpoint from the container CI
> side.

Well, that's because you know what the dependencies between various
projects are by heart ;)

But again, the introduction of project variants that install the
distro-provided dependencies, along with the proper integration with
GitLab CI, will mitigate this concern to a large degree.

-- 
Andrea Bolognani / Red Hat / Virtualization

Re: [libvirt-jenkins-ci PATCH] guests: allow for container image inheritance
Posted by Daniel P. Berrangé 3 years, 12 months ago
On Thu, Apr 30, 2020 at 10:27:30AM +0200, Andrea Bolognani wrote:
> On Wed, 2020-04-29 at 10:21 +0100, Daniel P. Berrangé wrote:
> > On Wed, Apr 29, 2020 at 11:10:41AM +0200, Andrea Bolognani wrote:
> > > So what I think we need is an additional flag that can be used to
> > > choose one of the two possible behaviors. This wouldn't be limited
> > > to the Dockerfile generator, since (unlike inheritance) it can apply
> > > also to VM management.
> > 
> > I think this problem is tangential to container inheritance and so
> > doesn't need to be dealt with here.
> > 
> > Instead, it should be solved by simply defining another project
> > "libvirt-devel", or "libvirt-dist" which pulls in the pre-built
> > distro packages for libvirt.
> 
> Yeah, the additional projects introduced in the patch you posted
> yesterday cover this use case quite nicely without having to
> introduce any additional behaviors in lcitool.
> 
> > > As an additional point, we really need to figure out a good way to
> > > store dependencies between projects into lcitool itself, so that you
> > > can tell it that you're interested in building eg. libosinfo and it
> > > will automatically take care of making osinfo-db-tools and osinfo-db
> > > available to you, either by installing the binary packages or their
> > > build dependencies. This is not a strict requirement for container
> > > inheritance, I think, but the more we go on the more this limitation
> > > is becoming painful.
> > 
> > I'm not really experiencing this as a painpoint from the container CI
> > side.
> 
> Well, that's because you know what the dependencies between various
> projects are by heart ;)

The dependancies just aren't that complicated - most of them simply
depend on libvirt, and we're already recording this info in the
apps eg in their configure.ac scripts, and RPM spec files. So I'm
not really seeing a clear need to record this dependancy info the
libvit-ci project as well.

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

Re: [libvirt-jenkins-ci PATCH] guests: allow for container image inheritance
Posted by Daniel P. Berrangé 3 years, 12 months ago
On Tue, Mar 31, 2020 at 03:28:20PM +0100, Daniel P. Berrangé wrote:
> Currently when creating a Dockerfile for a container, we include the
> full set of base packages, along with the packages for the project
> itself. If building a Perl binding, this would require us to install
> the base package, libvirt packages and Perl packages. With the use
> of the "--inherit libvirt-fedora-30" arg, it is possible to have a
> dockerfile that only adds the Perl packages, getting everything
> else from the parent image.
> 
> For example, a full Dockerfile for libvirt-go would thus be:
> 
>   FROM libvirt-centos-7:latest
> 
>   RUN yum install -y \
>           golang && \
>       yum autoremove -y && \
>       yum clean all -y
> 
> Note there is no need to set ENV either, as that's inherited from the
> parent container.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  guests/lcitool | 108 ++++++++++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 98 insertions(+), 10 deletions(-)

While this is a conceptually useful feature, I'm not actually intending
to use it anymore. Instead the new libvirt-minimal and libvirt-dist
projets would be used to create a self-contained dockerfile for downstream
components as illustrated with

ci      https://www.redhat.com/archives/libvir-list/2020-April/msg01407.html
python  https://www.redhat.com/archives/libvir-list/2020-April/msg01418.html
perl    https://www.redhat.com/archives/libvir-list/2020-April/msg01408.html


It could still be useful if QEMU adopts lcitool since their dockerfiles
currently make use of inheritance

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