jwa: Rework the Storage API of the web app (kubeflow#6321)

* wa(back): Add helper for deserializing JSON obj

In some cases we might need to construct Python k8s lib objects from the
JSONs that are provided by clients. I.e. the UI will be sending a PVC
object in json format, so the backend will need to create the
corresponding client.V1PersistentVolumeClaim object and submit it.

Signed-off-by: Kimonas Sotirchos <[email protected]>
Reviewed-by: Ilias Katsakioris <[email protected]>

* wa(back): Serialization helper

Add helper function for converting a k8s-client object into a dict that
can be sent as an HTTP response.

Signed-off-by: Kimonas Sotirchos <[email protected]>
Reviewed-by: Ilias Katsakioris <[email protected]>

* wa(back): Add dry run to Notebooks and PVCs

The backend will need to be able to create objects with dry-run, in
order to ensure they are valid. The backend will need to check that both
the Notebook and the PVCs can be created beforehand.

This way we avoid the scenario where we create PVCs but the Notebook
fails to be created, and the PVCs are never garbage collected.

Signed-off-by: Kimonas Sotirchos <[email protected]>
Reviewed-by: Ilias Katsakioris <[email protected]>

* wa(back): Update kubernetes to 0.17

In order to support dry-run we must use the 0.17 version of the Python
k8s client.

Signed-off-by: Kimonas Sotirchos <[email protected]>
Reviewed-by: Ilias Katsakioris <[email protected]>

* wa(back): Extend api module to patch pvcs

The backend will need to be able to PATCH PVCs in order to set the
ownerReference to the Notebook that mounts the PVCs.

Ref: arrikto/dev/issues/386#issuecomment-856700392

Signed-off-by: Kimonas Sotirchos <[email protected]>
Reviewed-by: Ilias Katsakioris <[email protected]>

* jwa(back): Work with new Volumes API

The backend API should not add any more layers of abstractions on top of
the K8s API. The backend should expect the client/UI to be sending the
entire PVC spec of a new PVC.

Refs: arrikto/dev/issues/386

Signed-off-by: Kimonas Sotirchos <[email protected]>
Reviewed-by: Ilias Katsakioris <[email protected]>

* jwa(back): Add unittests for new volumes API

Signed-off-by: Kimonas Sotirchos <[email protected]>
Reviewed-by: Ilias Katsakioris <[email protected]>

* jwa(back): Extend the PVC info returned

We want to show both the access mode and size of the existing PVCs, when
a user clicks on the dropdown to select which PVC to mount.

The backend will need to provide this information to the frontend. We
don't want to send the K8s list of PVCs since this will result in a lot
of unnecessary data to be sent.

Signed-off-by: Kimonas Sotirchos <[email protected]>
Reviewed-by: Ilias Katsakioris <[email protected]>

* jwa(front): Add proxy config for Rok

When developing the Rok flavor locally we will need to be able to open
the Rok chooser. This can be done by using Angular/webpack proxy to
bring the exposed rok service and the app under the same domain.

Signed-off-by: Kimonas Sotirchos <[email protected]>
Reviewed-by: Tasos Alexiou <[email protected]>

* jwa(front): Remove card from form

The form of the app should not be a big card, but a normal form.

Signed-off-by: Kimonas Sotirchos <[email protected]>
Reviewed-by: Tasos Alexiou <[email protected]>

* jwa(front): Install AceModule for yaml editing

Install AceModule to allow users to edit yamls of objects.

Signed-off-by: Kimonas Sotirchos <[email protected]>
Reviewed-by: Tasos Alexiou <[email protected]>

* wa(front): Change the styling of form sections

Signed-off-by: Kimonas Sotirchos <[email protected]>
Reviewed-by: Tasos Alexiou <[email protected]>

* jwa(front): Create common volume components

Component for:
* New PVC and configuring its spec
* Attaching an existing PVC in a Notebook

Signed-off-by: Kimonas Sotirchos <[email protected]>
Reviewed-by: Tasos Alexiou <[email protected]>

* jwa(front): Update Rok form for new Volume API

Signed-off-by: Kimonas Sotirchos <[email protected]>
Reviewed-by: Tasos Alexiou <[email protected]>

* jwa(front): Mark inputs as dirty when restoring Lab

When the UI autofills the form with values from a JupyterLab snapshot
then it should mark the touched fields as dirty. This way if a field has
errors the UI will make that input red.

Signed-off-by: Kimonas Sotirchos <[email protected]>
Reviewed-by: Tasos Alexiou <[email protected]>

* jwa: Update ConfigMap in manifests

Signed-off-by: Kimonas Sotirchos <[email protected]>

* jwa(front): Fix format errors

Signed-off-by: Kimonas Sotirchos <[email protected]>

components/crud-web-apps/common/backend/kubeflow/kubeflow/crud_backend/api/custom_resource.py

@@ -10,15 +10,12 @@ def create_custom_rsrc(group, version, kind, data, namespace):
                                                       namespace, kind, data)
 
 
-def delete_custom_rsrc(group, version, kind, name, namespace, foreground=True):
-    del_policy = client.V1DeleteOptions()
-    if foreground:
-        del_policy = client.V1DeleteOptions(propagation_policy="Foreground")
-
+def delete_custom_rsrc(group, version, kind, name, namespace,
+                       policy="Foreground"):
     authz.ensure_authorized("delete", group, version, kind, namespace)
-    return custom_api.delete_namespaced_custom_object(group, version,
-                                                      namespace, kind, name,
-                                                      del_policy)
+    return custom_api.delete_namespaced_custom_object(
+        group, version, namespace, kind, name, propagation_policy=policy
+    )
 
 
 def list_custom_rsrc(group, version, kind, namespace):

components/crud-web-apps/common/backend/kubeflow/kubeflow/crud_backend/api/notebook.py

@@ -1,5 +1,3 @@
-from kubernetes import client
-
 from .. import authz
 from . import custom_api, v1_core
 
@@ -13,13 +11,14 @@ def get_notebook(notebook, namespace):
     )
 
 
-def create_notebook(notebook, namespace):
+def create_notebook(notebook, namespace, dry_run=False):
     authz.ensure_authorized(
         "create", "kubeflow.org", "v1beta1", "notebooks", namespace
     )
+
     return custom_api.create_namespaced_custom_object(
-        "kubeflow.org", "v1beta1", namespace, "notebooks", notebook
-    )
+        "kubeflow.org", "v1beta1", namespace, "notebooks", notebook,
+        dry_run="All" if dry_run else None)
 
 
 def list_notebooks(namespace):
@@ -36,12 +35,12 @@ def delete_notebook(notebook, namespace):
         "delete", "kubeflow.org", "v1beta1", "notebooks", namespace
     )
     return custom_api.delete_namespaced_custom_object(
-        "kubeflow.org",
-        "v1beta1",
-        namespace,
-        "notebooks",
-        notebook,
-        client.V1DeleteOptions(propagation_policy="Foreground"),
+        group="kubeflow.org",
+        version="v1beta1",
+        namespace=namespace,
+        plural="notebooks",
+        name=notebook,
+        propagation_policy="Foreground",
     )
 
 

components/crud-web-apps/common/backend/kubeflow/kubeflow/crud_backend/api/pvc.py

@@ -2,11 +2,13 @@
 from . import v1_core
 
 
-def create_pvc(pvc, namespace):
+def create_pvc(pvc, namespace, dry_run=False):
     authz.ensure_authorized(
         "create", "", "v1", "persistentvolumeclaims", namespace
     )
-    return v1_core.create_namespaced_persistent_volume_claim(namespace, pvc)
+
+    return v1_core.create_namespaced_persistent_volume_claim(
+        namespace, pvc, dry_run="All" if dry_run else None)
 
 
 def delete_pvc(pvc, namespace):
@@ -21,3 +23,12 @@ def list_pvcs(namespace):
         "list", "", "v1", "persistentvolumeclaims", namespace
     )
     return v1_core.list_namespaced_persistent_volume_claim(namespace)
+
+
+def patch_pvc(name, namespace, pvc, auth=True):
+    if auth:
+        authz.ensure_authorized("patch", "", "v1", "persistentvolumeclaims",
+                                namespace)
+
+    return v1_core.patch_namespaced_persistent_volume_claim(name, namespace,
+                                                            pvc)

components/crud-web-apps/common/backend/kubeflow/kubeflow/crud_backend/api/utils.py

@@ -1,4 +1,7 @@
+import json
+
 from flask import jsonify
+from kubernetes import client
 
 from .. import authn
 
@@ -27,3 +30,21 @@ def failed_response(msg, error_code):
 
 def events_field_selector(kind, name):
     return "involvedObject.kind=%s,involvedObject.name=%s" % (kind, name)
+
+
+def deserialize(json_obj, klass):
+    """Convert a JSON object to a lib class object.
+
+    json_obj: The JSON object to deserialize
+    klass: The string name of the class i.e. V1Pod, V1Volume etc
+    """
+    try:
+        return client.ApiClient()._ApiClient__deserialize(json_obj, klass)
+    except ValueError as e:
+        raise ValueError("Failed to deserialize input into '%s': %s"
+                         % (klass, str(e)))
+
+
+def serialize(obj):
+    """Convert a K8s library object to JSON."""
+    return client.ApiClient().sanitize_for_serialization(obj)

components/crud-web-apps/common/backend/setup.py

@@ -3,7 +3,7 @@
 REQUIRES = [
     "Flask >= 1.1.1",
     "Flask-API >= 2.0",
-    "kubernetes >= 10.0.1, < 11.0.0",
+    "kubernetes >= 11.0.1",
     "requests >= 2.22.0",
     "urllib3 >= 1.25.7",
     "Werkzeug >= 0.16.0",

components/crud-web-apps/common/frontend/kubeflow-common-lib/projects/kubeflow/src/lib/form/section/section.component.html

@@ -1,5 +1,7 @@
 <div class="form--section-bottom">
-  <h3><lib-icon *ngIf="icon" [icon]="icon"></lib-icon>{{ title }}</h3>
+  <div class="header">
+    <lib-icon *ngIf="icon" [icon]="icon"></lib-icon>{{ title }}
+  </div>
   <p>{{ text }}</p>
   <p *ngIf="readOnly">*The cluster admin has disabled setting this section!</p>
 

components/crud-web-apps/common/frontend/kubeflow-common-lib/projects/kubeflow/src/lib/form/section/section.component.scss

@@ -1,11 +1,12 @@
-.wide {
-  width: 100%;
+.header,
+p {
+  margin-top: 0.2rem;
+  color: rgba(0, 0, 0, 0.66);
 }
 
-h3,
-p {
-  margin-block-start: 0.2rem;
-  color: rgba(0, 0, 0, 0.54);
+.header {
+  font-weight: 500;
+  font-size: 20px;
 }
 
 .lib-icon {
@@ -17,7 +18,3 @@ p {
 .form--section-bottom {
   margin-bottom: 0.5em;
 }
-
-.form--section-bottom > button {
-  margin-bottom: 0.5em;
-}

components/crud-web-apps/common/frontend/kubeflow-common-lib/projects/kubeflow/src/styles.scss

@@ -129,7 +129,7 @@ body {
 }
 
 .mat-button-base.form--button-margin {
-  margin-right: 8px;
+  margin-right: 16px;
 }
 
 // Make the outline grey instead of black

components/crud-web-apps/jupyter/backend/Makefile

@@ -6,6 +6,9 @@ install-deps:
 	popd
 	pip install -r requirements.txt
 
+unittest:
+	python -m unittest discover --pattern "*_test.py"
+
 run:
 	APP_PREFIX=/jupyter \
 	gunicorn -w 3 --bind 0.0.0.0:5000 --access-logfile - entrypoint:app

components/crud-web-apps/jupyter/backend/apps/common/form.py

@@ -13,23 +13,30 @@
 URI_REWRITE_ANNOTATION = "notebooks.kubeflow.org/http-rewrite-uri"
 
 
-def get_form_value(body, defaults, body_field, defaults_field=None):
+def get_form_value(body, defaults, body_field, defaults_field=None,
+                   optional=False):
     """
     Get the value to set by respecting the readOnly configuration for
     the field.
     If the field does not exist in the configuration then just use the form
     value.
     """
+    # The field in the defaults json not be the same in the request body
     if defaults_field is None:
         defaults_field = body_field
 
+    # If no default value exists then just return value in the request body.
+    # This is also useful if we add a new field and the configmap isn't updated
+    # yet
     user_value = body.get(body_field, None)
     if defaults_field not in defaults:
         return user_value
 
     readonly = defaults[defaults_field].get("readOnly", False)
     default_value = defaults[defaults_field]["value"]
 
+    # if the value of a field is readonly then the request/form should not
+    # contain this field
     if readonly:
         if body_field in body:
             raise BadRequest(
@@ -40,9 +47,14 @@ def get_form_value(body, defaults, body_field, defaults_field=None):
         log.info("Using default value for '%s': %s", body_field, default_value)
         return default_value
 
-    # field is not readonly
+    # field is not readonly and no value was provided
     if user_value is None:
-        raise BadRequest("No value provided for: %s" % body_field)
+        if not optional:
+            raise BadRequest("No value provided for: %s" % body_field)
+
+        # no value for field, but it was optional
+        log.info("No value provided for '%s'", defaults_field)
+        return None
 
     log.info("Using provided value for '%s': %s", body_field, user_value)
     return user_value
@@ -59,64 +71,6 @@ def is_config_volume(vol):
     return True
 
 
-def volume_from_config(config_vol, notebook):
-    """
-    Create a Volume Dict from the config.yaml. This dict has the same fields as
-    a Volume returned from the frontend
-    """
-    vol_name = config_vol["name"]["value"].replace(
-        "{notebook-name}", notebook["name"]
-    )
-    vol_class = utils.get_storage_class(config_vol["class"]["value"])
-
-    return {
-        "name": vol_name,
-        "type": config_vol["type"]["value"],
-        "size": config_vol["size"]["value"],
-        "mode": config_vol["accessModes"]["value"],
-        "path": config_vol["mountPath"]["value"],
-        "class": vol_class,
-        "extraFields": config_vol.get("extra", {}),
-    }
-
-
-def get_workspace_vol(body, defaults):
-    """
-    Checks the config and the form values and returns a Volume Dict for the
-    workspace.
-    """
-    if body.get("noWorkspace", False):
-        log.info("Requested to NOT use persistent storage for home dir")
-        return {}
-
-    workspace_vol = get_form_value(
-        body, defaults, "workspace", "workspaceVolume"
-    )
-
-    # check if it is a value from the config
-    if is_config_volume(workspace_vol):
-        workspace_vol = volume_from_config(workspace_vol, body)
-
-    return workspace_vol
-
-
-def get_data_vols(body, defaults):
-    """
-    Checks the config and the form values and returns a list of Volume
-    Dictionaries for the Notebook's Data Volumes.
-    """
-    vols = get_form_value(body, defaults, "datavols", "dataVolumes")
-    data_vols = []
-
-    for vol in vols:
-        if is_config_volume(vol):
-            vol = volume_from_config(vol, body)
-
-        data_vols.append(vol)
-
-    return data_vols
-
-
 # Notebook YAML processing
 def set_notebook_image(notebook, body, defaults):
     """

components/crud-web-apps/jupyter/backend/apps/common/routes/get.py

@@ -15,9 +15,11 @@ def get_config():
 @bp.route("/api/namespaces/<namespace>/pvcs")
 def get_pvcs(namespace):
     pvcs = api.list_pvcs(namespace).items
-    contents = [utils.pvc_dict_from_k8s_obj(pvc) for pvc in pvcs]
+    data = [{"name": pvc.metadata.name,
+             "size": pvc.spec.resources.requests["storage"],
+             "mode": pvc.spec.access_modes[0]} for pvc in pvcs]
 
-    return api.success_response("pvcs", contents)
+    return api.success_response("pvcs", data)
 
 
 @bp.route("/api/namespaces/<namespace>/poddefaults")

components/crud-web-apps/jupyter/backend/apps/common/utils.py

@@ -1,4 +1,6 @@
 import os
+import random
+import string
 
 from kubernetes import client
 from werkzeug import exceptions
@@ -23,6 +25,11 @@
 ]
 
 
+def random_string(size=9, chars=string.ascii_lowercase + string.digits):
+    """Create a random string."""
+    return "".join(random.choice(chars) for _ in range(size))
+
+
 def load_notebook_template(**kwargs):
     """
     kwargs: the parameters to be replaced in the yaml
@@ -106,16 +113,6 @@ def get_storage_class(vol):
 
 
 # Functions for transforming the data from k8s api
-def pvc_dict_from_k8s_obj(pvc):
-    return {
-        "name": pvc.metadata.name,
-        "namespace": pvc.metadata.namespace,
-        "size": pvc.spec.resources.requests["storage"],
-        "mode": pvc.spec.access_modes,
-        "class": pvc.spec.storage_class_name,
-    }
-
-
 def notebook_dict_from_k8s_obj(notebook):
     cntr = notebook["spec"]["template"]["spec"]["containers"][0]
     server_type = None