From 03e163106ef9bf64e34ce3c11fcf5289dbf59679 Mon Sep 17 00:00:00 2001 From: Julius Clausnitzer Date: Wed, 15 Jul 2026 11:30:17 +0200 Subject: [PATCH 1/7] WIP Signed-off-by: juliusclausnitzer --- api/v1alpha1/reservation_types.go | 6 + .../reservations/capacity_accounting.go | 68 ++++ .../reservations/capacity_accounting_test.go | 162 +++++++++ .../commitments/reservation_controller.go | 149 +++++++- .../reservation_controller_test.go | 326 ++++++++++++++++++ 5 files changed, 708 insertions(+), 3 deletions(-) diff --git a/api/v1alpha1/reservation_types.go b/api/v1alpha1/reservation_types.go index f52797654..f4b1b1b56 100644 --- a/api/v1alpha1/reservation_types.go +++ b/api/v1alpha1/reservation_types.go @@ -181,6 +181,12 @@ type ReservationSpec struct { const ( // ReservationConditionReady indicates whether the reservation is active and ready. ReservationConditionReady = "Ready" + + // ReservationConditionVMMisplaced indicates that one or more VMs have been detected + // on a host other than TargetHost (e.g. after a live migration), but the new host + // lacks sufficient capacity to accept the full reservation slot. The VM is tracked in + // its new location but TargetHost is not updated until capacity becomes available. + ReservationConditionVMMisplaced = "VMMisplaced" ) // CommittedResourceReservationStatus defines the status fields specific to committed resource reservations. diff --git a/internal/scheduling/reservations/capacity_accounting.go b/internal/scheduling/reservations/capacity_accounting.go index 2ccca9685..ed5208136 100644 --- a/internal/scheduling/reservations/capacity_accounting.go +++ b/internal/scheduling/reservations/capacity_accounting.go @@ -10,6 +10,74 @@ import ( "github.com/cobaltcore-dev/cortex/api/v1alpha1" ) +// HostHasCapacityForReservation reports whether hv has sufficient remaining capacity to +// accommodate the full Spec.Resources slot of res. +// +// It uses the same accounting as the scheduler's filter_has_enough_capacity: +// 1. Start from EffectiveCapacity (or Capacity when EffectiveCapacity is nil). +// 2. Subtract hv.Status.Allocation (VMs already running on this host). +// 3. For each other reservation in allReservations that is assigned to this host +// (via Spec.TargetHost or Status.Host), subtract its UnusedReservationCapacity. +// 4. Check that the remainder is ≥ res.Spec.Resources for every resource. +// +// The target reservation itself (matched by name) is excluded from the blocking +// calculation so we don't double-count it. +// Returns false when the hypervisor has no capacity data. +func HostHasCapacityForReservation(allReservations []v1alpha1.Reservation, hv hv1.Hypervisor, res *v1alpha1.Reservation) bool { + effCap := hv.Status.EffectiveCapacity + if effCap == nil { + effCap = hv.Status.Capacity + } + if effCap == nil { + return false + } + + free := make(map[hv1.ResourceName]resource.Quantity, len(effCap)) + for rn, qty := range effCap { + free[rn] = qty.DeepCopy() + } + + for rn, allocated := range hv.Status.Allocation { + if f, ok := free[rn]; ok { + f.Sub(allocated) + free[rn] = f + } + } + + for i := range allReservations { + other := &allReservations[i] + if other.Name == res.Name { + continue + } + // Only block resources from reservations that target or are confirmed on this host. + targetsThisHost := other.Spec.TargetHost == hv.Name || other.Status.Host == hv.Name + if !targetsThisHost { + continue + } + for rn, block := range UnusedReservationCapacity(other, false) { + if f, ok := free[rn]; ok { + f.Sub(block) + free[rn] = f + } + } + } + + zero := resource.Quantity{} + for rn, required := range res.Spec.Resources { + remaining, ok := free[rn] + if !ok { + return false + } + if remaining.Cmp(zero) < 0 { + return false + } + if remaining.Cmp(required) < 0 { + return false + } + } + return true +} + // UnusedReservationCapacity returns the resources a Reservation should block on its host(s). // This is the single source of truth used by both the capacity controller and // filter_has_enough_capacity to ensure consistent accounting. diff --git a/internal/scheduling/reservations/capacity_accounting_test.go b/internal/scheduling/reservations/capacity_accounting_test.go index 815a0a07f..db5caa63d 100644 --- a/internal/scheduling/reservations/capacity_accounting_test.go +++ b/internal/scheduling/reservations/capacity_accounting_test.go @@ -8,6 +8,7 @@ import ( hv1 "github.com/cobaltcore-dev/openstack-hypervisor-operator/api/v1" "k8s.io/apimachinery/pkg/api/resource" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "github.com/cobaltcore-dev/cortex/api/v1alpha1" ) @@ -170,3 +171,164 @@ func TestUnusedReservationCapacity(t *testing.T) { }) } } + +func TestHostHasCapacityForReservation(t *testing.T) { + gib := func(n int64) resource.Quantity { return *resource.NewQuantity(n*1024*1024*1024, resource.BinarySI) } + cpu := func(n int64) resource.Quantity { return *resource.NewQuantity(n, resource.DecimalSI) } + + hvWithCapacity := func(name string, memGiB, cpuCores int64) hv1.Hypervisor { + return hv1.Hypervisor{ + ObjectMeta: metav1.ObjectMeta{Name: name}, + Status: hv1.HypervisorStatus{ + EffectiveCapacity: map[hv1.ResourceName]resource.Quantity{ + hv1.ResourceMemory: gib(memGiB), + hv1.ResourceCPU: cpu(cpuCores), + }, + }, + } + } + + resWithSlot := func(name, targetHost string, memGiB, cpuCores int64) v1alpha1.Reservation { + return v1alpha1.Reservation{ + ObjectMeta: metav1.ObjectMeta{Name: name}, + Spec: v1alpha1.ReservationSpec{ + Type: v1alpha1.ReservationTypeCommittedResource, + TargetHost: targetHost, + Resources: map[hv1.ResourceName]resource.Quantity{ + hv1.ResourceMemory: gib(memGiB), + hv1.ResourceCPU: cpu(cpuCores), + }, + }, + Status: v1alpha1.ReservationStatus{Host: targetHost}, + } + } + + tests := []struct { + name string + hv hv1.Hypervisor + res *v1alpha1.Reservation + others []v1alpha1.Reservation + wantFits bool + }{ + { + name: "empty host: slot fits easily", + hv: hvWithCapacity("host-new", 960, 80), + res: func() *v1alpha1.Reservation { r := resWithSlot("res-1", "host-old", 480, 40); return &r }(), + wantFits: true, + }, + { + name: "host fully consumed by another reservation: no capacity", + hv: hvWithCapacity("host-new", 480, 40), + res: func() *v1alpha1.Reservation { r := resWithSlot("res-target", "host-old", 480, 40); return &r }(), + others: []v1alpha1.Reservation{ + resWithSlot("res-blocker", "host-new", 480, 40), + }, + wantFits: false, + }, + { + name: "host partially consumed, enough room left", + hv: hvWithCapacity("host-new", 960, 80), + res: func() *v1alpha1.Reservation { r := resWithSlot("res-target", "host-old", 480, 40); return &r }(), + others: []v1alpha1.Reservation{ + resWithSlot("res-blocker", "host-new", 480, 40), + }, + wantFits: true, + }, + { + name: "host partially consumed, exactly at boundary: fits", + hv: hvWithCapacity("host-new", 960, 80), + res: func() *v1alpha1.Reservation { r := resWithSlot("res-target", "host-old", 480, 40); return &r }(), + others: []v1alpha1.Reservation{ + resWithSlot("res-blocker-a", "host-new", 240, 20), + resWithSlot("res-blocker-b", "host-new", 240, 20), + }, + wantFits: true, + }, + { + name: "host partially consumed, one resource short (CPU)", + hv: hvWithCapacity("host-new", 960, 60), + res: func() *v1alpha1.Reservation { r := resWithSlot("res-target", "host-old", 480, 40); return &r }(), + others: []v1alpha1.Reservation{ + resWithSlot("res-blocker", "host-new", 480, 40), + }, + // 960-480=480 memory OK, but 60-40=20 CPU < 40 required + wantFits: false, + }, + { + name: "target reservation itself excluded from blocking calculation", + hv: hvWithCapacity("host-new", 480, 40), + res: func() *v1alpha1.Reservation { r := resWithSlot("res-target", "host-new", 480, 40); return &r }(), + others: []v1alpha1.Reservation{ + // Same name as res — should be ignored + resWithSlot("res-target", "host-new", 480, 40), + }, + wantFits: true, + }, + { + name: "reservations on other hosts do not count", + hv: hvWithCapacity("host-new", 480, 40), + res: func() *v1alpha1.Reservation { r := resWithSlot("res-target", "host-old", 480, 40); return &r }(), + others: []v1alpha1.Reservation{ + resWithSlot("res-on-other-host", "host-unrelated", 480, 40), + }, + wantFits: true, + }, + { + name: "hv with no capacity data: always false", + hv: hv1.Hypervisor{ + ObjectMeta: metav1.ObjectMeta{Name: "host-nocap"}, + }, + res: func() *v1alpha1.Reservation { r := resWithSlot("res-target", "host-old", 480, 40); return &r }(), + wantFits: false, + }, + { + name: "hv allocation already consumed memory: no room", + hv: hv1.Hypervisor{ + ObjectMeta: metav1.ObjectMeta{Name: "host-new"}, + Status: hv1.HypervisorStatus{ + EffectiveCapacity: map[hv1.ResourceName]resource.Quantity{ + hv1.ResourceMemory: gib(480), + hv1.ResourceCPU: cpu(40), + }, + Allocation: map[hv1.ResourceName]resource.Quantity{ + hv1.ResourceMemory: gib(100), + }, + }, + }, + res: func() *v1alpha1.Reservation { + r := resWithSlot("res-target", "host-old", 480, 40) + return &r + }(), + wantFits: false, // 480-100 = 380 GiB < 480 GiB required + }, + { + name: "reservation targeting via Status.Host (not TargetHost) still blocks", + hv: hvWithCapacity("host-new", 480, 40), + res: func() *v1alpha1.Reservation { r := resWithSlot("res-target", "host-old", 480, 40); return &r }(), + others: []v1alpha1.Reservation{ + // TargetHost empty but Status.Host = host-new + { + ObjectMeta: metav1.ObjectMeta{Name: "res-status-host"}, + Spec: v1alpha1.ReservationSpec{ + Type: v1alpha1.ReservationTypeCommittedResource, + Resources: map[hv1.ResourceName]resource.Quantity{ + hv1.ResourceMemory: gib(480), + hv1.ResourceCPU: cpu(40), + }, + }, + Status: v1alpha1.ReservationStatus{Host: "host-new"}, + }, + }, + wantFits: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := HostHasCapacityForReservation(tt.others, tt.hv, tt.res) + if got != tt.wantFits { + t.Errorf("HostHasCapacityForReservation() = %v, want %v", got, tt.wantFits) + } + }) + } +} diff --git a/internal/scheduling/reservations/commitments/reservation_controller.go b/internal/scheduling/reservations/commitments/reservation_controller.go index 598f3a667..b59b9027a 100644 --- a/internal/scheduling/reservations/commitments/reservation_controller.go +++ b/internal/scheduling/reservations/commitments/reservation_controller.go @@ -386,6 +386,16 @@ type reconcileAllocationsResult struct { // (still spawning), so we skip verification and requeue with a short interval. // For older allocations: we check the HV CRD; VMs not found are considered leaving and // removed from the reservation. +// +// Live migration: when a confirmed VM is absent from the expected host, all HV CRDs are +// scanned to detect whether it moved to a different host. +// - New host has capacity: Spec.TargetHost is updated; the existing Branch B in Reconcile +// will then advance Status.Host on the next cycle. +// - New host has no capacity: TargetHost is left unchanged; the VM's actual location is +// recorded in Status; the VMMisplaced condition is set. +// +// Only the migrated VM's host is considered — other allocated VMs in the reservation are +// not moved, consistent with the single-VM scope defined in issue #373. func (r *CommitmentReservationController) reconcileAllocations(ctx context.Context, res *v1alpha1.Reservation) (*reconcileAllocationsResult, error) { logger := LoggerFromContext(ctx) result := &reconcileAllocationsResult{} @@ -436,11 +446,39 @@ func (r *CommitmentReservationController) reconcileAllocations(ctx context.Conte existingStatusAllocations[k] = v } + // hvList is fetched lazily and only once — only needed when a confirmed VM is missing + // from its expected host and we need to search for it across all hypervisors. + var allHVs *hv1.HypervisorList + // allReservations is fetched lazily — needed alongside allHVs to check capacity. + var allReservations *v1alpha1.ReservationList + + // ensureHVsAndReservations fetches allHVs and allReservations on first call. + ensureHVsAndReservations := func() error { + if allHVs != nil { + return nil + } + allHVs = &hv1.HypervisorList{} + if err := r.List(ctx, allHVs); err != nil { + return fmt.Errorf("failed to list hypervisors: %w", err) + } + allReservations = &v1alpha1.ReservationList{} + if err := r.List(ctx, allReservations); err != nil { + return fmt.Errorf("failed to list reservations: %w", err) + } + return nil + } + // Build new Status.Allocations map based on HV CRD state. newStatusAllocations := make(map[string]string) // Track allocations to remove from Spec (stale/leaving VMs). var allocationsToRemove []string + // migrationTargetHost is set when exactly one confirmed VM is detected on a new host + // that has capacity — in that case we update Spec.TargetHost to the new host. + migrationTargetHost := "" + // misplacedVMs accumulates VM UUIDs that moved to a host without capacity. + var misplacedVMs []string + for vmUUID, allocation := range res.Spec.CommittedResourceReservation.Allocations { allocationAge := now.Sub(allocation.CreationTimestamp.Time) isInGracePeriod := allocationAge < r.Conf.AllocationGracePeriod.Duration @@ -464,7 +502,12 @@ func (r *CommitmentReservationController) reconcileAllocations(ctx context.Conte logger.V(1).Info("verified VM allocation via Hypervisor CRD", "vm", vmUUID, "host", expectedHost) - } else { + continue + } + + // VM not on the expected host. For unconfirmed post-grace VMs this is a clean + // stale allocation — remove it without further searching. + if !isConfirmed { allocationsToRemove = append(allocationsToRemove, vmUUID) logger.Info("removing stale allocation (VM not found on hypervisor)", "vm", vmUUID, @@ -472,6 +515,69 @@ func (r *CommitmentReservationController) reconcileAllocations(ctx context.Conte "expectedHost", expectedHost, "allocationAge", allocationAge, "gracePeriod", r.Conf.AllocationGracePeriod.Duration) + continue + } + + // Confirmed VM missing from expected host — could be a live migration. + // Scan all hypervisors to find where the VM actually landed. + if err := ensureHVsAndReservations(); err != nil { + return nil, err + } + + var foundHost string + for i := range allHVs.Items { + if allHVs.Items[i].Name == expectedHost { + continue // already checked + } + for _, inst := range allHVs.Items[i].Status.Instances { + if inst.ID == vmUUID { + foundHost = allHVs.Items[i].Name + break + } + } + if foundHost != "" { + break + } + } + + if foundHost == "" { + // VM is not on any known hypervisor — it has been terminated or evacuated. + allocationsToRemove = append(allocationsToRemove, vmUUID) + logger.Info("removing confirmed allocation (VM not found on any hypervisor)", + "vm", vmUUID, + "reservation", res.Name, + "expectedHost", expectedHost) + continue + } + + // VM found on a different host — live migration detected. + // Check whether the new host can absorb the full reservation slot. + var foundHV hv1.Hypervisor + for i := range allHVs.Items { + if allHVs.Items[i].Name == foundHost { + foundHV = allHVs.Items[i] + break + } + } + + if reservations.HostHasCapacityForReservation(allReservations.Items, foundHV, res) { + // New host has enough room — follow the VM. + logger.Info("VM live-migrated to host with capacity, updating TargetHost", + "vm", vmUUID, + "reservation", res.Name, + "oldHost", expectedHost, + "newHost", foundHost) + migrationTargetHost = foundHost + newStatusAllocations[vmUUID] = foundHost + } else { + // New host is over capacity — record misplacement; keep old TargetHost. + logger.Info("VM live-migrated to host without sufficient capacity, marking misplaced", + "vm", vmUUID, + "reservation", res.Name, + "expectedHost", expectedHost, + "actualHost", foundHost) + misplacedVMs = append(misplacedVMs, vmUUID) + newStatusAllocations[vmUUID] = foundHost } } @@ -487,10 +593,34 @@ func (r *CommitmentReservationController) reconcileAllocations(ctx context.Conte specChanged = true } + // Update TargetHost when the migrated VM moved to a host with capacity. + // This will be picked up by the TargetHost→Status.Host sync in the next Reconcile + // cycle (Branch B), which advances Status.Host and marks the reservation active on + // the new host. We do NOT update Status.Host here to avoid bypassing that sync path. + if migrationTargetHost != "" { + res.Spec.TargetHost = migrationTargetHost + specChanged = true + } + // Update Status.Allocations res.Status.CommittedResourceReservation.Allocations = newStatusAllocations - // Patch Spec if changed (stale allocations removed) + // Set or clear the VMMisplaced condition. + if len(misplacedVMs) > 0 { + meta.SetStatusCondition(&res.Status.Conditions, metav1.Condition{ + Type: v1alpha1.ReservationConditionVMMisplaced, + Status: metav1.ConditionTrue, + Reason: "MigratedToFullHost", + Message: fmt.Sprintf( + "VM(s) live-migrated to a host that lacks capacity for the reservation slot: %v", + misplacedVMs, + ), + }) + } else { + meta.RemoveStatusCondition(&res.Status.Conditions, v1alpha1.ReservationConditionVMMisplaced) + } + + // Patch Spec if changed (stale allocations removed and/or TargetHost updated) if specChanged { if err := r.Patch(ctx, res, client.MergeFrom(old)); err != nil { if client.IgnoreNotFound(err) == nil { @@ -509,8 +639,21 @@ func (r *CommitmentReservationController) reconcileAllocations(ctx context.Conte // the status update. Otherwise MergeFrom(old) would see no diff // and the status patch would be a no-op. old = res.DeepCopy() - // Re-apply the status update that was overwritten by the re-fetch. + // Re-apply status updates that were overwritten by the re-fetch. res.Status.CommittedResourceReservation.Allocations = newStatusAllocations + if len(misplacedVMs) > 0 { + meta.SetStatusCondition(&res.Status.Conditions, metav1.Condition{ + Type: v1alpha1.ReservationConditionVMMisplaced, + Status: metav1.ConditionTrue, + Reason: "MigratedToFullHost", + Message: fmt.Sprintf( + "VM(s) live-migrated to a host that lacks capacity for the reservation slot: %v", + misplacedVMs, + ), + }) + } else { + meta.RemoveStatusCondition(&res.Status.Conditions, v1alpha1.ReservationConditionVMMisplaced) + } } // Proactively remove this VM UUID from all other candidate reservations that still diff --git a/internal/scheduling/reservations/commitments/reservation_controller_test.go b/internal/scheduling/reservations/commitments/reservation_controller_test.go index 651852c2c..3b0edb281 100644 --- a/internal/scheduling/reservations/commitments/reservation_controller_test.go +++ b/internal/scheduling/reservations/commitments/reservation_controller_test.go @@ -7,6 +7,7 @@ import ( "context" "encoding/json" "errors" + "fmt" "net/http" "net/http/httptest" "testing" @@ -982,3 +983,328 @@ func TestCommitmentReservationController_DomainNameHint(t *testing.T) { }) } } + +// ============================================================================ +// Tests: live migration detection in reconcileAllocations +// ============================================================================ + +// newHVWithCapacity creates a Hypervisor CRD with the given instances and effective capacity. +func newHVWithCapacity(name string, memGiB, cpuCores int64, instances []hv1.Instance) *hv1.Hypervisor { + return &hv1.Hypervisor{ + ObjectMeta: metav1.ObjectMeta{Name: name}, + Status: hv1.HypervisorStatus{ + EffectiveCapacity: map[hv1.ResourceName]resource.Quantity{ + hv1.ResourceMemory: resource.MustParse(fmt.Sprintf("%dGi", memGiB)), + hv1.ResourceCPU: resource.MustParse(fmt.Sprintf("%d", cpuCores)), + }, + Instances: instances, + }, + } +} + +// newConfirmedCRReservation creates a ready CR reservation with one confirmed VM on host. +func newConfirmedCRReservation(name, host, vmUUID string, memGiB, cpuCores int64) *v1alpha1.Reservation { + return &v1alpha1.Reservation{ + ObjectMeta: metav1.ObjectMeta{Name: name}, + Spec: v1alpha1.ReservationSpec{ + Type: v1alpha1.ReservationTypeCommittedResource, + TargetHost: host, + Resources: map[hv1.ResourceName]resource.Quantity{ + hv1.ResourceMemory: resource.MustParse(fmt.Sprintf("%dGi", memGiB)), + hv1.ResourceCPU: resource.MustParse(fmt.Sprintf("%d", cpuCores)), + }, + CommittedResourceReservation: &v1alpha1.CommittedResourceReservationSpec{ + ProjectID: "test-project", + ResourceName: "test-flavor", + Allocations: map[string]v1alpha1.CommittedResourceAllocation{ + vmUUID: { + // allocation age well past any grace period + CreationTimestamp: metav1.NewTime(time.Now().Add(-1 * time.Hour)), + Resources: map[hv1.ResourceName]resource.Quantity{ + hv1.ResourceMemory: resource.MustParse(fmt.Sprintf("%dGi", memGiB)), + hv1.ResourceCPU: resource.MustParse(fmt.Sprintf("%d", cpuCores)), + }, + }, + }, + }, + }, + Status: v1alpha1.ReservationStatus{ + Host: host, + Conditions: []metav1.Condition{ + {Type: v1alpha1.ReservationConditionReady, Status: metav1.ConditionTrue, Reason: "ReservationActive"}, + }, + CommittedResourceReservation: &v1alpha1.CommittedResourceReservationStatus{ + Allocations: map[string]string{vmUUID: host}, + }, + }, + } +} + +// TestReconcileAllocations_LiveMigration_CapacityAvailable verifies that when a confirmed VM +// is detected on a different host that has sufficient capacity, TargetHost is updated to the +// new host and the VM is tracked at its actual location in Status.Allocations. +func TestReconcileAllocations_LiveMigration_CapacityAvailable(t *testing.T) { + scheme := newCRTestScheme(t) + config := ReservationControllerConfig{AllocationGracePeriod: metav1.Duration{Duration: 15 * time.Minute}} + + const ( + vmUUID = "vm-migrated" + oldHost = "host-old" + newHost = "host-new" + ) + + res := newConfirmedCRReservation("res-1", oldHost, vmUUID, 480, 40) + + // Old host: VM is gone. + hvOld := newTestHypervisorCRD(oldHost, []hv1.Instance{}) + + // New host: VM is present, has plenty of capacity, no other reservations blocking it. + hvNew := newHVWithCapacity(newHost, 960, 80, []hv1.Instance{ + {ID: vmUUID, Name: "vm-name", Active: true}, + }) + + k8sClient := newCRTestClient(scheme, res, hvOld, hvNew) + ctrl := &CommitmentReservationController{Client: k8sClient, Scheme: scheme, Conf: config} + ctx := WithNewGlobalRequestID(context.Background()) + + if _, err := ctrl.reconcileAllocations(ctx, res); err != nil { + t.Fatalf("reconcileAllocations() error = %v", err) + } + + var updated v1alpha1.Reservation + if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(res), &updated); err != nil { + t.Fatalf("failed to get updated reservation: %v", err) + } + + // Spec.TargetHost must be updated to the new host. + if updated.Spec.TargetHost != newHost { + t.Errorf("expected Spec.TargetHost=%q, got %q", newHost, updated.Spec.TargetHost) + } + + // VM must still be in Spec.Allocations (not removed). + if _, ok := updated.Spec.CommittedResourceReservation.Allocations[vmUUID]; !ok { + t.Errorf("expected VM %s to remain in Spec.Allocations after migration", vmUUID) + } + + // Status.Allocations must record the VM at its new actual host. + if updated.Status.CommittedResourceReservation == nil { + t.Fatal("expected Status.CommittedResourceReservation to be set") + } + if got := updated.Status.CommittedResourceReservation.Allocations[vmUUID]; got != newHost { + t.Errorf("expected Status.Allocations[%s]=%q, got %q", vmUUID, newHost, got) + } + + // VMMisplaced condition must NOT be set. + if cond := meta.FindStatusCondition(updated.Status.Conditions, v1alpha1.ReservationConditionVMMisplaced); cond != nil && cond.Status == metav1.ConditionTrue { + t.Errorf("expected VMMisplaced condition to be absent or false, got %+v", cond) + } +} + +// TestReconcileAllocations_LiveMigration_NoCapacity verifies that when a confirmed VM is +// detected on a different host that lacks sufficient capacity, TargetHost is left unchanged +// and the VMMisplaced condition is set. +func TestReconcileAllocations_LiveMigration_NoCapacity(t *testing.T) { + scheme := newCRTestScheme(t) + config := ReservationControllerConfig{AllocationGracePeriod: metav1.Duration{Duration: 15 * time.Minute}} + + const ( + vmUUID = "vm-migrated-no-cap" + oldHost = "host-old" + newHost = "host-new-full" + ) + + res := newConfirmedCRReservation("res-1", oldHost, vmUUID, 480, 40) + + // Old host: VM is gone. + hvOld := newTestHypervisorCRD(oldHost, []hv1.Instance{}) + + // New host: VM is present, but another reservation already blocks all capacity. + hvNew := newHVWithCapacity(newHost, 480, 40, []hv1.Instance{ + {ID: vmUUID, Name: "vm-name", Active: true}, + }) + blocker := &v1alpha1.Reservation{ + ObjectMeta: metav1.ObjectMeta{Name: "res-blocker"}, + Spec: v1alpha1.ReservationSpec{ + Type: v1alpha1.ReservationTypeCommittedResource, + TargetHost: newHost, + Resources: map[hv1.ResourceName]resource.Quantity{ + hv1.ResourceMemory: resource.MustParse("480Gi"), + hv1.ResourceCPU: resource.MustParse("40"), + }, + }, + Status: v1alpha1.ReservationStatus{Host: newHost}, + } + + k8sClient := newCRTestClient(scheme, res, hvOld, hvNew, blocker) + ctrl := &CommitmentReservationController{Client: k8sClient, Scheme: scheme, Conf: config} + ctx := WithNewGlobalRequestID(context.Background()) + + if _, err := ctrl.reconcileAllocations(ctx, res); err != nil { + t.Fatalf("reconcileAllocations() error = %v", err) + } + + var updated v1alpha1.Reservation + if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(res), &updated); err != nil { + t.Fatalf("failed to get updated reservation: %v", err) + } + + // Spec.TargetHost must remain unchanged (old host). + if updated.Spec.TargetHost != oldHost { + t.Errorf("expected Spec.TargetHost to remain %q, got %q", oldHost, updated.Spec.TargetHost) + } + + // VM must still be in Spec.Allocations (not removed). + if _, ok := updated.Spec.CommittedResourceReservation.Allocations[vmUUID]; !ok { + t.Errorf("expected VM %s to remain in Spec.Allocations after misplaced migration", vmUUID) + } + + // Status.Allocations must record the VM at its actual (new) host even though we can't follow it. + if updated.Status.CommittedResourceReservation == nil { + t.Fatal("expected Status.CommittedResourceReservation to be set") + } + if got := updated.Status.CommittedResourceReservation.Allocations[vmUUID]; got != newHost { + t.Errorf("expected Status.Allocations[%s]=%q (actual location), got %q", vmUUID, newHost, got) + } + + // VMMisplaced condition must be set to True. + cond := meta.FindStatusCondition(updated.Status.Conditions, v1alpha1.ReservationConditionVMMisplaced) + if cond == nil { + t.Fatal("expected VMMisplaced condition to be set") + } + if cond.Status != metav1.ConditionTrue { + t.Errorf("expected VMMisplaced condition status True, got %s", cond.Status) + } + if cond.Reason != "MigratedToFullHost" { + t.Errorf("expected VMMisplaced reason MigratedToFullHost, got %s", cond.Reason) + } +} + +// TestReconcileAllocations_LiveMigration_VMGone verifies that when a confirmed VM is absent +// from its expected host and cannot be found on any other hypervisor, it is treated as +// terminated and removed from Spec.Allocations normally. +func TestReconcileAllocations_LiveMigration_VMGone(t *testing.T) { + scheme := newCRTestScheme(t) + config := ReservationControllerConfig{AllocationGracePeriod: metav1.Duration{Duration: 15 * time.Minute}} + + const ( + vmUUID = "vm-gone" + oldHost = "host-old" + ) + + res := newConfirmedCRReservation("res-1", oldHost, vmUUID, 480, 40) + + // Old host: VM is gone. No other hypervisor has the VM either. + hvOld := newTestHypervisorCRD(oldHost, []hv1.Instance{}) + hvOther := newTestHypervisorCRD("host-other", []hv1.Instance{ + {ID: "some-other-vm", Name: "other-vm", Active: true}, + }) + + k8sClient := newCRTestClient(scheme, res, hvOld, hvOther) + ctrl := &CommitmentReservationController{Client: k8sClient, Scheme: scheme, Conf: config} + ctx := WithNewGlobalRequestID(context.Background()) + + if _, err := ctrl.reconcileAllocations(ctx, res); err != nil { + t.Fatalf("reconcileAllocations() error = %v", err) + } + + var updated v1alpha1.Reservation + if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(res), &updated); err != nil { + t.Fatalf("failed to get updated reservation: %v", err) + } + + // VM must be removed from Spec.Allocations. + if _, ok := updated.Spec.CommittedResourceReservation.Allocations[vmUUID]; ok { + t.Errorf("expected VM %s to be removed from Spec.Allocations when not found anywhere", vmUUID) + } + + // Status.Allocations must be empty. + if updated.Status.CommittedResourceReservation != nil && + len(updated.Status.CommittedResourceReservation.Allocations) != 0 { + t.Errorf("expected empty Status.Allocations after VM removal, got %v", + updated.Status.CommittedResourceReservation.Allocations) + } + + // Spec.TargetHost must remain unchanged. + if updated.Spec.TargetHost != oldHost { + t.Errorf("expected Spec.TargetHost to remain %q, got %q", oldHost, updated.Spec.TargetHost) + } + + // VMMisplaced condition must NOT be set. + if cond := meta.FindStatusCondition(updated.Status.Conditions, v1alpha1.ReservationConditionVMMisplaced); cond != nil && cond.Status == metav1.ConditionTrue { + t.Errorf("expected VMMisplaced condition to be absent or false after removal, got %+v", cond) + } +} + +// TestReconcileAllocations_LiveMigration_ClearsStaleVMMisplaced verifies that the +// VMMisplaced condition is removed when the misplaced VM is no longer present in +// Spec.Allocations on a subsequent reconcile (e.g. after it was removed by the operator). +func TestReconcileAllocations_LiveMigration_ClearsStaleVMMisplaced(t *testing.T) { + scheme := newCRTestScheme(t) + config := ReservationControllerConfig{AllocationGracePeriod: metav1.Duration{Duration: 15 * time.Minute}} + + const ( + vmUUID = "vm-was-misplaced" + host = "host-1" + ) + + // Reservation still carries the VM in Spec but it's now confirmed back on its host. + // The VMMisplaced condition is stale from a previous cycle. + res := &v1alpha1.Reservation{ + ObjectMeta: metav1.ObjectMeta{Name: "res-1"}, + Spec: v1alpha1.ReservationSpec{ + Type: v1alpha1.ReservationTypeCommittedResource, + TargetHost: host, + Resources: map[hv1.ResourceName]resource.Quantity{ + hv1.ResourceMemory: resource.MustParse("480Gi"), + hv1.ResourceCPU: resource.MustParse("40"), + }, + CommittedResourceReservation: &v1alpha1.CommittedResourceReservationSpec{ + ProjectID: "test-project", + ResourceName: "test-flavor", + Allocations: map[string]v1alpha1.CommittedResourceAllocation{ + vmUUID: { + CreationTimestamp: metav1.NewTime(time.Now().Add(-1 * time.Hour)), + Resources: map[hv1.ResourceName]resource.Quantity{ + hv1.ResourceMemory: resource.MustParse("480Gi"), + hv1.ResourceCPU: resource.MustParse("40"), + }, + }, + }, + }, + }, + Status: v1alpha1.ReservationStatus{ + Host: host, + Conditions: []metav1.Condition{ + {Type: v1alpha1.ReservationConditionReady, Status: metav1.ConditionTrue, Reason: "ReservationActive"}, + {Type: v1alpha1.ReservationConditionVMMisplaced, Status: metav1.ConditionTrue, Reason: "MigratedToFullHost"}, + }, + CommittedResourceReservation: &v1alpha1.CommittedResourceReservationStatus{ + Allocations: map[string]string{vmUUID: host}, + }, + }, + } + + // VM is now back on the expected host. + hv := newTestHypervisorCRD(host, []hv1.Instance{ + {ID: vmUUID, Name: "vm-name", Active: true}, + }) + + k8sClient := newCRTestClient(scheme, res, hv) + ctrl := &CommitmentReservationController{Client: k8sClient, Scheme: scheme, Conf: config} + ctx := WithNewGlobalRequestID(context.Background()) + + if _, err := ctrl.reconcileAllocations(ctx, res); err != nil { + t.Fatalf("reconcileAllocations() error = %v", err) + } + + var updated v1alpha1.Reservation + if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(res), &updated); err != nil { + t.Fatalf("failed to get updated reservation: %v", err) + } + + // VMMisplaced condition must be removed (VM is healthy on its expected host). + cond := meta.FindStatusCondition(updated.Status.Conditions, v1alpha1.ReservationConditionVMMisplaced) + if cond != nil && cond.Status == metav1.ConditionTrue { + t.Errorf("expected VMMisplaced condition to be cleared, still present: %+v", cond) + } +} From 844a38cb657a12cd95fa033d9c41d9a4bc415b74 Mon Sep 17 00:00:00 2001 From: Julius Clausnitzer Date: Thu, 16 Jul 2026 10:24:36 +0200 Subject: [PATCH 2/7] add reverse map for vm-hypervisor Signed-off-by: juliusclausnitzer --- .../commitments/reservation_controller.go | 59 +++++++++++-------- 1 file changed, 36 insertions(+), 23 deletions(-) diff --git a/internal/scheduling/reservations/commitments/reservation_controller.go b/internal/scheduling/reservations/commitments/reservation_controller.go index b59b9027a..5fac8f611 100644 --- a/internal/scheduling/reservations/commitments/reservation_controller.go +++ b/internal/scheduling/reservations/commitments/reservation_controller.go @@ -468,6 +468,33 @@ func (r *CommitmentReservationController) reconcileAllocations(ctx context.Conte return nil } + // vmUUIDToHost and hvByName are built lazily alongside allHVs/allReservations. + // vmUUIDToHost is a reverse index from VM UUID to hypervisor name, covering all + // hypervisors — the same pattern used by vm_source.go and failover/controller.go. + // Building it once amortises the cost across all confirmed-missing VMs in one reconcile. + var vmUUIDToHost map[string]string + var hvByName map[string]hv1.Hypervisor + + ensureReverseIndex := func() error { + if vmUUIDToHost != nil { + return nil + } + if err := ensureHVsAndReservations(); err != nil { + return err + } + vmUUIDToHost = make(map[string]string) + hvByName = make(map[string]hv1.Hypervisor, len(allHVs.Items)) + for _, hv := range allHVs.Items { + hvByName[hv.Name] = hv + for _, inst := range hv.Status.Instances { + if _, seen := vmUUIDToHost[inst.ID]; !seen { + vmUUIDToHost[inst.ID] = hv.Name + } + } + } + return nil + } + // Build new Status.Allocations map based on HV CRD state. newStatusAllocations := make(map[string]string) // Track allocations to remove from Spec (stale/leaving VMs). @@ -519,25 +546,17 @@ func (r *CommitmentReservationController) reconcileAllocations(ctx context.Conte } // Confirmed VM missing from expected host — could be a live migration. - // Scan all hypervisors to find where the VM actually landed. - if err := ensureHVsAndReservations(); err != nil { + // Build the reverse index (vmUUID → hvName) lazily on first miss. + if err := ensureReverseIndex(); err != nil { return nil, err } - var foundHost string - for i := range allHVs.Items { - if allHVs.Items[i].Name == expectedHost { - continue // already checked - } - for _, inst := range allHVs.Items[i].Status.Instances { - if inst.ID == vmUUID { - foundHost = allHVs.Items[i].Name - break - } - } - if foundHost != "" { - break - } + foundHost := vmUUIDToHost[vmUUID] + if foundHost == expectedHost { + // Index says it's still on the expected host — treat as present + // (race between HV CRD update and our per-host set built above). + newStatusAllocations[vmUUID] = expectedHost + continue } if foundHost == "" { @@ -552,13 +571,7 @@ func (r *CommitmentReservationController) reconcileAllocations(ctx context.Conte // VM found on a different host — live migration detected. // Check whether the new host can absorb the full reservation slot. - var foundHV hv1.Hypervisor - for i := range allHVs.Items { - if allHVs.Items[i].Name == foundHost { - foundHV = allHVs.Items[i] - break - } - } + foundHV := hvByName[foundHost] if reservations.HostHasCapacityForReservation(allReservations.Items, foundHV, res) { // New host has enough room — follow the VM. From 9bc0d646a9a1a1c850a85cacb0195eb55a609fd6 Mon Sep 17 00:00:00 2001 From: juliusclausnitzer Date: Tue, 28 Jul 2026 14:42:23 +0200 Subject: [PATCH 3/7] fix Signed-off-by: juliusclausnitzer --- .../commitments/reservation_controller.go | 21 +++++++------------ .../reservation_controller_test.go | 8 +++---- 2 files changed, 12 insertions(+), 17 deletions(-) diff --git a/internal/scheduling/reservations/commitments/reservation_controller.go b/internal/scheduling/reservations/commitments/reservation_controller.go index 5fac8f611..e8c906c39 100644 --- a/internal/scheduling/reservations/commitments/reservation_controller.go +++ b/internal/scheduling/reservations/commitments/reservation_controller.go @@ -382,20 +382,15 @@ type reconcileAllocationsResult struct { // reconcileAllocations verifies all allocations in Spec against actual VM state using the // Hypervisor CRD as the sole source of truth. // -// For new allocations (within grace period): the VM may not yet appear in the HV CRD -// (still spawning), so we skip verification and requeue with a short interval. -// For older allocations: we check the HV CRD; VMs not found are considered leaving and -// removed from the reservation. +// New allocations within the grace period are skipped — the VM may not yet appear in the +// HV CRD while it is still spawning. Older allocations are verified; VMs no longer present +// on their expected host are either followed to a new host (live migration) or removed. // -// Live migration: when a confirmed VM is absent from the expected host, all HV CRDs are -// scanned to detect whether it moved to a different host. -// - New host has capacity: Spec.TargetHost is updated; the existing Branch B in Reconcile -// will then advance Status.Host on the next cycle. -// - New host has no capacity: TargetHost is left unchanged; the VM's actual location is -// recorded in Status; the VMMisplaced condition is set. -// -// Only the migrated VM's host is considered — other allocated VMs in the reservation are -// not moved, consistent with the single-VM scope defined in issue #373. +// When a confirmed VM is absent from its expected host, all HV CRDs are searched to +// determine whether it live-migrated. If the new host has capacity for the reservation +// slot, Spec.TargetHost is updated so the reservation follows the VM. If not, TargetHost +// is left unchanged and the VMMisplaced condition is set. Each VM is evaluated +// independently; other allocated VMs in the same reservation are not affected. func (r *CommitmentReservationController) reconcileAllocations(ctx context.Context, res *v1alpha1.Reservation) (*reconcileAllocationsResult, error) { logger := LoggerFromContext(ctx) result := &reconcileAllocationsResult{} diff --git a/internal/scheduling/reservations/commitments/reservation_controller_test.go b/internal/scheduling/reservations/commitments/reservation_controller_test.go index 3b0edb281..cb0133a29 100644 --- a/internal/scheduling/reservations/commitments/reservation_controller_test.go +++ b/internal/scheduling/reservations/commitments/reservation_controller_test.go @@ -447,7 +447,7 @@ func newTestCRReservation(allocations map[string]metav1.Time) *v1alpha1.Reservat // newTestHypervisorCRD creates a test Hypervisor CRD with instances. // -//nolint:unparam // name parameter allows future test flexibility + func newTestHypervisorCRD(name string, instances []hv1.Instance) *hv1.Hypervisor { return &hv1.Hypervisor{ ObjectMeta: metav1.ObjectMeta{ @@ -995,7 +995,7 @@ func newHVWithCapacity(name string, memGiB, cpuCores int64, instances []hv1.Inst Status: hv1.HypervisorStatus{ EffectiveCapacity: map[hv1.ResourceName]resource.Quantity{ hv1.ResourceMemory: resource.MustParse(fmt.Sprintf("%dGi", memGiB)), - hv1.ResourceCPU: resource.MustParse(fmt.Sprintf("%d", cpuCores)), + hv1.ResourceCPU: resource.MustParse(strconv.FormatInt(cpuCores, 10)), }, Instances: instances, }, @@ -1011,7 +1011,7 @@ func newConfirmedCRReservation(name, host, vmUUID string, memGiB, cpuCores int64 TargetHost: host, Resources: map[hv1.ResourceName]resource.Quantity{ hv1.ResourceMemory: resource.MustParse(fmt.Sprintf("%dGi", memGiB)), - hv1.ResourceCPU: resource.MustParse(fmt.Sprintf("%d", cpuCores)), + hv1.ResourceCPU: resource.MustParse(strconv.FormatInt(cpuCores, 10)), }, CommittedResourceReservation: &v1alpha1.CommittedResourceReservationSpec{ ProjectID: "test-project", @@ -1022,7 +1022,7 @@ func newConfirmedCRReservation(name, host, vmUUID string, memGiB, cpuCores int64 CreationTimestamp: metav1.NewTime(time.Now().Add(-1 * time.Hour)), Resources: map[hv1.ResourceName]resource.Quantity{ hv1.ResourceMemory: resource.MustParse(fmt.Sprintf("%dGi", memGiB)), - hv1.ResourceCPU: resource.MustParse(fmt.Sprintf("%d", cpuCores)), + hv1.ResourceCPU: resource.MustParse(strconv.FormatInt(cpuCores, 10)), }, }, }, From ec9a0c8cb2d639ff17de872387f5e260de3642d2 Mon Sep 17 00:00:00 2001 From: juliusclausnitzer Date: Tue, 28 Jul 2026 15:37:20 +0200 Subject: [PATCH 4/7] fiix Signed-off-by: juliusclausnitzer --- .../reservations/commitments/reservation_controller_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/scheduling/reservations/commitments/reservation_controller_test.go b/internal/scheduling/reservations/commitments/reservation_controller_test.go index cb0133a29..2ec7bff54 100644 --- a/internal/scheduling/reservations/commitments/reservation_controller_test.go +++ b/internal/scheduling/reservations/commitments/reservation_controller_test.go @@ -995,7 +995,7 @@ func newHVWithCapacity(name string, memGiB, cpuCores int64, instances []hv1.Inst Status: hv1.HypervisorStatus{ EffectiveCapacity: map[hv1.ResourceName]resource.Quantity{ hv1.ResourceMemory: resource.MustParse(fmt.Sprintf("%dGi", memGiB)), - hv1.ResourceCPU: resource.MustParse(strconv.FormatInt(cpuCores, 10)), + hv1.ResourceCPU: resource.MustParse(fmt.Sprintf("%d", cpuCores)), }, Instances: instances, }, @@ -1011,7 +1011,7 @@ func newConfirmedCRReservation(name, host, vmUUID string, memGiB, cpuCores int64 TargetHost: host, Resources: map[hv1.ResourceName]resource.Quantity{ hv1.ResourceMemory: resource.MustParse(fmt.Sprintf("%dGi", memGiB)), - hv1.ResourceCPU: resource.MustParse(strconv.FormatInt(cpuCores, 10)), + hv1.ResourceCPU: resource.MustParse(fmt.Sprintf("%d", cpuCores)), }, CommittedResourceReservation: &v1alpha1.CommittedResourceReservationSpec{ ProjectID: "test-project", @@ -1022,7 +1022,7 @@ func newConfirmedCRReservation(name, host, vmUUID string, memGiB, cpuCores int64 CreationTimestamp: metav1.NewTime(time.Now().Add(-1 * time.Hour)), Resources: map[hv1.ResourceName]resource.Quantity{ hv1.ResourceMemory: resource.MustParse(fmt.Sprintf("%dGi", memGiB)), - hv1.ResourceCPU: resource.MustParse(strconv.FormatInt(cpuCores, 10)), + hv1.ResourceCPU: resource.MustParse(fmt.Sprintf("%d", cpuCores)), }, }, }, From bd6101488ac4378c18bb65893e19b93f52b36a5c Mon Sep 17 00:00:00 2001 From: juliusclausnitzer Date: Tue, 28 Jul 2026 16:23:20 +0200 Subject: [PATCH 5/7] fix Signed-off-by: juliusclausnitzer --- .../commitments/reservation_controller_test.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/internal/scheduling/reservations/commitments/reservation_controller_test.go b/internal/scheduling/reservations/commitments/reservation_controller_test.go index 2ec7bff54..c25106158 100644 --- a/internal/scheduling/reservations/commitments/reservation_controller_test.go +++ b/internal/scheduling/reservations/commitments/reservation_controller_test.go @@ -10,6 +10,7 @@ import ( "fmt" "net/http" "net/http/httptest" + "strconv" "testing" "time" @@ -995,7 +996,7 @@ func newHVWithCapacity(name string, memGiB, cpuCores int64, instances []hv1.Inst Status: hv1.HypervisorStatus{ EffectiveCapacity: map[hv1.ResourceName]resource.Quantity{ hv1.ResourceMemory: resource.MustParse(fmt.Sprintf("%dGi", memGiB)), - hv1.ResourceCPU: resource.MustParse(fmt.Sprintf("%d", cpuCores)), + hv1.ResourceCPU: resource.MustParse(strconv.FormatInt(cpuCores, 10)), }, Instances: instances, }, @@ -1011,7 +1012,7 @@ func newConfirmedCRReservation(name, host, vmUUID string, memGiB, cpuCores int64 TargetHost: host, Resources: map[hv1.ResourceName]resource.Quantity{ hv1.ResourceMemory: resource.MustParse(fmt.Sprintf("%dGi", memGiB)), - hv1.ResourceCPU: resource.MustParse(fmt.Sprintf("%d", cpuCores)), + hv1.ResourceCPU: resource.MustParse(strconv.FormatInt(cpuCores, 10)), }, CommittedResourceReservation: &v1alpha1.CommittedResourceReservationSpec{ ProjectID: "test-project", @@ -1022,7 +1023,7 @@ func newConfirmedCRReservation(name, host, vmUUID string, memGiB, cpuCores int64 CreationTimestamp: metav1.NewTime(time.Now().Add(-1 * time.Hour)), Resources: map[hv1.ResourceName]resource.Quantity{ hv1.ResourceMemory: resource.MustParse(fmt.Sprintf("%dGi", memGiB)), - hv1.ResourceCPU: resource.MustParse(fmt.Sprintf("%d", cpuCores)), + hv1.ResourceCPU: resource.MustParse(strconv.FormatInt(cpuCores, 10)), }, }, }, From 7dc91a5f4d8f060d0574a79a1a0328aafc27d6a9 Mon Sep 17 00:00:00 2001 From: juliusclausnitzer Date: Tue, 28 Jul 2026 17:08:49 +0200 Subject: [PATCH 6/7] simplify Signed-off-by: juliusclausnitzer --- .../commitments/reservation_controller.go | 56 +++++++------------ 1 file changed, 19 insertions(+), 37 deletions(-) diff --git a/internal/scheduling/reservations/commitments/reservation_controller.go b/internal/scheduling/reservations/commitments/reservation_controller.go index e8c906c39..18711ef35 100644 --- a/internal/scheduling/reservations/commitments/reservation_controller.go +++ b/internal/scheduling/reservations/commitments/reservation_controller.go @@ -463,33 +463,6 @@ func (r *CommitmentReservationController) reconcileAllocations(ctx context.Conte return nil } - // vmUUIDToHost and hvByName are built lazily alongside allHVs/allReservations. - // vmUUIDToHost is a reverse index from VM UUID to hypervisor name, covering all - // hypervisors — the same pattern used by vm_source.go and failover/controller.go. - // Building it once amortises the cost across all confirmed-missing VMs in one reconcile. - var vmUUIDToHost map[string]string - var hvByName map[string]hv1.Hypervisor - - ensureReverseIndex := func() error { - if vmUUIDToHost != nil { - return nil - } - if err := ensureHVsAndReservations(); err != nil { - return err - } - vmUUIDToHost = make(map[string]string) - hvByName = make(map[string]hv1.Hypervisor, len(allHVs.Items)) - for _, hv := range allHVs.Items { - hvByName[hv.Name] = hv - for _, inst := range hv.Status.Instances { - if _, seen := vmUUIDToHost[inst.ID]; !seen { - vmUUIDToHost[inst.ID] = hv.Name - } - } - } - return nil - } - // Build new Status.Allocations map based on HV CRD state. newStatusAllocations := make(map[string]string) // Track allocations to remove from Spec (stale/leaving VMs). @@ -541,17 +514,28 @@ func (r *CommitmentReservationController) reconcileAllocations(ctx context.Conte } // Confirmed VM missing from expected host — could be a live migration. - // Build the reverse index (vmUUID → hvName) lazily on first miss. - if err := ensureReverseIndex(); err != nil { + // Scan all HVs to find where the VM is now. The list is fetched lazily + // and shared across any further misses in this reconcile cycle. + if err := ensureHVsAndReservations(); err != nil { return nil, err } - foundHost := vmUUIDToHost[vmUUID] - if foundHost == expectedHost { - // Index says it's still on the expected host — treat as present - // (race between HV CRD update and our per-host set built above). - newStatusAllocations[vmUUID] = expectedHost - continue + var foundHost string + var foundHV hv1.Hypervisor + for _, hv := range allHVs.Items { + if hv.Name == expectedHost { + continue // already checked via hvInstanceSet above + } + for _, inst := range hv.Status.Instances { + if inst.ID == vmUUID { + foundHost = hv.Name + foundHV = hv + break + } + } + if foundHost != "" { + break + } } if foundHost == "" { @@ -566,8 +550,6 @@ func (r *CommitmentReservationController) reconcileAllocations(ctx context.Conte // VM found on a different host — live migration detected. // Check whether the new host can absorb the full reservation slot. - foundHV := hvByName[foundHost] - if reservations.HostHasCapacityForReservation(allReservations.Items, foundHV, res) { // New host has enough room — follow the VM. logger.Info("VM live-migrated to host with capacity, updating TargetHost", From c9b86886a5e20476311a582ffbee00120d701b7e Mon Sep 17 00:00:00 2001 From: juliusclausnitzer Date: Tue, 28 Jul 2026 17:19:45 +0200 Subject: [PATCH 7/7] consolidate tests Signed-off-by: juliusclausnitzer --- .../reservation_controller_test.go | 336 +++++------------- 1 file changed, 95 insertions(+), 241 deletions(-) diff --git a/internal/scheduling/reservations/commitments/reservation_controller_test.go b/internal/scheduling/reservations/commitments/reservation_controller_test.go index c25106158..eb05a730a 100644 --- a/internal/scheduling/reservations/commitments/reservation_controller_test.go +++ b/internal/scheduling/reservations/commitments/reservation_controller_test.go @@ -1019,7 +1019,6 @@ func newConfirmedCRReservation(name, host, vmUUID string, memGiB, cpuCores int64 ResourceName: "test-flavor", Allocations: map[string]v1alpha1.CommittedResourceAllocation{ vmUUID: { - // allocation age well past any grace period CreationTimestamp: metav1.NewTime(time.Now().Add(-1 * time.Hour)), Resources: map[hv1.ResourceName]resource.Quantity{ hv1.ResourceMemory: resource.MustParse(fmt.Sprintf("%dGi", memGiB)), @@ -1041,271 +1040,126 @@ func newConfirmedCRReservation(name, host, vmUUID string, memGiB, cpuCores int64 } } -// TestReconcileAllocations_LiveMigration_CapacityAvailable verifies that when a confirmed VM -// is detected on a different host that has sufficient capacity, TargetHost is updated to the -// new host and the VM is tracked at its actual location in Status.Allocations. -func TestReconcileAllocations_LiveMigration_CapacityAvailable(t *testing.T) { - scheme := newCRTestScheme(t) - config := ReservationControllerConfig{AllocationGracePeriod: metav1.Duration{Duration: 15 * time.Minute}} - +func TestReconcileAllocations_LiveMigration(t *testing.T) { const ( - vmUUID = "vm-migrated" + vmUUID = "vm-uuid" oldHost = "host-old" newHost = "host-new" ) - res := newConfirmedCRReservation("res-1", oldHost, vmUUID, 480, 40) - - // Old host: VM is gone. - hvOld := newTestHypervisorCRD(oldHost, []hv1.Instance{}) - - // New host: VM is present, has plenty of capacity, no other reservations blocking it. - hvNew := newHVWithCapacity(newHost, 960, 80, []hv1.Instance{ - {ID: vmUUID, Name: "vm-name", Active: true}, - }) - - k8sClient := newCRTestClient(scheme, res, hvOld, hvNew) - ctrl := &CommitmentReservationController{Client: k8sClient, Scheme: scheme, Conf: config} - ctx := WithNewGlobalRequestID(context.Background()) - - if _, err := ctrl.reconcileAllocations(ctx, res); err != nil { - t.Fatalf("reconcileAllocations() error = %v", err) - } - - var updated v1alpha1.Reservation - if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(res), &updated); err != nil { - t.Fatalf("failed to get updated reservation: %v", err) - } - - // Spec.TargetHost must be updated to the new host. - if updated.Spec.TargetHost != newHost { - t.Errorf("expected Spec.TargetHost=%q, got %q", newHost, updated.Spec.TargetHost) - } - - // VM must still be in Spec.Allocations (not removed). - if _, ok := updated.Spec.CommittedResourceReservation.Allocations[vmUUID]; !ok { - t.Errorf("expected VM %s to remain in Spec.Allocations after migration", vmUUID) - } - - // Status.Allocations must record the VM at its new actual host. - if updated.Status.CommittedResourceReservation == nil { - t.Fatal("expected Status.CommittedResourceReservation to be set") - } - if got := updated.Status.CommittedResourceReservation.Allocations[vmUUID]; got != newHost { - t.Errorf("expected Status.Allocations[%s]=%q, got %q", vmUUID, newHost, got) - } - - // VMMisplaced condition must NOT be set. - if cond := meta.FindStatusCondition(updated.Status.Conditions, v1alpha1.ReservationConditionVMMisplaced); cond != nil && cond.Status == metav1.ConditionTrue { - t.Errorf("expected VMMisplaced condition to be absent or false, got %+v", cond) - } -} - -// TestReconcileAllocations_LiveMigration_NoCapacity verifies that when a confirmed VM is -// detected on a different host that lacks sufficient capacity, TargetHost is left unchanged -// and the VMMisplaced condition is set. -func TestReconcileAllocations_LiveMigration_NoCapacity(t *testing.T) { - scheme := newCRTestScheme(t) config := ReservationControllerConfig{AllocationGracePeriod: metav1.Duration{Duration: 15 * time.Minute}} - const ( - vmUUID = "vm-migrated-no-cap" - oldHost = "host-old" - newHost = "host-new-full" - ) - - res := newConfirmedCRReservation("res-1", oldHost, vmUUID, 480, 40) - - // Old host: VM is gone. - hvOld := newTestHypervisorCRD(oldHost, []hv1.Instance{}) - - // New host: VM is present, but another reservation already blocks all capacity. - hvNew := newHVWithCapacity(newHost, 480, 40, []hv1.Instance{ - {ID: vmUUID, Name: "vm-name", Active: true}, - }) - blocker := &v1alpha1.Reservation{ - ObjectMeta: metav1.ObjectMeta{Name: "res-blocker"}, - Spec: v1alpha1.ReservationSpec{ - Type: v1alpha1.ReservationTypeCommittedResource, - TargetHost: newHost, - Resources: map[hv1.ResourceName]resource.Quantity{ - hv1.ResourceMemory: resource.MustParse("480Gi"), - hv1.ResourceCPU: resource.MustParse("40"), + tests := []struct { + name string + // extra objects beyond the base reservation and old host HV + extraObjects []client.Object + startConditions []metav1.Condition + // expected outcomes + wantTargetHost string + wantStatusHost string // expected in Status.Allocations[vmUUID]; "" means absent + wantSpecHasVM bool + wantVMMisplaced bool + }{ + { + name: "migrated to host with capacity: follow the VM", + extraObjects: []client.Object{ + newHVWithCapacity(newHost, 960, 80, []hv1.Instance{{ID: vmUUID, Active: true}}), }, + wantTargetHost: newHost, + wantStatusHost: newHost, + wantSpecHasVM: true, + wantVMMisplaced: false, }, - Status: v1alpha1.ReservationStatus{Host: newHost}, - } - - k8sClient := newCRTestClient(scheme, res, hvOld, hvNew, blocker) - ctrl := &CommitmentReservationController{Client: k8sClient, Scheme: scheme, Conf: config} - ctx := WithNewGlobalRequestID(context.Background()) - - if _, err := ctrl.reconcileAllocations(ctx, res); err != nil { - t.Fatalf("reconcileAllocations() error = %v", err) - } - - var updated v1alpha1.Reservation - if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(res), &updated); err != nil { - t.Fatalf("failed to get updated reservation: %v", err) - } - - // Spec.TargetHost must remain unchanged (old host). - if updated.Spec.TargetHost != oldHost { - t.Errorf("expected Spec.TargetHost to remain %q, got %q", oldHost, updated.Spec.TargetHost) - } - - // VM must still be in Spec.Allocations (not removed). - if _, ok := updated.Spec.CommittedResourceReservation.Allocations[vmUUID]; !ok { - t.Errorf("expected VM %s to remain in Spec.Allocations after misplaced migration", vmUUID) - } - - // Status.Allocations must record the VM at its actual (new) host even though we can't follow it. - if updated.Status.CommittedResourceReservation == nil { - t.Fatal("expected Status.CommittedResourceReservation to be set") - } - if got := updated.Status.CommittedResourceReservation.Allocations[vmUUID]; got != newHost { - t.Errorf("expected Status.Allocations[%s]=%q (actual location), got %q", vmUUID, newHost, got) - } - - // VMMisplaced condition must be set to True. - cond := meta.FindStatusCondition(updated.Status.Conditions, v1alpha1.ReservationConditionVMMisplaced) - if cond == nil { - t.Fatal("expected VMMisplaced condition to be set") - } - if cond.Status != metav1.ConditionTrue { - t.Errorf("expected VMMisplaced condition status True, got %s", cond.Status) - } - if cond.Reason != "MigratedToFullHost" { - t.Errorf("expected VMMisplaced reason MigratedToFullHost, got %s", cond.Reason) - } -} - -// TestReconcileAllocations_LiveMigration_VMGone verifies that when a confirmed VM is absent -// from its expected host and cannot be found on any other hypervisor, it is treated as -// terminated and removed from Spec.Allocations normally. -func TestReconcileAllocations_LiveMigration_VMGone(t *testing.T) { - scheme := newCRTestScheme(t) - config := ReservationControllerConfig{AllocationGracePeriod: metav1.Duration{Duration: 15 * time.Minute}} - - const ( - vmUUID = "vm-gone" - oldHost = "host-old" - ) - - res := newConfirmedCRReservation("res-1", oldHost, vmUUID, 480, 40) - - // Old host: VM is gone. No other hypervisor has the VM either. - hvOld := newTestHypervisorCRD(oldHost, []hv1.Instance{}) - hvOther := newTestHypervisorCRD("host-other", []hv1.Instance{ - {ID: "some-other-vm", Name: "other-vm", Active: true}, - }) - - k8sClient := newCRTestClient(scheme, res, hvOld, hvOther) - ctrl := &CommitmentReservationController{Client: k8sClient, Scheme: scheme, Conf: config} - ctx := WithNewGlobalRequestID(context.Background()) - - if _, err := ctrl.reconcileAllocations(ctx, res); err != nil { - t.Fatalf("reconcileAllocations() error = %v", err) - } - - var updated v1alpha1.Reservation - if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(res), &updated); err != nil { - t.Fatalf("failed to get updated reservation: %v", err) - } - - // VM must be removed from Spec.Allocations. - if _, ok := updated.Spec.CommittedResourceReservation.Allocations[vmUUID]; ok { - t.Errorf("expected VM %s to be removed from Spec.Allocations when not found anywhere", vmUUID) - } - - // Status.Allocations must be empty. - if updated.Status.CommittedResourceReservation != nil && - len(updated.Status.CommittedResourceReservation.Allocations) != 0 { - t.Errorf("expected empty Status.Allocations after VM removal, got %v", - updated.Status.CommittedResourceReservation.Allocations) - } - - // Spec.TargetHost must remain unchanged. - if updated.Spec.TargetHost != oldHost { - t.Errorf("expected Spec.TargetHost to remain %q, got %q", oldHost, updated.Spec.TargetHost) - } - - // VMMisplaced condition must NOT be set. - if cond := meta.FindStatusCondition(updated.Status.Conditions, v1alpha1.ReservationConditionVMMisplaced); cond != nil && cond.Status == metav1.ConditionTrue { - t.Errorf("expected VMMisplaced condition to be absent or false after removal, got %+v", cond) - } -} - -// TestReconcileAllocations_LiveMigration_ClearsStaleVMMisplaced verifies that the -// VMMisplaced condition is removed when the misplaced VM is no longer present in -// Spec.Allocations on a subsequent reconcile (e.g. after it was removed by the operator). -func TestReconcileAllocations_LiveMigration_ClearsStaleVMMisplaced(t *testing.T) { - scheme := newCRTestScheme(t) - config := ReservationControllerConfig{AllocationGracePeriod: metav1.Duration{Duration: 15 * time.Minute}} - - const ( - vmUUID = "vm-was-misplaced" - host = "host-1" - ) - - // Reservation still carries the VM in Spec but it's now confirmed back on its host. - // The VMMisplaced condition is stale from a previous cycle. - res := &v1alpha1.Reservation{ - ObjectMeta: metav1.ObjectMeta{Name: "res-1"}, - Spec: v1alpha1.ReservationSpec{ - Type: v1alpha1.ReservationTypeCommittedResource, - TargetHost: host, - Resources: map[hv1.ResourceName]resource.Quantity{ - hv1.ResourceMemory: resource.MustParse("480Gi"), - hv1.ResourceCPU: resource.MustParse("40"), - }, - CommittedResourceReservation: &v1alpha1.CommittedResourceReservationSpec{ - ProjectID: "test-project", - ResourceName: "test-flavor", - Allocations: map[string]v1alpha1.CommittedResourceAllocation{ - vmUUID: { - CreationTimestamp: metav1.NewTime(time.Now().Add(-1 * time.Hour)), + { + name: "migrated to full host: mark misplaced, keep TargetHost", + extraObjects: []client.Object{ + newHVWithCapacity(newHost, 480, 40, []hv1.Instance{{ID: vmUUID, Active: true}}), + &v1alpha1.Reservation{ + ObjectMeta: metav1.ObjectMeta{Name: "res-blocker"}, + Spec: v1alpha1.ReservationSpec{ + Type: v1alpha1.ReservationTypeCommittedResource, + TargetHost: newHost, Resources: map[hv1.ResourceName]resource.Quantity{ hv1.ResourceMemory: resource.MustParse("480Gi"), hv1.ResourceCPU: resource.MustParse("40"), }, }, + Status: v1alpha1.ReservationStatus{Host: newHost}, }, }, + wantTargetHost: oldHost, + wantStatusHost: newHost, + wantSpecHasVM: true, + wantVMMisplaced: true, }, - Status: v1alpha1.ReservationStatus{ - Host: host, - Conditions: []metav1.Condition{ - {Type: v1alpha1.ReservationConditionReady, Status: metav1.ConditionTrue, Reason: "ReservationActive"}, + { + name: "VM gone from all hosts: remove allocation", + extraObjects: []client.Object{newTestHypervisorCRD("host-other", []hv1.Instance{{ID: "other-vm"}})}, + wantTargetHost: oldHost, + wantStatusHost: "", + wantSpecHasVM: false, + wantVMMisplaced: false, + }, + { + name: "stale VMMisplaced cleared when VM back on expected host", + startConditions: []metav1.Condition{ {Type: v1alpha1.ReservationConditionVMMisplaced, Status: metav1.ConditionTrue, Reason: "MigratedToFullHost"}, }, - CommittedResourceReservation: &v1alpha1.CommittedResourceReservationStatus{ - Allocations: map[string]string{vmUUID: host}, - }, + // VM is back on oldHost — newHVWithCapacity for oldHost provided via the base setup below + wantTargetHost: oldHost, + wantStatusHost: oldHost, + wantSpecHasVM: true, + wantVMMisplaced: false, }, } - // VM is now back on the expected host. - hv := newTestHypervisorCRD(host, []hv1.Instance{ - {ID: vmUUID, Name: "vm-name", Active: true}, - }) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + scheme := newCRTestScheme(t) + res := newConfirmedCRReservation("res-1", oldHost, vmUUID, 480, 40) + res.Status.Conditions = append(res.Status.Conditions, tt.startConditions...) + + // oldHost HV always has the VM absent (already migrated away), except for the + // "stale VMMisplaced" case where the VM is back. + oldInstances := []hv1.Instance{} + if tt.wantStatusHost == oldHost { + oldInstances = []hv1.Instance{{ID: vmUUID, Active: true}} + } + objects := []client.Object{res, newTestHypervisorCRD(oldHost, oldInstances)} + objects = append(objects, tt.extraObjects...) - k8sClient := newCRTestClient(scheme, res, hv) - ctrl := &CommitmentReservationController{Client: k8sClient, Scheme: scheme, Conf: config} - ctx := WithNewGlobalRequestID(context.Background()) + k8sClient := newCRTestClient(scheme, objects...) + controller := &CommitmentReservationController{Client: k8sClient, Scheme: scheme, Conf: config} + ctx := WithNewGlobalRequestID(context.Background()) - if _, err := ctrl.reconcileAllocations(ctx, res); err != nil { - t.Fatalf("reconcileAllocations() error = %v", err) - } + if _, err := controller.reconcileAllocations(ctx, res); err != nil { + t.Fatalf("reconcileAllocations() error = %v", err) + } - var updated v1alpha1.Reservation - if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(res), &updated); err != nil { - t.Fatalf("failed to get updated reservation: %v", err) - } + var updated v1alpha1.Reservation + if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(res), &updated); err != nil { + t.Fatalf("failed to get updated reservation: %v", err) + } - // VMMisplaced condition must be removed (VM is healthy on its expected host). - cond := meta.FindStatusCondition(updated.Status.Conditions, v1alpha1.ReservationConditionVMMisplaced) - if cond != nil && cond.Status == metav1.ConditionTrue { - t.Errorf("expected VMMisplaced condition to be cleared, still present: %+v", cond) + if updated.Spec.TargetHost != tt.wantTargetHost { + t.Errorf("Spec.TargetHost = %q, want %q", updated.Spec.TargetHost, tt.wantTargetHost) + } + _, specHasVM := updated.Spec.CommittedResourceReservation.Allocations[vmUUID] + if specHasVM != tt.wantSpecHasVM { + t.Errorf("VM in Spec.Allocations = %v, want %v", specHasVM, tt.wantSpecHasVM) + } + var statusHost string + if updated.Status.CommittedResourceReservation != nil { + statusHost = updated.Status.CommittedResourceReservation.Allocations[vmUUID] + } + if statusHost != tt.wantStatusHost { + t.Errorf("Status.Allocations[%s] = %q, want %q", vmUUID, statusHost, tt.wantStatusHost) + } + cond := meta.FindStatusCondition(updated.Status.Conditions, v1alpha1.ReservationConditionVMMisplaced) + isMisplaced := cond != nil && cond.Status == metav1.ConditionTrue + if isMisplaced != tt.wantVMMisplaced { + t.Errorf("VMMisplaced condition = %v, want %v", isMisplaced, tt.wantVMMisplaced) + } + }) } }