-
Notifications
You must be signed in to change notification settings - Fork 178
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Feature Request]: Support a default-less StorageClass cluster in DS Projects #1701
Comments
/assign @shalberd |
@andrewballantyne: GitHub didn't allow me to assign the following users: shalberd. Note that only opendatahub-io members with read permissions, repo collaborators and people who have commented on this issue/PR can be assigned. Additionally, issues/PRs can only have 10 assignees at the same time. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
I'll be assigned to this for now until we figure that out. |
commenting ... maybe that helps :-) |
There we go! |
regarding looking for a default storageclass, if any, at least with oc get, it seems easy. The name column has (default) in case the default annotation is present. In future work, maybe once could make use of that to quickly get the default storageclass, if any. If I were using bash scripting, I'd just parse that or get that default one, if present, with oc get storageclasses -o jsonpath='{range .items[*].metadata}{..... |
So when you go to do the code logic, have a look at how we do any gets for other resources: export const getProjects = (withLabel?: string): Promise<ProjectKind[]> =>
k8sListResource<ProjectKind>({ // you'll likely need to create a StorageClassKind
model: ProjectModel, // the models are pretty simple (see below)
queryOptions: {} // you probably don't need any query options as you're not looking to get by name or namespace or label
}).then((listResource) => listResource.items);
// The project model -- see `/frontend/src/api/models/...`
export const ProjectModel: K8sModelCommon = {
apiVersion: 'v1',
apiGroup: 'project.openshift.io',
kind: 'Project',
plural: 'projects',
}; |
This will need multiple iterations and we track all incubating items via trackers now. See #1919 |
@andrewballantyne yes LOL turns out I forgot to also add my logic to the Storage Section - Add New Storage in addition to what I implemented here for New Workbench and creation of PVC along with a notebook. See https://github.com/opendatahub-io/odh-dashboard/pull/2503/files#r1489215627 and https://github.com/opendatahub-io/odh-dashboard/pull/2503/files#r1489362341 and https://github.com/opendatahub-io/odh-dashboard/pull/2503/files#r1489400693 |
haha, thanks @shalberd -- @alexcreasy is working to get your work in as you saw. I've got a lot of juggling of other things, so I'll leave you two discuss any questions and what have you -- Alex or you can let me know if things get stalled and needs my attention -- otherwise I'll come back through when it is all set and address getting it into |
Feature description
Today we have support for Jupyter to set a
spec.notebookController.storageClassName
that allows them, if set, to override and configure the Jupyter Notebooks with PVCs containing that particular storage class value.@shalberd has made a request for an early adoption of Storage Classes in DS Projects. We have been trying to prep for it for a while now but it seems unlikely to be done in a timely manner.
Sven's scenario is one of no default storage class on the OpenShift Cluster.
Upon talking with Sven, we have agreed to do the following on construction of new PVCs (does not apply to edit path):
spec.notebookController.storageClassName
valueconsole.error(...)
and do nothing else (continue with the flow today)Implementation is up for debate, but I imagine all the logic will be found in the
createPVC
function. We'll have it updated to take in the DashboardConfig & it will use Promises to fetch the storage class information and do the business logic.This design is intended to be low footprint and to allow for us to adopt the future storage class feature as a whole and improve everything across the board.
Describe alternatives you've considered
Wait for Storage Classes -- which doesn't have a timeline, best to go forward with something small for an easy win.
Anything else?
@shalberd has agreed to take a shot at doing this work. So I'll be DM'ing him about steps to get into the
incubation
branch.The text was updated successfully, but these errors were encountered: