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

Support specifying volume tags via PVC labels or annotations #1876

Open
tsjnsn opened this issue Dec 19, 2023 · 16 comments
Open

Support specifying volume tags via PVC labels or annotations #1876

tsjnsn opened this issue Dec 19, 2023 · 16 comments
Labels
kind/feature Categorizes issue or PR as related to a new feature.

Comments

@tsjnsn
Copy link

tsjnsn commented Dec 19, 2023

Is your feature request related to a problem?/Why is this needed
Applying unique sets of tags based on application requirements currently requires creating a storageclass specific to the application and set of tags. This makes it difficult to maintain tags when there is a lot of variety in keys and values

/feature

Describe the solution you'd like in detail
Ideally, you could specify list of tags in the PVC, that way the storage class can remain application-agnostic. One of the following options should work -

  • controller can inspect a pre-defined annotation or label on the PVC to determine which AWS tags should be applied to the volume
  • the existing templating for StorageClasses can be extended to expose PVC labels/annotations so any custom tags in the PVC can be referenced there

Describe alternatives you've considered

Additional context

@TomBillietKlarrio
Copy link

also: it would allow tags to be changed. Since you can't update storageclasses (they're immutable)

@michealliang123
Copy link

If we can use PVC lable against sc in dynamic provisioning,This is great for maintaining your own tags independently between each project!

@torredil
Copy link
Member

Hi all, thanks for the feature request - we've made a note of this and will evaluate it in the near future.

One key consideration is that there is currently a limitation in Kubernetes where CSI drivers do not have the ability to retrieve tags applied to a PVC from the CreateVolume request passed in by the external provisioner.

We will provide more updates as we make progress on the evaluation and any plans to support this, thanks!

/kind feature

@k8s-ci-robot k8s-ci-robot added the kind/feature Categorizes issue or PR as related to a new feature. label Apr 24, 2024
@ishaankalra
Copy link

@torredil Till then, cant we have the labels/annotation part at least, considering it cannot be changed once created?

@nickvanw
Copy link

If it's useful, we (PlanetScale) worked around this limitation by deploying https://github.com/mtougeron/k8s-pvc-tagger which supports AWS and GCP

@ConnorJC3
Copy link
Contributor

ConnorJC3 commented Jul 11, 2024

Hi all, after significant discussion, we don't currently plan to support this in the driver itself.

The Kubernetes community intentionally does not supply annotations/labels to CSI drivers: kubernetes-csi/external-provisioner#86. Because of this, the only solution would involve a work-around that directly bypasses this design and introduces race condition and correctness concerns.

We do support dynamic tag values based on the PVC name and namespace, and the PV name: https://github.com/kubernetes-sigs/aws-ebs-csi-driver/blob/master/docs/tagging.md#storageclass-tagging. We have seen these features utilized in usecases such as cost tagging and tagging volumes for DLM policies.

We are working on the ability to modify the tags of existing volumes using Kubernetes's VolumeAttributesClass feature. We expect to release this capability sometime in the next few EBS CSI releases. (Note, however, that because VACs are an alpha feature they are not yet supported by EKS. We are engaging with the upstream community to push VAC to beta/GA in future Kubernetes releases.)

@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue as fresh with /remove-lifecycle stale
  • Close this issue with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/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 Oct 9, 2024
@TomBillietKlarrio
Copy link

The VolumeAttributesClass would be a really nice solution. Since it's in Beta now in 1.31, any roadmap on when the CSI driver would support it?
For what it's worth, we currently use https://github.com/mtougeron/k8s-pvc-tagger as a "workaround"

@ConnorJC3
Copy link
Contributor

@TomBillietKlarrio modifying tags via VolumeAttributesClass is available in the EBS CSI Driver today, see https://github.com/kubernetes-sigs/aws-ebs-csi-driver/blob/master/docs/tagging.md#adding-modifying-and-deleting-tags-of-existing-volumes

Using dynamic substitution in those tags as can be done in the StorageClass (with the PVC and PV name/namespace) is not yet supported due to us waiting on a required upstream change, but should be available in a future release of the EBS CSI Driver.

@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue as fresh with /remove-lifecycle rotten
  • Close this issue with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

@k8s-ci-robot k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Nov 9, 2024
@AndrewSirenko
Copy link
Contributor

/remove-lifecycle rotten

Can modify tags via VAC now, see Volume modification via VolumeAttributesClass example.

We're waiting to close issue until #2093 is merged (interpolated tags), which will happen once the next kubernetes-csi/external-resizer is released after K8s v1.32.

@k8s-ci-robot k8s-ci-robot removed the lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. label Nov 11, 2024
@morremeyer
Copy link

morremeyer commented Nov 29, 2024

Hey everyone, just to clarify that I understood everything in here correctly:

  • Dynamic tags are only possible with the StorageClass based on PVC name and namespace, and the PV name
  • Static tags are both possible with the StorageClass and with VAC
  • Dynamic tags based on PVC name and namespace, and the PV name will be supported with VAC later on
  • There is no intention of supporting anything beyond that

If I get all this correctly, what is the recommended way to do configure per-volume tags that can not be inferred from PVC name, namespace or PV name?

We do have a requirement to set tags like e.g. owner for our internal cost attribution, which is not possible with any of the above since the owner is not inferred from the namespace necessarily in our case.

Note: As far as I'm aware, it would technically be possible to create one VAC per volume, but as far as I understand the design, that is not the intended use.

@ConnorJC3
Copy link
Contributor

Dynamic tags are only possible with the StorageClass based on PVC name and namespace, and the PV name

Correct, we are also adding this to VACs but are waiting on a release of the external-resizer sidecar that contains a feature that is needed to implement this feature. After that release occurs, a we will be able to add support for string interpolation during modify as well.

Static tags are both possible with the StorageClass and with VAC

Correct.

Dynamic tags based on PVC name and namespace, and the PV name will be supported with VAC later on

Correct, see my answer to question 1.

There is no intention of supporting anything beyond that

We don't intend to attempt to bypass/workaround the limitations set in place by Kubernetes: kubernetes-csi/external-provisioner#86. If Kubernetes were to change their mind and support per-PVC parameters in some way, we would likely make any necessary changes on our end to support that. If you need/want such a feature, I would strongly encourage asking upstream within Kubernetes for support.

If I get all this correctly, what is the recommended way to do configure per-volume tags that can not be inferred from PVC name, namespace or PV name?

With what is currently supported by Kubernetes, you would need to create a SC/VAC for each volume if the tags are truly unique per volume. In your example of an owner, it would probably make most sense to have an SC for each owner.

@calvinbui
Copy link

calvinbui commented Dec 5, 2024

you would need to create a SC/VAC for each volume if the tags are truly unique per volume

In our scenario, we have one owner/storageClass, but they like to use different tags to specify the environment, project, repo, etc on different volumes.

What would be the best practice here? Ideally I'd like keep only 1 SC and have a VAC for each volume.

@morremeyer
Copy link

@calvinbui If your tags never change, you can go with that approach. Note that a VAC is immutable after creation, so if you ever want to update any of the tags, you would need to create a new one and point the PVC to that new VAC.

While this works in theory, since Volume modifications have a 6 hour cooldown period, you will only be able to do any of these changes once every 6 hours for any volume.

For this reason, we went with using mtougeron/k8s-pvc-tagger as @TomBillietKlarrio has already mentioned, since that is not affected by the 6 hour period as it uses the tagging actions, not the volume modification actions.

@AndrewSirenko
Copy link
Contributor

AndrewSirenko commented Dec 5, 2024

one small correction @morremeyer

Volume modifications have a 6 hour cooldown period

While successfully calling EC2 modify-volume does have a 6 hour cooldown today, tag changes do not have this cooldown.

If the only difference between those 2 VACs are tags, you should not have to wait for any 6 hour cooldown. If you are, then there's a bug somewhere we should squash 😅. See this sequence diagram to understand the sequence of events between K8s, EBS CSI Driver, and AWS

We will likely update our documentation to explicitly call out that tag changes do not have the same cooldown restrictions as other types of modifications.

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

No branches or pull requests