Adding script to build Docker images with ETA installed #440
Conversation
docker/README.md
Outdated
|
|
||
| - [Docker Community Edition](https://hub.docker.com/search/?type=edition&offering=community) | ||
| - To build/run GPU-enabled images, you must install `nvidia-docker` by | ||
| following [these instructions](https://github.com/NVIDIA/nvidia-docker) |
There was a problem hiding this comment.
+1 issue made for this: #443
docker/Dockerfile
Outdated
| && add-apt-repository -y ppa:deadsnakes/ppa \ | ||
| && apt-get update \ | ||
| && apt-get -y --no-install-recommends install \ | ||
| sudo \ |
There was a problem hiding this comment.
If we want reproducible environments, we want to pin dependency versions.
There was a problem hiding this comment.
+1 issue made for this: #444
docker/Dockerfile
Outdated
| COPY eta/ eta/ | ||
| ARG TENSORFLOW_VERSION | ||
| RUN pip --no-cache-dir install --upgrade pip setuptools \ | ||
| && pip --no-cache-dir install -r eta/requirements.txt \ |
There was a problem hiding this comment.
There's quite a lot going on here... we really want to package ETA and make it installable via pip.
There was a problem hiding this comment.
Agree, but most of the extra work is non-Python related install, so there's a bit of work required to make this as clean as possible
docker/build.bash
Outdated
|
|
||
| # Build image | ||
| docker build \ | ||
| --file Dockerfile \ |
There was a problem hiding this comment.
No need to specify this, as it looks for Dockerfile by default.
docker/build.bash
Outdated
| git submodule init | ||
| git submodule update | ||
| cp config-example.json config.json | ||
| rm -rf .git/modules # trim the fat (removes ~500MB) |
There was a problem hiding this comment.
Since we're trying to keep .git/modules out of the build context, I'd create a .dockerignore and specify it there instead.
docker/README.md
Outdated
| ```shell | ||
| BASE_IMAGE="nvidia/cuda:9.0-cudnn7-runtime-ubuntu16.04" | ||
| TENSORFLOW_VERSION="tensorflow-gpu==1.12.0" | ||
| TAG="eta-gpu-ubuntu-16.04-cuda-9.0" |
There was a problem hiding this comment.
The tagging here is a little confusing... all the information is in the repository name, and none of it in the image tag. Generally, images are versioned by tag, so you want something more like voxel51/eta:gpu-ubuntu-16.04-cuda-9.0. Also missing the ETA version, which I think would be helpful.
docker/Dockerfile
Outdated
| && ln -s /usr/bin/python3.6 /usr/local/bin/python \ | ||
| && curl https://bootstrap.pypa.io/get-pip.py | python | ||
|
|
||
| WORKDIR /dev |
There was a problem hiding this comment.
Here's your problem - /dev is a tmpfs: https://superuser.com/questions/675568/shrink-or-delete-udev-partition-in-ubuntu/675664#675664
Change the working directory something like /usr/src/app instead.
There was a problem hiding this comment.
AHHHHHHH. Thanks for catching
docker/build.bash
Outdated
| TAG=$3 | ||
|
|
||
| # Clone ETA | ||
| git clone https://github.com/voxel51/eta |
There was a problem hiding this comment.
Not sure this is desirable - the usual pattern is that the Dockerfile is in the root of the project directory and you copy the code in.
There was a problem hiding this comment.
thanks for the rec- updated
|
@ecrews updated and now works. Take a look! |
|
Doing a second pass now! |
|
@ecrews thanks for your help! If there's anything else on your wishlist, it would be nice to track via Issues |
This PR adds a script for building Docker images with ETA installed.