-
Notifications
You must be signed in to change notification settings - Fork 811
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
Add the ability to provide arbitrary EBS volume tags #180
Comments
/feature |
/assign @dkoshkin |
This feature also has some nice security implications.... I personally would like my provisioners IAM policy to require certain tags to be applied and then only allow the attacher to attach such volumes in their policy. Otherwise I could i.e. hack into a worker node and reattach i.e. an etcd volume from another, totally unrelated EC2 (yeah, I know about encryption at rest for etcd 😉) Just best practice on AWS: https://aws.amazon.com/de/premiumsupport/knowledge-center/iam-policy-tags-restrict/ |
A StorageClass parameter would not be great for my use-case, which requires every EBS volume to be tagged with a product code for cost show-back purposes. We have hundreds of these codes. I'd rather not have to create the same number of StorageClasses (potentially multiplied by some combination of other parameters) for every cluster. Our ideal solution involves PersistentVolumeClaim annotations as mentioned in kubernetes/kubernetes#50898 (comment). |
There has been an ongoing discussion here. |
Issues go stale after 90d of inactivity. If this issue is safe to close now please do so with Send feedback to sig-testing, kubernetes/test-infra and/or fejta. |
/remove-lifecycle stale |
OK, looks like upstream does not like the either of the proposals. But this is a real production issue. Can we do some workaround like: introduce a config options (i.e., a cmdline flag) for the controller service so that it'll always attach some tags to all the dynamically provisioned EBS volumes. The default can be empty so that it matches the current behavior. Thoughts? |
@jieyu Given my understanding of the use cases, we need a way to dynamically tag the volumes given some user input. Passing the tag through controller service cmd flag won’t help in those cases |
I remember chatted with @saad-ali a while back. One potential solution to this problem could be user creates volume tags inside PV annotation, and we create a separated small controller that watches PV and it reads PV's annotation and tags the corresponding volume with the tags. |
The PVs are not created by a user PVCs are, I think it would be strange as a user to have add annotations because how would you do it, a controller :P? |
@leakingtapan this is our issue: we need to make sure all volumes dynamic provisioned are tagged with a cluster uuid so that we can safely delete the volume during cluster teardown. For that particular use case, we don't necessarily need to dynamically tag the volumes given some user input. That's the reason I was suggesting the above solution, which sounds less intrusive and backwards compatible. |
@jieyu I see your point. Just created a separate issue to track your request since I feel we might be able to solve it under that specific situation with your suggestion as one option. |
If we were to provide arbitrary EBS volume tagging functionality, I think we need to consider both dynamic provisioning and static provisioning case. If we want user to create tag annotations as part of PVC, this solves for dynamic provisioning. But I'm not sure how it will work for static provisioning. Another way is, maybe we could only provide this tagging feature to dynamic provisioned volumes and we require user to tag themselves if it is static provisioning. But I agree that asking user to add annotation in PV when dynamic provisioning is used is weird. |
I created a high level design #351 using operator for this problem. PTAL @dkoshkin @mgoodness |
Issues go stale after 90d of inactivity. If this issue is safe to close now please do so with Send feedback to sig-testing, kubernetes/test-infra and/or fejta. |
/remove-lifecycle stale |
Is there two different problems here? I can't create an EBS volume without an OWNER tag due to billing requirements forcing me to configure other storage. That can be the same tag every time when associated with the cluster. Perhaps that's an easier problem to solve first? Then figuring out how to provide a method for changing up tags depending on certain scenarios can come as a follow-up problem? |
@flickerfly you might want to checkout this feature #333 |
Works great, Thanks! |
Hi @wongma7
Thank you. |
@hywook4 every time you make new volume you'll redeploy ebs-csi driver with extra-key tags suggested in README.md. this extra-key tag will be Name and value will be provided by config-map that will also be made directly. i was using it with helm charts and i also had issue with naming EBS volumes in aws so i came with this solution. Script that will deploy EBS-csi driver along with config-map and helm chart
for this i had controller.yaml file in the same location as this script controller.yaml
And it works best. |
the extraTags feature is nice for setting a global "default". Could it be possible to set tags using some labels/annotations on the PV/PVC? |
the additional tagging of created EBS volumes is also important for tuning of backup plans/backup selections (assignments), especially in production environments |
Have the same requirement here in order to make the most out of AWS Backup. |
@harshdevl @wongma7 , is there a way to pass a tag with value containing a space like name=First Name, to the extraVolumeTags flag? |
Well I haven't tried something like that before but you can also make configmap of that with space names and then use it in ebs controller file like below. |
A bit offtop but for those who got to this issue like me, looking for just having a Name tag: I've just found out that Name tag is created if driver's chart is deployed with tags without k8sTagClusterId:
tags with k8sTagClusterId configured:
Tested on chart version 2.4.1 P.S. chart's values.yaml comment says |
The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs. This bot triages issues and PRs according to the following rules:
You can:
Please send feedback to sig-contributor-experience at kubernetes/community. /lifecycle stale |
/remove-lifecycle stale |
The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs. This bot triages issues and PRs according to the following rules:
You can:
Please send feedback to sig-contributor-experience at kubernetes/community. /lifecycle stale |
@rdpsin any news about this? |
@pierluigilenoci The driver supports StorageClass tags since v1.6.0. |
@rdpsin then this should be closed. |
I agree. |
Closing this since the CSI driver now supports tagging through StorageClass.parameters since v1.6.0. |
Is your feature request related to a problem? Please describe.
With dynamically provisioned volumes there is always a danger of having orphaned EBS volumes. With current implementation, the only way to tie an EBS volume to a resource in Kubernetes is via the
CSIVolumeName
tag that gets automatically applied or withvolumeHandle
in the PV spec. However, both of these will not work if the Kubernetes resource is deleted and the EBS volume persists (either due theReclaimPolicy
or some bug where the volume is not deleted).Providing a mechanism for operators to tag created volumes with custom tags, would help them in managing those EBS volumes, with this feature a user would be able to tag the volume with team, app, billing code, or whatever other tag they might already be using with other EBS volumes.
Describe the solution you'd like in detail
A new field can be added in the
StorageClass
parameters
tag:The driver would perform a single validation:
additionalTags
does not containCSIVolumeName
, as that is reserved to be used by the driver.Otherwise tags would be passed along to the AWS API request and if any of those validations fail, that error will be propagated back to the user via Kubernetes events.
Describe alternatives you've considered
Another option would be for the user to pass these tags in the PVC spec via an annotation. AFAIK this is not supported by the CSI spec.
Additional context
I might be wrong here but I believe the AWS cloud-provider supports this feature.
The text was updated successfully, but these errors were encountered: