Skip to content

Commit

Permalink
Fix docker#3281: Unexpected result when using build args with default…
Browse files Browse the repository at this point in the history
… values

Fix the issue when build arg is set to None instead of empty string. Usecase:

cat docker-compose.yml
.... args:
- http_proxy
- https_proxy
- no_proxy

If http_proxy, https_proxy, no_proxy environment variables are not defined then http_proxy,
https_proxy, no_proxy build args will be set to string None which breaks all downloads

With this change undefined build args will be set to empty string instead of string None

Signed-off-by: Andrey Devyatkin <andrey.a.devyatkin@gmail.com>
  • Loading branch information
Andrey9kin authored and rawkode committed Oct 18, 2016
1 parent 0d251cd commit 24b381a
Show file tree
Hide file tree
Showing 4 changed files with 5 additions and 37 deletions.
8 changes: 1 addition & 7 deletions compose/service.py
Original file line number Diff line number Diff line change
Expand Up @@ -701,12 +701,6 @@ def build(self, no_cache=False, pull=False, force_rm=False):

build_opts = self.options.get('build', {})
path = build_opts.get('context')
# If build argument is not defined and there is no environment variable
# with the same name then build argument value will be None
# Moreover it will be sent to the docker engine as None and then
# interpreted as string None which in many cases will fail the build
# That is why we filter out all pairs with value equal to None
buildargs = {k: v for k, v in build_opts.get('args', {}).items() if v != 'None'}
# python2 os.path() doesn't support unicode, so we need to encode it to
# a byte string
if not six.PY3:
Expand All @@ -721,7 +715,7 @@ def build(self, no_cache=False, pull=False, force_rm=False):
pull=pull,
nocache=no_cache,
dockerfile=build_opts.get('dockerfile', None),
buildargs=buildargs,
buildargs=build_opts.get('args', None),
)

try:
Expand Down
2 changes: 1 addition & 1 deletion compose/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -95,4 +95,4 @@ def microseconds_from_time_nano(time_nano):


def build_string_dict(source_dict):
return dict((k, str(v)) for k, v in source_dict.items())
return dict((k, str(v if v else '')) for k, v in source_dict.items())
2 changes: 1 addition & 1 deletion tests/unit/config/config_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -715,7 +715,7 @@ def test_build_args_allow_empty_properties(self):
).services[0]
assert 'args' in service['build']
assert 'foo' in service['build']['args']
assert service['build']['args']['foo'] == 'None'
assert service['build']['args']['foo'] == ''

def test_load_with_multiple_files_mismatched_networks_format(self):
base_file = config.ConfigFile(
Expand Down
30 changes: 2 additions & 28 deletions tests/unit/service_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -445,7 +445,7 @@ def test_create_container(self):
forcerm=False,
nocache=False,
rm=True,
buildargs={},
buildargs=None,
)

def test_ensure_image_exists_no_build(self):
Expand Down Expand Up @@ -481,33 +481,7 @@ def test_ensure_image_exists_force_build(self):
forcerm=False,
nocache=False,
rm=True,
buildargs={},
)

def test_ensure_filter_out_empty_build_args(self):
args = {u'no_proxy': 'None', u'https_proxy': 'something'}
service = Service('foo',
client=self.mock_client,
build={'context': '.', 'args': args})
self.mock_client.inspect_image.return_value = {'Id': 'abc123'}
self.mock_client.build.return_value = [
'{"stream": "Successfully built abcd"}',
]

with mock.patch('compose.service.log', autospec=True) as mock_log:
service.ensure_image_exists(do_build=BuildAction.force)

assert not mock_log.warn.called
self.mock_client.build.assert_called_once_with(
tag='default_foo',
dockerfile=None,
stream=True,
path='.',
pull=False,
forcerm=False,
nocache=False,
rm=True,
buildargs={u'https_proxy': 'something'},
buildargs=None,
)

def test_build_does_not_pull(self):
Expand Down

0 comments on commit 24b381a

Please sign in to comment.