Docker

[brandon-northcutt]

Hello Drake team,

I've created a pair of Docker images to build Drake within Ubuntu 16.04 Docker containers and to run a visual demonstration. These files may make it easier for users of heterogeneous development environments to build, demonstrate, and develop for Drake. I've created a pull request in case you want to include these files to the main repository. Instructions to build and test are in tools/docker/README.md.

---

This change is 

[jamiesnape]

Personally, I would put this in /setup instead of /tools, but others may disagree. Also, the copyright statement is inconsistent with the rest of drake. Also, I would add the docs to /drake/docs instead of a README.md.

[EricCousineau-TRI]

+@EricCousineau-TRI for feature review

---

Review status: 0 of 4 files reviewed at latest revision, all discussions resolved.

---

Comments from Reviewable

[EricCousineau-TRI]

Thanks again for putting this together! Just made a few minor comments. (NOTE: Just read the update on slack, so some of them may not apply anymore.)

Also, I concur with Jamie that this should go in /setup/ or /setup/docker.

NOTE: I am still somewhat of a Docker newbie, so please let me know if any of the below comments do not make sense.

---

Reviewed 1 of 4 files at r1.
Review status: 1 of 4 files reviewed at latest revision, 16 unresolved discussions.

---

tools/docker/Dockerfile.nvidia, line 1 at r1 (raw file):

# Copyright 2016-2017 Toyota Research Institute.  All rights reserved.

Per the Slack conversation, this copyright is not consistent with the rest of the code base. Would it be possible to remove this?

---

tools/docker/Dockerfile.nvidia, line 2 at r1 (raw file):

# Copyright 2016-2017 Toyota Research Institute.  All rights reserved.
FROM nvidia/cuda:8.0-devel-ubuntu16.04

Could you add a comment explaining why nvidia/cuda is necessary? (e.g., will have the correct LABEL com.nvidia.* directives, will have CUDA development libs installed, etc.), along with a URL to DockerHub? (e.g., if they want to switch to a different image, or use a ROS install base such as indigo 14.04, etc.)

---

tools/docker/Dockerfile.nvidia, line 4 at r1 (raw file):

FROM nvidia/cuda:8.0-devel-ubuntu16.04
RUN apt-get update && apt-get -y install --no-install-recommends \
  git \

Per Jeremy's comment on Slack, we should be able to leverage ./setup/ubuntu/16.04/install_prereqs.sh on this.

Would it be possible to change this docker image to do the following?

1. Only install git via the Dockerfiles.
2. Move as many of these into install_preqreqs.sh.
3. Directly clone from drake (e.g., to support CI more easily) rather than copying locally.
4. Add a comment on how to mount shared volumes in Dockerif someone wants their changes to persist outside of the Docker image.

---

tools/docker/Dockerfile.nvidia, line 10 at r1 (raw file):

  ca-certificates \
  apt-utils \
  && rm -rf /var/lib/apt/lists/* \

For people new to Docker, would it be possible to add comments regarding what is going on?
Perhaps this information could be in a small link to a Docker "Getting Started" tutorial in the README, and then referenced to from this Dockerfile.

---

tools/docker/Dockerfile.nvidia, line 13 at r1 (raw file):

  && apt-get clean all
RUN mkdir /drake
COPY / /drake

nit Rather than naming the root directory drake, can we name it drake-distro to keep consistent with the installation instructions?
Could you also include a reference to the installation instructions for drake here? (possibly relative to the repo, such that the version is the same?)

---

tools/docker/Dockerfile.nvidia, line 18 at r1 (raw file):

      && rm -rf /var/lib/apt/lists/* \
      && apt-get clean all
RUN mkdir /drake/build && cd /drake/build && cmake .. && make -j16 && cd /drake && bazel build ... && bazel test ...

Per Jeremy's other comment, we should probably block this on #6259, such that we only use bazel, and do not use cmake.
(At a later point, we will have an example of making a cmake project consume drake built by bazel.)

---

tools/docker/Dockerfile.nvidia, line 20 at r1 (raw file):

RUN mkdir /drake/build && cd /drake/build && cmake .. && make -j16 && cd /drake && bazel build ... && bazel test ...
COPY tools/docker/entrypoint.sh /
ENTRYPOINT ["/entrypoint.sh"]

Is it necessary to duplicate the entrypoint? Could we just do something like

ENTRYPOINT ["/drake/tools/docker/entrypoint.sh"]

since it's already there?

---

tools/docker/Dockerfile.opensource, line 18 at r1 (raw file):

      && rm -rf /var/lib/apt/lists/* \
      && apt-get clean all
RUN mkdir /drake/build && cd /drake/build && cmake .. && make -j16 && cd /drake && bazel build ... && bazel test ...

Is there a way to share the duplicate portions of these two scripts, or denote what is duplicated from the other image?
(e.g., INCLUDE if it is available?)

---

tools/docker/entrypoint.sh, line 2 at r1 (raw file):

#!/bin/bash
# Copyright 2016-2017 Toyota Research Institute.  All rights reserved.

Could you add a comment describing what this file does?

---

tools/docker/entrypoint.sh, line 3 at r1 (raw file):

#!/bin/bash
# Copyright 2016-2017 Toyota Research Institute.  All rights reserved.
set -e

nit Could you make this set -e -u as well?

---

tools/docker/entrypoint.sh, line 5 at r1 (raw file):

set -e
[[ $# -eq 0 ]] && {
  cd /drake/build/install/bin

Per the above comments, could you make this work using bazel run //drake/examples/Acrobot:acrobot_run_passive? (The corresponding drake-visualizer command could then be used from the aforementioned PR.)

---

tools/docker/README.md, line 6 at r1 (raw file):

# Drake Docker containers
There are two Docker containers provided that can build Drake in isolated
Ubuntu 16.04 environments.

Per the above comment, would it be possible to post a link to a simple Docker "Getting Started" tutorial to further explain these steps?

---

tools/docker/README.md, line 7 at r1 (raw file):

There are two Docker containers provided that can build Drake in isolated
Ubuntu 16.04 environments.
# Building

nit Could you add the command to do git clone ...?
(Just a minor thing for people who have alternative workflows - e.g., I like to spin my branches off using a submodule-aware version of git-new-workdir, but the symlinks do not play well with copying straight into a separate Docker image.)

---

tools/docker/README.md, line 8 at r1 (raw file):

Ubuntu 16.04 environments.
# Building
$ cd <drake-root-dir>  

nit Would it be possible to remove the trailing whitespace?

---

tools/docker/README.md, line 19 at r1 (raw file):

## Passive Acrobot Simulation
### Nvidia drivers:  (requires nvidia-docker plugin)  
$ xhost +local:root; nvidia-docker run -ti --rm -e DISPLAY \  

nit Could this be placed in an indentation block / code block? \```

---

tools/docker/README.md, line 38 at r1 (raw file):

-e QT_X11_NO_MITSHM=1 -v /tmp/.X11-unix:/tmp/.X11-unix \  
--privileged drake bash; xhost -local:root  

Could you add a "Tips" section, such as removing --rm if you want the image to persist, and simple command references such as docker image ls and docker container ls -a?

---

Comments from Reviewable

[jamiesnape]

@brandon-northcutt Can you rebase?

[brandon-northcutt]

Reviewed 8 of 8 files at r2, 3 of 7 files at r3.
Review status: 3 of 5 files reviewed at latest revision, 16 unresolved discussions.

---

setup/docker/entrypoint.sh, line 2 at r1 (raw file):

Previously, EricCousineau-TRI wrote…

Could you add a comment describing what this file does?

As a comment in the file?

---

setup/docker/entrypoint.sh, line 3 at r1 (raw file):

Previously, EricCousineau-TRI wrote…

nit Could you make this set -e -u as well?

Done.

---

setup/docker/entrypoint.sh, line 5 at r1 (raw file):

Previously, EricCousineau-TRI wrote…

Per the above comments, could you make this work using bazel run //drake/examples/Acrobot:acrobot_run_passive? (The corresponding drake-visualizer command could then be used from the aforementioned PR.)

Done.

---

setup/docker/Dockerfile.nvidia, line 1 at r1 (raw file):

Previously, EricCousineau-TRI wrote…

Per the Slack conversation, this copyright is not consistent with the rest of the code base. Would it be possible to remove this?

Removed copyright lines.

---

setup/docker/Dockerfile.nvidia, line 2 at r1 (raw file):

Previously, EricCousineau-TRI wrote…

Could you add a comment explaining why nvidia/cuda is necessary? (e.g., will have the correct LABEL com.nvidia.* directives, will have CUDA development libs installed, etc.), along with a URL to DockerHub? (e.g., if they want to switch to a different image, or use a ROS install base such as indigo 14.04, etc.)

The Cuda image was not necessary as Drake does not need Cuda functionality. I have based the container on vanilla 16.04 Ubuntu. Actually, public images are free on DockerHub, should we publish a drake:16.04 image where Drake is pre-build?

---

setup/docker/Dockerfile.nvidia, line 4 at r1 (raw file):

Previously, EricCousineau-TRI wrote…

Per Jeremy's comment on Slack, we should be able to leverage ./setup/ubuntu/16.04/install_prereqs.sh on this.

Would it be possible to change this docker image to do the following?

1. Only install git via the Dockerfiles.
2. Move as many of these into install_preqreqs.sh.
3. Directly clone from drake (e.g., to support CI more easily) rather than copying locally.
4. Add a comment on how to mount shared volumes in Dockerif someone wants their changes to persist outside of the Docker image.

I had to add sudo to install_prereqs.sh to make this work, but I was able to remove the additional apt-get lines. The workflow I envisioned is that a user will git clone drake and then run docker build -t drake -f setup/docker/Dockerfile.nvidia . from <drake-distro> so that they can test branches or experimental builds and not just the current state of master. One could -v share a volume to the Docker for persistent changes, but then the host / container isolation gets worse. I'd prefer to COPY code in so that changes made in the Docker are isolated from the host. I'm totally open to another workflow. What would be better? Maybe the drake image on DockerHub and then just a COPY for the user's personal code.

---

setup/docker/Dockerfile.nvidia, line 10 at r1 (raw file):

Previously, EricCousineau-TRI wrote…

For people new to Docker, would it be possible to add comments regarding what is going on?
Perhaps this information could be in a small link to a Docker "Getting Started" tutorial in the README, and then referenced to from this Dockerfile.

I added a Getting Started section to the documentation.

---

setup/docker/Dockerfile.nvidia, line 13 at r1 (raw file):

Previously, EricCousineau-TRI wrote…

nit Rather than naming the root directory drake, can we name it drake-distro to keep consistent with the installation instructions?
Could you also include a reference to the installation instructions for drake here? (possibly relative to the repo, such that the version is the same?)

Renamed /drake in Docker container to /drake-distro.

---

setup/docker/Dockerfile.nvidia, line 18 at r1 (raw file):

Previously, EricCousineau-TRI wrote…

Per Jeremy's other comment, we should probably block this on #6259, such that we only use bazel, and do not use cmake.
(At a later point, we will have an example of making a cmake project consume drake built by bazel.)

Fine by me. There will be less time waiting for build if we don't run cmake too.

---

setup/docker/Dockerfile.nvidia, line 20 at r1 (raw file):

Previously, EricCousineau-TRI wrote…

Is it necessary to duplicate the entrypoint? Could we just do something like

ENTRYPOINT ["/drake/tools/docker/entrypoint.sh"]

since it's already there?

Good point.

---

setup/docker/Dockerfile.opensource, line 18 at r1 (raw file):

Previously, EricCousineau-TRI wrote…

Is there a way to share the duplicate portions of these two scripts, or denote what is duplicated from the other image?
(e.g., INCLUDE if it is available?)

There is now only one Dockerfile as Cuda support wasn't necessary.

---

tools/docker/README.md, line 6 at r1 (raw file):

Previously, EricCousineau-TRI wrote…

Per the above comment, would it be possible to post a link to a simple Docker "Getting Started" tutorial to further explain these steps?

Getting Started section added.

---

tools/docker/README.md, line 7 at r1 (raw file):

Previously, EricCousineau-TRI wrote…

nit Could you add the command to do git clone ...?
(Just a minor thing for people who have alternative workflows - e.g., I like to spin my branches off using a submodule-aware version of git-new-workdir, but the symlinks do not play well with copying straight into a separate Docker image.)

Similar to the other workflow comment. Let's talk more about what is a good workflow to give flexibility, simplicity, and generality.

---

tools/docker/README.md, line 8 at r1 (raw file):

Previously, EricCousineau-TRI wrote…

nit Would it be possible to remove the trailing whitespace?

No more README.md.

---

tools/docker/README.md, line 19 at r1 (raw file):

Previously, EricCousineau-TRI wrote…

nit Could this be placed in an indentation block / code block? \```

Code blocks used in drake/doc/docker.rst.

---

tools/docker/README.md, line 38 at r1 (raw file):

Previously, EricCousineau-TRI wrote…

Could you add a "Tips" section, such as removing --rm if you want the image to persist, and simple command references such as docker image ls and docker container ls -a?

Have a look at the new Sphinx document and let me know if anything is missing.

---

Comments from Reviewable

[brandon-northcutt]

Reviewed 3 of 8 files at r2, 4 of 7 files at r3.
Review status: all files reviewed at latest revision, 16 unresolved discussions.

---

Comments from Reviewable

[EricCousineau-TRI]

Reviewed 2 of 7 files at r3.
Review status: all files reviewed at latest revision, 19 unresolved discussions.

---

drake/doc/docker.rst, line 8 at r3 (raw file):

.. _docker_intro:

Would it be possible to mention the docker image is provided as an experimental feature, stating that it is presently not under CI?

---

drake/doc/docker.rst, line 36 at r3 (raw file):

========
::

Just to make it explicit, could you mention that the Dockerfile will copy the workspace root of drake-distro, which the user should have already cloned drake per the Getting Drake section?

And would it still be possible to mention how a user can get their changes back onto their host system? Options that I am aware of:

• docker cp - maybe there's a git remote-helper that could possibly make it easy to fetch / push to a container?
• Mounting with -v - might it suffice to warn that this will affect isolation, and you should mount on a fresh clone, such that your builds don't collide?
• ssh / sshfs or something else - which seems like a lot of extra noise for file transfer...

---

drake/doc/docker.rst, line 42 at r3 (raw file):

If successful::

  $ drake images

Minor typo here and below: s/drake/docker/

---

drake/doc/docker.rst, line 48 at r3 (raw file):

Note:::

  $ drake ps

Minor typo here as well.

---

drake/doc/docker.rst, line 58 at r3 (raw file):

.. _docker_running_simulation:

Could you make a quick note here that run --rm will delete the container once it is done, and that a user should remove this flag if they want to test incremental builds on their system?

---

setup/docker/entrypoint.sh, line 2 at r1 (raw file):

Previously, brandon-northcutt (Brandon D. Northcutt) wrote…

As a comment in the file?

Yup! (Sorry for not clarifying that.)

---

setup/docker/Dockerfile.nvidia, line 2 at r1 (raw file):

Previously, brandon-northcutt (Brandon D. Northcutt) wrote…

The Cuda image was not necessary as Drake does not need Cuda functionality. I have based the container on vanilla 16.04 Ubuntu. Actually, public images are free on DockerHub, should we publish a drake:16.04 image where Drake is pre-build?

I would definitely like to see a public Drake prebuilt image; however, I am not sure what the best timing would be, and would be dependent on CI stuff.

I will create a separate issue on this, and will tag you, @jwnimmer-tri, and @stonier.

---

setup/docker/Dockerfile.nvidia, line 4 at r1 (raw file):

Previously, brandon-northcutt (Brandon D. Northcutt) wrote…

I had to add sudo to install_prereqs.sh to make this work, but I was able to remove the additional apt-get lines. The workflow I envisioned is that a user will git clone drake and then run docker build -t drake -f setup/docker/Dockerfile.nvidia . from <drake-distro> so that they can test branches or experimental builds and not just the current state of master. One could -v share a volume to the Docker for persistent changes, but then the host / container isolation gets worse. I'd prefer to COPY code in so that changes made in the Docker are isolated from the host. I'm totally open to another workflow. What would be better? Maybe the drake image on DockerHub and then just a COPY for the user's personal code.

BTW Ah, OK, copying does make more sense, thanks for explaining that!

I have added some comments above on adding some sidenotes.

---

Comments from Reviewable

[brandon-northcutt]

Reviewed 1 of 8 files at r2, 3 of 4 files at r4.
Review status: 4 of 5 files reviewed at latest revision, 8 unresolved discussions.

---

drake/doc/docker.rst, line 8 at r3 (raw file):

Previously, EricCousineau-TRI wrote…

Would it be possible to mention the docker image is provided as an experimental feature, stating that it is presently not under CI?

Done.

---

drake/doc/docker.rst, line 36 at r3 (raw file):

Previously, EricCousineau-TRI wrote…

Just to make it explicit, could you mention that the Dockerfile will copy the workspace root of drake-distro, which the user should have already cloned drake per the Getting Drake section?

And would it still be possible to mention how a user can get their changes back onto their host system? Options that I am aware of:

• docker cp - maybe there's a git remote-helper that could possibly make it easy to fetch / push to a container?
• Mounting with -v - might it suffice to warn that this will affect isolation, and you should mount on a fresh clone, such that your builds don't collide?
• ssh / sshfs or something else - which seems like a lot of extra noise for file transfer...

Added comments about getting code out of the Docker container.

---

drake/doc/docker.rst, line 42 at r3 (raw file):

Previously, EricCousineau-TRI wrote…

Minor typo here and below: s/drake/docker/

Thanks!

---

drake/doc/docker.rst, line 48 at r3 (raw file):

Previously, EricCousineau-TRI wrote…

Minor typo here as well.

Fixed.

---

drake/doc/docker.rst, line 58 at r3 (raw file):

Previously, EricCousineau-TRI wrote…

Could you make a quick note here that run --rm will delete the container once it is done, and that a user should remove this flag if they want to test incremental builds on their system?

Comment added.

---

setup/docker/entrypoint.sh, line 2 at r1 (raw file):

Previously, EricCousineau-TRI wrote…

Yup! (Sorry for not clarifying that.)

Done.

---

Comments from Reviewable

[brandon-northcutt]

Reviewed 1 of 4 files at r4.
Review status: all files reviewed at latest revision, 8 unresolved discussions.

---

Comments from Reviewable

[EricCousineau-TRI]

, minus minor follow-ups.

+@ggould-tri for platform review.

---

Reviewed 4 of 8 files at r2, 5 of 7 files at r3, 4 of 4 files at r4.
Review status: all files reviewed at latest revision, 3 unresolved discussions.

---

drake/doc/docker.rst, line 15 at r4 (raw file):

use drake on a variety of host operating systems.

Note: this docker image is provided as an experimental feature and are not

nit Capitalization s/this/This, grammar s/are/is

---

drake/doc/docker.rst, line 104 at r4 (raw file):

server and pass the necessary X11 parameters for graphical display of programs
within the Docker container. The `-i` switch assigns a tty for interactive
text connections within the console. `-rm` will clean up after the image, omit

nit Missing dash --rm

---

drake/doc/docker.rst, line 106 at r4 (raw file):

text connections within the console. `-rm` will clean up after the image, omit
this to allow the container's file system to persist.
See the `Docker Run Referencee

nit Spelling s/Referencee/Reference

---

Comments from Reviewable

[brandon-northcutt]

Review status: all files reviewed at latest revision, 3 unresolved discussions.

---

drake/doc/docker.rst, line 15 at r4 (raw file):

Previously, EricCousineau-TRI wrote…

nit Capitalization s/this/This, grammar s/are/is

Good catch.

---

drake/doc/docker.rst, line 104 at r4 (raw file):

Previously, EricCousineau-TRI wrote…

nit Missing dash --rm

Thanks.

---

drake/doc/docker.rst, line 106 at r4 (raw file):

Previously, EricCousineau-TRI wrote…

nit Spelling s/Referencee/Reference

Thanks.

---

Comments from Reviewable

[brandon-northcutt]

Reviewed 2 of 2 files at r5.
Review status: 4 of 5 files reviewed at latest revision, 3 unresolved discussions.

---

Comments from Reviewable

[brandon-northcutt]

Reviewed 1 of 1 files at r6.
Review status: all files reviewed at latest revision, 3 unresolved discussions.

---

Comments from Reviewable

[brandon-northcutt]

Reviewed 1 of 1 files at r7.
Review status: all files reviewed at latest revision, 3 unresolved discussions.

---

Comments from Reviewable

[brandon-northcutt]

Review status: all files reviewed at latest revision, 4 unresolved discussions.

---

a discussion (no related file):
I need to test nvidia-docker and docker...

---

Comments from Reviewable

[ggould-tri]

This will take a little while as I will need to get a 16.04 drake world to test this. I'll try to get to it this afternoon or tomorrow.

---

Review status: all files reviewed at latest revision, 4 unresolved discussions.

---

Comments from Reviewable

[ggould-tri]

Failed at the first couple of commands. Notes below.

---

Review status: 4 of 6 files reviewed at latest revision, 7 unresolved discussions.

---

drake/doc/docker.rst, line 27 at r8 (raw file):

system while also isolating these executables from the host file system.
Docker is available for all major operating systems. Please see Docker's
`Getting Started <https://docs.docker.com/get-started/>`_ for basic information

That document requires docker 1.13 or later; ubuntu 14.04 and 16.04 do not meet this. Users will be unable to run the recommended commands in that document.

---

drake/doc/docker.rst, line 50 at r8 (raw file):

::

  $ docker build -t drake -f setup/docker/Dockerfile .

unable to prepare context: unable to evaluate symlinks in Dockerfile path: lstat /home/ggould/git/drake-docker-test/setup/docker: no such file or directory

---

drake/doc/docker.rst, line 50 at r8 (raw file):

::

  $ docker build -t drake -f setup/docker/Dockerfile .

AFAICT on default ubuntu 16.04 install, the first docker command must be sudo for the daemon to start correctly.

---

Comments from Reviewable

[ggould-tri]

Review status: 4 of 6 files reviewed at latest revision, 6 unresolved discussions.

---

drake/doc/docker.rst, line 50 at r8 (raw file):

Previously, ggould-tri wrote…

unable to prepare context: unable to evaluate symlinks in Dockerfile path: lstat /home/ggould/git/drake-docker-test/setup/docker: no such file or directory

User error. Closing.

---

Comments from Reviewable

[ggould-tri]

Review status: 4 of 6 files reviewed at latest revision, 7 unresolved discussions.

---

drake/doc/docker.rst, line 50 at r8 (raw file):

::

  $ docker build -t drake -f setup/docker/Dockerfile .

unable to prepare context: unable to evaluate symlinks in Dockerfile path: lstat /home/ggould/git/drake-docker-test/setup/docker/Dockerfile: no such file or directory

---

Comments from Reviewable

[brandon-northcutt]

Reviewed 3 of 3 files at r8.
Review status: all files reviewed at latest revision, 7 unresolved discussions.

---

drake/doc/docker.rst, line 27 at r8 (raw file):

Previously, ggould-tri wrote…

That document requires docker 1.13 or later; ubuntu 14.04 and 16.04 do not meet this. Users will be unable to run the recommended commands in that document.

Only the operating system inside of the Docker container is intended to be constrained to Ubuntu 16.04. We can have additional Dockerfiles for each system we intend to support. Then CI can use these Docker images to verify build and test results on all those systems. But the additional benefit is that users can bring up Drake on any host system that can install Docker (most). I don't think we want to take on providing instructions on how to setup and install Docker on all the possible host systems. So I should probably amend this getting started to say "follow your preferred operating system instructions for getting Docker installed." It is certainly true that some users will have Ubuntu 16.04 as their host system, I'll bring up a 16.04 system and make sure these commands will run there.

---

drake/doc/docker.rst, line 50 at r8 (raw file):

Previously, ggould-tri wrote…

AFAICT on default ubuntu 16.04 install, the first docker command must be sudo for the daemon to start correctly.

After installing Docker you'll need to add yourself to the docker group and re-login to get access to the Docker daemon. I'll update the getting started.

---

Comments from Reviewable

[ggould-tri]

Review status: all files reviewed at latest revision, 7 unresolved discussions.

---

drake/doc/docker.rst, line 27 at r8 (raw file):

Previously, brandon-northcutt (Brandon D. Northcutt) wrote…

Only the operating system inside of the Docker container is intended to be constrained to Ubuntu 16.04. We can have additional Dockerfiles for each system we intend to support. Then CI can use these Docker images to verify build and test results on all those systems. But the additional benefit is that users can bring up Drake on any host system that can install Docker (most). I don't think we want to take on providing instructions on how to setup and install Docker on all the possible host systems. So I should probably amend this getting started to say "follow your preferred operating system instructions for getting Docker installed." It is certainly true that some users will have Ubuntu 16.04 as their host system, I'll bring up a 16.04 system and make sure these commands will run there.

My concern is mostly that you're linking to a document which directly says that all of our supported configurations are not supported ("Note: version 1.13 or higher is required"). So we should be clearer what versions of docker we support and whether users should be taking a PPA to get current docker versions.

---

Comments from Reviewable

[brandon-northcutt]

Review status: all files reviewed at latest revision, 6 unresolved discussions.

---

drake/doc/docker.rst, line 27 at r8 (raw file):

Previously, ggould-tri wrote…

My concern is mostly that you're linking to a document which directly says that all of our supported configurations are not supported ("Note: version 1.13 or higher is required"). So we should be clearer what versions of docker we support and whether users should be taking a PPA to get current docker versions.

What I mean is that our supported configuration is inside the Docker image where Docker is not installed at all and I think it is by design that we do not want to place restrictions on the host operating system software versions. It is interesting that Docker's official Getting Started page specifies 1.13 as a minimum version and 1.12.6 is provided by Apt. I will remove the link to Docker's Getting Started page.

---

Comments from Reviewable

[sammy-tri]

Review status: all files reviewed at latest revision, 9 unresolved discussions, some commit checks failed.

---

drake/doc/docker.rst, line 149 at r12 (raw file):

Previously, brandon-northcutt (Brandon D. Northcutt) wrote…

Does the next command work for you anyway?

It doesn't seem to, but possible for a different reason (btw I'm using Ubuntu 16.04 as the host OS if I haven't already mentioned it):

% nvidia-docker run --rm nvidia/cuda nvidia-smi
Using default tag: latest
latest: Pulling from nvidia/cuda
Digest: sha256:dcc2ef183f11ea9e915579614fda1a04695356583dae4eb4a5f36e29f15ae611
Status: Downloaded newer image for nvidia/cuda:latest
nvidia-docker | 2017/06/16 12:48:40 Error: Could not load UVM kernel module. Is nvidia-modprobe installed?

The same error appears if I run under sudo (though it doesn't download again...)

---

Comments from Reviewable

[brandon-northcutt]

Review status: all files reviewed at latest revision, 9 unresolved discussions, some commit checks failed.

---

drake/doc/docker.rst, line 149 at r12 (raw file):

Previously, sammy-tri (Sam Creasey) wrote…

It doesn't seem to, but possible for a different reason (btw I'm using Ubuntu 16.04 as the host OS if I haven't already mentioned it):

% nvidia-docker run --rm nvidia/cuda nvidia-smi
Using default tag: latest
latest: Pulling from nvidia/cuda
Digest: sha256:dcc2ef183f11ea9e915579614fda1a04695356583dae4eb4a5f36e29f15ae611
Status: Downloaded newer image for nvidia/cuda:latest
nvidia-docker | 2017/06/16 12:48:40 Error: Could not load UVM kernel module. Is nvidia-modprobe installed?

The same error appears if I run under sudo (though it doesn't download again...)

Just a sanity check, does this Ubuntu 16.04 host machine have an Nvidia GPU and the official drivers from Nvidia installed? Let me find a co-worker with this configuration and I'll step through there. My Ubuntu 16.04 machine is using the opensource drivers.

---

Comments from Reviewable

[sammy-tri]

Review status: all files reviewed at latest revision, 9 unresolved discussions, some commit checks failed.

---

drake/doc/docker.rst, line 149 at r12 (raw file):

Previously, brandon-northcutt (Brandon D. Northcutt) wrote…

Just a sanity check, does this Ubuntu 16.04 host machine have an Nvidia GPU and the official drivers from Nvidia installed? Let me find a co-worker with this configuration and I'll step through there. My Ubuntu 16.04 machine is using the opensource drivers.

Yep, NVIDIA driver version 370.28 (according to nvidia-settings). It also has an Intel GPU (it's a laptop) which is reported as Inactive Device "intel" in Xorg.0.log.

The opensource instructions worked, except for the part where it couldn't actually use GL from inside the container since it wasn't using the right GL library.

---

Comments from Reviewable

[brandon-northcutt]

Reviewed 1 of 1 files at r14.
Review status: all files reviewed at latest revision, 9 unresolved discussions.

---

drake/doc/docker.rst, line 149 at r12 (raw file):

Previously, sammy-tri (Sam Creasey) wrote…

Yep, NVIDIA driver version 370.28 (according to nvidia-settings). It also has an Intel GPU (it's a laptop) which is reported as Inactive Device "intel" in Xorg.0.log.

The opensource instructions worked, except for the part where it couldn't actually use GL from inside the container since it wasn't using the right GL library.

Okay, I've installed a vanilla Ubuntu 16.04 x86_64 with Nvidia and modified the instructions to a working state on that system. I've also verified this Docker system works in Fedora 25. Hopefully it will also work in MacOS and Windows Dockers. I'm hoping more users will test. Let me know if it works for you now!

---

Comments from Reviewable

[sammy-tri]

Reviewed 1 of 3 files at r8, 1 of 3 files at r9, 1 of 2 files at r12.
Review status: all files reviewed at latest revision, 7 unresolved discussions.

---

drake/doc/docker.rst, line 149 at r12 (raw file):

Previously, brandon-northcutt (Brandon D. Northcutt) wrote…

Okay, I've installed a vanilla Ubuntu 16.04 x86_64 with Nvidia and modified the instructions to a working state on that system. I've also verified this Docker system works in Fedora 25. Hopefully it will also work in MacOS and Windows Dockers. I'm hoping more users will test. Let me know if it works for you now!

The current state of the PR seems to have gone back to the (non-working) deb install instructions?

---

Comments from Reviewable

[sammy-tri]

Review status: all files reviewed at latest revision, 7 unresolved discussions.

---

drake/doc/docker.rst, line 149 at r12 (raw file):

Previously, sammy-tri (Sam Creasey) wrote…

The current state of the PR seems to have gone back to the (non-working) deb install instructions?

oh, wait, other instructions changed! will try.

---

Comments from Reviewable

[sammy-tri]

Tested and it seems to work. I think we're close to being done. Left a couple more comments.

---

Reviewed 1 of 8 files at r2, 1 of 5 files at r10, 1 of 1 files at r14.
Review status: all files reviewed at latest revision, 7 unresolved discussions.

---

drake/doc/docker.rst, line 137 at r11 (raw file):

Previously, brandon-northcutt (Brandon D. Northcutt) wrote…

Hmm. The .deb methods seem to require much more recent docker installations. I've added steps for compiling nvidia-docker Ubuntu 16.04.

The new recipe seems to work for me!

---

drake/doc/docker.rst, line 149 at r12 (raw file):

Previously, sammy-tri (Sam Creasey) wrote…

oh, wait, other instructions changed! will try.

Ok, it looks like I need to do sudo apt install nvidia-modprobe to get that package (so the error message was real, it seems!) I can now run nvidia stuff inside docker.

---

drake/doc/docker.rst, line 231 at r14 (raw file):

Note: these are currently not rendering properly due to VTK .obj/.mtl importing. A patch is expected any day.

I tried this and indeed the import failed but it seemed to run. Is this a director or drake patch we're waiting for?

---

Comments from Reviewable

[brandon-northcutt]

Review status: all files reviewed at latest revision, 5 unresolved discussions.

---

drake/doc/docker.rst, line 137 at r11 (raw file):

Previously, sammy-tri (Sam Creasey) wrote…

The new recipe seems to work for me!

Nice!

---

Comments from Reviewable

[brandon-northcutt]

Review status: all files reviewed at latest revision, 6 unresolved discussions.

---

drake/doc/docker.rst, line 231 at r14 (raw file):

Previously, sammy-tri (Sam Creasey) wrote…

I tried this and indeed the import failed but it seemed to run. Is this a director or drake patch we're waiting for?

I had another PR about the .obj/.mtl import problem (#6352). It looks like the upstream patch in VTK @liangfok mentioned has landed that mitigates that issue. If it still doesn't import, maybe we are waiting for the bazel packaged drake_visualizer to be re-rolled.

---

Comments from Reviewable

[sammy-tri]

Review status: all files reviewed at latest revision, 7 unresolved discussions.

---

drake/doc/docker.rst, line 149 at r12 (raw file):

Previously, sammy-tri (Sam Creasey) wrote…

Ok, it looks like I need to do sudo apt install nvidia-modprobe to get that package (so the error message was real, it seems!) I can now run nvidia stuff inside docker.

Can we add a mention that nvidia-modprobe needs to be installed before nvidia-docker? It'd be nice if others didn't have to track the dependency down like I did.

---

drake/doc/docker.rst, line 231 at r14 (raw file):

Previously, brandon-northcutt (Brandon D. Northcutt) wrote…

I had another PR about the .obj/.mtl import problem (#6352). It looks like the upstream patch in VTK @liangfok mentioned has landed that mitigates that issue. If it still doesn't import, maybe we are waiting for the bazel packaged drake_visualizer to be re-rolled.

If there's not actually a patch in progress, perhaps we should remove the "any day" promise. I think this PR is plenty useful on it's own, just don't want to get anyone's hopes up.

---

Comments from Reviewable

[brandon-northcutt]

Review status: all files reviewed at latest revision, 6 unresolved discussions.

---

drake/doc/docker.rst, line 149 at r12 (raw file):

Previously, sammy-tri (Sam Creasey) wrote…

Can we add a mention that nvidia-modprobe needs to be installed before nvidia-docker? It'd be nice if others didn't have to track the dependency down like I did.

I don't have any of the xenial repository nvidia packages installed on my test machine. Maybe we can get together and compare packages?

---

drake/doc/docker.rst, line 231 at r14 (raw file):

Previously, sammy-tri (Sam Creasey) wrote…

If there's not actually a patch in progress, perhaps we should remove the "any day" promise. I think this PR is plenty useful on it's own, just don't want to get anyone's hopes up.

Should I add a PR providing .mtl files so it the IIWA pick and place renders correctly now? I could make the default demonstration acrobot, but it's not as fun to look at as the IIWA pick and place or the bowling ball.

---

Comments from Reviewable

[sammy-tri]

Review status: all files reviewed at latest revision, 6 unresolved discussions.

---

drake/doc/docker.rst, line 149 at r12 (raw file):

Previously, brandon-northcutt (Brandon D. Northcutt) wrote…

I don't have any of the xenial repository nvidia packages installed on my test machine. Maybe we can get together and compare packages?

nvidia-modprobe looks like it came from xenial/multiverse on my machine. I'm curious how you would have gotten nvidia-docker to run without it using either recipe since both complained about it until I installed the package.

---

drake/doc/docker.rst, line 231 at r14 (raw file):

Previously, brandon-northcutt (Brandon D. Northcutt) wrote…

Should I add a PR providing .mtl files so it the IIWA pick and place renders correctly now? I could make the default demonstration acrobot, but it's not as fun to look at as the IIWA pick and place or the bowling ball.

I don't think a one-off fix adding those specific mtl files is worth it for just that demo to work. I'm fine with noting the issue that importing is currently broken without getting peoples hopes up that the fix is comine soon.

---

Comments from Reviewable

[brandon-northcutt]

Review status: 5 of 7 files reviewed at latest revision, 6 unresolved discussions.

---

drake/doc/docker.rst, line 149 at r12 (raw file):

Previously, sammy-tri (Sam Creasey) wrote…

nvidia-modprobe looks like it came from xenial/multiverse on my machine. I'm curious how you would have gotten nvidia-docker to run without it using either recipe since both complained about it until I installed the package.

Ah, I think I see what is going on. I installed Nvidia drivers via the .run file from Nvidia rather than with apt packages. nvidia-modprobe is indeed required when using apt rolled Nvidia drivers. I added the apt command for Ubuntu 16.04 driver installation.

---

drake/doc/docker.rst, line 231 at r14 (raw file):

Previously, sammy-tri (Sam Creasey) wrote…

I don't think a one-off fix adding those specific mtl files is worth it for just that demo to work. I'm fine with noting the issue that importing is currently broken without getting peoples hopes up that the fix is comine soon.

Done.

---

Comments from Reviewable

[brandon-northcutt]

Reviewed 1 of 1 files at r15, 1 of 1 files at r16.
Review status: all files reviewed at latest revision, 6 unresolved discussions.

---

Comments from Reviewable

[sammy-tri]

pending the remaining open issue

---

Reviewed 1 of 1 files at r16.
Review status: all files reviewed at latest revision, 2 unresolved discussions.

---

drake/doc/docker.rst, line 149 at r12 (raw file):

Previously, brandon-northcutt (Brandon D. Northcutt) wrote…

Ah, I think I see what is going on. I installed Nvidia drivers via the .run file from Nvidia rather than with apt packages. nvidia-modprobe is indeed required when using apt rolled Nvidia drivers. I added the apt command for Ubuntu 16.04 driver installation.

Oh, heh, that explains it. FWIW I'm using nvidia-370 but I'm honestly not sure there's any "correct" version to specify here, so I don't have a better suggestion.

---

Comments from Reviewable

[brandon-northcutt]

Review status: all files reviewed at latest revision, 1 unresolved discussion.

---

drake/doc/index.rst, line 121 at r9 (raw file):

Previously, brandon-northcutt (Brandon D. Northcutt) wrote…

Sure thing.

Looks like it's pretty much ready!

---

Comments from Reviewable

[RussTedrake]

Review status: all files reviewed at latest revision, all discussions resolved.

---

drake/doc/index.rst, line 121 at r9 (raw file):

Previously, brandon-northcutt (Brandon D. Northcutt) wrote…

Looks like it's pretty much ready!

Looks like a lot if whitespace changes to this file? Otherwise aok

---

Comments from Reviewable

[brandon-northcutt]

Review status: 3 of 7 files reviewed at latest revision, all discussions resolved.

---

drake/doc/index.rst, line 121 at r9 (raw file):

Previously, RussTedrake (Russ Tedrake) wrote…

Looks like a lot if whitespace changes to this file? Otherwise aok

I have un-twiddled the whitespace. Thanks!

---

Comments from Reviewable

[brandon-northcutt]

Reviewed 2 of 2 files at r17.
Review status: all files reviewed at latest revision, all discussions resolved.

---

Comments from Reviewable

[brandon-northcutt]

Commits squashed down to one. All issues resolved.

---

Review status: all files reviewed at latest revision, all discussions resolved, some commit checks pending.

---

Comments from Reviewable

[sammy-tri]

-(status: curate commits before merging)

---

Reviewed 1 of 1 files at r15, 2 of 2 files at r17.
Review status: all files reviewed at latest revision, all discussions resolved, all commit checks successful.

---

Comments from Reviewable