From c560f8ed1702930787db648ba2644adcdba65161 Mon Sep 17 00:00:00 2001 From: Aaron Virshup Date: Wed, 18 Oct 2017 13:57:46 -0700 Subject: [PATCH 01/12] Fix dockerfile writing if build includes a "copy_from" step --- dockermake/step.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/dockermake/step.py b/dockermake/step.py index ee1df23..51aabec 100644 --- a/dockermake/step.py +++ b/dockermake/step.py @@ -209,6 +209,24 @@ def build(self, client, pull=False, usecache=True): stage = staging.StagedFile(self.sourceimage, self.sourcepath, self.destpath) stage.stage(self.base_image, self.buildname) + @property + def dockerfile_lines(self): + w1 = colored( + 'WARNING: this build includes files that are built in other images!!! The generated' + '\n Dockerfile must be built in a directory that contains' + ' the file/directory:', + 'red', attrs=['bold']) + w2 = colored(' ' + self.sourcepath, 'red') + w3 = (colored(' from image ', 'red') + + colored(self.sourcepath, 'blue', attrs=['bold'])) + print('\n'.join((w1, w2, w3))) + return ["", + "# Warning: the file \"%s\" from the image \"%s\"" + " must be present in this build context!!" % + (self.sourcepath, self.sourceimage), + "ADD %s %s" % (os.path.basename(self.sourcepath), self.destpath), + ''] + class BuildError(Exception): def __init__(self, dockerfile, item, build_args): From 964c8df602a34bafc701014106d20cca4831ed7a Mon Sep 17 00:00:00 2001 From: Aaron Virshup Date: Wed, 18 Oct 2017 14:07:12 -0700 Subject: [PATCH 02/12] Add the test --- .gitignore | 1 + dockermake/builds.py | 2 ++ test/data/write.yml | 9 +++++++++ test/test_features.py | 6 ++++++ 4 files changed, 18 insertions(+) create mode 100644 test/data/write.yml diff --git a/.gitignore b/.gitignore index 067409f..e33d2ea 100644 --- a/.gitignore +++ b/.gitignore @@ -2,6 +2,7 @@ docker_makefiles Dockerfile.fail _docker_make_tmp dockerfile.fail +MANIFEST # IntelliJ project files .idea diff --git a/dockermake/builds.py b/dockermake/builds.py index 67019ff..2f1287b 100644 --- a/dockermake/builds.py +++ b/dockermake/builds.py @@ -43,6 +43,8 @@ def __init__(self, imagename, targetname, steps, sourcebuilds, from_image): self.from_image = from_image def write_dockerfile(self, output_dir): + """ Used only to write a Dockerfile that will NOT be built by docker-make + """ if not os.path.exists(output_dir): os.makedirs(output_dir) diff --git a/test/data/write.yml b/test/data/write.yml new file mode 100644 index 0000000..4d9188e --- /dev/null +++ b/test/data/write.yml @@ -0,0 +1,9 @@ +writetarget: + FROM: miniconda + description: "For testing write capabilities, not meant to be built" + copy_from: + temp: + /b: /c + +temp: + FROM: blah diff --git a/test/test_features.py b/test/test_features.py index 79c5646..53f8e60 100644 --- a/test/test_features.py +++ b/test/test_features.py @@ -1,3 +1,4 @@ +import os import pytest from dockermake.__main__ import _runargs as run_docker_make @@ -50,6 +51,11 @@ def test_ignore_directory(img6): _check_files('target_ignore_directory', d=False) +def test_dockerfile_write(tmpdir): + run_docker_make('-f data/write.yml -p -n --dockerfile-dir %s writetarget' % tmpdir) + assert os.path.isfile(os.path.join(tmpdir, 'Dockerfile.writetarget')) + + def _check_files(img, **present): for f, record in _FILES.items(): if not present.get(f, True): From eb2db3c6682346da09662ebdffe5aa17cb20b92e Mon Sep 17 00:00:00 2001 From: Aaron Virshup Date: Fri, 20 Oct 2017 11:53:44 -0700 Subject: [PATCH 03/12] Error checks for copy_from syntax --- dockermake/imagedefs.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/dockermake/imagedefs.py b/dockermake/imagedefs.py index efa3c58..b56a3d1 100644 --- a/dockermake/imagedefs.py +++ b/dockermake/imagedefs.py @@ -93,6 +93,20 @@ def _check_yaml_and_paths(ymlfilepath, yamldefs): if key in defn: defn[key] = _get_abspath(pathroot, defn[key]) + if 'copy_from' in defn: + if not isinstance(defn['copy_from'], dict): + raise errors.ParsingFailure(( + 'Syntax error in file "%s": \n' + + 'The "copy_from" field in image definition "%s" is not \n' + 'a key:value list.') % (ymlfilepath, imagename)) + for otherimg, value in defn.get('copy_from', {}).items(): + if not isinstance(value, dict): + raise errors.ParsingFailure(( + 'Syntax error in field:\n' + ' %s . copy_from . %s\nin file "%s". \n' + 'All entries must be of the form "sourcepath: destpath"')% + (imagename, otherimg, ymlfilepath)) + # save the file path for logging defn['_sourcefile'] = relpath From 806faf5b87f6068a853200d7e0e0d2a53fec00f6 Mon Sep 17 00:00:00 2001 From: Aaron Virshup Date: Fri, 20 Oct 2017 12:06:16 -0700 Subject: [PATCH 04/12] Better CLI error message for build failures --- dockermake/__main__.py | 7 ++++--- dockermake/errors.py | 23 ++++++++++++++++++++++- dockermake/step.py | 16 +--------------- 3 files changed, 27 insertions(+), 19 deletions(-) diff --git a/dockermake/__main__.py b/dockermake/__main__.py index 0592ef4..6330376 100755 --- a/dockermake/__main__.py +++ b/dockermake/__main__.py @@ -25,15 +25,16 @@ from . import errors +RED_ERROR = termcolor.colored('FATAL ERROR:', 'red') + def main(): parser = cli.make_arg_parser() args = parser.parse_args() try: run(args) - except errors.UserException as exc: - red_error = termcolor.colored('FATAL ERROR:', 'red') - print(red_error, exc.args[0], file=sys.stderr) + except (errors.UserException, errors.BuildError) as exc: + print(RED_ERROR, exc.args[0], file=sys.stderr) sys.exit(exc.CODE) diff --git a/dockermake/errors.py b/dockermake/errors.py index fe97f61..c52f582 100644 --- a/dockermake/errors.py +++ b/dockermake/errors.py @@ -1,3 +1,5 @@ +from __future__ import print_function + # Copyright 2017 Autodesk Inc. # # Licensed under the Apache License, Version 2.0 (the "License"); @@ -11,7 +13,9 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. - +from io import StringIO +import pprint +from termcolor import cprint class UserException(Exception): """ @@ -68,3 +72,20 @@ class ParsingFailure(UserException): class MultipleIgnoreError(UserException): CODE = 51 + + +class BuildError(Exception): + CODE = 200 + + def __init__(self, dockerfile, item, build_args): + with open('dockerfile.fail', 'w') as dff: + print(dockerfile, file=dff) + with StringIO() as stream: + cprint('Docker build failure', 'red', attrs=['bold'], file=stream) + print(u'\n -------- Docker daemon output --------', file=stream) + pprint.pprint(item, stream, indent=4) + print(u' -------- Arguments to client.build --------', file=stream) + pprint.pprint(build_args, stream, indent=4) + print(u'This dockerfile was written to dockerfile.fail', file=stream) + stream.seek(0) + super(BuildError, self).__init__(stream.read()) diff --git a/dockermake/step.py b/dockermake/step.py index 51aabec..51983e9 100644 --- a/dockermake/step.py +++ b/dockermake/step.py @@ -14,7 +14,6 @@ from __future__ import print_function import os -import pprint from io import StringIO, BytesIO import sys @@ -136,7 +135,7 @@ def build(self, client, pull=False, usecache=True): try: utils.stream_docker_logs(stream, self.buildname) except ValueError as e: - raise BuildError(dockerfile, e.args[0], build_args) + raise errors.BuildError(dockerfile, e.args[0], build_args) # remove the temporary dockerfile if tempdir is not None: @@ -227,16 +226,3 @@ def dockerfile_lines(self): "ADD %s %s" % (os.path.basename(self.sourcepath), self.destpath), ''] - -class BuildError(Exception): - def __init__(self, dockerfile, item, build_args): - with open('dockerfile.fail', 'w') as dff: - print(dockerfile, file=dff) - with BytesIO() as stream: - print('\n -------- Docker daemon output --------', file=stream) - pprint.pprint(item, stream, indent=4) - print(' -------- Arguments to client.build --------', file=stream) - pprint.pprint(build_args, stream, indent=4) - print('This dockerfile was written to dockerfile.fail', file=stream) - stream.seek(0) - super(BuildError, self).__init__(stream.read()) From 4abf76cf85609854216d639769c7bb6c1a8baa7f Mon Sep 17 00:00:00 2001 From: Aaron Virshup Date: Mon, 23 Oct 2017 10:22:29 -0700 Subject: [PATCH 05/12] Fix cache busting in python 3 --- dockermake/builds.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dockermake/builds.py b/dockermake/builds.py index 2f1287b..413155a 100644 --- a/dockermake/builds.py +++ b/dockermake/builds.py @@ -120,7 +120,7 @@ def build(self, client, def _get_stack_key(self, istep): names = [self.from_image] - for i in xrange(istep+1): + for i in range(istep+1): step = self.steps[i] if isinstance(step, FileCopyStep): continue From 2435bc09674104efe00149aebc34d2c25da99855 Mon Sep 17 00:00:00 2001 From: Aaron Virshup Date: Wed, 1 Nov 2017 15:28:03 -0700 Subject: [PATCH 06/12] Better build error handling --- dockermake/__main__.py | 11 +++++++---- dockermake/cli.py | 1 + dockermake/imagedefs.py | 11 +++++------ dockermake/staging.py | 4 +--- dockermake/step.py | 6 +++--- 5 files changed, 17 insertions(+), 16 deletions(-) diff --git a/dockermake/__main__.py b/dockermake/__main__.py index 6330376..640f8ad 100755 --- a/dockermake/__main__.py +++ b/dockermake/__main__.py @@ -31,11 +31,14 @@ def main(): parser = cli.make_arg_parser() args = parser.parse_args() - try: + if args.debug: run(args) - except (errors.UserException, errors.BuildError) as exc: - print(RED_ERROR, exc.args[0], file=sys.stderr) - sys.exit(exc.CODE) + else: + try: + run(args) + except (errors.UserException, errors.BuildError) as exc: + print(RED_ERROR, exc.args[0], file=sys.stderr) + sys.exit(exc.CODE) def _runargs(argstring): diff --git a/dockermake/cli.py b/dockermake/cli.py index 7430d61..ccd3d73 100644 --- a/dockermake/cli.py +++ b/dockermake/cli.py @@ -82,6 +82,7 @@ def make_arg_parser(): help="Print version and exit.") hh.add_argument('--help-yaml', action='store_true', help="Print summary of YAML file format and exit.") + hh.add_argument('--debug', action='store_true') return parser diff --git a/dockermake/imagedefs.py b/dockermake/imagedefs.py index b56a3d1..084037c 100644 --- a/dockermake/imagedefs.py +++ b/dockermake/imagedefs.py @@ -43,13 +43,12 @@ def __init__(self, makefile_path): print('Copy cache directory: %s' % staging.TMPDIR) try: self.ymldefs = self.parse_yaml(self.makefile_path) + except errors.UserException: + raise except Exception as exc: - if isinstance(exc, errors.UserException): - raise - else: - raise errors.ParsingFailure('Failed to read file %s:\n' % self.makefile_path + - str(exc)) - self.all_targets = self.ymldefs.pop('_ALL_', None) + raise errors.ParsingFailure('Failed to read file %s:\n' % self.makefile_path + + str(exc)) + self.all_targets = self.ymldefs.pop('_ALL_', []) self._external_dockerfiles = {} def parse_yaml(self, filename): diff --git a/dockermake/staging.py b/dockermake/staging.py index 060bfaf..82a1092 100644 --- a/dockermake/staging.py +++ b/dockermake/staging.py @@ -62,8 +62,6 @@ def stage(self, startimage, newimage): startimage (str): name of the image to stage these files into newimage (str): name of the created image """ - from .step import BuildError - client = utils.get_client() cprint(' Copying file from "%s:/%s" \n to "%s://%s/"' % (self.sourceimage, self.sourcepath, startimage, self.destpath), @@ -111,7 +109,7 @@ def stage(self, startimage, newimage): try: utils.stream_docker_logs(stream, newimage) except ValueError as e: - raise BuildError(dockerfile, e.args[0], build_args=buildargs) + raise errors.BuildError(dockerfile, e.args[0], build_args=buildargs) def _setcache(self, client): if self._sourceobj is None: # get image and set up cache if necessary diff --git a/dockermake/step.py b/dockermake/step.py index 51983e9..cbc0166 100644 --- a/dockermake/step.py +++ b/dockermake/step.py @@ -18,7 +18,7 @@ import sys from termcolor import cprint, colored -import docker.utils +import docker.utils, docker.errors from . import utils from . import staging @@ -134,8 +134,8 @@ def build(self, client, pull=False, usecache=True): stream = client.build(**build_args) try: utils.stream_docker_logs(stream, self.buildname) - except ValueError as e: - raise errors.BuildError(dockerfile, e.args[0], build_args) + except (ValueError, docker.errors.APIError) as e: + raise errors.BuildError(dockerfile, str(e), build_args) # remove the temporary dockerfile if tempdir is not None: From 513044c6954b66ea2531ae53fe9fb592b9daa49b Mon Sep 17 00:00:00 2001 From: Aaron Virshup Date: Thu, 2 Nov 2017 09:30:36 -0700 Subject: [PATCH 07/12] Add options to enable --cache-from option --- dockermake/cli.py | 18 ++++++++++++++++-- dockermake/imagedefs.py | 35 ++++++++++++++++++++++------------- dockermake/staging.py | 7 ++++++- dockermake/step.py | 36 ++++++++++++++++++++---------------- dockermake/utils.py | 17 ++++++++++------- 5 files changed, 74 insertions(+), 39 deletions(-) diff --git a/dockermake/cli.py b/dockermake/cli.py index ccd3d73..5e531c6 100644 --- a/dockermake/cli.py +++ b/dockermake/cli.py @@ -47,6 +47,18 @@ def make_arg_parser(): ca = parser.add_argument_group('Image caching') ca.add_argument('--pull', action='store_true', help='Always try to pull updated FROM images') + ca.add_argument('--cache-repo', + help='Repository to use for cached images. This allows you to invoke the ' + '`docker build --build-from` option for each image.' + 'For instance, running ' + '`docker-make foo bar --cache-repo docker.io/cache` will use ' + 'docker.io/cache/foo as a cache for `foo` and docker.io/cache/bar as a cache' + 'for `bar`.', + default='') + ca.add_argument('--cache-tag', + help='Tag to use for cached images; ' + 'can be used with the --cache-repo option (see above).', + default='') ca.add_argument('--no-cache', action='store_true', help="Rebuild every layer") ca.add_argument('--bust-cache', action='append', @@ -60,8 +72,10 @@ def make_arg_parser(): help="Prepend this repository to all built images, e.g.\n" "`docker-make hello-world -u quay.io/elvis` will tag the image " "as `quay.io/elvis/hello-world`. You can add a ':' to the end to " - "image names into tags:\n `docker-make -u quay.io/elvis/repo: hello-world` " - "will create the image in the elvis repository: quay.io/elvis/repo:hello-world") + "image names into tags:\n " + "`docker-make -u quay.io/elvis/repo: hello-world` " + "will create the " + "image in the elvis repository: quay.io/elvis/repo:hello-world") rt.add_argument('--tag', '-t', type=str, help='Tag all built images with this tag. If image names are ALREADY tags (i.e.,' ' your repo name ends in a ":"), this will append the tag name with a dash. ' diff --git a/dockermake/imagedefs.py b/dockermake/imagedefs.py index 084037c..e0e6834 100644 --- a/dockermake/imagedefs.py +++ b/dockermake/imagedefs.py @@ -26,6 +26,7 @@ from . import builds from . import staging from . import errors +from . import utils RECOGNIZED_KEYS = set(('requires build_directory build copy_from FROM description _sourcefile' ' FROM_DOCKERFILE ignore ignorefile') @@ -120,7 +121,7 @@ def _check_yaml_and_paths(ymlfilepath, yamldefs): 'Field "%s" in image "%s" in file "%s" not recognized' % (key, imagename, relpath)) - def generate_build(self, image, targetname, rebuilds=None): + def generate_build(self, image, targetname, rebuilds=None, cache_repo='', cache_tag=''): """ Separate the build into a series of one or more intermediate steps. Each specified build directory gets its own step @@ -129,8 +130,14 @@ def generate_build(self, image, targetname, rebuilds=None): image (str): name of the image as defined in the dockermake.py file targetname (str): name to tag the final built image with rebuilds (List[str]): list of image layers to rebuild (i.e., without docker's cache) + cache_repo (str): repository to get images for caches in builds + cache_tag (str): tags to use from repository for caches in builds """ from_image = self.get_external_base_image(image) + if cache_repo or cache_tag: + from_cache = utils.generate_name(image, cache_repo, cache_tag) + else: + from_cache = None if from_image is None: raise errors.NoBaseError("No base image found in %s's dependencies" % image) if isinstance(from_image, ExternalDockerfile): @@ -150,12 +157,12 @@ def generate_build(self, image, targetname, rebuilds=None): for base_name in self.sort_dependencies(image): istep += 1 buildname = 'dmkbuild_%s_%d' % (image, istep) - build_steps.append(dockermake.step.BuildStep(base_name, - base_image, - self.ymldefs[base_name], - buildname, - bust_cache=base_name in rebuilds, - build_first=build_first)) + build_steps.append( + dockermake.step.BuildStep( + base_name, base_image, self.ymldefs[base_name], + buildname, bust_cache=base_name in rebuilds, + build_first=build_first, from_cache=from_cache)) + base_image = buildname build_first = None @@ -164,14 +171,16 @@ def generate_build(self, image, targetname, rebuilds=None): for sourcepath, destpath in iteritems(files): istep += 1 buildname = 'dmkbuild_%s_%d' % (image, istep) - build_steps.append(dockermake.step.FileCopyStep(sourceimage, sourcepath, - base_image, destpath, - buildname, - self.ymldefs[base_name], - base_name)) + build_steps.append( + dockermake.step.FileCopyStep( + sourceimage, sourcepath, destpath, + base_name, base_image, self.ymldefs[base_name], + buildname, bust_cache=base_name in rebuilds, + build_first=build_first, from_cache=from_cache)) base_image = buildname - sourcebuilds = [self.generate_build(img, img) for img in sourceimages] + sourcebuilds = [self.generate_build(img, img, cache_repo=cache_repo, cache_tag=cache_tag) + for img in sourceimages] return builds.BuildTarget(imagename=image, targetname=targetname, diff --git a/dockermake/staging.py b/dockermake/staging.py index 82a1092..b5285a2 100644 --- a/dockermake/staging.py +++ b/dockermake/staging.py @@ -47,13 +47,15 @@ class StagedFile(object): sourceimage (str): name of the image to copy from sourcepath (str): path in the source image destpath (str): path in the target image + from_cache (str or list): use this(these) image(s) to resolve build cache """ - def __init__(self, sourceimage, sourcepath, destpath): + def __init__(self, sourceimage, sourcepath, destpath, from_cache=None): self.sourceimage = sourceimage self.sourcepath = sourcepath self.destpath = destpath self._sourceobj = None self._cachedir = None + self.from_cache = from_cache def stage(self, startimage, newimage): """ Copies the file from source to target @@ -104,6 +106,9 @@ def stage(self, startimage, newimage): tag=newimage, decode=True) + if self.from_cache: + buildargs['from_cache'] = self.from_cache + # Build and show logs stream = client.api.build(**buildargs) try: diff --git a/dockermake/step.py b/dockermake/step.py index cbc0166..576852b 100644 --- a/dockermake/step.py +++ b/dockermake/step.py @@ -35,10 +35,11 @@ class BuildStep(object): img_def (dict): yaml definition of this image buildname (str): what to call this image, once built bust_cache(bool): never use docker cache for this build step + from_cache (str or list): use this(these) image(s) to resolve build cache """ def __init__(self, imagename, baseimage, img_def, buildname, - build_first=None, bust_cache=False): + build_first=None, bust_cache=False, from_cache=None): self.imagename = imagename self.baseimage = baseimage self.dockerfile_lines = ['FROM %s\n' % baseimage, img_def.get('build', '')] @@ -49,6 +50,10 @@ def __init__(self, imagename, baseimage, img_def, buildname, self.build_first = build_first self.custom_exclude = self._get_ignorefile(img_def) self.ignoredefs_file = img_def.get('ignorefile', img_def['_sourcefile']) + if from_cache and isinstance(str, from_cache): + self.from_cache = [from_cache] + else: + self.from_cache = from_cache @staticmethod def _get_ignorefile(img_def): @@ -98,6 +103,9 @@ def build(self, client, pull=False, usecache=True): nocache=not usecache, decode=True, rm=True) + if usecache and self.from_cache: + build_args['from_cache'] = self.from_cache + if self.build_dir is not None: tempdir = self.write_dockerfile(dockerfile) context_path = os.path.abspath(os.path.expanduser(self.build_dir)) @@ -180,24 +188,19 @@ class FileCopyStep(BuildStep): Args: sourceimage (str): name of image to copy file from sourcepath (str): file path in source image - base_image (str): name of image to copy file into destpath (str): directory to copy the file into - buildname (str): name of the built image - ymldef (Dict): yml definition of this build step - definitionname (str): name of this definition + imagename (str): name of this image definition + baseimage (str): base image for this step + img_def (dict): yaml definition of this image + buildname (str): what to call this image, once built + from_cache (str or list): use this(these) image(s) to resolve build cache """ - - bust_cache = False # can't bust this - - def __init__(self, sourceimage, sourcepath, base_image, destpath, buildname, - ymldef, definitionname): + def __init__(self, sourceimage, sourcepath, destpath, *args, **kwargs): + kwargs.pop('bust_cache', None) + super().__init__(*args, **kwargs) self.sourceimage = sourceimage self.sourcepath = sourcepath - self.base_image = base_image self.destpath = destpath - self.buildname = buildname - self.definitionname = definitionname - self.sourcefile = ymldef['_sourcefile'] def build(self, client, pull=False, usecache=True): """ @@ -205,8 +208,9 @@ def build(self, client, pull=False, usecache=True): `pull` and `usecache` are for compatibility only. They're irrelevant because hey were applied when BUILDING self.sourceimage """ - stage = staging.StagedFile(self.sourceimage, self.sourcepath, self.destpath) - stage.stage(self.base_image, self.buildname) + stage = staging.StagedFile(self.sourceimage, self.sourcepath, self.destpath, + from_cache=self.from_cache) + stage.stage(self.baseimage, self.buildname) @property def dockerfile_lines(self): diff --git a/dockermake/utils.py b/dockermake/utils.py index bfee1a8..26162df 100644 --- a/dockermake/utils.py +++ b/dockermake/utils.py @@ -15,7 +15,6 @@ import collections import os -import sys import textwrap import yaml @@ -50,8 +49,8 @@ def list_image_defs(args, defs): return -def generate_name(image, args): - repo_base = args.repository +def generate_name(image, repo, tag): + repo_base = repo if repo_base is not None: if repo_base[-1] not in ':/': @@ -60,11 +59,11 @@ def generate_name(image, args): else: repo_name = image - if args.tag: + if tag: if ':' in repo_name: - repo_name += '-'+args.tag + repo_name += '-'+tag else: - repo_name += ':'+args.tag + repo_name += ':'+tag return repo_name @@ -109,7 +108,11 @@ def build_targets(args, defs, targets): print("\nREGISTRY LOGIN SUCCESS:",registry) built, warnings = [], [] - builders = [defs.generate_build(t, generate_name(t, args), rebuilds=args.bust_cache) + builders = [defs.generate_build(t, + generate_name(t, args.repository, args.tag), + rebuilds=args.bust_cache, + cache_repo=args.cache_repo, + cache_tag=args.cache_tag) for t in targets] for b in builders: b.build(client, From 66a7a3eb006c8c829078aaf5ec973ffcbef495ba Mon Sep 17 00:00:00 2001 From: Aaron Virshup Date: Thu, 2 Nov 2017 20:38:14 -0500 Subject: [PATCH 08/12] Fix `dockerfile_lines` attribute --- dockermake/step.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/dockermake/step.py b/dockermake/step.py index 576852b..b10e829 100644 --- a/dockermake/step.py +++ b/dockermake/step.py @@ -42,7 +42,7 @@ def __init__(self, imagename, baseimage, img_def, buildname, build_first=None, bust_cache=False, from_cache=None): self.imagename = imagename self.baseimage = baseimage - self.dockerfile_lines = ['FROM %s\n' % baseimage, img_def.get('build', '')] + self.img_def = img_def self.buildname = buildname self.build_dir = img_def.get('build_directory', None) self.bust_cache = bust_cache @@ -180,6 +180,11 @@ def build_external_dockerfile(client, image): image.built = True cprint(" Finished building Dockerfile at %s" % image.path, 'green') + @property + def dockerfile_lines(self): + return ['FROM %s\n' % self.baseimage, + self.img_def.get('build', '')] + class FileCopyStep(BuildStep): """ @@ -214,6 +219,9 @@ def build(self, client, pull=False, usecache=True): @property def dockerfile_lines(self): + """ + Used only when printing dockerfiles, not for building + """ w1 = colored( 'WARNING: this build includes files that are built in other images!!! The generated' '\n Dockerfile must be built in a directory that contains' From ea4443c4fc372df8d8586625849945fc30be2ef8 Mon Sep 17 00:00:00 2001 From: Aaron Virshup Date: Thu, 2 Nov 2017 20:47:29 -0500 Subject: [PATCH 09/12] Use py2 super --- dockermake/step.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dockermake/step.py b/dockermake/step.py index b10e829..e132704 100644 --- a/dockermake/step.py +++ b/dockermake/step.py @@ -202,7 +202,7 @@ class FileCopyStep(BuildStep): """ def __init__(self, sourceimage, sourcepath, destpath, *args, **kwargs): kwargs.pop('bust_cache', None) - super().__init__(*args, **kwargs) + super(FileCopyStep, self).__init__(*args, **kwargs) self.sourceimage = sourceimage self.sourcepath = sourcepath self.destpath = destpath From bc4482ea09bf2d46c10df6927062ac959d9948fa Mon Sep 17 00:00:00 2001 From: Aaron Virshup Date: Thu, 2 Nov 2017 21:22:59 -0500 Subject: [PATCH 10/12] Caching tests --- test/data/simple.yml | 4 ++++ test/test_features.py | 31 +++++++++++++++++++++++++++++++ 2 files changed, 35 insertions(+) create mode 100644 test/data/simple.yml diff --git a/test/data/simple.yml b/test/data/simple.yml new file mode 100644 index 0000000..3288c7f --- /dev/null +++ b/test/data/simple.yml @@ -0,0 +1,4 @@ +simple-target: + FROM: python:3.6-slim + build: | + RUN echo "spam egg foo bar" > /opt/sometext.txt diff --git a/test/test_features.py b/test/test_features.py index 53f8e60..91a5064 100644 --- a/test/test_features.py +++ b/test/test_features.py @@ -6,6 +6,11 @@ # note: these tests MUST be run with CWD REPO_ROOT/tests +@pytest.fixture(scope='session') +def docker_client(): + import docker + return docker.from_env() + img1 = creates_images(*'target2_bases target3_bases'.split()) def test_multiple_bases(img1): @@ -56,6 +61,32 @@ def test_dockerfile_write(tmpdir): assert os.path.isfile(os.path.join(tmpdir, 'Dockerfile.writetarget')) +img7 = creates_images('simple-target') +@pytest.fixture(scope='function') +def twin_simple_targets(img7, docker_client): + run_docker_make('-f data/simple.yml simple-target') + image1 = docker_client.images.get('simple-target') + run_docker_make('-f data/simple.yml simple-target --no-cache') + image2 = docker_client.images.get('simple-target') + return image1, image2 + + +def test_no_cache(twin_simple_targets): + image1, image2 = twin_simple_targets + assert image1.id != image2.id + + +def test_explicit_cache_from(twin_simple_targets, docker_client): + image1, image2 = twin_simple_targets + image1.tag('img1repo/simple-target', tag='img1tag') + image2.tag('img2repo/simple-target', tag='img2tag') + + run_docker_make('-f data/simple.yml simple-target' + ' --cache-repo img1repo --cache-tag img1tag') + final_image = docker_client.images.get('simple-target') + assert final_image.id == image1.id + + def _check_files(img, **present): for f, record in _FILES.items(): if not present.get(f, True): From 77452102c28ce0b3516a03025001e3e116a374a4 Mon Sep 17 00:00:00 2001 From: Aaron Virshup Date: Thu, 2 Nov 2017 21:23:27 -0500 Subject: [PATCH 11/12] Fix cache_from spelling --- dockermake/imagedefs.py | 8 ++++---- dockermake/staging.py | 10 +++++----- dockermake/step.py | 18 +++++++++--------- 3 files changed, 18 insertions(+), 18 deletions(-) diff --git a/dockermake/imagedefs.py b/dockermake/imagedefs.py index e0e6834..8713569 100644 --- a/dockermake/imagedefs.py +++ b/dockermake/imagedefs.py @@ -135,9 +135,9 @@ def generate_build(self, image, targetname, rebuilds=None, cache_repo='', cache_ """ from_image = self.get_external_base_image(image) if cache_repo or cache_tag: - from_cache = utils.generate_name(image, cache_repo, cache_tag) + cache_from = utils.generate_name(image, cache_repo, cache_tag) else: - from_cache = None + cache_from = None if from_image is None: raise errors.NoBaseError("No base image found in %s's dependencies" % image) if isinstance(from_image, ExternalDockerfile): @@ -161,7 +161,7 @@ def generate_build(self, image, targetname, rebuilds=None, cache_repo='', cache_ dockermake.step.BuildStep( base_name, base_image, self.ymldefs[base_name], buildname, bust_cache=base_name in rebuilds, - build_first=build_first, from_cache=from_cache)) + build_first=build_first, cache_from=cache_from)) base_image = buildname build_first = None @@ -176,7 +176,7 @@ def generate_build(self, image, targetname, rebuilds=None, cache_repo='', cache_ sourceimage, sourcepath, destpath, base_name, base_image, self.ymldefs[base_name], buildname, bust_cache=base_name in rebuilds, - build_first=build_first, from_cache=from_cache)) + build_first=build_first, cache_from=cache_from)) base_image = buildname sourcebuilds = [self.generate_build(img, img, cache_repo=cache_repo, cache_tag=cache_tag) diff --git a/dockermake/staging.py b/dockermake/staging.py index b5285a2..6d1f7b4 100644 --- a/dockermake/staging.py +++ b/dockermake/staging.py @@ -47,15 +47,15 @@ class StagedFile(object): sourceimage (str): name of the image to copy from sourcepath (str): path in the source image destpath (str): path in the target image - from_cache (str or list): use this(these) image(s) to resolve build cache + cache_from (str or list): use this(these) image(s) to resolve build cache """ - def __init__(self, sourceimage, sourcepath, destpath, from_cache=None): + def __init__(self, sourceimage, sourcepath, destpath, cache_from=None): self.sourceimage = sourceimage self.sourcepath = sourcepath self.destpath = destpath self._sourceobj = None self._cachedir = None - self.from_cache = from_cache + self.cache_from = cache_from def stage(self, startimage, newimage): """ Copies the file from source to target @@ -106,8 +106,8 @@ def stage(self, startimage, newimage): tag=newimage, decode=True) - if self.from_cache: - buildargs['from_cache'] = self.from_cache + if self.cache_from: + buildargs['cache_from'] = self.cache_from # Build and show logs stream = client.api.build(**buildargs) diff --git a/dockermake/step.py b/dockermake/step.py index e132704..2507205 100644 --- a/dockermake/step.py +++ b/dockermake/step.py @@ -35,11 +35,11 @@ class BuildStep(object): img_def (dict): yaml definition of this image buildname (str): what to call this image, once built bust_cache(bool): never use docker cache for this build step - from_cache (str or list): use this(these) image(s) to resolve build cache + cache_from (str or list): use this(these) image(s) to resolve build cache """ def __init__(self, imagename, baseimage, img_def, buildname, - build_first=None, bust_cache=False, from_cache=None): + build_first=None, bust_cache=False, cache_from=None): self.imagename = imagename self.baseimage = baseimage self.img_def = img_def @@ -50,10 +50,10 @@ def __init__(self, imagename, baseimage, img_def, buildname, self.build_first = build_first self.custom_exclude = self._get_ignorefile(img_def) self.ignoredefs_file = img_def.get('ignorefile', img_def['_sourcefile']) - if from_cache and isinstance(str, from_cache): - self.from_cache = [from_cache] + if cache_from and isinstance(cache_from, str): + self.cache_from = [cache_from] else: - self.from_cache = from_cache + self.cache_from = cache_from @staticmethod def _get_ignorefile(img_def): @@ -103,8 +103,8 @@ def build(self, client, pull=False, usecache=True): nocache=not usecache, decode=True, rm=True) - if usecache and self.from_cache: - build_args['from_cache'] = self.from_cache + if usecache and self.cache_from: + build_args['cache_from'] = self.cache_from if self.build_dir is not None: tempdir = self.write_dockerfile(dockerfile) @@ -198,7 +198,7 @@ class FileCopyStep(BuildStep): baseimage (str): base image for this step img_def (dict): yaml definition of this image buildname (str): what to call this image, once built - from_cache (str or list): use this(these) image(s) to resolve build cache + cache_from (str or list): use this(these) image(s) to resolve build cache """ def __init__(self, sourceimage, sourcepath, destpath, *args, **kwargs): kwargs.pop('bust_cache', None) @@ -214,7 +214,7 @@ def build(self, client, pull=False, usecache=True): hey were applied when BUILDING self.sourceimage """ stage = staging.StagedFile(self.sourceimage, self.sourcepath, self.destpath, - from_cache=self.from_cache) + cache_from=self.cache_from) stage.stage(self.baseimage, self.buildname) @property From 6bdf12319b0d38ae70f938c8fcbc3db18d29ecc7 Mon Sep 17 00:00:00 2001 From: Aaron Virshup Date: Thu, 2 Nov 2017 22:24:25 -0500 Subject: [PATCH 12/12] path object issues --- test/test_features.py | 1 + 1 file changed, 1 insertion(+) diff --git a/test/test_features.py b/test/test_features.py index 91a5064..734db4f 100644 --- a/test/test_features.py +++ b/test/test_features.py @@ -57,6 +57,7 @@ def test_ignore_directory(img6): def test_dockerfile_write(tmpdir): + tmpdir = str(tmpdir) run_docker_make('-f data/write.yml -p -n --dockerfile-dir %s writetarget' % tmpdir) assert os.path.isfile(os.path.join(tmpdir, 'Dockerfile.writetarget'))