-
Notifications
You must be signed in to change notification settings - Fork 2
fix(vm): recreate unschedulable intvirtvmi's pod #1512
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
Conversation
Reviewer's GuideIntroduces a workaround in the KVVM sync handler to detect VMs stuck in Unschedulable phase, refresh their spec when the NodeSelector is changed, and recreate the associated pod to allow new scheduling. Sequence diagram for handling Unschedulable VirtualMachine with changed NodeSelectorsequenceDiagram
participant H as SyncKvvmHandler
participant VM as VirtualMachine
participant KVVM as InternalKVVM
participant Pod as Pod
participant Equality as equality.Semantic
H->>VM: Check VM status
H->>KVVM: Check KVVM status
alt VM is Unschedulable
H->>H: isNodeSelectorChanged()
H->>KVVM: Get current KVVM
H->>KVVM: Create new KVVM from VM spec
H->>Equality: Compare NodeSelector
alt NodeSelector changed
H->>KVVM: updateKVVM()
H->>Pod: Delete Pod
end
end
Class diagram for updated SyncKvvmHandler methodsclassDiagram
class SyncKvvmHandler {
+syncKVVM(ctx, s)
+isVMUnschedulable(vm, kvvm) bool
+isNodeSelectorChanged(ctx, s) (bool, error)
+updateKVVM(ctx, s) error
}
class VirtualMachineState {
+KVVM(ctx)
}
class VirtualMachine {
Status: Phase
}
class InternalKVVM {
Status: PrintableStatus
Spec: TemplateSpec
}
class TemplateSpec {
NodeSelector
}
SyncKvvmHandler --> VirtualMachineState
SyncKvvmHandler --> VirtualMachine
SyncKvvmHandler --> InternalKVVM
InternalKVVM --> TemplateSpec
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
e0b5355
to
526973b
Compare
Workflow has started. The target step completed with status: success. |
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.
images/virtualization-artifact/pkg/controller/vm/internal/sync_kvvm.go
Outdated
Show resolved
Hide resolved
return false, fmt.Errorf("failed to update internal virtual machine: %w", err) | ||
} | ||
|
||
err = object.DeleteObject(ctx, h.client, pod) |
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.
Why pod? I think, kvvmi will be better
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 thought so too, but when we delete a KVVMI, the KVVM becomes Stopped if the runPolicy is Manual or AlwaysOnUnlessStoppedManually, and the VM remains in the Pending state. This doesn't provide a good user experience, even if we stop the VM in this case.
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.
Hm, we have patch on kubevirt, it will restart kvvm if kvvm has condition restartrequired and kvvmi in pending or scheduling phases. Can you check it, mb we should do nothing
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.
This problem arises without the RestartRequired
condition present.
This workaround is required due to a bug in the KVVM workflow. When a KVVM is created with a non-existent nodeSelector and cannot be scheduled, it remains unschedulable even if the nodeSelector is changed or removed. Signed-off-by: Roman Sysoev <roman.sysoev@flant.com>
Signed-off-by: Roman Sysoev <roman.sysoev@flant.com>
Signed-off-by: Roman Sysoev <roman.sysoev@flant.com>
2d3b9a3
to
2648246
Compare
This workaround is required due to a bug in the KVVM workflow. When a KVVM is created with conflicting placement rules and cannot be scheduled, it remains unschedulable even if these rules are changed or removed. Signed-off-by: Roman Sysoev <roman.sysoev@flant.com> (cherry picked from commit 69d4f1a)
Description
Why do we need it, and what problem does it solve?
This workaround is required due to a bug in the KVVM workflow.
When a KVVM is created with conflicting placement rules and cannot be scheduled, it remains unschedulable even if these rules are changed or removed.
What is the expected result?
A
VirtualMachine
can now be scheduled if it was created with conflicting placement rules that are changed after creation.Checklist