diff --git a/internal/scheduling/nova/plugins/filters/filter_has_enough_capacity.go b/internal/scheduling/nova/plugins/filters/filter_has_enough_capacity.go index 5b471f789..88e2f07d5 100644 --- a/internal/scheduling/nova/plugins/filters/filter_has_enough_capacity.go +++ b/internal/scheduling/nova/plugins/filters/filter_has_enough_capacity.go @@ -41,6 +41,16 @@ type FilterHasEnoughCapacity struct { // // In case the project and flavor match, space reserved is unlocked (slotting). // +// Capacity accounting uses two sources: hv.Status.Allocation (aggregate real-time usage of +// all running VMs) and Reservation.Status.Allocations (which VMs are confirmed on a slot, +// maintained by the reservation controller with a one-reconcile-cycle lag). During the window +// between a VM starting and the reservation controller reconciling, a VM appears in both +// sources — a conservative transient over-count that self-corrects on the next reconcile. +// +// During a CR reservation migration (TargetHost != Status.Host), both the source and target +// host are blocked with the full slot. The source block is intentionally conservative to +// preserve rollback capacity if the migration fails. +// // Please note that, if num_instances is larger than 1, there needs to be enough // capacity to place all instances on the same host. This limitation is necessary // because we can't spread out instances, as the final set of valid hosts is not @@ -170,41 +180,61 @@ func (s *FilterHasEnoughCapacity) Run(traceLog *slog.Logger, request api.Externa continue } - // For CR reservations with allocations, calculate remaining (unallocated) resources to block. - // This prevents double-blocking of resources already consumed by running instances. + // For CR reservations with allocations, compute the effective block: + // confirmed = sum of resources for VMs present in both Spec and Status allocations + // specOnly = sum of resources for VMs present in Spec but not yet in Status + // remaining = max(0, Spec.Resources - confirmed) [clamped: never negative] + // block = max(remaining, specOnly) [spec-only VM must be fully covered] + // + // Clamping: if confirmed VMs exceed slot size (e.g. after resize), block = 0. + // Oversize spec-only: if a pending VM is larger than the remaining slot, block its full size. var resourcesToBlock map[hv1.ResourceName]resource.Quantity if reservation.Spec.Type == v1alpha1.ReservationTypeCommittedResource && // if the reservation is not being migrated, block only unused resources reservation.Spec.TargetHost == reservation.Status.Host && reservation.Spec.CommittedResourceReservation != nil && - reservation.Status.CommittedResourceReservation != nil && - len(reservation.Spec.CommittedResourceReservation.Allocations) > 0 && - len(reservation.Status.CommittedResourceReservation.Allocations) > 0 { - // Start with full reservation resources - resourcesToBlock = make(map[hv1.ResourceName]resource.Quantity) - for k, v := range reservation.Spec.Resources { - resourcesToBlock[k] = v.DeepCopy() + len(reservation.Spec.CommittedResourceReservation.Allocations) > 0 { + confirmedResources := make(map[hv1.ResourceName]resource.Quantity) + specOnlyResources := make(map[hv1.ResourceName]resource.Quantity) + + statusAllocs := map[string]string{} + if reservation.Status.CommittedResourceReservation != nil { + statusAllocs = reservation.Status.CommittedResourceReservation.Allocations } - // Subtract already-allocated resources because those consume already resources on the host for instanceUUID, allocation := range reservation.Spec.CommittedResourceReservation.Allocations { - // Only subtract if allocation is already present in status (VM is actually running) - if _, isRunning := reservation.Status.CommittedResourceReservation.Allocations[instanceUUID]; !isRunning { - continue - } - + _, isConfirmed := statusAllocs[instanceUUID] for resourceName, quantity := range allocation.Resources { - if current, ok := resourcesToBlock[resourceName]; ok { - current.Sub(quantity) - resourcesToBlock[resourceName] = current - traceLog.Debug("subtracting allocated resources from reservation", - "reservation", reservation.Name, - "instanceUUID", instanceUUID, - "resource", resourceName, - "quantity", quantity.String()) + if isConfirmed { + existing := confirmedResources[resourceName] + existing.Add(quantity) + confirmedResources[resourceName] = existing + } else { + existing := specOnlyResources[resourceName] + existing.Add(quantity) + specOnlyResources[resourceName] = existing } } } + + resourcesToBlock = make(map[hv1.ResourceName]resource.Quantity) + zero := resource.Quantity{} + for resourceName, slotSize := range reservation.Spec.Resources { + confirmed := confirmedResources[resourceName] + specOnly := specOnlyResources[resourceName] + + remaining := slotSize.DeepCopy() + remaining.Sub(confirmed) + if remaining.Cmp(zero) < 0 { + remaining = zero.DeepCopy() + } + + if specOnly.Cmp(remaining) > 0 { + resourcesToBlock[resourceName] = specOnly.DeepCopy() + } else { + resourcesToBlock[resourceName] = remaining + } + } } else { // For other reservation types or CR without allocations, block full resources resourcesToBlock = reservation.Spec.Resources @@ -229,7 +259,7 @@ func (s *FilterHasEnoughCapacity) Run(traceLog *slog.Logger, request api.Externa "reservationType", reservation.Spec.Type, "freeCPU", freeCPU.String(), "blocked", cpu.String()) - freeCPU = resource.MustParse("0") + freeCPU = resource.Quantity{} } freeResourcesByHost[host]["cpu"] = freeCPU } @@ -244,7 +274,7 @@ func (s *FilterHasEnoughCapacity) Run(traceLog *slog.Logger, request api.Externa "reservationType", reservation.Spec.Type, "freeMemory", freeMemory.String(), "blocked", memory.String()) - freeMemory = resource.MustParse("0") + freeMemory = resource.Quantity{} } freeResourcesByHost[host]["memory"] = freeMemory } diff --git a/internal/scheduling/nova/plugins/filters/filter_has_enough_capacity_test.go b/internal/scheduling/nova/plugins/filters/filter_has_enough_capacity_test.go index cabc3a3b4..5b026408f 100644 --- a/internal/scheduling/nova/plugins/filters/filter_has_enough_capacity_test.go +++ b/internal/scheduling/nova/plugins/filters/filter_has_enough_capacity_test.go @@ -94,10 +94,22 @@ func newHypervisorWithBothCapacities(name, cpuCap, cpuEffCap, memCap, memEffCap } } +// newCommittedReservation creates a reservation where TargetHost and Status.Host are the same. func newCommittedReservation( + name, host, projectID, flavorName, flavorGroup, cpu, memory string, + specAllocations map[string]v1alpha1.CommittedResourceAllocation, + statusAllocations map[string]string, +) *v1alpha1.Reservation { + + return newMigratingReservation(name, host, host, projectID, flavorName, flavorGroup, cpu, memory, specAllocations, statusAllocations) +} + +// newMigratingReservation creates a reservation where TargetHost and Status.Host may differ, +// used for in-progress reservation migrations or pending placements. +func newMigratingReservation( name, targetHost, observedHost, projectID, flavorName, flavorGroup, cpu, memory string, - specAllocations map[string]v1alpha1.CommittedResourceAllocation, // Spec allocations for CR - statusAllocations map[string]string, // Status allocations for CR (instance UUID -> host) + specAllocations map[string]v1alpha1.CommittedResourceAllocation, + statusAllocations map[string]string, ) *v1alpha1.Reservation { res := &v1alpha1.Reservation{ @@ -266,6 +278,20 @@ func newNovaRequestWithIntent(instanceUUID, projectID, flavorName, flavorGroup s } } +func assertActivations(t *testing.T, activations map[string]float64, expectedHosts, filteredHosts []string) { + t.Helper() + for _, host := range expectedHosts { + if _, ok := activations[host]; !ok { + t.Errorf("expected host %s to pass, got activations: %v", host, activations) + } + } + for _, host := range filteredHosts { + if _, ok := activations[host]; ok { + t.Errorf("expected host %s to be filtered, got activations: %v", host, activations) + } + } +} + // ============================================================================ // Tests // ============================================================================ @@ -325,8 +351,8 @@ func TestFilterHasEnoughCapacity_ReservationTypes(t *testing.T) { { name: "CommittedResourceReservation of other project blocks some hosts", reservations: []*v1alpha1.Reservation{ - newCommittedReservation("res-1", "host1", "host1", "project-A", "m1.large", "gp-1", "8", "16Gi", nil, nil), - newCommittedReservation("res-2", "host2", "host2", "project-A", "m1.large", "gp-1", "4", "8Gi", nil, nil), + newCommittedReservation("res-1", "host1", "project-A", "m1.large", "gp-1", "8", "16Gi", nil, nil), + newCommittedReservation("res-2", "host2", "project-A", "m1.large", "gp-1", "4", "8Gi", nil, nil), }, request: newNovaRequest("instance-123", "project-B", "m1.small", "gp-1", 4, "8Gi", false, []string{"host1", "host2", "host3", "host4"}), opts: FilterHasEnoughCapacityOpts{LockReserved: false}, @@ -336,20 +362,37 @@ func TestFilterHasEnoughCapacity_ReservationTypes(t *testing.T) { { name: "CommittedResourceReservation of other project blocks only unused resources of reservation", reservations: []*v1alpha1.Reservation{ - newCommittedReservation("res-1 half used", "host1", "host1", "project-A", "m1.large", "gp-1", "8", "16Gi", crSpecAllocs(crVm("vm-1", "4", "8Gi")), map[string]string{"vm-1": "host1"}), - newCommittedReservation("res-2 fully used", "host2", "host2", "project-A", "m1.large", "gp-1", "4", "8Gi", crSpecAllocs(crVm("vm-2", "4", "8Gi")), map[string]string{"vm-2": "host2"}), + newCommittedReservation("res-1 half used", "host1", "project-A", "m1.large", "gp-1", "8", "16Gi", crSpecAllocs(crVm("vm-1", "4", "8Gi")), map[string]string{"vm-1": "host1"}), + newCommittedReservation("res-2 fully used", "host2", "project-A", "m1.large", "gp-1", "4", "8Gi", crSpecAllocs(crVm("vm-2", "4", "8Gi")), map[string]string{"vm-2": "host2"}), }, request: newNovaRequest("instance-123", "project-B", "m1.small", "gp-1", 4, "8Gi", false, []string{"host1", "host2", "host3", "host4"}), opts: FilterHasEnoughCapacityOpts{LockReserved: false}, expectedHosts: []string{"host1", "host2", "host3"}, filteredHosts: []string{"host4"}, }, + { + // host1: 8 CPU free, 16Gi free. + // Slot=8cpu/16Gi, two confirmed VMs: vm-1=3cpu/6Gi + vm-2=2cpu/4Gi. + // Correct: confirmed sum=5cpu/10Gi → remaining=3cpu/6Gi → block=3cpu/6Gi → free=5cpu/10Gi. + // Bug (only one VM counted): block=5cpu/10Gi → free=3cpu/6Gi → 4-cpu request wrongly filtered. + name: "CommittedResourceReservation blocks only remaining capacity when multiple VMs are confirmed in one slot", + reservations: []*v1alpha1.Reservation{ + newCommittedReservation("res-multi-vm", "host1", "project-A", "m1.large", "gp-1", "8", "16Gi", + crSpecAllocs(crVm("vm-1", "3", "6Gi"), crVm("vm-2", "2", "4Gi")), + map[string]string{"vm-1": "host1", "vm-2": "host1"}, + ), + }, + request: newNovaRequest("instance-123", "project-B", "m1.small", "gp-1", 4, "8Gi", false, []string{"host1", "host2", "host3", "host4"}), + opts: FilterHasEnoughCapacityOpts{LockReserved: false}, + expectedHosts: []string{"host1", "host2", "host3"}, // host1: 5cpu/10Gi free → 4cpu/8Gi request passes + filteredHosts: []string{"host4"}, + }, { name: "CommittedResourceReservation of other project blocks both source and target host during migration, ignoring used resources", reservations: []*v1alpha1.Reservation{ - newCommittedReservation("res-1", "host1", "host1", "project-A", "m1.large", "gp-1", "4", "8Gi", nil, nil), - newCommittedReservation("res-2", "host1", "host2", "project-A", "m1.large", "gp-1", "2", "4Gi", nil, nil), // migration reservation from host1 to host2 - newCommittedReservation("res-3", "host2", "host1", "project-A", "m1.large", "gp-1", "2", "4Gi", crSpecAllocs(crVm("vm-1", "2", "4Gi")), map[string]string{"vm-1": "host1"}), // migration reservation from host2 to host1 + newCommittedReservation("res-1", "host1", "project-A", "m1.large", "gp-1", "4", "8Gi", nil, nil), + newMigratingReservation("res-2", "host1", "host2", "project-A", "m1.large", "gp-1", "2", "4Gi", nil, nil), // migration reservation from host1 to host2 + newMigratingReservation("res-3", "host2", "host1", "project-A", "m1.large", "gp-1", "2", "4Gi", crSpecAllocs(crVm("vm-1", "2", "4Gi")), map[string]string{"vm-1": "host1"}), // migration reservation from host2 to host1 }, request: newNovaRequest("instance-123", "project-B", "m1.small", "gp-1", 2, "4Gi", false, []string{"host1", "host2", "host3", "host4"}), opts: FilterHasEnoughCapacityOpts{LockReserved: false}, @@ -360,10 +403,10 @@ func TestFilterHasEnoughCapacity_ReservationTypes(t *testing.T) { name: "CommittedResourceReservation unlocks for matching project and flavor group", reservations: []*v1alpha1.Reservation{ // all three reservations 1,2,3 are required to have enough capacity for the request - newCommittedReservation("res-1-unused", "host1", "host1", "project-A", "some flavor", "gp-1", "2", "4Gi", nil, nil), // fully unused reservation - newCommittedReservation("res-2-pending-used", "host1", "host1", "project-A", "some flavor", "gp-1", "2", "4Gi", crSpecAllocs(crVm("vm-1", "2", "4Gi")), nil), // reservation with a pending allocation - newCommittedReservation("res-3-used", "host1", "host1", "project-A", "some flavor", "gp-1", "2", "4Gi", crSpecAllocs(crVm("vm-2", "1", "1Gi")), map[string]string{"vm-2": "host1"}), // used reservation - newCommittedReservation("res-4", "host2", "host2", "project-A", "some flavor", "gp-2", "4", "8Gi", nil, nil), // different flavor group, should still block + newCommittedReservation("res-1-unused", "host1", "project-A", "some flavor", "gp-1", "2", "4Gi", nil, nil), // fully unused reservation + newCommittedReservation("res-2-pending-used", "host1", "project-A", "some flavor", "gp-1", "2", "4Gi", crSpecAllocs(crVm("vm-1", "2", "4Gi")), nil), // reservation with a pending allocation + newCommittedReservation("res-3-used", "host1", "project-A", "some flavor", "gp-1", "2", "4Gi", crSpecAllocs(crVm("vm-2", "1", "1Gi")), map[string]string{"vm-2": "host1"}), // used reservation + newCommittedReservation("res-4", "host2", "project-A", "some flavor", "gp-2", "4", "8Gi", nil, nil), // different flavor group, should still block }, request: newNovaRequest("instance-123", "project-A", "m1.large", "gp-1", 8, "16Gi", false, []string{"host1", "host2", "host3", "host4"}), opts: FilterHasEnoughCapacityOpts{LockReserved: false}, @@ -373,8 +416,8 @@ func TestFilterHasEnoughCapacity_ReservationTypes(t *testing.T) { { name: "CommittedResourceReservation stays locked when LockReserved is true", reservations: []*v1alpha1.Reservation{ - newCommittedReservation("res-1", "host1", "host1", "project-A", "m1.large", "gp-1", "8", "16Gi", nil, nil), - newCommittedReservation("res-2", "host3", "host3", "project-A", "m1.large", "gp-1", "16", "32Gi", nil, nil), + newCommittedReservation("res-1", "host1", "project-A", "m1.large", "gp-1", "8", "16Gi", nil, nil), + newCommittedReservation("res-2", "host3", "project-A", "m1.large", "gp-1", "16", "32Gi", nil, nil), }, request: newNovaRequest("instance-123", "project-A", "m1.large", "gp-1", 4, "8Gi", false, []string{"host1", "host2", "host3", "host4"}), opts: FilterHasEnoughCapacityOpts{LockReserved: true}, @@ -384,7 +427,7 @@ func TestFilterHasEnoughCapacity_ReservationTypes(t *testing.T) { { name: "Empty reservation type defaults to CommittedResourceReservation behavior", reservations: []*v1alpha1.Reservation{ - newCommittedReservation("legacy-res", "host1", "host1", "project-A", "m1.large", "gp-1", "4", "8Gi", nil, nil), + newCommittedReservation("legacy-res", "host1", "project-A", "m1.large", "gp-1", "4", "8Gi", nil, nil), }, request: newNovaRequest("instance-123", "project-A", "m1.large", "gp-1", 4, "8Gi", false, []string{"host1", "host2"}), opts: FilterHasEnoughCapacityOpts{LockReserved: false}, @@ -394,9 +437,9 @@ func TestFilterHasEnoughCapacity_ReservationTypes(t *testing.T) { { name: "All hosts blocked by reservations - none pass", reservations: []*v1alpha1.Reservation{ - newCommittedReservation("res-1", "host1", "host1", "project-X", "m1.xlarge", "gp-1", "8", "16Gi", nil, nil), - newCommittedReservation("res-2", "host2", "host2", "project-X", "m1.xlarge", "gp-1", "4", "8Gi", nil, nil), - newCommittedReservation("res-3", "host3", "host3", "project-X", "m1.xlarge", "gp-1", "16", "32Gi", nil, nil), + newCommittedReservation("res-1", "host1", "project-X", "m1.xlarge", "gp-1", "8", "16Gi", nil, nil), + newCommittedReservation("res-2", "host2", "project-X", "m1.xlarge", "gp-1", "4", "8Gi", nil, nil), + newCommittedReservation("res-3", "host3", "project-X", "m1.xlarge", "gp-1", "16", "32Gi", nil, nil), }, request: newNovaRequest("instance-123", "project-A", "m1.small", "gp-1", 4, "8Gi", false, []string{"host1", "host2", "host3", "host4"}), opts: FilterHasEnoughCapacityOpts{LockReserved: false}, @@ -406,7 +449,7 @@ func TestFilterHasEnoughCapacity_ReservationTypes(t *testing.T) { { name: "Pending reservation (only TargetHost set) blocks capacity on desired host", reservations: []*v1alpha1.Reservation{ - newCommittedReservation("pending-res", "host1", "", "project-X", "m1.large", "gp-1", "8", "16Gi", nil, nil), + newMigratingReservation("pending-res", "host1", "", "project-X", "m1.large", "gp-1", "8", "16Gi", nil, nil), }, request: newNovaRequest("instance-123", "project-A", "m1.small", "gp-1", 4, "8Gi", false, []string{"host1", "host2", "host3", "host4"}), opts: FilterHasEnoughCapacityOpts{LockReserved: false}, @@ -416,8 +459,8 @@ func TestFilterHasEnoughCapacity_ReservationTypes(t *testing.T) { { name: "Multiple reservations: pending and placed block different hosts", reservations: []*v1alpha1.Reservation{ - newCommittedReservation("pending-res", "host1", "", "project-X", "m1.large", "gp-1", "8", "16Gi", nil, nil), - newCommittedReservation("placed-res", "host2", "host3", "project-X", "m1.large", "gp-1", "4", "8Gi", nil, nil), + newMigratingReservation("pending-res", "host1", "", "project-X", "m1.large", "gp-1", "8", "16Gi", nil, nil), + newMigratingReservation("placed-res", "host2", "host3", "project-X", "m1.large", "gp-1", "4", "8Gi", nil, nil), }, request: newNovaRequest("instance-123", "project-A", "m1.small", "gp-1", 4, "8Gi", false, []string{"host1", "host2", "host3", "host4"}), opts: FilterHasEnoughCapacityOpts{LockReserved: false}, @@ -427,13 +470,50 @@ func TestFilterHasEnoughCapacity_ReservationTypes(t *testing.T) { { name: "Reservation with no host is skipped", reservations: []*v1alpha1.Reservation{ - newCommittedReservation("no-host-res", "", "", "project-X", "m1.large", "gp-1", "8", "16Gi", nil, nil), + newCommittedReservation("no-host-res", "", "project-X", "m1.large", "gp-1", "8", "16Gi", nil, nil), }, request: newNovaRequest("instance-123", "project-A", "m1.small", "gp-1", 4, "8Gi", false, []string{"host1", "host2", "host3", "host4"}), opts: FilterHasEnoughCapacityOpts{LockReserved: false}, expectedHosts: []string{"host1", "host2", "host3"}, filteredHosts: []string{"host4"}, }, + { + // host1: 8 CPU free, 16Gi free (shared hypervisors). + // Reservation slot=4cpu/8Gi, but confirmed VM consumed 6cpu/10Gi (exceeds slot after resize). + // Unclamped: block = -2cpu/-2Gi → free becomes {10cpu,18Gi}; passes 9-cpu request (wrong). + // Clamped: block = 0 → free stays {8cpu,16Gi}; filtered for 9-cpu request. + name: "Confirmed VMs exceeding reservation size: block clamped to 0", + reservations: []*v1alpha1.Reservation{ + newCommittedReservation("res-oversized-vm", "host1", "project-X", "m1.large", "gp-1", "4", "8Gi", + crSpecAllocs(crVm("vm-1", "6", "10Gi")), + map[string]string{"vm-1": "host1"}, + ), + }, + request: newNovaRequest("instance-123", "project-A", "m1.small", "gp-1", 9, "17Gi", false, []string{"host1", "host2", "host3", "host4"}), + opts: FilterHasEnoughCapacityOpts{LockReserved: false}, + // host1: 8 free CPU < 9, 16Gi < 17Gi → filtered; host2: 4 < 9 → filtered; host4: no capacity → filtered + expectedHosts: []string{"host3"}, + filteredHosts: []string{"host1", "host2", "host4"}, + }, + { + // host1: 8 CPU free, 16Gi free (shared hypervisors). + // Reservation slot=8cpu/16Gi, confirmed vm-1=4cpu/8Gi (remaining={4,8Gi}). + // Spec-only vm-2=6cpu/12Gi EXCEEDS remaining → block must be {6,12Gi}, not {4,8Gi}. + // Without fix: block={4,8Gi} → free={4cpu,8Gi}; 3-cpu/5Gi request passes (wrong). + // With fix: block={6,12Gi} → free={2cpu,4Gi}; filtered for 3-cpu/5Gi request. + name: "Spec-only VM larger than remaining slot: block covers spec-only VM", + reservations: []*v1alpha1.Reservation{ + newCommittedReservation("res-spec-only-oversize", "host1", "project-X", "m1.large", "gp-1", "8", "16Gi", + crSpecAllocs(crVm("vm-1", "4", "8Gi"), crVm("vm-2", "6", "12Gi")), + map[string]string{"vm-1": "host1"}, // vm-2 is spec-only + ), + }, + request: newNovaRequest("instance-123", "project-A", "m1.small", "gp-1", 3, "5Gi", false, []string{"host1", "host2", "host3", "host4"}), + opts: FilterHasEnoughCapacityOpts{LockReserved: false}, + // host1: 2 free CPU < 3 requested, 4Gi < 5Gi → filtered; host4: no capacity → filtered + expectedHosts: []string{"host2", "host3"}, + filteredHosts: []string{"host1", "host4"}, + }, { name: "FailoverReservation blocks hosts for non-evacuation request even when instance is in Allocations", reservations: []*v1alpha1.Reservation{ @@ -496,7 +576,7 @@ func TestFilterHasEnoughCapacity_ReservationTypes(t *testing.T) { objects = append(objects, h.DeepCopy()) } for _, r := range tt.reservations { - objects = append(objects, r) + objects = append(objects, r.DeepCopy()) } step := &FilterHasEnoughCapacity{} @@ -507,18 +587,7 @@ func TestFilterHasEnoughCapacity_ReservationTypes(t *testing.T) { if err != nil { t.Fatalf("expected no error, got %v", err) } - - for _, host := range tt.expectedHosts { - if _, ok := result.Activations[host]; !ok { - t.Errorf("expected host %s to be present in activations, but got %+v", host, result.Activations) - } - } - - for _, host := range tt.filteredHosts { - if _, ok := result.Activations[host]; ok { - t.Errorf("expected host %s to be filtered out", host) - } - } + assertActivations(t, result.Activations, tt.expectedHosts, tt.filteredHosts) }) } } @@ -526,6 +595,27 @@ func TestFilterHasEnoughCapacity_ReservationTypes(t *testing.T) { func TestFilterHasEnoughCapacity_IgnoredReservationTypes(t *testing.T) { scheme := buildTestScheme(t) + // Two-host scenario: CR on host1 (4cpu/8Gi, project-X), Failover on host2 (4cpu/8Gi). + // Each host: 8 CPU free after base allocation → after reservation: 4 CPU free each. + twoHostHVs := []*hv1.Hypervisor{ + newHypervisor("host1", "16", "8", "32Gi", "16Gi"), + newHypervisor("host2", "16", "8", "32Gi", "16Gi"), + } + twoHostRes := []*v1alpha1.Reservation{ + newCommittedReservation("cr-res", "host1", "project-X", "m1.large", "gp-1", "4", "8Gi", nil, nil), + newFailoverReservation("failover-res", "host2", "4", "8Gi", map[string]string{"other-vm": "host3"}), + } + + // Single-host scenario: both CR (4cpu/8Gi) and Failover (2cpu/4Gi) on host1. + // host1: 12 CPU free → after both reservations: 6 CPU free. + singleHostHVs := []*hv1.Hypervisor{ + newHypervisor("host1", "12", "0", "24Gi", "0"), + } + singleHostRes := []*v1alpha1.Reservation{ + newCommittedReservation("cr-res", "host1", "project-X", "m1.large", "gp-1", "4", "8Gi", nil, nil), + newFailoverReservation("failover-res", "host1", "2", "4Gi", map[string]string{"other-vm": "host3"}), + } + tests := []struct { name string hypervisors []*hv1.Hypervisor @@ -538,60 +628,36 @@ func TestFilterHasEnoughCapacity_IgnoredReservationTypes(t *testing.T) { // Two-host scenario tests (CR on host1, Failover on host2) // host1: 8 CPU free, host2: 8 CPU free, CR blocks 4 on host1, Failover blocks 4 on host2 { - name: "Two hosts: No ignore - both hosts blocked by reservations", - hypervisors: []*hv1.Hypervisor{ - newHypervisor("host1", "16", "8", "32Gi", "16Gi"), // 8 CPU free - newHypervisor("host2", "16", "8", "32Gi", "16Gi"), // 8 CPU free - }, - reservations: []*v1alpha1.Reservation{ - newCommittedReservation("cr-res", "host1", "host1", "project-X", "m1.large", "gp-1", "4", "8Gi", nil, nil), - newFailoverReservation("failover-res", "host2", "4", "8Gi", map[string]string{"other-vm": "host3"}), - }, + name: "Two hosts: No ignore - both hosts blocked by reservations", + hypervisors: twoHostHVs, + reservations: twoHostRes, request: newNovaRequest("instance-123", "project-A", "m1.large", "gp-1", 8, "16Gi", false, []string{"host1", "host2"}), ignoredReservationTypes: nil, expectedHosts: []string{}, filteredHosts: []string{"host1", "host2"}, }, { - name: "Two hosts: Ignore CR only - host1 passes, host2 blocked by failover", - hypervisors: []*hv1.Hypervisor{ - newHypervisor("host1", "16", "8", "32Gi", "16Gi"), - newHypervisor("host2", "16", "8", "32Gi", "16Gi"), - }, - reservations: []*v1alpha1.Reservation{ - newCommittedReservation("cr-res", "host1", "host1", "project-X", "m1.large", "gp-1", "4", "8Gi", nil, nil), - newFailoverReservation("failover-res", "host2", "4", "8Gi", map[string]string{"other-vm": "host3"}), - }, + name: "Two hosts: Ignore CR only - host1 passes, host2 blocked by failover", + hypervisors: twoHostHVs, + reservations: twoHostRes, request: newNovaRequest("instance-123", "project-A", "m1.large", "gp-1", 8, "16Gi", false, []string{"host1", "host2"}), ignoredReservationTypes: []v1alpha1.ReservationType{v1alpha1.ReservationTypeCommittedResource}, expectedHosts: []string{"host1"}, filteredHosts: []string{"host2"}, }, { - name: "Two hosts: Ignore Failover only - host2 passes, host1 blocked by CR", - hypervisors: []*hv1.Hypervisor{ - newHypervisor("host1", "16", "8", "32Gi", "16Gi"), - newHypervisor("host2", "16", "8", "32Gi", "16Gi"), - }, - reservations: []*v1alpha1.Reservation{ - newCommittedReservation("cr-res", "host1", "host1", "project-X", "m1.large", "gp-1", "4", "8Gi", nil, nil), - newFailoverReservation("failover-res", "host2", "4", "8Gi", map[string]string{"other-vm": "host3"}), - }, + name: "Two hosts: Ignore Failover only - host2 passes, host1 blocked by CR", + hypervisors: twoHostHVs, + reservations: twoHostRes, request: newNovaRequest("instance-123", "project-A", "m1.large", "gp-1", 8, "16Gi", false, []string{"host1", "host2"}), ignoredReservationTypes: []v1alpha1.ReservationType{v1alpha1.ReservationTypeFailover}, expectedHosts: []string{"host2"}, filteredHosts: []string{"host1"}, }, { - name: "Two hosts: Ignore both - both hosts pass", - hypervisors: []*hv1.Hypervisor{ - newHypervisor("host1", "16", "8", "32Gi", "16Gi"), - newHypervisor("host2", "16", "8", "32Gi", "16Gi"), - }, - reservations: []*v1alpha1.Reservation{ - newCommittedReservation("cr-res", "host1", "host1", "project-X", "m1.large", "gp-1", "4", "8Gi", nil, nil), - newFailoverReservation("failover-res", "host2", "4", "8Gi", map[string]string{"other-vm": "host3"}), - }, + name: "Two hosts: Ignore both - both hosts pass", + hypervisors: twoHostHVs, + reservations: twoHostRes, request: newNovaRequest("instance-123", "project-A", "m1.large", "gp-1", 8, "16Gi", false, []string{"host1", "host2"}), ignoredReservationTypes: []v1alpha1.ReservationType{v1alpha1.ReservationTypeCommittedResource, v1alpha1.ReservationTypeFailover}, expectedHosts: []string{"host1", "host2"}, @@ -602,56 +668,36 @@ func TestFilterHasEnoughCapacity_IgnoredReservationTypes(t *testing.T) { // host1: 12 CPU free, CR blocks 4, Failover blocks 2 → 6 free when both active // Large VM (12 CPU) - only fits if BOTH reservations are ignored { - name: "Single host, Large VM (12 CPU): No ignore - blocked (6 free < 12 needed)", - hypervisors: []*hv1.Hypervisor{ - newHypervisor("host1", "12", "0", "24Gi", "0"), // 12 CPU free - }, - reservations: []*v1alpha1.Reservation{ - newCommittedReservation("cr-res", "host1", "host1", "project-X", "m1.large", "gp-1", "4", "8Gi", nil, nil), - newFailoverReservation("failover-res", "host1", "2", "4Gi", map[string]string{"other-vm": "host3"}), - }, + name: "Single host, Large VM (12 CPU): No ignore - blocked (6 free < 12 needed)", + hypervisors: singleHostHVs, + reservations: singleHostRes, request: newNovaRequest("instance-123", "project-A", "m1.large", "gp-1", 12, "24Gi", false, []string{"host1"}), ignoredReservationTypes: nil, expectedHosts: []string{}, filteredHosts: []string{"host1"}, }, { - name: "Single host, Large VM (12 CPU): Ignore CR - blocked (10 free < 12 needed)", - hypervisors: []*hv1.Hypervisor{ - newHypervisor("host1", "12", "0", "24Gi", "0"), - }, - reservations: []*v1alpha1.Reservation{ - newCommittedReservation("cr-res", "host1", "host1", "project-X", "m1.large", "gp-1", "4", "8Gi", nil, nil), - newFailoverReservation("failover-res", "host1", "2", "4Gi", map[string]string{"other-vm": "host3"}), - }, + name: "Single host, Large VM (12 CPU): Ignore CR - blocked (10 free < 12 needed)", + hypervisors: singleHostHVs, + reservations: singleHostRes, request: newNovaRequest("instance-123", "project-A", "m1.large", "gp-1", 12, "24Gi", false, []string{"host1"}), ignoredReservationTypes: []v1alpha1.ReservationType{v1alpha1.ReservationTypeCommittedResource}, expectedHosts: []string{}, filteredHosts: []string{"host1"}, }, { - name: "Single host, Large VM (12 CPU): Ignore Failover - blocked (8 free < 12 needed)", - hypervisors: []*hv1.Hypervisor{ - newHypervisor("host1", "12", "0", "24Gi", "0"), - }, - reservations: []*v1alpha1.Reservation{ - newCommittedReservation("cr-res", "host1", "host1", "project-X", "m1.large", "gp-1", "4", "8Gi", nil, nil), - newFailoverReservation("failover-res", "host1", "2", "4Gi", map[string]string{"other-vm": "host3"}), - }, + name: "Single host, Large VM (12 CPU): Ignore Failover - blocked (8 free < 12 needed)", + hypervisors: singleHostHVs, + reservations: singleHostRes, request: newNovaRequest("instance-123", "project-A", "m1.large", "gp-1", 12, "24Gi", false, []string{"host1"}), ignoredReservationTypes: []v1alpha1.ReservationType{v1alpha1.ReservationTypeFailover}, expectedHosts: []string{}, filteredHosts: []string{"host1"}, }, { - name: "Single host, Large VM (12 CPU): Ignore both - passes (12 free = 12 needed)", - hypervisors: []*hv1.Hypervisor{ - newHypervisor("host1", "12", "0", "24Gi", "0"), - }, - reservations: []*v1alpha1.Reservation{ - newCommittedReservation("cr-res", "host1", "host1", "project-X", "m1.large", "gp-1", "4", "8Gi", nil, nil), - newFailoverReservation("failover-res", "host1", "2", "4Gi", map[string]string{"other-vm": "host3"}), - }, + name: "Single host, Large VM (12 CPU): Ignore both - passes (12 free = 12 needed)", + hypervisors: singleHostHVs, + reservations: singleHostRes, request: newNovaRequest("instance-123", "project-A", "m1.large", "gp-1", 12, "24Gi", false, []string{"host1"}), ignoredReservationTypes: []v1alpha1.ReservationType{v1alpha1.ReservationTypeCommittedResource, v1alpha1.ReservationTypeFailover}, expectedHosts: []string{"host1"}, @@ -660,56 +706,36 @@ func TestFilterHasEnoughCapacity_IgnoredReservationTypes(t *testing.T) { // Failover-size VM (10 CPU) - fits if CR is ignored (10 free = 10 needed) { - name: "Single host, Failover-size VM (10 CPU): No ignore - blocked (6 free < 10 needed)", - hypervisors: []*hv1.Hypervisor{ - newHypervisor("host1", "12", "0", "24Gi", "0"), - }, - reservations: []*v1alpha1.Reservation{ - newCommittedReservation("cr-res", "host1", "host1", "project-X", "m1.large", "gp-1", "4", "8Gi", nil, nil), - newFailoverReservation("failover-res", "host1", "2", "4Gi", map[string]string{"other-vm": "host3"}), - }, + name: "Single host, Failover-size VM (10 CPU): No ignore - blocked (6 free < 10 needed)", + hypervisors: singleHostHVs, + reservations: singleHostRes, request: newNovaRequest("instance-123", "project-A", "m1.large", "gp-1", 10, "20Gi", false, []string{"host1"}), ignoredReservationTypes: nil, expectedHosts: []string{}, filteredHosts: []string{"host1"}, }, { - name: "Single host, Failover-size VM (10 CPU): Ignore CR - passes (10 free = 10 needed)", - hypervisors: []*hv1.Hypervisor{ - newHypervisor("host1", "12", "0", "24Gi", "0"), - }, - reservations: []*v1alpha1.Reservation{ - newCommittedReservation("cr-res", "host1", "host1", "project-X", "m1.large", "gp-1", "4", "8Gi", nil, nil), - newFailoverReservation("failover-res", "host1", "2", "4Gi", map[string]string{"other-vm": "host3"}), - }, + name: "Single host, Failover-size VM (10 CPU): Ignore CR - passes (10 free = 10 needed)", + hypervisors: singleHostHVs, + reservations: singleHostRes, request: newNovaRequest("instance-123", "project-A", "m1.large", "gp-1", 10, "20Gi", false, []string{"host1"}), ignoredReservationTypes: []v1alpha1.ReservationType{v1alpha1.ReservationTypeCommittedResource}, expectedHosts: []string{"host1"}, filteredHosts: []string{}, }, { - name: "Single host, Failover-size VM (10 CPU): Ignore Failover - blocked (8 free < 10 needed)", - hypervisors: []*hv1.Hypervisor{ - newHypervisor("host1", "12", "0", "24Gi", "0"), - }, - reservations: []*v1alpha1.Reservation{ - newCommittedReservation("cr-res", "host1", "host1", "project-X", "m1.large", "gp-1", "4", "8Gi", nil, nil), - newFailoverReservation("failover-res", "host1", "2", "4Gi", map[string]string{"other-vm": "host3"}), - }, + name: "Single host, Failover-size VM (10 CPU): Ignore Failover - blocked (8 free < 10 needed)", + hypervisors: singleHostHVs, + reservations: singleHostRes, request: newNovaRequest("instance-123", "project-A", "m1.large", "gp-1", 10, "20Gi", false, []string{"host1"}), ignoredReservationTypes: []v1alpha1.ReservationType{v1alpha1.ReservationTypeFailover}, expectedHosts: []string{}, filteredHosts: []string{"host1"}, }, { - name: "Single host, Failover-size VM (10 CPU): Ignore both - passes (12 free > 10 needed)", - hypervisors: []*hv1.Hypervisor{ - newHypervisor("host1", "12", "0", "24Gi", "0"), - }, - reservations: []*v1alpha1.Reservation{ - newCommittedReservation("cr-res", "host1", "host1", "project-X", "m1.large", "gp-1", "4", "8Gi", nil, nil), - newFailoverReservation("failover-res", "host1", "2", "4Gi", map[string]string{"other-vm": "host3"}), - }, + name: "Single host, Failover-size VM (10 CPU): Ignore both - passes (12 free > 10 needed)", + hypervisors: singleHostHVs, + reservations: singleHostRes, request: newNovaRequest("instance-123", "project-A", "m1.large", "gp-1", 10, "20Gi", false, []string{"host1"}), ignoredReservationTypes: []v1alpha1.ReservationType{v1alpha1.ReservationTypeCommittedResource, v1alpha1.ReservationTypeFailover}, expectedHosts: []string{"host1"}, @@ -718,56 +744,36 @@ func TestFilterHasEnoughCapacity_IgnoredReservationTypes(t *testing.T) { // CR-size VM (8 CPU) - fits if Failover is ignored (8 free = 8 needed) { - name: "Single host, CR-size VM (8 CPU): No ignore - blocked (6 free < 8 needed)", - hypervisors: []*hv1.Hypervisor{ - newHypervisor("host1", "12", "0", "24Gi", "0"), - }, - reservations: []*v1alpha1.Reservation{ - newCommittedReservation("cr-res", "host1", "host1", "project-X", "m1.large", "gp-1", "4", "8Gi", nil, nil), - newFailoverReservation("failover-res", "host1", "2", "4Gi", map[string]string{"other-vm": "host3"}), - }, + name: "Single host, CR-size VM (8 CPU): No ignore - blocked (6 free < 8 needed)", + hypervisors: singleHostHVs, + reservations: singleHostRes, request: newNovaRequest("instance-123", "project-A", "m1.large", "gp-1", 8, "16Gi", false, []string{"host1"}), ignoredReservationTypes: nil, expectedHosts: []string{}, filteredHosts: []string{"host1"}, }, { - name: "Single host, CR-size VM (8 CPU): Ignore CR - passes (10 free > 8 needed)", - hypervisors: []*hv1.Hypervisor{ - newHypervisor("host1", "12", "0", "24Gi", "0"), - }, - reservations: []*v1alpha1.Reservation{ - newCommittedReservation("cr-res", "host1", "host1", "project-X", "m1.large", "gp-1", "4", "8Gi", nil, nil), - newFailoverReservation("failover-res", "host1", "2", "4Gi", map[string]string{"other-vm": "host3"}), - }, + name: "Single host, CR-size VM (8 CPU): Ignore CR - passes (10 free > 8 needed)", + hypervisors: singleHostHVs, + reservations: singleHostRes, request: newNovaRequest("instance-123", "project-A", "m1.large", "gp-1", 8, "16Gi", false, []string{"host1"}), ignoredReservationTypes: []v1alpha1.ReservationType{v1alpha1.ReservationTypeCommittedResource}, expectedHosts: []string{"host1"}, filteredHosts: []string{}, }, { - name: "Single host, CR-size VM (8 CPU): Ignore Failover - passes (8 free = 8 needed)", - hypervisors: []*hv1.Hypervisor{ - newHypervisor("host1", "12", "0", "24Gi", "0"), - }, - reservations: []*v1alpha1.Reservation{ - newCommittedReservation("cr-res", "host1", "host1", "project-X", "m1.large", "gp-1", "4", "8Gi", nil, nil), - newFailoverReservation("failover-res", "host1", "2", "4Gi", map[string]string{"other-vm": "host3"}), - }, + name: "Single host, CR-size VM (8 CPU): Ignore Failover - passes (8 free = 8 needed)", + hypervisors: singleHostHVs, + reservations: singleHostRes, request: newNovaRequest("instance-123", "project-A", "m1.large", "gp-1", 8, "16Gi", false, []string{"host1"}), ignoredReservationTypes: []v1alpha1.ReservationType{v1alpha1.ReservationTypeFailover}, expectedHosts: []string{"host1"}, filteredHosts: []string{}, }, { - name: "Single host, CR-size VM (8 CPU): Ignore both - passes (12 free > 8 needed)", - hypervisors: []*hv1.Hypervisor{ - newHypervisor("host1", "12", "0", "24Gi", "0"), - }, - reservations: []*v1alpha1.Reservation{ - newCommittedReservation("cr-res", "host1", "host1", "project-X", "m1.large", "gp-1", "4", "8Gi", nil, nil), - newFailoverReservation("failover-res", "host1", "2", "4Gi", map[string]string{"other-vm": "host3"}), - }, + name: "Single host, CR-size VM (8 CPU): Ignore both - passes (12 free > 8 needed)", + hypervisors: singleHostHVs, + reservations: singleHostRes, request: newNovaRequest("instance-123", "project-A", "m1.large", "gp-1", 8, "16Gi", false, []string{"host1"}), ignoredReservationTypes: []v1alpha1.ReservationType{v1alpha1.ReservationTypeCommittedResource, v1alpha1.ReservationTypeFailover}, expectedHosts: []string{"host1"}, @@ -796,18 +802,7 @@ func TestFilterHasEnoughCapacity_IgnoredReservationTypes(t *testing.T) { if err != nil { t.Fatalf("expected no error, got %v", err) } - - for _, host := range tt.expectedHosts { - if _, ok := result.Activations[host]; !ok { - t.Errorf("expected host %s to be present in activations, but got %+v", host, result.Activations) - } - } - - for _, host := range tt.filteredHosts { - if _, ok := result.Activations[host]; ok { - t.Errorf("expected host %s to be filtered out, but it was present", host) - } - } + assertActivations(t, result.Activations, tt.expectedHosts, tt.filteredHosts) }) } } @@ -835,7 +830,7 @@ func TestFilterHasEnoughCapacity_ReserveForCommittedResourceIntent(t *testing.T) }, reservations: []*v1alpha1.Reservation{ // Existing CR reservation on host1 for same project+flavor group - newCommittedReservation("existing-cr", "host1", "host1", "project-A", "m1.large", "gp-1", "8", "16Gi", nil, nil), + newCommittedReservation("existing-cr", "host1", "project-A", "m1.large", "gp-1", "8", "16Gi", nil, nil), }, // Request with reserve_for_committed_resource intent (scheduling a new CR reservation) request: newNovaRequestWithIntent("new-reservation-uuid", "project-A", "m1.large", "gp-1", 4, "8Gi", "reserve_for_committed_resource", false, []string{"host1", "host2"}), @@ -851,7 +846,7 @@ func TestFilterHasEnoughCapacity_ReserveForCommittedResourceIntent(t *testing.T) }, reservations: []*v1alpha1.Reservation{ // Existing CR reservation on host1 for same project+flavor group - newCommittedReservation("existing-cr", "host1", "host1", "project-A", "m1.large", "gp-1", "8", "16Gi", nil, nil), + newCommittedReservation("existing-cr", "host1", "project-A", "m1.large", "gp-1", "8", "16Gi", nil, nil), }, // Normal VM create request (no special intent) - CR reservation should be unlocked request: newNovaRequest("vm-instance-123", "project-A", "m1.large", "gp-1", 4, "8Gi", false, []string{"host1", "host2"}), @@ -867,7 +862,7 @@ func TestFilterHasEnoughCapacity_ReserveForCommittedResourceIntent(t *testing.T) }, reservations: []*v1alpha1.Reservation{ // Existing CR reservation on host1 for different project - newCommittedReservation("other-project-cr", "host1", "host1", "project-B", "m1.large", "gp-1", "8", "16Gi", nil, nil), + newCommittedReservation("other-project-cr", "host1", "project-B", "m1.large", "gp-1", "8", "16Gi", nil, nil), }, // Request with reserve_for_committed_resource intent request: newNovaRequestWithIntent("new-reservation-uuid", "project-A", "m1.large", "gp-1", 4, "8Gi", "reserve_for_committed_resource", false, []string{"host1", "host2"}), @@ -882,9 +877,9 @@ func TestFilterHasEnoughCapacity_ReserveForCommittedResourceIntent(t *testing.T) }, reservations: []*v1alpha1.Reservation{ // Three existing CR reservations on host1 for same project+flavor group - newCommittedReservation("cr-1", "host1", "host1", "project-A", "m1.large", "gp-1", "8", "16Gi", nil, nil), - newCommittedReservation("cr-2", "host1", "host1", "project-A", "m1.large", "gp-1", "8", "16Gi", nil, nil), - newCommittedReservation("cr-3", "host1", "host1", "project-A", "m1.large", "gp-1", "8", "16Gi", nil, nil), + newCommittedReservation("cr-1", "host1", "project-A", "m1.large", "gp-1", "8", "16Gi", nil, nil), + newCommittedReservation("cr-2", "host1", "project-A", "m1.large", "gp-1", "8", "16Gi", nil, nil), + newCommittedReservation("cr-3", "host1", "project-A", "m1.large", "gp-1", "8", "16Gi", nil, nil), }, // Request with reserve_for_committed_resource intent, needs 10 CPU // After blocking all 3 reservations (24 CPU), only 8 CPU free -> should fail @@ -900,9 +895,9 @@ func TestFilterHasEnoughCapacity_ReserveForCommittedResourceIntent(t *testing.T) }, reservations: []*v1alpha1.Reservation{ // Three existing CR reservations on host1 for same project+flavor group - newCommittedReservation("cr-1", "host1", "host1", "project-A", "m1.large", "gp-1", "8", "16Gi", nil, nil), - newCommittedReservation("cr-2", "host1", "host1", "project-A", "m1.large", "gp-1", "8", "16Gi", nil, nil), - newCommittedReservation("cr-3", "host1", "host1", "project-A", "m1.large", "gp-1", "8", "16Gi", nil, nil), + newCommittedReservation("cr-1", "host1", "project-A", "m1.large", "gp-1", "8", "16Gi", nil, nil), + newCommittedReservation("cr-2", "host1", "project-A", "m1.large", "gp-1", "8", "16Gi", nil, nil), + newCommittedReservation("cr-3", "host1", "project-A", "m1.large", "gp-1", "8", "16Gi", nil, nil), }, // Normal VM create request, needs 10 CPU // All 3 reservations unlocked for matching project+flavor -> 32 CPU free -> should pass @@ -918,7 +913,7 @@ func TestFilterHasEnoughCapacity_ReserveForCommittedResourceIntent(t *testing.T) }, reservations: []*v1alpha1.Reservation{ // Existing CR reservation on host1 blocks all 8 free CPU - newCommittedReservation("existing-cr", "host1", "host1", "project-A", "m1.large", "gp-1", "8", "16Gi", nil, nil), + newCommittedReservation("existing-cr", "host1", "project-A", "m1.large", "gp-1", "8", "16Gi", nil, nil), }, // Request with reserve_for_committed_resource intent // IgnoredReservationTypes is a safety flag that overrides everything, including intent @@ -938,7 +933,7 @@ func TestFilterHasEnoughCapacity_ReserveForCommittedResourceIntent(t *testing.T) }, reservations: []*v1alpha1.Reservation{ // Existing CR reservation on host1 blocks all 8 free CPU - newCommittedReservation("existing-cr", "host1", "host1", "project-B", "m1.large", "gp-1", "8", "16Gi", nil, nil), + newCommittedReservation("existing-cr", "host1", "project-B", "m1.large", "gp-1", "8", "16Gi", nil, nil), }, // Normal VM create request (different project, so unlocking via project match won't work) // But IgnoredReservationTypes should make it work @@ -970,19 +965,58 @@ func TestFilterHasEnoughCapacity_ReserveForCommittedResourceIntent(t *testing.T) if err != nil { t.Fatalf("expected no error, got %v", err) } + assertActivations(t, result.Activations, tt.expectedHosts, tt.filteredHosts) + }) + } +} - for _, host := range tt.expectedHosts { - if _, ok := result.Activations[host]; !ok { - t.Errorf("expected host %s to be present in activations, but got %+v", host, result.Activations) - } - } +// TestFilterHasEnoughCapacity_PlannedCRDoesNotBlock verifies that a CommittedResource CRD +// in "planned" state has no child Reservation CRDs and therefore blocks no capacity. +// This is correct by design: the filter reads only Reservation CRDs, so planned CRDs +// have no effect regardless of the committed amount. +func TestFilterHasEnoughCapacity_PlannedCRDoesNotBlock(t *testing.T) { + scheme := buildTestScheme(t) - for _, host := range tt.filteredHosts { - if _, ok := result.Activations[host]; ok { - t.Errorf("expected host %s to be filtered out, but it was present", host) - } - } - }) + // A planned CommittedResource: StartTime not yet reached, no Reservation CRDs created. + plannedCR := &v1alpha1.CommittedResource{ + ObjectMeta: metav1.ObjectMeta{Name: "cr-planned-uuid-1"}, + Spec: v1alpha1.CommittedResourceSpec{ + CommitmentUUID: "uuid-1", + ProjectID: "project-A", + DomainID: "domain-A", + FlavorGroupName: "gp-1", + ResourceType: v1alpha1.CommittedResourceTypeMemory, + Amount: resource.MustParse("16Gi"), + AvailabilityZone: "az-1", + State: v1alpha1.CommitmentStatusPlanned, + }, + Status: v1alpha1.CommittedResourceStatus{ + Conditions: []metav1.Condition{ + { + Type: v1alpha1.CommittedResourceConditionReady, + Status: metav1.ConditionFalse, + Reason: v1alpha1.CommittedResourceReasonPlanned, + LastTransitionTime: metav1.Now(), + }, + }, + }, + } + + hv := newHypervisor("host1", "16", "8", "32Gi", "16Gi") // 8 CPU free, 16Gi free + objects := []client.Object{hv, plannedCR} + // No Reservation CRDs — planned CommittedResources have none. + + step := &FilterHasEnoughCapacity{} + step.Client = fake.NewClientBuilder().WithScheme(scheme).WithObjects(objects...).Build() + step.Options = FilterHasEnoughCapacityOpts{LockReserved: false} + + request := newNovaRequest("instance-123", "project-A", "m1.large", "gp-1", 4, "8Gi", false, []string{"host1"}) + result, err := step.Run(slog.Default(), request) + if err != nil { + t.Fatalf("expected no error, got %v", err) + } + if _, ok := result.Activations["host1"]; !ok { + t.Error("expected host1 to pass: planned CommittedResource has no child Reservations and must not block capacity") } } @@ -1054,18 +1088,206 @@ func TestFilterHasEnoughCapacity_NilEffectiveCapacityFallback(t *testing.T) { if err != nil { t.Fatalf("expected no error, got %v", err) } + assertActivations(t, result.Activations, tt.expectedHosts, tt.filteredHosts) + }) + } +} - for _, host := range tt.expectedHosts { - if _, ok := result.Activations[host]; !ok { - t.Errorf("expected host %s to be present in activations, but got %+v", host, result.Activations) - } - } +// TestFilterHasEnoughCapacity_VMInterReservationMigration covers all realistic phases of a VM +// migrating from res-a (on hv-a) to res-b (on hv-b). +// +// Six binary state variables per phase: +// - VM in hv-a allocation (affects hv-a free capacity directly) +// - VM in hv-b allocation (affects hv-b free capacity directly) +// - VM in res-a Spec.Allocations +// - VM in res-a Status.Allocations +// - VM in res-b Spec.Allocations +// - VM in res-b Status.Allocations +// +// Capacity accounting per host: +// +// free = HV.EffectiveCapacity - HV.Allocation +// block = max(slot - confirmed, specOnly) [clamped ≥ 0; else full slot when spec allocs empty] +// net = free - block +// +// All phases use: VM=4cpu/8Gi, slot=8cpu/16Gi, HV total=12cpu/24Gi, request=3cpu/6Gi (project-C). +func TestFilterHasEnoughCapacity_VMInterReservationMigration(t *testing.T) { + scheme := buildTestScheme(t) + + const ( + owner = "project-A" // project owning the reservations and the migrating VM + thirdParty = "project-C" // project making the placement request + flavorGroup = "gp-1" + slotCPU = "8" + slotMem = "16Gi" + hvCapCPU = "12" + hvCapMem = "24Gi" + vmCPU = "4" + vmMem = "8Gi" + ) + + tests := []struct { + name string + hvA *hv1.Hypervisor // allocation=vmCPU/vmMem when VM present, "0"/"0" when absent + hvB *hv1.Hypervisor + resA *v1alpha1.Reservation + resB *v1alpha1.Reservation + expectedHosts []string + filteredHosts []string + }{ + { + // VM fully on hv-a, confirmed in res-a. res-b exists but is empty. + // + // hv-a: free=12-4=8cpu. res-a confirmed → block=slot-confirmed=8-4=4 → net=4. Passes 3-cpu req. + // hv-b: free=12cpu. res-b no allocs → block=full slot=8 → net=4. Passes. + name: "Phase 1: VM on hv-a, confirmed in res-a, res-b empty", + hvA: newHypervisor("hv-a", hvCapCPU, vmCPU, hvCapMem, vmMem), + hvB: newHypervisor("hv-b", hvCapCPU, "0", hvCapMem, "0"), + resA: newCommittedReservation("res-a", "hv-a", owner, "m1.large", flavorGroup, slotCPU, slotMem, + crSpecAllocs(crVm("vm-1", vmCPU, vmMem)), + map[string]string{"vm-1": "hv-a"}, + ), + resB: newCommittedReservation("res-b", "hv-b", owner, "m1.large", flavorGroup, slotCPU, slotMem, nil, nil), + expectedHosts: []string{"hv-a", "hv-b"}, + filteredHosts: []string{}, + }, + { + // Placement pipeline wrote VM into res-b spec. VM is still running on hv-a (not yet migrated). + // + // hv-a: free=8, res-a confirmed → block=4 → net=4. Passes. + // hv-b: free=12, res-b spec-only(4) → remaining=8, specOnly=4, block=max(8,4)=8 → net=4. Passes. + // + // res-b blocks its full slot even though VM is only in spec: remaining(8) > specOnly(4). + name: "Phase 2: VM on hv-a, confirmed in res-a; added to res-b spec only (migration initiated)", + hvA: newHypervisor("hv-a", hvCapCPU, vmCPU, hvCapMem, vmMem), + hvB: newHypervisor("hv-b", hvCapCPU, "0", hvCapMem, "0"), + resA: newCommittedReservation("res-a", "hv-a", owner, "m1.large", flavorGroup, slotCPU, slotMem, + crSpecAllocs(crVm("vm-1", vmCPU, vmMem)), + map[string]string{"vm-1": "hv-a"}, + ), + resB: newCommittedReservation("res-b", "hv-b", owner, "m1.large", flavorGroup, slotCPU, slotMem, + crSpecAllocs(crVm("vm-1", vmCPU, vmMem)), + nil, + ), + expectedHosts: []string{"hv-a", "hv-b"}, + filteredHosts: []string{}, + }, + { + // VM appears in both HV allocations during live migration (transient double-presence). + // res-a: confirmed. res-b: spec-only (controller has not reconciled hv-b yet). + // + // hv-a: free=8, res-a confirmed → block=4 → net=4. Passes. + // hv-b: free=8, res-b spec-only → block=8 → net=0. FAILS — conservative until controller confirms. + name: "Phase 3: VM in both HV allocs (live migration in progress); res-a confirmed, res-b spec-only", + hvA: newHypervisor("hv-a", hvCapCPU, vmCPU, hvCapMem, vmMem), + hvB: newHypervisor("hv-b", hvCapCPU, vmCPU, hvCapMem, vmMem), + resA: newCommittedReservation("res-a", "hv-a", owner, "m1.large", flavorGroup, slotCPU, slotMem, + crSpecAllocs(crVm("vm-1", vmCPU, vmMem)), + map[string]string{"vm-1": "hv-a"}, + ), + resB: newCommittedReservation("res-b", "hv-b", owner, "m1.large", flavorGroup, slotCPU, slotMem, + crSpecAllocs(crVm("vm-1", vmCPU, vmMem)), + nil, + ), + expectedHosts: []string{"hv-a"}, + filteredHosts: []string{"hv-b"}, + }, + { + // VM has arrived on hv-b (in alloc) but left hv-a. Controller lag: res-a still confirmed, res-b spec-only. + // + // hv-a: free=12 (VM gone), res-a confirmed → block=4 → net=8. Passes. + // hv-b: free=8, res-b spec-only → block=8 → net=0. FAILS — res-b not confirmed yet. + name: "Phase 4: VM arrived on hv-b (in alloc), left hv-a; res-a still confirmed, res-b spec-only", + hvA: newHypervisor("hv-a", hvCapCPU, "0", hvCapMem, "0"), + hvB: newHypervisor("hv-b", hvCapCPU, vmCPU, hvCapMem, vmMem), + resA: newCommittedReservation("res-a", "hv-a", owner, "m1.large", flavorGroup, slotCPU, slotMem, + crSpecAllocs(crVm("vm-1", vmCPU, vmMem)), + map[string]string{"vm-1": "hv-a"}, + ), + resB: newCommittedReservation("res-b", "hv-b", owner, "m1.large", flavorGroup, slotCPU, slotMem, + crSpecAllocs(crVm("vm-1", vmCPU, vmMem)), + nil, + ), + expectedHosts: []string{"hv-a"}, + filteredHosts: []string{"hv-b"}, + }, + { + // Controller confirmed VM in res-b. res-a cleanup not done yet (stale confirmed entry). + // + // hv-a: free=12 (VM gone), res-a confirmed(stale) → block=8-4=4 → net=8. Passes. + // hv-b: free=8, res-b confirmed → block=8-4=4 → net=4. Passes. + // + // hv-a gets its remaining slot capacity back once res-a is cleaned up (phases 6→7). + name: "Phase 5: VM confirmed in res-b; res-a still has confirmed entry (stale, not yet cleaned)", + hvA: newHypervisor("hv-a", hvCapCPU, "0", hvCapMem, "0"), + hvB: newHypervisor("hv-b", hvCapCPU, vmCPU, hvCapMem, vmMem), + resA: newCommittedReservation("res-a", "hv-a", owner, "m1.large", flavorGroup, slotCPU, slotMem, + crSpecAllocs(crVm("vm-1", vmCPU, vmMem)), + map[string]string{"vm-1": "hv-a"}, + ), + resB: newCommittedReservation("res-b", "hv-b", owner, "m1.large", flavorGroup, slotCPU, slotMem, + crSpecAllocs(crVm("vm-1", vmCPU, vmMem)), + map[string]string{"vm-1": "hv-b"}, + ), + expectedHosts: []string{"hv-a", "hv-b"}, + filteredHosts: []string{}, + }, + { + // VM removed from res-a Spec.Allocations, but res-a Status.Allocations still stale (one controller cycle lag). + // spec allocs empty → else branch: res-a blocks its full slot regardless of status. + // Status allocations are not consulted when spec allocs are absent. + // + // hv-a: free=12, res-a spec-empty → block=full slot=8 → net=4. Passes. + // hv-b: free=8, res-b confirmed → block=4 → net=4. Passes. + name: "Phase 6: VM removed from res-a spec, res-a status stale; res-b confirmed", + hvA: newHypervisor("hv-a", hvCapCPU, "0", hvCapMem, "0"), + hvB: newHypervisor("hv-b", hvCapCPU, vmCPU, hvCapMem, vmMem), + resA: newCommittedReservation("res-a", "hv-a", owner, "m1.large", flavorGroup, slotCPU, slotMem, + nil, // spec allocs cleared + map[string]string{"vm-1": "hv-a"}, // status stale — not consulted when spec is empty + ), + resB: newCommittedReservation("res-b", "hv-b", owner, "m1.large", flavorGroup, slotCPU, slotMem, + crSpecAllocs(crVm("vm-1", vmCPU, vmMem)), + map[string]string{"vm-1": "hv-b"}, + ), + expectedHosts: []string{"hv-a", "hv-b"}, + filteredHosts: []string{}, + }, + { + // Migration fully complete: res-a is empty, VM confirmed in res-b. + // Identical blocking to phase 6 for hv-a (spec-empty → full slot block). + // + // hv-a: free=12, res-a empty → block=8 → net=4. Passes. + // hv-b: free=8, res-b confirmed → block=4 → net=4. Passes. + name: "Phase 7: Migration complete — res-a empty, VM confirmed in res-b", + hvA: newHypervisor("hv-a", hvCapCPU, "0", hvCapMem, "0"), + hvB: newHypervisor("hv-b", hvCapCPU, vmCPU, hvCapMem, vmMem), + resA: newCommittedReservation("res-a", "hv-a", owner, "m1.large", flavorGroup, slotCPU, slotMem, + nil, nil, + ), + resB: newCommittedReservation("res-b", "hv-b", owner, "m1.large", flavorGroup, slotCPU, slotMem, + crSpecAllocs(crVm("vm-1", vmCPU, vmMem)), + map[string]string{"vm-1": "hv-b"}, + ), + expectedHosts: []string{"hv-a", "hv-b"}, + filteredHosts: []string{}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + objects := []client.Object{tt.hvA, tt.hvB, tt.resA, tt.resB} - for _, host := range tt.filteredHosts { - if _, ok := result.Activations[host]; ok { - t.Errorf("expected host %s to be filtered out", host) - } + step := &FilterHasEnoughCapacity{} + step.Client = fake.NewClientBuilder().WithScheme(scheme).WithObjects(objects...).Build() + step.Options = FilterHasEnoughCapacityOpts{LockReserved: false} + + request := newNovaRequest("instance-new", thirdParty, "m1.small", flavorGroup, 3, "6Gi", false, []string{"hv-a", "hv-b"}) + result, err := step.Run(slog.Default(), request) + if err != nil { + t.Fatalf("unexpected error: %v", err) } + assertActivations(t, result.Activations, tt.expectedHosts, tt.filteredHosts) }) } }