change builder API to return imageID

this allows more flexibility running CLI builder, especially we don't
have anymore to parse build output to get build status/imageID and can
pass the console FDs to buildkit

Signed-off-by: Nicolas De Loof <[email protected]>

compose/service.py

@@ -1,6 +1,5 @@
 import enum
 import itertools
-import json
 import logging
 import os
 import re
@@ -1125,8 +1124,9 @@ class Service:
                 'Impossible to perform platform-targeted builds for API version < 1.35'
             )
 
-        builder = self.client if not cli else _CLIBuilder(progress)
-        build_output = builder.build(
+        builder = _ClientBuilder(self.client) if not cli else _CLIBuilder(progress)
+        return builder.build(
+            service=self,
             path=path,
             tag=self.image_name,
             rm=rm,
@@ -1147,30 +1147,7 @@ class Service:
             gzip=gzip,
             isolation=build_opts.get('isolation', self.options.get('isolation', None)),
             platform=self.platform,
-        )
-
-        try:
-            all_events = list(stream_output(build_output, output_stream))
-        except StreamOutputError as e:
-            raise BuildError(self, str(e))
-
-        # Ensure the HTTP connection is not reused for another
-        # streaming command, as the Docker daemon can sometimes
-        # complain about it
-        self.client.close()
-
-        image_id = None
-
-        for event in all_events:
-            if 'stream' in event:
-                match = re.search(r'Successfully built ([0-9a-f]+)', event.get('stream', ''))
-                if match:
-                    image_id = match.group(1)
-
-        if image_id is None:
-            raise BuildError(self, event if all_events else 'Unknown')
-
-        return image_id
+            output_stream=output_stream)
 
     def get_cache_from(self, build_opts):
         cache_from = build_opts.get('cache_from', None)
@@ -1827,20 +1804,77 @@ def rewrite_build_path(path):
     return path
 
 
+class _ClientBuilder:
+    def __init__(self, client):
+        self.client = client
+
+    def build(self, service, path, tag=None, quiet=False, fileobj=None,
+              nocache=False, rm=False, timeout=None,
+              custom_context=False, encoding=None, pull=False,
+              forcerm=False, dockerfile=None, container_limits=None,
+              decode=False, buildargs=None, gzip=False, shmsize=None,
+              labels=None, cache_from=None, target=None, network_mode=None,
+              squash=None, extra_hosts=None, platform=None, isolation=None,
+              use_config_proxy=True, output_stream=sys.stdout):
+        build_output = self.client.build(
+            path=path,
+            tag=tag,
+            nocache=nocache,
+            rm=rm,
+            pull=pull,
+            forcerm=forcerm,
+            dockerfile=dockerfile,
+            labels=labels,
+            cache_from=cache_from,
+            buildargs=buildargs,
+            network_mode=network_mode,
+            target=target,
+            shmsize=shmsize,
+            extra_hosts=extra_hosts,
+            container_limits=container_limits,
+            gzip=gzip,
+            isolation=isolation,
+            platform=platform)
+
+        try:
+            all_events = list(stream_output(build_output, output_stream))
+        except StreamOutputError as e:
+            raise BuildError(service, str(e))
+
+        # Ensure the HTTP connection is not reused for another
+        # streaming command, as the Docker daemon can sometimes
+        # complain about it
+        self.client.close()
+
+        image_id = None
+
+        for event in all_events:
+            if 'stream' in event:
+                match = re.search(r'Successfully built ([0-9a-f]+)', event.get('stream', ''))
+                if match:
+                    image_id = match.group(1)
+
+        if image_id is None:
+            raise BuildError(service, event if all_events else 'Unknown')
+
+        return image_id
+
+
 class _CLIBuilder:
     def __init__(self, progress):
         self._progress = progress
 
-    def build(self, path, tag=None, quiet=False, fileobj=None,
+    def build(self, service, path, tag=None, quiet=False, fileobj=None,
               nocache=False, rm=False, timeout=None,
               custom_context=False, encoding=None, pull=False,
               forcerm=False, dockerfile=None, container_limits=None,
               decode=False, buildargs=None, gzip=False, shmsize=None,
               labels=None, cache_from=None, target=None, network_mode=None,
               squash=None, extra_hosts=None, platform=None, isolation=None,
-              use_config_proxy=True):
+              use_config_proxy=True, output_stream=sys.stdout):
         """
         Args:
+            service (str): Service to be built
             path (str): Path to the directory containing the Dockerfile
             buildargs (dict): A dictionary of build arguments
             cache_from (:py:class:`list`): A list of images used for build
@@ -1889,6 +1923,7 @@ class _CLIBuilder:
                 configuration file (``~/.docker/config.json`` by default)
                 contains a proxy configuration, the corresponding environment
                 variables will be set in the container being built.
+            output_stream (writer): stream to use for build logs
         Returns:
             A generator for the build output.
         """
@@ -1921,33 +1956,18 @@ class _CLIBuilder:
 
         args = command_builder.build([path])
 
-        magic_word = "Successfully built "
-        appear = False
-        with subprocess.Popen(args, stdout=subprocess.PIPE,
+        with subprocess.Popen(args, stdout=output_stream, stderr=sys.stderr,
                               universal_newlines=True) as p:
-            while True:
-                line = p.stdout.readline()
-                if not line:
-                    break
-                if line.startswith(magic_word):
-                    appear = True
-                yield json.dumps({"stream": line})
-
             p.communicate()
             if p.returncode != 0:
-                raise StreamOutputError()
+                raise BuildError(service, "Build failed")
 
         with open(iidfile) as f:
             line = f.readline()
             image_id = line.split(":")[1].strip()
         os.remove(iidfile)
 
-        # In case of `DOCKER_BUILDKIT=1`
-        # there is no success message already present in the output.
-        # Since that's the way `Service::build` gets the `image_id`
-        # it has to be added `manually`
-        if not appear:
-            yield json.dumps({"stream": "{}{}\n".format(magic_word, image_id)})
+        return image_id
 
 
 class _CommandBuilder: