instancetype: Ignore unexpected existing CRs during restore #8891
Conversation
Signed-off-by: Lee Yarwood <lyarwood@redhat.com>
5cc62c0 failed to accommodate repeat attempts to reconcile a VirtualMachineRestore that in turn lead to multiple calls to restoreInstancetypeControllerRevision being made for the same ControllerRevision. This change handles this case by ignoring any existing ControllerRevisions found during the restore, assuming that the existing ControllerRevision was created by a previous call to restoreInstancetypeControllerRevision. Future changes will check the contents of this ControllerRevision against that of the VirtualMachineSnapshot to ensure they match. Signed-off-by: Lee Yarwood <lyarwood@redhat.com>
kubevirt-bot
added
release-note-none
|
/cherrypick release-0.58 This is fallout from #8466 and was uncovered downstream when an unrelated issue in a test environment caused the creation of a |
|
@lyarwood: once the present PR merges, I will cherry-pick it on top of release-0.58 in a new PR and assign it to you. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
/retest |
1 similar comment
|
/retest |
There was a problem hiding this comment.
Thanks @lyarwood, looks good to me and totally makes sense.
Left a tiny nit
| originalVM.Spec.Instancetype = getVMInstancetypeMatcher() | ||
| originalVM.Spec.Preference = getVMPreferenceMatcher() |
There was a problem hiding this comment.
nit: why do we need these functions? can't we simply pass the matcher directly?
There was a problem hiding this comment.
Yeah we can, this is something I need to go back and clean up in a follow up, at the time I originally wrote these tests I was having a hard time understanding how to pass things into tables. This worked and made it past the review for the original PR so I've stuck with it here so this change can be backported.
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mhenriks The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
kubevirt-bot
added
the
approved
|
@lyarwood: The following test failed, say
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
|
/retest |
|
@lyarwood: new pull request created: #8912 In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
/area instancetype
What this PR does / why we need it:
5cc62c0 failed to accommodate repeat
attempts to reconcile a VirtualMachineRestore that in turn lead to
multiple calls to restoreInstancetypeControllerRevision being made for
the same ControllerRevision.
This change handles this case by ignoring any existing
ControllerRevisions found during the restore, assuming that the existing
ControllerRevision was created by a previous call to
restoreInstancetypeControllerRevision.
Future changes will check the contents of this ControllerRevision
against that of the VirtualMachineSnapshot to ensure they match.
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close the issue(s) when PR gets merged):Fixes #8890
Special notes for your reviewer:
Release note: