Changes required for Jupyter-Scheduler integration #1832

iameskild · 2023-06-13T13:02:00Z

Reference Issues or PRs

This PR makes several modifications that are required for Jupyter-Scheduler to work with Nebari, specifically the Argo-Workflows extension that I built. A few additional changes were also required on the Nebari-Workflow-Controller side; see PR 16.

The main changes include:

Renaming jupyter_notebook_config to jupyter_server_config and enabling jupyter_server.serverapp.ServerApp on JupyterHub - these changes are required for Jupyter Scheduler to work with our current version of JupyterHub.
Mounting the ARGO_TOKEN specific to the group the user is in (admin, developer or viewer) which means that only users with Argo admin and developer roles will be allowed to submit jobs.
Mounting a view-only conda-store API token that is used to validate which conda-store environments/kernels are useable.

What does this implement/fix?

Put a x in the boxes that apply

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds a feature)
Breaking change (fix or feature that would cause existing features not to work as expected)
Documentation Update
Code style update (formatting, renaming)
Refactoring (no functional changes, no API changes)
Build related changes
Other (please describe):

Testing

Did you test the pull request locally?
Did you add new tests?

Any other comments?

pavithraes · 2023-06-13T14:40:15Z

We need some people to test this out, before including it in a release.

src/_nebari/stages/input_vars.py

Adam-D-Lewis · 2023-07-04T13:48:01Z

src/_nebari/template/stages/07-kubernetes-services/jupyterhub.tf

+  conda-store-environments                           = var.conda-store-environments
+  default-conda-store-namespace                      = var.conda-store-default-namespace
+  conda-store-cdsdashboard-token                     = module.kubernetes-conda-store-server.service-tokens.cdsdashboards
+  conda-store-argo-workflows-jupyter-scheduler-token = module.kubernetes-conda-store-server.service-tokens.argo-workflows-jupyter-scheduler


passing token so it can be set on jupyter user pods?

Is jupyter scheduler a pod that runs?

Jupyter-Scheduler is a lab extension that runs while the user pod is running. Getting access to view the conda-store makes it so we can more reliably set the path for the conda-store environments (mostly for this function).

The conda-store feature is also a traitlet, albeit just a basic toggle.

.../template/stages/07-kubernetes-services/modules/kubernetes/services/jupyterhub/configmaps.tf

...s-services/modules/kubernetes/services/jupyterhub/files/jupyter/jupyter_server_config.py.tpl

Adam-D-Lewis · 2023-07-04T17:24:27Z

...7-kubernetes-services/modules/kubernetes/services/jupyterhub/files/jupyterhub/03-profiles.py

+    ADMIN = "admin"
+    DEVELOPER = "developer"
+    ANALYST = "analyst"
+
+    base = "argo-"
+
+    if ANALYST in groups:
+        argo_sa = base + "view"
+    if DEVELOPER in groups:
+        argo_sa = base + "dev"
+    if ADMIN in groups:
+        argo_sa = base + "admin"
+    else:
+        return {}


This seems a bit clunky, plus we might get rid of analyst, dev, and admin. Not sure if there's a better way. Maybe check the individual argo permissions of the groups themselves.

I agree! The difficult part is that roles aren't available in the current spawner.user.auth_state. I think the other alternative is to perform a keycloak API call. We would probably also need to add keycloak secrets to the hub deployment.

I have this particular enhancement in this issue.

Adam-D-Lewis · 2023-07-04T17:28:37Z

...7-kubernetes-services/modules/kubernetes/services/jupyterhub/files/jupyterhub/03-profiles.py

+        "ARGO_TOKEN": {
+            "valueFrom": {
+                "secretKeyRef": {
+                    "name": f"{argo_sa}.service-account-token",


Did the modifications you made create this token or did it already exist?

This token already exists, it's the default argo-admin, argo-dev and argo-view secrets. I'm not entirely sure if they were being used prior to this... The ARGO_TOKEN used when someone creates a workflow from the UI (and the one that can be copied from there) are not the same as any of those above.

I have also included this the enhancements issue, namely, the ability to create short-lived tokens on a per user basis.

Adam-D-Lewis · 2023-07-04T17:30:52Z

...7-kubernetes-services/modules/kubernetes/services/jupyterhub/files/jupyterhub/03-profiles.py

+
+def profile_conda_store_viewer_token():
+    return {
+        "CONDA_STORE_TOKEN": {


Any way this token could be misused by the user? It's used by ARGO_WORKFLOWS_EXECUTOR, I see - https://github.com/search?q=repo%3Anebari-dev%2Fargo-workflows-executor+CONDA_STORE_TOKEN&type=code

Is Argo Workflows Executor a package that's installable or a pod that's running or its included on the jupyterhub pod?

This token is view-only in scope so if the user was motivated enough, they could view all of the namespaces/environments that exist.

Later down the road we should scope this token to just what the user can see. But for now this works.

Adam-D-Lewis · 2023-07-04T17:33:04Z

...nebari/template/stages/07-kubernetes-services/modules/kubernetes/services/jupyterhub/main.tf

+
+  data = {
+    "conda-store-api-token"    = var.conda-store-argo-workflows-jupyter-scheduler-token
+    "conda-store-service-name" = var.conda-store-service-name


This is a url I guess. I want to double check this later and make sure there's not a better way.

yeah this is an internal service url. conda-store-service-name, as a variable name, is used elsewhere in the other services so I just kept the same name.

costrouc

Nothing controversial here to me. I like the changes. @iameskild and @Adam-D-Lewis has this PR been tested with a deployment?

...ri/template/stages/07-kubernetes-services/modules/kubernetes/services/argo-workflows/main.tf

costrouc · 2023-07-18T19:25:50Z

...7-kubernetes-services/modules/kubernetes/services/jupyterhub/files/jupyterhub/03-profiles.py

+
+def profile_conda_store_viewer_token():
+    return {
+        "CONDA_STORE_TOKEN": {


Later down the road we should scope this token to just what the user can see. But for now this works.

iameskild added 4 commits May 31, 2023 12:05

Add ARGO env vars to user profile, update jupyter_server_config name

1f54261

Add auth-mode=client, update jupyter_server_config

a1927b9

Use conda-store API to validate conda env used

a74a3d2

Update jupyter_server_config

124bdca

iameskild added needs: review 👀 This PR is complete and ready for reviewing area: user experience 👩🏻‍💻 area: JupyterLab area: integration/Argo labels Jun 13, 2023

iameskild requested review from costrouc, Adam-D-Lewis and viniciusdc and removed request for Adam-D-Lewis June 13, 2023 13:02

Merge branch 'develop' into jupyter_scheduler_changes

c92c88b

iameskild requested a review from Adam-D-Lewis June 13, 2023 13:06

iameskild added the needs: documentation 📖 This item is missing docs label Jun 13, 2023

iameskild mentioned this pull request Jun 19, 2023

Docs for Jupyter Scheduler integration nebari-dev/nebari-docs#335

Merged

18 tasks

iameskild and others added 2 commits June 20, 2023 12:58

Merge branch 'develop' into jupyter_scheduler_changes

3963778

Fix dev, viewer tokens

a5e31e8

iameskild added this to the Release 2023.7.1 milestone Jun 27, 2023

Merge branch 'develop' into jupyter_scheduler_changes

57697f7

Adam-D-Lewis reviewed Jul 4, 2023

View reviewed changes

src/_nebari/stages/input_vars.py Show resolved Hide resolved

Adam-D-Lewis reviewed Jul 4, 2023

View reviewed changes

.../template/stages/07-kubernetes-services/modules/kubernetes/services/jupyterhub/configmaps.tf Show resolved Hide resolved

Adam-D-Lewis reviewed Jul 4, 2023

View reviewed changes

...s-services/modules/kubernetes/services/jupyterhub/files/jupyter/jupyter_server_config.py.tpl Show resolved Hide resolved

Adam-D-Lewis reviewed Jul 4, 2023

View reviewed changes

Update profile_argo_token fn

7a15495

iameskild and others added 3 commits July 11, 2023 08:45

Merge branch 'develop' into jupyter_scheduler_changes

479ea9b

Add configmap for valid_argo_roles

5e5aff2

update valid-argo-roles name

9cdcd16

iameskild mentioned this pull request Jul 12, 2023

Enable NWC to mutate workflows created by service accounts nebari-dev/nebari-workflow-controller#16

Merged

18 tasks

standardize argo naming

ff864ae

iameskild requested a review from Adam-D-Lewis July 17, 2023 16:14

costrouc approved these changes Jul 18, 2023

View reviewed changes

iameskild and others added 4 commits July 18, 2023 16:44

Use /etc/argo instead

c21afa4

Add ARGO_NAMESPACE to env vars

80bf287

Merge branch 'develop' into jupyter_scheduler_changes

2177e7c

Update name to argo_jupyter_scheduler

87e11f0

iameskild merged commit 46f7c24 into develop Jul 20, 2023

iameskild deleted the jupyter_scheduler_changes branch July 20, 2023 16:59

costrouc added a commit that referenced this pull request Aug 3, 2023

Changes to account for #1832 and #1868

aa6a93b

costrouc mentioned this pull request Aug 3, 2023

Extension Mechanism Implementation #1833

Merged

27 tasks

iameskild pushed a commit that referenced this pull request Aug 9, 2023

Changes to account for #1832 and #1868

142182c

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Changes required for Jupyter-Scheduler integration #1832

Changes required for Jupyter-Scheduler integration #1832

iameskild commented Jun 13, 2023 •

edited

Loading

pavithraes commented Jun 13, 2023 •

edited

Loading

Adam-D-Lewis Jul 4, 2023

Adam-D-Lewis Jul 4, 2023

iameskild Jul 11, 2023 •

edited

Loading

Adam-D-Lewis Jul 4, 2023 •

edited

Loading

iameskild Jul 11, 2023

Adam-D-Lewis Jul 4, 2023

iameskild Jul 11, 2023

Adam-D-Lewis Jul 4, 2023

Adam-D-Lewis Jul 4, 2023

iameskild Jul 11, 2023

costrouc Jul 18, 2023

Adam-D-Lewis Jul 4, 2023

iameskild Jul 11, 2023

costrouc left a comment

costrouc Jul 18, 2023

Changes required for Jupyter-Scheduler integration #1832

Changes required for Jupyter-Scheduler integration #1832

Conversation

iameskild commented Jun 13, 2023 • edited Loading

Reference Issues or PRs

What does this implement/fix?

Testing

Any other comments?

pavithraes commented Jun 13, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

iameskild Jul 11, 2023 • edited Loading

Choose a reason for hiding this comment

Adam-D-Lewis Jul 4, 2023 • 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

Choose a reason for hiding this comment

costrouc left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

iameskild commented Jun 13, 2023 •

edited

Loading

pavithraes commented Jun 13, 2023 •

edited

Loading

iameskild Jul 11, 2023 •

edited

Loading

Adam-D-Lewis Jul 4, 2023 •

edited

Loading