Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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)
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
}

Expand Down Expand Up @@ -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
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
//
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down
Loading