dashboards_server icon indicating copy to clipboard operation
dashboards_server copied to clipboard

Support /api/spawn for tmpnb

Open parente opened this issue 9 years ago • 15 comments

The current implementation assumes a standalone kernel gateway or notebook server. This is equivalent to tmpnb: false mode in thebe. It also needs to support the /api/spawn mode of operation to launch containers on tmpnb (or compatible) clusters.

Reason: tmpnb with its configurable-http-proxy has the only robust mechanism for reaping kernels around. A single kernel gateway is fine for testing, but it leaks kernels.

  • [ ] call /api/spawn on tmpnb to request a kernel gateway (make sure each client talks to its own kernel gateway instance)
  • [ ] support KERNEL_GATEWAY_URL and KERNEL_CLUSTER_URL properties (see comments)
  • [ ] support passing tmpnb auth token to /api/spawn

parente avatar Jan 18 '16 01:01 parente

This should mostly just require updates to the /public/js/kernel.js file. When we initially wrote the node app, we took the kernel.js from the thebe work and removed the spawn code (since we didn't need it at the time). So we could just look at what we had previously and add it back in.

jhpedemonte avatar Jan 18 '16 21:01 jhpedemonte

See https://github.com/jhpedemonte/dashboards/blob/0b05e9e914478230baa0e42888e9ac0f3143e2e7/urth/dashboard/converter/static/urth/dashboard.js for previous code which had call to /api/spawn.

jhpedemonte avatar Jan 18 '16 21:01 jhpedemonte

This should mostly just require updates to the /public/js/kernel.js file.

Does the logic for calling /api/spawn before start kernel really belong in the browser frontend where we have to deal with CORS, etc.? Or is it better to do that back in NodeJS as an intercept on POST /api/kernels or something so that the client is none the wiser?

parente avatar Jan 20 '16 17:01 parente

Yeah, we could move it to the backend as you suggest. I'm fine with that.

jhpedemonte avatar Jan 20 '16 18:01 jhpedemonte

I was wrong about the CORS though. We'd still be going through the nodejs proxy. But then the proxy would need to grow a /api/spawn endpoint which is a little weird since it's not a standard API.

I'm still leaning toward backend, but let's see what shakes out in the impl.

parente avatar Jan 20 '16 19:01 parente

As part of support /api/spawn, let's get the Makefile in this repo supporting a run-tmpnb-kernel-gateway mode (or something) where the kernel gateway image defined by Dockerfile.kernel is brought up in a small tmpnb cluster. This will make dev of this work item and #21 easier.

This is blocked by the 0.3.1 release of kernel gateway and the rebuild of the minimal-kernel image in docker-stacks for the root path fix. (jupyter-incubator/kernel_gateway#83)

parente avatar Jan 25 '16 16:01 parente

@nitind and I updated the Makefile to include targets for tmpnb (#24). We still need a 0.3.1 release of docker-stacks/minimal-kernel before you can hit the kernel_gateway endpoints once they're up and running. Once that lands, we'll update the git SHA of the docker hub image we're pulling.

jtyberg avatar Jan 25 '16 18:01 jtyberg

tmpnb provides an API_AUTH_TOKEN to protect the /api endpoints. When we enable tmpnb "mode", the app should also look for this token from the environment so it can pass it along.

jtyberg avatar Jan 25 '16 19:01 jtyberg

@jtyberg auth token support is issue #20

parente avatar Jan 25 '16 19:01 parente

KG 0.3.1 is now in jupyter/minimal-kernel:0017b56d93c9 and beyond.

parente avatar Jan 25 '16 22:01 parente

Instead of having a spawn or tmpnb or whatever boolean parameter here, I think we want to have two string parameters named after the concepts in the dashboards roadmap and security analysis:

  1. KERNEL_GATEWAY_URL points to a single jupyter-incubator/kernel_gateway instance or jupyter/notebook server.
  2. KERNEL_CLUSTER_URL points to a jupyter/tmpnb instance which spawns jupyter-incubator/kernel_gateway or jupyter/notebook containers on demand. If this option is set, KERNEL_GATEWAY_URL is ignored because the gateway URL is returned in the tmpnb /api/spawn response body (and is unique for every call to spawn.)

parente avatar Jan 25 '16 22:01 parente

Digging into the code more today, there's a problem with the current design in that there's one /api end point assumed to proxy to one KG backend. With tmpnb, there's going to be N kernel gateways, one per client (for now). The node app is going to have to track which client is talking to which KG.

Strawman: adopt the same mode tmpnb uses. Include a unique identifier in the /api path from the frontend JS to the backend JS. Let the backend route to the proper KG based on that path.

parente avatar Jan 26 '16 22:01 parente

We already supply a unique ID from the client (/public/js/kernel.js as the header X-jupyter-session-id) to the proxy app. We use this as a way to keep track of loaded notebooks by user. Could use same here.

jhpedemonte avatar Jan 27 '16 16:01 jhpedemonte

Discussed this again today. Decided to back-burner for now and get everything working with a single kernel gateway first. The primary sticking point is how to reap the kernels, but that might be easily solvable in the nodejs app (see #25) and can be improved later (see #31).

We'll leave the tmpnb automation that we added to the project in for now, but call out in the makefile that it's not working yet. We'll return to this later.

parente avatar Jan 27 '16 17:01 parente

A little update on this. The api.js now has a very explicit route for POST /kernels to handle data munging that we have to perform when the client JS requests a kernel like injecting auth information and setting the kernel name properly to match the notebook. We could conceivably inject whatever calls are necessary to spawn a KG within this handler as well before spawning the kernel itself. If we do so, I think it would need to be a plug-in point since tmpnb is just one of many potential KG managers with its own API.

parente avatar May 02 '16 12:05 parente