From e2febecb9e92b0e06a334ce3fefd37c6d610154f Mon Sep 17 00:00:00 2001
From: Samuel Veloso <svelosol@redhat.com>
Date: Mon, 27 Jun 2022 16:03:53 +0200
Subject: [PATCH] ODH Notebook Controller: Enable culling

---
 odh-notebook-controller/README.md             | 79 +++++++++++++++++--
 .../gen_kubeflow_manifests.sh                 |  2 +-
 odh-notebook-controller/gen_odh_manifests.sh  |  2 +-
 .../overlays/openshift/kustomization.yaml     |  2 +-
 .../openshift/manager_openshift_patch.yaml    | 18 +++++
 .../kf-notebook-controller/rbac/role.yaml     |  1 +
 .../base/kustomization.yaml                   |  2 +-
 tests/basictests/odh-notebook-controller.sh   |  2 +-
 .../kubeflow-jupyter-oauth-notebook.yaml      |  7 +-
 .../thoth-generic-oauth-notebook.yaml         | 12 +--
 .../notebooks/thoth-minimal-notebook.yaml     | 14 ++--
 .../thoth-minimal-oauth-notebook.yaml         | 14 ++--
 12 files changed, 120 insertions(+), 35 deletions(-)

diff --git a/odh-notebook-controller/README.md b/odh-notebook-controller/README.md
index be3e45bff..cd86f0a64 100644
--- a/odh-notebook-controller/README.md
+++ b/odh-notebook-controller/README.md
@@ -29,7 +29,8 @@ Add the following configuration to your `KfDef` object to install the
 Create a notebook object with the image and other parameters such as the
 environment variables, resource limits, tolerations, etc:
 
-```yaml
+```shell
+notebook_namespace=$(oc config view --minify -o jsonpath='{..namespace}')
 cat <<EOF | oc apply -f -
 ---
 apiVersion: kubeflow.org/v1
@@ -43,14 +44,16 @@ spec:
     spec:
       containers:
         - name: thoth-minimal-oauth-notebook
-          image: quay.io/thoth-station/s2i-minimal-notebook:v0.2.2
+          image: quay.io/thoth-station/s2i-minimal-notebook:v0.3.0
           imagePullPolicy: Always
           workingDir: /opt/app-root/src
           env:
-            - name: JUPYTER_NOTEBOOK_PORT
-              value: "8888"
             - name: NOTEBOOK_ARGS
-              value: "--NotebookApp.token='' --NotebookApp.password=''"
+              value: |
+                --ServerApp.port=8888
+                --ServerApp.token=''
+                --ServerApp.password=''
+                --ServerApp.base_url=/notebook/${notebook_namespace}/thoth-minimal-oauth-notebook
           ports:
             - name: notebook-port
               containerPort: 8888
@@ -63,14 +66,14 @@ spec:
               cpu: "1"
               memory: 1Gi
           livenessProbe:
-            initialDelaySeconds: 5
+            initialDelaySeconds: 10
             periodSeconds: 5
             timeoutSeconds: 1
             successThreshold: 1
             failureThreshold: 3
             httpGet:
               scheme: HTTP
-              path: /api
+              path: /notebook/${notebook_namespace}/thoth-minimal-oauth-notebook/api
               port: notebook-port
 EOF
 ```
@@ -78,11 +81,71 @@ EOF
 Open the notebook URL in your browser:
 
 ```shell
-firefox "$(oc get route thoth-minimal-oauth-notebook -o jsonpath='{.spec.host}')"
+firefox "$(oc get route thoth-minimal-oauth-notebook -o jsonpath='{.spec.host}')/notebook/${notebook_namespace}/thoth-minimal-oauth-notebook"
 ```
 
 Find more examples in the [notebook tests folder](../tests/resources/notebook-controller/).
 
+## Notebook Culling
+
+The notebook controller will scale to zero all the notebooks with last activity
+older than the idle time. The controller will set the
+`notebooks.kubeflow.org/last-activity` annotation when it detects a kernel with
+activity.
+
+To enable this feature, create a configmap with the culling configuration:
+
+- **ENABLE_CULLING**: Enable culling feature (false by default).
+- **IDLENESS_CHECK_PERIOD**: Polling frequency to update notebook last activity.
+- **CULL_IDLE_TIME**: Maximum time to scale notebook to zero if no activity.
+
+When the controller scales down the notebook pods, it will add the
+`kubeflow-resource-stopped` annotation. Remove this annotation to start the
+notebook server again.
+
+For example, poll notebooks activity every 5 minutes and shutdown those that
+have been in an idle state for more than 60 minutes:
+
+```yaml
+cat <<EOF | oc apply -f -
+---
+apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: notebook-controller-culler-config
+data:
+  ENABLE_CULLING: "true"
+  CULL_IDLE_TIME: "60" # In minutes (1 hour)
+  IDLENESS_CHECK_PERIOD: "5" # In minutes
+EOF
+```
+
+Restart the notebook controller deployment to refresh the configuration:
+
+```shell
+oc rollout restart deploy/notebook-controller-deployment
+```
+
+### Culling endpoint
+
+The notebook controller [polls the
+activity](https://github.com/kubeflow/kubeflow/blob/100657e8d1072136adf0a39315498b3d510c7c49/components/notebook-controller/pkg/culler/culler.go#L153-L155)
+from a specific path:
+
+```go
+url := fmt.Sprintf(
+    "http://%s.%s.svc.%s/notebook/%s/%s/api/kernels",
+    nm, ns, domain, ns, nm)
+```
+
+Make sure the notebook is exposing the kernels at this path by configuring the
+`base_url` parameter:
+
+```shell
+jupyter lab ... \
+  --ServerApp.base_url=/notebook/${nb_namespace}/${nb_name}
+```
+
 ## Updating Manifests
 
 The upstream code must be adapted before being deployed with the Opendatahub
diff --git a/odh-notebook-controller/gen_kubeflow_manifests.sh b/odh-notebook-controller/gen_kubeflow_manifests.sh
index 1419daa2b..d4ce84508 100755
--- a/odh-notebook-controller/gen_kubeflow_manifests.sh
+++ b/odh-notebook-controller/gen_kubeflow_manifests.sh
@@ -5,7 +5,7 @@ ctrl_dir="kf-notebook-controller"
 ctrl_repository="github.com/opendatahub-io/kubeflow"
 ctrl_branch="master"
 ctrl_image="quay.io/opendatahub/kubeflow-notebook-controller"
-ctrl_tag="1.6-343096a"
+ctrl_tag="1.6-55d78ca"
 ctrl_namespace="opendatahub"
 
 cleanup() {
diff --git a/odh-notebook-controller/gen_odh_manifests.sh b/odh-notebook-controller/gen_odh_manifests.sh
index 2eb101552..6c3bc419a 100755
--- a/odh-notebook-controller/gen_odh_manifests.sh
+++ b/odh-notebook-controller/gen_odh_manifests.sh
@@ -5,7 +5,7 @@ ctrl_dir="odh-notebook-controller"
 ctrl_repository="github.com/opendatahub-io/kubeflow"
 ctrl_branch="master"
 ctrl_image="quay.io/opendatahub/odh-notebook-controller"
-ctrl_tag="1.6-343096a"
+ctrl_tag="1.6-55d78ca"
 ctrl_namespace="opendatahub"
 
 cleanup() {
diff --git a/odh-notebook-controller/kf-notebook-controller/overlays/openshift/kustomization.yaml b/odh-notebook-controller/kf-notebook-controller/overlays/openshift/kustomization.yaml
index c37d2170f..ccf5b640c 100644
--- a/odh-notebook-controller/kf-notebook-controller/overlays/openshift/kustomization.yaml
+++ b/odh-notebook-controller/kf-notebook-controller/overlays/openshift/kustomization.yaml
@@ -11,7 +11,7 @@ commonLabels:
 images:
   - name: public.ecr.aws/j1r0q0g6/notebooks/notebook-controller
     newName: quay.io/opendatahub/kubeflow-notebook-controller
-    newTag: 1.6-343096a
+    newTag: 1.6-55d78ca
 configMapGenerator:
   - name: config
     behavior: merge
diff --git a/odh-notebook-controller/kf-notebook-controller/overlays/openshift/manager_openshift_patch.yaml b/odh-notebook-controller/kf-notebook-controller/overlays/openshift/manager_openshift_patch.yaml
index 586442c6e..8c548ea5f 100644
--- a/odh-notebook-controller/kf-notebook-controller/overlays/openshift/manager_openshift_patch.yaml
+++ b/odh-notebook-controller/kf-notebook-controller/overlays/openshift/manager_openshift_patch.yaml
@@ -13,6 +13,24 @@ spec:
                 configMapKeyRef:
                   name: config
                   key: ADD_FSGROUP
+            - name: ENABLE_CULLING
+              valueFrom:
+                configMapKeyRef:
+                  name: notebook-controller-culler-config
+                  key: ENABLE_CULLING
+                  optional: true
+            - name: CULL_IDLE_TIME
+              valueFrom:
+                configMapKeyRef:
+                  name: notebook-controller-culler-config
+                  key: CULL_IDLE_TIME
+                  optional: true
+            - name: IDLENESS_CHECK_PERIOD
+              valueFrom:
+                configMapKeyRef:
+                  name: notebook-controller-culler-config
+                  key: IDLENESS_CHECK_PERIOD
+                  optional: true
           resources:
             limits:
               cpu: 500m
diff --git a/odh-notebook-controller/kf-notebook-controller/rbac/role.yaml b/odh-notebook-controller/kf-notebook-controller/rbac/role.yaml
index 51b6e04fb..7273f3abb 100644
--- a/odh-notebook-controller/kf-notebook-controller/rbac/role.yaml
+++ b/odh-notebook-controller/kf-notebook-controller/rbac/role.yaml
@@ -19,6 +19,7 @@ rules:
   - create
   - get
   - list
+  - patch
   - watch
 - apiGroups:
   - ""
diff --git a/odh-notebook-controller/odh-notebook-controller/base/kustomization.yaml b/odh-notebook-controller/odh-notebook-controller/base/kustomization.yaml
index 58671648f..30ab88c86 100644
--- a/odh-notebook-controller/odh-notebook-controller/base/kustomization.yaml
+++ b/odh-notebook-controller/odh-notebook-controller/base/kustomization.yaml
@@ -6,4 +6,4 @@ resources:
 images:
   - name: quay.io/opendatahub/odh-notebook-controller
     newName: quay.io/opendatahub/odh-notebook-controller
-    newTag: 1.6-343096a
+    newTag: 1.6-55d78ca
diff --git a/tests/basictests/odh-notebook-controller.sh b/tests/basictests/odh-notebook-controller.sh
index f5e2e133b..76b33bad8 100644
--- a/tests/basictests/odh-notebook-controller.sh
+++ b/tests/basictests/odh-notebook-controller.sh
@@ -67,7 +67,7 @@ function test_notebook_creation() {
     # Verify notebook is reachable trough the Openshift Route
     os::cmd::try_until_text "oc get route ${notebook_name}" "${notebook_name}" $odhdefaulttimeout $odhdefaultinterval
     local notebook_host="$(oc get route ${notebook_name} -o jsonpath='{.spec.host}')"
-    os::cmd::try_until_text "curl -k -s -o /dev/null -w \"%{http_code}\" https://${notebook_host}/api" "200" $odhdefaulttimeout $odhdefaultinterval
+    os::cmd::try_until_text "curl -k -s -o /dev/null -w \"%{http_code}\" https://${notebook_host}/notebook/${ODHPROJECT}/${notebook_name}/api" "200" $odhdefaulttimeout $odhdefaultinterval
 }
 
 function test_notebook_deletion() {
diff --git a/tests/resources/notebook-controller/notebooks/kubeflow-jupyter-oauth-notebook.yaml b/tests/resources/notebook-controller/notebooks/kubeflow-jupyter-oauth-notebook.yaml
index 667e97afd..83859a8d8 100644
--- a/tests/resources/notebook-controller/notebooks/kubeflow-jupyter-oauth-notebook.yaml
+++ b/tests/resources/notebook-controller/notebooks/kubeflow-jupyter-oauth-notebook.yaml
@@ -16,9 +16,6 @@ spec:
         - name: kubeflow-jupyter-oauth-notebook
           image: public.ecr.aws/j1r0q0g6/notebooks/notebook-servers/jupyter-scipy:v1.5.0
           imagePullPolicy: Always
-          env:
-            - name: NB_PREFIX
-              value: /
           ports:
             - name: notebook-port
               containerPort: 8888
@@ -31,14 +28,14 @@ spec:
               cpu: "1"
               memory: 1Gi
           livenessProbe:
-            initialDelaySeconds: 5
+            initialDelaySeconds: 10
             periodSeconds: 5
             timeoutSeconds: 1
             successThreshold: 1
             failureThreshold: 3
             httpGet:
               scheme: HTTP
-              path: /api
+              path: /notebook/opendatahub/kubeflow-jupyter-oauth-notebook/api
               port: notebook-port
 
 ---
diff --git a/tests/resources/notebook-controller/notebooks/thoth-generic-oauth-notebook.yaml b/tests/resources/notebook-controller/notebooks/thoth-generic-oauth-notebook.yaml
index 2c6402a6d..a9814171e 100644
--- a/tests/resources/notebook-controller/notebooks/thoth-generic-oauth-notebook.yaml
+++ b/tests/resources/notebook-controller/notebooks/thoth-generic-oauth-notebook.yaml
@@ -14,10 +14,12 @@ spec:
           imagePullPolicy: Always
           workingDir: /opt/app-root/src
           env:
-            - name: JUPYTER_NOTEBOOK_PORT
-              value: "8888"
             - name: NOTEBOOK_ARGS
-              value: "--NotebookApp.token='' --NotebookApp.password=''"
+              value: |
+                --ServerApp.port=8888
+                --ServerApp.token=''
+                --ServerApp.password=''
+                --ServerApp.base_url=/notebook/opendatahub/thoth-generic-oauth-notebook
           ports:
             - name: notebook-port
               containerPort: 8888
@@ -30,12 +32,12 @@ spec:
               cpu: "1"
               memory: 1Gi
           livenessProbe:
-            initialDelaySeconds: 5
+            initialDelaySeconds: 10
             periodSeconds: 5
             timeoutSeconds: 1
             successThreshold: 1
             failureThreshold: 3
             httpGet:
               scheme: HTTP
-              path: /api
+              path: /notebook/opendatahub/thoth-generic-oauth-notebook/api
               port: notebook-port
diff --git a/tests/resources/notebook-controller/notebooks/thoth-minimal-notebook.yaml b/tests/resources/notebook-controller/notebooks/thoth-minimal-notebook.yaml
index 077ef2782..a8222198e 100644
--- a/tests/resources/notebook-controller/notebooks/thoth-minimal-notebook.yaml
+++ b/tests/resources/notebook-controller/notebooks/thoth-minimal-notebook.yaml
@@ -8,14 +8,16 @@ spec:
     spec:
       containers:
         - name: thoth-minimal-notebook
-          image: quay.io/thoth-station/s2i-minimal-notebook:v0.2.2
+          image: quay.io/thoth-station/s2i-minimal-notebook:v0.3.0
           imagePullPolicy: Always
           workingDir: /opt/app-root/src
           env:
-            - name: JUPYTER_NOTEBOOK_PORT
-              value: "8888"
             - name: NOTEBOOK_ARGS
-              value: "--NotebookApp.token='' --NotebookApp.password=''"
+              value: |
+                --ServerApp.port=8888
+                --ServerApp.token=''
+                --ServerApp.password=''
+                --ServerApp.base_url=/notebook/opendatahub/thoth-minimal-notebook
           ports:
             - name: notebook-port
               containerPort: 8888
@@ -28,12 +30,12 @@ spec:
               cpu: "1"
               memory: 1Gi
           livenessProbe:
-            initialDelaySeconds: 5
+            initialDelaySeconds: 10
             periodSeconds: 5
             timeoutSeconds: 1
             successThreshold: 1
             failureThreshold: 3
             httpGet:
               scheme: HTTP
-              path: /api
+              path: /notebook/opendatahub/thoth-minimal-notebook/api
               port: notebook-port
diff --git a/tests/resources/notebook-controller/notebooks/thoth-minimal-oauth-notebook.yaml b/tests/resources/notebook-controller/notebooks/thoth-minimal-oauth-notebook.yaml
index 13f73180c..eca5681c5 100644
--- a/tests/resources/notebook-controller/notebooks/thoth-minimal-oauth-notebook.yaml
+++ b/tests/resources/notebook-controller/notebooks/thoth-minimal-oauth-notebook.yaml
@@ -10,14 +10,16 @@ spec:
     spec:
       containers:
         - name: thoth-minimal-oauth-notebook
-          image: quay.io/thoth-station/s2i-minimal-notebook:v0.2.2
+          image: quay.io/thoth-station/s2i-minimal-notebook:v0.3.0
           imagePullPolicy: Always
           workingDir: /opt/app-root/src
           env:
-            - name: JUPYTER_NOTEBOOK_PORT
-              value: "8888"
             - name: NOTEBOOK_ARGS
-              value: "--NotebookApp.token='' --NotebookApp.password=''"
+              value: |
+                --ServerApp.port=8888
+                --ServerApp.token=''
+                --ServerApp.password=''
+                --ServerApp.base_url=/notebook/opendatahub/thoth-minimal-oauth-notebook
           ports:
             - name: notebook-port
               containerPort: 8888
@@ -30,12 +32,12 @@ spec:
               cpu: "1"
               memory: 1Gi
           livenessProbe:
-            initialDelaySeconds: 5
+            initialDelaySeconds: 10
             periodSeconds: 5
             timeoutSeconds: 1
             successThreshold: 1
             failureThreshold: 3
             httpGet:
               scheme: HTTP
-              path: /api
+              path: /notebook/opendatahub/thoth-minimal-oauth-notebook/api
               port: notebook-port