Create a set of Dart dockerfiles

Create a set of Dart dockerfiles

These dockerfles are designed to follow the same patters as the ones for
nodejs, Python and Go at

  https://github.com/GoogleCloudPlatform/nodejs-docker
  https://github.com/GoogleCloudPlatform/python-docker
  https://github.com/GoogleCloudPlatform/golang-docker

R=kevmoo@google.com, kustermann@google.com, proppy@google.com
BUG=

Committed: https://github.com/dart-lang/dart_docker/commit/6b06e98

[Søren Gjesse, 2014-07-02 14:22:44]

I will push the build images to the docker.io repository as well.

[proppy, 2014-07-02 20:55:56]

https://codereview.chromium.org/367903008/diff/1/base/Dockerfile
File base/Dockerfile (right):

https://codereview.chromium.org/367903008/diff/1/base/Dockerfile#newcode4
base/Dockerfile:4: RUN wget -q
http://storage.googleapis.com/dart-archive/channels/stable/release/37644/linu...
&& dpkg -i dart_1.5.1-1_amd64.deb && rm dart_1.5.1-1_amd64.deb
is pub also included?

https://codereview.chromium.org/367903008/diff/1/base/README.md
File base/README.md (right):

https://codereview.chromium.org/367903008/diff/1/base/README.md#newcode23
base/README.md:23: ENV PUB_CACHE /var/cache/pub
should we add that to the Dockerfile instead?

https://codereview.chromium.org/367903008/diff/1/base/README.md#newcode24
base/README.md:24: RUN /usr/lib/dart/bin/pub get
is pub in the PATH?

https://codereview.chromium.org/367903008/diff/1/base/README.md#newcode26
base/README.md:26: RUN /usr/lib/dart/bin/pub get
Made some comment below, I'm not sure if this is necessary.

https://codereview.chromium.org/367903008/diff/1/base/README.md#newcode27
base/README.md:27: ADD . /app
Do you need to do that twice?

https://codereview.chromium.org/367903008/diff/1/base/README.md#newcode48
base/README.md:48: When a Docker image is build symbolic links are not folowed.
This means that
followed

https://codereview.chromium.org/367903008/diff/1/base/README.md#newcode49
base/README.md:49: when the `package` directory is added it will contain
sym-links to the host
We don't have this issue if the packages directory is not part of the build
context, maybe we could add it to .dockerignore.

https://github.com/dotcloud/docker/pull/6579

https://codereview.chromium.org/367903008/diff/1/build.sh
File build.sh (right):

https://codereview.chromium.org/367903008/diff/1/build.sh#newcode2
build.sh:2: # Build the Docker images for Dart.
not sure we need this script, this could be taken care of my docker.io automated
build.

https://codereview.chromium.org/367903008/diff/1/hello/README.md
File hello/README.md (right):

https://codereview.chromium.org/367903008/diff/1/hello/README.md#newcode4
hello/README.md:4: [docker](https://docker.io) image for a simple Dart HTTP
server.
+serving an hello world page

https://codereview.chromium.org/367903008/diff/1/hello/bin/server.dart
File hello/bin/server.dart (right):

https://codereview.chromium.org/367903008/diff/1/hello/bin/server.dart#newcode13
hello/bin/server.dart:13: request.response.redirect('/index.html');
can you add some basic server side templating

https://codereview.chromium.org/367903008/diff/1/runtime/Dockerfile
File runtime/Dockerfile (right):

https://codereview.chromium.org/367903008/diff/1/runtime/Dockerfile#newcode6
runtime/Dockerfile:6: ONBUILD ENV PUB_CACHE /var/cache/pub
can we move that to the google/dart image?

https://codereview.chromium.org/367903008/diff/1/runtime/Dockerfile#newcode9
runtime/Dockerfile:9: ONBUILD RUN /usr/lib/dart/bin/pub get
maybe we can get rid of that if we suggest adding a .dockerignore file with
packages in the application directory.

https://codereview.chromium.org/367903008/diff/1/runtime/README.md
File runtime/README.md (right):

https://codereview.chromium.org/367903008/diff/1/runtime/README.md#newcode3
runtime/README.md:3:
[`google/dart-runtime`](https://index.docker.io/u/google/dart-runtime)
can you update the description to match the python image, I think it is simpler:
https://github.com/GoogleCloudPlatform/python-docker/tree/master/runtime

https://codereview.chromium.org/367903008/diff/1/runtime/README.md#newcode22
runtime/README.md:22: docker build -t my/app .
I would remove the my/ prefix (it is not valid)

https://codereview.chromium.org/367903008/diff/1/runtime/README.md#newcode27
runtime/README.md:27: docker run -d -p 8080:8080 my/app
I would remove the my/ prefix (it is not valid)

https://codereview.chromium.org/367903008/diff/1/runtime/README.md#newcode40
runtime/README.md:40: - has a file named `bin/server.dart` as the entrypoint
script.
is that standard to have a bin subdirectory in a dart source repo?

[kustermann, 2014-07-03 09:40:05]

I'm not sure if "dart-runtime" is the correct name. If we assume bin/server.dart
and that it listens on 8080 we might as well call it "dart-server"?

https://codereview.chromium.org/367903008/diff/1/base/Dockerfile
File base/Dockerfile (right):

https://codereview.chromium.org/367903008/diff/1/base/Dockerfile#newcode4
base/Dockerfile:4: RUN wget -q
http://storage.googleapis.com/dart-archive/channels/stable/release/37644/linu...
&& dpkg -i dart_1.5.1-1_amd64.deb && rm dart_1.5.1-1_amd64.deb
On 2014/07/02 20:55:55, proppy wrote:
> is pub also included?

Yes, the dart sdk contains all sdk components: DartVM, dartanalyzer, dart2js
compiler and pub.

https://codereview.chromium.org/367903008/diff/1/base/README.md
File base/README.md (right):

https://codereview.chromium.org/367903008/diff/1/base/README.md#newcode23
base/README.md:23: ENV PUB_CACHE /var/cache/pub
On 2014/07/02 20:55:55, proppy wrote:
> should we add that to the Dockerfile instead?

I think we can remove that line entirely.

https://codereview.chromium.org/367903008/diff/1/base/README.md#newcode24
base/README.md:24: RUN /usr/lib/dart/bin/pub get
On 2014/07/02 20:55:55, proppy wrote:
> is pub in the PATH?

Soeren said there is only /usr/bin/dart symlink to /usr/lib/dart/bin/dart. The
other tools in the sdk are not symlinked to not pollute /usr/bin. 

We discussed adding /usr/lib/dart/bin to PATH?!

https://codereview.chromium.org/367903008/diff/1/base/README.md#newcode27
base/README.md:27: ADD . /app
On 2014/07/02 20:55:55, proppy wrote:
> Do you need to do that twice?

I don't think so.

https://codereview.chromium.org/367903008/diff/1/hello/README.md
File hello/README.md (right):

https://codereview.chromium.org/367903008/diff/1/hello/README.md#newcode14
hello/README.md:14: docker run -d -p 8080:8080 google/nodejs-hello
nodejs -> dart

https://codereview.chromium.org/367903008/diff/1/hello/bin/server.dart
File hello/bin/server.dart (right):

https://codereview.chromium.org/367903008/diff/1/hello/bin/server.dart#newcode28
hello/bin/server.dart:28: onError: (e, stackTrace) => print('Error processing
request $e\n$stackTrace'));
long line.

https://codereview.chromium.org/367903008/diff/1/runtime/Dockerfile
File runtime/Dockerfile (right):

https://codereview.chromium.org/367903008/diff/1/runtime/Dockerfile#newcode9
runtime/Dockerfile:9: ONBUILD RUN /usr/lib/dart/bin/pub get
On 2014/07/02 20:55:55, proppy wrote:
> maybe we can get rid of that if we suggest adding a .dockerignore file with
> packages in the application directory.

I think Soeren did this in order to cause less docker images, since only the
"ADD . /app" will usually create a different image ID (the previous steps should
always result in the same image ID). i.e. it might be a space saving technique.

[Søren Gjesse, 2014-07-03 10:36:41]

PTAL

https://codereview.chromium.org/367903008/diff/1/base/Dockerfile
File base/Dockerfile (right):

https://codereview.chromium.org/367903008/diff/1/base/Dockerfile#newcode4
base/Dockerfile:4: RUN wget -q
http://storage.googleapis.com/dart-archive/channels/stable/release/37644/linu...
&& dpkg -i dart_1.5.1-1_amd64.deb && rm dart_1.5.1-1_amd64.deb
On 2014/07/02 20:55:55, proppy wrote:
> is pub also included?

Yes, pub is part of the Dart SDK.

https://codereview.chromium.org/367903008/diff/1/base/README.md
File base/README.md (right):

https://codereview.chromium.org/367903008/diff/1/base/README.md#newcode23
base/README.md:23: ENV PUB_CACHE /var/cache/pub
On 2014/07/02 20:55:55, proppy wrote:
> should we add that to the Dockerfile instead?

Maybe this is not needed after all.

https://codereview.chromium.org/367903008/diff/1/base/README.md#newcode23
base/README.md:23: ENV PUB_CACHE /var/cache/pub
On 2014/07/03 09:40:05, kustermann wrote:
> On 2014/07/02 20:55:55, proppy wrote:
> > should we add that to the Dockerfile instead?
> 
> I think we can remove that line entirely.

Yes we can - the default pub cache location is fine. The user inside the
container is root and $HOME is /, so the default pub cache is  /.pub-cache

https://codereview.chromium.org/367903008/diff/1/base/README.md#newcode24
base/README.md:24: RUN /usr/lib/dart/bin/pub get
On 2014/07/02 20:55:55, proppy wrote:
> is pub in the PATH?

Not on the default path.

The .deb-file we have installs the Dart SDK into /usr/lib/dart, and creates a
link from /usr/bin/dart to /usr/lib/dart/bin/dart.

https://codereview.chromium.org/367903008/diff/1/base/README.md#newcode24
base/README.md:24: RUN /usr/lib/dart/bin/pub get
On 2014/07/03 09:40:05, kustermann wrote:
> On 2014/07/02 20:55:55, proppy wrote:
> > is pub in the PATH?
> 
> Soeren said there is only /usr/bin/dart symlink to /usr/lib/dart/bin/dart. The
> other tools in the sdk are not symlinked to not pollute /usr/bin. 
> 
> We discussed adding /usr/lib/dart/bin to PATH?!

Added /usr/lib/dart/bin to PATH in the base image.

https://codereview.chromium.org/367903008/diff/1/base/README.md#newcode26
base/README.md:26: RUN /usr/lib/dart/bin/pub get
On 2014/07/02 20:55:55, proppy wrote:
> Made some comment below, I'm not sure if this is necessary.

See below.

https://codereview.chromium.org/367903008/diff/1/base/README.md#newcode27
base/README.md:27: ADD . /app
On 2014/07/02 20:55:55, proppy wrote:
> Do you need to do that twice?

No, that is a mistake.

https://codereview.chromium.org/367903008/diff/1/base/README.md#newcode27
base/README.md:27: ADD . /app
On 2014/07/03 09:40:05, kustermann wrote:
> On 2014/07/02 20:55:55, proppy wrote:
> > Do you need to do that twice?
> 
> I don't think so.

Removed.

https://codereview.chromium.org/367903008/diff/1/base/README.md#newcode48
base/README.md:48: When a Docker image is build symbolic links are not folowed.
This means that
On 2014/07/02 20:55:55, proppy wrote:
> followed

Done.

https://codereview.chromium.org/367903008/diff/1/base/README.md#newcode49
base/README.md:49: when the `package` directory is added it will contain
sym-links to the host
On 2014/07/02 20:55:55, proppy wrote:
> We don't have this issue if the packages directory is not part of the build
> context, maybe we could add it to .dockerignore.
> 
> https://github.com/dotcloud/docker/pull/6579

That is an option, but it will require users to remember to add the
.dockerignore. It is a nicer option though. I which the ADD command did have the
exclude option.

I tried to use .dockerignore in the hello sample, but t did not seem to work.

https://codereview.chromium.org/367903008/diff/1/build.sh
File build.sh (right):

https://codereview.chromium.org/367903008/diff/1/build.sh#newcode2
build.sh:2: # Build the Docker images for Dart.
On 2014/07/02 20:55:55, proppy wrote:
> not sure we need this script, this could be taken care of my docker.io
automated
> build.

OK - I have not looked into that yet.

https://codereview.chromium.org/367903008/diff/1/hello/README.md
File hello/README.md (right):

https://codereview.chromium.org/367903008/diff/1/hello/README.md#newcode4
hello/README.md:4: [docker](https://docker.io) image for a simple Dart HTTP
server.
On 2014/07/02 20:55:55, proppy wrote:
> +serving an hello world page

Done.

https://codereview.chromium.org/367903008/diff/1/hello/README.md#newcode14
hello/README.md:14: docker run -d -p 8080:8080 google/nodejs-hello
On 2014/07/03 09:40:05, kustermann wrote:
> nodejs -> dart

Done.

https://codereview.chromium.org/367903008/diff/1/hello/bin/server.dart
File hello/bin/server.dart (right):

https://codereview.chromium.org/367903008/diff/1/hello/bin/server.dart#newcode13
hello/bin/server.dart:13: request.response.redirect('/index.html');
On 2014/07/02 20:55:55, proppy wrote:
> can you add some basic server side templating

I want to keep this simple.

https://codereview.chromium.org/367903008/diff/1/hello/bin/server.dart#newcode28
hello/bin/server.dart:28: onError: (e, stackTrace) => print('Error processing
request $e\n$stackTrace'));
On 2014/07/03 09:40:05, kustermann wrote:
> long line.

Done.

https://codereview.chromium.org/367903008/diff/1/runtime/Dockerfile
File runtime/Dockerfile (right):

https://codereview.chromium.org/367903008/diff/1/runtime/Dockerfile#newcode6
runtime/Dockerfile:6: ONBUILD ENV PUB_CACHE /var/cache/pub
On 2014/07/02 20:55:55, proppy wrote:
> can we move that to the google/dart image?

Yes - but maybe we don't need it after all.

https://codereview.chromium.org/367903008/diff/1/runtime/Dockerfile#newcode9
runtime/Dockerfile:9: ONBUILD RUN /usr/lib/dart/bin/pub get
On 2014/07/02 20:55:55, proppy wrote:
> maybe we can get rid of that if we suggest adding a .dockerignore file with
> packages in the application directory.

Yes.

https://codereview.chromium.org/367903008/diff/1/runtime/Dockerfile#newcode9
runtime/Dockerfile:9: ONBUILD RUN /usr/lib/dart/bin/pub get
On 2014/07/03 09:40:05, kustermann wrote:
> On 2014/07/02 20:55:55, proppy wrote:
> > maybe we can get rid of that if we suggest adding a .dockerignore file with
> > packages in the application directory.
> 
> I think Soeren did this in order to cause less docker images, since only the
> "ADD . /app" will usually create a different image ID (the previous steps
should
> always result in the same image ID). i.e. it might be a space saving
technique.

Yes the first invocation of 'RUN /usr/lib/dart/bin/pub get' creates a layer
where the pub cache is populated with the packages currently used. This is
assuming that this will not change that often. However when adding the rest of
the application the package dir will be wrong. Running 'RUN
/usr/lib/dart/bin/pub get' again or adding packages to .dockerignore fixes this.
I find that requiring .dockerignore will lead to issues, however if you have it
the secone invocation of 'RUN /usr/lib/dart/bin/pub get' should be a no-op.

https://codereview.chromium.org/367903008/diff/1/runtime/README.md
File runtime/README.md (right):

https://codereview.chromium.org/367903008/diff/1/runtime/README.md#newcode3
runtime/README.md:3:
[`google/dart-runtime`](https://index.docker.io/u/google/dart-runtime)
On 2014/07/02 20:55:55, proppy wrote:
> can you update the description to match the python image, I think it is
simpler:
> https://github.com/GoogleCloudPlatform/python-docker/tree/master/runtime

Done.

https://codereview.chromium.org/367903008/diff/1/runtime/README.md#newcode22
runtime/README.md:22: docker build -t my/app .
On 2014/07/02 20:55:55, proppy wrote:
> I would remove the my/ prefix (it is not valid)

Changed to my-app.

https://codereview.chromium.org/367903008/diff/1/runtime/README.md#newcode27
runtime/README.md:27: docker run -d -p 8080:8080 my/app
On 2014/07/02 20:55:55, proppy wrote:
> I would remove the my/ prefix (it is not valid)

Changed to my-app.

https://codereview.chromium.org/367903008/diff/1/runtime/README.md#newcode40
runtime/README.md:40: - has a file named `bin/server.dart` as the entrypoint
script.
On 2014/07/02 20:55:55, proppy wrote:
> is that standard to have a bin subdirectory in a dart source repo?

Ye, this it the directory structure we are suggesting. bin/ for server side
code, web/ for client side code and lib/ for shared code.

[kustermann, 2014-07-03 12:39:00]

lgtm with comments

https://codereview.chromium.org/367903008/diff/20001/base/Dockerfile
File base/Dockerfile (right):

https://codereview.chromium.org/367903008/diff/20001/base/Dockerfile#newcode6
base/Dockerfile:6: ENV PATH /usr/lib/dart/bin:$PATH
@proppy: We were discussing whether it makes sense to add "apt-get update &&
apt-get upgrade" here. It doesn't look like the base "google/debian:wheezy"
image does this. But we need to ensure, that if our users re-build & re-deploy
they deploy a healthy system with all security updates in.

https://codereview.chromium.org/367903008/diff/20001/base/README.md
File base/README.md (right):

https://codereview.chromium.org/367903008/diff/20001/base/README.md#newcode23
base/README.md:23: ENV PUB_CACHE /var/cache/pub
Remove this line?

https://codereview.chromium.org/367903008/diff/20001/build.sh
File build.sh (right):

https://codereview.chromium.org/367903008/diff/20001/build.sh#newcode12
build.sh:12: docker tag google/dart google/dart-hello:$VERSION
google/dart -> google/dart-hello ?

[Søren Gjesse, 2014-07-03 14:25:38]

https://codereview.chromium.org/367903008/diff/20001/base/README.md
File base/README.md (right):

https://codereview.chromium.org/367903008/diff/20001/base/README.md#newcode23
base/README.md:23: ENV PUB_CACHE /var/cache/pub
On 2014/07/03 12:39:00, kustermann wrote:
> Remove this line?

Thanks, removed.

https://codereview.chromium.org/367903008/diff/20001/build.sh
File build.sh (right):

https://codereview.chromium.org/367903008/diff/20001/build.sh#newcode12
build.sh:12: docker tag google/dart google/dart-hello:$VERSION
On 2014/07/03 12:39:00, kustermann wrote:
> google/dart -> google/dart-hello ?

Sure, done.

[Adam, 2014-07-04 00:19:34]

https://codereview.chromium.org/367903008/diff/40001/runtime/README.md
File runtime/README.md (right):

https://codereview.chromium.org/367903008/diff/40001/runtime/README.md#newcode7
runtime/README.md:7: It can automatically bundle a Python application and its
dependencies with
Bundle a `Dart` application ?

[Søren Gjesse, 2014-07-04 07:15:12]

https://codereview.chromium.org/367903008/diff/40001/runtime/README.md
File runtime/README.md (right):

https://codereview.chromium.org/367903008/diff/40001/runtime/README.md#newcode7
runtime/README.md:7: It can automatically bundle a Python application and its
dependencies with
On 2014/07/04 00:19:34, Adam wrote:
> Bundle a `Dart` application ?

Thanks, changed.

[proppy, 2014-07-07 23:47:59]

https://codereview.chromium.org/367903008/diff/1/base/README.md
File base/README.md (right):

https://codereview.chromium.org/367903008/diff/1/base/README.md#newcode49
base/README.md:49: when the `package` directory is added it will contain
sym-links to the host
On 2014/07/03 10:36:40, Søren Gjesse wrote:
> On 2014/07/02 20:55:55, proppy wrote:
> > We don't have this issue if the packages directory is not part of the build
> > context, maybe we could add it to .dockerignore.
> > 
> > https://github.com/dotcloud/docker/pull/6579
> 
> That is an option, but it will require users to remember to add the
> .dockerignore. It is a nicer option though. I which the ADD command did have
the
> exclude option.
> 
> I tried to use .dockerignore in the hello sample, but t did not seem to work.

.dockerignore is only support by docker 1.1

https://codereview.chromium.org/367903008/diff/20001/base/Dockerfile
File base/Dockerfile (right):

https://codereview.chromium.org/367903008/diff/20001/base/Dockerfile#newcode6
base/Dockerfile:6: ENV PATH /usr/lib/dart/bin:$PATH
On 2014/07/03 12:39:00, kustermann wrote:
> @proppy: We were discussing whether it makes sense to add "apt-get update &&
> apt-get upgrade" here. It doesn't look like the base "google/debian:wheezy"
> image does this. But we need to ensure, that if our users re-build & re-deploy
> they deploy a healthy system with all security updates in.

It's not good practice to run apt-get upgrade in dockerfile, the base debian
image should be kept up to date instead.

[kevmoo, 2014-07-08 05:50:33]

DBC

https://codereview.chromium.org/367903008/diff/60001/base/Dockerfile
File base/Dockerfile (right):

https://codereview.chromium.org/367903008/diff/60001/base/Dockerfile#newcode4
base/Dockerfile:4: RUN wget -q
http://storage.googleapis.com/dart-archive/channels/stable/release/37644/linu...
&& dpkg -i dart_1.5.1-1_amd64.deb && rm dart_1.5.1-1_amd64.deb
FYI: you can use ADD w/ an HTTP resource and avoid installing wget

For example: https://github.com/kevmoo/docker-dart/blob/master/stable/Dockerfile

[kustermann, 2014-07-08 08:49:08]

[+isdal]

https://codereview.chromium.org/367903008/diff/20001/base/Dockerfile
File base/Dockerfile (right):

https://codereview.chromium.org/367903008/diff/20001/base/Dockerfile#newcode6
base/Dockerfile:6: ENV PATH /usr/lib/dart/bin:$PATH
On 2014/07/07 23:47:59, proppy wrote:
> On 2014/07/03 12:39:00, kustermann wrote:
> > @proppy: We were discussing whether it makes sense to add "apt-get update &&
> > apt-get upgrade" here. It doesn't look like the base "google/debian:wheezy"
> > image does this. But we need to ensure, that if our users re-build &
re-deploy
> > they deploy a healthy system with all security updates in.
> 
> It's not good practice to run apt-get upgrade in dockerfile, the base debian
> image should be kept up to date instead.

We saw you didn't do it for the other runtimes and discussed this a bit:

A linux distribution consists of a large number libraries/applications (even a
small debian). If a security bug gets discovered and fixed in the distribution,
users should get them as quickly as possible. A non-debatable requirement is
that users need to re-build images and re-deploy. Having an "apt-get upgrade" in
here, means users get it as soon as the distribution made a bugfix available. So
once debian has fixed a bug, they can build/deploy within minutes.

Otherwise, it depends how quickly we update the base debian image. Looking at
[0] it seems like "google/debian:wheezy" has not been touched since April --
that's *3 months*!

So:
 - What is the release schedule for the base images?
 - Who is responsible for making the releases (what if he is on vacation ...)?

I think we should take these things seriously.

[0] https://github.com/GoogleCloudPlatform/debian-docker/commits/master

[Søren Gjesse, 2014-08-12 13:08:50]

PTAL

https://codereview.chromium.org/367903008/diff/60001/base/Dockerfile
File base/Dockerfile (right):

https://codereview.chromium.org/367903008/diff/60001/base/Dockerfile#newcode4
base/Dockerfile:4: RUN wget -q
http://storage.googleapis.com/dart-archive/channels/stable/release/37644/linu...
&& dpkg -i dart_1.5.1-1_amd64.deb && rm dart_1.5.1-1_amd64.deb
On 2014/07/08 05:50:33, kevmoo wrote:
> FYI: you can use ADD w/ an HTTP resource and avoid installing wget
> 
> For example:
https://github.com/kevmoo/docker-dart/blob/master/stable/Dockerfile

Done.

[kevmoo, 2014-08-12 19:47:04]

lgtm

[Søren Gjesse, 2014-08-27 07:32:26]

Committed patchset #5 manually as 6b06e98 (presubmit successful).