Unexpected result when using build args with default values

[thomas-riccardi]

Scenario:

Files

Dockerfile:

FROM ubuntu:14.04

ARG FOO=1
RUN echo "-${FOO}-"

CMD /bin/bash

docker-compose.yml:

version: '2'

services:
  test:
    build:
      context: .
      args:
        - FOO

Execution:

$ ./docker-compose-1.6.2 --verbose config
compose.config.config.find: Using configuration files: ./docker-compose.yml
networks: {}
services:
  test:
    build:
      args:
        FOO: None
      context: /home/riccardi/git/ses-docker/test-default-build-arg
version: '2.0'
volumes: {}

$ ./docker-compose-1.6.2 --verbose build
compose.config.config.find: Using configuration files: ./docker-compose.yml
docker.auth.auth.load_config: File doesn't exist
compose.cli.command.get_client: docker-compose version 1.6.2, build 4d72027
docker-py version: 1.7.2
CPython version: 2.7.9
OpenSSL version: OpenSSL 1.0.1e 11 Feb 2013
compose.cli.command.get_client: Docker base_url: http+docker://localunixsocket
compose.cli.command.get_client: Docker version: KernelVersion=4.2.0-35-generic, Os=linux, BuildTime=2016-03-10T15:54:52.312835708+00:00, ApiVersion=1.22, Version=1.10.3, GitCommit=20f81dd, Arch=amd64, GoVersion=go1.5.3
compose.service.build: Building test
compose.cli.verbose_proxy.proxy_callable: docker build <- (pull=False, stream=True, nocache=False, tag=u'testdefaultbuildarg_test', buildargs={u'FOO': 'None'}, rm=True, forcerm=False, path='/home/riccardi/git/ses-docker/test-default-build-arg', dockerfile=None)
docker.api.build._set_auth_headers: Looking for auth config
docker.api.build._set_auth_headers: No auth config in memory - loading from filesystem
docker.auth.auth.load_config: File doesn't exist
docker.api.build._set_auth_headers: No auth config found
compose.cli.verbose_proxy.proxy_callable: docker build -> <generator object _stream_helper at 0x7f56bafb3a50>
Step 1 : FROM ubuntu:14.04
 ---> b549a9959a66
Step 2 : ARG FOO=1
 ---> Using cache
 ---> 4774113d6ec5
Step 3 : RUN echo "-${FOO}-"
 ---> Running in dabd31837074
-None-
 ---> f8a99349af3b
Removing intermediate container dabd31837074
Step 4 : CMD /bin/bash
 ---> Running in 487f5e789c38
 ---> 6c484f426fb5
Removing intermediate container 487f5e789c38
Successfully built 6c484f426fb5
compose.cli.verbose_proxy.proxy_callable: docker close <- ()
compose.cli.verbose_proxy.proxy_callable: docker close -> None

( same result with 1.7.1-rc1 which includes PR #2938 )

Issue

Expected result:

prints -1-.

Actual result:

prints -None-.

Details:

Compose has no value for FOO build arg from its environment, so it could either send an empty string to docker build, or better: not send this build arg to docker build.
The second one would be great: it would open the possibility to use the default value for the build arg as defined in the Dockerfile. ( For now the workaround is to duplicate the default values from Dockerfile to .env, only works with >=1.7.0).
The first one would be still better than the current behavior.

Current behavior: no value in Compose environment is represented in python as None, then casted to a string "None", which is probably always bad.

[dnephin]

I guess if the value is unset we should omit it from the args sent to build.

[thomas-riccardi]

@dnephin exactly, that would be my preferred solution. This way, default values defined in the Dockerfile could apply, avoiding duplication of default values.

This could be seen as a new feature though. The intermediate step, if needed, would be to just send an empty string instead of the "None" string.

[carletes]

This could be seen as a new feature though. The intermediate step, if needed, would be to just send an empty string instead of the None string.

Yep, that would also be helpful for building images behind HTTP proxies.

I have the following docker-compose.yml file:

version: "2"

services:
  hello:
    build:
      context: .
      args:
        - http_proxy
    command: sh

and the following Dockerfile:

FROM alpine

RUN set -x && env

When I run docker-compose build hello without the environment variable http_proxy, I get None:

$ docker-compose build --no-cache hello
Building hello
Step 1 : FROM alpine
 ---> 13e1761bf172
Step 2 : RUN set -x && env
 ---> Running in 0ec8d98bf685
HOSTNAME=27c9668b3d5e
SHLVL=1
HOME=/root
http_proxy=None
PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
PWD=/
+ env
 ---> 212f8c97b934
Removing intermediate container 0ec8d98bf685
Successfully built 212f8c97b934
$

What I would expect is, in fact, the same behaviour as when http_proxy is defined to be empty:

$ env http_proxy= docker-compose build --no-cache hello
Building hello
Step 1 : FROM alpine
 ---> 13e1761bf172
Step 2 : RUN set -x && env
 ---> Running in 6bfec420e409
HOSTNAME=27c9668b3d5e
SHLVL=1
HOME=/root
http_proxy=
PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
PWD=/
+ env
 ---> c5bf6c0e0190
Removing intermediate container 6bfec420e409
Successfully built c5bf6c0e0190
(docker-compose)carlos@elouard:~/x$ 

If that were the case, my docker-compose.yml and Dockerfile would work both at $WORK, where I have an HTTP proxy in front of me, and at home, where I do not have such constraint.

[thomas-riccardi]

@aanand what about FOO="None" ?
The fix at e3e8a61 seems to be done too late: after the undefined environment variable is casted to string, it should probably be done before this cast, so the information is already lost, and explicit "FOO=None" will probably not be forwarded to docker build.
In some cases "None" is a valid string value.

[aanand]

@triccardi-systran You're looking at the wrong commit - a67ba55 is the actual fix.

[thomas-riccardi]

@aanand my bad, the commits listed on this page by github are misleading: PR #3449 is missing, but early commits from this PR are (that's where I found e3e8a61).

I confirm this works: now undefined build args are passed to docker build as empty strings.

However, this is the proposed behavior 1. The proposed behavior 2 would be better IMO, as initially explained. Should I open a new issue for that ?
Alas, compose 1.8.0 will implement behavior 1, and changing later to behavior 2 would be a breaking change, thus reducing the probability to be included soon. Releasing behavior 2 directly from compose 1.7.0 state would avoid any breakage (because the current state is already broken).

[aanand]

It's not too late - we're not committed to the current behaviour until the stable 1.8.0 release. Feel free to open a new issue so we can discuss which behaviour would be ideal.

[thomas-riccardi]

@aanand I opened a new issue as you suggested, it was 2 weeks ago, I'm starting to worry about compose 1.8.0 being released with this issue not fully resolved.