Skip to content

Add PVC mode in UI, UI RWO warning and documentation of volumes in Kale - #949

Open
ada333 wants to merge 4 commits into
kubeflow:mainfrom
ada333:issue-947-pvc-followup
Open

Add PVC mode in UI, UI RWO warning and documentation of volumes in Kale#949
ada333 wants to merge 4 commits into
kubeflow:mainfrom
ada333:issue-947-pvc-followup

Conversation

@ada333

@ada333 ada333 commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

This PR is followup to mounting volumes in kale #907
Adds docs for user
Fetches PVC mode and displays it in UI - left panel and in the dropdown:

Screenshot From 2026-09-01 12-53-48 Screenshot From 2026-09-01 12-53-55 Screenshot From 2026-09-01 12-54-01

Also adds warning before running the pipeline when some mounted PVC has RWO or RWOP mode:
Screenshot From 2026-09-01 13-00-48

…g volumes

Signed-off-by: Adam Maly <amaly@redhat.com>
@review-notebook-app

Copy link
Copy Markdown

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@google-oss-prow

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign jesuino for approval. For more information see the Kubernetes Code Review Process.

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

Details Needs approval from an approver in each of these files:

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

Signed-off-by: Adam Maly <amaly@redhat.com>
@google-oss-prow google-oss-prow Bot added size/XL and removed size/L labels Aug 30, 2026
Signed-off-by: Adam Maly <amaly@redhat.com>
Signed-off-by: Adam Maly <amaly@redhat.com>
@ada333
ada333 marked this pull request as ready for review September 1, 2026 11:24
@google-oss-prow
google-oss-prow Bot requested a review from ederign September 1, 2026 11:24
@harshhh817

Copy link
Copy Markdown
Contributor

Went through the backend and the new warning path. The list_pvcs shape change is handled cleanly — both consumers (Commands.listPvcs and AddVolumeDialog) are updated in the same PR, and since the labextension ships in the same package as the backend since #670 there's no skew risk. Three things.

1. The pre-run warning fails open in two ways, and both are silent.

const pvcModes = new Map(pvcs.map(p => [p.name, p.access_modes]));
const restrictedVolumes = metadata.volumes.filter(v =>
  isRestrictedAccessMode(pvcModes.get(v.name)),
);

If a volume in metadata.volumes isn't in what listPvcs() returned, pvcModes.get() is undefined, isRestrictedAccessMode(undefined) returns false, and the run proceeds with no warning. The surrounding catch {} does the same thing when the RPC fails outright.

Best-effort is the right call for a warning — but is it worth distinguishing "checked, and it's fine" from "couldn't check"? Right now an RWO volume that list_pvcs didn't return looks identical to a safe RWX one. The compile-time _warn_rwo_volumes is a second line of defence, but that only logs and doesn't block, so a user who'd have cancelled at the dialog won't get the chance.

2. The RWO/RWOP set is now hardcoded in two languages.

compiler.py has rwo_modes = {"ReadWriteOnce", "ReadWriteOncePod"}; volumeUtils.ts has m === 'ReadWriteOnce' || m === 'ReadWriteOncePod'. Same policy, two copies, nothing keeping them in sync — add a mode on one side and the other silently disagrees.

This is exactly the drift #935 is building shared_constants.json to prevent. Would it be worth landing this after #935 and sourcing the mode list from there, or at least leaving a note so it gets picked up in that sweep?

3. isReadWriteOnce is introduced already deprecated.

/** @deprecated Use isRestrictedAccessMode instead */
export const isReadWriteOnce = isRestrictedAccessMode;

It doesn't exist on main and nothing in the PR references it, so there's no caller to keep working — is it needed at all, or a leftover from an earlier iteration?

The _warn_rwo_volumes docstring rewrite and the RWOP addition both read well, and the volumes doc is a genuinely useful thing to have.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants