From cf35f51a386f91bd40946339961b1133369c0dd7 Mon Sep 17 00:00:00 2001 From: Simon Li Date: Tue, 11 Feb 2020 18:05:02 +0000 Subject: [PATCH 01/21] Add abstract interface for container engines --- repo2docker/engine.py | 245 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 245 insertions(+) create mode 100644 repo2docker/engine.py diff --git a/repo2docker/engine.py b/repo2docker/engine.py new file mode 100644 index 000000000..151960695 --- /dev/null +++ b/repo2docker/engine.py @@ -0,0 +1,245 @@ +""" +Interface for a repo2docker container engine +""" + +from abc import ABC, abstractmethod + + +# Based on https://docker-py.readthedocs.io/en/4.2.0/containers.html + + +class Container(ABC): + """ + Abstract container returned by repo2docker engines + """ + + @abstractmethod + def reload(self): + """ + Refresh container attributes + """ + + @abstractmethod + def logs(self, stream=False): + """ + Get the container logs. + + Parameters + ---------- + stream : bool + If `True` return an iterator over the log lines, otherwise return all + logs + + Returns + ------- + str or iterator + """ + + @abstractmethod + def kill(self, *, signal="KILL"): + """ + Send a signal to the container + + Parameters + ---------- + signal : str + The signal, default `KILL` + """ + + @abstractmethod + def remove(self): + """ + Remove the container + """ + + @abstractmethod + def stop(self, *, timeout=10): + """ + Stop the container + + Parameters + ---------- + timeout : If the container doesn't gracefully stop after this timeout kill it + """ + + @property + @abstractmethod + def status(self): + """ + The status of the container + + Returns + ------- + str : The status of the container. + Values include `created` `running` `exited`. + + TODO: Does Docker have a fixed list of these? + """ + + +class ContainerEngine(ABC): + """ + Abstract container engine + """ + + # containers = Container + + # Based on https://docker-py.readthedocs.io/en/4.2.0/api.html#module-docker.api.build + + @abstractmethod + def build( + self, + *, + buildargs={}, + cache_from=[], + container_limits={}, + forcerm=False, + rm=False, + tag="", + custom_context=False, + decode=False, + dockerfile="", + fileobj=None, + path="", + ): + """ + Build a container + + Parameters + ---------- + buildargs : dict + Dictionary of build arguments + cache_from : list[str] + List of images to chech for caching + container_limits : dict + Dictionary of resources limits. These keys are supported: + - `cpusetcpus` + - `cpushares` + - `memory` + - `memswap` + forcerm : bool + Always remove containers including unsuccessful builds + rm : bool + Remove intermediate containers + tag : str + Tag to add to the image + + custom_context : bool + If `True` fileobj is a Tar file object containing the build context + TODO: Specific to Docker + decode : bool + If `True` decode responses into dicts + TODO: repo2docker sets this to True but it's not clear what other clients should return + dockerfile : str + Path to Dockerfile within the build context + fileobj : tarfile + A tar file-like object containing the build context + TODO: Specific to Docker, other clients can untar this to a tempdir + path : str + path to the Dockerfile + """ + + @abstractmethod + def images(self): + """ + List images + + Returns + ------- + list[str] : List of images + """ + + @abstractmethod + def inspect_image(self, image): + """ + Get information about an image + + TODO: This is specific to the engine, can we convert it to a standard format? + + Parameters + ---------- + image : str + The image + + Returns + ------- + dict + """ + + @abstractmethod + def push(self, image_spec, *, stream=True): + """ + Push image to a registry + + Parameters + ---------- + image_spec : str + The repository spec to push to + stream : bool + If `True` return output logs as a generator + """ + + # Note this is different from the Docker client which has Client.containers.run + def run( + image_spec, + *, + command=[], + environment=[], + detach=False, + ports={}, + publish_all_ports=False, + remove=False, + volumes={}, + ): + """ + Run a container + + Parameters + ---------- + image_spec : str + The image to run + command : list[str] + The command to run + environment : list[str] + List of environment variables in the form `ENVVAR=value` + detach : bool + If `True` run container in background + ports : dict + Container port bindings in the format expected by the engine + TODO: Should we use a fixed format and convert to whatever's required by the engine? + publish_all_ports : bool + If `True` publish all ports to host + remove : bool + If `True` delete container when it completes + volumes : dict + Volume bindings in the format expected by the engine + TODO: Should we use a fixed format and convert to whatever's required by the engine? + + Returns + ------- + Container : the running container + + Raises + ------ + NotImplementedError + This engine does not support running containers + """ + raise NotImplementedError("Running containers not supported") + + +class ContainerEngineException(Exception): + """ + Base class for exceptions in the container engine + """ + + +class BuildError(ContainerEngineException): + """ + Container build error + """ + + +class ImageLoadError(ContainerEngineException): + """ + Container load/push error + """ From 0610edfc20977cfa60f400433700ce874f6ce927 Mon Sep 17 00:00:00 2001 From: Simon Li Date: Tue, 11 Feb 2020 18:07:16 +0000 Subject: [PATCH 02/21] Add docker implementation of abstract engine --- repo2docker/docker.py | 107 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 107 insertions(+) create mode 100644 repo2docker/docker.py diff --git a/repo2docker/docker.py b/repo2docker/docker.py new file mode 100644 index 000000000..b6ec041d8 --- /dev/null +++ b/repo2docker/docker.py @@ -0,0 +1,107 @@ +""" +Docker container engine for repo2docker +""" + +import docker +from .engine import Container, ContainerEngine, ContainerEngineException + + +class DockerContainer(Container): + def __init__(self, container): + self._c = container + + def reload(self): + return self._c.reload() + + def logs(self, stream=False): + return self._c.logs(stream=stream) + + def kill(self, *, signal="KILL"): + return self._c.kill(signal=signal) + + def remove(self): + return self._c.remove() + + def stop(self, *, timeout=10): + return self._c.stop(timeout=timeout) + + @property + def status(self): + return self._c.status + + +class DockerEngine(ContainerEngine): + """ + https://docker-py.readthedocs.io/en/4.2.0/api.html#module-docker.api.build + """ + + def __init__(self): + try: + self._apiclient = docker.APIClient( + version="auto", **docker.utils.kwargs_from_env() + ) + self._client = docker.from_env(version="auto") + except docker.errors.DockerException as e: + raise ContainerEngineException(e) + + def build( + self, + *, + buildargs=None, + cache_from=None, + container_limits=None, + forcerm=False, + rm=False, + tag="", + custom_context=False, + decode=False, + dockerfile="", + fileobj=None, + path="", + ): + return self._apiclient.build( + buildargs=buildargs, + cache_from=cache_from, + container_limits=container_limits, + forcerm=forcerm, + rm=rm, + tag=tag, + custom_context=custom_context, + decode=decode, + dockerfile=dockerfile, + fileobj=fileobj, + path=path, + ) + + def images(self): + return self._apiclient.images() + + def inspect_image(self, image): + return self._apiclient.inspect_image(image) + + def push(self, image_spec, *, stream=True): + return self._apiclient.push(image_spec, stream=stream) + + def run( + self, + image_spec, + *, + command=None, + environment=None, + detach=False, + ports=None, + publish_all_ports=False, + remove=False, + volumes=None, + ): + container = self._client.containers.run( + image_spec, + command=command, + environment=(environment or []), + detach=detach, + ports=(ports or {}), + publish_all_ports=publish_all_ports, + remove=remove, + volumes=(volumes or {}), + ) + return DockerContainer(container) From 2864b692ef260996e3c42e5dda29bcf9bc22bf57 Mon Sep 17 00:00:00 2001 From: Simon Li Date: Tue, 11 Feb 2020 18:30:18 +0000 Subject: [PATCH 03/21] app: use DockerEngine --- repo2docker/__main__.py | 6 +++--- repo2docker/app.py | 31 ++++++++++++------------------- 2 files changed, 15 insertions(+), 22 deletions(-) diff --git a/repo2docker/__main__.py b/repo2docker/__main__.py index a52a734a9..28883c7a7 100644 --- a/repo2docker/__main__.py +++ b/repo2docker/__main__.py @@ -2,8 +2,8 @@ import sys import os import logging -import docker from .app import Repo2Docker +from .engine import BuildError, ImageLoadError from . import __version__ from .utils import validate_and_generate_port_mapping, is_valid_docker_image_name @@ -371,12 +371,12 @@ def main(): r2d.initialize() try: r2d.start() - except docker.errors.BuildError as e: + except BuildError as e: # This is only raised by us if r2d.log_level == logging.DEBUG: r2d.log.exception(e) sys.exit(1) - except docker.errors.ImageLoadError as e: + except ImageLoadError as e: # This is only raised by us if r2d.log_level == logging.DEBUG: r2d.log.exception(e) diff --git a/repo2docker/app.py b/repo2docker/app.py index 8a804905c..296a25194 100755 --- a/repo2docker/app.py +++ b/repo2docker/app.py @@ -16,10 +16,9 @@ import tempfile import time -import docker +from .engine import BuildError, ContainerEngineException, ImageLoadError +from .docker import DockerEngine from urllib.parse import urlparse -from docker.utils import kwargs_from_env -from docker.errors import DockerException import escapism from pythonjsonlogger import jsonlogger @@ -474,7 +473,7 @@ def initialize(self): def push_image(self): """Push docker image to registry""" - client = docker.APIClient(version="auto", **kwargs_from_env()) + client = DockerEngine() # Build a progress setup for each layer, and only emit per-layer # info every 1.5s progress_layers = {} @@ -492,7 +491,7 @@ def push_image(self): continue if "error" in progress: self.log.error(progress["error"], extra=dict(phase="failed")) - raise docker.errors.ImageLoadError(progress["error"]) + raise ImageLoadError(progress["error"]) if "id" not in progress: continue # deprecated truncated-progress data @@ -528,7 +527,7 @@ def start_container(self): Returns running container """ - client = docker.from_env(version="auto") + client = DockerEngine() docker_host = os.environ.get("DOCKER_HOST") if docker_host: @@ -565,10 +564,7 @@ def start_container(self): container_volumes = {} if self.volumes: - api_client = docker.APIClient( - version="auto", **docker.utils.kwargs_from_env() - ) - image = api_client.inspect_image(self.output_image_spec) + image = client.inspect_image(self.output_image_spec) image_workdir = image["ContainerConfig"]["WorkingDir"] for k, v in self.volumes.items(): @@ -588,7 +584,7 @@ def start_container(self): run_kwargs.update(self.extra_run_kwargs) - container = client.containers.run(self.output_image_spec, **run_kwargs) + container = client.run(self.output_image_spec, **run_kwargs) while container.status == "created": time.sleep(0.5) @@ -645,7 +641,7 @@ def find_image(self): if self.dry_run: return False # check if we already have an image for this content - client = docker.APIClient(version="auto", **kwargs_from_env()) + client = DockerEngine() for image in client.images(): if image["RepoTags"] is not None: for tag in image["RepoTags"]: @@ -660,12 +656,9 @@ def build(self): # Check if r2d can connect to docker daemon if not self.dry_run: try: - docker_client = docker.APIClient(version="auto", **kwargs_from_env()) - except DockerException as e: - self.log.error( - "\nDocker client initialization error: %s.\nCheck if docker is running on the host.\n", - e, - ) + docker_client = DockerEngine() + except ContainerEngineException as e: + self.log.error("\nContainer engine initialization error: %s\n", e) self.exit(1) # If the source to be executed is a directory, continue using the @@ -755,7 +748,7 @@ def build(self): self.log.info(l["stream"], extra=dict(phase="building")) elif "error" in l: self.log.info(l["error"], extra=dict(phase="failure")) - raise docker.errors.BuildError(l["error"], build_log="") + raise BuildError(l["error"], build_log="") elif "status" in l: self.log.info( "Fetching base image...\r", extra=dict(phase="building") From 9ac8a8314cf4e23c5f48bfe220abd4361bc22e7b Mon Sep 17 00:00:00 2001 From: Simon Li Date: Tue, 11 Feb 2020 19:01:18 +0000 Subject: [PATCH 04/21] Handle attrs["State"]["ExitCode"] --- repo2docker/app.py | 2 +- repo2docker/docker.py | 4 ++++ repo2docker/engine.py | 7 +++++++ tests/unit/test_ports.py | 2 +- 4 files changed, 13 insertions(+), 2 deletions(-) diff --git a/repo2docker/app.py b/repo2docker/app.py index 296a25194..b938b8173 100755 --- a/repo2docker/app.py +++ b/repo2docker/app.py @@ -607,7 +607,7 @@ def wait_for_container(self, container): if container.status == "running": self.log.info("Stopping container...\n", extra=dict(phase="running")) container.kill() - exit_code = container.attrs["State"]["ExitCode"] + exit_code = container.exitcode container.wait() diff --git a/repo2docker/docker.py b/repo2docker/docker.py index b6ec041d8..04edb6f42 100644 --- a/repo2docker/docker.py +++ b/repo2docker/docker.py @@ -25,6 +25,10 @@ def remove(self): def stop(self, *, timeout=10): return self._c.stop(timeout=timeout) + @property + def exitcode(self): + return self._c.attrs["State"]["ExitCode"] + @property def status(self): return self._c.status diff --git a/repo2docker/engine.py b/repo2docker/engine.py index 151960695..0d667af0f 100644 --- a/repo2docker/engine.py +++ b/repo2docker/engine.py @@ -62,6 +62,13 @@ def stop(self, *, timeout=10): timeout : If the container doesn't gracefully stop after this timeout kill it """ + @property + @abstractmethod + def exitcode(self): + """ + The container exit code if exited + """ + @property @abstractmethod def status(self): diff --git a/tests/unit/test_ports.py b/tests/unit/test_ports.py index 407a3e02c..edb71424a 100644 --- a/tests/unit/test_ports.py +++ b/tests/unit/test_ports.py @@ -77,7 +77,7 @@ def _cleanup(): container.reload() assert container.status == "running" - port_mapping = container.attrs["NetworkSettings"]["Ports"] + port_mapping = container._c.attrs["NetworkSettings"]["Ports"] if all_ports: port = port_mapping["8888/tcp"][0]["HostPort"] From c521c3c95022e06d9ad52c194edc52260c302e34 Mon Sep 17 00:00:00 2001 From: Simon Li Date: Tue, 11 Feb 2020 21:05:10 +0000 Subject: [PATCH 05/21] Remove constant docker args --- repo2docker/app.py | 1 - repo2docker/buildpacks/base.py | 3 --- repo2docker/buildpacks/docker.py | 3 --- repo2docker/docker.py | 16 ++++++++-------- repo2docker/engine.py | 30 +++++++++++++++++------------- 5 files changed, 25 insertions(+), 28 deletions(-) diff --git a/repo2docker/app.py b/repo2docker/app.py index b938b8173..ddad7c8ea 100755 --- a/repo2docker/app.py +++ b/repo2docker/app.py @@ -576,7 +576,6 @@ def start_container(self): run_kwargs = dict( publish_all_ports=self.all_ports, ports=ports, - detach=True, command=run_cmd, volumes=container_volumes, environment=self.environment, diff --git a/repo2docker/buildpacks/base.py b/repo2docker/buildpacks/base.py index 145d29648..46a407ad3 100644 --- a/repo2docker/buildpacks/base.py +++ b/repo2docker/buildpacks/base.py @@ -607,9 +607,6 @@ def _filter_tar(tar): tag=image_spec, custom_context=True, buildargs=build_args, - decode=True, - forcerm=True, - rm=True, container_limits=limits, cache_from=cache_from, ) diff --git a/repo2docker/buildpacks/docker.py b/repo2docker/buildpacks/docker.py index 5a0981655..8326aecfe 100644 --- a/repo2docker/buildpacks/docker.py +++ b/repo2docker/buildpacks/docker.py @@ -50,9 +50,6 @@ def build( dockerfile=self.binder_path(self.dockerfile), tag=image_spec, buildargs=build_args, - decode=True, - forcerm=True, - rm=True, container_limits=limits, cache_from=cache_from, ) diff --git a/repo2docker/docker.py b/repo2docker/docker.py index 04edb6f42..8e3fbbc86 100644 --- a/repo2docker/docker.py +++ b/repo2docker/docker.py @@ -54,11 +54,11 @@ def build( buildargs=None, cache_from=None, container_limits=None, - forcerm=False, - rm=False, + # forcerm=False, + # rm=False, tag="", custom_context=False, - decode=False, + # decode=False, dockerfile="", fileobj=None, path="", @@ -67,11 +67,11 @@ def build( buildargs=buildargs, cache_from=cache_from, container_limits=container_limits, - forcerm=forcerm, - rm=rm, + forcerm=True, + rm=True, tag=tag, custom_context=custom_context, - decode=decode, + decode=True, dockerfile=dockerfile, fileobj=fileobj, path=path, @@ -92,7 +92,7 @@ def run( *, command=None, environment=None, - detach=False, + # detach=False, ports=None, publish_all_ports=False, remove=False, @@ -102,7 +102,7 @@ def run( image_spec, command=command, environment=(environment or []), - detach=detach, + detach=True, ports=(ports or {}), publish_all_ports=publish_all_ports, remove=remove, diff --git a/repo2docker/engine.py b/repo2docker/engine.py index 0d667af0f..0d818c7ff 100644 --- a/repo2docker/engine.py +++ b/repo2docker/engine.py @@ -100,11 +100,11 @@ def build( buildargs={}, cache_from=[], container_limits={}, - forcerm=False, - rm=False, + # forcerm=False, + # rm=False, tag="", custom_context=False, - decode=False, + # decode=False, dockerfile="", fileobj=None, path="", @@ -124,19 +124,22 @@ def build( - `cpushares` - `memory` - `memswap` - forcerm : bool - Always remove containers including unsuccessful builds - rm : bool - Remove intermediate containers + # forcerm : bool + # Always remove containers including unsuccessful builds + # always True + # rm : bool + # Remove intermediate containers + # always True tag : str Tag to add to the image custom_context : bool If `True` fileobj is a Tar file object containing the build context TODO: Specific to Docker - decode : bool - If `True` decode responses into dicts - TODO: repo2docker sets this to True but it's not clear what other clients should return + # decode : bool + # If `True` decode responses into dicts + # TODO: repo2docker sets this to True but it's not clear what other clients should return + # always True dockerfile : str Path to Dockerfile within the build context fileobj : tarfile @@ -192,7 +195,7 @@ def run( *, command=[], environment=[], - detach=False, + # detach=False, ports={}, publish_all_ports=False, remove=False, @@ -209,8 +212,9 @@ def run( The command to run environment : list[str] List of environment variables in the form `ENVVAR=value` - detach : bool - If `True` run container in background + # detach : bool + # If `True` run container in background + # always True ports : dict Container port bindings in the format expected by the engine TODO: Should we use a fixed format and convert to whatever's required by the engine? From b6bead74cdfbbcfaf9605851b2ed7a802d2632b9 Mon Sep 17 00:00:00 2001 From: Simon Li Date: Tue, 11 Feb 2020 21:25:05 +0000 Subject: [PATCH 06/21] Python 3.5: can't have trailing , in function arg list --- repo2docker/docker.py | 4 ++-- repo2docker/engine.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/repo2docker/docker.py b/repo2docker/docker.py index 8e3fbbc86..8f6dd3b94 100644 --- a/repo2docker/docker.py +++ b/repo2docker/docker.py @@ -61,7 +61,7 @@ def build( # decode=False, dockerfile="", fileobj=None, - path="", + path="" ): return self._apiclient.build( buildargs=buildargs, @@ -96,7 +96,7 @@ def run( ports=None, publish_all_ports=False, remove=False, - volumes=None, + volumes=None ): container = self._client.containers.run( image_spec, diff --git a/repo2docker/engine.py b/repo2docker/engine.py index 0d818c7ff..3d8366876 100644 --- a/repo2docker/engine.py +++ b/repo2docker/engine.py @@ -107,7 +107,7 @@ def build( # decode=False, dockerfile="", fileobj=None, - path="", + path="" ): """ Build a container @@ -199,7 +199,7 @@ def run( ports={}, publish_all_ports=False, remove=False, - volumes={}, + volumes={} ): """ Run a container From 787d1d3aebf9d788703c941516030182a4f55e5b Mon Sep 17 00:00:00 2001 From: Simon Li Date: Thu, 13 Feb 2020 11:46:29 +0000 Subject: [PATCH 07/21] Container engine is an entrypoint: repo2docker.engines --- repo2docker/__main__.py | 5 +++++ repo2docker/app.py | 36 ++++++++++++++++++++++++++++++++---- setup.py | 4 +++- 3 files changed, 40 insertions(+), 5 deletions(-) diff --git a/repo2docker/__main__.py b/repo2docker/__main__.py index 28883c7a7..1dd49ebe9 100644 --- a/repo2docker/__main__.py +++ b/repo2docker/__main__.py @@ -217,6 +217,8 @@ def get_argparser(): "--cache-from", action="append", default=[], help=Repo2Docker.cache_from.help ) + argparser.add_argument("--engine", help="Name of the container engine") + return argparser @@ -351,6 +353,9 @@ def make_r2d(argv=None): if args.cache_from: r2d.cache_from = args.cache_from + if args.engine: + r2d.engine = args.engine + r2d.environment = args.environment # if the source exists locally we don't want to delete it at the end diff --git a/repo2docker/app.py b/repo2docker/app.py index ddad7c8ea..f77a1d368 100755 --- a/repo2docker/app.py +++ b/repo2docker/app.py @@ -11,6 +11,7 @@ import sys import logging import os +import entrypoints import getpass import shutil import tempfile @@ -381,6 +382,33 @@ def _user_name_default(self): config=True, ) + engine = Unicode( + "docker", + config=True, + help=""" + Name of the container engine. + + Defaults to 'docker'. + """, + ) + + def get_engine(self): + """Return an instance of the container engine. + + Currently no arguments are passed to the engine constructor. + """ + engines = entrypoints.get_group_named("repo2docker.engines") + try: + entry = engines[self.engine] + except KeyError: + raise ContainerEngineException( + "Container engine '{}' not found. Available engines: {}".format( + self.engine, ",".join(engines.keys()) + ) + ) + engine_class = entry.load() + return engine_class() + def fetch(self, url, ref, checkout_path): """Fetch the contents of `url` and place it in `checkout_path`. @@ -473,7 +501,7 @@ def initialize(self): def push_image(self): """Push docker image to registry""" - client = DockerEngine() + client = self.get_engine() # Build a progress setup for each layer, and only emit per-layer # info every 1.5s progress_layers = {} @@ -527,7 +555,7 @@ def start_container(self): Returns running container """ - client = DockerEngine() + client = self.get_engine() docker_host = os.environ.get("DOCKER_HOST") if docker_host: @@ -640,7 +668,7 @@ def find_image(self): if self.dry_run: return False # check if we already have an image for this content - client = DockerEngine() + client = self.get_engine() for image in client.images(): if image["RepoTags"] is not None: for tag in image["RepoTags"]: @@ -655,7 +683,7 @@ def build(self): # Check if r2d can connect to docker daemon if not self.dry_run: try: - docker_client = DockerEngine() + docker_client = self.get_engine() except ContainerEngineException as e: self.log.error("\nContainer engine initialization error: %s\n", e) self.exit(1) diff --git a/setup.py b/setup.py index af0611497..27c19c93c 100644 --- a/setup.py +++ b/setup.py @@ -48,6 +48,7 @@ def get_identifier(json): version=versioneer.get_version(), install_requires=[ "docker", + "entrypoints", "escapism", "jinja2", "python-json-logger", @@ -89,6 +90,7 @@ def get_identifier(json): "console_scripts": [ "jupyter-repo2docker = repo2docker.__main__:main", "repo2docker = repo2docker.__main__:main", - ] + ], + "repo2docker.engines": ["docker = repo2docker.docker:DockerEngine"], }, ) From a73595ce4c70940701ee085cd251f34ed3883451 Mon Sep 17 00:00:00 2001 From: Simon Li Date: Fri, 14 Feb 2020 13:15:04 +0000 Subject: [PATCH 08/21] Require image info in a fixed format --- repo2docker/app.py | 7 +++---- repo2docker/docker.py | 5 +++-- repo2docker/engine.py | 26 ++++++++++++++++++++++++-- 3 files changed, 30 insertions(+), 8 deletions(-) diff --git a/repo2docker/app.py b/repo2docker/app.py index f77a1d368..0e83c50c5 100755 --- a/repo2docker/app.py +++ b/repo2docker/app.py @@ -670,10 +670,9 @@ def find_image(self): # check if we already have an image for this content client = self.get_engine() for image in client.images(): - if image["RepoTags"] is not None: - for tag in image["RepoTags"]: - if tag == self.output_image_spec + ":latest": - return True + for tag in image.tags: + if tag == self.output_image_spec + ":latest": + return True return False def build(self): diff --git a/repo2docker/docker.py b/repo2docker/docker.py index 8f6dd3b94..d158037b0 100644 --- a/repo2docker/docker.py +++ b/repo2docker/docker.py @@ -3,7 +3,7 @@ """ import docker -from .engine import Container, ContainerEngine, ContainerEngineException +from .engine import Container, ContainerEngine, ContainerEngineException, Image class DockerContainer(Container): @@ -78,7 +78,8 @@ def build( ) def images(self): - return self._apiclient.images() + images = self._apiclient.images() + return [Image(tags=image["RepoTags"]) for image in images] def inspect_image(self, image): return self._apiclient.inspect_image(image) diff --git a/repo2docker/engine.py b/repo2docker/engine.py index 3d8366876..118c8c8e3 100644 --- a/repo2docker/engine.py +++ b/repo2docker/engine.py @@ -84,9 +84,31 @@ def status(self): """ +class Image: + """ + Information about a container image + """ + + def __init__(self, *, tags): + self._tags = tags or [] + + @property + def tags(self): + """ + A list of tags associated with an image. + + If locally built images have a localhost prefix this prefix should be removed or the image may not be recognised. + If there are no tags [] will be returned. + """ + return self._tags + + def __repr__(self): + return "Image(tags={})".format(self.tags) + + class ContainerEngine(ABC): """ - Abstract container engine + Abstract container engine. """ # containers = Container @@ -156,7 +178,7 @@ def images(self): Returns ------- - list[str] : List of images + list[Image] : List of Image objects. """ @abstractmethod From 4a06ef19d95952d59be9dae34612826df84c7138 Mon Sep 17 00:00:00 2001 From: Simon Li Date: Fri, 14 Feb 2020 13:37:31 +0000 Subject: [PATCH 09/21] ContainerEngine is now LoggingConfigurable --- repo2docker/app.py | 2 +- repo2docker/docker.py | 3 ++- repo2docker/engine.py | 25 +++++++++++++++++++------ 3 files changed, 22 insertions(+), 8 deletions(-) diff --git a/repo2docker/app.py b/repo2docker/app.py index 0e83c50c5..4c00e190f 100755 --- a/repo2docker/app.py +++ b/repo2docker/app.py @@ -407,7 +407,7 @@ def get_engine(self): ) ) engine_class = entry.load() - return engine_class() + return engine_class(parent=self) def fetch(self, url, ref, checkout_path): """Fetch the contents of `url` and place it in `checkout_path`. diff --git a/repo2docker/docker.py b/repo2docker/docker.py index d158037b0..53fc33b54 100644 --- a/repo2docker/docker.py +++ b/repo2docker/docker.py @@ -39,7 +39,8 @@ class DockerEngine(ContainerEngine): https://docker-py.readthedocs.io/en/4.2.0/api.html#module-docker.api.build """ - def __init__(self): + def __init__(self, *, parent): + super().__init__(parent=parent) try: self._apiclient = docker.APIClient( version="auto", **docker.utils.kwargs_from_env() diff --git a/repo2docker/engine.py b/repo2docker/engine.py index 118c8c8e3..cb6a58d42 100644 --- a/repo2docker/engine.py +++ b/repo2docker/engine.py @@ -3,6 +3,7 @@ """ from abc import ABC, abstractmethod +from traitlets.config import LoggingConfigurable # Based on https://docker-py.readthedocs.io/en/4.2.0/containers.html @@ -106,16 +107,27 @@ def __repr__(self): return "Image(tags={})".format(self.tags) -class ContainerEngine(ABC): +class ContainerEngine(LoggingConfigurable): """ Abstract container engine. + + Inherits from LoggingConfigurable, which means it has a log property. + Initialised with a reference to the parent so can also be configured using traitlets. """ - # containers = Container + def __init__(self, *, parent): + """ + Initialise the container engine + + Parameters + ---------- + parent: Application + Reference to the parent application so that its configuration file can be used in this plugin. + """ + super().__init__(parent=parent) # Based on https://docker-py.readthedocs.io/en/4.2.0/api.html#module-docker.api.build - @abstractmethod def build( self, *, @@ -170,8 +182,8 @@ def build( path : str path to the Dockerfile """ + raise NotImplementedError("build not implemented") - @abstractmethod def images(self): """ List images @@ -180,8 +192,8 @@ def images(self): ------- list[Image] : List of Image objects. """ + raise NotImplementedError("images not implemented") - @abstractmethod def inspect_image(self, image): """ Get information about an image @@ -197,8 +209,8 @@ def inspect_image(self, image): ------- dict """ + raise NotImplementedError("inspect_image not implemented") - @abstractmethod def push(self, image_spec, *, stream=True): """ Push image to a registry @@ -210,6 +222,7 @@ def push(self, image_spec, *, stream=True): stream : bool If `True` return output logs as a generator """ + raise NotImplementedError("push not implemented") # Note this is different from the Docker client which has Client.containers.run def run( From 17af0c7293ebc2ea9031d55926488d144d34ea86 Mon Sep 17 00:00:00 2001 From: Simon Li Date: Fri, 14 Feb 2020 13:37:59 +0000 Subject: [PATCH 10/21] BuildError only takes positional params --- repo2docker/app.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/repo2docker/app.py b/repo2docker/app.py index 4c00e190f..0bab5d97a 100755 --- a/repo2docker/app.py +++ b/repo2docker/app.py @@ -774,7 +774,7 @@ def build(self): self.log.info(l["stream"], extra=dict(phase="building")) elif "error" in l: self.log.info(l["error"], extra=dict(phase="failure")) - raise BuildError(l["error"], build_log="") + raise BuildError(l["error"]) elif "status" in l: self.log.info( "Fetching base image...\r", extra=dict(phase="building") From 8215f0b065df1dade9ed8d33f970812004aec127 Mon Sep 17 00:00:00 2001 From: Simon Li Date: Fri, 14 Feb 2020 13:40:02 +0000 Subject: [PATCH 11/21] Remove commented engine.py code --- repo2docker/docker.py | 4 ---- repo2docker/engine.py | 18 +----------------- 2 files changed, 1 insertion(+), 21 deletions(-) diff --git a/repo2docker/docker.py b/repo2docker/docker.py index 53fc33b54..d08224fde 100644 --- a/repo2docker/docker.py +++ b/repo2docker/docker.py @@ -55,11 +55,8 @@ def build( buildargs=None, cache_from=None, container_limits=None, - # forcerm=False, - # rm=False, tag="", custom_context=False, - # decode=False, dockerfile="", fileobj=None, path="" @@ -94,7 +91,6 @@ def run( *, command=None, environment=None, - # detach=False, ports=None, publish_all_ports=False, remove=False, diff --git a/repo2docker/engine.py b/repo2docker/engine.py index cb6a58d42..5e6b72398 100644 --- a/repo2docker/engine.py +++ b/repo2docker/engine.py @@ -134,11 +134,8 @@ def build( buildargs={}, cache_from=[], container_limits={}, - # forcerm=False, - # rm=False, tag="", custom_context=False, - # decode=False, dockerfile="", fileobj=None, path="" @@ -158,22 +155,12 @@ def build( - `cpushares` - `memory` - `memswap` - # forcerm : bool - # Always remove containers including unsuccessful builds - # always True - # rm : bool - # Remove intermediate containers - # always True tag : str Tag to add to the image custom_context : bool If `True` fileobj is a Tar file object containing the build context TODO: Specific to Docker - # decode : bool - # If `True` decode responses into dicts - # TODO: repo2docker sets this to True but it's not clear what other clients should return - # always True dockerfile : str Path to Dockerfile within the build context fileobj : tarfile @@ -226,11 +213,11 @@ def push(self, image_spec, *, stream=True): # Note this is different from the Docker client which has Client.containers.run def run( + self, image_spec, *, command=[], environment=[], - # detach=False, ports={}, publish_all_ports=False, remove=False, @@ -247,9 +234,6 @@ def run( The command to run environment : list[str] List of environment variables in the form `ENVVAR=value` - # detach : bool - # If `True` run container in background - # always True ports : dict Container port bindings in the format expected by the engine TODO: Should we use a fixed format and convert to whatever's required by the engine? From 00df21069ee2556d75677698ed44f67aee9945b7 Mon Sep 17 00:00:00 2001 From: Simon Li Date: Fri, 14 Feb 2020 16:20:55 +0000 Subject: [PATCH 12/21] ContainerEngine .build .push default to returning strings (stream) --- repo2docker/app.py | 12 ++++++++++-- repo2docker/docker.py | 8 +++++--- repo2docker/engine.py | 35 ++++++++++++++++++++++++++++------- 3 files changed, 43 insertions(+), 12 deletions(-) diff --git a/repo2docker/app.py b/repo2docker/app.py index 0bab5d97a..52fd45efe 100755 --- a/repo2docker/app.py +++ b/repo2docker/app.py @@ -507,7 +507,12 @@ def push_image(self): progress_layers = {} layers = {} last_emit_time = time.time() - for chunk in client.push(self.output_image_spec, stream=True): + for chunk in client.push(self.output_image_spec): + if client.string_output: + self.log.info(chunk, extra=dict(phase="pushing")) + continue + # else this is Docker output + # each chunk can be one or more lines of json events # split lines here in case multiple are delivered at once for line in chunk.splitlines(): @@ -770,7 +775,10 @@ def build(self): self.cache_from, self.extra_build_kwargs, ): - if "stream" in l: + if docker_client.string_output: + self.log.info(l, extra=dict(phase="building")) + # else this is Docker output + elif "stream" in l: self.log.info(l["stream"], extra=dict(phase="building")) elif "error" in l: self.log.info(l["error"], extra=dict(phase="failure")) diff --git a/repo2docker/docker.py b/repo2docker/docker.py index d08224fde..6e0e04f4c 100644 --- a/repo2docker/docker.py +++ b/repo2docker/docker.py @@ -13,7 +13,7 @@ def __init__(self, container): def reload(self): return self._c.reload() - def logs(self, stream=False): + def logs(self, *, stream=False): return self._c.logs(stream=stream) def kill(self, *, signal="KILL"): @@ -39,6 +39,8 @@ class DockerEngine(ContainerEngine): https://docker-py.readthedocs.io/en/4.2.0/api.html#module-docker.api.build """ + string_output = False + def __init__(self, *, parent): super().__init__(parent=parent) try: @@ -82,8 +84,8 @@ def images(self): def inspect_image(self, image): return self._apiclient.inspect_image(image) - def push(self, image_spec, *, stream=True): - return self._apiclient.push(image_spec, stream=stream) + def push(self, image_spec): + return self._apiclient.push(image_spec, stream=True) def run( self, diff --git a/repo2docker/engine.py b/repo2docker/engine.py index 5e6b72398..9e0aa6dd1 100644 --- a/repo2docker/engine.py +++ b/repo2docker/engine.py @@ -21,19 +21,18 @@ def reload(self): """ @abstractmethod - def logs(self, stream=False): + def logs(self, *, stream=False): """ Get the container logs. Parameters ---------- stream : bool - If `True` return an iterator over the log lines, otherwise return all - logs + If `True` return an iterator over the log lines, otherwise return all logs Returns ------- - str or iterator + str or generator of log strings """ @abstractmethod @@ -115,6 +114,16 @@ class ContainerEngine(LoggingConfigurable): Initialised with a reference to the parent so can also be configured using traitlets. """ + string_output = True + """ + Whether progress events should be strings or an object. + + Originally Docker was the only container engine supported by repo2docker. + Some operations including build() and push() would return generators of events in a Docker specific format. + This format of events is not easily constructable with other engines so the default is to return strings and raise an exception if an error occurs. + If an engine returns docker style events set this variable to False. + """ + def __init__(self, *, parent): """ Initialise the container engine @@ -168,6 +177,13 @@ def build( TODO: Specific to Docker, other clients can untar this to a tempdir path : str path to the Dockerfile + + Returns + ------- + A generator of strings. If an error occurs an exception must be thrown. + + If `string_output=True` this should instead be whatever Docker returns: + https://github.com/jupyter/repo2docker/blob/0.11.0/repo2docker/app.py#L725-L735 """ raise NotImplementedError("build not implemented") @@ -198,7 +214,7 @@ def inspect_image(self, image): """ raise NotImplementedError("inspect_image not implemented") - def push(self, image_spec, *, stream=True): + def push(self, image_spec): """ Push image to a registry @@ -206,8 +222,13 @@ def push(self, image_spec, *, stream=True): ---------- image_spec : str The repository spec to push to - stream : bool - If `True` return output logs as a generator + + Returns + ------- + A generator of strings. If an error occurs an exception must be thrown. + + If `string_output=True` this should instead be whatever Docker returns: + https://github.com/jupyter/repo2docker/blob/0.11.0/repo2docker/app.py#L469-L495 """ raise NotImplementedError("push not implemented") From ad5701dd3acb5696042cc5e5e1bc51994546aa49 Mon Sep 17 00:00:00 2001 From: Simon Li Date: Fri, 14 Feb 2020 16:27:25 +0000 Subject: [PATCH 13/21] Remove unnecessary import --- repo2docker/app.py | 1 - 1 file changed, 1 deletion(-) diff --git a/repo2docker/app.py b/repo2docker/app.py index 52fd45efe..75f1d54b1 100755 --- a/repo2docker/app.py +++ b/repo2docker/app.py @@ -18,7 +18,6 @@ import time from .engine import BuildError, ContainerEngineException, ImageLoadError -from .docker import DockerEngine from urllib.parse import urlparse import escapism from pythonjsonlogger import jsonlogger From 4647dd07028199f48c000d0344065f7265eaa612 Mon Sep 17 00:00:00 2001 From: Simon Li Date: Fri, 14 Feb 2020 16:38:42 +0000 Subject: [PATCH 14/21] Container .build .run need to support **kwargs --- repo2docker/docker.py | 8 ++++++-- repo2docker/engine.py | 6 ++++-- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/repo2docker/docker.py b/repo2docker/docker.py index 6e0e04f4c..0c501f8c1 100644 --- a/repo2docker/docker.py +++ b/repo2docker/docker.py @@ -61,7 +61,8 @@ def build( custom_context=False, dockerfile="", fileobj=None, - path="" + path="", + **kwargs, ): return self._apiclient.build( buildargs=buildargs, @@ -75,6 +76,7 @@ def build( dockerfile=dockerfile, fileobj=fileobj, path=path, + **kwargs, ) def images(self): @@ -96,7 +98,8 @@ def run( ports=None, publish_all_ports=False, remove=False, - volumes=None + volumes=None, + **kwargs, ): container = self._client.containers.run( image_spec, @@ -107,5 +110,6 @@ def run( publish_all_ports=publish_all_ports, remove=remove, volumes=(volumes or {}), + **kwargs, ) return DockerContainer(container) diff --git a/repo2docker/engine.py b/repo2docker/engine.py index 9e0aa6dd1..7dede1661 100644 --- a/repo2docker/engine.py +++ b/repo2docker/engine.py @@ -147,7 +147,8 @@ def build( custom_context=False, dockerfile="", fileobj=None, - path="" + path="", + **kwargs ): """ Build a container @@ -242,7 +243,8 @@ def run( ports={}, publish_all_ports=False, remove=False, - volumes={} + volumes={}, + **kwargs ): """ Run a container From 52b672a17a891df59a22bb8dca001eb0eddf83d6 Mon Sep 17 00:00:00 2001 From: Simon Li Date: Fri, 14 Feb 2020 16:49:08 +0000 Subject: [PATCH 15/21] Docker: print helpful message if daemon not running --- repo2docker/docker.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/repo2docker/docker.py b/repo2docker/docker.py index 0c501f8c1..ef516b0a7 100644 --- a/repo2docker/docker.py +++ b/repo2docker/docker.py @@ -47,9 +47,8 @@ def __init__(self, *, parent): self._apiclient = docker.APIClient( version="auto", **docker.utils.kwargs_from_env() ) - self._client = docker.from_env(version="auto") except docker.errors.DockerException as e: - raise ContainerEngineException(e) + raise ContainerEngineException("Check if docker is running on the host.", e) def build( self, @@ -101,7 +100,8 @@ def run( volumes=None, **kwargs, ): - container = self._client.containers.run( + client = docker.from_env(version="auto") + container = client.containers.run( image_spec, command=command, environment=(environment or []), From 5c2c8f73858f984e96d518b5b10ef983c509b1da Mon Sep 17 00:00:00 2001 From: Simon Li Date: Fri, 14 Feb 2020 17:27:01 +0000 Subject: [PATCH 16/21] Python 3.5: can't have trailing , in function arg list (again) --- repo2docker/docker.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/repo2docker/docker.py b/repo2docker/docker.py index ef516b0a7..cabe79f84 100644 --- a/repo2docker/docker.py +++ b/repo2docker/docker.py @@ -61,7 +61,10 @@ def build( dockerfile="", fileobj=None, path="", - **kwargs, + # fmt: off + # black adds a trailing , but this is invalid in Python 3.5 + **kwargs + # fmt: on ): return self._apiclient.build( buildargs=buildargs, @@ -98,7 +101,10 @@ def run( publish_all_ports=False, remove=False, volumes=None, - **kwargs, + # fmt: off + # black adds a trailing , but this is invalid in Python 3.5 + **kwargs + # fmt: on ): client = docker.from_env(version="auto") container = client.containers.run( From 4ed6685dc995021d370be8bbb3387c1212e8bfe5 Mon Sep 17 00:00:00 2001 From: Simon Li Date: Fri, 14 Feb 2020 15:13:09 +0000 Subject: [PATCH 17/21] Fix unit tests --- tests/unit/test_app.py | 4 ++-- tests/unit/test_editable.py | 6 ++++-- tests/unit/test_external_scripts.py | 2 +- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/tests/unit/test_app.py b/tests/unit/test_app.py index d45c28a4c..25b39cdb1 100644 --- a/tests/unit/test_app.py +++ b/tests/unit/test_app.py @@ -14,7 +14,7 @@ def test_find_image(): images = [{"RepoTags": ["some-org/some-repo:latest"]}] - with patch("repo2docker.app.docker.APIClient") as FakeDockerClient: + with patch("repo2docker.docker.docker.APIClient") as FakeDockerClient: instance = FakeDockerClient.return_value instance.images.return_value = images @@ -28,7 +28,7 @@ def test_find_image(): def test_dont_find_image(): images = [{"RepoTags": ["some-org/some-image-name:latest"]}] - with patch("repo2docker.app.docker.APIClient") as FakeDockerClient: + with patch("repo2docker.docker.docker.APIClient") as FakeDockerClient: instance = FakeDockerClient.return_value instance.images.return_value = images diff --git a/tests/unit/test_editable.py b/tests/unit/test_editable.py index a84e4ba2e..6d711e25f 100644 --- a/tests/unit/test_editable.py +++ b/tests/unit/test_editable.py @@ -44,12 +44,14 @@ def test_editable_by_host(): try: with tempfile.NamedTemporaryFile(dir=DIR, prefix="testfile", suffix=".txt"): - status, output = container.exec_run(["sh", "-c", "ls testfile????????.txt"]) + status, output = container._c.exec_run( + ["sh", "-c", "ls testfile????????.txt"] + ) assert status == 0 assert re.match(br"^testfile\w{8}\.txt\n$", output) is not None # After exiting the with block the file should stop existing # in the container as well as locally - status, output = container.exec_run(["sh", "-c", "ls testfile????????.txt"]) + status, output = container._c.exec_run(["sh", "-c", "ls testfile????????.txt"]) assert status == 2 assert re.match(br"^testfile\w{8}\.txt\n$", output) is None diff --git a/tests/unit/test_external_scripts.py b/tests/unit/test_external_scripts.py index 1265d3ac5..d03debe55 100644 --- a/tests/unit/test_external_scripts.py +++ b/tests/unit/test_external_scripts.py @@ -32,7 +32,7 @@ def get_build_script_files(self): assert container.status == "running" try: - status, output = container.exec_run(["sh", "-c", "cat /tmp/my_extra_script"]) + status, output = container._c.exec_run(["sh", "-c", "cat /tmp/my_extra_script"]) assert status == 0 assert output.decode("utf-8") == "Hello World of Absolute Paths!" finally: From 0e7e0f003ef13f9b4b681790afcb60a1798ee740 Mon Sep 17 00:00:00 2001 From: Simon Li Date: Fri, 14 Feb 2020 22:00:34 +0000 Subject: [PATCH 18/21] Use standard-ish image_inspect format --- repo2docker/app.py | 2 +- repo2docker/docker.py | 3 ++- repo2docker/engine.py | 18 +++++++++++++++--- 3 files changed, 18 insertions(+), 5 deletions(-) diff --git a/repo2docker/app.py b/repo2docker/app.py index 75f1d54b1..470e9c0a9 100755 --- a/repo2docker/app.py +++ b/repo2docker/app.py @@ -597,7 +597,7 @@ def start_container(self): container_volumes = {} if self.volumes: image = client.inspect_image(self.output_image_spec) - image_workdir = image["ContainerConfig"]["WorkingDir"] + image_workdir = image.config["WorkingDir"] for k, v in self.volumes.items(): container_volumes[os.path.abspath(k)] = { diff --git a/repo2docker/docker.py b/repo2docker/docker.py index cabe79f84..7a3735463 100644 --- a/repo2docker/docker.py +++ b/repo2docker/docker.py @@ -86,7 +86,8 @@ def images(self): return [Image(tags=image["RepoTags"]) for image in images] def inspect_image(self, image): - return self._apiclient.inspect_image(image) + image = self._apiclient.inspect_image(image) + return Image(tags=image["RepoTags"], config=image["ContainerConfig"]) def push(self, image_spec): return self._apiclient.push(image_spec, stream=True) diff --git a/repo2docker/engine.py b/repo2docker/engine.py index 7dede1661..c0d7f6f53 100644 --- a/repo2docker/engine.py +++ b/repo2docker/engine.py @@ -89,8 +89,9 @@ class Image: Information about a container image """ - def __init__(self, *, tags): + def __init__(self, *, tags, config=None): self._tags = tags or [] + self._config = config @property def tags(self): @@ -102,8 +103,19 @@ def tags(self): """ return self._tags + @property + def config(self): + """ + A dictionary of image configuration information + + If this is `None` the information has not been loaded. + If not `None` this must include the following fields: + - WorkingDir: The default working directory + """ + return self._config + def __repr__(self): - return "Image(tags={})".format(self.tags) + return "Image(tags={},config={})".format(self.tags, self.config) class ContainerEngine(LoggingConfigurable): @@ -211,7 +223,7 @@ def inspect_image(self, image): Returns ------- - dict + Image object with .config dict. """ raise NotImplementedError("inspect_image not implemented") From e32c7b02df9f5e3726dec1a83df2db79d37e838e Mon Sep 17 00:00:00 2001 From: Simon Li Date: Fri, 14 Feb 2020 22:01:08 +0000 Subject: [PATCH 19/21] describe ContainerEngine volume and port formats --- repo2docker/engine.py | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/repo2docker/engine.py b/repo2docker/engine.py index c0d7f6f53..6b9fa1170 100644 --- a/repo2docker/engine.py +++ b/repo2docker/engine.py @@ -80,7 +80,8 @@ def status(self): str : The status of the container. Values include `created` `running` `exited`. - TODO: Does Docker have a fixed list of these? + Full list of statuses: + https://github.com/moby/moby/blob/v19.03.5/api/swagger.yaml#L4832 """ @@ -182,12 +183,10 @@ def build( custom_context : bool If `True` fileobj is a Tar file object containing the build context - TODO: Specific to Docker dockerfile : str Path to Dockerfile within the build context fileobj : tarfile A tar file-like object containing the build context - TODO: Specific to Docker, other clients can untar this to a tempdir path : str path to the Dockerfile @@ -270,15 +269,14 @@ def run( environment : list[str] List of environment variables in the form `ENVVAR=value` ports : dict - Container port bindings in the format expected by the engine - TODO: Should we use a fixed format and convert to whatever's required by the engine? + Container port bindings in the form generated by `repo2docker.utils.validate_and_generate_port_mapping` + https://github.com/jupyter/repo2docker/blob/0.11.0/repo2docker/utils.py#L95 publish_all_ports : bool If `True` publish all ports to host remove : bool If `True` delete container when it completes volumes : dict - Volume bindings in the format expected by the engine - TODO: Should we use a fixed format and convert to whatever's required by the engine? + Volume bindings in the form `{src : dest}` Returns ------- From 5778ed0381f909dd714340cfcd8dabad5c10c2b4 Mon Sep 17 00:00:00 2001 From: Simon Li Date: Tue, 31 Mar 2020 16:02:46 +0100 Subject: [PATCH 20/21] docker.py: remove workaround for old black --- repo2docker/docker.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/repo2docker/docker.py b/repo2docker/docker.py index 7a3735463..cb3a5dbaa 100644 --- a/repo2docker/docker.py +++ b/repo2docker/docker.py @@ -61,10 +61,7 @@ def build( dockerfile="", fileobj=None, path="", - # fmt: off - # black adds a trailing , but this is invalid in Python 3.5 **kwargs - # fmt: on ): return self._apiclient.build( buildargs=buildargs, @@ -102,10 +99,7 @@ def run( publish_all_ports=False, remove=False, volumes=None, - # fmt: off - # black adds a trailing , but this is invalid in Python 3.5 **kwargs - # fmt: on ): client = docker.from_env(version="auto") container = client.containers.run( From d47b74492f966241e217735aad508e6f3d005b29 Mon Sep 17 00:00:00 2001 From: Simon Li Date: Sun, 21 Feb 2021 00:19:54 +0000 Subject: [PATCH 21/21] Add Container.wait() --- repo2docker/docker.py | 7 +++++-- repo2docker/engine.py | 10 ++++++++-- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/repo2docker/docker.py b/repo2docker/docker.py index cb3a5dbaa..d637674ad 100644 --- a/repo2docker/docker.py +++ b/repo2docker/docker.py @@ -25,6 +25,9 @@ def remove(self): def stop(self, *, timeout=10): return self._c.stop(timeout=timeout) + def wait(self): + return self._c.wait() + @property def exitcode(self): return self._c.attrs["State"]["ExitCode"] @@ -61,7 +64,7 @@ def build( dockerfile="", fileobj=None, path="", - **kwargs + **kwargs, ): return self._apiclient.build( buildargs=buildargs, @@ -99,7 +102,7 @@ def run( publish_all_ports=False, remove=False, volumes=None, - **kwargs + **kwargs, ): client = docker.from_env(version="auto") container = client.containers.run( diff --git a/repo2docker/engine.py b/repo2docker/engine.py index 6b9fa1170..2db8bfca2 100644 --- a/repo2docker/engine.py +++ b/repo2docker/engine.py @@ -62,6 +62,12 @@ def stop(self, *, timeout=10): timeout : If the container doesn't gracefully stop after this timeout kill it """ + @abstractmethod + def wait(self): + """ + Wait for the container to stop + """ + @property @abstractmethod def exitcode(self): @@ -161,7 +167,7 @@ def build( dockerfile="", fileobj=None, path="", - **kwargs + **kwargs, ): """ Build a container @@ -255,7 +261,7 @@ def run( publish_all_ports=False, remove=False, volumes={}, - **kwargs + **kwargs, ): """ Run a container