From 449dcc9d7b10a05aa8a682c6d2f5d1a686eb8843 Mon Sep 17 00:00:00 2001 From: Dat Tran Date: Thu, 11 Aug 2016 07:54:28 -0700 Subject: [PATCH 1/2] support --build-arg for build command Signed-off-by: Dat Tran --- compose/cli/main.py | 20 ++++++++++++++------ compose/config/__init__.py | 1 + compose/config/config.py | 6 ++++++ compose/project.py | 4 ++-- compose/service.py | 12 +++++++++--- tests/unit/config/config_test.py | 19 +++++++++++++++++++ tests/unit/service_test.py | 19 +++++++++++++++++++ 7 files changed, 70 insertions(+), 11 deletions(-) diff --git a/compose/cli/main.py b/compose/cli/main.py index 810b13f54..78e3d84b5 100644 --- a/compose/cli/main.py +++ b/compose/cli/main.py @@ -209,18 +209,26 @@ class TopLevelCommand(object): e.g. `composetest_db`. If you change a service's `Dockerfile` or the contents of its build directory, you can run `docker-compose build` to rebuild it. - Usage: build [options] [SERVICE...] + Usage: build [options] [--build-arg key=val...] [SERVICE...] Options: - --force-rm Always remove intermediate containers. - --no-cache Do not use cache when building the image. - --pull Always attempt to pull a newer version of the image. + --force-rm Always remove intermediate containers. + --no-cache Do not use cache when building the image. + --pull Always attempt to pull a newer version of the image. + --build-arg key=val Set build-time variables for one service. """ + service_names = options['SERVICE'] + build_args = options.get('--build-arg', None) + + if not service_names and build_args: + raise UserError("Need service name for --build-arg option") + self.project.build( - service_names=options['SERVICE'], + service_names=service_names, no_cache=bool(options.get('--no-cache', False)), pull=bool(options.get('--pull', False)), - force_rm=bool(options.get('--force-rm', False))) + force_rm=bool(options.get('--force-rm', False)), + build_args=build_args) def bundle(self, config_options, options): """ diff --git a/compose/config/__init__.py b/compose/config/__init__.py index 7cf71eb98..b6e5e8d38 100644 --- a/compose/config/__init__.py +++ b/compose/config/__init__.py @@ -7,5 +7,6 @@ from .config import ConfigurationError from .config import DOCKER_CONFIG_KEYS from .config import find from .config import load +from .config import merge_build_args from .config import merge_environment from .config import parse_environment diff --git a/compose/config/config.py b/compose/config/config.py index dbf64bae2..718d3bf02 100644 --- a/compose/config/config.py +++ b/compose/config/config.py @@ -602,6 +602,12 @@ def resolve_environment(service_dict, environment=None): return dict(resolve_env_var(k, v, environment) for k, v in six.iteritems(env)) +def merge_build_args(base, override, environment): + override_args = parse_build_arguments(override) + override_dict = dict(resolve_env_var(k, v, environment) for k, v in six.iteritems(override_args)) + base.update(override_dict) + + def resolve_build_args(build, environment): args = parse_build_arguments(build.get('args')) return dict(resolve_env_var(k, v, environment) for k, v in six.iteritems(args)) diff --git a/compose/project.py b/compose/project.py index 5c21f3bf0..a75d71efc 100644 --- a/compose/project.py +++ b/compose/project.py @@ -307,10 +307,10 @@ class Project(object): 'Restarting') return containers - def build(self, service_names=None, no_cache=False, pull=False, force_rm=False): + def build(self, service_names=None, no_cache=False, pull=False, force_rm=False, build_args=None): for service in self.get_services(service_names): if service.can_be_built(): - service.build(no_cache, pull, force_rm) + service.build(no_cache, pull, force_rm, build_args) else: log.info('%s uses an image, skipping' % service.name) diff --git a/compose/service.py b/compose/service.py index 712d5ac15..a889dd58c 100644 --- a/compose/service.py +++ b/compose/service.py @@ -21,6 +21,7 @@ from . import __version__ from . import const from . import progress_stream from .config import DOCKER_CONFIG_KEYS +from .config import merge_build_args from .config import merge_environment from .config.types import ServicePort from .config.types import VolumeSpec @@ -803,13 +804,18 @@ class Service(object): return [build_spec(secret) for secret in self.secrets] - def build(self, no_cache=False, pull=False, force_rm=False): + def build(self, no_cache=False, pull=False, force_rm=False, build_args=None): log.info('Building %s' % self.name) build_opts = self.options.get('build', {}) - path = build_opts.get('context') + + self_args_opts = build_opts.get('args', None) + if self_args_opts and build_args: + merge_build_args(self_args_opts, build_args, self.options.get('environment')) + # python2 os.stat() doesn't support unicode on some UNIX, so we # encode it to a bytestring to be safe + path = build_opts.get('context') if not six.PY3 and not IS_WINDOWS_PLATFORM: path = path.encode('utf8') @@ -822,8 +828,8 @@ class Service(object): pull=pull, nocache=no_cache, dockerfile=build_opts.get('dockerfile', None), - buildargs=build_opts.get('args', None), cache_from=build_opts.get('cache_from', None), + buildargs=self_args_opts ) try: diff --git a/tests/unit/config/config_test.py b/tests/unit/config/config_test.py index d016ae4e2..3e3bd2bbf 100644 --- a/tests/unit/config/config_test.py +++ b/tests/unit/config/config_test.py @@ -15,6 +15,7 @@ import yaml from ...helpers import build_config_details from compose.config import config from compose.config import types +from compose.config.config import merge_build_args from compose.config.config import resolve_build_args from compose.config.config import resolve_environment from compose.config.config import V1 @@ -2881,6 +2882,24 @@ class EnvTest(unittest.TestCase): {'arg1': 'value1', 'empty_arg': '', 'env_arg': 'value2', 'no_env': None}, ) + @mock.patch.dict(os.environ) + def test_merge_build_args(self): + os.environ['env_arg'] = 'value2' + + base = { + 'arg1': 'arg1_value', + 'arg2': 'arg2_value' + } + override = { + 'arg1': 'arg1_new_value', + 'arg2': 'arg2_value' + } + self.assertEqual(base['arg1'], 'arg1_value') + + merge_build_args(base, override, os.environ) + + self.assertEqual(base['arg1'], 'arg1_new_value') + @pytest.mark.xfail(IS_WINDOWS_PLATFORM, reason='paths use slash') @mock.patch.dict(os.environ) def test_resolve_path(self): diff --git a/tests/unit/service_test.py b/tests/unit/service_test.py index 5dc5265e6..ca7410414 100644 --- a/tests/unit/service_test.py +++ b/tests/unit/service_test.py @@ -513,6 +513,25 @@ class ServiceTest(unittest.TestCase): self.assertEqual(self.mock_client.build.call_count, 1) self.assertFalse(self.mock_client.build.call_args[1]['pull']) + def test_build_with_override_build_args(self): + self.mock_client.build.return_value = [ + b'{"stream": "Successfully built 12345"}', + ] + + build_args = [ + 'arg1=arg1_new_value', + 'arg2=arg2_value' + ] + service = Service('foo', client=self.mock_client, + build={'context': '.', 'args': {'arg1': 'arg1', 'arg2': 'arg2'}}) + service.build(build_args=build_args) + + called_build_args = self.mock_client.build.call_args[1]['buildargs'] + + for arg in called_build_args: + if "arg1=" in arg: + self.assertEquals(arg, 'arg1=arg1_new_value') + def test_config_dict(self): self.mock_client.inspect_image.return_value = {'Id': 'abcd'} service = Service( From d5a2d37d059cd1de5865fc5eaa05461d94aadfff Mon Sep 17 00:00:00 2001 From: Joffrey F Date: Tue, 7 Mar 2017 17:47:41 -0800 Subject: [PATCH 2/2] Properly resolve build args against host environment values Signed-off-by: Joffrey F --- compose/cli/main.py | 4 +++ compose/config/__init__.py | 2 +- compose/config/config.py | 12 ++----- compose/service.py | 11 +++--- tests/integration/service_test.py | 20 ++++++++++- tests/unit/config/config_test.py | 58 ++----------------------------- tests/unit/service_test.py | 18 +++++----- 7 files changed, 43 insertions(+), 82 deletions(-) diff --git a/compose/cli/main.py b/compose/cli/main.py index 78e3d84b5..4d31e25e8 100644 --- a/compose/cli/main.py +++ b/compose/cli/main.py @@ -22,6 +22,7 @@ from ..bundle import MissingDigests from ..bundle import serialize_bundle from ..config import ConfigurationError from ..config import parse_environment +from ..config import resolve_build_args from ..config.environment import Environment from ..config.serialize import serialize_config from ..config.types import VolumeSpec @@ -219,6 +220,9 @@ class TopLevelCommand(object): """ service_names = options['SERVICE'] build_args = options.get('--build-arg', None) + if build_args: + environment = Environment.from_env_file(self.project_dir) + build_args = resolve_build_args(build_args, environment) if not service_names and build_args: raise UserError("Need service name for --build-arg option") diff --git a/compose/config/__init__.py b/compose/config/__init__.py index b6e5e8d38..b629edf66 100644 --- a/compose/config/__init__.py +++ b/compose/config/__init__.py @@ -7,6 +7,6 @@ from .config import ConfigurationError from .config import DOCKER_CONFIG_KEYS from .config import find from .config import load -from .config import merge_build_args from .config import merge_environment from .config import parse_environment +from .config import resolve_build_args diff --git a/compose/config/config.py b/compose/config/config.py index 718d3bf02..c85ffdabb 100644 --- a/compose/config/config.py +++ b/compose/config/config.py @@ -602,14 +602,8 @@ def resolve_environment(service_dict, environment=None): return dict(resolve_env_var(k, v, environment) for k, v in six.iteritems(env)) -def merge_build_args(base, override, environment): - override_args = parse_build_arguments(override) - override_dict = dict(resolve_env_var(k, v, environment) for k, v in six.iteritems(override_args)) - base.update(override_dict) - - -def resolve_build_args(build, environment): - args = parse_build_arguments(build.get('args')) +def resolve_build_args(buildargs, environment): + args = parse_build_arguments(buildargs) return dict(resolve_env_var(k, v, environment) for k, v in six.iteritems(args)) @@ -1057,7 +1051,7 @@ def normalize_build(service_dict, working_dir, environment): build.update(service_dict['build']) if 'args' in build: build['args'] = build_string_dict( - resolve_build_args(build, environment) + resolve_build_args(build.get('args'), environment) ) service_dict['build'] = build diff --git a/compose/service.py b/compose/service.py index a889dd58c..29ee70476 100644 --- a/compose/service.py +++ b/compose/service.py @@ -21,7 +21,6 @@ from . import __version__ from . import const from . import progress_stream from .config import DOCKER_CONFIG_KEYS -from .config import merge_build_args from .config import merge_environment from .config.types import ServicePort from .config.types import VolumeSpec @@ -804,14 +803,14 @@ class Service(object): return [build_spec(secret) for secret in self.secrets] - def build(self, no_cache=False, pull=False, force_rm=False, build_args=None): + def build(self, no_cache=False, pull=False, force_rm=False, build_args_override=None): log.info('Building %s' % self.name) build_opts = self.options.get('build', {}) - self_args_opts = build_opts.get('args', None) - if self_args_opts and build_args: - merge_build_args(self_args_opts, build_args, self.options.get('environment')) + build_args = build_opts.get('args', {}).copy() + if build_args_override: + build_args.update(build_args_override) # python2 os.stat() doesn't support unicode on some UNIX, so we # encode it to a bytestring to be safe @@ -829,7 +828,7 @@ class Service(object): nocache=no_cache, dockerfile=build_opts.get('dockerfile', None), cache_from=build_opts.get('cache_from', None), - buildargs=self_args_opts + buildargs=build_args ) try: diff --git a/tests/integration/service_test.py b/tests/integration/service_test.py index ddc2f3bae..12ec8a993 100644 --- a/tests/integration/service_test.py +++ b/tests/integration/service_test.py @@ -597,12 +597,30 @@ class ServiceTest(DockerClientTestCase): with open(os.path.join(base_dir, 'Dockerfile'), 'w') as f: f.write("FROM busybox\n") f.write("ARG build_version\n") + f.write("RUN echo ${build_version}\n") service = self.create_service('buildwithargs', build={'context': text_type(base_dir), 'args': {"build_version": "1"}}) service.build() assert service.image() + assert "build_version=1" in service.image()['ContainerConfig']['Cmd'] + + def test_build_with_build_args_override(self): + base_dir = tempfile.mkdtemp() + self.addCleanup(shutil.rmtree, base_dir) + + with open(os.path.join(base_dir, 'Dockerfile'), 'w') as f: + f.write("FROM busybox\n") + f.write("ARG build_version\n") + f.write("RUN echo ${build_version}\n") + + service = self.create_service('buildwithargs', + build={'context': text_type(base_dir), + 'args': {"build_version": "1"}}) + service.build(build_args_override={'build_version': '2'}) + assert service.image() + assert "build_version=2" in service.image()['ContainerConfig']['Cmd'] def test_start_container_stays_unprivileged(self): service = self.create_service('web') @@ -1057,7 +1075,7 @@ class ServiceTest(DockerClientTestCase): one_off_container = service.create_container(one_off=True) self.assertNotEqual(one_off_container.name, 'my-web-container') - @pytest.mark.skipif(True, reason="Broken on 1.11.0rc1") + @pytest.mark.skipif(True, reason="Broken on 1.11.0 - 17.03.0") def test_log_drive_invalid(self): service = self.create_service('web', logging={'driver': 'xxx'}) expected_error_msg = "logger: no log driver named 'xxx' is registered" diff --git a/tests/unit/config/config_test.py b/tests/unit/config/config_test.py index 3e3bd2bbf..d7d342afa 100644 --- a/tests/unit/config/config_test.py +++ b/tests/unit/config/config_test.py @@ -15,7 +15,6 @@ import yaml from ...helpers import build_config_details from compose.config import config from compose.config import types -from compose.config.config import merge_build_args from compose.config.config import resolve_build_args from compose.config.config import resolve_environment from compose.config.config import V1 @@ -1523,7 +1522,7 @@ class ConfigTest(unittest.TestCase): assert actual == { 'image': 'alpine:edge', 'volumes': ['.:/app'], - 'ports': ['5432'] + 'ports': types.ServicePort.parse('5432') } def test_merge_service_dicts_heterogeneous_2(self): @@ -1542,40 +1541,7 @@ class ConfigTest(unittest.TestCase): assert actual == { 'image': 'alpine:edge', 'volumes': ['.:/app'], - 'ports': ['5432'] - } - - def test_merge_build_args(self): - base = { - 'build': { - 'context': '.', - 'args': { - 'ONE': '1', - 'TWO': '2', - }, - } - } - override = { - 'build': { - 'args': { - 'TWO': 'dos', - 'THREE': '3', - }, - } - } - actual = config.merge_service_dicts( - base, - override, - DEFAULT_VERSION) - assert actual == { - 'build': { - 'context': '.', - 'args': { - 'ONE': '1', - 'TWO': 'dos', - 'THREE': '3', - }, - } + 'ports': types.ServicePort.parse('5432') } def test_merge_logging_v1(self): @@ -2878,28 +2844,10 @@ class EnvTest(unittest.TestCase): } } self.assertEqual( - resolve_build_args(build, Environment.from_env_file(build['context'])), + resolve_build_args(build['args'], Environment.from_env_file(build['context'])), {'arg1': 'value1', 'empty_arg': '', 'env_arg': 'value2', 'no_env': None}, ) - @mock.patch.dict(os.environ) - def test_merge_build_args(self): - os.environ['env_arg'] = 'value2' - - base = { - 'arg1': 'arg1_value', - 'arg2': 'arg2_value' - } - override = { - 'arg1': 'arg1_new_value', - 'arg2': 'arg2_value' - } - self.assertEqual(base['arg1'], 'arg1_value') - - merge_build_args(base, override, os.environ) - - self.assertEqual(base['arg1'], 'arg1_new_value') - @pytest.mark.xfail(IS_WINDOWS_PLATFORM, reason='paths use slash') @mock.patch.dict(os.environ) def test_resolve_path(self): diff --git a/tests/unit/service_test.py b/tests/unit/service_test.py index ca7410414..b3c8c4d7d 100644 --- a/tests/unit/service_test.py +++ b/tests/unit/service_test.py @@ -461,7 +461,7 @@ class ServiceTest(unittest.TestCase): forcerm=False, nocache=False, rm=True, - buildargs=None, + buildargs={}, cache_from=None, ) @@ -498,7 +498,7 @@ class ServiceTest(unittest.TestCase): forcerm=False, nocache=False, rm=True, - buildargs=None, + buildargs={}, cache_from=None, ) @@ -518,19 +518,17 @@ class ServiceTest(unittest.TestCase): b'{"stream": "Successfully built 12345"}', ] - build_args = [ - 'arg1=arg1_new_value', - 'arg2=arg2_value' - ] + build_args = { + 'arg1': 'arg1_new_value', + } service = Service('foo', client=self.mock_client, build={'context': '.', 'args': {'arg1': 'arg1', 'arg2': 'arg2'}}) - service.build(build_args=build_args) + service.build(build_args_override=build_args) called_build_args = self.mock_client.build.call_args[1]['buildargs'] - for arg in called_build_args: - if "arg1=" in arg: - self.assertEquals(arg, 'arg1=arg1_new_value') + assert called_build_args['arg1'] == build_args['arg1'] + assert called_build_args['arg2'] == 'arg2' def test_config_dict(self): self.mock_client.inspect_image.return_value = {'Id': 'abcd'}