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 build args will not passed to docker engine if they are equal to 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 ad2c93b commit 24ed5d5
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 3 deletions.
8 changes: 7 additions & 1 deletion compose/service.py
Original file line number Diff line number Diff line change
Expand Up @@ -701,6 +701,12 @@ 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 @@ -715,7 +721,7 @@ def build(self, no_cache=False, pull=False, force_rm=False):
pull=pull,
nocache=no_cache,
dockerfile=build_opts.get('dockerfile', None),
buildargs=build_opts.get('args', None),
buildargs=buildargs,
)

try:
Expand Down
30 changes: 28 additions & 2 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=None,
buildargs={},
)

def test_ensure_image_exists_no_build(self):
Expand Down Expand Up @@ -481,7 +481,33 @@ def test_ensure_image_exists_force_build(self):
forcerm=False,
nocache=False,
rm=True,
buildargs=None,
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'},
)

def test_build_does_not_pull(self):
Expand Down

0 comments on commit 24ed5d5

Please sign in to comment.