Fix missing SSH port and namespace in Welcome notebook on Kubeflow (#133)#149
Open
SannaPersson wants to merge 8 commits into
Open
Fix missing SSH port and namespace in Welcome notebook on Kubeflow (#133)#149SannaPersson wants to merge 8 commits into
SannaPersson wants to merge 8 commits into
Conversation
…ab, add new environment variables for dashboard, and clean up commented code for discord signup URL.
Removed nvflare_dashboards from return statement in kubernetes_utils.py.
…innelab#133) The Welcome notebook only worked when JupyterHub injected SSH_PORT_<user>, NAMESPACE, and an overridden HOSTNAME via extraEnv. Pods spawned by Kubeflow saw "N/A" for the SSH command and had no namespace info at all. Add MAIA.workspace_env.discover_workspace_env() with a Kubeflow fallback path: NB_PREFIX -> user, the service-account namespace file -> namespace, and an in-cluster Service lookup -> SSH port + hostname (read from a new maia.kthcloud.io/ssh-hostname annotation set by maia-namespace-base). The Welcome notebook now calls the helper, so JupyterHub behaviour is unchanged while Kubeflow users see correct values.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes the Welcome notebook’s missing SSH command/namespace when MAIA workspaces are spawned by Kubeflow (where JupyterHub extraEnv variables aren’t injected), by adding a Python helper that can fall back to in-cluster Kubernetes lookups and by annotating per-user SSH Services with a discoverable hostname.
Changes:
- Add
MAIA.workspace_env.discover_workspace_env()with Kubeflow fallback resolution (NB_PREFIX, serviceaccount namespace file, Service lookup for SSH port/hostname). - Update
docker/MAIA-Workspace/Welcome.ipynbto use the helper and display the namespace. - Extend the
maia-namespace-baseHelm chart withsshHostnamevalue rendered as a Service annotation; bump chart to1.7.2.
Reviewed changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
MAIA/workspace_env.py |
New env discovery helper with optional Kubernetes Service-based fallback for Kubeflow-spawned notebooks. |
tests/test_workspace_env.py |
Adds unit tests covering env precedence, Kubeflow fallback behavior, and failure modes. |
docker/MAIA-Workspace/Welcome.ipynb |
Switches env cell to call discover_workspace_env() and adds a namespace display cell. |
charts/maia-namespace-base/values.yaml |
Adds new sshHostname chart value (default empty). |
charts/maia-namespace-base/templates/ssh_service.yaml |
Emits maia.kthcloud.io/ssh-hostname annotation on SSH Services when configured. |
charts/maia-namespace-base/Chart.yaml |
Bumps chart/app version to 1.7.2. |
MAIA/maia_admin.py |
Passes ssh_hostname through into chart values as sshHostname. |
MAIA/kubernetes_utils.py |
Adds a defensive labels existence check; modifies get_namespace_details return line. |
| maia_workspace_apps["xnat"] = "N/A" | ||
|
|
||
| return maia_workspace_apps, remote_desktop_dict, ssh_ports, monai_models, orthanc_list, deployed_clusters, nvflare_dashboards | ||
| return maia_workspace_apps, remote_desktop_dict, ssh_ports, monai_models, orthanc_list, deployed_clusters #, nvflare_dashboards |
Collaborator
There was a problem hiding this comment.
Keep returning the nvflare_dashboards and simply update the docstrings
Comment on lines
+45
to
+47
| try: | ||
| services = core_v1.list_namespaced_service(namespace).items | ||
| except Exception: |
Collaborator
There was a problem hiding this comment.
Good suggestion, implement that
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Simone Bendazzoli <simonebendazzoli93@gmail.com>
| maia_workspace_apps["xnat"] = "N/A" | ||
|
|
||
| return maia_workspace_apps, remote_desktop_dict, ssh_ports, monai_models, orthanc_list, deployed_clusters, nvflare_dashboards | ||
| return maia_workspace_apps, remote_desktop_dict, ssh_ports, monai_models, orthanc_list, deployed_clusters #, nvflare_dashboards |
Collaborator
There was a problem hiding this comment.
Keep returning the nvflare_dashboards and simply update the docstrings
Comment on lines
+45
to
+47
| try: | ||
| services = core_v1.list_namespaced_service(namespace).items | ||
| except Exception: |
Collaborator
There was a problem hiding this comment.
Good suggestion, implement that
Collaborator
There was a problem hiding this comment.
maia-namespace-base chart is no longer used, so this needs to be moved to the maia-namespace chart
Collaborator
There was a problem hiding this comment.
For from MAIA.workspace_env import discover_workspace_env to work, the notebook needs either to install the maia-toolkit package or to have the Python file available in the environment
Signed-off-by: Simone Bendazzoli <simben@kth.se>
Collaborator
|
/format |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #133. The Welcome notebook in
MAIA-Workspaceonly populated SSH port / namespace when JupyterHub injectedSSH_PORT_<user>,NAMESPACE, and an overriddenHOSTNAMEviaextraEnv. Pods spawned by Kubeflow's Notebook Controller showedN/Afor the SSH command and had no namespace information.MAIA.workspace_env.discover_workspace_env()resolves the five workspace fields with a Kubeflow fallback path:NB_PREFIX→ user,/var/run/secrets/.../namespace→ namespace, in-clusterServicelookup → SSH port and hostname.sshHostnameonmaia-namespace-base(default"") is rendered as amaia.kthcloud.io/ssh-hostnameannotation on each per-user SSHService.MAIA_install_project_toolkitalready runscreate_maia_namespace_values, which now passescluster_config.ssh_hostnamethrough.1.7.2.Test plan
python -m pytest tests/test_workspace_env.py -v— 14 tests covering JupyterHub-only path, Kubeflow fallback, NodePort vs ClusterIP, unrelated-service rejection, all-missing →N/A, missing hostname annotation, K8s API failure, factory exception, env-var precedence over service lookup, JupyterHub-encoded usernames,NB_PREFIXtrailing slash, namespace file stripping.helm templateofmaia-namespace-basewithsshHostname=ssh.example.comand unset — annotation appears only when the value is set.python -c "import json; json.load(open('docker/MAIA-Workspace/Welcome.ipynb'))"— notebook is valid JSON.Welcome.ipynb, run the env cell, confirm namespace andssh maia-user@<host> -p <port>populate correctly.Notes
get/list servicesin its own namespace. The defaultdefault-editorrole used by Kubeflow Profiles already grants this, but worth confirming on the target cluster.kubernetesis already insetup.cfginstall_requires, so the helper imports work in the workspace image without a Dockerfile change.HOSTNAMEonly whenJUPYTERHUB_USERis also set; otherwise the helper reads the SSH host from the new Service annotation.