whole-tale / girder_wholetale

Girder plugin providing basic Whole Tale functionality

BSD 3-Clause "New" or "Revised" License

3 stars 5 forks source link

Added cullIdleInstances handler #485

Closed craig-willis closed 3 years ago

craig-willis commented 3 years ago

Fixes https://github.com/whole-tale/girder_wholetale/issues/477#

Test case

~Requires https://github.com/whole-tale/girder/pull/11 or wholetale/girder:heartbeat image based on that PR~
Requires https://github.com/whole-tale/deploy-dev/pull/49
Using girder-shell, temporarily set the idleTimeout on the JupyterLab image to 1 minute
```
docker exec -it <girder> bash
$ girder-shell
```

from girder.plugins.wholetale.models.image import Image image = Image().findOne({"name": "JupyterLab"}) image["idleTimeout"] = 1 Image().save(image)


* Create and run a JupyterLab tale
* Optionally tail girder logs (`docker exec -it <girder> tail -f  tail -f /home/girder/.girder/logs/info.log`)
   * `Culling idle instances message should appear every `heartbeat` seconds (5 min default)
* With IDE open in either iframe or popout, wait for 5 minutes
   * Confirm that instance is not stopped
* Select new tale tab (e.g., Metadata)  or browser tab, wait for 5 minutes
   * If tailing logs, note `Stopping instance X: idle timeout exceeded`
   * Confirm that instance is stopped
* Go to https://tmp-X.local.wholetale.org 
   * Confirm that custom 404 page is displayed

codecov[bot] commented 3 years ago

Codecov Report

Merging #485 (cc21756) into master (2093b96) will increase coverage by 0.36%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #485      +/-   ##
==========================================
+ Coverage   92.74%   93.11%   +0.36%     
==========================================
  Files          52       52              
  Lines        3930     3948      +18     
==========================================
+ Hits         3645     3676      +31     
+ Misses        285      272      -13

Impacted Files	Coverage Δ
server/__init__.py	`93.01% <100.00%> (+0.02%)`	:arrow_up:
server/models/image.py	`91.30% <100.00%> (+4.94%)`	:arrow_up:
server/models/instance.py	`86.41% <100.00%> (+1.18%)`	:arrow_up:
server/rest/image.py	`97.36% <100.00%> (+14.93%)`	:arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 2093b96...cc21756. Read the comment docs.

Xarthisius commented 3 years ago

It's missing the REST component (ability to add idleTimeout when creating Image via POST, or updating it via PUT). As such the change in #49 setting idleTimeout to 120m is basically noop. Otherwise LGTM!

craig-willis commented 3 years ago

Thanks for catching that. I think I've addressed it.