Remove deps

[krancour]

Closes #66

[martinmaly]

is it possible that this line may terminate the bash pipeline? or will Docker do the right thing and drop this line and execute the pipeline in its entirety?

[krancour]

If any one piece of that fails, the whole RUN directive fails and the Docker build fails in turn, which is what we'd want.

[duglin]

I think he means the comment - yes it will terminate the RUN - this was recently "fixed" in Docker to act this way. We should remove the comment.

[krancour]

Ah... I'm sorry. I didn't understand that he meant the comment.

We (Deis) have used comments like this without any ill-effect.

@duglin can you elaborate on the "fix"? Are you saying that this was fine in older versions of Docker but not in a newer or upcoming version?

[krancour]

[kent@mbp k8s]$ docker -v
Docker version 1.12.2, build bb80604

The output shows everything ran as expected:

[kent@mbp k8s]$ make docker
Building k8s-broker for linux
Building Docker
Sending build context to Docker daemon 68.63 MB
Step 1 : FROM debian
 ---> 73e72bf822ca
Step 2 : ENV HELM_VERSION v2.0.0-rc.2
 ---> Using cache
 ---> f1e5608a0de5
Step 3 : RUN export DEBIAN_FRONTEND=noninteractive  && apt-get update  && apt-get install -y     ca-certificates     curl  && curl -Ls -o /tmp/helm-${HELM_VERSION}-linux-amd64.tar.gz     http://storage.googleapis.com/kubernetes-helm/helm-${HELM_VERSION}-linux-amd64.tar.gz  && mkdir -p /opt/services  && tar -C /opt/services -x -z -f /tmp/helm-${HELM_VERSION}-linux-amd64.tar.gz     --strip-components=1 linux-amd64/helm  && rm -f /tmp/helm-${HELM_VERSION}-linux-amd64.tar.gz  && apt-get purge -y --auto-remove curl  && apt-get clean  && rm -rf /var/lib/apt/lists/*
 ---> Using cache
 ---> 5938208989cc
Step 4 : ENTRYPOINT /opt/services/k8s-broker --helm_binary /opt/services/helm
 ---> Using cache
 ---> 6751815d501c
Successfully built 6751815d501c

Also note that if the comment were problematic, the Dockerfile would fail to compile because Docker would expect a directive on line 30 and would find some bash nonsense instead.

So unless this behavior has really, truly changed in a recent version of Docker, there is no problem here.

[duglin]

Yes, in the latest docker, this Dockerfile will fail:

FROM ubuntu
RUN echo hello \
# comment
world

because "WORLD" is an unknown command. Now, it gets interesting if the line after the comment just happened to start with a Dockerfile cmd/word :-) but that's a different issue.

But the net is that we should remove the # cleanup line because eventually when a new Docker is released it'll break us.

[martinmaly]

@duglin thanks for your help clarifying! the code review tool I use most of the time pinpoints location of feedback exactly so I forget to be as specific on GitHub. Doug was exactly right about what I was referring to.

[krancour]

@duglin thanks for the info. I'll remove the comment in question.

[krancour]

The controversial # cleanup comment has been removed the contrib/broker/k8s/Dockerfile. This is read for re-review.

[martinmaly]

LGTM

[duglin]

LGTM

[arschles]

LGTM

[arschles]

waiting for CI, then I'll merge