diff --git a/internal/scheduling/reservations/capacity_accounting.go b/internal/scheduling/reservations/capacity_accounting.go index 2ccca9685..ab305d5d5 100644 --- a/internal/scheduling/reservations/capacity_accounting.go +++ b/internal/scheduling/reservations/capacity_accounting.go @@ -10,6 +10,71 @@ import ( "github.com/cobaltcore-dev/cortex/api/v1alpha1" ) +// HostHasCapacityForReservation reports whether hv has sufficient remaining capacity to +// absorb res moving to it — i.e. whether the unfilled portion of res's slot fits alongside +// everything already committed on the host. +// +// 1. Start from EffectiveCapacity (or Capacity when EffectiveCapacity is nil). +// 2. Subtract hv.Status.Allocation (VMs physically running on this host). +// 3. For each other reservation assigned to this host (via Spec.TargetHost or Status.Host), +// subtract its UnusedReservationCapacity. +// 4. Check that the remainder is ≥ UnusedReservationCapacity(res): the unfilled portion of +// res's slot. Confirmed VMs in res already appear in hv.Status.Allocation (step 2), so +// comparing against the full slot would count them twice. +// +// res itself is excluded from step 3 to avoid subtracting its own block from free capacity. +// 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 && other.Namespace == res.Namespace { + 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 resourceName, block := range UnusedReservationCapacity(other, false) { + if f, ok := free[resourceName]; ok { + f.Sub(block) + free[resourceName] = f + } + } + } + + for resourceName, required := range UnusedReservationCapacity(res, false) { + remaining, ok := free[resourceName] + if !ok { + 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..d13a1c400 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,162 @@ 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}, + } + } + deref := func(r *v1alpha1.Reservation) v1alpha1.Reservation { return *r } + + 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: resWithSlot("res-1", "host-old", 480, 40), + wantFits: true, + }, + { + name: "host fully consumed by another reservation: no capacity", + hv: hvWithCapacity("host-new", 480, 40), + res: resWithSlot("res-target", "host-old", 480, 40), + others: []v1alpha1.Reservation{ + deref(resWithSlot("res-blocker", "host-new", 480, 40)), + }, + wantFits: false, + }, + { + name: "host partially consumed, enough room left", + hv: hvWithCapacity("host-new", 960, 80), + res: resWithSlot("res-target", "host-old", 480, 40), + others: []v1alpha1.Reservation{ + deref(resWithSlot("res-blocker", "host-new", 480, 40)), + }, + wantFits: true, + }, + { + name: "host partially consumed, exactly at boundary: fits", + hv: hvWithCapacity("host-new", 960, 80), + res: resWithSlot("res-target", "host-old", 480, 40), + others: []v1alpha1.Reservation{ + deref(resWithSlot("res-blocker-a", "host-new", 240, 20)), + deref(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: resWithSlot("res-target", "host-old", 480, 40), + others: []v1alpha1.Reservation{ + deref(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: resWithSlot("res-target", "host-new", 480, 40), + others: []v1alpha1.Reservation{ + // Same name as res — should be ignored + deref(resWithSlot("res-target", "host-new", 480, 40)), + }, + wantFits: true, + }, + { + name: "reservations on other hosts do not count", + hv: hvWithCapacity("host-new", 480, 40), + res: resWithSlot("res-target", "host-old", 480, 40), + others: []v1alpha1.Reservation{ + deref(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: resWithSlot("res-target", "host-old", 480, 40), + 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: resWithSlot("res-target", "host-old", 480, 40), + wantFits: false, // 480-100 = 380 GiB remaining < 480 GiB slot (no confirmed VMs, so full slot is required) + }, + { + name: "reservation targeting via Status.Host (not TargetHost) still blocks", + hv: hvWithCapacity("host-new", 480, 40), + res: resWithSlot("res-target", "host-old", 480, 40), + 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..f6bf5a385 100644 --- a/internal/scheduling/reservations/commitments/reservation_controller.go +++ b/internal/scheduling/reservations/commitments/reservation_controller.go @@ -382,10 +382,16 @@ 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 handled as follows: +// +// Live migration: when a confirmed VM is found on a different host, the reservation follows +// it only when the reservation has exactly one allocated VM and the new host has capacity. +// In all other cases (multiple VMs, or new host at capacity), the migrated VM is removed +// from the reservation so the slot remains available for re-use on the original host. +// Moving TargetHost when other VMs are present would cause those remaining VMs to appear +// misplaced on the next reconcile cycle. func (r *CommitmentReservationController) reconcileAllocations(ctx context.Context, res *v1alpha1.Reservation) (*reconcileAllocationsResult, error) { logger := LoggerFromContext(ctx) result := &reconcileAllocationsResult{} @@ -436,11 +442,37 @@ func (r *CommitmentReservationController) reconcileAllocations(ctx context.Conte existingStatusAllocations[k] = v } + // allHVs and allReservations are fetched lazily — only needed when a confirmed VM is + // missing from its expected host and we need to scan for a live migration. + var allHVs *hv1.HypervisorList + var allReservations *v1alpha1.ReservationList + + ensureHVsAndReservations := func() error { + if allHVs != nil && allReservations != nil { + return nil + } + hvs := &hv1.HypervisorList{} + if err := r.List(ctx, hvs); err != nil { + return fmt.Errorf("failed to list hypervisors: %w", err) + } + res := &v1alpha1.ReservationList{} + if err := r.List(ctx, res); err != nil { + return fmt.Errorf("failed to list reservations: %w", err) + } + allHVs = hvs + allReservations = res + 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 the reservation has exactly one VM, that VM + // live-migrated to a new host, and the new host has capacity. + migrationTargetHost := "" + for vmUUID, allocation := range res.Spec.CommittedResourceReservation.Allocations { allocationAge := now.Sub(allocation.CreationTimestamp.Time) isInGracePeriod := allocationAge < r.Conf.AllocationGracePeriod.Duration @@ -464,7 +496,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 +509,73 @@ 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 HVs lazily; the list is shared across any further misses this cycle. + if err := ensureHVsAndReservations(); err != nil { + return nil, err + } + + 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 == "" { + // VM is not on any known hypervisor. This covers two cases: + // 1. The VM was terminated or evacuated — correct to remove. + // 2. The VM is mid-live-migration: it has left host-old's HV CRD but + // host-new's CRD has not been updated yet. In this window the VM + // is incorrectly treated as gone and removed from the reservation. + // A VM CRD with lifecycle state (migrating/active) would close this + // gap; without one we accept this narrow race as a known limitation. + 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. + // + // Follow the VM only when this is the sole VM in the reservation and the new + // host has capacity. Moving TargetHost with multiple VMs present would cause + // the remaining VMs to appear misplaced on the next reconcile. When there are + // multiple VMs, or the new host is at capacity, remove this VM so the slot + // on the original host remains available for re-use. + isSingleVM := len(res.Spec.CommittedResourceReservation.Allocations) == 1 + if isSingleVM && reservations.HostHasCapacityForReservation(allReservations.Items, foundHV, res) { + 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 { + logger.Info("removing VM from reservation after live migration: either multiple VMs present or new host lacks capacity", + "vm", vmUUID, + "reservation", res.Name, + "expectedHost", expectedHost, + "actualHost", foundHost, + "singleVM", isSingleVM) + allocationsToRemove = append(allocationsToRemove, vmUUID) } } @@ -487,10 +591,19 @@ func (r *CommitmentReservationController) reconcileAllocations(ctx context.Conte specChanged = true } + // Advance both TargetHost and Status.Host in the same patch cycle to avoid a + // transient state where Status.Host lags behind TargetHost and blocks capacity + // accounting on the old host during the next reconcile. + if migrationTargetHost != "" { + res.Spec.TargetHost = migrationTargetHost + res.Status.Host = migrationTargetHost + specChanged = true + } + // Update Status.Allocations res.Status.CommittedResourceReservation.Allocations = newStatusAllocations - // Patch Spec if changed (stale allocations removed) + // 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 +622,11 @@ 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 migrationTargetHost != "" { + res.Status.Host = migrationTargetHost + } } // 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..a776ac0cf 100644 --- a/internal/scheduling/reservations/commitments/reservation_controller_test.go +++ b/internal/scheduling/reservations/commitments/reservation_controller_test.go @@ -7,8 +7,10 @@ import ( "context" "encoding/json" "errors" + "fmt" "net/http" "net/http/httptest" + "strconv" "testing" "time" @@ -446,7 +448,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{ @@ -982,3 +984,235 @@ 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(strconv.FormatInt(cpuCores, 10)), + }, + Instances: instances, + }, + } +} + +// newConfirmedCRReservation creates a ready CR reservation with one confirmed VM on host. +// slotMemGiB/slotCPU define the full reservation slot; vmMemGiB/vmCPU define what the VM +// actually consumes — these may be smaller, leaving an unfilled remainder in the slot. +func newConfirmedCRReservation(name, host, vmUUID string, slotMemGiB, slotCPU, vmMemGiB, vmCPU 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", slotMemGiB)), + hv1.ResourceCPU: resource.MustParse(strconv.FormatInt(slotCPU, 10)), + }, + 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(fmt.Sprintf("%dGi", vmMemGiB)), + hv1.ResourceCPU: resource.MustParse(strconv.FormatInt(vmCPU, 10)), + }, + }, + }, + }, + }, + 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}, + }, + }, + } +} + +func TestReconcileAllocations_LiveMigration(t *testing.T) { + const ( + vmUUID = "vm-uuid" + vm2UUID = "vm-uuid-2" + oldHost = "host-old" + newHost = "host-new" + ) + + config := ReservationControllerConfig{AllocationGracePeriod: metav1.Duration{Duration: 15 * time.Minute}} + + tests := []struct { + name string + // reservation to use; nil uses the default single-VM reservation + reservation *v1alpha1.Reservation + // extra objects beyond the base reservation and old host HV + extraObjects []client.Object + // expected outcomes after first reconcile pass + wantTargetHost string + wantStatusHost string // expected in Status.Allocations[vmUUID]; "" means absent + wantSpecHasVM bool + // if true, run a second reconcile pass and assert state is stable + assertSecondPass bool + }{ + { + name: "single VM, new host has capacity: follow the VM", + extraObjects: []client.Object{ + newHVWithCapacity(newHost, 960, 80, []hv1.Instance{{ID: vmUUID, Active: true}}), + }, + wantTargetHost: newHost, + wantStatusHost: newHost, + wantSpecHasVM: true, + assertSecondPass: true, + }, + { + name: "single VM, new host at capacity: remove VM, slot stays on old host", + extraObjects: []client.Object{ + // VM (240Gi/20) running on newHost; a full-slot blocker leaves no room for + // the 240Gi/20 slot remainder. + 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: "", + wantSpecHasVM: false, + }, + { + name: "multiple VMs, one migrated: remove migrated VM, never update TargetHost", + reservation: func() *v1alpha1.Reservation { + res := newConfirmedCRReservation("res-1", oldHost, vmUUID, 480, 40, 240, 20) + // Add a second VM confirmed on oldHost. + res.Spec.CommittedResourceReservation.Allocations[vm2UUID] = v1alpha1.CommittedResourceAllocation{ + CreationTimestamp: metav1.NewTime(time.Now().Add(-1 * time.Hour)), + Resources: map[hv1.ResourceName]resource.Quantity{ + hv1.ResourceMemory: resource.MustParse("240Gi"), + hv1.ResourceCPU: resource.MustParse("20"), + }, + } + res.Status.CommittedResourceReservation.Allocations[vm2UUID] = oldHost + return res + }(), + extraObjects: []client.Object{ + // vmUUID has migrated to newHost with plenty of capacity; vm2UUID stays on oldHost. + // Supply oldHost HV explicitly so vm2UUID is present on it. + newTestHypervisorCRD(oldHost, []hv1.Instance{{ID: vm2UUID, Active: true}}), + newHVWithCapacity(newHost, 960, 80, []hv1.Instance{{ID: vmUUID, Active: true}}), + }, + wantTargetHost: oldHost, + wantStatusHost: "", + wantSpecHasVM: false, + }, + { + name: "VM gone from all hosts: remove allocation", + extraObjects: []client.Object{newTestHypervisorCRD("host-other", []hv1.Instance{{ID: "other-vm"}})}, + wantTargetHost: oldHost, + wantStatusHost: "", + wantSpecHasVM: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + scheme := newCRTestScheme(t) + + res := tt.reservation + if res == nil { + res = newConfirmedCRReservation("res-1", oldHost, vmUUID, 480, 40, 240, 20) + } + + var objects []client.Object + objects = append(objects, res) + + // Add an oldHost HV unless the test provides its own via extraObjects. + addsOldHostHV := false + for _, obj := range tt.extraObjects { + if hv, ok := obj.(*hv1.Hypervisor); ok && hv.Name == oldHost { + addsOldHostHV = true + break + } + } + if !addsOldHostHV { + objects = append(objects, newTestHypervisorCRD(oldHost, []hv1.Instance{})) + } + objects = append(objects, tt.extraObjects...) + + k8sClient := newCRTestClient(scheme, objects...) + controller := &CommitmentReservationController{Client: k8sClient, Scheme: scheme, Conf: config} + ctx := WithNewGlobalRequestID(context.Background()) + + 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) + } + + 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) + } + + // For the migration-follow case: run a second reconcile to confirm state is + // stable and Status.Host was advanced to the new host in the first pass. + if tt.assertSecondPass { + if _, err := controller.reconcileAllocations(ctx, &updated); err != nil { + t.Fatalf("second reconcileAllocations() error = %v", err) + } + var updated2 v1alpha1.Reservation + if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(res), &updated2); err != nil { + t.Fatalf("failed to get reservation after second pass: %v", err) + } + if updated2.Spec.TargetHost != tt.wantTargetHost { + t.Errorf("second pass: Spec.TargetHost = %q, want %q", updated2.Spec.TargetHost, tt.wantTargetHost) + } + if updated2.Status.Host != tt.wantTargetHost { + t.Errorf("second pass: Status.Host = %q, want %q", updated2.Status.Host, tt.wantTargetHost) + } + _, specHasVM2 := updated2.Spec.CommittedResourceReservation.Allocations[vmUUID] + if !specHasVM2 { + t.Errorf("second pass: VM unexpectedly removed from Spec.Allocations") + } + var statusHost2 string + if updated2.Status.CommittedResourceReservation != nil { + statusHost2 = updated2.Status.CommittedResourceReservation.Allocations[vmUUID] + } + if statusHost2 != tt.wantStatusHost { + t.Errorf("second pass: Status.Allocations[%s] = %q, want %q", vmUUID, statusHost2, tt.wantStatusHost) + } + } + }) + } +}