From 669c0a22aa0ba1d067ce090892c69563b588ac59 Mon Sep 17 00:00:00 2001 From: Markus Wieland Date: Wed, 4 Nov 2026 16:41:42 +0100 Subject: [PATCH 1/3] refactor: unify available capacity calculation Signed-off-by: Markus Wieland --- .../infrastructure/kvm_host_capacity.go | 115 ++-- .../infrastructure/kvm_host_capacity_test.go | 127 ----- internal/scheduling/capacity/capacity.go | 214 +++++++ internal/scheduling/capacity/capacity_test.go | 411 ++++++++++++++ .../filters/filter_has_enough_capacity.go | 211 ++----- .../reservations/accounting/accounting.go | 171 ------ .../accounting/accounting_test.go | 530 ------------------ .../reservations/accounting/flavor_groups.go | 4 + .../reservations/capacity/controller.go | 5 +- .../host_oversubscription_controller.go | 12 +- .../commitments/integration_test.go | 9 +- 11 files changed, 742 insertions(+), 1067 deletions(-) create mode 100644 internal/scheduling/capacity/capacity.go create mode 100644 internal/scheduling/capacity/capacity_test.go delete mode 100644 internal/scheduling/reservations/accounting/accounting.go delete mode 100644 internal/scheduling/reservations/accounting/accounting_test.go diff --git a/internal/knowledge/kpis/plugins/infrastructure/kvm_host_capacity.go b/internal/knowledge/kpis/plugins/infrastructure/kvm_host_capacity.go index 0885f24cb..983c89246 100644 --- a/internal/knowledge/kpis/plugins/infrastructure/kvm_host_capacity.go +++ b/internal/knowledge/kpis/plugins/infrastructure/kvm_host_capacity.go @@ -12,18 +12,13 @@ import ( "github.com/cobaltcore-dev/cortex/api/v1alpha1" "github.com/cobaltcore-dev/cortex/internal/knowledge/db" "github.com/cobaltcore-dev/cortex/internal/knowledge/kpis/plugins" + "github.com/cobaltcore-dev/cortex/internal/scheduling/capacity" "github.com/cobaltcore-dev/cortex/pkg/conf" "github.com/prometheus/client_golang/prometheus" hv1 "github.com/cobaltcore-dev/openstack-hypervisor-operator/api/v1" ) -// hostReservationResources holds aggregated CPU and memory reservation quantities for a single hypervisor. -type hostReservationResources struct { - cpu resource.Quantity - memory resource.Quantity -} - type KVMHostCapacityKPI struct { // Common base for all KPIs that provides standard functionality. plugins.BaseKPI[struct{}] // No options passed through yaml config @@ -59,69 +54,16 @@ func (k *KVMHostCapacityKPI) Describe(ch chan<- *prometheus.Desc) { ch <- k.capacityPerHost } -// aggregateReservationsByHost groups Ready reservations by host, returning per-host -// failover totals and committed-resource "not yet in use" totals. -func aggregateReservationsByHost(reservations []v1alpha1.Reservation) ( - failoverByHost map[string]hostReservationResources, - committedNotInUseByHost map[string]hostReservationResources, -) { - - failoverByHost = make(map[string]hostReservationResources) - committedNotInUseByHost = make(map[string]hostReservationResources) - - for _, reservation := range reservations { - if reservation.Spec.SchedulingDomain != v1alpha1.SchedulingDomainNova { - continue - } - - if !reservation.IsReady() { - continue - } - - host := reservation.Status.Host - if host == "" { - continue - } - - switch reservation.Spec.Type { - case v1alpha1.ReservationTypeFailover: - entry := failoverByHost[host] - cpuQty := reservation.Spec.Resources[hv1.ResourceCPU] - entry.cpu.Add(cpuQty) - memQty := reservation.Spec.Resources[hv1.ResourceMemory] - entry.memory.Add(memQty) - failoverByHost[host] = entry - - case v1alpha1.ReservationTypeCommittedResource: - // Compute remaining slot capacity: slot - sum of confirmed VM resources. - cpuNotInUse := reservation.Spec.Resources[hv1.ResourceCPU].DeepCopy() - memNotInUse := reservation.Spec.Resources[hv1.ResourceMemory].DeepCopy() - if reservation.Status.CommittedResourceReservation != nil { - zero := resource.MustParse("0") - for _, alloc := range reservation.Status.CommittedResourceReservation.Allocation { - if cpu, ok := alloc.Resources[hv1.ResourceCPU]; ok { - cpuNotInUse.Sub(cpu) - } - if mem, ok := alloc.Resources[hv1.ResourceMemory]; ok { - memNotInUse.Sub(mem) - } - } - if cpuNotInUse.Cmp(zero) < 0 { - cpuNotInUse = zero.DeepCopy() - } - if memNotInUse.Cmp(zero) < 0 { - memNotInUse = zero.DeepCopy() - } - } - - entry := committedNotInUseByHost[host] - entry.cpu.Add(cpuNotInUse) - entry.memory.Add(memNotInUse) - committedNotInUseByHost[host] = entry +// novaReservations returns the subset of reservations in the Nova scheduling domain. +// The shared capacity math is domain-agnostic; this KPI only accounts for Nova. +func novaReservations(reservations []v1alpha1.Reservation) []v1alpha1.Reservation { + out := make([]v1alpha1.Reservation, 0, len(reservations)) + for _, r := range reservations { + if r.Spec.SchedulingDomain == v1alpha1.SchedulingDomainNova { + out = append(out, r) } } - - return failoverByHost, committedNotInUseByHost + return out } func (k *KVMHostCapacityKPI) getHypervisors() ([]kvmHost, error) { @@ -150,7 +92,20 @@ func (k *KVMHostCapacityKPI) Collect(ch chan<- prometheus.Metric) { return } - failoverByHost, committedNotInUseByHost := aggregateReservationsByHost(reservations.Items) + // This KPI only accounts for ready Nova reservations, grouped by Status.Host. The + // shared capacity math is domain- and readiness-agnostic and keys off both TargetHost + // and Status.Host, so filter to ready Nova reservations and pin TargetHost to + // Status.Host — this reproduces the KPI's historical "committed slot minus confirmed + // VMs" accounting (UnusedReservationCapacity only nets out confirmed VMs when + // TargetHost == Status.Host, i.e. not mid-migration). + blocking := make([]v1alpha1.Reservation, 0, len(reservations.Items)) + for _, r := range novaReservations(reservations.Items) { + if !r.IsReady() || r.Status.Host == "" { + continue + } + r.Spec.TargetHost = r.Status.Host + blocking = append(blocking, r) + } for _, hypervisor := range hypervisors { cpuTotal, hasCPUTotal := hypervisor.getResourceCapacity(hv1.ResourceCPU) @@ -165,15 +120,21 @@ func (k *KVMHostCapacityKPI) Collect(ch chan<- prometheus.Metric) { cpuUsed := hypervisor.getResourceAllocation(hv1.ResourceCPU) ramUsed := hypervisor.getResourceAllocation(hv1.ResourceMemory) - // Get reservation data for this hypervisor (zero-value if absent). - failoverRes := failoverByHost[hypervisor.Name] - committedRes := committedNotInUseByHost[hypervisor.Name] - - cpuReserved := committedRes.cpu - ramReserved := committedRes.memory - - cpuFailover := failoverRes.cpu - ramFailover := failoverRes.memory + // Reservation blocks for this host, split into committed-not-in-use and failover + // by the shared capacity math (ClampToZero is irrelevant here — we clamp PAYG below). + b, err := capacity.DescribeHostUsage(hypervisor.Hypervisor, blocking, capacity.Options{ + BlockUnplacedReservations: true, + }) + if err != nil { + // getResourceCapacity already confirmed cpu+ram above, so this is unreachable + // in practice; skip defensively if the capacity source ever disagrees. + log.V(1).Info("skipping hypervisor without capacity data", "host", hypervisor.Name) + continue + } + cpuReserved := b.ReservedCommitted[hv1.ResourceCPU] + ramReserved := b.ReservedCommitted[hv1.ResourceMemory] + cpuFailover := b.ReservedFailover[hv1.ResourceCPU] + ramFailover := b.ReservedFailover[hv1.ResourceMemory] labels := hypervisor.getHostLabels() diff --git a/internal/knowledge/kpis/plugins/infrastructure/kvm_host_capacity_test.go b/internal/knowledge/kpis/plugins/infrastructure/kvm_host_capacity_test.go index 1a921a8c8..3bd933111 100644 --- a/internal/knowledge/kpis/plugins/infrastructure/kvm_host_capacity_test.go +++ b/internal/knowledge/kpis/plugins/infrastructure/kvm_host_capacity_test.go @@ -877,130 +877,3 @@ func TestKVMResourceCapacityKPI_Collect(t *testing.T) { }) } } - -func TestAggregateReservationsByHost(t *testing.T) { - tests := []struct { - name string - reservations []v1alpha1.Reservation - expectedFailover map[string]hostReservationResources - expectedCommittedNotInUse map[string]hostReservationResources - }{ - { - name: "empty reservations", - reservations: nil, - expectedFailover: map[string]hostReservationResources{}, - expectedCommittedNotInUse: map[string]hostReservationResources{}, - }, - { - name: "reservation with no ready condition is skipped", - reservations: []v1alpha1.Reservation{ - { - Spec: v1alpha1.ReservationSpec{ - Type: v1alpha1.ReservationTypeFailover, - SchedulingDomain: v1alpha1.SchedulingDomainNova, - Resources: map[hv1.ResourceName]resource.Quantity{ - hv1.ResourceCPU: resource.MustParse("10"), - }, - }, - Status: v1alpha1.ReservationStatus{ - Host: "host-1", - // No conditions - }, - }, - }, - expectedFailover: map[string]hostReservationResources{}, - expectedCommittedNotInUse: map[string]hostReservationResources{}, - }, - { - name: "reservation with empty host is skipped", - reservations: []v1alpha1.Reservation{ - { - Spec: v1alpha1.ReservationSpec{ - Type: v1alpha1.ReservationTypeFailover, - SchedulingDomain: v1alpha1.SchedulingDomainNova, - Resources: map[hv1.ResourceName]resource.Quantity{ - hv1.ResourceCPU: resource.MustParse("10"), - }, - }, - Status: v1alpha1.ReservationStatus{ - Host: "", - Conditions: []v1.Condition{ - {Type: v1alpha1.ReservationConditionReady, Status: v1.ConditionTrue}, - }, - }, - }, - }, - expectedFailover: map[string]hostReservationResources{}, - expectedCommittedNotInUse: map[string]hostReservationResources{}, - }, - { - name: "committed resource with nil spec does not panic", - reservations: []v1alpha1.Reservation{ - { - Spec: v1alpha1.ReservationSpec{ - Type: v1alpha1.ReservationTypeCommittedResource, - SchedulingDomain: v1alpha1.SchedulingDomainNova, - Resources: map[hv1.ResourceName]resource.Quantity{ - hv1.ResourceCPU: resource.MustParse("16"), - hv1.ResourceMemory: resource.MustParse("64Gi"), - }, - CommittedResourceReservation: nil, - }, - Status: v1alpha1.ReservationStatus{ - Host: "host-1", - Conditions: []v1.Condition{ - {Type: v1alpha1.ReservationConditionReady, Status: v1.ConditionTrue}, - }, - }, - }, - }, - expectedFailover: map[string]hostReservationResources{}, - expectedCommittedNotInUse: map[string]hostReservationResources{ - "host-1": { - cpu: resource.MustParse("16"), - memory: resource.MustParse("64Gi"), - }, - }, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - failover, committed := aggregateReservationsByHost(tt.reservations) - - if len(failover) != len(tt.expectedFailover) { - t.Errorf("failover map length: expected %d, got %d", len(tt.expectedFailover), len(failover)) - } - for host, expected := range tt.expectedFailover { - actual, ok := failover[host] - if !ok { - t.Errorf("failover: host %q not found", host) - continue - } - if actual.cpu.Cmp(expected.cpu) != 0 { - t.Errorf("failover[%s].cpu: expected %s, got %s", host, expected.cpu.String(), actual.cpu.String()) - } - if actual.memory.Cmp(expected.memory) != 0 { - t.Errorf("failover[%s].memory: expected %s, got %s", host, expected.memory.String(), actual.memory.String()) - } - } - - if len(committed) != len(tt.expectedCommittedNotInUse) { - t.Errorf("committed map length: expected %d, got %d", len(tt.expectedCommittedNotInUse), len(committed)) - } - for host, expected := range tt.expectedCommittedNotInUse { - actual, ok := committed[host] - if !ok { - t.Errorf("committed: host %q not found", host) - continue - } - if actual.cpu.Cmp(expected.cpu) != 0 { - t.Errorf("committed[%s].cpu: expected %s, got %s", host, expected.cpu.String(), actual.cpu.String()) - } - if actual.memory.Cmp(expected.memory) != 0 { - t.Errorf("committed[%s].memory: expected %s, got %s", host, expected.memory.String(), actual.memory.String()) - } - } - }) - } -} diff --git a/internal/scheduling/capacity/capacity.go b/internal/scheduling/capacity/capacity.go new file mode 100644 index 000000000..0f1c1c21f --- /dev/null +++ b/internal/scheduling/capacity/capacity.go @@ -0,0 +1,214 @@ +// Copyright SAP SE +// SPDX-License-Identifier: Apache-2.0 + +// Package capacity holds the single source of truth for computing the usage and +// available capacity of a hypervisor including reservations. It is a self-contained +// helper over Reservation/Hypervisor CRDs with no dependency on the scheduling +// pipeline or the knowledge extractors, so it can be imported from below both the +// scheduling tree and the knowledge KPI tree without forming an import cycle. +// +// Three consumers share this math: the Nova capacity filter +// (filter_has_enough_capacity), the reservation controllers (via the accounting +// package), and the KVM host-capacity KPI. +package capacity + +import ( + "errors" + "slices" + + hv1 "github.com/cobaltcore-dev/openstack-hypervisor-operator/api/v1" + "k8s.io/apimachinery/pkg/api/resource" + + "github.com/cobaltcore-dev/cortex/api/v1alpha1" +) + +// ErrNoCapacityData is returned by DescribeHostUsage when the hypervisor reports neither +// EffectiveCapacity nor Capacity — typically a host that has not been fully discovered +// yet. Callers should skip such a host. +var ErrNoCapacityData = errors.New("hypervisor has no capacity data") + +// HostUsage is the per-resource usage accounting for a single hypervisor, including +// reservations. All maps are keyed by hv1.ResourceName. +type HostUsage struct { + // Capacity is the starting capacity: Status.EffectiveCapacity (capacity × + // overcommit ratio), falling back to Status.Capacity when EffectiveCapacity is nil. + Capacity map[hv1.ResourceName]resource.Quantity + // Allocated is Status.Allocation (aggregate real-time usage of all running VMs). + // Empty when Options.IgnoreAllocations is set. + Allocated map[hv1.ResourceName]resource.Quantity + // ReservedCommitted is the per-resource block from CommittedResourceReservations + // (and any other non-failover reservation type that blocks), i.e. the slot capacity + // not yet filled by confirmed VMs. + ReservedCommitted map[hv1.ResourceName]resource.Quantity + // ReservedFailover is the per-resource block from FailoverReservations. + ReservedFailover map[hv1.ResourceName]resource.Quantity + // Free is Capacity − Allocated − ReservedCommitted − ReservedFailover. Negative + // values indicate over-subscription unless Options.ClampToZero floored them at 0. + Free map[hv1.ResourceName]resource.Quantity +} + +// Options tunes how DescribeHostUsage accounts for allocations and reservations. +type Options struct { + // IgnoreAllocations skips subtracting Status.Allocation (and makes + // UnusedReservationCapacity block the full slot). Used for empty-datacenter + // capacity queries. + IgnoreAllocations bool + // IgnoredReservationTypes lists reservation types whose capacity is not blocked. + IgnoredReservationTypes []v1alpha1.ReservationType + // ClampToZero floors each Free entry at 0. Consumers that only care about + // remaining headroom (the filter, the KPI) set this; consumers that need to + // detect over-subscription (the reservation controllers) leave it false. + ClampToZero bool + // BlockUnplacedReservations, when false (the default), skips reservations whose + // Spec.TargetHost is empty — ignoring evicted reservations whose Status.Host may + // still lag. When true, a reservation blocks on Status.Host even with an empty + // TargetHost (the stricter semantics used for fit checks). + BlockUnplacedReservations bool + // ExcludeReservation, when set, skips the reservation with this name+namespace — + // e.g. to exclude the reservation being placed from its own fit check. + ExcludeReservation *v1alpha1.Reservation +} + +// DescribeHostUsage computes the usage breakdown for hv, subtracting allocation and +// the reservations that target this host. reservations may contain reservations for +// other hosts; only those targeting hv (via Spec.TargetHost or Status.Host == hv.Name) +// are counted. Returns ErrNoCapacityData when hv reports neither EffectiveCapacity nor +// Capacity. +func DescribeHostUsage(hv hv1.Hypervisor, reservations []v1alpha1.Reservation, opts Options) (HostUsage, error) { + effCap := hv.Status.EffectiveCapacity + if effCap == nil { + effCap = hv.Status.Capacity + } + if effCap == nil { + return HostUsage{}, ErrNoCapacityData + } + + u := HostUsage{ + Capacity: make(map[hv1.ResourceName]resource.Quantity, len(effCap)), + Allocated: make(map[hv1.ResourceName]resource.Quantity), + ReservedCommitted: make(map[hv1.ResourceName]resource.Quantity), + ReservedFailover: make(map[hv1.ResourceName]resource.Quantity), + Free: make(map[hv1.ResourceName]resource.Quantity, len(effCap)), + } + for rn, qty := range effCap { + u.Capacity[rn] = qty.DeepCopy() + u.Free[rn] = qty.DeepCopy() + } + + if !opts.IgnoreAllocations { + for rn, allocated := range hv.Status.Allocation { + u.Allocated[rn] = allocated.DeepCopy() + if f, ok := u.Free[rn]; ok { + f.Sub(allocated) + u.Free[rn] = f + } + } + } + + for i := range reservations { + res := &reservations[i] + if opts.ExcludeReservation != nil && + res.Name == opts.ExcludeReservation.Name && + res.Namespace == opts.ExcludeReservation.Namespace { + continue + } + if slices.Contains(opts.IgnoredReservationTypes, res.Spec.Type) { + continue + } + if !opts.BlockUnplacedReservations && res.Spec.TargetHost == "" { + continue // evicted; status.Host may lag until reconcile + } + if res.Spec.TargetHost != hv.Name && res.Status.Host != hv.Name { + continue + } + block := UnusedReservationCapacity(res, opts.IgnoreAllocations) + bucket := u.ReservedCommitted + if res.Spec.Type == v1alpha1.ReservationTypeFailover { + bucket = u.ReservedFailover + } + for rn, qty := range block { + if existing, ok := bucket[rn]; ok { + existing.Add(qty) + bucket[rn] = existing + } else { + bucket[rn] = qty.DeepCopy() + } + if f, ok := u.Free[rn]; ok { + f.Sub(qty) + u.Free[rn] = f + } + } + } + + if opts.ClampToZero { + zero := resource.MustParse("0") + for rn, f := range u.Free { + if f.Cmp(zero) < 0 { + u.Free[rn] = zero.DeepCopy() + } + } + } + return u, nil +} + +// Fits reports whether required fits within u.Free — i.e. every requested resource +// has at least that much free capacity remaining. A required resource absent from +// u.Free (the host does not report it) counts as not fitting. +func (u HostUsage) Fits(required map[hv1.ResourceName]resource.Quantity) bool { + for rn, qty := range required { + remaining, ok := u.Free[rn] + if !ok { + return false + } + if remaining.Cmp(qty) < 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 the capacity filter, the capacity controller, +// and the host-capacity KPI to ensure consistent accounting. +// +// CommittedResourceReservations: confirmed VMs already appear in hv.Status.Allocation, +// so blocking the full slot would double-count them. The remaining block is +// max(slot − sum(confirmed VM resources), 0) per resource. Skipped when ignoreAllocations +// is true or when mid-migration (TargetHost != Status.Host). +// +// FailoverReservations: always block the full Spec.Resources. +// +// FailoverHostReservations: block nothing. FailoverHostReservation is a new feature +// that reserves an entire host, and is intentionally not yet counted as blocked capacity +// by any consumer of this function (capacity filter, capacity controller, oversubscription +// controller). Returning an empty block here keeps all of them consistent in one place. +func UnusedReservationCapacity(res *v1alpha1.Reservation, ignoreAllocations bool) map[hv1.ResourceName]resource.Quantity { + if res.Spec.Type == v1alpha1.ReservationTypeFailoverHost { + // TODO: When failover reservations are in use, account for them. + return nil + } + if res.Spec.Type == v1alpha1.ReservationTypeCommittedResource && + !ignoreAllocations && + res.Spec.TargetHost == res.Status.Host && + res.Status.CommittedResourceReservation != nil && + len(res.Status.CommittedResourceReservation.Allocation) > 0 { + // Compute remaining slot capacity: slot - sum of confirmed VM resources. + result := make(map[hv1.ResourceName]resource.Quantity, len(res.Spec.Resources)) + zero := resource.MustParse("0") + for rn, slotQty := range res.Spec.Resources { + remaining := slotQty.DeepCopy() + for _, alloc := range res.Status.CommittedResourceReservation.Allocation { + if used, ok := alloc.Resources[rn]; ok { + remaining.Sub(used) + } + } + if remaining.Cmp(zero) < 0 { + remaining = zero.DeepCopy() + } + result[rn] = remaining + } + return result + } + // No confirmed allocations, or ignoreAllocations, or mid-migration: block the full slot. + return res.Spec.Resources +} diff --git a/internal/scheduling/capacity/capacity_test.go b/internal/scheduling/capacity/capacity_test.go new file mode 100644 index 000000000..f4ecf9fc0 --- /dev/null +++ b/internal/scheduling/capacity/capacity_test.go @@ -0,0 +1,411 @@ +// Copyright SAP SE +// SPDX-License-Identifier: Apache-2.0 + +package capacity + +import ( + "testing" + + hv1 "github.com/cobaltcore-dev/openstack-hypervisor-operator/api/v1" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "github.com/cobaltcore-dev/cortex/api/v1alpha1" +) + +func gib(n int64) resource.Quantity { + return *resource.NewQuantity(n*1024*1024*1024, resource.BinarySI) +} +func cpu(n int64) resource.Quantity { return *resource.NewQuantity(n, resource.DecimalSI) } + +func hvWithEffCap(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), + }, + }, + } +} + +func crSlot(name, host 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: gib(memGiB), + hv1.ResourceCPU: cpu(cpuCores), + }, + }, + Status: v1alpha1.ReservationStatus{Host: host}, + } +} + +func memGiB(m map[hv1.ResourceName]resource.Quantity) int64 { + q := m[hv1.ResourceMemory] + return q.Value() / (1024 * 1024 * 1024) +} +func cpus(m map[hv1.ResourceName]resource.Quantity) int64 { + q := m[hv1.ResourceCPU] + return q.Value() +} + +// mustDescribe calls DescribeHostUsage and fails the test on an unexpected error. +func mustDescribe(t *testing.T, hv hv1.Hypervisor, res []v1alpha1.Reservation, opts Options) HostUsage { + t.Helper() + u, err := DescribeHostUsage(hv, res, opts) + if err != nil { + t.Fatalf("DescribeHostUsage() unexpected error: %v", err) + } + return u +} + +func TestDescribeHostUsage_NoData(t *testing.T) { + _, err := DescribeHostUsage(hv1.Hypervisor{ObjectMeta: metav1.ObjectMeta{Name: "h"}}, nil, Options{}) + if err == nil { + t.Fatal("expected ErrNoCapacityData, got nil") + } +} + +func TestDescribeHostUsage_CapacityFallback(t *testing.T) { + hv := hv1.Hypervisor{ + ObjectMeta: metav1.ObjectMeta{Name: "h"}, + Status: hv1.HypervisorStatus{ + Capacity: map[hv1.ResourceName]resource.Quantity{hv1.ResourceMemory: gib(512)}, + }, + } + b := mustDescribe(t, hv, nil, Options{}) + if memGiB(b.Free) != 512 { + t.Errorf("expected 512 GiB free from Capacity fallback, got %d", memGiB(b.Free)) + } +} + +func TestDescribeHostUsage_AllocationSubtracted(t *testing.T) { + hv := hvWithEffCap("h", 1024, 256) + hv.Status.Allocation = map[hv1.ResourceName]resource.Quantity{ + hv1.ResourceMemory: gib(512), + hv1.ResourceCPU: cpu(128), + } + b := mustDescribe(t, hv, nil, Options{}) + if memGiB(b.Free) != 512 || cpus(b.Free) != 128 { + t.Errorf("free = %d GiB / %d CPU, want 512 / 128", memGiB(b.Free), cpus(b.Free)) + } + if memGiB(b.Allocated) != 512 { + t.Errorf("Allocated = %d GiB, want 512", memGiB(b.Allocated)) + } +} + +func TestDescribeHostUsage_IgnoreAllocations(t *testing.T) { + hv := hvWithEffCap("h", 1024, 256) + hv.Status.Allocation = map[hv1.ResourceName]resource.Quantity{hv1.ResourceMemory: gib(512)} + b := mustDescribe(t, hv, nil, Options{IgnoreAllocations: true}) + if memGiB(b.Free) != 1024 { + t.Errorf("free = %d GiB, want 1024 (allocations ignored)", memGiB(b.Free)) + } + if len(b.Allocated) != 0 { + t.Errorf("Allocated should be empty, got %+v", b.Allocated) + } +} + +func TestDescribeHostUsage_CommittedVsFailoverSplit(t *testing.T) { + hv := hvWithEffCap("h", 1024, 256) + failover := v1alpha1.Reservation{ + ObjectMeta: metav1.ObjectMeta{Name: "fo"}, + Spec: v1alpha1.ReservationSpec{ + Type: v1alpha1.ReservationTypeFailover, + TargetHost: "h", + Resources: map[hv1.ResourceName]resource.Quantity{ + hv1.ResourceMemory: gib(64), hv1.ResourceCPU: cpu(16), + }, + }, + Status: v1alpha1.ReservationStatus{Host: "h"}, + } + committed := crSlot("cr", "h", 128, 32) + b := mustDescribe(t, hv, []v1alpha1.Reservation{failover, committed}, Options{}) + if memGiB(b.ReservedFailover) != 64 || cpus(b.ReservedFailover) != 16 { + t.Errorf("failover = %d GiB / %d CPU, want 64 / 16", memGiB(b.ReservedFailover), cpus(b.ReservedFailover)) + } + if memGiB(b.ReservedCommitted) != 128 || cpus(b.ReservedCommitted) != 32 { + t.Errorf("committed = %d GiB / %d CPU, want 128 / 32", memGiB(b.ReservedCommitted), cpus(b.ReservedCommitted)) + } + // Free = 1024 - 64 - 128 = 832, 256 - 16 - 32 = 208 + if memGiB(b.Free) != 832 || cpus(b.Free) != 208 { + t.Errorf("free = %d GiB / %d CPU, want 832 / 208", memGiB(b.Free), cpus(b.Free)) + } +} + +func TestDescribeHostUsage_MultipleSameTypeSummed(t *testing.T) { + hv := hvWithEffCap("h", 1024, 256) + b := mustDescribe(t, hv, []v1alpha1.Reservation{ + crSlot("cr-1", "h", 100, 10), + crSlot("cr-2", "h", 50, 5), + }, Options{}) + if memGiB(b.ReservedCommitted) != 150 || cpus(b.ReservedCommitted) != 15 { + t.Errorf("committed sum = %d GiB / %d CPU, want 150 / 15", memGiB(b.ReservedCommitted), cpus(b.ReservedCommitted)) + } +} + +func TestDescribeHostUsage_OtherHostIgnored(t *testing.T) { + hv := hvWithEffCap("h", 1024, 256) + b := mustDescribe(t, hv, []v1alpha1.Reservation{crSlot("other", "other-host", 1024, 256)}, Options{}) + if memGiB(b.Free) != 1024 { + t.Errorf("free = %d GiB, want 1024 (other host ignored)", memGiB(b.Free)) + } +} + +func TestDescribeHostUsage_OverSubscribedNegative(t *testing.T) { + hv := hvWithEffCap("h", 1024, 256) + b := mustDescribe(t, hv, []v1alpha1.Reservation{crSlot("cr", "h", 2048, 512)}, Options{}) + freeMem := b.Free[hv1.ResourceMemory] + if freeMem.Value() >= 0 { + t.Errorf("expected negative free memory, got %d", freeMem.Value()) + } +} + +func TestDescribeHostUsage_ClampToZero(t *testing.T) { + hv := hvWithEffCap("h", 1024, 256) + b := mustDescribe(t, hv, []v1alpha1.Reservation{crSlot("cr", "h", 2048, 512)}, Options{ClampToZero: true}) + if memGiB(b.Free) != 0 || cpus(b.Free) != 0 { + t.Errorf("free = %d GiB / %d CPU, want 0 / 0 (clamped)", memGiB(b.Free), cpus(b.Free)) + } +} + +func TestDescribeHostUsage_IgnoredReservationTypes(t *testing.T) { + hv := hvWithEffCap("h", 1024, 256) + b := mustDescribe(t, hv, []v1alpha1.Reservation{crSlot("cr", "h", 512, 128)}, Options{ + IgnoredReservationTypes: []v1alpha1.ReservationType{v1alpha1.ReservationTypeCommittedResource}, + }) + if memGiB(b.Free) != 1024 { + t.Errorf("free = %d GiB, want 1024 (type ignored)", memGiB(b.Free)) + } +} + +func TestDescribeHostUsage_UnplacedSkippedByDefault(t *testing.T) { + hv := hvWithEffCap("h", 1024, 256) + // TargetHost empty, Status.Host = h. Default BlockUnplacedReservations=false skips it. + res := crSlot("cr", "", 512, 128) + res.Status.Host = "h" + b := mustDescribe(t, hv, []v1alpha1.Reservation{res}, Options{}) + if memGiB(b.Free) != 1024 { + t.Errorf("free = %d GiB, want 1024 (unplaced skipped)", memGiB(b.Free)) + } + // With BlockUnplacedReservations it blocks on Status.Host. + b = mustDescribe(t, hv, []v1alpha1.Reservation{res}, Options{BlockUnplacedReservations: true}) + if memGiB(b.Free) != 512 { + t.Errorf("free = %d GiB, want 512 (unplaced blocked on Status.Host)", memGiB(b.Free)) + } +} + +func TestDescribeHostUsage_ExcludeReservation(t *testing.T) { + hv := hvWithEffCap("h", 1024, 256) + target := crSlot("target", "h", 512, 128) + b := mustDescribe(t, hv, []v1alpha1.Reservation{target}, Options{ExcludeReservation: &target}) + if memGiB(b.Free) != 1024 { + t.Errorf("free = %d GiB, want 1024 (excluded reservation not blocked)", memGiB(b.Free)) + } +} + +func TestDescribeHostUsage_ConfirmedVMNotDoubleCounted(t *testing.T) { + hv := hvWithEffCap("h", 1024, 256) + hv.Status.Allocation = map[hv1.ResourceName]resource.Quantity{hv1.ResourceMemory: gib(256)} + // 512 GiB slot, fully filled by one confirmed VM → block = 0. + slot := v1alpha1.Reservation{ + ObjectMeta: metav1.ObjectMeta{Name: "cr"}, + Spec: v1alpha1.ReservationSpec{ + Type: v1alpha1.ReservationTypeCommittedResource, + TargetHost: "h", + Resources: map[hv1.ResourceName]resource.Quantity{hv1.ResourceMemory: gib(512)}, + }, + Status: v1alpha1.ReservationStatus{ + Host: "h", + CommittedResourceReservation: &v1alpha1.CommittedResourceReservationStatus{ + Allocation: map[string]v1alpha1.CommittedResourceAllocation{ + "vm-1": { + ObjectRef: corev1.ObjectReference{Name: "vm-1"}, + Resources: map[hv1.ResourceName]resource.Quantity{hv1.ResourceMemory: gib(512)}, + }, + }, + }, + }, + } + b := mustDescribe(t, hv, []v1alpha1.Reservation{slot}, Options{}) + // 1024 - 256 (alloc) - 0 (slot filled) = 768 + if memGiB(b.Free) != 768 { + t.Errorf("free = %d GiB, want 768", memGiB(b.Free)) + } +} + +func TestDescribeHostUsage_MidMigrationFullSlot(t *testing.T) { + hv := hvWithEffCap("new-host", 1024, 256) + // TargetHost != Status.Host → mid-migration → full slot blocked despite confirmed VMs. + slot := v1alpha1.Reservation{ + ObjectMeta: metav1.ObjectMeta{Name: "cr"}, + Spec: v1alpha1.ReservationSpec{ + Type: v1alpha1.ReservationTypeCommittedResource, + TargetHost: "new-host", + Resources: map[hv1.ResourceName]resource.Quantity{hv1.ResourceMemory: gib(480)}, + }, + Status: v1alpha1.ReservationStatus{ + Host: "old-host", + CommittedResourceReservation: &v1alpha1.CommittedResourceReservationStatus{ + Allocation: map[string]v1alpha1.CommittedResourceAllocation{ + "vm-1": {Resources: map[hv1.ResourceName]resource.Quantity{hv1.ResourceMemory: gib(480)}}, + }, + }, + }, + } + b := mustDescribe(t, hv, []v1alpha1.Reservation{slot}, Options{}) + if memGiB(b.ReservedCommitted) != 480 { + t.Errorf("committed = %d GiB, want 480 (full slot, mid-migration)", memGiB(b.ReservedCommitted)) + } +} + +func TestUnusedReservationCapacity(t *testing.T) { + memBytes := func(m map[hv1.ResourceName]resource.Quantity) int64 { + q := m[hv1.ResourceMemory] + return q.Value() + } + tests := []struct { + name string + res *v1alpha1.Reservation + ignoreAllocations bool + wantMemoryBytes int64 + }{ + { + name: "failover: full slot blocked", + res: &v1alpha1.Reservation{Spec: v1alpha1.ReservationSpec{ + Type: v1alpha1.ReservationTypeFailover, + Resources: map[hv1.ResourceName]resource.Quantity{hv1.ResourceMemory: gib(480)}, + }}, + wantMemoryBytes: 480 * 1024 * 1024 * 1024, + }, + { + name: "failover host: nothing blocked", + res: &v1alpha1.Reservation{Spec: v1alpha1.ReservationSpec{ + Type: v1alpha1.ReservationTypeFailoverHost, + Resources: map[hv1.ResourceName]resource.Quantity{hv1.ResourceMemory: gib(480)}, + }}, + wantMemoryBytes: 0, + }, + { + name: "CR fully consumed by confirmed VM: block 0", + res: &v1alpha1.Reservation{ + Spec: v1alpha1.ReservationSpec{ + Type: v1alpha1.ReservationTypeCommittedResource, + Resources: map[hv1.ResourceName]resource.Quantity{hv1.ResourceMemory: gib(480)}, + }, + Status: v1alpha1.ReservationStatus{ + CommittedResourceReservation: &v1alpha1.CommittedResourceReservationStatus{ + Allocation: map[string]v1alpha1.CommittedResourceAllocation{ + "vm-1": {Resources: map[hv1.ResourceName]resource.Quantity{hv1.ResourceMemory: gib(480)}}, + }, + }, + }, + }, + wantMemoryBytes: 0, + }, + { + name: "CR ignoreAllocations: full slot blocked", + res: &v1alpha1.Reservation{ + Spec: v1alpha1.ReservationSpec{ + Type: v1alpha1.ReservationTypeCommittedResource, + Resources: map[hv1.ResourceName]resource.Quantity{hv1.ResourceMemory: gib(480)}, + }, + Status: v1alpha1.ReservationStatus{ + CommittedResourceReservation: &v1alpha1.CommittedResourceReservationStatus{ + Allocation: map[string]v1alpha1.CommittedResourceAllocation{ + "vm-1": {Resources: map[hv1.ResourceName]resource.Quantity{hv1.ResourceMemory: gib(480)}}, + }, + }, + }, + }, + ignoreAllocations: true, + wantMemoryBytes: 480 * 1024 * 1024 * 1024, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := memBytes(UnusedReservationCapacity(tt.res, tt.ignoreAllocations)); got != tt.wantMemoryBytes { + t.Errorf("UnusedReservationCapacity() memory = %d, want %d", got, tt.wantMemoryBytes) + } + }) + } +} + +func TestFits(t *testing.T) { + req := func(memGiBv, cpuCores int64) map[hv1.ResourceName]resource.Quantity { + return map[hv1.ResourceName]resource.Quantity{ + hv1.ResourceMemory: gib(memGiBv), + hv1.ResourceCPU: cpu(cpuCores), + } + } + + tests := []struct { + name string + hv hv1.Hypervisor + others []v1alpha1.Reservation + required map[hv1.ResourceName]resource.Quantity + wantFits bool + }{ + { + name: "empty host: request fits easily", + hv: hvWithEffCap("host", 960, 80), + required: req(480, 40), + wantFits: true, + }, + { + name: "host fully consumed by a reservation: no room", + hv: hvWithEffCap("host", 480, 40), + others: []v1alpha1.Reservation{crSlot("blocker", "host", 480, 40)}, + required: req(480, 40), + wantFits: false, + }, + { + name: "host partially consumed, enough room left", + hv: hvWithEffCap("host", 960, 80), + others: []v1alpha1.Reservation{crSlot("blocker", "host", 480, 40)}, + required: req(480, 40), + wantFits: true, + }, + { + name: "one resource short (CPU)", + hv: hvWithEffCap("host", 960, 60), + others: []v1alpha1.Reservation{crSlot("blocker", "host", 480, 40)}, + required: req(480, 40), // mem 480 OK, but 60-40=20 CPU < 40 + wantFits: false, + }, + { + name: "allocation already consumed memory: no room", + hv: func() hv1.Hypervisor { + h := hvWithEffCap("host", 480, 40) + h.Status.Allocation = map[hv1.ResourceName]resource.Quantity{hv1.ResourceMemory: gib(100)} + return h + }(), + required: req(480, 40), // 480-100=380 < 480 + wantFits: false, + }, + { + name: "required resource the host does not report: does not fit", + hv: hvWithEffCap("host", 960, 80), + required: map[hv1.ResourceName]resource.Quantity{hv1.ResourceName("disk"): gib(10)}, + wantFits: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + usage, err := DescribeHostUsage(tt.hv, tt.others, Options{BlockUnplacedReservations: true}) + if err != nil { + t.Fatalf("DescribeHostUsage() error: %v", err) + } + if got := usage.Fits(tt.required); got != tt.wantFits { + t.Errorf("Fits() = %v, want %v", got, tt.wantFits) + } + }) + } +} 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 345c59b90..de232cc5e 100644 --- a/internal/scheduling/nova/plugins/filters/filter_has_enough_capacity.go +++ b/internal/scheduling/nova/plugins/filters/filter_has_enough_capacity.go @@ -12,9 +12,9 @@ import ( api "github.com/cobaltcore-dev/cortex/api/external/nova" "github.com/cobaltcore-dev/cortex/api/v1alpha1" + "github.com/cobaltcore-dev/cortex/internal/scheduling/capacity" "github.com/cobaltcore-dev/cortex/internal/scheduling/compute" "github.com/cobaltcore-dev/cortex/internal/scheduling/lib" - resv "github.com/cobaltcore-dev/cortex/internal/scheduling/reservations/accounting" hv1 "github.com/cobaltcore-dev/openstack-hypervisor-operator/api/v1" "k8s.io/apimachinery/pkg/api/resource" ) @@ -72,59 +72,24 @@ func (s *FilterHasEnoughCapacity) Run(traceLog logr.Logger, request compute.Plac ignoreAllocations := s.Options.IgnoreAllocations || opts.AssumeEmptyHosts ignoredReservationTypes := slices.Concat(s.Options.IgnoredReservationTypes, opts.IgnoredReservationTypes) - // This map holds the free resources per host. - freeResourcesByHost := make(map[string]map[hv1.ResourceName]resource.Quantity) - - // The hypervisor resource auto-discovers its current utilization. - // We can use the hypervisor status to calculate the total capacity - // and then subtract the actual resource allocation from virtual machines. + // Collect the candidate hypervisors for this request. hvs := &hv1.HypervisorList{} if err := s.Client.List(context.Background(), hvs); err != nil { traceLog.Error(err, "failed to list hypervisors") return nil, err } - for _, hv := range hvs.Items { - if _, ok := result.Activations[hv.Name]; !ok { - continue - } - var sourceMap map[hv1.ResourceName]resource.Quantity - if hv.Status.EffectiveCapacity == nil { - traceLog.V(1).Info("hypervisor with nil effective capacity, use capacity instead (overprovisioning not considered)", "host", hv.Name) - sourceMap = hv.Status.Capacity - } else { - // Start with the total effective capacity which is capacity * overcommit ratio. - sourceMap = hv.Status.EffectiveCapacity - } - // Deep-copy the map to avoid mutating the informer cache. - copied := make(map[hv1.ResourceName]resource.Quantity, len(sourceMap)) - for k, v := range sourceMap { - copied[k] = v.DeepCopy() - } - freeResourcesByHost[hv.Name] = copied - - // Subtract allocated resources (skip when ignoring allocations for empty-datacenter capacity queries). - if !ignoreAllocations { - for resourceName, allocated := range hv.Status.Allocation { - free, ok := freeResourcesByHost[hv.Name][resourceName] - if !ok { - traceLog.Error( - nil, - "hypervisor with allocation for unknown resource", - "host", hv.Name, "resource", resourceName, - ) - continue - } - free.Sub(allocated) - freeResourcesByHost[hv.Name][resourceName] = free - } - } - } - // Subtract reserved resources by Reservations. + // Decide, per reservation, whether it should block capacity for THIS request. + // The request-aware unlock/slotting logic (CR project+flavor match, failover + // evacuation/reuse, not-ready double-booking) lives here; the resulting set of + // blocking reservations is then fed to the shared capacity math, which performs + // the effective-capacity / allocation / reservation subtraction identically for + // every consumer. var reservations v1alpha1.ReservationList if err := s.Client.List(context.Background(), &reservations); err != nil { return nil, err } + reservationsToBlock := make([]v1alpha1.Reservation, 0, len(reservations.Items)) for _, reservation := range reservations.Items { // Check if this reservation type should be ignored — applies regardless of ready state. if slices.Contains(ignoredReservationTypes, reservation.Spec.Type) { @@ -205,124 +170,68 @@ func (s *FilterHasEnoughCapacity) Run(traceLog logr.Logger, request compute.Plac } } - // Block resources on BOTH Spec.TargetHost (desired) AND Status.Host (actual). - // This ensures capacity is blocked during the transition period when a reservation - // is being placed (TargetHost set) and after it's placed (Host set). - // If both are the same, we only subtract once. - hostsToBlock := make(map[string]struct{}) - if reservation.Spec.TargetHost != "" { - hostsToBlock[reservation.Spec.TargetHost] = struct{}{} - } - if reservation.Status.Host != "" { - hostsToBlock[reservation.Status.Host] = struct{}{} - } - if len(hostsToBlock) == 0 { + if reservation.Spec.TargetHost == "" && reservation.Status.Host == "" { traceLog.V(1).Info("skipping reservation with no host", "reservation", reservation.Name) continue } + reservationsToBlock = append(reservationsToBlock, reservation) + } - // CommittedResourceReservations: block = max(slot − confirmed VM resources, 0). - // Confirmed VMs appear in hv.Status.Allocation; this subtracts only the unconfirmed remainder - // to avoid double-counting. FailoverReservations: block = Spec.Resources (always fully blocked). - resourcesToBlock := resv.UnusedReservationCapacity(&reservation, ignoreAllocations) - - // Block the calculated resources on each host - for host := range hostsToBlock { - // Skip hosts that don't have a corresponding Hypervisor resource. - if _, hostExists := freeResourcesByHost[host]; !hostExists { - traceLog.V(1).Info("skipping reservation for unknown host", - "reservation", reservation.Name, - "host", host) - continue - } - if cpu, ok := resourcesToBlock["cpu"]; ok { - if freeCPU, exists := freeResourcesByHost[host]["cpu"]; exists { - freeCPU.Sub(cpu) - if freeCPU.Value() < 0 { - traceLog.V(1).Info("negative free CPU after blocking reservation", - "host", host, - "reservation", reservation.Name, - "reservationType", reservation.Spec.Type, - "freeCPU", freeCPU.String(), - "blocked", cpu.String()) - freeCPU = resource.Quantity{} - } - freeResourcesByHost[host]["cpu"] = freeCPU - } - } - if memory, ok := resourcesToBlock["memory"]; ok { - if freeMemory, exists := freeResourcesByHost[host]["memory"]; exists { - freeMemory.Sub(memory) - if freeMemory.Value() < 0 { - traceLog.V(1).Info("negative free memory after blocking reservation", - "host", host, - "reservation", reservation.Name, - "reservationType", reservation.Spec.Type, - "freeMemory", freeMemory.String(), - "blocked", memory.String()) - freeMemory = resource.Quantity{} - } - freeResourcesByHost[host]["memory"] = freeMemory - } - } + // Compute usage per candidate host via the shared capacity math. + // DescribeHostUsage starts from EffectiveCapacity (falling back to Capacity), subtracts + // allocation (unless ignored), and subtracts each surviving reservation's block on + // both its TargetHost and Status.Host, flooring at zero. + usageByHost := make(map[string]capacity.HostUsage) + for _, hv := range hvs.Items { + if _, ok := result.Activations[hv.Name]; !ok { + continue } + if hv.Status.EffectiveCapacity == nil { + traceLog.V(1).Info("hypervisor with nil effective capacity, use capacity instead (overprovisioning not considered)", "host", hv.Name) + } + usage, err := capacity.DescribeHostUsage(hv, reservationsToBlock, capacity.Options{ + IgnoreAllocations: ignoreAllocations, + BlockUnplacedReservations: true, + ClampToZero: true, + }) + if err != nil { + continue // no capacity data; dropped below as an un-encountered host + } + usageByHost[hv.Name] = usage + } + + // A host must fit the full request: flavor resources × NumInstances. We can't spread + // instances across hosts, so all requested instances must land on the same host. + if request.Flavor.VCPUs == 0 { + return nil, errors.New("flavor has 0 vcpus") + } + if request.Flavor.MemoryMB == 0 { + return nil, errors.New("flavor has 0 memory") + } + // Nova's memory_mb field is named "megabytes" in the API docs, but the values are in + // mebibytes (1 MiB = 2^20 bytes). Multiply by 1<<20 to convert to the bytes used in + // hv.Status. + //nolint:gosec // flavor fields are small, non-negative counts. + required := map[hv1.ResourceName]resource.Quantity{ + "cpu": *resource.NewQuantity(int64(request.Flavor.VCPUs*request.NumInstances), resource.DecimalSI), + "memory": *resource.NewQuantity(int64(request.Flavor.MemoryMB*request.NumInstances*(1<<20)), resource.BinarySI), } hostsEncountered := make(map[string]struct{}) - for host, free := range freeResourcesByHost { + for host, usage := range usageByHost { hostsEncountered[host] = struct{}{} - // Check cpu capacity. - if request.Flavor.VCPUs == 0 { - return nil, errors.New("flavor has 0 vcpus") - } - freeCPU, ok := free["cpu"] - if !ok || freeCPU.Value() < 0 { + freeCPU := usage.Free["cpu"] + freeMemory := usage.Free["memory"] + if !usage.Fits(required) { traceLog.V(1).Info( - "host with invalid CPU capacity", - "host", host, "freeCPU", freeCPU.String(), - ) - delete(result.Activations, host) // host is already overloaded, remove from activations - continue - } - // Calculate how many instances can fit on this host, based on cpu. - //nolint:gosec // We're checking for underflows above (< 0). - vcpuSlots := uint64(freeCPU.Value()) / - request.Flavor.VCPUs - if vcpuSlots < request.NumInstances { - traceLog.V(1).Info( - "filtering host due to insufficient CPU capacity", - "host", host, "requested", request.Flavor.VCPUs, - "available", freeCPU.String(), - ) - delete(result.Activations, host) - continue - } - - // Check memory capacity. - if request.Flavor.MemoryMB == 0 { - return nil, errors.New("flavor has 0 memory") - } - freeMemory, ok := free["memory"] - if !ok || freeMemory.Value() < 0 { - traceLog.V(1).Info( - "host with invalid memory capacity", - "host", host, "freeMemory", freeMemory.String(), - ) - delete(result.Activations, host) // host is already overloaded, remove from activations - continue - } - // Calculate how many instances can fit on this host, based on memory. - // Nova's memory_mb field is named "megabytes" in the API docs, but the values - // are in mebibytes (1 MiB = 2^20 bytes). Multiply by 1<<20 to convert to bytes. - //nolint:gosec // We're checking for underflows above (< 0). - memorySlots := uint64(freeMemory.Value()) / - (request.Flavor.MemoryMB * (1 << 20)) - if memorySlots < request.NumInstances { - traceLog.V(1).Info( - "filtering host due to insufficient RAM capacity", - "host", host, "requested_mb", request.Flavor.MemoryMB, - "available_mb", freeMemory.String(), + "filtering host due to insufficient capacity", + "host", host, + "requested_cpus", request.Flavor.VCPUs, + "requested_memory_mb", request.Flavor.MemoryMB, + "num_instances", request.NumInstances, + "available_cpus", freeCPU.String(), + "available_memory", freeMemory.String(), ) delete(result.Activations, host) continue diff --git a/internal/scheduling/reservations/accounting/accounting.go b/internal/scheduling/reservations/accounting/accounting.go deleted file mode 100644 index f896800e9..000000000 --- a/internal/scheduling/reservations/accounting/accounting.go +++ /dev/null @@ -1,171 +0,0 @@ -// Copyright SAP SE -// SPDX-License-Identifier: Apache-2.0 - -// Package accounting holds the reservation capacity-accounting math and flavor -// group lookup. Both are pure, self-contained helpers over Reservation/Knowledge -// CRDs with no dependency on the scheduling pipeline, so they can live below both -// the reservation controllers and the Nova scheduler tree without forming an -// import cycle. -package accounting - -import ( - hv1 "github.com/cobaltcore-dev/openstack-hypervisor-operator/api/v1" - "k8s.io/apimachinery/pkg/api/resource" - - "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 { - // TODO consider refactor with HostFreeCapacity - 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 -} - -// HostFreeCapacity computes the remaining free capacity on hv after subtracting -// hv.Status.Allocation and UnusedReservationCapacity for all reservations on this host. -// Negative values indicate over-subscription for that resource. -// Returns nil when the hypervisor has no capacity data. -// Reservations not targeting this host (via Spec.TargetHost or Status.Host) are ignored. -func HostFreeCapacity(hostReservations []v1alpha1.Reservation, hv hv1.Hypervisor) map[hv1.ResourceName]resource.Quantity { - // TODO consider refactor with HostHasCapacityForReservation - effCap := hv.Status.EffectiveCapacity - if effCap == nil { - effCap = hv.Status.Capacity - } - if effCap == nil { - return nil - } - - 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 hostReservations { - res := &hostReservations[i] - if res.Spec.TargetHost == "" { - continue // evicted; status.Host may lag until reconcile - } - if res.Spec.TargetHost != hv.Name && res.Status.Host != hv.Name { - continue - } - for rn, block := range UnusedReservationCapacity(res, false) { - if f, ok := free[rn]; ok { - f.Sub(block) - free[rn] = f - } - } - } - return free -} - -// 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. -// -// CommittedResourceReservations: confirmed VMs already appear in hv.Status.Allocation, -// so blocking the full slot would double-count them. The remaining block is -// max(slot − sum(confirmed VM resources), 0) per resource. Skipped when ignoreAllocations -// is true or when mid-migration (TargetHost != Status.Host). -// -// FailoverReservations: always block the full Spec.Resources. -// -// FailoverHostReservations: block nothing. FailoverHostReservation is a new feature -// that reserves an entire host, and is intentionally not yet counted as blocked capacity -// by any consumer of this function (capacity filter, capacity controller, oversubscription -// controller). Returning an empty block here keeps all of them consistent in one place. -func UnusedReservationCapacity(res *v1alpha1.Reservation, ignoreAllocations bool) map[hv1.ResourceName]resource.Quantity { - if res.Spec.Type == v1alpha1.ReservationTypeFailoverHost { - // TODO: When failover reservations are in use, account for them. - return nil - } - if res.Spec.Type == v1alpha1.ReservationTypeCommittedResource && - !ignoreAllocations && - res.Spec.TargetHost == res.Status.Host && - res.Status.CommittedResourceReservation != nil && - len(res.Status.CommittedResourceReservation.Allocation) > 0 { - // Compute remaining slot capacity: slot - sum of confirmed VM resources. - result := make(map[hv1.ResourceName]resource.Quantity, len(res.Spec.Resources)) - zero := resource.MustParse("0") - for rn, slotQty := range res.Spec.Resources { - remaining := slotQty.DeepCopy() - for _, alloc := range res.Status.CommittedResourceReservation.Allocation { - if used, ok := alloc.Resources[rn]; ok { - remaining.Sub(used) - } - } - if remaining.Cmp(zero) < 0 { - remaining = zero.DeepCopy() - } - result[rn] = remaining - } - return result - } - // No confirmed allocations, or ignoreAllocations, or mid-migration: block the full slot. - return res.Spec.Resources -} diff --git a/internal/scheduling/reservations/accounting/accounting_test.go b/internal/scheduling/reservations/accounting/accounting_test.go deleted file mode 100644 index 1d00239e0..000000000 --- a/internal/scheduling/reservations/accounting/accounting_test.go +++ /dev/null @@ -1,530 +0,0 @@ -// Copyright SAP SE -// SPDX-License-Identifier: Apache-2.0 - -package accounting - -import ( - "testing" - - hv1 "github.com/cobaltcore-dev/openstack-hypervisor-operator/api/v1" - corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/resource" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - - "github.com/cobaltcore-dev/cortex/api/v1alpha1" -) - -func TestUnusedReservationCapacity(t *testing.T) { - gib := func(n int64) resource.Quantity { return *resource.NewQuantity(n*1024*1024*1024, resource.BinarySI) } - memBytes := func(m map[hv1.ResourceName]resource.Quantity) int64 { - q, ok := m[hv1.ResourceMemory] - if !ok { - return 0 - } - return q.Value() - } - - tests := []struct { - name string - res *v1alpha1.Reservation - ignoreAllocations bool - wantMemoryBytes int64 - }{ - { - name: "failover: full slot blocked", - res: &v1alpha1.Reservation{ - Spec: v1alpha1.ReservationSpec{ - Type: v1alpha1.ReservationTypeFailover, - Resources: map[hv1.ResourceName]resource.Quantity{hv1.ResourceMemory: gib(480)}, - }, - }, - wantMemoryBytes: 480 * 1024 * 1024 * 1024, - }, - { - name: "failover host: nothing blocked (not yet counted as reserved)", - res: &v1alpha1.Reservation{ - Spec: v1alpha1.ReservationSpec{ - Type: v1alpha1.ReservationTypeFailoverHost, - Resources: map[hv1.ResourceName]resource.Quantity{hv1.ResourceMemory: gib(480)}, - }, - }, - wantMemoryBytes: 0, - }, - { - name: "CR no allocations: full slot blocked", - res: &v1alpha1.Reservation{ - Spec: v1alpha1.ReservationSpec{ - Type: v1alpha1.ReservationTypeCommittedResource, - Resources: map[hv1.ResourceName]resource.Quantity{hv1.ResourceMemory: gib(480)}, - CommittedResourceReservation: &v1alpha1.CommittedResourceReservationSpec{}, - }, - }, - wantMemoryBytes: 480 * 1024 * 1024 * 1024, - }, - { - name: "CR 1 confirmed VM in status.allocations, slot=480Gi: block = max(480Gi-480Gi, 0) = 0", - res: &v1alpha1.Reservation{ - Spec: v1alpha1.ReservationSpec{ - Type: v1alpha1.ReservationTypeCommittedResource, - Resources: map[hv1.ResourceName]resource.Quantity{hv1.ResourceMemory: gib(480)}, - CommittedResourceReservation: &v1alpha1.CommittedResourceReservationSpec{}, - }, - Status: v1alpha1.ReservationStatus{ - CommittedResourceReservation: &v1alpha1.CommittedResourceReservationStatus{ - Allocation: map[string]v1alpha1.CommittedResourceAllocation{ - "vm-1": { - ObjectRef: corev1.ObjectReference{Name: "vm-1"}, - Resources: map[hv1.ResourceName]resource.Quantity{hv1.ResourceMemory: gib(480)}, - }, - }, - }, - }, - }, - wantMemoryBytes: 0, - }, - { - name: "CR slot fully consumed by 2 confirmed VMs: block = max(480Gi-240Gi-240Gi, 0) = 0", - res: &v1alpha1.Reservation{ - Spec: v1alpha1.ReservationSpec{ - Type: v1alpha1.ReservationTypeCommittedResource, - Resources: map[hv1.ResourceName]resource.Quantity{hv1.ResourceMemory: gib(480)}, - CommittedResourceReservation: &v1alpha1.CommittedResourceReservationSpec{}, - }, - Status: v1alpha1.ReservationStatus{ - CommittedResourceReservation: &v1alpha1.CommittedResourceReservationStatus{ - Allocation: map[string]v1alpha1.CommittedResourceAllocation{ - "vm-1": { - ObjectRef: corev1.ObjectReference{Name: "vm-1"}, - Resources: map[hv1.ResourceName]resource.Quantity{hv1.ResourceMemory: gib(240)}, - }, - "vm-2": { - ObjectRef: corev1.ObjectReference{Name: "vm-2"}, - Resources: map[hv1.ResourceName]resource.Quantity{hv1.ResourceMemory: gib(240)}, - }, - }, - }, - }, - }, - wantMemoryBytes: 0, - }, - { - name: "CR slot with no confirmed VMs: full slot blocked (conservative — per-VM sizes not tracked yet)", - res: &v1alpha1.Reservation{ - Spec: v1alpha1.ReservationSpec{ - Type: v1alpha1.ReservationTypeCommittedResource, - Resources: map[hv1.ResourceName]resource.Quantity{hv1.ResourceMemory: gib(480)}, - CommittedResourceReservation: &v1alpha1.CommittedResourceReservationSpec{}, - }, - }, - wantMemoryBytes: 480 * 1024 * 1024 * 1024, - }, - { - name: "CR mid-migration (TargetHost != Status.Host): full slot blocked despite confirmed VMs", - res: &v1alpha1.Reservation{ - Spec: v1alpha1.ReservationSpec{ - Type: v1alpha1.ReservationTypeCommittedResource, - TargetHost: "new-host", - Resources: map[hv1.ResourceName]resource.Quantity{hv1.ResourceMemory: gib(480)}, - CommittedResourceReservation: &v1alpha1.CommittedResourceReservationSpec{}, - }, - Status: v1alpha1.ReservationStatus{ - Host: "old-host", // differs from TargetHost → migration in progress - CommittedResourceReservation: &v1alpha1.CommittedResourceReservationStatus{ - Allocation: map[string]v1alpha1.CommittedResourceAllocation{ - "vm-1": { - ObjectRef: corev1.ObjectReference{Name: "vm-1"}, - Resources: map[hv1.ResourceName]resource.Quantity{hv1.ResourceMemory: gib(480)}, - }, - }, - }, - }, - }, - wantMemoryBytes: 480 * 1024 * 1024 * 1024, - }, - { - name: "CR ignoreAllocations=true: full slot blocked regardless of confirmed VMs", - res: &v1alpha1.Reservation{ - Spec: v1alpha1.ReservationSpec{ - Type: v1alpha1.ReservationTypeCommittedResource, - Resources: map[hv1.ResourceName]resource.Quantity{hv1.ResourceMemory: gib(480)}, - CommittedResourceReservation: &v1alpha1.CommittedResourceReservationSpec{}, - }, - Status: v1alpha1.ReservationStatus{ - CommittedResourceReservation: &v1alpha1.CommittedResourceReservationStatus{ - Allocation: map[string]v1alpha1.CommittedResourceAllocation{ - "vm-1": { - ObjectRef: corev1.ObjectReference{Name: "vm-1"}, - Resources: map[hv1.ResourceName]resource.Quantity{hv1.ResourceMemory: gib(480)}, - }, - }, - }, - }, - }, - ignoreAllocations: true, - wantMemoryBytes: 480 * 1024 * 1024 * 1024, - }, - { - name: "no memory resource: block = 0", - res: &v1alpha1.Reservation{ - Spec: v1alpha1.ReservationSpec{ - Type: v1alpha1.ReservationTypeFailover, - Resources: map[hv1.ResourceName]resource.Quantity{}, - }, - }, - wantMemoryBytes: 0, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got := memBytes(UnusedReservationCapacity(tt.res, tt.ignoreAllocations)) - if got != tt.wantMemoryBytes { - t.Errorf("UnusedReservationCapacity() memory = %d, want %d", got, tt.wantMemoryBytes) - } - }) - } -} - -func TestHostFreeCapacity(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) } - - hvWithCap := 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), - }, - }, - } - } - crSlot := func(name, host 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: gib(memGiB), - hv1.ResourceCPU: cpu(cpuCores), - }, - }, - Status: v1alpha1.ReservationStatus{Host: host}, - } - } - freeMemGiB := func(free map[hv1.ResourceName]resource.Quantity) int64 { - q := free[hv1.ResourceMemory] - return q.Value() / (1024 * 1024 * 1024) - } - freeCPU := func(free map[hv1.ResourceName]resource.Quantity) int64 { - q := free[hv1.ResourceCPU] - return q.Value() - } - - t.Run("no capacity data returns nil", func(t *testing.T) { - hv := hv1.Hypervisor{ObjectMeta: metav1.ObjectMeta{Name: "host"}} - if got := HostFreeCapacity(nil, hv); got != nil { - t.Errorf("expected nil, got %v", got) - } - }) - - t.Run("no reservations and no allocation: free = effective capacity", func(t *testing.T) { - hv := hvWithCap("host", 1024, 256) - free := HostFreeCapacity(nil, hv) - if freeMemGiB(free) != 1024 { - t.Errorf("expected 1024 GiB free, got %d", freeMemGiB(free)) - } - if freeCPU(free) != 256 { - t.Errorf("expected 256 CPU free, got %d", freeCPU(free)) - } - }) - - t.Run("allocation subtracted from capacity", func(t *testing.T) { - hv := hvWithCap("host", 1024, 256) - hv.Status.Allocation = map[hv1.ResourceName]resource.Quantity{ - hv1.ResourceMemory: gib(512), - hv1.ResourceCPU: cpu(128), - } - free := HostFreeCapacity(nil, hv) - if freeMemGiB(free) != 512 { - t.Errorf("expected 512 GiB free, got %d", freeMemGiB(free)) - } - if freeCPU(free) != 128 { - t.Errorf("expected 128 CPU free, got %d", freeCPU(free)) - } - }) - - t.Run("reservation blocks subtracted", func(t *testing.T) { - hv := hvWithCap("host", 1024, 256) - slots := []v1alpha1.Reservation{ - crSlot("slot-1", "host", 512, 128), - } - free := HostFreeCapacity(slots, hv) - if freeMemGiB(free) != 512 { - t.Errorf("expected 512 GiB free, got %d", freeMemGiB(free)) - } - if freeCPU(free) != 128 { - t.Errorf("expected 128 CPU free, got %d", freeCPU(free)) - } - }) - - t.Run("over-subscribed: negative free values", func(t *testing.T) { - // 5 x 1TiB slots on a 4TiB host — the production scenario - hv := hvWithCap("host", 4096, 256) - slots := []v1alpha1.Reservation{ - crSlot("slot-0", "host", 1024, 128), - crSlot("slot-1", "host", 1024, 128), - crSlot("slot-2", "host", 1024, 128), - crSlot("slot-3", "host", 1024, 128), - crSlot("slot-4", "host", 1024, 128), - } - free := HostFreeCapacity(slots, hv) - if freeMemGiB(free) != -1024 { - t.Errorf("expected -1024 GiB (over-subscribed), got %d GiB", freeMemGiB(free)) - } - if freeCPU(free) != -384 { - t.Errorf("expected -384 CPU (over-subscribed), got %d", freeCPU(free)) - } - }) - - t.Run("allocation + reservations combined", func(t *testing.T) { - hv := hvWithCap("host", 1024, 256) - hv.Status.Allocation = map[hv1.ResourceName]resource.Quantity{ - hv1.ResourceMemory: gib(256), - hv1.ResourceCPU: cpu(64), - } - slots := []v1alpha1.Reservation{ - crSlot("slot-1", "host", 512, 128), - } - free := HostFreeCapacity(slots, hv) - // 1024 - 256 (alloc) - 512 (slot) = 256 GiB free - if freeMemGiB(free) != 256 { - t.Errorf("expected 256 GiB free, got %d", freeMemGiB(free)) - } - // 256 - 64 (alloc) - 128 (slot) = 64 free - if freeCPU(free) != 64 { - t.Errorf("expected 64 CPU free, got %d", freeCPU(free)) - } - }) - - t.Run("confirmed VM reduces slot block (not double counted)", func(t *testing.T) { - hv := hvWithCap("host", 1024, 256) - // 256 GiB confirmed VM already counted in Allocation - hv.Status.Allocation = map[hv1.ResourceName]resource.Quantity{ - hv1.ResourceMemory: gib(256), - } - // 512 GiB slot with 1 confirmed VM → block = 0 (VM in HV allocation; slot unblocked) - slot := v1alpha1.Reservation{ - ObjectMeta: metav1.ObjectMeta{Name: "slot-1"}, - Spec: v1alpha1.ReservationSpec{ - Type: v1alpha1.ReservationTypeCommittedResource, - TargetHost: "host", - Resources: map[hv1.ResourceName]resource.Quantity{hv1.ResourceMemory: gib(512)}, - CommittedResourceReservation: &v1alpha1.CommittedResourceReservationSpec{}, - }, - Status: v1alpha1.ReservationStatus{ - Host: "host", - CommittedResourceReservation: &v1alpha1.CommittedResourceReservationStatus{ - Allocation: map[string]v1alpha1.CommittedResourceAllocation{ - "vm-1": { - ObjectRef: corev1.ObjectReference{Name: "vm-1"}, - Resources: map[hv1.ResourceName]resource.Quantity{hv1.ResourceMemory: gib(512)}, - }, - }, - }, - }, - } - free := HostFreeCapacity([]v1alpha1.Reservation{slot}, hv) - // 1024 - 256 (hv.Allocation) - 0 (slot block = 0, VMs confirmed) = 768 - if freeMemGiB(free) != 768 { - t.Errorf("expected 768 GiB free, got %d", freeMemGiB(free)) - } - }) - - t.Run("reservations on other hosts are ignored even if passed in", func(t *testing.T) { - hv := hvWithCap("host", 1024, 256) - slots := []v1alpha1.Reservation{ - crSlot("slot-other", "other-host", 1024, 256), // different host — must not block - } - free := HostFreeCapacity(slots, hv) - if freeMemGiB(free) != 1024 { - t.Errorf("expected 1024 GiB free (other host ignored), got %d", freeMemGiB(free)) - } - }) - - t.Run("falls back to Capacity when EffectiveCapacity nil", func(t *testing.T) { - hv := hv1.Hypervisor{ - ObjectMeta: metav1.ObjectMeta{Name: "host"}, - Status: hv1.HypervisorStatus{ - Capacity: map[hv1.ResourceName]resource.Quantity{ - hv1.ResourceMemory: gib(512), - }, - }, - } - free := HostFreeCapacity(nil, hv) - if freeMemGiB(free) != 512 { - t.Errorf("expected 512 GiB, got %d", freeMemGiB(free)) - } - }) -} - -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/accounting/flavor_groups.go b/internal/scheduling/reservations/accounting/flavor_groups.go index 9d115592d..77a36d20c 100644 --- a/internal/scheduling/reservations/accounting/flavor_groups.go +++ b/internal/scheduling/reservations/accounting/flavor_groups.go @@ -1,6 +1,10 @@ // Copyright SAP SE // SPDX-License-Identifier: Apache-2.0 +// Package accounting holds flavor-group lookup over Knowledge CRDs, with no +// dependency on the scheduling pipeline, so it can live below both the reservation +// controllers and the Nova scheduler tree without forming an import cycle. The +// per-host capacity math lives in the sibling capacity package. package accounting import ( diff --git a/internal/scheduling/reservations/capacity/controller.go b/internal/scheduling/reservations/capacity/controller.go index 90c9c2b01..f18a2c220 100644 --- a/internal/scheduling/reservations/capacity/controller.go +++ b/internal/scheduling/reservations/capacity/controller.go @@ -29,6 +29,7 @@ import ( "github.com/cobaltcore-dev/cortex/api/scheduling" "github.com/cobaltcore-dev/cortex/api/v1alpha1" "github.com/cobaltcore-dev/cortex/internal/knowledge/extractor/plugins/compute" + caplib "github.com/cobaltcore-dev/cortex/internal/scheduling/capacity" sched "github.com/cobaltcore-dev/cortex/internal/scheduling/compute" "github.com/cobaltcore-dev/cortex/internal/scheduling/nova" "github.com/cobaltcore-dev/cortex/internal/scheduling/reservations" @@ -921,7 +922,7 @@ func (c *Reconciler) blockedResourcesByHost(ctx context.Context) (map[string]map continue } - resourcesToBlock := accounting.UnusedReservationCapacity(res, false) + resourcesToBlock := caplib.UnusedReservationCapacity(res, false) for host := range hostsToBlock { if blocked[host] == nil { blocked[host] = make(map[string]int64) @@ -992,7 +993,7 @@ func (c *Reconciler) reservedResourcesByGroupAZ( "reservation", res.Name, "type", res.Spec.Type, "az", az) continue } - blockedRes := accounting.UnusedReservationCapacity(res, false) + blockedRes := caplib.UnusedReservationCapacity(res, false) // Route each reservation to exactly one bucket by type; the two are disjoint. bucket := committed if res.Spec.Type == v1alpha1.ReservationTypeFailover { diff --git a/internal/scheduling/reservations/commitments/host_oversubscription_controller.go b/internal/scheduling/reservations/commitments/host_oversubscription_controller.go index b116465f7..6200a683e 100644 --- a/internal/scheduling/reservations/commitments/host_oversubscription_controller.go +++ b/internal/scheduling/reservations/commitments/host_oversubscription_controller.go @@ -21,8 +21,8 @@ import ( "sigs.k8s.io/controller-runtime/pkg/reconcile" "github.com/cobaltcore-dev/cortex/api/v1alpha1" + "github.com/cobaltcore-dev/cortex/internal/scheduling/capacity" "github.com/cobaltcore-dev/cortex/internal/scheduling/reservations" - "github.com/cobaltcore-dev/cortex/internal/scheduling/reservations/accounting" "github.com/cobaltcore-dev/cortex/pkg/multicluster" hv1 "github.com/cobaltcore-dev/openstack-hypervisor-operator/api/v1" ) @@ -268,13 +268,13 @@ func (r *HostOversubscriptionController) SetupWithManager(mgr ctrl.Manager, mcl // computeViolations returns the per-resource excess (positive quantity) for any resource where // the host is over-subscribed. Returns nil if capacity data is unavailable, empty map if no violation. func computeViolations(allReservations []v1alpha1.Reservation, hv hv1.Hypervisor) map[hv1.ResourceName]resource.Quantity { - free := accounting.HostFreeCapacity(allReservations, hv) - if free == nil { + b, err := capacity.DescribeHostUsage(hv, allReservations, capacity.Options{}) + if err != nil { return nil } zero := resource.MustParse("0") violations := make(map[hv1.ResourceName]resource.Quantity) - for rn, f := range free { + for rn, f := range b.Free { if f.Cmp(zero) < 0 { excess := f.DeepCopy() excess.Neg() @@ -334,7 +334,7 @@ func selectEvictionTarget( } entries := make([]allocatedEntry, len(allocated)) for i, res := range allocated { - unusedCap := accounting.UnusedReservationCapacity(res, false) + unusedCap := capacity.UnusedReservationCapacity(res, false) buckets := make(map[hv1.ResourceName]int64) for rn, total := range res.Spec.Resources { if t := total.Value(); t != 0 { @@ -382,7 +382,7 @@ func evictReservation( logger := LoggerFromContext(ctx).WithValues("component", "host-oversubscription", "reservation", res.Name) - freed := accounting.UnusedReservationCapacity(res, false) + freed := capacity.UnusedReservationCapacity(res, false) old := res.DeepCopy() res.Spec.TargetHost = "" diff --git a/internal/scheduling/reservations/commitments/integration_test.go b/internal/scheduling/reservations/commitments/integration_test.go index 9bc0ee94b..4f5dbc2e8 100644 --- a/internal/scheduling/reservations/commitments/integration_test.go +++ b/internal/scheduling/reservations/commitments/integration_test.go @@ -27,10 +27,10 @@ import ( schedulerdelegationapi "github.com/cobaltcore-dev/cortex/api/external/nova" "github.com/cobaltcore-dev/cortex/api/v1alpha1" + "github.com/cobaltcore-dev/cortex/internal/scheduling/capacity" sched "github.com/cobaltcore-dev/cortex/internal/scheduling/compute" "github.com/cobaltcore-dev/cortex/internal/scheduling/lib" "github.com/cobaltcore-dev/cortex/internal/scheduling/reservations" - "github.com/cobaltcore-dev/cortex/internal/scheduling/reservations/accounting" hv1 "github.com/cobaltcore-dev/openstack-hypervisor-operator/api/v1" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/meta" @@ -1388,9 +1388,12 @@ func TestCROversubscriptionRemediation(t *testing.T) { placedSlots = append(placedSlots, r) } } - free := accounting.HostFreeCapacity(placedSlots, *hv) + b, err := capacity.DescribeHostUsage(*hv, placedSlots, capacity.Options{}) + if err != nil { + t.Fatalf("unexpected error computing host capacity: %v", err) + } zero := resource.MustParse("0") - for rn, f := range free { + for rn, f := range b.Free { if f.Cmp(zero) < 0 { t.Errorf("host still over-subscribed for %s by %s after remediation", rn, f.String()) } From a2e815055b888982ef663a196a360464928a47ef Mon Sep 17 00:00:00 2001 From: Markus Wieland Date: Wed, 4 Nov 2026 16:54:12 +0100 Subject: [PATCH 2/3] refactor: move capacity logic to a dedicated library and update imports Signed-off-by: Markus Wieland --- .../kpis/plugins/infrastructure/kvm_host_capacity.go | 2 +- internal/scheduling/{ => lib}/capacity/capacity.go | 4 ++-- internal/scheduling/{ => lib}/capacity/capacity_test.go | 0 .../nova/plugins/filters/filter_has_enough_capacity.go | 2 +- internal/scheduling/reservations/capacity/controller.go | 2 +- .../commitments/host_oversubscription_controller.go | 2 +- .../scheduling/reservations/commitments/integration_test.go | 2 +- 7 files changed, 7 insertions(+), 7 deletions(-) rename internal/scheduling/{ => lib}/capacity/capacity.go (98%) rename internal/scheduling/{ => lib}/capacity/capacity_test.go (100%) diff --git a/internal/knowledge/kpis/plugins/infrastructure/kvm_host_capacity.go b/internal/knowledge/kpis/plugins/infrastructure/kvm_host_capacity.go index 983c89246..0d3e4dfe6 100644 --- a/internal/knowledge/kpis/plugins/infrastructure/kvm_host_capacity.go +++ b/internal/knowledge/kpis/plugins/infrastructure/kvm_host_capacity.go @@ -12,7 +12,7 @@ import ( "github.com/cobaltcore-dev/cortex/api/v1alpha1" "github.com/cobaltcore-dev/cortex/internal/knowledge/db" "github.com/cobaltcore-dev/cortex/internal/knowledge/kpis/plugins" - "github.com/cobaltcore-dev/cortex/internal/scheduling/capacity" + "github.com/cobaltcore-dev/cortex/internal/scheduling/lib/capacity" "github.com/cobaltcore-dev/cortex/pkg/conf" "github.com/prometheus/client_golang/prometheus" diff --git a/internal/scheduling/capacity/capacity.go b/internal/scheduling/lib/capacity/capacity.go similarity index 98% rename from internal/scheduling/capacity/capacity.go rename to internal/scheduling/lib/capacity/capacity.go index 0f1c1c21f..f89230f2a 100644 --- a/internal/scheduling/capacity/capacity.go +++ b/internal/scheduling/lib/capacity/capacity.go @@ -8,8 +8,8 @@ // scheduling tree and the knowledge KPI tree without forming an import cycle. // // Three consumers share this math: the Nova capacity filter -// (filter_has_enough_capacity), the reservation controllers (via the accounting -// package), and the KVM host-capacity KPI. +// (filter_has_enough_capacity), the reservation controllers, and the KVM +// host-capacity KPI. package capacity import ( diff --git a/internal/scheduling/capacity/capacity_test.go b/internal/scheduling/lib/capacity/capacity_test.go similarity index 100% rename from internal/scheduling/capacity/capacity_test.go rename to internal/scheduling/lib/capacity/capacity_test.go 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 de232cc5e..8994bcf7a 100644 --- a/internal/scheduling/nova/plugins/filters/filter_has_enough_capacity.go +++ b/internal/scheduling/nova/plugins/filters/filter_has_enough_capacity.go @@ -12,7 +12,7 @@ import ( api "github.com/cobaltcore-dev/cortex/api/external/nova" "github.com/cobaltcore-dev/cortex/api/v1alpha1" - "github.com/cobaltcore-dev/cortex/internal/scheduling/capacity" + "github.com/cobaltcore-dev/cortex/internal/scheduling/lib/capacity" "github.com/cobaltcore-dev/cortex/internal/scheduling/compute" "github.com/cobaltcore-dev/cortex/internal/scheduling/lib" hv1 "github.com/cobaltcore-dev/openstack-hypervisor-operator/api/v1" diff --git a/internal/scheduling/reservations/capacity/controller.go b/internal/scheduling/reservations/capacity/controller.go index f18a2c220..2a5db9daa 100644 --- a/internal/scheduling/reservations/capacity/controller.go +++ b/internal/scheduling/reservations/capacity/controller.go @@ -29,7 +29,7 @@ import ( "github.com/cobaltcore-dev/cortex/api/scheduling" "github.com/cobaltcore-dev/cortex/api/v1alpha1" "github.com/cobaltcore-dev/cortex/internal/knowledge/extractor/plugins/compute" - caplib "github.com/cobaltcore-dev/cortex/internal/scheduling/capacity" + caplib "github.com/cobaltcore-dev/cortex/internal/scheduling/lib/capacity" sched "github.com/cobaltcore-dev/cortex/internal/scheduling/compute" "github.com/cobaltcore-dev/cortex/internal/scheduling/nova" "github.com/cobaltcore-dev/cortex/internal/scheduling/reservations" diff --git a/internal/scheduling/reservations/commitments/host_oversubscription_controller.go b/internal/scheduling/reservations/commitments/host_oversubscription_controller.go index 6200a683e..be539e358 100644 --- a/internal/scheduling/reservations/commitments/host_oversubscription_controller.go +++ b/internal/scheduling/reservations/commitments/host_oversubscription_controller.go @@ -21,7 +21,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/reconcile" "github.com/cobaltcore-dev/cortex/api/v1alpha1" - "github.com/cobaltcore-dev/cortex/internal/scheduling/capacity" + "github.com/cobaltcore-dev/cortex/internal/scheduling/lib/capacity" "github.com/cobaltcore-dev/cortex/internal/scheduling/reservations" "github.com/cobaltcore-dev/cortex/pkg/multicluster" hv1 "github.com/cobaltcore-dev/openstack-hypervisor-operator/api/v1" diff --git a/internal/scheduling/reservations/commitments/integration_test.go b/internal/scheduling/reservations/commitments/integration_test.go index 4f5dbc2e8..53ab9005f 100644 --- a/internal/scheduling/reservations/commitments/integration_test.go +++ b/internal/scheduling/reservations/commitments/integration_test.go @@ -27,7 +27,7 @@ import ( schedulerdelegationapi "github.com/cobaltcore-dev/cortex/api/external/nova" "github.com/cobaltcore-dev/cortex/api/v1alpha1" - "github.com/cobaltcore-dev/cortex/internal/scheduling/capacity" + "github.com/cobaltcore-dev/cortex/internal/scheduling/lib/capacity" sched "github.com/cobaltcore-dev/cortex/internal/scheduling/compute" "github.com/cobaltcore-dev/cortex/internal/scheduling/lib" "github.com/cobaltcore-dev/cortex/internal/scheduling/reservations" From 75fdd2bb657e22d1737ad97b52ae1ace57f0c9db Mon Sep 17 00:00:00 2001 From: Markus Wieland Date: Wed, 4 Nov 2026 17:58:09 +0100 Subject: [PATCH 3/3] refactor: update import paths for capacity library across multiple files Signed-off-by: Markus Wieland --- .../nova/plugins/filters/filter_has_enough_capacity.go | 2 +- internal/scheduling/reservations/capacity/controller.go | 2 +- .../scheduling/reservations/commitments/integration_test.go | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) 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 8994bcf7a..50c5bd3bc 100644 --- a/internal/scheduling/nova/plugins/filters/filter_has_enough_capacity.go +++ b/internal/scheduling/nova/plugins/filters/filter_has_enough_capacity.go @@ -12,9 +12,9 @@ import ( api "github.com/cobaltcore-dev/cortex/api/external/nova" "github.com/cobaltcore-dev/cortex/api/v1alpha1" - "github.com/cobaltcore-dev/cortex/internal/scheduling/lib/capacity" "github.com/cobaltcore-dev/cortex/internal/scheduling/compute" "github.com/cobaltcore-dev/cortex/internal/scheduling/lib" + "github.com/cobaltcore-dev/cortex/internal/scheduling/lib/capacity" hv1 "github.com/cobaltcore-dev/openstack-hypervisor-operator/api/v1" "k8s.io/apimachinery/pkg/api/resource" ) diff --git a/internal/scheduling/reservations/capacity/controller.go b/internal/scheduling/reservations/capacity/controller.go index 2a5db9daa..ec3d58388 100644 --- a/internal/scheduling/reservations/capacity/controller.go +++ b/internal/scheduling/reservations/capacity/controller.go @@ -29,8 +29,8 @@ import ( "github.com/cobaltcore-dev/cortex/api/scheduling" "github.com/cobaltcore-dev/cortex/api/v1alpha1" "github.com/cobaltcore-dev/cortex/internal/knowledge/extractor/plugins/compute" - caplib "github.com/cobaltcore-dev/cortex/internal/scheduling/lib/capacity" sched "github.com/cobaltcore-dev/cortex/internal/scheduling/compute" + caplib "github.com/cobaltcore-dev/cortex/internal/scheduling/lib/capacity" "github.com/cobaltcore-dev/cortex/internal/scheduling/nova" "github.com/cobaltcore-dev/cortex/internal/scheduling/reservations" "github.com/cobaltcore-dev/cortex/internal/scheduling/reservations/accounting" diff --git a/internal/scheduling/reservations/commitments/integration_test.go b/internal/scheduling/reservations/commitments/integration_test.go index 53ab9005f..803ea45d1 100644 --- a/internal/scheduling/reservations/commitments/integration_test.go +++ b/internal/scheduling/reservations/commitments/integration_test.go @@ -27,9 +27,9 @@ import ( schedulerdelegationapi "github.com/cobaltcore-dev/cortex/api/external/nova" "github.com/cobaltcore-dev/cortex/api/v1alpha1" - "github.com/cobaltcore-dev/cortex/internal/scheduling/lib/capacity" sched "github.com/cobaltcore-dev/cortex/internal/scheduling/compute" "github.com/cobaltcore-dev/cortex/internal/scheduling/lib" + "github.com/cobaltcore-dev/cortex/internal/scheduling/lib/capacity" "github.com/cobaltcore-dev/cortex/internal/scheduling/reservations" hv1 "github.com/cobaltcore-dev/openstack-hypervisor-operator/api/v1" corev1 "k8s.io/api/core/v1"