fix: correct volumeRestorePolicy assignment in `VirtualMachineRestore#2726
Open
acinko-rh wants to merge 1 commit into
Open
fix: correct volumeRestorePolicy assignment in `VirtualMachineRestore#2726acinko-rh wants to merge 1 commit into
volumeRestorePolicy assignment in `VirtualMachineRestore#2726acinko-rh wants to merge 1 commit into
Conversation
Signed-off-by: Adam Cinko <acinko@redhat.com>
Walkthrough
ChangesVolume Restore Policy Rename
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
|
Report bugs in Issues Welcome! 🎉This pull request will be automatically processed with the following features: 🔄 Automatic Actions
📋 Available CommandsPR Status Management
Review & Approval
Testing & Validation
Cherry-pick Operations
Label Management
✅ Merge RequirementsThis PR will be automatically approved when the following conditions are met:
📊 Review ProcessApprovers and ReviewersApprovers:
Reviewers:
Available Labels
AI Features
💡 Tips
For more information, please refer to the project documentation or contact the maintainers. |
redhat-qe-bot
changed the title
volumeRestorePolicy assignment in VirtualMachineRestore objectvolumeRestorePolicy assignment in `VirtualMachineRestore
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ocp_resources/virtual_machine_restore.py`:
- Line 26: The change in VirtualMachineRestore from volumeNamePolicy to
volumeRestorePolicy is correct per KubeVirt API but is inconsistent with the
VirtualMachineClone implementation; update VirtualMachineClone to use the same
naming and spec key (replace the volume_name_policy parameter and
volumeNamePolicy spec key with volume_restore_policy / volumeRestorePolicy or
another consistent pair), or if the difference is intentional, add a clear
comment/docstring in both VirtualMachineRestore and VirtualMachineClone
explaining the reason; locate symbols VirtualMachineClone, volume_name_policy,
and volumeNamePolicy in the clone class and align them with
VirtualMachineRestore's volume_restore_policy and volumeRestorePolicy.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5bcc1612-ba07-4b04-bc62-9d5ffa24c899
📒 Files selected for processing (1)
ocp_resources/virtual_machine_restore.py
Contributor
|
/lgtm |
rnetser
reviewed
May 20, 2026
| yaml_file=None, | ||
| delete_timeout=TIMEOUT_4MINUTES, | ||
| volume_name_policy=None, | ||
| volume_restore_policy=None, |
Collaborator
There was a problem hiding this comment.
this introdcues a breaking change
how was this verified if this is a wrong name?
Contributor
There was a problem hiding this comment.
It was introduced only 2 weeks ago #2709
I believe @vsibirsk looked at the VMClone and thought it's the same field name for VMRestore, but it's different (and Vasiliy noticed this in the initial review for @acinko-rh's PR in the tests repo)
VirtualMachineRestore.spec.volumeRestorePolicy
VirtualMachineClone.spec.volumeNamePolicy
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
6 participants
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.
Short description:
There was a mistake in naming of one attribute. This provides a fix.
More details:
The attribute was
volume_name_policymapping tovolumeNamePolicyin kubevirt. The actual mapping should bevolume_restore_policytovolumeRestorePolicy.What this PR does / why we need it:
Which issue(s) this PR fixes:
Fixes the assignment of the restore policy in the test code. It actually applies in kubevirt.
Special notes for reviewer:
https://redhat.atlassian.net/browse/CNV-80304
Bug:
Summary by CodeRabbit