feat: Add a way to mount PVCs into Pipeline

[ada333]

Summary

This PR implements issue #843: users can now select PersistentVolumeClaims (PVCs) from the Kale JupyterLab panel and have them automatically mounted into every step of the compiled KFP v2 pipeline via kubernetes.mount_pvc(...).

What was deleted and why

ROK / KFP v1 volume machinery — the entire legacy volume system (ROK snapshot integration, KFP v1 VolumeOp/dsl.PipelineVolume wrappers, and the CLI flags that drove them) was removed. ROK is not used with KFP v2 and the old code was dead weight that made the config model harder to extend. marshal_volume was cleaned out from VolumeConfig entirely (only marshal_path as a plain field remains, used for the artifact-based data-passing path). Also golden test file titanic.py was deleted since it was referencing old snapshot and volumes API.

Backend changes

File	Change
kale/common/podutils.py	get_namespace() now also accepts KALE_NAMESPACE env var (for local dev); get_pod_name() now also accepts KALE_POD_NAME. This lets the Volumes panel work when running make jupyter outside a cluster.
kale/rpc/nb.py	Added list_pvcs(request) — lists all PVCs in the current namespace, returns sorted names; degrades to [] on any error. Added list_notebook_volumes(request) — introspects the notebook pod spec to return all PVC mounts as {"name", "mount_point"} dicts; also degrades to [] on error. Removed the now-dead final_snapshot field from explore_notebook()'s response (leftover from the ROK snapshot cleanup).
kale/common/k8sutils.py	Added get_pvc_access_modes() — best-effort helper to read a PVC's accessModes, returns [] on error.
kale/templates/pipeline_template.jinja2	The per-step task block now iterates over volumes: emits kubernetes.mount_pvc(…) for every type == 'pvc' entry, and additionally emits task.set_env_variable(name="KALE_VOLUME_<NAME>", value="<mount_path>") when expose_as_env_var is True. The to_kale_env_var_name Jinja filter is registered in compiler.py to handle name normalisation (my-data → KALE_VOLUME_MY_DATA).
kale/compiler.py	Added _warn_rwo_volumes() — checks each PVC's accessModes at compile time and emits a [KALE WARNING] log if any volume is ReadWriteOnce, because concurrent mounts across nodes will fail at runtime. Added _check_unique_volume_env_vars() — fails the compile with a clear error if two PVCs would derive the same KALE_VOLUME_* env var name, instead of silently letting the second overwrite the first at runtime.
kale/tests/assets/kfp_dsl/mount_pvc.py	Golden-file test fixture: a two-step pipeline with a raw-data PVC mounted at /data on every step, no env var.
kale/tests/assets/kfp_dsl/mount_pvc_env_var.py	Golden-file test fixture: same pipeline but with expose_as_env_var=True, so each step also has set_env_variable(name="KALE_VOLUME_RAW_DATA", value="/data").
kale/tests/unit_tests/test_rpc_nb.py	Added unit test coverage for list_pvcs, list_notebook_volumes (sorted mounts, non-PVC volumes ignored, dedup across containers, empty pod, namespace/pod read errors) and get_pvc_access_modes.

Frontend changes

File	What it does
labextension/src/widgets/LeftPanelTypes.ts	Added IVolumeConfig type (name, mount_point, type: 'pvc', expose_as_env_var?) and the volumes field to IKaleNotebookMetadata.
labextension/src/components/volumes/volumeUtils.ts	Shared utilities: deriveEnvVarName() (mirrors the backend Jinja filter), getMountPathError() (POSIX path validation — must be absolute, no trailing slash, no ./.. traversal segments, only [A-Za-z0-9._-] per segment), IAddFormState, INotebookVolume.
labextension/src/components/volumes/VolumeRow.tsx	A single row in the volumes list — shows PVC name, mount path, edit/remove buttons, and optionally the KALE_VOLUME_* env var name with a one-click copy button.
labextension/src/components/volumes/AddVolumeDialog.tsx	Dialog for adding or editing one volume manually: a free-solo Autocomplete combobox backed by nb.list_pvcs for suggestions, validation for duplicate volume names, duplicate mount paths, and POSIX-invalid mount paths, and an "Expose mount path as env var" checkbox with a live preview + duplicate-env-var check. The "Select from notebook" button (which opens NotebookVolumesDialog) is now sized to its content instead of stretching full-width. Helper text under the PVC name / Mount path fields now uses the same readable color as the rest of the form instead of the default (too-dark-for-dark-theme) MUI text color.
labextension/src/components/volumes/NotebookVolumesDialog.tsx	Dialog that calls nb.list_notebook_volumes to show PVCs currently mounted on the notebook pod. Each row has a checkbox to include it and a second "env" toggle to set expose_as_env_var. Supports "Select all".
labextension/src/components/VolumesPanel.tsx	Top-level volumes section rendered inside the left panel — renders the list of VolumeRows, tracks duplicate mount paths, drives the add/edit dialogs, and handles clipboard copy with a 2-second "Copied!" feedback.
labextension/src/widgets/hooks/useNotebookMetadata.ts	Added updateVolumes callback wired to notebook metadata persistence.
labextension/src/widgets/LeftPanel.tsx	Mounts VolumesPanel in the "Volumes" section of the left panel, passing volumes from metadata and the updateVolumes setter. The "Advanced Kale settings live in JupyterLab Settings" notice is now always rendered last (after Pipeline Metadata and Volumes), instead of being sandwiched in between them.
labextension/src/lib/Commands.ts	Added listPvcs() and listNotebookVolumes() — thin RPC wrappers over nb.list_pvcs and nb.list_notebook_volumes. Removed a stale reference to exploration.final_snapshot (that field no longer exists on the backend response, so the "Resuming notebook before/after step" message was always silently wrong — it now just says "Resuming notebook at step").

Testing steps

Automated tests

• pytest kale/tests/unit_tests/test_rpc_nb.py — list_pvcs, list_notebook_volumes, get_pvc_access_modes.
• pytest kale/tests/unit_tests/test_compiler.py — _check_unique_volume_env_vars, to_kale_env_var_name filter.
• pytest kale/tests/unit_tests/test_config.py — VolumeConfig / expose_as_env_var round-trip through NotebookConfig.
• pytest kale/tests/e2e/test_e2e.py — golden-file DSL compilation for the mount_pvc / mount_pvc_env_var notebook fixtures.

Manual UI/e2e testing

1. Create a PVC in the namespace Kale/your notebook is running in, e.g.:

   apiVersion: v1
   kind: PersistentVolumeClaim
   metadata:
     name: myvolume
   spec:
     accessModes:
       - ReadWriteOnce
     resources:
       requests:
         storage: 10Mi
     storageClassName: standard

2. Open a notebook, enable Kale, and go to the Volumes section. Click + to add a volume:
   • Confirm the PVC name field autocompletes/suggests myvolume.
   • Set the mount path (e.g. /data) and, optionally, check "Expose mount path as env var" and confirm the KALE_VOLUME_MYVOLUME preview.
   • Save the volume.
3. Add a second volume:
   • Reuse the same volume name as the first → the name field should show "This volume name is already used by another volume" and the Add button should be disabled.
   • Reuse the same mount path as the first → "Mount path already used by another volume".
   • Try an invalid mount path such as data (not absolute), /my data (space), or /../etc/passwd (traversal) → the POSIX validation error should appear and block submission.
4. If the notebook server pod itself has PVCs mounted, click Select from notebook (now a normal, non-stretched button) and confirm it lists the pod's existing PVC mounts and lets you bulk-add them.
5. Confirm the "Advanced Kale settings live in JupyterLab Settings" notice always renders at the very bottom of the panel, below the Volumes section, regardless of how many volumes are configured.
6. Compile/run the pipeline and confirm:
   • The compiled DSL calls kubernetes.mount_pvc(...) for every step, and task.set_env_variable(name="KALE_VOLUME_...", ...) if "expose as env var" was checked.
   • If the PVC's access mode is ReadWriteOnce, the Kale compile output logs [KALE WARNING] PVC '...' has accessMode ReadWriteOnce....
   • The resulting KFP pipeline run actually mounts the PVC at the configured path in each step pod.

What still needs to be done

• RWO PVC warning is currently only a compile-time log line — no dedicated warning surfaced in the JupyterLab UI or at runtime inside the pod.
• Add user-facing documentation for the volume-mounting feature (currently only the design proposal in docs/proposals/0843-mount-volumes/README.md).

[review-notebook-app]

Check out this pull request on 

See visual diffs & provide feedback on Jupyter Notebooks.

---

Powered by ReviewNB

[harshhh817]

Had a read through the Python side of this (I've been in nbprocessor.py / pipeline.py for #909). Big cleanup — nice to see the Rok/v1 volume machinery go. Two things and one small note; given you flagged this as a staged PR, some of it may just be "not yet".

1. expose_as_env_var looks inert.

The field says:

# When True the compiler injects a KALE_VOLUME_<NAME> env var into every
# pipeline step alongside the mount_pvc call, making the mount path
# discoverable at runtime without hard-coding it.
expose_as_env_var = Field(type=bool, default=False)

but KALE_VOLUME appears exactly once in the whole diff — in that comment. The template's volume block only emits the mount:

{%- for volume in volumes %}
{%- if volume.type == 'pvc' %}
    kubernetes.mount_pvc({{ step.name }}_task, pvc_name="{{ volume.name }}", mount_path="{{ volume.mount_point }}")
{%- endif %}
{%- endfor %}

and mount_pvc.py has no env var either. The tests cover the config round-trip (test_volume_config_expose_as_env_var, and the propagation-through-NotebookConfig one) but not that anything is emitted, so nothing catches it.

If it's deliberately staged like the snapshot fields, it'd help to say so the same way — those carry an explicit "parsed and stored but have no effect on compilation" note, and this one reads as implemented. Otherwise it's a missing set_env_variable in the loop.

2. list_pvcs turns every failure into an empty list.

except Exception as e:
    ...  # returns []

The docstring is upfront that this is for graceful degradation, which is fair for a combobox. The part I'd push on: a 403 from a missing RBAC rule and "this namespace genuinely has no PVCs" are the same empty dropdown, and the former is the one people will actually hit and be unable to diagnose.

It's also the opposite direction from #906, which is currently making the other silent RPC failure (unreachable KFP) visible and actionable. Would it be worth letting a permission error through — or returning something the UI can distinguish — so the two RPC paths behave consistently?

Small note: marshal_volume is dropped cleanly — the field, the _set_marshal_path assignment, and the assertion in test_config.py all go together, no dangling references. marshal_path stays as a plain config field, which reads right for the v2 artifact-based path.

Happy to take another pass once the later commits land.

[ada333]

hi @harshhh817 thanks for the review, I want to add most of this in other commits, I am just doing this in smaller parts in draft PR so the CI gets triggered on every commit and I can check that nothing breaks. I will let you know when this PR is ready for full review.

[ada333]

Preview of the mounting volumes functionality:

Untitled.video.mp4

[ada333]

hi @Fruchix @jesuino @StefanoFioravanzo @hmtosi @harshhh817 the PR for mounting volumes in Kale is ready for review!
You can see the preview in the video in the comment above.

[Copilot]

Pull request overview

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Implements PVC selection and mounting support for KFP v2 pipelines, including a new JupyterLab “Volumes” panel to configure PVC mounts and optional env-var exposure, while removing legacy ROK/KFP v1 volume machinery.

Changes:

• Add Volumes panel UI (add/edit/select-from-notebook) and persist volumes in notebook metadata.
• Add RPC endpoints to list PVCs and notebook pod PVC mounts; add compiler-time warning for ReadWriteOnce PVCs.
• Update pipeline template to emit kubernetes.mount_pvc(...) per step and optionally inject KALE_VOLUME_* env vars; add/adjust tests and fixtures.

Reviewed changes

Copilot reviewed 31 out of 31 changed files in this pull request and generated 9 comments.

Show a summary per file

File	Description
labextension/style/index.css	Adds dedicated styling for the new Volumes panel UI.
labextension/src/widgets/hooks/useNotebookMetadata.ts	Adds updateVolumes metadata updater for persistence.
labextension/src/widgets/LeftPanelTypes.ts	Introduces IVolumeConfig and volumes metadata field + default.
labextension/src/widgets/LeftPanel.tsx	Renders the new VolumesPanel in the left panel.
labextension/src/lib/RPCUtils.tsx	Removes legacy Rok tooltip helper (ROK deprecation).
labextension/src/lib/Commands.ts	Adds RPC wrappers listPvcs and listNotebookVolumes.
labextension/src/components/volumes/volumeUtils.ts	Adds env-var name derivation and shared volume form types.
labextension/src/components/volumes/VolumeRow.tsx	Adds row UI for a configured volume + optional copy env-var.
labextension/src/components/volumes/NotebookVolumesDialog.tsx	Adds dialog to select existing notebook pod PVC mounts.
labextension/src/components/volumes/AddVolumeDialog.tsx	Adds add/edit dialog with PVC suggestions and validation.
labextension/src/components/VolumesPanel.tsx	Adds top-level volumes list management + dialogs + copy behavior.
labextension/src/components/AnnotationInput.tsx	Deletes legacy volume annotation UI component.
labextension/src/components/AdvancedSettings.tsx	Removes volumes panel embedding and related styled wrappers.
kale/tests/unit_tests/test_rpc_nb.py	Adds unit tests for list_pvcs and get_pvc_access_modes.
kale/tests/unit_tests/test_config.py	Updates marshal-path tests and adds expose_as_env_var coverage.
kale/tests/unit_tests/test_compiler.py	Adds test for the Jinja to_kale_env_var_name filter.
kale/tests/e2e/test_e2e.py	Extends e2e goldens to include mount_pvc scenarios.
kale/tests/assets/notebooks/mount_pvc.ipynb	Adds notebook fixture for PVC mount compilation.
kale/tests/assets/notebooks/mount_pvc_env_var.ipynb	Adds notebook fixture for PVC mount + env-var exposure.
kale/tests/assets/kfp_dsl/titanic.py	Removes legacy golden referencing old snapshot/volumes API.
kale/tests/assets/kfp_dsl/mount_pvc.py	Adds golden compiled DSL output for PVC mounting.
kale/tests/assets/kfp_dsl/mount_pvc_env_var.py	Adds golden compiled DSL output for PVC + env-var exposure.
kale/templates/pipeline_template.jinja2	Emits mount_pvc and optional env vars per step; conditional import.
kale/rpc/nb.py	Adds list_pvcs / list_volumes RPCs and removes snapshot env handling.
kale/processors/nbprocessor.py	Clarifies snapshot_volumes as forward-compatible/no-op for v2.
kale/pipeline.py	Removes marshal volume logic and adds expose_as_env_var to VolumeConfig.
kale/compiler.py	Adds RWO PVC warning and registers to_kale_env_var_name Jinja filter.
kale/common/podutils.py	Adds env-var fallbacks for namespace/pod name to support local dev.
kale/common/k8sutils.py	Adds best-effort helper to fetch PVC access modes.
kale/cli.py	Removes old CLI flags tied to legacy volume machinery.
FAQ.md	Removes outdated Rok and RWX/RWO guidance tied to old volume approach.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[harshhh817]

Took another pass now that it's ready. expose_as_env_var is properly wired this time — the to_kale_env_var_name filter, the set_env_variable in the volume loop, the KALE_VOLUME_RAW_DATA lines in mount_pvc.py, and the parametrized name-derivation tests all line up. Thanks for picking that up.

One thing on the derivation:

template_env.filters["to_kale_env_var_name"] = lambda s: (
    "KALE_VOLUME_" + re.sub(r"[^A-Z0-9]", "_", s.upper())
)

Every non-alphanumeric collapses to _, and PVC names are DNS subdomains, so both - and . are legal separators. That makes the mapping lossy:

PVC names	derived env var
my-data, my.data	KALE_VOLUME_MY_DATA
raw-data, raw.data	KALE_VOLUME_RAW_DATA
a-b.c, a.b-c	KALE_VOLUME_A_B_C

If two such volumes are both mounted with expose_as_env_var, the template emits two set_env_variable calls with the same name and the second silently wins — so one volume's path is quietly wrong at runtime rather than failing loudly.

Low probability (you'd need two PVCs differing only in separator), but it's the silent kind. You already guard the analogous case for mount paths — duplicateMountPaths / mountDuplicate in the UI — so it might be worth the same treatment here. A compile-time check that the derived names are unique across volumes would also cover the CLI path (kale --nb ...), which never goes through those frontend guards.

Separately, list_pvcs still turns every failure into []. I raised that last time and I don't want to relitigate it — just noting it's unchanged in case it was missed rather than decided. Happy either way.

The rest reads well to me: marshal_volume and _set_marshal_path come out cleanly with their test assertion, and the golden file makes the volume behaviour easy to eyeball.

[ada333]

Took another pass now that it's ready. expose_as_env_var is properly wired this time — the to_kale_env_var_name filter, the set_env_variable in the volume loop, the KALE_VOLUME_RAW_DATA lines in mount_pvc.py, and the parametrized name-derivation tests all line up. Thanks for picking that up.

One thing on the derivation:

template_env.filters["to_kale_env_var_name"] = lambda s: (
    "KALE_VOLUME_" + re.sub(r"[^A-Z0-9]", "_", s.upper())
)

Every non-alphanumeric collapses to _, and PVC names are DNS subdomains, so both - and . are legal separators. That makes the mapping lossy:

PVC names derived env var
my-data, my.data KALE_VOLUME_MY_DATA
raw-data, raw.data KALE_VOLUME_RAW_DATA
a-b.c, a.b-c KALE_VOLUME_A_B_C
If two such volumes are both mounted with expose_as_env_var, the template emits two set_env_variable calls with the same name and the second silently wins — so one volume's path is quietly wrong at runtime rather than failing loudly.

Low probability (you'd need two PVCs differing only in separator), but it's the silent kind. You already guard the analogous case for mount paths — duplicateMountPaths / mountDuplicate in the UI — so it might be worth the same treatment here. A compile-time check that the derived names are unique across volumes would also cover the CLI path (kale --nb ...), which never goes through those frontend guards.

Separately, list_pvcs still turns every failure into []. I raised that last time and I don't want to relitigate it — just noting it's unchanged in case it was missed rather than decided. Happy either way.

The rest reads well to me: marshal_volume and _set_marshal_path come out cleanly with their test assertion, and the golden file makes the volume behaviour easy to eyeball.

hey @harshhh817 thanks for your review! Good point with the env vars, I think it is not likely to happen but I added safeguard (both in UI and compile) to ensure that the PVCs do not have same env vars. For list_pvcs I decided to leave it as it is for now - there is still UI text saying that no PVC was found which I think is enough. If it causes trouble for users we can create an issue and add some better error message later.

[jesuino]

Thanks for your work on this! Please see my comments and here are some UI suggestions:

1. I would keep the advanced settings message always on the bottom. Volumes being after it is just strange:

2. If I understand correctly the "Select from Notebook" will only make sense when Kale is running on a Notebook that is running on Kubernetes, correct? I would rethink the form in a way that the add from notebook is not this big not proportional button and perhaps disable it if Kale is not running on a pod or if there's no volume available

Finally I missed the steps to test this in your PR. I did the following and I can say that the feature is working well!

• Created this PVC on Kubernetes:

apiVersion: v1
kind: PersistentVolumeClaim
metadata:
  name: myvolume
spec:
  accessModes:
    - ReadWriteOnce
  resources:
    requests:
      storage: 10Mi
  storageClassName: standard

• Then I created the following pipeline in Kale:

• I can see that the volume was mounted,But HTML reports are all empty and it is happening for other pipelines too. I wonder if this broke here or if it was broken after the KFP update to 2.17 and it worked as expected:

[ada333]

@jesuino thanks for review! I agree with all the points and I added them to PR

[jesuino]

Thanks for your work on this!

This is working correctly and do improve performance for pipelines, especially when does heavily uses storage.

[ederign]

/approve
/lgtm

[google-oss-prow]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ederign

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [ederign]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment