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..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 false }, 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..6979c74f25 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 { + ReleaseByKVVMI(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.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 e80f7ad03c..94461823ea 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, } } +// 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) ReleaseByKVVMI(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..e0b30c8a02 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.ReleaseByKVVMI(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.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()) + }) + It("Should respect a limit greater than one", func() { limiter := NewInboundMigrationLimiter(true, 2) Expect(limiter.TryAcquire(newKVVMI("first", "first-migration"), targetNode)).To(BeTrue())