Skip to content
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

Multiple storage classes for same provisioner #306

Merged
merged 3 commits into from
Mar 22, 2023
Merged

Multiple storage classes for same provisioner #306

merged 3 commits into from
Mar 22, 2023

Conversation

samene
Copy link

@samene samene commented Mar 15, 2023

We have multiple disks mounted on a node and depending on the type of storage requirement we want to create the local path pvc on a particular disk. Today we can specify multiple paths on a node via the nodePathMap: but the final path selection is random.

I propose to create multiple storage classes with a parameter of path that specifies the path to use. That way, using a storage class like below I can provision the storage on the right folder.

apiVersion: storage.k8s.io/v1
kind: StorageClass
metadata:
  name: ssd-local-path
  labels:
    app.kubernetes.io/name: local-path-provisioner
    helm.sh/chart: local-path-provisioner-0.0.23
    app.kubernetes.io/instance: local-path-provisioner
    app.kubernetes.io/version: "v0.0.23"
    app.kubernetes.io/managed-by: Helm
provisioner: cluster.local/local-path-provisioner
parameters:
  path: /data/ssd
volumeBindingMode: WaitForFirstConsumer
reclaimPolicy: Delete

If the path parameter is not found on the storage class, it falls back to existing behaviour.

@derekbit
Copy link
Member

@samene
In general, LGTM.
Can you use a more specific name rather than path?
In addition, please update the README.md for the new parameter as well.
Thank you.

@derekbit
Copy link
Member

v0.0.24 will be released by this Wed. Put in v0.0.25 if can't catch up with v0.0.24.

@samene
Copy link
Author

samene commented Mar 20, 2023

@derekbit I have updated the README.md. Can you suggest a better name for the path parameter?

@derekbit
Copy link
Member

derekbit commented Mar 22, 2023

@derekbit I have updated the README.md. Can you suggest a better name for the path parameter?

@samene
I don't have a better idea...nodePath might be more clear than path, WDYT?
BTW, can you help rebase and address the comment? I will releasev0.0.24 after getting this PR merged.
Thank you very much.

Copy link
Member

@derekbit derekbit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.
@samene Thanks for your contribution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants