From 312e2dd5f1e055e7a715952cfc7dd62128765000 Mon Sep 17 00:00:00 2001 From: Daniil Antoshin Date: Mon, 6 Jul 2026 09:38:21 +0200 Subject: [PATCH 1/2] fix(core): release inbound migration slot when VMI is deleted The inbound migration limiter caps concurrent inbound live migrations per node with an in-memory slot registry. A slot is only released when the owning VMI reaches a terminal MigrationState, and the KVVMI watcher dropped delete events, so a VMI deleted mid-migration (e.g. cleanup, or a launcher that went unresponsive before the migration failed) leaked its slot forever. With the default limit of 1 per node this starves every subsequent inbound migration to that node: targets wait for a slot past the qemu-timeout and panic with 'timed out waiting for domain to be defined'. Reconcile on VMI delete and release any slot held by that VMI. A deleted VMI cannot have an active migration, so dropping its slots is safe. Signed-off-by: Daniil Antoshin --- .../livemigration/internal/watcher/kvvmi.go | 2 +- .../live_migration_controller.go | 2 +- .../live_migration_reconciler.go | 11 +++++++-- .../pkg/livemigration/inbound_limiter.go | 23 +++++++++++++++++++ .../pkg/livemigration/inbound_limiter_test.go | 21 +++++++++++++++++ 5 files changed, 55 insertions(+), 4 deletions(-) diff --git a/images/virtualization-artifact/pkg/controller/livemigration/internal/watcher/kvvmi.go b/images/virtualization-artifact/pkg/controller/livemigration/internal/watcher/kvvmi.go index 275dac03fd..df67a8fd7d 100644 --- a/images/virtualization-artifact/pkg/controller/livemigration/internal/watcher/kvvmi.go +++ b/images/virtualization-artifact/pkg/controller/livemigration/internal/watcher/kvvmi.go @@ -42,7 +42,7 @@ func (w *KVVMIWatcher) Watch(mgr manager.Manager, ctr controller.Controller) err &handler.TypedEnqueueRequestForObject[*virtv1.VirtualMachineInstance]{}, predicate.TypedFuncs[*virtv1.VirtualMachineInstance]{ CreateFunc: func(e event.TypedCreateEvent[*virtv1.VirtualMachineInstance]) bool { return false }, - DeleteFunc: func(e event.TypedDeleteEvent[*virtv1.VirtualMachineInstance]) bool { return false }, + DeleteFunc: func(e event.TypedDeleteEvent[*virtv1.VirtualMachineInstance]) bool { return true }, UpdateFunc: func(e event.TypedUpdateEvent[*virtv1.VirtualMachineInstance]) bool { return !equality.Semantic.DeepEqual(e.ObjectOld.Status, e.ObjectNew.Status) }, diff --git a/images/virtualization-artifact/pkg/controller/livemigration/live_migration_controller.go b/images/virtualization-artifact/pkg/controller/livemigration/live_migration_controller.go index f92902fa0d..c417ef0dd4 100644 --- a/images/virtualization-artifact/pkg/controller/livemigration/live_migration_controller.go +++ b/images/virtualization-artifact/pkg/controller/livemigration/live_migration_controller.go @@ -52,7 +52,7 @@ func SetupController( handlers := []Handler{ internal.NewDynamicSettingsHandler(client, limiter), } - r := NewReconciler(client, handlers...) + r := NewReconciler(client, limiter, handlers...) c, err := controller.New(ControllerName, mgr, controller.Options{ Reconciler: r, diff --git a/images/virtualization-artifact/pkg/controller/livemigration/live_migration_reconciler.go b/images/virtualization-artifact/pkg/controller/livemigration/live_migration_reconciler.go index e99e32ddf3..3f2b29addc 100644 --- a/images/virtualization-artifact/pkg/controller/livemigration/live_migration_reconciler.go +++ b/images/virtualization-artifact/pkg/controller/livemigration/live_migration_reconciler.go @@ -42,15 +42,21 @@ type Handler interface { Name() string } +type InboundSlotReleaser interface { + ReleaseByVMI(namespace, name string) +} + type Reconciler struct { handlers []Handler client client.Client + limiter InboundSlotReleaser } -func NewReconciler(client client.Client, handlers ...Handler) *Reconciler { +func NewReconciler(client client.Client, limiter InboundSlotReleaser, handlers ...Handler) *Reconciler { return &Reconciler{ handlers: handlers, client: client, + limiter: limiter, } } @@ -80,7 +86,8 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (reco } if kvvmi.IsEmpty() { - log.Info("Reconcile observe an absent VirtualMachineInstance: it may be deleted") + log.Info("Reconcile observe an absent VirtualMachineInstance: it may be deleted; releasing inbound migration slot") + r.limiter.ReleaseByVMI(req.Namespace, req.Name) return reconcile.Result{}, nil } diff --git a/images/virtualization-artifact/pkg/livemigration/inbound_limiter.go b/images/virtualization-artifact/pkg/livemigration/inbound_limiter.go index e80f7ad03c..266c160f56 100644 --- a/images/virtualization-artifact/pkg/livemigration/inbound_limiter.go +++ b/images/virtualization-artifact/pkg/livemigration/inbound_limiter.go @@ -121,6 +121,29 @@ func (l *InboundMigrationLimiter) Release(kvvmi *virtv1.VirtualMachineInstance, } } +// ReleaseByVMI frees any slot held by the given VMI on any node, regardless of +// the migration UID. A deleted VMI can no longer have an active migration, so +// dropping every owner keyed by this VMI is safe. This covers the case where a +// VMI is removed before its migration reaches a terminal state, which would +// otherwise leak the slot and starve subsequent inbound migrations to that node. +func (l *InboundMigrationLimiter) ReleaseByVMI(namespace, name string) { + prefix := namespace + "/" + name + "/" + + l.mu.Lock() + defer l.mu.Unlock() + + for targetNode, owners := range l.slots { + for owner := range owners { + if strings.HasPrefix(owner, prefix) { + delete(owners, owner) + } + } + if len(owners) == 0 { + delete(l.slots, targetNode) + } + } +} + // Restore rebuilds the in-memory registry after a controller restart or leader // change by scanning VMIs annotated with inbound-migration-slot=acquired. // diff --git a/images/virtualization-artifact/pkg/livemigration/inbound_limiter_test.go b/images/virtualization-artifact/pkg/livemigration/inbound_limiter_test.go index d9718ba2d0..9c7402184b 100644 --- a/images/virtualization-artifact/pkg/livemigration/inbound_limiter_test.go +++ b/images/virtualization-artifact/pkg/livemigration/inbound_limiter_test.go @@ -90,6 +90,27 @@ var _ = Describe("InboundMigrationLimiter", func() { Expect(limiter.TryAcquire(newKVVMI("second", "second-migration"), targetNode)).To(BeTrue()) }) + It("Should release the slot by VMI when the migration UID is unknown", func() { + limiter := NewInboundMigrationLimiter(true, 1) + first := newKVVMI("first", "first-migration") + Expect(limiter.TryAcquire(first, targetNode)).To(BeTrue()) + Expect(limiter.TryAcquire(newKVVMI("second", "second-migration"), targetNode)).To(BeFalse()) + + limiter.ReleaseByVMI(namespace, "first") + Expect(limiter.TryAcquire(newKVVMI("second", "second-migration"), targetNode)).To(BeTrue()) + }) + + It("Should not release a different VMI sharing a name prefix", func() { + limiter := NewInboundMigrationLimiter(true, 2) + Expect(limiter.TryAcquire(newKVVMI("vm", "vm-migration"), targetNode)).To(BeTrue()) + Expect(limiter.TryAcquire(newKVVMI("vm-2", "vm-2-migration"), targetNode)).To(BeTrue()) + + limiter.ReleaseByVMI(namespace, "vm") + // "vm-2" must still hold its slot: only one of the two is free now. + Expect(limiter.TryAcquire(newKVVMI("third", "third-migration"), targetNode)).To(BeTrue()) + Expect(limiter.TryAcquire(newKVVMI("fourth", "fourth-migration"), targetNode)).To(BeFalse()) + }) + It("Should respect a limit greater than one", func() { limiter := NewInboundMigrationLimiter(true, 2) Expect(limiter.TryAcquire(newKVVMI("first", "first-migration"), targetNode)).To(BeTrue()) From e288ecf147b0e20854dc2d924cf89c6d6269e832 Mon Sep 17 00:00:00 2001 From: Daniil Antoshin Date: Tue, 7 Jul 2026 09:38:44 +0200 Subject: [PATCH 2/2] refactor(core): drop redundant DeleteFunc and rename ReleaseByVMI to ReleaseByKVVMI Signed-off-by: Daniil Antoshin --- .../pkg/controller/livemigration/internal/watcher/kvvmi.go | 1 - .../pkg/controller/livemigration/live_migration_reconciler.go | 4 ++-- .../pkg/livemigration/inbound_limiter.go | 4 ++-- .../pkg/livemigration/inbound_limiter_test.go | 4 ++-- 4 files changed, 6 insertions(+), 7 deletions(-) diff --git a/images/virtualization-artifact/pkg/controller/livemigration/internal/watcher/kvvmi.go b/images/virtualization-artifact/pkg/controller/livemigration/internal/watcher/kvvmi.go index df67a8fd7d..c7d73e782b 100644 --- a/images/virtualization-artifact/pkg/controller/livemigration/internal/watcher/kvvmi.go +++ b/images/virtualization-artifact/pkg/controller/livemigration/internal/watcher/kvvmi.go @@ -42,7 +42,6 @@ func (w *KVVMIWatcher) Watch(mgr manager.Manager, ctr controller.Controller) err &handler.TypedEnqueueRequestForObject[*virtv1.VirtualMachineInstance]{}, predicate.TypedFuncs[*virtv1.VirtualMachineInstance]{ CreateFunc: func(e event.TypedCreateEvent[*virtv1.VirtualMachineInstance]) bool { return false }, - DeleteFunc: func(e event.TypedDeleteEvent[*virtv1.VirtualMachineInstance]) bool { return true }, UpdateFunc: func(e event.TypedUpdateEvent[*virtv1.VirtualMachineInstance]) bool { return !equality.Semantic.DeepEqual(e.ObjectOld.Status, e.ObjectNew.Status) }, diff --git a/images/virtualization-artifact/pkg/controller/livemigration/live_migration_reconciler.go b/images/virtualization-artifact/pkg/controller/livemigration/live_migration_reconciler.go index 3f2b29addc..6979c74f25 100644 --- a/images/virtualization-artifact/pkg/controller/livemigration/live_migration_reconciler.go +++ b/images/virtualization-artifact/pkg/controller/livemigration/live_migration_reconciler.go @@ -43,7 +43,7 @@ type Handler interface { } type InboundSlotReleaser interface { - ReleaseByVMI(namespace, name string) + ReleaseByKVVMI(namespace, name string) } type Reconciler struct { @@ -87,7 +87,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (reco if kvvmi.IsEmpty() { log.Info("Reconcile observe an absent VirtualMachineInstance: it may be deleted; releasing inbound migration slot") - r.limiter.ReleaseByVMI(req.Namespace, req.Name) + r.limiter.ReleaseByKVVMI(req.Namespace, req.Name) return reconcile.Result{}, nil } diff --git a/images/virtualization-artifact/pkg/livemigration/inbound_limiter.go b/images/virtualization-artifact/pkg/livemigration/inbound_limiter.go index 266c160f56..94461823ea 100644 --- a/images/virtualization-artifact/pkg/livemigration/inbound_limiter.go +++ b/images/virtualization-artifact/pkg/livemigration/inbound_limiter.go @@ -121,12 +121,12 @@ func (l *InboundMigrationLimiter) Release(kvvmi *virtv1.VirtualMachineInstance, } } -// ReleaseByVMI frees any slot held by the given VMI on any node, regardless of +// ReleaseByKVVMI frees any slot held by the given VMI on any node, regardless of // the migration UID. A deleted VMI can no longer have an active migration, so // dropping every owner keyed by this VMI is safe. This covers the case where a // VMI is removed before its migration reaches a terminal state, which would // otherwise leak the slot and starve subsequent inbound migrations to that node. -func (l *InboundMigrationLimiter) ReleaseByVMI(namespace, name string) { +func (l *InboundMigrationLimiter) ReleaseByKVVMI(namespace, name string) { prefix := namespace + "/" + name + "/" l.mu.Lock() diff --git a/images/virtualization-artifact/pkg/livemigration/inbound_limiter_test.go b/images/virtualization-artifact/pkg/livemigration/inbound_limiter_test.go index 9c7402184b..e0b30c8a02 100644 --- a/images/virtualization-artifact/pkg/livemigration/inbound_limiter_test.go +++ b/images/virtualization-artifact/pkg/livemigration/inbound_limiter_test.go @@ -96,7 +96,7 @@ var _ = Describe("InboundMigrationLimiter", func() { Expect(limiter.TryAcquire(first, targetNode)).To(BeTrue()) Expect(limiter.TryAcquire(newKVVMI("second", "second-migration"), targetNode)).To(BeFalse()) - limiter.ReleaseByVMI(namespace, "first") + limiter.ReleaseByKVVMI(namespace, "first") Expect(limiter.TryAcquire(newKVVMI("second", "second-migration"), targetNode)).To(BeTrue()) }) @@ -105,7 +105,7 @@ var _ = Describe("InboundMigrationLimiter", func() { Expect(limiter.TryAcquire(newKVVMI("vm", "vm-migration"), targetNode)).To(BeTrue()) Expect(limiter.TryAcquire(newKVVMI("vm-2", "vm-2-migration"), targetNode)).To(BeTrue()) - limiter.ReleaseByVMI(namespace, "vm") + limiter.ReleaseByKVVMI(namespace, "vm") // "vm-2" must still hold its slot: only one of the two is free now. Expect(limiter.TryAcquire(newKVVMI("third", "third-migration"), targetNode)).To(BeTrue()) Expect(limiter.TryAcquire(newKVVMI("fourth", "fourth-migration"), targetNode)).To(BeFalse())