Add resources for ope-notebook-culler cronjob #3

dystewart · 2024-01-18T20:02:24Z

Addresses: nerc-project/operations#337

This PR adds:

a cronjob to delete notebooks in the rhods-notebooks namespace that have been running for 48 hours straight or more.
the docker resources for the cronjob
updates to the README.md to include new cronjob

cronjobs/ope-notebook-culler/cronjob.yaml

DanNiESh · 2024-01-19T14:17:15Z

One another minor suggestion, can you add the steps to run this cronjob in the readme file?

dystewart · 2024-01-19T19:50:01Z

Also updated README.md

IsaiahStapleton

Looks good

README.md

DanNiESh · 2024-01-22T13:27:10Z

docker/src/ope-notebook-culler.sh

+echo $(date)
+
+for notebook in $running_notebooks; do
+	oc delete notebook.kubeflow.org $notebook


I think this comment won't delete the pvcs connected to the notebooks. When the same student relaunch a notebook, will that pvc be automatically assigned to the user?

Now I think about it, oc delete command will equally kill all notebooks no matter how long they have been running. One user may just start a notebook 1 minute before. Then the cronjob kills it immediately. That's not what we expect. Can you make the cronjob job run every 24 hours, and then only kill notebooks that are running more than 48 hours?

Both of these assumptions are correct, the pvc does need to be deleted and we don't want to delete notebooks that have been running less than 48 hours. The script I had before checked the running time of the notebook and deleted the pvc and notebook, maybe that can be used, I will copy it below:

`
#!/bin/sh

set_time=24 hours time_limit=$(($(date -u +"%s") - $(date -d '-$set_time' +%s))) running_notebooks=$(oc get notebook -o custom-columns=:metadata.name) for notebook in $running_notebooks; do start_time=$(oc get notebook $notebook -o jsonpath='{.status.containerState.running.startedAt}') running_time=$(($(date -u +"%s") - $(date -u -d "$start_time" +"%s"))) if [ "$running_time" -gt "$time_limit" ]; then echo "Shutting down pod $notebook" oc delete notebook $notebook oc delete pvc $notebook fi done

`

@dystewart is it necessary to delete the whole notebook, or can the StatefulSet be scaled to 0 instead?

@computate If I'm not mistaken, when you scale the StatefulSet to 0 the pod is terminated but then starts back up again. That's why we delete the whole notebook because it ensures the notebook, statefulset and pod are actually deleted.

computate · 2024-01-22T15:42:46Z

@dystewart can you please provide a description for this PR, and link it to an issue? Is this related to nerc-project/operations#337?

dystewart · 2024-01-23T03:19:15Z

Updated description and linked associated issue

dystewart · 2024-01-23T03:23:29Z

Also made requested updates to code @DanNiESh

README.md

cronjobs/ope-notebook-culler/cronjob.yaml

computate · 2024-01-23T16:01:33Z

Do you think that we can adjust the PR to "shut down" the notebooks in the rhods-notebooks namespace rather than delete them as the issue nerc-project/operations#337 describes, based on the sample culler code provided in Slack?

computate

Can we please adjust this PR to only affect certain classes and not CS506? See the Ensure that notebooks in Lance Galletti's CS506 course are not deleted and have a longer uptime issue for more details.

DanNiESh · 2024-01-23T16:53:13Z

Can we please adjust this PR to only affect certain classes and not CS506? See the Ensure that notebooks in Lance Galletti's CS506 course are not deleted and have a longer uptime issue for more details.

Dylan is also working on another issue to distinguish users in rhods-notebooks to their course group. Once that is completed, we can customize this cronjob to only apply to cs210 and ece440 classes which I think could be a follow-up PR if we don't want this PR to be blocked by that issue.

computate · 2024-01-23T17:32:19Z

@DanNiESh as this PR currently is configured to delete all notebooks in the rhods-notebooks namespace, including the CS506 notebooks which are required to be available all semester, I consider it a blocker if Lance Galletti is not onboard with the changes that will be applied to his notebooks in this PR.

DanNiESh · 2024-01-23T17:51:50Z

@computate Gotcha! Once Dylan updates the PR to "shutdown notebooks" rather than delete them, we'll test and verify it in ope-testing namespace. Do you think "shutdown" notebooks works for Lance Galletti? Ultimately we will modify this cronjob to only apply to cs210 and ece440 classes once this issue is resolved.

File formatting to make git happy Formatting

dystewart · 2024-01-24T00:36:09Z

@computate @DanNiESh I have updated the cronjob to utilize the code we were provided and tested in ope-rhods-testing and confirmed its working. @DanNiESh will confirm

DanNiESh · 2024-01-24T13:50:43Z

Looks good! I tested and verified this cronjob worked. Some bugs I got during testing the README and cronjob:

I noticed that you changed the directory name to cronjobs/notebok-culler, is notebok-culler a typo?
When I try to run kubectl create -n ope-rhods-testing-1fef2f job --from=cronjob/notebok-culler nb-culler, I got the error Error from server (NotFound): cronjobs.batch "notebok-culler" not found

DanNiESh · 2024-01-24T13:21:09Z

cronjobs/notebok-culler/cronjob.yaml

+                  #threshold to stop running notebooks. Currently set to 24 hours
+                  cutoff_time=86400
+                  current_time=$(date +%s)
+                  notebooks=$(oc get notebooks -n ope-rhods-testing-1fef2f -o jsonpath='{range .items[?(@.status.containerState.running)]}{.metadata.name}{" "}{.metadata.namespace}{" "}{.status.containerState.running.startedAt}{"\n"}{end}')


This should be rhods-notebooks rather than ope-rhods-testing-1fef2f

DanNiESh · 2024-01-24T13:21:19Z

docker/src/ope-notebook-culler.sh

+#threshold to stop running notebooks. Currently set to 24 hours
+cutoff_time=86400
+current_time=$(date +%s)
+notebooks=$(oc get notebooks -n ope-rhods-testing-1fef2f -o jsonpath='{range .items[?(@.status.containerState.running)]}{.metadata.name}{" "}{.metadata.namespace}{" "}{.status.containerState.running.startedAt}{"\n"}{end}')


Same. Not ope-rhods-testing-1fef2f

DanNiESh · 2024-01-24T13:23:25Z

docker/Dockerfile

@@ -0,0 +1,6 @@
+FROM quay.io/operate-first/opf-toolbox:v0.8.0


Is this Dockerfile still needed?

DanNiESh · 2024-01-24T13:34:52Z

docker/src/ope-notebook-culler.sh

@@ -0,0 +1,24 @@
+#!/bin/bash


Is this script being used anywhere?

DanNiESh · 2024-01-24T13:40:02Z

README.md

+1. Ensure you followed the steps above
+2. Verify the cronjob ope-notebook-culler exists
+```
+	oc get cronjob ope-notebook-culler


The new cronjob name is nb-culler. So the comand should be changed tooc get cronjob nb-culler

DanNiESh · 2024-01-24T13:40:15Z

README.md

+
+3. Run:
+```
+	kubectl create -n rhods-notebooks job --from=cronjob/ope-notebook-culler ope-notebook-culler


same. nb-culler

DanNiESh · 2024-01-24T13:51:25Z

README.md

+Alternatively, to run the script immediately: 
+
+1. Ensure you followed the steps above
+2. Verify the cronjob ope-notebook-culler exists


nb-culler

DanNiESh · 2024-01-24T13:52:02Z

README.md

+
+### ope-notebook-culler
+
+This cronjob runs once every 24 hours at 7am, removing all notebooks & pvcs from the rhods-notebooks namespace. To add resources to the rhods-notebooks namespace:


Now we just shutdown notebooks and pvcs

DanNiESh · 2024-01-24T13:52:27Z

README.md

+	oc project rhods-notebooks
+```
+
+3. From cronjobs/ope-notebook-culler/ directory run:


The directory name has changed.

Also removes unused docker resources after switching cronjob logic to mirror redhat internal's Updates README.md to reflect all these changes

DanNiESh · 2024-01-31T14:19:16Z

@dystewart Can you update the script to shutdown notebooks in specific classes and also resolve the commit conflicts?

DanNiESh self-requested a review January 19, 2024 13:17

DanNiESh requested changes Jan 19, 2024

View reviewed changes

cronjobs/ope-notebook-culler/cronjob.yaml Outdated Show resolved Hide resolved

cronjobs/ope-notebook-culler/cronjob.yaml Outdated Show resolved Hide resolved

cronjobs/ope-notebook-culler/cronjob.yaml Outdated Show resolved Hide resolved

dystewart force-pushed the cron branch from 9952d9c to 7bcfa9d Compare January 19, 2024 19:48

dystewart requested review from DanNiESh and IsaiahStapleton January 19, 2024 19:50

IsaiahStapleton approved these changes Jan 19, 2024

View reviewed changes

DanNiESh reviewed Jan 22, 2024

View reviewed changes

dystewart force-pushed the cron branch from 7bcfa9d to 90113c6 Compare January 23, 2024 03:19

DanNiESh reviewed Jan 23, 2024

View reviewed changes

README.md Outdated Show resolved Hide resolved

cronjobs/ope-notebook-culler/cronjob.yaml Outdated Show resolved Hide resolved

dystewart force-pushed the cron branch from 90113c6 to a823c62 Compare January 23, 2024 14:27

computate mentioned this pull request Jan 23, 2024

Ensure that notebooks in Lance Galletti's CS506 course are not deleted and have a longer uptime nerc-project/operations#404

Closed

computate suggested changes Jan 23, 2024

View reviewed changes

Add docker resources underlying toolkit container image

d2bbdac

dystewart force-pushed the cron branch from b6f738e to 68415fe Compare January 23, 2024 22:41

dystewart added 2 commits January 23, 2024 17:44

Add resources for ope-notebook-culler cronjob

8a34990

File formatting to make git happy Formatting

Update README.md to include instructions for running ope-notebook-culler

d70cc73

dystewart force-pushed the cron branch from 68415fe to d70cc73 Compare January 23, 2024 22:44

DanNiESh requested changes Jan 24, 2024

View reviewed changes

Renames cronjob and makes naming consistent across cronjob resources

69e730f

Also removes unused docker resources after switching cronjob logic to mirror redhat internal's Updates README.md to reflect all these changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add resources for ope-notebook-culler cronjob #3

Add resources for ope-notebook-culler cronjob #3

dystewart commented Jan 18, 2024 •

edited

Loading

DanNiESh commented Jan 19, 2024

dystewart commented Jan 19, 2024

IsaiahStapleton left a comment

DanNiESh Jan 22, 2024

DanNiESh Jan 22, 2024

IsaiahStapleton Jan 22, 2024

computate Jan 22, 2024

IsaiahStapleton Jan 22, 2024

computate commented Jan 22, 2024

dystewart commented Jan 23, 2024

dystewart commented Jan 23, 2024

computate commented Jan 23, 2024

computate left a comment

DanNiESh commented Jan 23, 2024

computate commented Jan 23, 2024 •

edited

Loading

DanNiESh commented Jan 23, 2024

dystewart commented Jan 24, 2024

DanNiESh commented Jan 24, 2024 •

edited

Loading

DanNiESh Jan 24, 2024

DanNiESh Jan 24, 2024

DanNiESh Jan 24, 2024

DanNiESh Jan 24, 2024

DanNiESh Jan 24, 2024

DanNiESh Jan 24, 2024

DanNiESh Jan 24, 2024

DanNiESh Jan 24, 2024

DanNiESh Jan 24, 2024

DanNiESh commented Jan 31, 2024

		@@ -0,0 +1,6 @@
		FROM quay.io/operate-first/opf-toolbox:v0.8.0


		### ope-notebook-culler

		This cronjob runs once every 24 hours at 7am, removing all notebooks & pvcs from the rhods-notebooks namespace. To add resources to the rhods-notebooks namespace:

Add resources for ope-notebook-culler cronjob #3

Are you sure you want to change the base?

Add resources for ope-notebook-culler cronjob #3

Conversation

dystewart commented Jan 18, 2024 • edited Loading

DanNiESh commented Jan 19, 2024

dystewart commented Jan 19, 2024

IsaiahStapleton left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

computate commented Jan 22, 2024

dystewart commented Jan 23, 2024

dystewart commented Jan 23, 2024

computate commented Jan 23, 2024

computate left a comment

Choose a reason for hiding this comment

DanNiESh commented Jan 23, 2024

computate commented Jan 23, 2024 • edited Loading

DanNiESh commented Jan 23, 2024

dystewart commented Jan 24, 2024

DanNiESh commented Jan 24, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DanNiESh commented Jan 31, 2024

dystewart commented Jan 18, 2024 •

edited

Loading

computate commented Jan 23, 2024 •

edited

Loading

DanNiESh commented Jan 24, 2024 •

edited

Loading