Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
CSI support proposal #1661
CSI support proposal #1661
Changes from 14 commits
2f3dd0d
e16f9fe
b608c76
65dfbd9
6cfde00
0f637ee
a65a7d5
27248be
5c78351
93c45ea
360310d
e7ddb0b
7d3ab7b
184973e
b711216
136f3b0
a135590
083c099
2c4c2ca
981ac34
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a case where you'd want to use Velero's current VolumeSnapshotter instead of CSI even if CSI was used to provision the volume? For example, if the CSI driver supported provisioning but not snapshotting?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, that's a good possibility. I know there's gRPC methods for finding out if the driver supports snapshots, but I'd have to check to see if we can query the driver from k8s.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Our in-tree VolumeSnapshotters won't work as-is for CSI volumes, even if they're on the right platform, due to how the GetVolumeID/SetVolumeID functions work (and maybe other things too, not sure). We could look at changing that if we did want to be able to support using them to snapshot CSI volumes, but right now it won't work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update: currently there is no way to query what a driver supports from k8s. So we can't really know if CSI snapshotting is supported via a query to something like the storageclass at the moment.
I would say to simplify things, if CSI volumes are in use but the driver doesn't have snapshotting, restic would be the best option.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@betta1 Antony, is this summary of your experiments with making a VolumeSnapshotter plugin accurate? Is there anything I missed here that you encountered?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nrb yes this is accurate and is in line with our initial experiments with a CSI VolumeSnapshotter plugin -- it's been a while but i remember our biggest issue was that the VolumeSnapshotter interface didn't provide some of the metadata like PVC/NameSpace/StorageClass/ that are required for CSI snapshots. We preferred the approach you had taken with your CSI prototype utilizing Backup and restore Item actions since with item actions we can get access to these metadata. Btw I tested your prototype and had filed an issue (https://github.com/vmware-tanzu/velero-plugin-for-csi/issues/1) -- backup worked ok, ran into some issue with restore.