-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
In backup sync flow put snapshotHandle as source in CSI VSContent #7924
In backup sync flow put snapshotHandle as source in CSI VSContent #7924
Conversation
Signed-off-by: Anshul Ahuja <anshulahuja@microsoft.com>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #7924 +/- ##
==========================================
- Coverage 58.79% 58.77% -0.03%
==========================================
Files 345 345
Lines 28766 28768 +2
==========================================
- Hits 16914 16908 -6
- Misses 10423 10430 +7
- Partials 1429 1430 +1 ☔ View full report in Codecov by Sentry. |
// For creating static VSContent, we should put snapshothandle in source rather than volume handle. | ||
// Because if VSContent syncs to a different cluster, having volumeHandle will force rePUTs on the snapshot | ||
snapCont.Spec.Source.SnapshotHandle = snapCont.Status.SnapshotHandle | ||
snapCont.Spec.Source.VolumeHandle = nil |
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.
Will the SnapshotHandle
value be unique globally ? Or does it depend on:
- whether the storage across clusters is shared ?
- the csi driver of the storage provider ?
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.
snapshotHandle in my experience is the unique identifier at the cloud provider level.
As long as it's the same cloud provider, same provisioner, then same snapshothandle = same unique content globally.
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.
snapshotHandle for aws provisioner would be the same value as SnapshotId of an ec2 EBS snapshot
"SnapshotId": "snap-066877671789bd71b"
which for the same account credentials should be unique globally regardless of cluster.
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.
My understanding is also that same that it will mostly be unique across
Using a GUID in any naming ensures that.
In case of Azure it's a fully qualified URL with subscription, RG and snapshot name.
But again any GUID should ensure uniqueness.
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.
@shubham-pampattiwar any other thoughts?
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.
If possible, Could you provide a link to the change/PR that is causing the rePUTs on the snapshot ?
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.
tagged on slack
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.
added a few questions
Thank you for contributing to Velero!
Please add a summary of your change
Does your change fix a particular issue?
Fixes #(issue)
https://kubernetes.io/docs/concepts/storage/volume-snapshots/
Context:
We have few customers which have used same storage bucket across clusters, as a result of that their VSContents are syncing across clusters, After the sync the other cluster does not have permission on the snapshot of original cluster, it starts to rePUT the snapshot.
This leads to significant load on the CSI driver.
Please indicate you've done the following:
/kind changelog-not-required
as a comment on this pull request.site/content/docs/main
.