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..5fac8f611 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,66 @@ 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 + } + + // 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). 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 +529,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 +542,55 @@ 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. + // Build the reverse index (vmUUID → hvName) lazily on first miss. + if err := ensureReverseIndex(); 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 + } + + 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. + 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", + "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 +606,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 +652,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) + } +}