Well-defined Docker images and Make targets

[baldassarreFe]

At present, there is a bit of confusion between the Docker image and the Makefile. In short:

• the regular Dockerfile installs dev dependencies
• the Development-Dockerfile installs all that the regular does (note: the install commands are duplicated, we are not using the FROM command), plus Jupyter
• the run target in the Makefile relies on the openmined/pysyft image from DockerHub, but once the container is up, it tries to start a notebook server and fails
• the custom target does the exact same things as the run, with the difference that it accepts an image as a parameter, so that one can build his own dev image based on Development-Dockerfile (with Jupyter) and make it work

From my point of view (close-to-zero experience with this kind of deployments), we should have two images on DockerHub:

• openmined/pysyft that ships without dev dependencies (e.g. no gmp-dev and mpfr-dev) and with a working installation of PySyft, based either on a PyPi package or simply installed via git checkout tags/working-release && python3 setup.py install (this is to develop some application with PySyft)
• openmined/pysyft-dev that has all the additional dependencies and uses the src folder as source for the PySyft modules (this is to develop PySyft itself)

Regarding the Makefile, I suggest keeping the test and install targets docker agnostic, meaning that if they are run on the local machine they use what's available locally and if they are used inside the container they use the container:

install: 
    python3 setup.py install

test: 
   pytest && pytest --flake8

image = openmined/pysyft
docker: 
    docker run --it $(image) ...

image-dev = openmined/pysyft-dev
docker-dev: 
    docker run --it $(image-dev) ... jupyter notebook

Examples:

• run tests locally: make install && make test
• run tests in docker make docker-dev make test
• build a different image and use it with docker: docker build -f MyDockerfile -t MyImage && make docker image=MyImage

Provided that this structure will need refinement and additional details, does it make sense in general?
Can someone with experience suggest improvements?

[swaroopch]

Thanks for explaining the problem so clearly. I agree with most of your recommendations, one nit is to have simpler commands than make docker-dev make test which is confusing. We can follow different terminology like we do in https://github.com/OpenMined/PyYashe/blob/master/Makefile or we can use a Makefile rule such as:

(untested)

docker_%:
  docker run -i -t openmined/pyyashe make $(shell echo $@ | sed s/^docker_//) $(MAKEFLAGS)"

So that make docker_test becomes make test inside the Docker environment.

[FalkW]

Or we could just use docker-compose to create the image?

Basically what you want are two images:

#1 - dev - having: RUN ["pip3", "install", "jupyter"] in the dockerfile which gets started with: docker run --rm -it -v $(PWD)/notebooks:/notebooks -w /notebooks -p 8888:8888 openmined/pysyft jupyter notebook --ip=0.0.0.0 --allow-root

and

#2 without jupyter which should be started with: docker run --rm -it -v $(PWD)/notebooks:/notebooks -w /notebooks -p 8888:8888 openmined/pysyft notebook --ip=0.0.0.0 --allow-root
I assume.

By defining differences in the docker-compose.yml and passing the parameters during build it might be easier.

[baldassarreFe]

Hi, thanks for suggesting Docker Compose.

I have no experience with it so I'm not able to evaluate it. In case we only want to launch one container, what are the benefits of doing it with docker-compose instead of just running docker run?

Btw, in the second of your examples there still is notebook --ip=0.0.0.0 --allow-root, which is the list of parameters that you'd pass to the command jupyter. I think that the simplest command to run when not in dev mode is a bash shell.

I've opened a PR yesterday, please check out #270 and tell me what you think ;)

[FalkW]

Most likely it would work without Compose as well but I would definitely use an entrypoint script to parse ENV variables.

I'll try to finish my implementation today basically it would allow all beginners to build the image via:
docker-compose build << for an image without Jupyter
or
docker-compose build --build-arg INST_JUPYTER=true << for an image without Jupyter

To start the container all users would only have to execute:
docker-compose up

It's not finished yet but let's see how it turns out. My main goal it to get rid of two "Dockerfiles" to keep it clean.

[FalkW]

Okay it seems I got a little confused. Is there a reason why the Dockerfile doesn't have the RUN ["pip3", "install", "jupyter"] included? After looking a little deeper into the matter is seems it should be included in there as well since it is always used?.. This also explains why I was chasing an error in my code which is also in the current code:

docker: Error response from daemon: oci runtime error: container_linux.go:265: starting container process caused "exec: "jupyter": executable file not found in $PATH".
make: *** [run] Error 127

So having that in mind I completely agree with your commits for:
Development-Dockerfile --> no comments, referring to the base image makes sense

Dockerfile --> Thanks for the better formatting ;) but I think Jupyter needs to be included there as well if you want to call it in the Makefile, right?

[baldassarreFe]

Well, yes and no, I agree that now all the development is probably done through Jupyter and for this reason it's good to have an image with everything pre-installed. On the other hand, in the future, people will want to have a "production" version of PySyft that only contains the dependencies to make it run.

In other words, the dev image with Jupyter, Scipy and all the rest is for people who want to develop PySyft itself, the base image is for people that want to develop with PySyft.

Imagine (for absurd) that you want to deploy a container with numpy to make matrix computation available through a rest api, wouldn't it be weird to have Jupyter installed too?

That's why I kept the two Dockerfiles separated (I think Scipy should also be moved to the dev version) and the Docker target in the makefile simply opens a shell, from which one can easily invoke make notebook if that's his goal.

[FalkW]

@baldassarreFe you've got a point there, indeed. Just in general I'd think it is a better idea to go down to one Dockerfile to keep everything clean because as you said "... the Docker target in the makefile simply opens a shell, from which one can easily invoke make notebook if that's his goal."

But for now I'd suggest that your pull request is committed because it is way better than what is in place now (basically a not working docker implementation).

[baldassarreFe]

Closed with #270, please check if everything works as expected