fix: properly display storage (#708)

* fix: do not use ephemeral storage in server responses - use proper storage numbers

* fix: use same format for memory and storage

* fix: use username in pvc name

* chore: fix telepresence script

Co-authored-by: Rok Roškar <[email protected]>

helm-chart/renku-notebooks/values.yaml

@@ -73,18 +73,12 @@ serverOptions:
     type: enum
     default: 1G
     options: [1G, 2G]
-  # Int values are also supported
-  # gpu_request:
-  #   displayName: Number of GPUs
-  #   type: int
-  #   default: 0
-  #   range: [0, 0]
-  # disk_request:
-  #   order: 4
-  #   displayName: Amount of disk space requested
-  #   type: enum
-  #   default: "1G"
-  #   options: ["1G", "10G"]
+  disk_request:
+    order: 4
+    displayName: Amount of storage
+    type: enum
+    default: "1G"
+    options: ["1G"]
     ## set allow_any_value to true to not enforce value checks
     ## arbitrary PV sizes
     # allow_any_value: true

renku_notebooks/api/classes/storage.py

@@ -6,7 +6,7 @@
 import re
 from urllib.parse import urlparse
 
-from ...util.kubernetes_ import get_k8s_client, make_server_name
+from ...util.kubernetes_ import get_k8s_client, make_pvc_name
 from ...util.file_size import parse_file_size
 
 
@@ -123,14 +123,12 @@ def __init__(self, *args, **kwargs):
         k8s_client, k8s_namespace = get_k8s_client()
         self.k8s_client = k8s_client
         self.k8s_namespace = k8s_namespace
-        self.name = (
-            make_server_name(
-                self.namespace,
-                self.project,
-                self.root_branch_name,
-                self.root_commit_sha,
-            )
-            + "-pvc"
+        self.name = make_pvc_name(
+            self.user.safe_username,
+            self.namespace,
+            self.project,
+            self.root_branch_name,
+            self.root_commit_sha,
         )
         self.creation_date = (
             None if not self.exists else self.pvc.metadata.creation_timestamp

renku_notebooks/api/schemas.py

@@ -155,7 +155,7 @@ class UserPodResources(
         {
             "cpu": fields.Str(required=True),
             "memory": fields.Str(required=True),
-            "ephemeral-storage": fields.Str(required=False),
+            "storage": fields.Str(required=False),
             "gpu": fields.Str(required=False),
         }
     )
@@ -223,7 +223,8 @@ def get_pod_status(pod):
             status.update(conditions_summary)
             return status
 
-        def get_pod_resources(pod):
+        def get_server_resources(server):
+            pod = server.pod
             try:
                 for container in pod.spec.containers:
                     if container.name == "notebook":
@@ -232,14 +233,38 @@ def get_pod_resources(pod):
                         # ref: https://kubernetes.io/docs/concepts/configuration/
                         #   manage-compute-resources-container/#how-pods-with-resource-limits-are-run
                         if (
-                            "cpu" in resources
+                            "cpu" in resources.keys()
                             and isinstance(resources["cpu"], str)
                             and str.endswith(resources["cpu"], "m")
                             and resources["cpu"][:-1].isdigit()
                         ):
                             resources["cpu"] = str(int(resources["cpu"][:-1]) / 1000)
+                        if "memory" in resources.keys():
+                            try:
+                                memory_formatted = (
+                                    str(round(int(resources["memory"]) / 1073741824))
+                                    + "G"
+                                )
+                            except ValueError:
+                                pass
+                            else:
+                                resources["memory"] = memory_formatted
             except (AttributeError, IndexError):
                 resources = {}
+            # add storage if PVCs are used
+            if server.session_pvc is not None and server.session_pvc.pvc is not None:
+                resources["storage"] = server.session_pvc.pvc.spec.resources.requests[
+                    "storage"
+                ]
+            # add storage if emptyDir is used and size limit is set
+            else:
+                volume_name = pod.metadata.name[:54] + "-git-repo"
+                volumes = [i for i in pod.spec.volumes if i.name == volume_name]
+                if len(volumes) == 1 and volumes[0].empty_dir.size_limit is not None:
+                    resources["storage"] = volumes[0].empty_dir.size_limit
+            # remove ephemeral-storage if present
+            if "ephemeral-storage" in resources.keys():
+                resources.pop("ephemeral-storage")
             return resources
 
         pod = server.pod
@@ -254,7 +279,7 @@ def get_pod_resources(pod):
             "started": pod.status.start_time,
             "status": get_pod_status(pod),
             "url": server.server_url,
-            "resources": get_pod_resources(pod),
+            "resources": get_server_resources(server),
             "image": server.image,
         }
 

renku_notebooks/util/kubernetes_.py

@@ -100,3 +100,13 @@ def make_server_name(namespace, project, branch, commit_sha):
     return "{project}-{hash}".format(
         project=project[:54], hash=md5(server_string.encode()).hexdigest()[:8]
     )
+
+
+def make_pvc_name(safe_username, namespace, project, branch, commit_sha):
+    """Form a 16-digit hash persistent volume ID."""
+    server_string = f"{safe_username}{namespace}{project}{branch}{commit_sha}"
+    return "{username}-{project}-{hash}".format(
+        username=safe_username[:10],
+        project=project[:44],
+        hash=md5(server_string.encode()).hexdigest()[:8],
+    )

run-telepresence.sh

@@ -55,8 +55,8 @@ export JUPYTERHUB_SERVICE_PREFIX
 export JUPYTERHUB_URL
 export FLASK_APP=`pwd`/renku_notebooks/wsgi.py
 export FLASK_DEBUG=0
-export NOTEBOOKS_SERVER_OPTIONS_DEFAULTS_PATH=`pwd`/tests/dummy_server_options_defaults.json
-export NOTEBOOKS_SERVER_OPTIONS_UI_PATH=`pwd`/tests/dummy_server_options_ui.json
+export NOTEBOOKS_SERVER_OPTIONS_DEFAULTS_PATH=`pwd`/tests/dummy_server_defaults.json
+export NOTEBOOKS_SERVER_OPTIONS_UI_PATH=`pwd`/tests/dummy_server_options.json
 
 echo ""
 echo "================================================================================================================="

tests/conftest.py

@@ -48,6 +48,7 @@
     "NOTEBOOKS_SERVER_OPTIONS_DEFAULTS_PATH"
 ] = "tests/dummy_server_defaults.json"
 os.environ["NOTEBOOKS_SERVER_OPTIONS_UI_PATH"] = "tests/dummy_server_options.json"
+os.environ["NOTEBOOKS_SESSION_PVS_ENABLED"] = "false"
 
 
 @pytest.fixture(scope="session", autouse=True)
@@ -337,7 +338,17 @@ def create_pod(username, server_name, payload):
                         }
                     },
                 }
-            ]
+            ],
+            "volumes": [
+                {
+                    "name": server_name + "-git-repo",
+                    "empty_dir": {
+                        "size_limit": payload.get("serverOptions", {}).get(
+                            "disk_request"
+                        )
+                    },
+                }
+            ],
         },
     }
 

tests/test_autosaves.py

@@ -22,7 +22,7 @@
 from unittest.mock import patch
 
 from renku_notebooks.api import config
-from renku_notebooks.util.kubernetes_ import make_server_name
+from renku_notebooks.util.kubernetes_ import make_pvc_name
 from tests.conftest import _AttributeDictionary, CustomList
 
 AUTHORIZED_HEADERS = {"Authorization": "token 8f7e09b3bf6b8a20"}
@@ -43,8 +43,9 @@ def _setup_pvcs(project, branches, commits, commited_datetime):
         mocker.patch("renku_notebooks.api.classes.user.User._get_pvcs").return_value = [
             V1PersistentVolumeClaim(
                 metadata=V1ObjectMeta(
-                    name=make_server_name("dummyuser", project, branches[i], commits[i])
-                    + "-pvc",
+                    name=make_pvc_name(
+                        "dummyuser", "dummyuser", project, branches[i], commits[i]
+                    ),
                     annotations={
                         config.RENKU_ANNOTATION_PREFIX + "projectName": project,
                         config.RENKU_ANNOTATION_PREFIX + "namespace": "dummyuser",
@@ -127,8 +128,9 @@ def test_autosaves_only_pvs(setup_pvcs, setup_project, patch_config, client):
                 "commit": "1235435",
                 "date": tstamp.isoformat(),
                 "pvs": True,
-                "name": make_server_name("dummyuser", "project", "branch1", "1235435")
-                + "-pvc",
+                "name": make_pvc_name(
+                    "dummyuser", "dummyuser", "project", "branch1", "1235435"
+                ),
             }
         ],
         "pvsSupport": True,
@@ -160,8 +162,9 @@ def test_autosaves_branches_pvs(setup_pvcs, setup_project, patch_config, client)
                 "commit": "1235435",
                 "date": tstamp.isoformat(),
                 "pvs": True,
-                "name": make_server_name("dummyuser", "project", "branch1", "1235435")
-                + "-pvc",
+                "name": make_pvc_name(
+                    "dummyuser", "dummyuser", "project", "branch1", "1235435"
+                ),
             },
             {
                 "branch": "branch2",