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

Pass k8s API metadata through parameters in CreateVolume #370

Closed
davidz627 opened this issue Oct 16, 2019 · 15 comments · Fixed by #399
Closed

Pass k8s API metadata through parameters in CreateVolume #370

davidz627 opened this issue Oct 16, 2019 · 15 comments · Fixed by #399
Assignees
Labels
kind/feature Categorizes issue or PR as related to a new feature.

Comments

@davidz627
Copy link
Contributor

/kind feature

There is a use case for passing in k8s API metadata through parameters in CreateVolume. For example this information could be used by drivers to label underlying resources and associate them with k8s objects.

This could be done through a reserved parameter namespace e.g. kubernetes.io/pvc.name, kubernetes.io/pvc.namespace
It could be enabled by a flag so that it is not a breaking change (since some drivers may do strict validation on their own well-known parameters).

/cc @jsafrane @msau42

@k8s-ci-robot k8s-ci-robot added the kind/feature Categorizes issue or PR as related to a new feature. label Oct 16, 2019
@davidz627
Copy link
Contributor Author

existing tags in k8s for PD are of the form:
kubernetes.io/created-for/pv/name
kubernetes.io/created-for/pvc/name
kubernetes.io/created-for/pvc/namespace

@jsafrane
Copy link
Contributor

jsafrane commented Oct 17, 2019

There is a long discussion in #86.

Technically, I would perhaps reuse expansion introduced in #69 to all parameters, so it's more obvious. Passing PVC name + namespace + PV name would not harm. Passing PVC annotations (as introduced by the PR) is a topic for discussion, as it makes PVC not portable across clusters.

@davidz627
Copy link
Contributor Author

I would like to avoid the sticky topic of passing in annotations and just get the parameters for PVC name, namespace, PV name for now. Wondering if anyone is working on that or if I wanted to use it in the near future if I would have to pick that up :)

@jsafrane
Copy link
Contributor

Wondering if anyone is working on that

Nobody that I know about. However, it should be a relatively simple PR, right? All the variable expansion is already done, we just need to extend it to all storageClass.parameters.

@davidz627
Copy link
Contributor Author

I will pick it up
/assign

@fejta-bot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jan 16, 2020
@davidz627
Copy link
Contributor Author

didn't have time to work on this - the issue is available to pick up by anyone if interested. I will comment on this issue if I start working on it actively
/remove-lifecycle stale

@k8s-ci-robot k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jan 16, 2020
@kubernetes-csi kubernetes-csi deleted a comment from k8s-ci-robot Jan 16, 2020
@hoyho
Copy link
Contributor

hoyho commented Jan 17, 2020

/assign

@aibarbetta
Copy link
Contributor

Hi @hoyho, are you already working on this one? I happen to need this feature quite urgently and can take care of making the change if you want

@yoesmat
Copy link

yoesmat commented Jan 17, 2020

How would the PV name be used? Isn't it the same as the volume name today in the CreateVolume request?

@travisghansen
Copy link

@yoesmat volume name is generally pvc-{guid}..It's scope would be global, where is the pvc name would be unique to namespace etc.

@yoesmat
Copy link

yoesmat commented Jan 17, 2020

@travisghansen, I understand why the PVC name/namespace is useful. I am asking about PV name.

@travisghansen
Copy link

@yoesmat ok, I must have misunderstood the question. Thanks!

@yoesmat
Copy link

yoesmat commented Jan 17, 2020

Thanks @travisghansen :)!

@hoyho
Copy link
Contributor

hoyho commented Jan 18, 2020

Hi @hoyho, are you already working on this one? I happen to need this feature quite urgently and can take care of making the change if you want

Yes, I do have a local branch to add this feature. Since @zetsub0u have opened a PR , let's wait for it to merge.

kbsonlong pushed a commit to kbsonlong/external-provisioner that referenced this issue Dec 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants