-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Restart required correctly propagate #11904
Conversation
Removing the `DeepCopy` from updateStatus allow us to detect changes to the Status inside the "sync" and acaccurately represent changes. Signed-off-by: Luboslav Pivarc <lpivarc@redhat.com>
VM CRD has a status subresource enabled. This means that "Update" ignores changes to the status stanza. Also propagate status changes from "hotplug sync" by returning the cloned vm. Signed-off-by: Luboslav Pivarc <lpivarc@redhat.com>
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Otherwise if a VM gets created with RunStrategy == Halted|Manual and then switched to RerunOnFailure the VM will not start. Signed-off-by: Antonio Cardace <acardace@redhat.com>
b13b210
to
8b619e0
Compare
/hold |
/cc |
|
||
// only add the condition if the VM has been started at least once | ||
if vmi != nil { | ||
vm.Status.Conditions = append(vm.Status.Conditions, virtv1.VirtualMachineCondition{ |
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.
can you please use the conditionmanager and the UpdateCondition method
What this PR does
This PR fixes cases where RestartRequired condition is not propagated to Kubernetes API. The cause is
Update
call that should not be called when only.Status
is changed (note if a change was made the condition will be missing). Also ensure that we detect a.Status
change in theupdateStatus
.Fixes #
Special notes for your reviewer
Release note
Based on #11726 and #11836
This PR only adds 8b619e0 on top of #11836 to fix an API regression.