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

orthogonal distinction between volumes usable for snapshots vs. volumes that can be created from snapshots #3618

Open
ebblake opened this issue Jan 27, 2025 · 2 comments

Comments

@ebblake
Copy link

ebblake commented Jan 27, 2025

Is your feature request related to a problem? Please describe:

The KubeSAN CSI driver supports both Linear and Thin modes of volumes. A Linear volume uses a shared LVM that can be simultaneously active on many nodes, but cannot be live-extended, does not have an efficient snapshot mechanism, and cannot be cloned any more efficiently than a full-on copy. A Thin volume uses an LVM thin pool plus some backend magic to provide RWX on top of lvm's mandatory exclusive access from one node at a time, but for the sake of CDI, this means it is easy to extend (online or offline), extremely efficient to snapshot (lvm snapshots take less than a second), and possible to clone. We have some ideas how to make cloning faster than a mere copy, but the fact that the upcoming v0.10.0 release is clonable is good enough to use the annotation cdi.kubevirt.io/clone-strategy: csi-clone on a Thin StorageClass, even while a Linear StorageClass must use strategy copy.

As part of supporting snapshots in KubeSAN, it is easy to use CSI calls driven by the usual kubernetes PVC management to create a new Thin volume from either a Snapshot or another Thin volume. And the code used to create a new Thin volume from a source is easy to reuse with Linear volumes. Thus, we have the situation where it is easy to create a Linear volume pre-populated with content from a Thin Volume or Thin Snapshot, even though Linear itself cannot have a snapshot or clone taken.

Describe the solution you'd like:
The existing clone-strategy is unclear whether it refers to the source side or the destination side. KubeSAN is an example where the support is asymmetric: we intentionally have Thin volume with "usable as snapshot or clone source, can be created from snapshot or clone contents", but Linear volume has "cannot create a snapshot or clone, but can be created from snapshot or clone contents" - and it is not obvious which policy is best for the StorageProfile corresponding to a Linear StorageClass.

I think it would be wise to have two orthogonal attributes - one describing whether a given StorageClass can be used as a source (for example, a provider might document that read-only volumes can be sourced but not read-write; in KubeSAN's case, Thin can be a source but not Linear), and one describing whether the StorageClass supports efficient pre-population of a new volume (in KubeSAN's case, both Linear and Thin support this, although Linear won't actually benefit from it unless there is another Thin StorageClass installed at the same time to actually produce such sources).

Describe alternatives you've considered:
For now, KubeSAN will just document that Linear StorageClasses have to use clone-strategy:copy, for lack of any better condition that more accurately describes what it can (or can't) actually do

Additional context:
https://gitlab.com/kubesan/kubesan/-/merge_requests/150#note_2314786232

@ebblake
Copy link
Author

ebblake commented Jan 27, 2025

An alternative might be keeping just the one attribute for clone-strategy, but adding in a fourth enum value applicable to KubeSAN Linear (destination creation from contents can be more efficient than 'copy', but not as full-featured as 'snapshot' or 'csi-clone' that require symmetry on both source and destination support)

@akalenyu
Copy link
Collaborator

Another alternative could be for CDI to have some baked in differentiation logic between the cases.
We actually do that for some provisioners today

"driver.longhorn.io": func(sc *storagev1.StorageClass) string {
migratable := sc.Parameters["migratable"]
if migratable == "true" {
return "driver.longhorn.io/migratable"
}
return "driver.longhorn.io"
},

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

No branches or pull requests

2 participants