Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
127 changes: 90 additions & 37 deletions compose/config/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
from __future__ import unicode_literals

import codecs
import functools
import logging
import operator
import os
Expand Down Expand Up @@ -437,6 +438,11 @@ def resolve_environment(service_dict):
return dict(resolve_env_var(k, v) for k, v in six.iteritems(env))


def resolve_build_args(build):
args = parse_build_arguments(build.get('args'))
return dict(resolve_env_var(k, v) for k, v in six.iteritems(args))


def validate_extended_service_dict(service_dict, filename, service):
error_prefix = "Cannot extend service '%s' in %s:" % (service, filename)

Expand Down Expand Up @@ -474,12 +480,16 @@ def process_service(service_config):
for path in to_list(service_dict['env_file'])
]

if 'build' in service_dict:
if isinstance(service_dict['build'], six.string_types):
service_dict['build'] = resolve_build_path(working_dir, service_dict['build'])
elif isinstance(service_dict['build'], dict) and 'context' in service_dict['build']:
path = service_dict['build']['context']
service_dict['build']['context'] = resolve_build_path(working_dir, path)

if 'volumes' in service_dict and service_dict.get('volume_driver') is None:
service_dict['volumes'] = resolve_volume_paths(working_dir, service_dict)

if 'build' in service_dict:
service_dict['build'] = resolve_build_path(working_dir, service_dict['build'])

if 'labels' in service_dict:
service_dict['labels'] = parse_labels(service_dict['labels'])

Expand Down Expand Up @@ -515,6 +525,8 @@ def finalize_service(service_config):
if 'restart' in service_dict:
service_dict['restart'] = parse_restart_spec(service_dict['restart'])

normalize_build(service_dict, service_config.working_dir)

return normalize_v1_service_format(service_dict)


Expand Down Expand Up @@ -579,10 +591,31 @@ def merge_mapping(mapping, parse_func):

if version == 1:
legacy_v1_merge_image_or_build(d, base, override)
else:
merge_build(d, base, override)

return d


def merge_build(output, base, override):
build = {}

if 'build' in base:
if isinstance(base['build'], six.string_types):
build['context'] = base['build']
else:
build.update(base['build'])

if 'build' in override:
if isinstance(override['build'], six.string_types):
build['context'] = override['build']
else:
build.update(override['build'])

if build:
output['build'] = build


def legacy_v1_merge_image_or_build(output, base, override):
output.pop('image', None)
output.pop('build', None)
Expand All @@ -602,29 +635,41 @@ def merge_environment(base, override):
return env


def parse_environment(environment):
if not environment:
def split_env(env):
if isinstance(env, six.binary_type):
env = env.decode('utf-8', 'replace')
if '=' in env:
return env.split('=', 1)
else:
return env, None


def split_label(label):
if '=' in label:
return label.split('=', 1)
else:
return label, ''


def parse_dict_or_list(split_func, type_name, arguments):
if not arguments:
return {}

if isinstance(environment, list):
return dict(split_env(e) for e in environment)
if isinstance(arguments, list):
return dict(split_func(e) for e in arguments)

if isinstance(environment, dict):
return dict(environment)
if isinstance(arguments, dict):
return dict(arguments)

raise ConfigurationError(
"environment \"%s\" must be a list or mapping," %
environment
"%s \"%s\" must be a list or mapping," %
(type_name, arguments)
)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is almost identical to parse_environment() (and really parse_labels() is as well), I think we should have just one function that takes two params for the differences between them:

def parse_dict_or_list(split_func, type_name, arguments):
    ...

parse_build_arguments = functools.partial(parse_dict_or_list, split_env, 'build arguments')
parse_environment = functools.partial(parse_dict_or_list, split_env, 'build arguments')
parse_labels = functools.partial(parse_dict_or_list, split_label, 'labels')

These could be single line functions instead of using functools.partial, I'm partial to functools.



def split_env(env):
if isinstance(env, six.binary_type):
env = env.decode('utf-8', 'replace')
if '=' in env:
return env.split('=', 1)
else:
return env, None
parse_build_arguments = functools.partial(parse_dict_or_list, split_env, 'build arguments')
parse_environment = functools.partial(parse_dict_or_list, split_env, 'environment')
parse_labels = functools.partial(parse_dict_or_list, split_label, 'labels')


def resolve_env_var(key, val):
Expand Down Expand Up @@ -670,6 +715,26 @@ def resolve_volume_path(working_dir, volume):
return container_path


def normalize_build(service_dict, working_dir):
build = {}

# supported in V1 only
if 'dockerfile' in service_dict:
build['dockerfile'] = service_dict.pop('dockerfile')

if 'build' in service_dict:
# Shortcut where specifying a string is treated as the build context
if isinstance(service_dict['build'], six.string_types):
build['context'] = service_dict.pop('build')
else:
build.update(service_dict['build'])
if 'args' in build:
build['args'] = resolve_build_args(build)

if build:
service_dict['build'] = build


def resolve_build_path(working_dir, build_path):
if is_url(build_path):
return build_path
Expand All @@ -682,7 +747,13 @@ def is_url(build_path):

def validate_paths(service_dict):
if 'build' in service_dict:
build_path = service_dict['build']
build = service_dict.get('build', {})

if isinstance(build, six.string_types):
build_path = build
elif isinstance(build, dict) and 'context' in build:
build_path = build['context']
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would expect that build would be normalized to always be a dict() by the time it gets here, do we really need the string case still?

If we do need it, could we have process_service() always return a dict for build so we don't have to worry about the variation after that point.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The issue is that if we normalize build in process_service then v1 schema validation will fail. We could potentially add a normalize_build() immediately after validate_against_service_schema, but then we still need conditional logic in validation.py that you also commented on.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah true, so I guess it can't be normalized before finalize_service(), ok


if (
not is_url(build_path) and
(not os.path.exists(build_path) or not os.access(build_path, os.R_OK))
Expand Down Expand Up @@ -737,24 +808,6 @@ def join_path_mapping(pair):
return ":".join((host, container))


def parse_labels(labels):
if not labels:
return {}

if isinstance(labels, list):
return dict(split_label(e) for e in labels)

if isinstance(labels, dict):
return dict(labels)


def split_label(label):
if '=' in label:
return label.split('=', 1)
else:
return label, ''


def parse_ulimits(ulimits):
if not ulimits:
return {}
Expand Down
15 changes: 14 additions & 1 deletion compose/config/service_schema_v2.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,20 @@
"type": "object",

"properties": {
"build": {"type": "string"},
"build": {
"oneOf": [
{"type": "string"},
{
"type": "object",
"properties": {
"context": {"type": "string"},
"dockerfile": {"type": "string"},
"args": {"$ref": "#/definitions/list_or_dict"}
},
"additionalProperties": false
}
]
},
"cap_add": {"type": "array", "items": {"type": "string"}, "uniqueItems": true},
"cap_drop": {"type": "array", "items": {"type": "string"}, "uniqueItems": true},
"cgroup_parent": {"type": "string"},
Expand Down
17 changes: 14 additions & 3 deletions compose/config/validation.py
Original file line number Diff line number Diff line change
Expand Up @@ -151,18 +151,29 @@ def handle_error_for_schema_with_id(error, service_name):
VALID_NAME_CHARS)

if schema_id == '#/definitions/constraints':
# Build context could in 'build' or 'build.context' and dockerfile could be
# in 'dockerfile' or 'build.dockerfile'
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With my proposed change to have process_service() always return build as a dict, I don't think we'll need to worry about this here either. We can assume a consistent representation.

context = False
dockerfile = 'dockerfile' in error.instance
if 'build' in error.instance:
if isinstance(error.instance['build'], six.string_types):
context = True
else:
context = 'context' in error.instance['build']
dockerfile = dockerfile or 'dockerfile' in error.instance['build']

# TODO: only applies to v1
if 'image' in error.instance and 'build' in error.instance:
if 'image' in error.instance and context:
return (
"Service '{}' has both an image and build path specified. "
"A service can either be built to image or use an existing "
"image, not both.".format(service_name))
if 'image' not in error.instance and 'build' not in error.instance:
if 'image' not in error.instance and not context:
return (
"Service '{}' has neither an image nor a build path "
"specified. At least one must be provided.".format(service_name))
# TODO: only applies to v1
if 'image' in error.instance and 'dockerfile' in error.instance:
if 'image' in error.instance and dockerfile:
return (
"Service '{}' has both an image and alternate Dockerfile. "
"A service can either be built to image or use an existing "
Expand Down
6 changes: 4 additions & 2 deletions compose/service.py
Original file line number Diff line number Diff line change
Expand Up @@ -619,7 +619,8 @@ def _get_container_host_config(self, override_options, one_off=False):
def build(self, no_cache=False, pull=False, force_rm=False):
log.info('Building %s' % self.name)

path = self.options['build']
build_opts = self.options.get('build', {})
path = build_opts.get('context')
# python2 os.path() doesn't support unicode, so we need to encode it to
# a byte string
if not six.PY3:
Expand All @@ -633,7 +634,8 @@ def build(self, no_cache=False, pull=False, force_rm=False):
forcerm=force_rm,
pull=pull,
nocache=no_cache,
dockerfile=self.options.get('dockerfile', None),
dockerfile=build_opts.get('dockerfile', None),
buildargs=build_opts.get('args', None),
)

try:
Expand Down
78 changes: 64 additions & 14 deletions docs/compose-file.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@ those files, all the [services](#service-configuration-reference) are declared
at the root of the document.

Version 1 files do not support the declaration of
named [volumes](#volume-configuration-reference)
named [volumes](#volume-configuration-reference) or
[build arguments](#args).

Example:

Expand Down Expand Up @@ -89,6 +90,30 @@ definition.

### build

Configuration options that are applied at build time.

In version 1 this must be given as a string representing the context.

build: .

In version 2 this can alternatively be given as an object with extra options.

version: 2
services:
web:
build: .

version: 2
services:
web:
build:
context: .
dockerfile: Dockerfile-alternate
args:
buildno: 1
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have #2622 to finalize this, but I think what we'd like to do is only include the v2 docs in this section, and have a separate section at the bottom for migration, and differences between the versions.


#### context

Either a path to a directory containing a Dockerfile, or a url to a git repository.

When the value supplied is a relative path, it is interpreted as relative to the
Expand All @@ -99,9 +124,46 @@ Compose will build and tag it with a generated name, and use that image thereaft

build: /path/to/build/dir

Using `build` together with `image` is not allowed. Attempting to do so results in
build:
context: /path/to/build/dir

Using `context` together with `image` is not allowed. Attempting to do so results in
an error.

#### dockerfile

Alternate Dockerfile.

Compose will use an alternate file to build with. A build path must also be
specified using the `build` key.

build:
context: /path/to/build/dir
dockerfile: Dockerfile-alternate

Using `dockerfile` together with `image` is not allowed. Attempting to do so results in an error.

#### args

Add build arguments. You can use either an array or a dictionary. Any
boolean values; true, false, yes, no, need to be enclosed in quotes to ensure
they are not converted to True or False by the YML parser.

Build arguments with only a key are resolved to their environment value on the
machine Compose is running on.

> **Note:** Introduced in version 2 of the compose file format.

build:
args:
buildno: 1
user: someuser

build:
args:
- buildno=1
- user=someuser

### cap_add, cap_drop

Add or drop container capabilities.
Expand Down Expand Up @@ -162,18 +224,6 @@ Custom DNS search domains. Can be a single value or a list.
- dc1.example.com
- dc2.example.com

### dockerfile

Alternate Dockerfile.

Compose will use an alternate file to build with. A build path must also be
specified using the `build` key.

build: /path/to/build/dir
dockerfile: Dockerfile-alternate

Using `dockerfile` together with `image` is not allowed. Attempting to do so results in an error.

### env_file

Add environment variables from a file. Can be a single value or a list.
Expand Down
Loading