-
Notifications
You must be signed in to change notification settings - Fork 66
Validate kubevirt-velero-plugin is enabled to ensure VM resource protection #2311
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
base: main
Are you sure you want to change the base?
Conversation
6c8ac7a to
cffbd42
Compare
bf0c9e0 to
1a53e5e
Compare
1a53e5e to
f7846dc
Compare
raaizik
left a comment
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.
Here's my review.
Some more comments:
- It would be good to include some unit tests for the new validation logic — for example, verifying that
isKubeVirtEnabled()correctly detects the plugin when present or missing, and thatrecipeVMBackupValidate()behaves accordingly. This would help catch regressions early. - Is there already documentation in the codebase that covers the requirement for Velero and the kubevirt-velero-plugin when protecting VM resources? If not, it might be worth adding or referencing it in a follow-up PR to make this dependency clearer to users.
accaaf6 to
9b178ce
Compare
|
Small nit. Other than that LGTM. |
9b178ce to
6ad6d73
Compare
8d93e3a to
7b0121e
Compare
7b0121e to
814c77a
Compare
286c806 to
4e99673
Compare
2bfa44f to
58b5294
Compare
internal/controller/vrg_recipe.go
Outdated
| if err := recipeVMBackupValidate(ctx, reader, recipeElements, vrg, ramenConfig, log); err != nil { | ||
| return recipeElements, fmt.Errorf("recipe %v VM backup validation error: %w", recipeNamespacedName.String(), err) | ||
| } | ||
|
|
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.
I don't think this call should happen within RecipeElementsGet. Please move the call outside to the place where you already have the recipe elements.
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.
Done.
58b5294 to
f99e9bb
Compare
2d436cf to
aa2f8e1
Compare
This update introduces validation logic to ensure that the kubevirt-velero-plugin is properly enabled and configured within the Velero deployment for DR protection of VirtualMachines using the internal recipe `vm-recipe`. The plugin is essential for enabling backup and restore operations for KubeVirt-managed VirtualMachines (VMs), DataVolumes (DVs), PersistentVolumeClaims (PVCs), and associated resources. Includes unit tests to verify the correctness of the validation logic. Signed-off-by: pruthvitd <[email protected]>
aa2f8e1 to
9dc09fc
Compare
This PR introduces validation logic to ensure that the kubevirt-velero-plugin is properly enabled and configured in the Velero deployment for Disaster Recovery (DR) protection of VirtualMachines using the internal recipe vm-recipe. The plugin is essential for enabling backup and restore operations for KubeVirt-managed VirtualMachines (VMs), DataVolumes (DVs), PersistentVolumeClaims (PVCs), and associated resources.
Key changes:
Fixes: 2313