Add option to shutdown VM from Datalab #1130
Conversation
| @@ -185,6 +186,21 @@ function handleRequest(request: http.ServerRequest, | |||
| return; | |||
| } | |||
|
|
|||
| if (path.indexOf('/_killvm') == 0) { | |||
There was a problem hiding this comment.
This should be "/_stopvm" rather than "/_killvm"
| try { | ||
| let vminfo = info.getVmInfo(); | ||
| childProcess.execSync( | ||
| 'gcloud compute instances stop ' + vminfo.VM_NAME + ' --zone ' + vminfo.VM_ZONE, |
There was a problem hiding this comment.
This should explicitly set the project rather than assuming that the current project (which the user can change) is the hosting project.
| @@ -185,6 +186,21 @@ function handleRequest(request: http.ServerRequest, | |||
| return; | |||
| } | |||
|
|
|||
| if (path.indexOf('/_killvm') == 0) { | |||
| if ('POST' != request.method) { | |||
There was a problem hiding this comment.
I think it would be cleaner if this whole block was moved to a helper method called something like "stopHandler"
| if (vminfo && vminfo.VM_NAME) { | ||
| dialogContent += '<hr><div>This Datalab instance is running from a Google Cloud VM: <b>' + vminfo.VM_NAME + '</b></span></div>' + | ||
| '<div><a href="javascript:manageVm()">Click here to manage the VM</a></div>' + | ||
| '<br><button id="killVmButton" title="Shutdown VM" class="btn btn-danger" onclick="killVm()">Shutdown VM</button>'; |
There was a problem hiding this comment.
Change "Shutdown VM" to "Stop VM".
|
Some tangential comments on this dialog... its a mix of fyi links and user actions. Can you put the ones that are user actions together (manage, vm details link) on top and fyi stuff below? Also, what is usage statistics? Why is it in the user's view? Restart server -- this wasn't meant to be exposed to users either. |
|
@nikhilk This is the info dialog though, this is why I think information should appear first. Eventually we should move this button to a settings pane anyway, it'll always feel out of place here. What do you think? Usage statistics is actually usage reporting opt in/out, I can rename it to "Usage reporting", or "Usage reporting preferences" |
yebrahim
force-pushed
the
yebrahim/vm-kill-button
branch
3 times, most recently
from
0dddf4c
to
663c8ce
Compare
yebrahim
force-pushed
the
yebrahim/vm-kill-button
branch
from
663c8ce
to
037d8a9
Compare
| @@ -160,7 +160,10 @@ function xhr(url, callback, method) { | |||
| method = method || "GET"; | |||
|
|
|||
| let request = new XMLHttpRequest(); | |||
| request.onreadystatechange = callback.bind(request); | |||
| request.onreadystatechange = function() { | |||
| if (request.readyState === 4 && request.status === 200) | |||
There was a problem hiding this comment.
This should have opening and closing brackets.
| @@ -25,6 +25,11 @@ then | |||
| . ~/startup.sh | |||
| fi | |||
|
|
|||
| # Get VM information if running on google cloud | |||
| machine_metadata_url="http://metadata.google.internal/computeMetadata/v1/instance" | |||
There was a problem hiding this comment.
Are these variables actually used anywhere?
| childProcess.execSync( | ||
| 'gcloud compute instances stop ' + vminfo.vm_name + | ||
| ' --project ' + vminfo.vm_project + ' --zone ' + vminfo.vm_zone, | ||
| {env: process.env}); |
There was a problem hiding this comment.
After the gcloud command, we should run 'sudo shutdown -P now' to make sure the VM shuts itself off.
There was a problem hiding this comment.
Can't do this inside of a docker container.
yebrahim
force-pushed
the
yebrahim/vm-kill-button
branch
from
5ab72ef
to
78022b4
Compare
|
error: Forever detected script exited with code: 0 With new image in gateway mode... |
|
@dovy I'm not able to reproduce that error (using the current release of the datalab and datalab-gateway images). Can you please file a separate issue with more details? |
|
@dovy When does this happen? Why is it related to this PR? The shutdown button shouldn't appear in the gateway mode. Please file a new issue with more details. |
|
Had to end up deleting my local image and specifying the full version. If I did a pull on local, but ran it without the :local-xxx it failed. Resolved. |
This PR adds a link in the profile menu to manage the VM hosting Datalab and a button to shut it down. The needed information is stored in environment variables that are computed in the startup script. The VM section appears only if Datalab is hosted inside a google cloud VM.