Skip to content

Commit 75d8a41

Browse files
authored
feat: user home directories, shared group storage, and NSS wrapper (#30)
* feat: add shared group directories and NSS wrapper - Add shared-storage RWX PVC volume support with per-group subPaths mounted at /shared/<group> inside user pods - Add init container that creates group directories with chmod 2775 (setgid bit) so new files inherit the group and are group-writable - Add libnss_wrapper.so configuration so whoami/id report the real username instead of 'jovyan', with NB_UMASK=0002 - Refactor pre_spawn_hook into focused single-responsibility functions: _get_user_groups, _setup_shared_storage, _setup_nss_wrapper - Orchestrator _pre_spawn_hook chains Nebi auth, shared storage, and NSS wrapper; always registered (NSS runs even without shared storage) - Add sharedStorage.groups allowlist and mountPathPrefix values - Add jupyterhub.custom.shared-storage-* config keys * fix: correct NSS GID to 1000 (jovyan) and always create ~/shared dir - gid default was 100 but z2jh sets pod securityContext GID to 1000; add jovyan:x:1000: group entry so 'groups' command resolves the name - when shared PVC is disabled, mkdir -p ~/shared instead of removing it so users always see the directory regardless of storage configuration * fix: store groups in auth_state and always create ~/shared/<group> dirs - EnvoyOIDCAuthenticator now stores parsed groups in auth_state so the spawner can read them at spawn time (JupyterHub groups table is empty when manage_groups is not enabled) - refresh_user also re-parses groups from the refreshed IdToken to keep auth_state current - _pre_spawn_hook always resolves user groups, not only when shared PVC is enabled - _setup_nss_wrapper creates local ~/shared/<group> dirs per group when no shared PVC is configured, so users always see their group dirs * feat: add in-cluster NFS server for shared storage on RWO-only clusters Deploys quay.io/nebari/volume-nfs backed by a single RWO PVC and re-exports it as RWX NFS, enabling shared group directories on providers like Hetzner that only provide ReadWriteOnce storage (hcloud-volumes). - templates/nfs-server.yaml: NFS Deployment, Service, backend RWO PVC - templates/shared-pvc.yaml: StorageClass + PV (NFS path) + PVC when nfsServer.enabled; falls back to external RWX PVC otherwise - values.yaml: sharedStorage.nfsServer.{enabled,storageClass,image} fields * fix: add DaemonSet to install nfs-common on k3s worker nodes k3s worker nodes on minimal OS images (Hetzner) ship without nfs-common, causing NFS PV mounts to fail with 'bad option'. The DaemonSet uses nsenter to install nfs-common on the host via apt-get, skipping if already present. Gated on sharedStorage.nfsServer.installClient (default false). * fix: use alpine:3 sleep for DaemonSet pause container * fix: NFS PV path /exports not / (overlayfs cannot be exported) * fix: remove spawner.user.groups (DetachedInstanceError in async); add try-except _get_user_groups accessed spawner.user.groups (SQLAlchemy lazy-loaded relationship) from an async pre_spawn_hook, causing DetachedInstanceError which silently aborted _setup_shared_storage and _setup_nss_wrapper. Groups are now read only from auth_state (stored by EnvoyOIDCAuthenticator). Each step is individually wrapped in try-except so failures are logged and don't prevent subsequent steps from running. * fix: address code review findings (I1-I7, C1-C3, M1, N3) I1: set c.KubeSpawner.fs_gid=100 explicitly so shared dir file ownership is deterministic (GID 100 = users group) rather than relying on z2jh default I2: add Helm validation in _helpers.tpl that fails at template time if sharedStorage.enabled and jupyterhub.custom.shared-storage-enabled diverge I3: use Path(g).name like classic Nebari so /projects/myproj -> myproj, not projects/myproj; deduplicate groups to prevent duplicate mountPaths I4: add nodeSelector/nodeAffinity support to NFS server Deployment so deployers can pin it to worker nodes and avoid slow RWO PVC reattachment I7: add argocd.argoproj.io/sync-options: Prune=false to StorageClass and PersistentVolume to prevent accidental deletion during ArgoCD force sync C1: add chown 0:100 before chmod 2775 in initialize-shared-mounts init container so shared dirs are explicitly owned by GID 100 (users) C2: use printf instead of echo '...' for NSS file writes to safely handle special characters in usernames without shell quoting issues C3: deduplicate groups in _get_user_groups (via Path.name already handles most cases; added explicit dedup set for belt-and-suspenders) M1: log exception with exc_info=True in refresh_user JWT parse failure N3: merge into existing lifecycle_hooks instead of replacing; warn if a postStart hook already exists before overwriting Logging: added comprehensive info/debug/warning logging throughout all pre-spawn hook functions for both happy and failure paths * docs: add JupyterLab profiles design spec * Revert "docs: add JupyterLab profiles design spec" This reverts commit 56c22cf. * feat: add JupyterLab profiles for CPU/RAM resource sizing (closes #31) Exposes a profile selector in JupyterHub matching the classic Nebari experience. Profiles are defined under jupyterhub.custom.profiles in values.yaml and passed directly to c.KubeSpawner.profile_list via get_config(). Default profiles: - Small: 1 CPU / 2 GB RAM (default) - Medium: 4 CPU / 8 GB RAM kubespawner_override accepts any KubeSpawner trait so GPU profiles, custom images, and node selectors work without code changes in the future. When profiles list is empty, no selector is shown (single-instance mode). * fix: add descriptive names to default server profiles Update default profile display_name and description to be more user-friendly (e.g. "Small Instance" with "Stable environment with 1 CPU / 2 GB RAM" instead of just "Small" / "1 CPU / 2 GB RAM"). * split: move JupyterLab profiles to separate branch Profiles feature (#31) is out of scope for this PR. Moved to local branch feat/jupyterlab-profiles for a follow-up PR. * test: add k3d-based e2e smoke test Replaces the inline-bash test workflow with a pytest-based suite that manages the k3d cluster, helm install, and pod-wait lifecycle. Conftest exposes a 'cluster' session fixture and a 'hub_url' fixture that port-forwards proxy-public. CI runs uvx pytest tests/e2e -v with PYTHONUNBUFFERED=1 so live logs stream into the workflow output. Locally: uvx pytest tests/e2e -v # fresh cluster K3D_CLUSTER=k3d-nebari-dev uvx pytest tests/e2e -v # reuse * test: add NFS-backed shared-storage e2e tests Switches the e2e harness to kind (k3d's busybox-on-scratch nodes lack a package manager, so the chart's nfs-common installer DaemonSet can't provision NFS client tools). New tests in tests/e2e/test_shared_storage.py exercise the full PR #30 spawn path against a real cluster: - test_user_in_group_can_write: alice-data writes /shared/data/... - test_shared_dir_is_group_owned: dir mode is 2775 (setgid) The DummyAuth shim in tests/e2e/fixtures/test-values.yaml maps the login username to user+groups (alice-data -> alice in [data]) so we can fake auth_state without running Keycloak. Everything else (spawner hook, init container, NSS wrapper, NFS mount) is real. Chart changes: - sharedStorage.nfsServer.mountOptions added (default []). Tests pass [nfsvers=3] because kind nodes use overlayfs which fails the volume-nfs image's NFSv4 root export. Production unchanged. Conftest infrastructure: - kind cluster fixture with KIND_KEEP=1 reuse - hosts-entry workaround so kubelet's host mount.nfs can resolve the cluster-internal NFS service FQDN (kind nodes have no cluster DNS in their host resolv.conf) - structured logging + step counter + per-cycle pod state, events from kubectl describe, and node-level kubelet journal lines - autouse failure-dump fixture (kubectl get pods/events + hub and singleuser logs) * refactor(tests/e2e): split conftest into deep modules Conftest had grown to 577 lines mixing five concerns. Extracted into focused modules each with a small interface and large hidden impl: _process.py subprocess + kubectl helpers + step counter _hub.py HubClient (cookie/login/spawn/stop session) _pod_observer.py wait_for_pod_ready + dedup'd pod-state polling _cluster.py kind lifecycle + helm install + NFS hosts workaround conftest.py shrinks to 218 lines holding only fixtures that compose the modules. Eliminates duplication of: - cookie-jar login flow (was repeated in _login_and_spawn + _stop_server) - two parallel subprocess wrappers (_run + _kctl) - inline pod-state polling loop in the spawn flow Tests still pass locally (3 passed in 35s, cluster reused). * ci: speed up e2e — disable z2jh prePuller + cache kindest/node prePuller hooks pre-pull singleuser images on every node before helm install completes. On a single-node test cluster this is pure overhead (~30s of blocking wait). Disable in test-values.yaml. kindest/node image pull was the largest variable cost in CI: 9s on a fast runner, 130s on a slow one. Cache it as a docker tarball keyed on the kind version so subsequent runs are deterministic and fast. Expected: total CI time drops from variable 3-5min to ~90-120s. * ci: fix kindest/node cache-save step under set -e Previous heuristic used `[ -n "$img" ] && docker save` which exits 1 when grep finds no image, killing the whole step. Hardcode the v1.32.2 tag (fixed by kind v0.27.0) and use plain commands so set -e only triggers on real failures. * ci: drop kindest/node cache attempt GH Actions ubuntu-latest runners come with kindest/node preinstalled (tagged "<none>"). The actual image fetch on cache miss was only ~12s because docker just verifies the digest. The cache step was earning ~5s in the best case and breaking the workflow when docker save couldn't find the v1.32.2 tag (image is referenced by digest, not tag). prePuller-disable change is keeping its ~30s saving — sufficient win without the cache complexity. * test(e2e): expand shared-storage suite to full permission contract 9 tests (was 2) covering the per-group /shared/<group> contract end-to-end: - dir is root:users 2775 (parametrized over groups) - pod is member of users group; NB_UMASK=0002 in env - new files inherit gid=100 mode 0664; new subdirs gid=100 mode 02775 (setgid propagation) - multi-group user sees + writes every group dir - user does not see groups they don't belong to (mount-time isolation) - file written by one user is readable + appendable by a groupmate from a separate pod (cross-user collaboration) Conftest adds PathStat + SpawnedUser.stat()/path_exists() so tests assert against typed fields (mode/uid/gid) instead of parsing stat strings — keeps tests short and behavior-focused. * ci: cache singleuser image across runs to skip ~73s cold pull Singleuser image (multi-GB) is currently pulled by kubelet inside the kind node on first user spawn, costing ~73s of every CI run. Pull it once on the runner host, save as tar, cache it (key = image ref so a values.yaml bump auto-invalidates), and side-load with `kind load image-archive`. Pre-create the cluster in the workflow so the side-load happens before any pod is scheduled — the pytest fixture's ensure_cluster() reuses the existing cluster. Cache hit: skips the ~90s registry pull entirely; only kind-load (~20s) remains. Cache miss: pull + save once (~120s), then every subsequent run benefits. * docs(shared-storage): position external RWX as primary, mark in-cluster NFS as transitional Addresses comment on issue #29: the bundled `nfsServer.enabled=true` path relies on `quay.io/nebari/volume-nfs:0.8-repack`, a manifest-schema repack of an abandoned upstream image (nebari-dev/nebari-docker-images#230). We should not be carrying that workaround image as the recommended path for a greenfield chart. The chart already supported bringing your own RWX StorageClass; this change makes that path the documented primary: - values.yaml: reframe the sharedStorage block. Recommend an external RWX class with provider-specific examples (Longhorn, EFS, Filestore, Azure Files, nfs-subdir-external-provisioner). Add a deprecation note on the nfsServer.image block linking to issue #29. - README: add a "Shared Storage" section with the same matrix and an explicit pointer to the issue tracking removal of the in-cluster NFS path. No template changes — the external-RWX path was already rendered when nfsServer.enabled=false. Verified via `helm template` that setting only `sharedStorage.storageClass=longhorn` produces a single RWX PVC and no nfs-server pod. * docs(shared-storage): correct provider list — only longhorn is NIC-provisioned Previous commit listed EFS/Filestore/Azure Files as recommended RWX backends. NIC does not provision those — they are separate cloud-managed services no one in NIC has wired up. NIC's actual storage reality: hetzner : longhorn (longhorn.Install in pkg/provider/hetzner) aws : longhorn (longhorn.Install in pkg/provider/aws) existing : longhorn (longhorn.Install in pkg/provider/existing) gcp : standard-rwo (no RWX provisioned) azure : managed-csi (no RWX provisioned) local : (no storage layer) So the accurate recommendation is just longhorn. Updated values.yaml and README to say so directly. The in-cluster NFS fallback stays — it covers the providers where NIC has not yet wired up an RWX class — with a pointer to issue #29 for tracking removal once that lands everywhere. --------- Co-authored-by: Amit Kumar <aktech@users.noreply.github.com>
1 parent 8f2e545 commit 75d8a41

20 files changed

Lines changed: 1668 additions & 50 deletions

.github/workflows/test.yaml

Lines changed: 47 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -7,63 +7,66 @@ on:
77
branches: [main]
88

99
jobs:
10-
test:
10+
e2e:
1111
runs-on: ubuntu-latest
1212
timeout-minutes: 15
1313

1414
steps:
1515
- name: Checkout
1616
uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6.0.1
1717

18-
- name: Install k3d
19-
run: curl -s https://raw.githubusercontent.com/k3d-io/k3d/main/install.sh | bash
20-
21-
- name: Create k3d cluster
22-
run: k3d cluster create test --wait
23-
24-
- name: Install Helm
18+
- name: Install kind
2519
run: |
26-
curl -fsSL https://raw.githubusercontent.com/helm/helm/main/scripts/get-helm-3 | bash
20+
curl -fsSLo /tmp/kind https://kind.sigs.k8s.io/dl/v0.27.0/kind-linux-amd64
21+
sudo install -m 0755 /tmp/kind /usr/local/bin/kind
2722
28-
- name: Update Helm dependencies
29-
run: helm dependency update
23+
- name: Install Helm
24+
run: curl -fsSL https://raw.githubusercontent.com/helm/helm/main/scripts/get-helm-3 | bash
3025

31-
- name: Deploy chart
32-
run: helm install data-science-pack . --namespace default --set nebariapp.enabled=false
26+
- name: Install uv
27+
run: curl -LsSf https://astral.sh/uv/install.sh | sh
3328

34-
- name: Wait for pods with progress
29+
# Singleuser image is multi-GB and is the dominant cold-start cost
30+
# (~73s pulled in-cluster during the first test). Cache the tar across
31+
# CI runs and side-load it into kind so kubelet never goes to the
32+
# registry. Cache key is the image ref so a `values.yaml` bump
33+
# invalidates automatically.
34+
- name: Resolve singleuser image ref
35+
id: img
3536
run: |
36-
for i in {1..60}; do
37-
echo "=== Attempt $i/60 ==="
38-
kubectl get pods
39-
echo "--- Hub logs (last 10 lines) ---"
40-
kubectl logs -l component=hub --tail=10 2>/dev/null || echo "(not ready)"
41-
echo "--- Proxy logs (last 5 lines) ---"
42-
kubectl logs -l component=proxy --tail=5 2>/dev/null || echo "(not ready)"
43-
if kubectl wait --for=condition=ready pod -l component=hub --timeout=5s 2>/dev/null && \
44-
kubectl wait --for=condition=ready pod -l component=proxy --timeout=5s 2>/dev/null; then
45-
echo "All pods ready!"
46-
exit 0
47-
fi
48-
sleep 5
49-
done
50-
echo "Timeout waiting for pods"
51-
exit 1
37+
REF=$(yq -r '.jupyterhub.singleuser.image | .name + ":" + .tag' values.yaml)
38+
echo "ref=$REF" >> "$GITHUB_OUTPUT"
39+
echo "tar=/tmp/singleuser.tar" >> "$GITHUB_OUTPUT"
5240
53-
- name: Test JupyterHub endpoint
41+
- name: Cache singleuser image
42+
id: img-cache
43+
uses: actions/cache@27d5ce7f107fe9357f9df03efb73ab90386fccae # v5.0.5
44+
with:
45+
path: ${{ steps.img.outputs.tar }}
46+
key: singleuser-${{ steps.img.outputs.ref }}
47+
48+
- name: Pull + save singleuser image (cache miss only)
49+
if: steps.img-cache.outputs.cache-hit != 'true'
5450
run: |
55-
kubectl port-forward svc/proxy-public 8000:80 &
56-
PF_PID=$!
57-
sleep 5
58-
echo "=== Response from /hub/login ==="
59-
curl -sS http://localhost:8000/hub/login || true
60-
echo ""
61-
echo "=== Checking for JupyterHub ==="
62-
curl -sS http://localhost:8000/hub/login | grep -q "JupyterHub"
63-
RESULT=$?
64-
kill $PF_PID 2>/dev/null || true
65-
exit $RESULT
51+
docker pull "${{ steps.img.outputs.ref }}"
52+
docker save "${{ steps.img.outputs.ref }}" -o "${{ steps.img.outputs.tar }}"
53+
54+
# Pre-create the cluster here (instead of letting the pytest fixture
55+
# do it) so we can side-load the cached image before any pod is
56+
# scheduled. The fixture's ensure_cluster() reuses an existing one.
57+
- name: Create kind cluster
58+
run: kind create cluster --name nbtest-e2e --wait 60s
59+
60+
- name: Side-load singleuser image into kind node
61+
run: kind load image-archive "${{ steps.img.outputs.tar }}" --name nbtest-e2e
62+
63+
- name: Run e2e tests
64+
env:
65+
KIND_CLUSTER: "nbtest-e2e" # reuse the cluster created above
66+
KIND_KEEP: "1" # runner is ephemeral, skip teardown
67+
PYTHONUNBUFFERED: "1" # stream logs live (no block buffering on non-TTY)
68+
run: uvx pytest tests/e2e -v --color=yes
6669

67-
- name: Check hub logs for errors
70+
- name: Hub logs on failure
6871
if: failure()
69-
run: kubectl logs -l component=hub --tail=100
72+
run: kubectl logs -l component=hub --tail=200 || true

README.md

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,26 @@ make down
5858

5959
See `values.yaml` for all configuration options. The chart wraps the [JupyterHub Helm chart](https://z2jh.jupyter.org/) - all `jupyterhub.*` values are passed through.
6060

61+
## Shared Storage
62+
63+
Per-group shared directories (`/shared/<group>` in every user pod) need a
64+
ReadWriteMany `StorageClass` on the cluster. On NIC-managed clusters that's
65+
[Longhorn](https://longhorn.io/), installed by NIC's storage layer:
66+
67+
```yaml
68+
sharedStorage:
69+
enabled: true
70+
storageClass: longhorn
71+
size: 100Gi
72+
```
73+
74+
For clusters where NIC has not yet wired up an RWX class (local dev, current
75+
GCP/Azure paths), the chart includes a transitional
76+
`sharedStorage.nfsServer.enabled=true` mode that runs an in-cluster NFS
77+
server pod. It depends on the `quay.io/nebari/volume-nfs` workaround image
78+
and is tracked for removal in
79+
[issue #29](https://github.com/nebari-dev/nebari-data-science-pack/issues/29).
80+
6181
## Architecture
6282

6383
```

config/jupyterhub/00-gateway-auth.py

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,17 +93,27 @@ async def authenticate(self, handler, data=None):
9393

9494
username = claims.get("preferred_username") or claims.get("sub")
9595
if not username:
96-
self.log.warning("No username claim in IdToken: %s", list(claims.keys()))
96+
self.log.warning("authenticate: no username claim in IdToken: %s", list(claims.keys()))
9797
return None
9898

9999
# Extract groups from the token (set by the "groups" scope / group mapper)
100100
groups = claims.get("groups", [])
101+
if not groups:
102+
self.log.warning(
103+
"authenticate: no groups claim in IdToken for %s "
104+
"(check Keycloak client scope / group mapper configuration)",
105+
username,
106+
)
101107
# Keycloak returns groups as paths (e.g. "/admin"), strip leading slash
102108
groups = [g.strip("/") for g in groups]
103109

104110
# Determine admin from group membership
105111
admin_groups = set(get_config("custom.admin-groups", ["admin"]))
106112
is_admin = bool(admin_groups & set(groups))
113+
self.log.info(
114+
"authenticate: user=%s groups=%s is_admin=%s",
115+
username, groups, is_admin,
116+
)
107117

108118
return {
109119
"name": username,
@@ -114,6 +124,7 @@ async def authenticate(self, handler, data=None):
114124
"id_token": id_token,
115125
"access_token": access_token,
116126
"refresh_token": refresh_token,
127+
"groups": groups, # stored so spawner can read at spawn time
117128
}.items() if v is not None
118129
},
119130
}
@@ -130,6 +141,7 @@ async def refresh_user(self, user, handler=None):
130141
"""
131142
if handler is None:
132143
# No request context (e.g. internal API call) — can't read cookies
144+
self.log.debug("refresh_user: no handler for %s (internal call), returning True", user.name)
133145
return True
134146

135147
id_token, access_token, refresh_token = self._extract_envoy_cookies(handler)
@@ -157,14 +169,35 @@ async def refresh_user(self, user, handler=None):
157169
)
158170
return True
159171

172+
# Re-parse groups from the refreshed IdToken so auth_state stays current
173+
try:
174+
payload_b64 = id_token.split(".")[1]
175+
payload_b64 += "=" * (4 - len(payload_b64) % 4)
176+
claims = json.loads(base64.urlsafe_b64decode(payload_b64))
177+
groups = [g.strip("/") for g in claims.get("groups", [])]
178+
self.log.debug("refresh_user: refreshed groups for %s: %s", user.name, groups)
179+
except Exception:
180+
self.log.warning(
181+
"refresh_user: failed to parse groups from IdToken for %s "
182+
"— auth_state will have empty groups until next full login",
183+
user.name,
184+
exc_info=True,
185+
)
186+
groups = []
187+
160188
auth_state = {
161189
k: v for k, v in {
162190
"id_token": id_token,
163191
"access_token": access_token,
164192
"refresh_token": refresh_token,
193+
"groups": groups,
165194
}.items() if v is not None
166195
}
167196

197+
self.log.debug(
198+
"refresh_user: auth_state refreshed for %s (access_token=%s, groups=%s)",
199+
user.name, bool(access_token), groups,
200+
)
168201
return {"name": user.name, "auth_state": auth_state}
169202

170203

0 commit comments

Comments
 (0)