Move pvc annotation removal from CSI RIA to regular PVC RIA#8755
Merged
shubham-pampattiwar merged 1 commit intovelero-io:mainfrom Mar 11, 2025
Merged
Move pvc annotation removal from CSI RIA to regular PVC RIA#8755shubham-pampattiwar merged 1 commit intovelero-io:mainfrom
shubham-pampattiwar merged 1 commit intovelero-io:mainfrom
Conversation
e32ba31 to
419759e
Compare
Combine existing PVC non-CSI RIAs and move annotation removal out of the CSI plugin to fix issues with CSI volumes when using fs-backup Signed-off-by: Scott Seago <sseago@redhat.com>
419759e to
fe14a2c
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #8755 +/- ##
==========================================
+ Coverage 59.53% 59.54% +0.01%
==========================================
Files 371 370 -1
Lines 40196 40168 -28
==========================================
- Hits 23932 23920 -12
+ Misses 14767 14752 -15
+ Partials 1497 1496 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
kaovilai
approved these changes
Mar 5, 2025
Collaborator
There was a problem hiding this comment.
Could perhaps add a comment in this file that more generic actions to make PVC restore successful are in ../pvc_action.go
blackpiglet
reviewed
Mar 6, 2025
blackpiglet
approved these changes
Mar 11, 2025
shubham-pampattiwar
approved these changes
Mar 11, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Combine existing PVC non-CSI RIAs and move annotation removal out of the CSI plugin to fix issues with
CSI volumes when using fs-backup
Thank you for contributing to Velero!
Please add a summary of your change
The CSI RIA plugin removed several annotations on restore. Because the CSI plugin exits early without action when not restoring via CSI snapshot or datamover, these annotations were not removed when CSI volumes were restored via fs-backup. For the linked issue, this meant that restoring to a cluster with a different CSI driver would fail when using fs-backup. These annotations should always be removed, not just when using CSI snapshots, so this code needed to be moved to a regular RIA that runs in all cases.
There were already two non-CSI RIA plugins for PVCs. Rather than create a third one, this PR combines those into one, to avoid needing 3 different grpc calls -- the CSI plugin remains separate.
Because the non-CSI plugin already handled selected node in a more targeted way (already documented process for explicit node change via configmap, and if there is no configmap and the selected node does not exist in restore cluster, then the annotation is removed), the migrated annotation removal functionality no longer applies to the selected node, so now the behavior will match documented behavior in all cases.
Does your change fix a particular issue?
Fixes #8731
Please indicate you've done the following:
make new-changelog) or comment/kind changelog-not-requiredon this PR.site/content/docs/main.