jupyter / tmpnb

Creates temporary Jupyter Notebook servers using Docker containers. [DEPRECATED - See BinderHub project]
https://github.com/jupyterhub/binderhub
BSD 3-Clause "New" or "Revised" License
528 stars 123 forks source link

Admin server with DELETE /api/pool handler #198

Closed parente closed 8 years ago

parente commented 8 years ago

Use case: I want to docker pull or docker build a new container image on the tmpnb box. I don't want containers already in the pool based on the old image to linger indefinitely until they are used up. I want to immediately drain the pool of idle, available containers and replenish the pool with containers from the new image. While doing so, I do not want to disturb containers that are currently in-use. In practice, I should be able to do something as simple as:

eval `docker-machine env my-tmpnb-host`
docker build -t my-notebook-image .
docker exec -i tmpnb-pool curl -X "DELETE" http://127.0.0.1:10000/api/pool

to get the desired effect.

Since pool draining is a costly operation, this PR adds a new "admin" server that listens on a port separate from the main tmpnb server. The admin server only listens on localhost by default and supports an optional ADMIN_AUTH_TOKEN env var as a shared-secret client authorization mechanism.

The admin server currently implements a single handler for DELETE /api/pool that implements the behavior described above. However, it can grow to support other admin operations in the future (e.g., PUT to change config settings like total number containers in the pool).

I'm open to other verbs and resource names. DELETE /api/pool was the first thing that came to mind.

captainsafia commented 8 years ago

Neat-o. You might want to add a note about configuring the ADMIN_AUTH_TOKEN env var to the qucikstart section in the docs, too. Code comments, inline.

rgbkrk commented 8 years ago

Awesome, thanks for doing this @parente!

rgbkrk commented 8 years ago

/api/pool is fine. There's only one pool with a single tmpnb node, so it's the resource.

parente commented 8 years ago

I'll push one more commit later today with the README additions. Then it should be GTG. Thanks for the quick reviews!

parente commented 8 years ago

Updated the README.

captainsafia commented 8 years ago

Looks good to me.

I will leave merging to our grown up-in-residence, @rgbkrk.

rgbkrk commented 8 years ago

Ha, you can merge too @captainsafia

parente commented 8 years ago

DH build is failing. So either I broke it by de-duping the requirements install step in the Dockerfile or DH is just having issues.

Running make tmpnb-image again locally works fine so I'm guessing it's the latter. Will keep an eye on it.

parente commented 8 years ago

After 3 failed attempts with no useful info in the log as to the failure, I tickled the Dockerfile a bit based on the hunch that maybe the earlier version of Docker used on DH for builds was getting tripped up by the ADD to a path without a trailing slash. Switched to using the safer COPY directive and made the path more explicit in 8a35b355147afd76d52ba601d22cb4245a7d07cb. Build now succeeds.