From 16160cc8b2ff3e32151bf02ecd3c298d831a102f Mon Sep 17 00:00:00 2001 From: mblos Date: Tue, 12 May 2026 16:20:42 +0200 Subject: [PATCH 01/10] feat: update committed resource reservations allocations as part of the pipeline --- .../filter_weigher_pipeline_controller.go | 223 ++++++++++-- ...filter_weigher_pipeline_controller_test.go | 342 +++++++++++++++++- 2 files changed, 540 insertions(+), 25 deletions(-) diff --git a/internal/scheduling/nova/filter_weigher_pipeline_controller.go b/internal/scheduling/nova/filter_weigher_pipeline_controller.go index 279ac1c3e..046945120 100644 --- a/internal/scheduling/nova/filter_weigher_pipeline_controller.go +++ b/internal/scheduling/nova/filter_weigher_pipeline_controller.go @@ -14,11 +14,14 @@ import ( api "github.com/cobaltcore-dev/cortex/api/external/nova" "github.com/cobaltcore-dev/cortex/api/v1alpha1" "k8s.io/apimachinery/pkg/api/meta" + "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/util/retry" "github.com/cobaltcore-dev/cortex/internal/scheduling/lib" "github.com/cobaltcore-dev/cortex/internal/scheduling/nova/plugins/filters" "github.com/cobaltcore-dev/cortex/internal/scheduling/nova/plugins/weighers" + "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" ctrl "sigs.k8s.io/controller-runtime" @@ -62,7 +65,7 @@ func (c *FilterWeigherPipelineController) Reconcile(ctx context.Context, req ctr return ctrl.Result{}, client.IgnoreNotFound(err) } old := decision.DeepCopy() - if err := c.process(ctx, decision); err != nil { + if _, err := c.process(ctx, decision); err != nil { return ctrl.Result{}, err } patch := client.MergeFrom(old) @@ -74,14 +77,16 @@ func (c *FilterWeigherPipelineController) Reconcile(ctx context.Context, req ctr // Process the decision from the API. Should create and return the updated decision. func (c *FilterWeigherPipelineController) ProcessNewDecisionFromAPI(ctx context.Context, decision *v1alpha1.Decision) error { - c.processMu.Lock() - defer c.processMu.Unlock() - + // Early check before acquiring the mutex — no need to hold the lock just to fail. pipelineConf, ok := c.PipelineConfigs[decision.Spec.PipelineRef.Name] if !ok { return fmt.Errorf("pipeline %s not configured", decision.Spec.PipelineRef.Name) } - err := c.process(ctx, decision) + + c.processMu.Lock() + defer c.processMu.Unlock() + + request, err := c.process(ctx, decision) if err != nil { meta.SetStatusCondition(&decision.Status.Conditions, metav1.Condition{ Type: v1alpha1.DecisionConditionReady, @@ -98,25 +103,195 @@ func (c *FilterWeigherPipelineController) ProcessNewDecisionFromAPI(ctx context. }) } if pipelineConf.Spec.CreateHistory { - c.upsertHistory(ctx, decision, err) + c.upsertHistory(ctx, decision, request, err) + if err == nil && decision.Status.Result != nil && request != nil { + if decision.Status.Result.TargetHost != nil && isUserVMPlacement(decision.Spec.Intent) { + c.recordCRAllocation(ctx, decision, *request) + } + if decision.Status.Result.TargetHost == nil { + c.logNoHostFound(ctx, decision, *request) + } + } } return err } -func (c *FilterWeigherPipelineController) upsertHistory(ctx context.Context, decision *v1alpha1.Decision, pipelineErr error) { +// isUserVMPlacement returns true for intents that represent actual VM +// placements from Nova. Returns false for Cortex-internal synthetic requests +// (failover and CR reservation scheduling), which must not update allocations. +func isUserVMPlacement(intent v1alpha1.SchedulingIntent) bool { + switch intent { + case api.ReserveForCommittedResourceIntent, api.ReserveForFailoverIntent: + return false + default: + return true + } +} + +// recordCRAllocation writes the placed VM UUID into the matching Reservation +// Spec.CommittedResourceReservation.Allocations after a successful Nova placement. +// Best-effort: any failure is logged but never propagated to the caller. +func (c *FilterWeigherPipelineController) recordCRAllocation(ctx context.Context, decision *v1alpha1.Decision, request api.ExternalSchedulerRequest) { log := ctrl.LoggerFrom(ctx) - var az *string + instanceUUID := request.Spec.Data.InstanceUUID + projectID := request.Context.ProjectID + flavorName := request.Spec.Data.Flavor.Data.Name + selectedHost := *decision.Status.Result.TargetHost + + // Resolve flavor → flavor group. Flavors not in any group are PAYG — nothing to do. + fgClient := reservations.FlavorGroupKnowledgeClient{Client: c.Client} + flavorGroups, err := fgClient.GetAllFlavorGroups(ctx, nil) + if err != nil { + log.Error(err, "CR allocation: failed to get flavor groups", "instanceUUID", instanceUUID) + return + } + flavorGroupName, flavorInGroup, err := reservations.FindFlavorInGroups(flavorName, flavorGroups) + if err != nil { + log.V(1).Info("CR allocation: flavor not in any group, PAYG placement", "flavor", flavorName) + return + } + + // List all CR reservations and filter to candidates matching this placement. + var reservationList v1alpha1.ReservationList + if err := c.List(ctx, &reservationList, + client.MatchingLabels{v1alpha1.LabelReservationType: v1alpha1.ReservationTypeLabelCommittedResource}, + ); err != nil { + log.Error(err, "CR allocation: failed to list reservations", "instanceUUID", instanceUUID) + return + } - if decision.Spec.NovaRaw != nil { - var request api.ExternalSchedulerRequest - err := json.Unmarshal(decision.Spec.NovaRaw.Raw, &request) - if err != nil { - log.Error(err, "failed to unmarshal novaRaw for history, using defaults") - } else { - azStr := request.Spec.Data.AvailabilityZone - az = &azStr + var candidates []v1alpha1.Reservation + for _, res := range reservationList.Items { + cr := res.Spec.CommittedResourceReservation + if cr == nil { + continue + } + if res.Spec.TargetHost != selectedHost || cr.ProjectID != projectID || cr.ResourceGroup != flavorGroupName { + continue } + // Idempotency: if this VM UUID is already recorded, the work is done. + if _, exists := cr.Allocations[instanceUUID]; exists { + log.Info("CR allocation: VM UUID already in reservation, skipping", + "instanceUUID", instanceUUID, "reservation", res.Name) + return + } + candidates = append(candidates, res) + } + + if len(candidates) == 0 { + log.V(1).Info("CR allocation: no matching reservation slot, PAYG placement", + "instanceUUID", instanceUUID, "host", selectedHost, + "projectID", projectID, "flavorGroup", flavorGroupName) + return + } + + vmMemoryBytes := int64(flavorInGroup.MemoryMB) * 1024 * 1024 //nolint:gosec // flavor memory bounded by specs + vmCPUs := int64(flavorInGroup.VCPUs) //nolint:gosec // VCPUs bounded by specs + + slotName := pickReservationSlot(candidates, vmMemoryBytes) + if slotName == "" { + log.Error(nil, "CR allocation: no reservation slot has sufficient remaining capacity", + "instanceUUID", instanceUUID, "vmMemoryBytes", vmMemoryBytes, + "host", selectedHost, "candidates", len(candidates)) + return + } + + log.Info("CR allocation: writing VM UUID into reservation", + "instanceUUID", instanceUUID, "reservation", slotName, + "projectID", projectID, "flavorGroup", flavorGroupName, "host", selectedHost) + + vmResources := map[hv1.ResourceName]resource.Quantity{ + hv1.ResourceMemory: *resource.NewQuantity(vmMemoryBytes, resource.BinarySI), + hv1.ResourceCPU: *resource.NewQuantity(vmCPUs, resource.DecimalSI), + } + if retryErr := retry.RetryOnConflict(retry.DefaultRetry, func() error { + latest := &v1alpha1.Reservation{} + if err := c.Get(ctx, client.ObjectKey{Name: slotName}, latest); err != nil { + return err + } + if latest.Spec.CommittedResourceReservation.Allocations == nil { + latest.Spec.CommittedResourceReservation.Allocations = make(map[string]v1alpha1.CommittedResourceAllocation) + } + latest.Spec.CommittedResourceReservation.Allocations[instanceUUID] = v1alpha1.CommittedResourceAllocation{ + CreationTimestamp: metav1.Now(), + Resources: vmResources, + } + return c.Update(ctx, latest) + }); retryErr != nil { + log.Error(retryErr, "CR allocation: failed to patch reservation", + "reservation", slotName, "instanceUUID", instanceUUID) + return + } + + log.Info("CR allocation: done", "instanceUUID", instanceUUID, "reservation", slotName) +} + +// pickReservationSlot selects the reservation slot with the least remaining +// memory that can still fully fit vmMemoryBytes. +// Tiebreaks: least remaining CPU, then reservation name (lexicographic). +// Returns the slot name, or "" if no slot fits. +func pickReservationSlot(candidates []v1alpha1.Reservation, vmMemoryBytes int64) string { + bestName := "" + var bestRemMem, bestRemCPU int64 + + for _, res := range candidates { + cr := res.Spec.CommittedResourceReservation + + totalMemQ := res.Spec.Resources[hv1.ResourceMemory] + totalCPUQ := res.Spec.Resources[hv1.ResourceCPU] + totalMem := totalMemQ.Value() + totalCPU := totalCPUQ.Value() + + var usedMem, usedCPU int64 + for _, alloc := range cr.Allocations { + memQ := alloc.Resources[hv1.ResourceMemory] + cpuQ := alloc.Resources[hv1.ResourceCPU] + usedMem += memQ.Value() + usedCPU += cpuQ.Value() + } + + remMem := max(totalMem-usedMem, 0) + remCPU := max(totalCPU-usedCPU, 0) + + if remMem < vmMemoryBytes { + continue // Slot doesn't have enough remaining capacity. + } + + if bestName == "" || + remMem < bestRemMem || + (remMem == bestRemMem && remCPU < bestRemCPU) || + (remMem == bestRemMem && remCPU == bestRemCPU && res.Name < bestName) { + bestName = res.Name + bestRemMem = remMem + bestRemCPU = remCPU + } + } + + return bestName +} + +// logNoHostFound logs the context needed to classify no-host-found failures +// by CR coverage (cases A/B/C/D from ticket #345). +// TODO(#345): replace with CommittedResource CRD lookup and metric emission. +func (c *FilterWeigherPipelineController) logNoHostFound(ctx context.Context, decision *v1alpha1.Decision, request api.ExternalSchedulerRequest) { + log := ctrl.LoggerFrom(ctx) + log.Info("no-host-found for nova scheduling request", + "instanceUUID", request.Spec.Data.InstanceUUID, + "projectID", request.Context.ProjectID, + "flavor", request.Spec.Data.Flavor.Data.Name, + "intent", decision.Spec.Intent, + "pipeline", decision.Spec.PipelineRef.Name, + ) +} + +func (c *FilterWeigherPipelineController) upsertHistory(ctx context.Context, decision *v1alpha1.Decision, request *api.ExternalSchedulerRequest, pipelineErr error) { + log := ctrl.LoggerFrom(ctx) + + var az *string + if request != nil { + azStr := request.Spec.Data.AvailabilityZone + az = &azStr } if upsertErr := c.HistoryManager.CreateOrUpdateHistory(ctx, decision, az, pipelineErr); upsertErr != nil { @@ -124,23 +299,23 @@ func (c *FilterWeigherPipelineController) upsertHistory(ctx context.Context, dec } } -func (c *FilterWeigherPipelineController) process(ctx context.Context, decision *v1alpha1.Decision) error { +func (c *FilterWeigherPipelineController) process(ctx context.Context, decision *v1alpha1.Decision) (*api.ExternalSchedulerRequest, error) { log := ctrl.LoggerFrom(ctx) startedAt := time.Now() // So we can measure sync duration. pipeline, ok := c.Pipelines[decision.Spec.PipelineRef.Name] if !ok { log.Error(nil, "pipeline not found or not ready", "pipelineName", decision.Spec.PipelineRef.Name) - return errors.New("pipeline not found or not ready") + return nil, errors.New("pipeline not found or not ready") } if decision.Spec.NovaRaw == nil { log.Error(nil, "skipping decision, no novaRaw spec defined") - return errors.New("no novaRaw spec defined") + return nil, errors.New("no novaRaw spec defined") } var request api.ExternalSchedulerRequest if err := json.Unmarshal(decision.Spec.NovaRaw.Raw, &request); err != nil { log.Error(err, "failed to unmarshal novaRaw spec") - return err + return nil, err } if intent, err := request.GetIntent(); err != nil { @@ -155,13 +330,13 @@ func (c *FilterWeigherPipelineController) process(ctx context.Context, decision pipelineConf, ok := c.PipelineConfigs[decision.Spec.PipelineRef.Name] if !ok { log.Error(nil, "pipeline config not found", "pipelineName", decision.Spec.PipelineRef.Name) - return errors.New("pipeline config not found") + return nil, errors.New("pipeline config not found") } if pipelineConf.Spec.IgnorePreselection { log.Info("gathering all placement candidates before filtering") if err := c.gatherer.MutateWithAllCandidates(ctx, &request); err != nil { log.Error(err, "failed to gather all placement candidates") - return err + return nil, err } log.Info("gathered all placement candidates", "numHosts", len(request.Hosts)) } @@ -169,7 +344,7 @@ func (c *FilterWeigherPipelineController) process(ctx context.Context, decision result, err := pipeline.Run(request) if err != nil { log.Error(err, "failed to run pipeline") - return err + return nil, err } decision.Status.Result = &result meta.SetStatusCondition(&decision.Status.Conditions, metav1.Condition{ @@ -179,7 +354,7 @@ func (c *FilterWeigherPipelineController) process(ctx context.Context, decision Message: "pipeline run succeeded", }) log.Info("decision processed successfully", "duration", time.Since(startedAt)) - return nil + return &request, nil } // The base controller will delegate the pipeline creation down to this method. diff --git a/internal/scheduling/nova/filter_weigher_pipeline_controller_test.go b/internal/scheduling/nova/filter_weigher_pipeline_controller_test.go index 752725df8..a597992d4 100644 --- a/internal/scheduling/nova/filter_weigher_pipeline_controller_test.go +++ b/internal/scheduling/nova/filter_weigher_pipeline_controller_test.go @@ -12,6 +12,7 @@ import ( "time" corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" @@ -21,8 +22,9 @@ import ( api "github.com/cobaltcore-dev/cortex/api/external/nova" "github.com/cobaltcore-dev/cortex/api/v1alpha1" - + "github.com/cobaltcore-dev/cortex/internal/knowledge/extractor/plugins/compute" "github.com/cobaltcore-dev/cortex/internal/scheduling/lib" + hv1 "github.com/cobaltcore-dev/openstack-hypervisor-operator/api/v1" ) // mockCandidateGatherer implements CandidateGatherer for testing @@ -926,5 +928,343 @@ func TestFilterWeigherPipelineController_IgnorePreselection(t *testing.T) { } } +func TestIsUserVMPlacement(t *testing.T) { + tests := []struct { + intent v1alpha1.SchedulingIntent + expected bool + }{ + {api.CreateIntent, true}, + {api.LiveMigrationIntent, true}, + {api.EvacuateIntent, true}, + {api.RebuildIntent, true}, + {api.ResizeIntent, true}, + {api.ReserveForCommittedResourceIntent, false}, + {api.ReserveForFailoverIntent, false}, + {v1alpha1.SchedulingIntentUnknown, true}, + } + for _, tt := range tests { + if got := isUserVMPlacement(tt.intent); got != tt.expected { + t.Errorf("isUserVMPlacement(%q) = %v, want %v", tt.intent, got, tt.expected) + } + } +} + +func TestPickReservationSlot(t *testing.T) { + // vmMemBytes for a 4096 MiB flavor. + const vmMemBytes = int64(4096) * 1024 * 1024 + + makeSlot := func(name string, totalMemMiB, totalCPU, usedMemMiB, usedCPU int64) v1alpha1.Reservation { + var allocs map[string]v1alpha1.CommittedResourceAllocation + if usedMemMiB > 0 || usedCPU > 0 { + allocs = map[string]v1alpha1.CommittedResourceAllocation{ + "vm-existing": { + Resources: map[hv1.ResourceName]resource.Quantity{ + hv1.ResourceMemory: *resource.NewQuantity(usedMemMiB*1024*1024, resource.BinarySI), + hv1.ResourceCPU: *resource.NewQuantity(usedCPU, resource.DecimalSI), + }, + }, + } + } + return v1alpha1.Reservation{ + ObjectMeta: metav1.ObjectMeta{Name: name}, + Spec: v1alpha1.ReservationSpec{ + Resources: map[hv1.ResourceName]resource.Quantity{ + hv1.ResourceMemory: *resource.NewQuantity(totalMemMiB*1024*1024, resource.BinarySI), + hv1.ResourceCPU: *resource.NewQuantity(totalCPU, resource.DecimalSI), + }, + CommittedResourceReservation: &v1alpha1.CommittedResourceReservationSpec{ + Allocations: allocs, + }, + }, + } + } + + tests := []struct { + name string + candidates []v1alpha1.Reservation + want string + }{ + { + name: "no candidates", + candidates: nil, + want: "", + }, + { + name: "single slot fits", + candidates: []v1alpha1.Reservation{makeSlot("a", 8192, 8, 0, 0)}, + want: "a", + }, + { + name: "single slot too small after allocations", + candidates: []v1alpha1.Reservation{makeSlot("a", 8192, 8, 8192, 8)}, // fully consumed + want: "", + }, + { + name: "picks slot with least remaining memory", + candidates: []v1alpha1.Reservation{ + makeSlot("large", 8192, 8, 0, 0), // 8192 MiB remaining + makeSlot("small", 6144, 8, 0, 0), // 6144 MiB remaining, still fits + }, + want: "small", + }, + { + name: "CPU tiebreak on equal remaining memory", + candidates: []v1alpha1.Reservation{ + makeSlot("more-cpu", 6144, 8, 0, 0), // remCPU = 8 + makeSlot("less-cpu", 6144, 4, 0, 0), // remCPU = 4 + }, + want: "less-cpu", + }, + { + name: "name tiebreak on equal remaining memory and CPU", + candidates: []v1alpha1.Reservation{ + makeSlot("slot-b", 6144, 4, 0, 0), + makeSlot("slot-a", 6144, 4, 0, 0), + }, + want: "slot-a", + }, + { + name: "missing resource keys treated as zero remaining", + candidates: []v1alpha1.Reservation{ + { + ObjectMeta: metav1.ObjectMeta{Name: "empty-res"}, + Spec: v1alpha1.ReservationSpec{ + CommittedResourceReservation: &v1alpha1.CommittedResourceReservationSpec{}, + }, + }, + }, + want: "", + }, + { + name: "partially used slot still fits", + candidates: []v1alpha1.Reservation{makeSlot("partial", 8192, 8, 2048, 2)}, // 6144 MiB remaining + want: "partial", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := pickReservationSlot(tt.candidates, vmMemBytes) + if got != tt.want { + t.Errorf("pickReservationSlot() = %q, want %q", got, tt.want) + } + }) + } +} + +func TestRecordCRAllocation(t *testing.T) { + scheme := runtime.NewScheme() + if err := v1alpha1.AddToScheme(scheme); err != nil { + t.Fatalf("AddToScheme: %v", err) + } + + const ( + instanceUUID = "vm-uuid-1" + projectID = "project-1" + flavorName = "m1.large" + flavorGroup = "m1" + selectedHost = "compute-1" + ) + + ratio := uint64(2048) + fg := compute.FlavorGroupFeature{ + Name: flavorGroup, + Flavors: []compute.FlavorInGroup{{Name: flavorName, VCPUs: 2, MemoryMB: 4096}}, + LargestFlavor: compute.FlavorInGroup{Name: flavorName, VCPUs: 2, MemoryMB: 4096}, + SmallestFlavor: compute.FlavorInGroup{Name: flavorName, VCPUs: 2, MemoryMB: 4096}, + RamCoreRatio: &ratio, + } + + flavorKnowledge := func() *v1alpha1.Knowledge { + raw, err := v1alpha1.BoxFeatureList([]compute.FlavorGroupFeature{fg}) + if err != nil { + t.Fatalf("BoxFeatureList: %v", err) + } + return &v1alpha1.Knowledge{ + ObjectMeta: metav1.ObjectMeta{Name: "flavor-groups"}, + Status: v1alpha1.KnowledgeStatus{ + Raw: raw, + Conditions: []metav1.Condition{{ + Type: v1alpha1.KnowledgeConditionReady, + Status: metav1.ConditionTrue, + Reason: "Ready", + LastTransitionTime: metav1.Now(), + }}, + }, + } + } + + makeReservation := func(name string, memMiB, cpus int64, proj, group, host string, allocs map[string]v1alpha1.CommittedResourceAllocation) *v1alpha1.Reservation { + return &v1alpha1.Reservation{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Labels: map[string]string{ + v1alpha1.LabelReservationType: v1alpha1.ReservationTypeLabelCommittedResource, + }, + }, + Spec: v1alpha1.ReservationSpec{ + Type: v1alpha1.ReservationTypeCommittedResource, + TargetHost: host, + Resources: map[hv1.ResourceName]resource.Quantity{ + hv1.ResourceMemory: *resource.NewQuantity(memMiB*1024*1024, resource.BinarySI), + hv1.ResourceCPU: *resource.NewQuantity(cpus, resource.DecimalSI), + }, + CommittedResourceReservation: &v1alpha1.CommittedResourceReservationSpec{ + ProjectID: proj, + ResourceGroup: group, + Allocations: allocs, + }, + }, + } + } + + makeRequest := func(uuid, proj, flavor string) api.ExternalSchedulerRequest { + return api.ExternalSchedulerRequest{ + Spec: api.NovaObject[api.NovaSpec]{ + Data: api.NovaSpec{ + InstanceUUID: uuid, + Flavor: api.NovaObject[api.NovaFlavor]{ + Data: api.NovaFlavor{Name: flavor, MemoryMB: 4096, VCPUs: 2}, + }, + }, + }, + Context: api.NovaRequestContext{ProjectID: proj}, + } + } + + makeDecision := func(host string) *v1alpha1.Decision { + h := host + return &v1alpha1.Decision{ + Status: v1alpha1.DecisionStatus{ + Result: &v1alpha1.DecisionResult{TargetHost: &h}, + }, + } + } + + vmAlloc := func() map[string]v1alpha1.CommittedResourceAllocation { + return map[string]v1alpha1.CommittedResourceAllocation{ + instanceUUID: { + CreationTimestamp: metav1.Now(), + Resources: map[hv1.ResourceName]resource.Quantity{ + hv1.ResourceMemory: *resource.NewQuantity(int64(4096)*1024*1024, resource.BinarySI), + hv1.ResourceCPU: *resource.NewQuantity(2, resource.DecimalSI), + }, + }, + } + } + + tests := []struct { + name string + objects []client.Object + request api.ExternalSchedulerRequest + decision *v1alpha1.Decision + checkSlot string + expectAllocation bool + }{ + { + name: "writes allocation into matching reservation", + objects: []client.Object{ + flavorKnowledge(), + makeReservation("slot-1", 8192, 8, projectID, flavorGroup, selectedHost, nil), + }, + request: makeRequest(instanceUUID, projectID, flavorName), + decision: makeDecision(selectedHost), + checkSlot: "slot-1", + expectAllocation: true, + }, + { + name: "idempotent: UUID already in allocations", + objects: []client.Object{ + flavorKnowledge(), + makeReservation("slot-1", 8192, 8, projectID, flavorGroup, selectedHost, vmAlloc()), + }, + request: makeRequest(instanceUUID, projectID, flavorName), + decision: makeDecision(selectedHost), + checkSlot: "slot-1", + expectAllocation: true, + }, + { + name: "PAYG: flavor not in any group", + objects: []client.Object{ + flavorKnowledge(), + makeReservation("slot-1", 8192, 8, projectID, flavorGroup, selectedHost, nil), + }, + request: makeRequest(instanceUUID, projectID, "unknown-flavor"), + decision: makeDecision(selectedHost), + checkSlot: "slot-1", + expectAllocation: false, + }, + { + name: "no matching reservation: host mismatch", + objects: []client.Object{ + flavorKnowledge(), + makeReservation("slot-1", 8192, 8, projectID, flavorGroup, "other-host", nil), + }, + request: makeRequest(instanceUUID, projectID, flavorName), + decision: makeDecision(selectedHost), + checkSlot: "slot-1", + expectAllocation: false, + }, + { + name: "no slot fits: all capacity used", + objects: []client.Object{ + flavorKnowledge(), + makeReservation("slot-full", 4096, 2, projectID, flavorGroup, selectedHost, + map[string]v1alpha1.CommittedResourceAllocation{ + "other-vm": { + Resources: map[hv1.ResourceName]resource.Quantity{ + hv1.ResourceMemory: *resource.NewQuantity(int64(4096)*1024*1024, resource.BinarySI), + hv1.ResourceCPU: *resource.NewQuantity(2, resource.DecimalSI), + }, + }, + }), + }, + request: makeRequest(instanceUUID, projectID, flavorName), + decision: makeDecision(selectedHost), + checkSlot: "slot-full", + expectAllocation: false, + }, + { + name: "no knowledge CRD: logs error, no allocation", + objects: []client.Object{ + makeReservation("slot-1", 8192, 8, projectID, flavorGroup, selectedHost, nil), + }, + request: makeRequest(instanceUUID, projectID, flavorName), + decision: makeDecision(selectedHost), + checkSlot: "slot-1", + expectAllocation: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(tt.objects...). + Build() + + controller := &FilterWeigherPipelineController{ + BasePipelineController: lib.BasePipelineController[lib.FilterWeigherPipeline[api.ExternalSchedulerRequest]]{ + Client: fakeClient, + }, + } + + controller.recordCRAllocation(context.Background(), tt.decision, tt.request) + + var res v1alpha1.Reservation + if err := fakeClient.Get(context.Background(), client.ObjectKey{Name: tt.checkSlot}, &res); err != nil { + t.Fatalf("Get reservation %q: %v", tt.checkSlot, err) + } + _, hasAlloc := res.Spec.CommittedResourceReservation.Allocations[instanceUUID] + if tt.expectAllocation && !hasAlloc { + t.Errorf("expected allocation for UUID %q but none found", instanceUUID) + } + if !tt.expectAllocation && hasAlloc { + t.Errorf("expected no allocation for UUID %q but one was written", instanceUUID) + } + }) + } +} + // Error variable for testing var errGathererFailed = errors.New("gatherer failed") From ed5d788175f2e03c4ce8e7e9af9a497662fded51 Mon Sep 17 00:00:00 2001 From: mblos Date: Tue, 12 May 2026 17:12:56 +0200 Subject: [PATCH 02/10] feat: write metrics for reservation effectiveness for no-host-found failures --- cmd/manager/main.go | 4 +- internal/scheduling/nova/cr_allocation.go | 187 +++++++++ internal/scheduling/nova/cr_metrics.go | 120 ++++++ internal/scheduling/nova/cr_metrics_test.go | 392 ++++++++++++++++++ .../filter_weigher_pipeline_controller.go | 164 +------- 5 files changed, 706 insertions(+), 161 deletions(-) create mode 100644 internal/scheduling/nova/cr_allocation.go create mode 100644 internal/scheduling/nova/cr_metrics.go create mode 100644 internal/scheduling/nova/cr_metrics_test.go diff --git a/cmd/manager/main.go b/cmd/manager/main.go index c5d5fa323..be41d31f3 100644 --- a/cmd/manager/main.go +++ b/cmd/manager/main.go @@ -397,8 +397,10 @@ func main() { if slices.Contains(mainConfig.EnabledControllers, "nova-pipeline-controllers") { // Filter-weigher pipeline controller setup. filterWeigherController := &nova.FilterWeigherPipelineController{ - Monitor: filterWeigherPipelineMonitor, + Monitor: filterWeigherPipelineMonitor, + NoHostFoundCounter: nova.NewNoHostFoundCounter(), } + metrics.Registry.MustRegister(filterWeigherController.NoHostFoundCounter) // Inferred through the base controller. filterWeigherController.Client = multiclusterClient if err := filterWeigherController.SetupWithManager(mgr, multiclusterClient); err != nil { diff --git a/internal/scheduling/nova/cr_allocation.go b/internal/scheduling/nova/cr_allocation.go new file mode 100644 index 000000000..2c7e60241 --- /dev/null +++ b/internal/scheduling/nova/cr_allocation.go @@ -0,0 +1,187 @@ +// Copyright SAP SE +// SPDX-License-Identifier: Apache-2.0 + +package nova + +import ( + "context" + "fmt" + + api "github.com/cobaltcore-dev/cortex/api/external/nova" + "github.com/cobaltcore-dev/cortex/api/v1alpha1" + "k8s.io/apimachinery/pkg/api/resource" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/util/retry" + + "github.com/cobaltcore-dev/cortex/internal/knowledge/extractor/plugins/compute" + "github.com/cobaltcore-dev/cortex/internal/scheduling/reservations" + hv1 "github.com/cobaltcore-dev/openstack-hypervisor-operator/api/v1" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +// recordCRAllocation writes the placed VM UUID into the matching Reservation +// Spec.CommittedResourceReservation.Allocations after a successful Nova placement. +// Best-effort: any failure is logged but never propagated to the caller. +func (c *FilterWeigherPipelineController) recordCRAllocation(ctx context.Context, decision *v1alpha1.Decision, request api.ExternalSchedulerRequest) { + log := ctrl.LoggerFrom(ctx) + + instanceUUID := request.Spec.Data.InstanceUUID + projectID := request.Context.ProjectID + flavorName := request.Spec.Data.Flavor.Data.Name + selectedHost := *decision.Status.Result.TargetHost + + flavorGroupName, flavorInGroup, err := c.resolveFlavorGroup(ctx, flavorName) + if err != nil { + log.V(1).Info("CR allocation: flavor not in any group, PAYG placement", "flavor", flavorName) + return + } + + // List all CR reservations and filter to candidates matching this placement. + var reservationList v1alpha1.ReservationList + if err := c.List(ctx, &reservationList, + client.MatchingLabels{v1alpha1.LabelReservationType: v1alpha1.ReservationTypeLabelCommittedResource}, + ); err != nil { + log.Error(err, "CR allocation: failed to list reservations", "instanceUUID", instanceUUID) + return + } + + var candidates []v1alpha1.Reservation + for _, res := range reservationList.Items { + cr := res.Spec.CommittedResourceReservation + if cr == nil { + continue + } + if res.Spec.TargetHost != selectedHost || cr.ProjectID != projectID || cr.ResourceGroup != flavorGroupName { + continue + } + // Idempotency: if this VM UUID is already recorded, the work is done. + if _, exists := cr.Allocations[instanceUUID]; exists { + log.Info("CR allocation: VM UUID already in reservation, skipping", + "instanceUUID", instanceUUID, "reservation", res.Name) + return + } + candidates = append(candidates, res) + } + + if len(candidates) == 0 { + log.V(1).Info("CR allocation: no matching reservation slot, PAYG placement", + "instanceUUID", instanceUUID, "host", selectedHost, + "projectID", projectID, "flavorGroup", flavorGroupName) + return + } + + vmMemoryBytes := int64(flavorInGroup.MemoryMB) * 1024 * 1024 //nolint:gosec // flavor memory bounded by specs + vmCPUs := int64(flavorInGroup.VCPUs) //nolint:gosec // VCPUs bounded by specs + + slotName := pickReservationSlot(candidates, vmMemoryBytes) + if slotName == "" { + log.Error(nil, "CR allocation: no reservation slot has sufficient remaining capacity", + "instanceUUID", instanceUUID, "vmMemoryBytes", vmMemoryBytes, + "host", selectedHost, "candidates", len(candidates)) + return + } + + log.Info("CR allocation: writing VM UUID into reservation", + "instanceUUID", instanceUUID, "reservation", slotName, + "projectID", projectID, "flavorGroup", flavorGroupName, "host", selectedHost) + + vmResources := map[hv1.ResourceName]resource.Quantity{ + hv1.ResourceMemory: *resource.NewQuantity(vmMemoryBytes, resource.BinarySI), + hv1.ResourceCPU: *resource.NewQuantity(vmCPUs, resource.DecimalSI), + } + if retryErr := retry.RetryOnConflict(retry.DefaultRetry, func() error { + latest := &v1alpha1.Reservation{} + if err := c.Get(ctx, client.ObjectKey{Name: slotName}, latest); err != nil { + return err + } + if latest.Spec.CommittedResourceReservation == nil { + return fmt.Errorf("reservation %s lost CommittedResourceReservation spec", slotName) + } + if latest.Spec.CommittedResourceReservation.Allocations == nil { + latest.Spec.CommittedResourceReservation.Allocations = make(map[string]v1alpha1.CommittedResourceAllocation) + } + latest.Spec.CommittedResourceReservation.Allocations[instanceUUID] = v1alpha1.CommittedResourceAllocation{ + CreationTimestamp: metav1.Now(), + Resources: vmResources, + } + return c.Update(ctx, latest) + }); retryErr != nil { + log.Error(retryErr, "CR allocation: failed to patch reservation", + "reservation", slotName, "instanceUUID", instanceUUID) + return + } + + log.Info("CR allocation: done", "instanceUUID", instanceUUID, "reservation", slotName) +} + +// pickReservationSlot selects the reservation slot with the least remaining +// memory that can still fully fit vmMemoryBytes. +// Tiebreaks: least remaining CPU, then reservation name (lexicographic). +// Returns the slot name, or "" if no slot fits. +func pickReservationSlot(candidates []v1alpha1.Reservation, vmMemoryBytes int64) string { + bestName := "" + var bestRemMem, bestRemCPU int64 + + for _, res := range candidates { + cr := res.Spec.CommittedResourceReservation + + totalCPUQ := res.Spec.Resources[hv1.ResourceCPU] + totalCPU := totalCPUQ.Value() + + var usedCPU int64 + for _, alloc := range cr.Allocations { + cpuQ := alloc.Resources[hv1.ResourceCPU] + usedCPU += cpuQ.Value() + } + + remMem := reservationRemainingMemory(res) + remCPU := max(totalCPU-usedCPU, 0) + + if remMem < vmMemoryBytes { + continue // Slot doesn't have enough remaining capacity. + } + + if bestName == "" || + remMem < bestRemMem || + (remMem == bestRemMem && remCPU < bestRemCPU) || + (remMem == bestRemMem && remCPU == bestRemCPU && res.Name < bestName) { + bestName = res.Name + bestRemMem = remMem + bestRemCPU = remCPU + } + } + + return bestName +} + +// reservationRemainingMemory returns how many bytes of memory remain +// unallocated in a reservation slot. Returns 0 if the slot is full or nil. +func reservationRemainingMemory(res v1alpha1.Reservation) int64 { + cr := res.Spec.CommittedResourceReservation + if cr == nil { + return 0 + } + totalMemQ := res.Spec.Resources[hv1.ResourceMemory] + var usedMem int64 + for _, alloc := range cr.Allocations { + allocMemQ := alloc.Resources[hv1.ResourceMemory] + usedMem += allocMemQ.Value() + } + return max(totalMemQ.Value()-usedMem, 0) +} + +// resolveFlavorGroup looks up which flavor group the given flavor belongs to. +// Returns an error if the flavor is not in any group (PAYG). +func (c *FilterWeigherPipelineController) resolveFlavorGroup(ctx context.Context, flavorName string) (string, *compute.FlavorInGroup, error) { + fgClient := reservations.FlavorGroupKnowledgeClient{Client: c.Client} + flavorGroups, err := fgClient.GetAllFlavorGroups(ctx, nil) + if err != nil { + return "", nil, err + } + groupName, flavor, err := reservations.FindFlavorInGroups(flavorName, flavorGroups) + if err != nil { + return "", nil, err + } + return groupName, flavor, nil +} diff --git a/internal/scheduling/nova/cr_metrics.go b/internal/scheduling/nova/cr_metrics.go new file mode 100644 index 000000000..e3afb1029 --- /dev/null +++ b/internal/scheduling/nova/cr_metrics.go @@ -0,0 +1,120 @@ +// Copyright SAP SE +// SPDX-License-Identifier: Apache-2.0 + +package nova + +import ( + "context" + + api "github.com/cobaltcore-dev/cortex/api/external/nova" + "github.com/cobaltcore-dev/cortex/api/v1alpha1" + "github.com/prometheus/client_golang/prometheus" + "k8s.io/apimachinery/pkg/api/resource" + + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +// NewNoHostFoundCounter creates the Prometheus counter for no-host-found classification. +// Register it with the metrics registry before assigning it to the controller. +func NewNoHostFoundCounter() *prometheus.CounterVec { + return prometheus.NewCounterVec(prometheus.CounterOpts{ + Name: "cortex_nova_no_host_found_total", + Help: "Nova no-host-found results classified by committed resource coverage (cases A/B/C/D).", + }, []string{"case", "flavor_group", "intent"}) +} + +// classifyNoHostFound determines why no host was found for a nova placement request, +// in terms of committed resource coverage: +// +// - D: project has no active CommittedResources for the flavor group +// - A: CommittedResources exist but are fully occupied (used >= capacity) +// - B: CommittedResources have remaining capacity but no free Reservation slot +// - C: free Reservation slots exist but placement constraints excluded all candidates +func classifyNoHostFound( + activeCRs []v1alpha1.CommittedResource, + reservations []v1alpha1.Reservation, + projectID, flavorGroupName string, +) string { + + if len(activeCRs) == 0 { + return "D" + } + + totalCapacity := resource.Quantity{} + totalUsed := resource.Quantity{} + for _, cr := range activeCRs { + totalCapacity.Add(cr.Spec.Amount) + if used, ok := cr.Status.UsedResources["memory"]; ok { + totalUsed.Add(used) + } + } + if totalUsed.Cmp(totalCapacity) >= 0 { + return "A" + } + + for _, res := range reservations { + cr := res.Spec.CommittedResourceReservation + if cr == nil || cr.ProjectID != projectID || cr.ResourceGroup != flavorGroupName { + continue + } + if reservationRemainingMemory(res) > 0 { + return "C" + } + } + return "B" +} + +// logNoHostFound classifies a no-host-found result and emits a log line and metric. +func (c *FilterWeigherPipelineController) logNoHostFound(ctx context.Context, decision *v1alpha1.Decision, request api.ExternalSchedulerRequest) { + log := ctrl.LoggerFrom(ctx) + + projectID := request.Context.ProjectID + flavorName := request.Spec.Data.Flavor.Data.Name + instanceUUID := request.Spec.Data.InstanceUUID + intent := decision.Spec.Intent + + flavorGroupName, _, err := c.resolveFlavorGroup(ctx, flavorName) + if err != nil { + log.V(1).Info("no-host-found: PAYG flavor, not CR-relevant", + "instanceUUID", instanceUUID, "flavor", flavorName, "intent", intent) + return + } + + var crList v1alpha1.CommittedResourceList + if err := c.List(ctx, &crList); err != nil { + log.Error(err, "no-host-found: failed to list committed resources", "instanceUUID", instanceUUID) + return + } + var activeCRs []v1alpha1.CommittedResource + for _, cr := range crList.Items { + if cr.Spec.ProjectID != projectID || cr.Spec.FlavorGroupName != flavorGroupName { + continue + } + if cr.Spec.State != v1alpha1.CommitmentStatusConfirmed && cr.Spec.State != v1alpha1.CommitmentStatusGuaranteed { + continue + } + activeCRs = append(activeCRs, cr) + } + + var reservationList v1alpha1.ReservationList + if err := c.List(ctx, &reservationList, + client.MatchingLabels{v1alpha1.LabelReservationType: v1alpha1.ReservationTypeLabelCommittedResource}, + ); err != nil { + log.Error(err, "no-host-found: failed to list reservations", "instanceUUID", instanceUUID) + return + } + + noHostFoundCase := classifyNoHostFound(activeCRs, reservationList.Items, projectID, flavorGroupName) + + log.Info("no-host-found classified", + "case", noHostFoundCase, + "instanceUUID", instanceUUID, + "projectID", projectID, + "flavorGroup", flavorGroupName, + "intent", intent, + ) + if c.NoHostFoundCounter != nil { + c.NoHostFoundCounter.WithLabelValues(noHostFoundCase, flavorGroupName, string(intent)).Inc() + } +} diff --git a/internal/scheduling/nova/cr_metrics_test.go b/internal/scheduling/nova/cr_metrics_test.go new file mode 100644 index 000000000..6df9018b3 --- /dev/null +++ b/internal/scheduling/nova/cr_metrics_test.go @@ -0,0 +1,392 @@ +// Copyright SAP SE +// SPDX-License-Identifier: Apache-2.0 + +package nova + +import ( + "context" + "testing" + + "github.com/prometheus/client_golang/prometheus" + "github.com/prometheus/client_golang/prometheus/testutil" + "k8s.io/apimachinery/pkg/api/resource" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + api "github.com/cobaltcore-dev/cortex/api/external/nova" + "github.com/cobaltcore-dev/cortex/api/v1alpha1" + "github.com/cobaltcore-dev/cortex/internal/knowledge/extractor/plugins/compute" + "github.com/cobaltcore-dev/cortex/internal/scheduling/lib" + hv1 "github.com/cobaltcore-dev/openstack-hypervisor-operator/api/v1" +) + +// makeCR builds a CommittedResource for testing. +func makeCR(state v1alpha1.CommitmentStatus, amountMiB, usedMiB int64) v1alpha1.CommittedResource { + cr := v1alpha1.CommittedResource{ + Spec: v1alpha1.CommittedResourceSpec{ + State: state, + Amount: *resource.NewQuantity(amountMiB*1024*1024, resource.BinarySI), + }, + } + if usedMiB > 0 { + cr.Status.UsedResources = map[string]resource.Quantity{ + "memory": *resource.NewQuantity(usedMiB*1024*1024, resource.BinarySI), + } + } + return cr +} + +// makeSlot builds a Reservation slot for testing. +func makeSlot(projectID, flavorGroup string, totalMemMiB, allocatedMemMiB int64) v1alpha1.Reservation { + res := v1alpha1.Reservation{ + ObjectMeta: metav1.ObjectMeta{Name: "slot-" + flavorGroup}, + Spec: v1alpha1.ReservationSpec{ + Resources: map[hv1.ResourceName]resource.Quantity{ + hv1.ResourceMemory: *resource.NewQuantity(totalMemMiB*1024*1024, resource.BinarySI), + }, + CommittedResourceReservation: &v1alpha1.CommittedResourceReservationSpec{ + ProjectID: projectID, + ResourceGroup: flavorGroup, + }, + }, + } + if allocatedMemMiB > 0 { + res.Spec.CommittedResourceReservation.Allocations = map[string]v1alpha1.CommittedResourceAllocation{ + "some-vm": { + Resources: map[hv1.ResourceName]resource.Quantity{ + hv1.ResourceMemory: *resource.NewQuantity(allocatedMemMiB*1024*1024, resource.BinarySI), + }, + }, + } + } + return res +} + +func TestClassifyNoHostFound(t *testing.T) { + const ( + proj = "project-1" + group = "kvm_v2_hana_s" + ) + + tests := []struct { + name string + activeCRs []v1alpha1.CommittedResource + reservations []v1alpha1.Reservation + expectedCase string + }{ + { + name: "D: no active CRs for project+flavor group", + activeCRs: nil, + reservations: nil, + expectedCase: "D", + }, + { + name: "A: CRs fully occupied (used == capacity)", + activeCRs: []v1alpha1.CommittedResource{ + makeCR(v1alpha1.CommitmentStatusConfirmed, 8192, 8192), + }, + reservations: nil, + expectedCase: "A", + }, + { + name: "A: CRs fully occupied (used > capacity)", + activeCRs: []v1alpha1.CommittedResource{ + makeCR(v1alpha1.CommitmentStatusConfirmed, 8192, 10000), + }, + reservations: nil, + expectedCase: "A", + }, + { + name: "A: multiple CRs, total used >= total capacity", + activeCRs: []v1alpha1.CommittedResource{ + makeCR(v1alpha1.CommitmentStatusConfirmed, 4096, 4096), + makeCR(v1alpha1.CommitmentStatusGuaranteed, 4096, 4096), + }, + reservations: nil, + expectedCase: "A", + }, + { + name: "B: CRs have capacity but no free reservation slot", + activeCRs: []v1alpha1.CommittedResource{ + makeCR(v1alpha1.CommitmentStatusConfirmed, 8192, 4096), + }, + reservations: []v1alpha1.Reservation{ + makeSlot(proj, group, 8192, 8192), // slot fully allocated + }, + expectedCase: "B", + }, + { + name: "B: CRs have capacity, no slots at all", + activeCRs: []v1alpha1.CommittedResource{ + makeCR(v1alpha1.CommitmentStatusConfirmed, 8192, 0), + }, + reservations: nil, + expectedCase: "B", + }, + { + name: "C: free slot exists", + activeCRs: []v1alpha1.CommittedResource{ + makeCR(v1alpha1.CommitmentStatusConfirmed, 8192, 4096), + }, + reservations: []v1alpha1.Reservation{ + makeSlot(proj, group, 8192, 4096), // slot has 4096 MiB free + }, + expectedCase: "C", + }, + { + name: "C: one slot full, one slot free", + activeCRs: []v1alpha1.CommittedResource{ + makeCR(v1alpha1.CommitmentStatusConfirmed, 16384, 4096), + }, + reservations: []v1alpha1.Reservation{ + makeSlot(proj, group, 8192, 8192), // full + makeSlot(proj, group, 8192, 0), // free + }, + expectedCase: "C", + }, + { + name: "slots for other project are ignored", + activeCRs: []v1alpha1.CommittedResource{ + makeCR(v1alpha1.CommitmentStatusConfirmed, 8192, 0), + }, + reservations: []v1alpha1.Reservation{ + makeSlot("other-project", group, 8192, 0), // different project + }, + expectedCase: "B", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := classifyNoHostFound(tt.activeCRs, tt.reservations, proj, group) + if got != tt.expectedCase { + t.Errorf("classifyNoHostFound() = %q, want %q", got, tt.expectedCase) + } + }) + } +} + +func TestReservationRemainingMemory(t *testing.T) { + tests := []struct { + name string + totalMemMiB int64 + usedMemMiB int64 + wantBytes int64 + }{ + {"empty slot", 8192, 0, 8192 * 1024 * 1024}, + {"partially used", 8192, 4096, 4096 * 1024 * 1024}, + {"fully used", 8192, 8192, 0}, + {"over-allocated (clamped to zero)", 4096, 8192, 0}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + res := makeSlot("proj", "group", tt.totalMemMiB, tt.usedMemMiB) + got := reservationRemainingMemory(res) + if got != tt.wantBytes { + t.Errorf("reservationRemainingMemory() = %d, want %d", got, tt.wantBytes) + } + }) + } +} + +func TestLogNoHostFound(t *testing.T) { + scheme := runtime.NewScheme() + if err := v1alpha1.AddToScheme(scheme); err != nil { + t.Fatalf("AddToScheme: %v", err) + } + + const ( + projectID = "project-1" + flavorName = "m1.large" + flavorGroup = "m1" + instanceID = "vm-uuid-1" + ) + + ratio := uint64(2048) + fg := compute.FlavorGroupFeature{ + Name: flavorGroup, + Flavors: []compute.FlavorInGroup{{Name: flavorName, VCPUs: 2, MemoryMB: 4096}}, + LargestFlavor: compute.FlavorInGroup{Name: flavorName, VCPUs: 2, MemoryMB: 4096}, + SmallestFlavor: compute.FlavorInGroup{Name: flavorName, VCPUs: 2, MemoryMB: 4096}, + RamCoreRatio: &ratio, + } + + flavorKnowledge := func() *v1alpha1.Knowledge { + raw, err := v1alpha1.BoxFeatureList([]compute.FlavorGroupFeature{fg}) + if err != nil { + t.Fatalf("BoxFeatureList: %v", err) + } + return &v1alpha1.Knowledge{ + ObjectMeta: metav1.ObjectMeta{Name: "flavor-groups"}, + Status: v1alpha1.KnowledgeStatus{ + Raw: raw, + Conditions: []metav1.Condition{{ + Type: v1alpha1.KnowledgeConditionReady, + Status: metav1.ConditionTrue, + Reason: "Ready", + LastTransitionTime: metav1.Now(), + }}, + }, + } + } + + makeCRObject := func(state v1alpha1.CommitmentStatus, amountMiB, usedMiB int64) *v1alpha1.CommittedResource { + cr := &v1alpha1.CommittedResource{ + ObjectMeta: metav1.ObjectMeta{Name: "cr-1"}, + Spec: v1alpha1.CommittedResourceSpec{ + ProjectID: projectID, + FlavorGroupName: flavorGroup, + State: state, + Amount: *resource.NewQuantity(amountMiB*1024*1024, resource.BinarySI), + }, + } + if usedMiB > 0 { + cr.Status.UsedResources = map[string]resource.Quantity{ + "memory": *resource.NewQuantity(usedMiB*1024*1024, resource.BinarySI), + } + } + return cr + } + + makeReservationSlot := func(name string, totalMemMiB, allocatedMemMiB int64) *v1alpha1.Reservation { + res := &v1alpha1.Reservation{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Labels: map[string]string{ + v1alpha1.LabelReservationType: v1alpha1.ReservationTypeLabelCommittedResource, + }, + }, + Spec: v1alpha1.ReservationSpec{ + Resources: map[hv1.ResourceName]resource.Quantity{ + hv1.ResourceMemory: *resource.NewQuantity(totalMemMiB*1024*1024, resource.BinarySI), + }, + CommittedResourceReservation: &v1alpha1.CommittedResourceReservationSpec{ + ProjectID: projectID, + ResourceGroup: flavorGroup, + }, + }, + } + if allocatedMemMiB > 0 { + res.Spec.CommittedResourceReservation.Allocations = map[string]v1alpha1.CommittedResourceAllocation{ + "some-vm": { + Resources: map[hv1.ResourceName]resource.Quantity{ + hv1.ResourceMemory: *resource.NewQuantity(allocatedMemMiB*1024*1024, resource.BinarySI), + }, + }, + } + } + return res + } + + tests := []struct { + name string + objects []client.Object + payg bool + expectedCase string // "" means no counter increment expected + }{ + { + name: "D: no active CRs", + objects: []client.Object{flavorKnowledge()}, + expectedCase: "D", + }, + { + name: "A: CRs fully occupied", + objects: []client.Object{ + flavorKnowledge(), + makeCRObject(v1alpha1.CommitmentStatusConfirmed, 8192, 8192), + }, + expectedCase: "A", + }, + { + name: "B: capacity exists but no free slot", + objects: []client.Object{ + flavorKnowledge(), + makeCRObject(v1alpha1.CommitmentStatusConfirmed, 8192, 4096), + makeReservationSlot("slot-full", 8192, 8192), + }, + expectedCase: "B", + }, + { + name: "C: free slot exists", + objects: []client.Object{ + flavorKnowledge(), + makeCRObject(v1alpha1.CommitmentStatusConfirmed, 8192, 4096), + makeReservationSlot("slot-free", 8192, 0), + }, + expectedCase: "C", + }, + { + name: "PAYG: flavor not in any group", + objects: []client.Object{flavorKnowledge()}, + payg: true, + expectedCase: "", + }, + { + name: "no knowledge CRD: no counter increment", + objects: []client.Object{ + makeCRObject(v1alpha1.CommitmentStatusConfirmed, 8192, 0), + }, + expectedCase: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(tt.objects...). + Build() + + counter := NewNoHostFoundCounter() + reg := prometheus.NewRegistry() + reg.MustRegister(counter) + + controller := &FilterWeigherPipelineController{ + BasePipelineController: lib.BasePipelineController[lib.FilterWeigherPipeline[api.ExternalSchedulerRequest]]{ + Client: fakeClient, + }, + NoHostFoundCounter: counter, + } + + requestFlavorName := flavorName + if tt.payg { + requestFlavorName = "unknown-flavor" + } + request := api.ExternalSchedulerRequest{ + Spec: api.NovaObject[api.NovaSpec]{ + Data: api.NovaSpec{ + InstanceUUID: instanceID, + Flavor: api.NovaObject[api.NovaFlavor]{ + Data: api.NovaFlavor{Name: requestFlavorName}, + }, + }, + }, + Context: api.NovaRequestContext{ProjectID: projectID}, + } + decision := &v1alpha1.Decision{ + Spec: v1alpha1.DecisionSpec{Intent: api.CreateIntent}, + } + + controller.logNoHostFound(context.Background(), decision, request) + + if tt.expectedCase == "" { + total := testutil.ToFloat64(counter.WithLabelValues("A", flavorGroup, string(api.CreateIntent))) + + testutil.ToFloat64(counter.WithLabelValues("B", flavorGroup, string(api.CreateIntent))) + + testutil.ToFloat64(counter.WithLabelValues("C", flavorGroup, string(api.CreateIntent))) + + testutil.ToFloat64(counter.WithLabelValues("D", flavorGroup, string(api.CreateIntent))) + if total != 0 { + t.Errorf("expected no counter increment, got total %.0f", total) + } + } else { + got := testutil.ToFloat64(counter.WithLabelValues(tt.expectedCase, flavorGroup, string(api.CreateIntent))) + if got != 1 { + t.Errorf("counter[case=%q, flavorGroup=%q, intent=%q] = %.0f, want 1", + tt.expectedCase, flavorGroup, string(api.CreateIntent), got) + } + } + }) + } +} diff --git a/internal/scheduling/nova/filter_weigher_pipeline_controller.go b/internal/scheduling/nova/filter_weigher_pipeline_controller.go index 046945120..e2a21d50e 100644 --- a/internal/scheduling/nova/filter_weigher_pipeline_controller.go +++ b/internal/scheduling/nova/filter_weigher_pipeline_controller.go @@ -13,15 +13,13 @@ import ( api "github.com/cobaltcore-dev/cortex/api/external/nova" "github.com/cobaltcore-dev/cortex/api/v1alpha1" + "github.com/prometheus/client_golang/prometheus" "k8s.io/apimachinery/pkg/api/meta" - "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/client-go/util/retry" "github.com/cobaltcore-dev/cortex/internal/scheduling/lib" "github.com/cobaltcore-dev/cortex/internal/scheduling/nova/plugins/filters" "github.com/cobaltcore-dev/cortex/internal/scheduling/nova/plugins/weighers" - "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" ctrl "sigs.k8s.io/controller-runtime" @@ -48,6 +46,9 @@ type FilterWeigherPipelineController struct { Monitor lib.FilterWeigherPipelineMonitor // Candidate gatherer to get all placement candidates if needed. gatherer CandidateGatherer + + // NoHostFoundCounter counts no-host-found results by CR coverage case (A/B/C/D). + NoHostFoundCounter *prometheus.CounterVec } // The type of pipeline this controller manages. @@ -128,163 +129,6 @@ func isUserVMPlacement(intent v1alpha1.SchedulingIntent) bool { } } -// recordCRAllocation writes the placed VM UUID into the matching Reservation -// Spec.CommittedResourceReservation.Allocations after a successful Nova placement. -// Best-effort: any failure is logged but never propagated to the caller. -func (c *FilterWeigherPipelineController) recordCRAllocation(ctx context.Context, decision *v1alpha1.Decision, request api.ExternalSchedulerRequest) { - log := ctrl.LoggerFrom(ctx) - - instanceUUID := request.Spec.Data.InstanceUUID - projectID := request.Context.ProjectID - flavorName := request.Spec.Data.Flavor.Data.Name - selectedHost := *decision.Status.Result.TargetHost - - // Resolve flavor → flavor group. Flavors not in any group are PAYG — nothing to do. - fgClient := reservations.FlavorGroupKnowledgeClient{Client: c.Client} - flavorGroups, err := fgClient.GetAllFlavorGroups(ctx, nil) - if err != nil { - log.Error(err, "CR allocation: failed to get flavor groups", "instanceUUID", instanceUUID) - return - } - flavorGroupName, flavorInGroup, err := reservations.FindFlavorInGroups(flavorName, flavorGroups) - if err != nil { - log.V(1).Info("CR allocation: flavor not in any group, PAYG placement", "flavor", flavorName) - return - } - - // List all CR reservations and filter to candidates matching this placement. - var reservationList v1alpha1.ReservationList - if err := c.List(ctx, &reservationList, - client.MatchingLabels{v1alpha1.LabelReservationType: v1alpha1.ReservationTypeLabelCommittedResource}, - ); err != nil { - log.Error(err, "CR allocation: failed to list reservations", "instanceUUID", instanceUUID) - return - } - - var candidates []v1alpha1.Reservation - for _, res := range reservationList.Items { - cr := res.Spec.CommittedResourceReservation - if cr == nil { - continue - } - if res.Spec.TargetHost != selectedHost || cr.ProjectID != projectID || cr.ResourceGroup != flavorGroupName { - continue - } - // Idempotency: if this VM UUID is already recorded, the work is done. - if _, exists := cr.Allocations[instanceUUID]; exists { - log.Info("CR allocation: VM UUID already in reservation, skipping", - "instanceUUID", instanceUUID, "reservation", res.Name) - return - } - candidates = append(candidates, res) - } - - if len(candidates) == 0 { - log.V(1).Info("CR allocation: no matching reservation slot, PAYG placement", - "instanceUUID", instanceUUID, "host", selectedHost, - "projectID", projectID, "flavorGroup", flavorGroupName) - return - } - - vmMemoryBytes := int64(flavorInGroup.MemoryMB) * 1024 * 1024 //nolint:gosec // flavor memory bounded by specs - vmCPUs := int64(flavorInGroup.VCPUs) //nolint:gosec // VCPUs bounded by specs - - slotName := pickReservationSlot(candidates, vmMemoryBytes) - if slotName == "" { - log.Error(nil, "CR allocation: no reservation slot has sufficient remaining capacity", - "instanceUUID", instanceUUID, "vmMemoryBytes", vmMemoryBytes, - "host", selectedHost, "candidates", len(candidates)) - return - } - - log.Info("CR allocation: writing VM UUID into reservation", - "instanceUUID", instanceUUID, "reservation", slotName, - "projectID", projectID, "flavorGroup", flavorGroupName, "host", selectedHost) - - vmResources := map[hv1.ResourceName]resource.Quantity{ - hv1.ResourceMemory: *resource.NewQuantity(vmMemoryBytes, resource.BinarySI), - hv1.ResourceCPU: *resource.NewQuantity(vmCPUs, resource.DecimalSI), - } - if retryErr := retry.RetryOnConflict(retry.DefaultRetry, func() error { - latest := &v1alpha1.Reservation{} - if err := c.Get(ctx, client.ObjectKey{Name: slotName}, latest); err != nil { - return err - } - if latest.Spec.CommittedResourceReservation.Allocations == nil { - latest.Spec.CommittedResourceReservation.Allocations = make(map[string]v1alpha1.CommittedResourceAllocation) - } - latest.Spec.CommittedResourceReservation.Allocations[instanceUUID] = v1alpha1.CommittedResourceAllocation{ - CreationTimestamp: metav1.Now(), - Resources: vmResources, - } - return c.Update(ctx, latest) - }); retryErr != nil { - log.Error(retryErr, "CR allocation: failed to patch reservation", - "reservation", slotName, "instanceUUID", instanceUUID) - return - } - - log.Info("CR allocation: done", "instanceUUID", instanceUUID, "reservation", slotName) -} - -// pickReservationSlot selects the reservation slot with the least remaining -// memory that can still fully fit vmMemoryBytes. -// Tiebreaks: least remaining CPU, then reservation name (lexicographic). -// Returns the slot name, or "" if no slot fits. -func pickReservationSlot(candidates []v1alpha1.Reservation, vmMemoryBytes int64) string { - bestName := "" - var bestRemMem, bestRemCPU int64 - - for _, res := range candidates { - cr := res.Spec.CommittedResourceReservation - - totalMemQ := res.Spec.Resources[hv1.ResourceMemory] - totalCPUQ := res.Spec.Resources[hv1.ResourceCPU] - totalMem := totalMemQ.Value() - totalCPU := totalCPUQ.Value() - - var usedMem, usedCPU int64 - for _, alloc := range cr.Allocations { - memQ := alloc.Resources[hv1.ResourceMemory] - cpuQ := alloc.Resources[hv1.ResourceCPU] - usedMem += memQ.Value() - usedCPU += cpuQ.Value() - } - - remMem := max(totalMem-usedMem, 0) - remCPU := max(totalCPU-usedCPU, 0) - - if remMem < vmMemoryBytes { - continue // Slot doesn't have enough remaining capacity. - } - - if bestName == "" || - remMem < bestRemMem || - (remMem == bestRemMem && remCPU < bestRemCPU) || - (remMem == bestRemMem && remCPU == bestRemCPU && res.Name < bestName) { - bestName = res.Name - bestRemMem = remMem - bestRemCPU = remCPU - } - } - - return bestName -} - -// logNoHostFound logs the context needed to classify no-host-found failures -// by CR coverage (cases A/B/C/D from ticket #345). -// TODO(#345): replace with CommittedResource CRD lookup and metric emission. -func (c *FilterWeigherPipelineController) logNoHostFound(ctx context.Context, decision *v1alpha1.Decision, request api.ExternalSchedulerRequest) { - log := ctrl.LoggerFrom(ctx) - log.Info("no-host-found for nova scheduling request", - "instanceUUID", request.Spec.Data.InstanceUUID, - "projectID", request.Context.ProjectID, - "flavor", request.Spec.Data.Flavor.Data.Name, - "intent", decision.Spec.Intent, - "pipeline", decision.Spec.PipelineRef.Name, - ) -} - func (c *FilterWeigherPipelineController) upsertHistory(ctx context.Context, decision *v1alpha1.Decision, request *api.ExternalSchedulerRequest, pipelineErr error) { log := ctrl.LoggerFrom(ctx) From 9b7d443fc8a12b9068f480fe3c14bad18884aff9 Mon Sep 17 00:00:00 2001 From: mblos Date: Wed, 13 May 2026 07:58:15 +0200 Subject: [PATCH 03/10] fixes --- internal/scheduling/nova/cr_allocation.go | 26 ++++++++++++++----- internal/scheduling/nova/cr_metrics.go | 10 +++++-- .../filter_weigher_pipeline_controller.go | 7 +++-- ...filter_weigher_pipeline_controller_test.go | 22 +++++++++++++--- 4 files changed, 49 insertions(+), 16 deletions(-) diff --git a/internal/scheduling/nova/cr_allocation.go b/internal/scheduling/nova/cr_allocation.go index 2c7e60241..5a29ef1e2 100644 --- a/internal/scheduling/nova/cr_allocation.go +++ b/internal/scheduling/nova/cr_allocation.go @@ -5,6 +5,7 @@ package nova import ( "context" + "errors" "fmt" api "github.com/cobaltcore-dev/cortex/api/external/nova" @@ -33,7 +34,12 @@ func (c *FilterWeigherPipelineController) recordCRAllocation(ctx context.Context flavorGroupName, flavorInGroup, err := c.resolveFlavorGroup(ctx, flavorName) if err != nil { - log.V(1).Info("CR allocation: flavor not in any group, PAYG placement", "flavor", flavorName) + if errors.Is(err, errFlavorNotInGroup) { + log.V(1).Info("CR allocation: flavor not in any group, PAYG placement", "flavor", flavorName) + } else { + log.Error(err, "CR allocation: failed to resolve flavor group", + "flavor", flavorName, "instanceUUID", instanceUUID) + } return } @@ -74,7 +80,7 @@ func (c *FilterWeigherPipelineController) recordCRAllocation(ctx context.Context vmMemoryBytes := int64(flavorInGroup.MemoryMB) * 1024 * 1024 //nolint:gosec // flavor memory bounded by specs vmCPUs := int64(flavorInGroup.VCPUs) //nolint:gosec // VCPUs bounded by specs - slotName := pickReservationSlot(candidates, vmMemoryBytes) + slotName := pickReservationSlot(candidates, vmMemoryBytes, vmCPUs) if slotName == "" { log.Error(nil, "CR allocation: no reservation slot has sufficient remaining capacity", "instanceUUID", instanceUUID, "vmMemoryBytes", vmMemoryBytes, @@ -116,10 +122,10 @@ func (c *FilterWeigherPipelineController) recordCRAllocation(ctx context.Context } // pickReservationSlot selects the reservation slot with the least remaining -// memory that can still fully fit vmMemoryBytes. +// memory that can still fully fit vmMemoryBytes and vmCPUs. // Tiebreaks: least remaining CPU, then reservation name (lexicographic). // Returns the slot name, or "" if no slot fits. -func pickReservationSlot(candidates []v1alpha1.Reservation, vmMemoryBytes int64) string { +func pickReservationSlot(candidates []v1alpha1.Reservation, vmMemoryBytes, vmCPUs int64) string { bestName := "" var bestRemMem, bestRemCPU int64 @@ -138,7 +144,7 @@ func pickReservationSlot(candidates []v1alpha1.Reservation, vmMemoryBytes int64) remMem := reservationRemainingMemory(res) remCPU := max(totalCPU-usedCPU, 0) - if remMem < vmMemoryBytes { + if remMem < vmMemoryBytes || remCPU < vmCPUs { continue // Slot doesn't have enough remaining capacity. } @@ -171,8 +177,14 @@ func reservationRemainingMemory(res v1alpha1.Reservation) int64 { return max(totalMemQ.Value()-usedMem, 0) } +// errFlavorNotInGroup is returned by resolveFlavorGroup when the flavor is not +// part of any configured flavor group (PAYG placement). Callers should +// distinguish this from real lookup errors. +var errFlavorNotInGroup = errors.New("flavor not in any group") + // resolveFlavorGroup looks up which flavor group the given flavor belongs to. -// Returns an error if the flavor is not in any group (PAYG). +// Returns errFlavorNotInGroup (PAYG) if the flavor is not in any group. +// Returns a different error for transient failures (Knowledge CRD unavailable, etc). func (c *FilterWeigherPipelineController) resolveFlavorGroup(ctx context.Context, flavorName string) (string, *compute.FlavorInGroup, error) { fgClient := reservations.FlavorGroupKnowledgeClient{Client: c.Client} flavorGroups, err := fgClient.GetAllFlavorGroups(ctx, nil) @@ -181,7 +193,7 @@ func (c *FilterWeigherPipelineController) resolveFlavorGroup(ctx context.Context } groupName, flavor, err := reservations.FindFlavorInGroups(flavorName, flavorGroups) if err != nil { - return "", nil, err + return "", nil, errFlavorNotInGroup } return groupName, flavor, nil } diff --git a/internal/scheduling/nova/cr_metrics.go b/internal/scheduling/nova/cr_metrics.go index e3afb1029..ec99a3be3 100644 --- a/internal/scheduling/nova/cr_metrics.go +++ b/internal/scheduling/nova/cr_metrics.go @@ -5,6 +5,7 @@ package nova import ( "context" + "errors" api "github.com/cobaltcore-dev/cortex/api/external/nova" "github.com/cobaltcore-dev/cortex/api/v1alpha1" @@ -76,8 +77,13 @@ func (c *FilterWeigherPipelineController) logNoHostFound(ctx context.Context, de flavorGroupName, _, err := c.resolveFlavorGroup(ctx, flavorName) if err != nil { - log.V(1).Info("no-host-found: PAYG flavor, not CR-relevant", - "instanceUUID", instanceUUID, "flavor", flavorName, "intent", intent) + if errors.Is(err, errFlavorNotInGroup) { + log.V(1).Info("no-host-found: PAYG flavor, not CR-relevant", + "instanceUUID", instanceUUID, "flavor", flavorName, "intent", intent) + } else { + log.Error(err, "no-host-found: failed to resolve flavor group", + "instanceUUID", instanceUUID, "flavor", flavorName) + } return } diff --git a/internal/scheduling/nova/filter_weigher_pipeline_controller.go b/internal/scheduling/nova/filter_weigher_pipeline_controller.go index e2a21d50e..2e00f5d59 100644 --- a/internal/scheduling/nova/filter_weigher_pipeline_controller.go +++ b/internal/scheduling/nova/filter_weigher_pipeline_controller.go @@ -78,15 +78,14 @@ func (c *FilterWeigherPipelineController) Reconcile(ctx context.Context, req ctr // Process the decision from the API. Should create and return the updated decision. func (c *FilterWeigherPipelineController) ProcessNewDecisionFromAPI(ctx context.Context, decision *v1alpha1.Decision) error { - // Early check before acquiring the mutex — no need to hold the lock just to fail. + c.processMu.Lock() + defer c.processMu.Unlock() + pipelineConf, ok := c.PipelineConfigs[decision.Spec.PipelineRef.Name] if !ok { return fmt.Errorf("pipeline %s not configured", decision.Spec.PipelineRef.Name) } - c.processMu.Lock() - defer c.processMu.Unlock() - request, err := c.process(ctx, decision) if err != nil { meta.SetStatusCondition(&decision.Status.Conditions, metav1.Condition{ diff --git a/internal/scheduling/nova/filter_weigher_pipeline_controller_test.go b/internal/scheduling/nova/filter_weigher_pipeline_controller_test.go index a597992d4..a57e261cb 100644 --- a/internal/scheduling/nova/filter_weigher_pipeline_controller_test.go +++ b/internal/scheduling/nova/filter_weigher_pipeline_controller_test.go @@ -950,8 +950,11 @@ func TestIsUserVMPlacement(t *testing.T) { } func TestPickReservationSlot(t *testing.T) { - // vmMemBytes for a 4096 MiB flavor. - const vmMemBytes = int64(4096) * 1024 * 1024 + // vmMemBytes and vmCPUs for a 4096 MiB / 2 vCPU flavor. + const ( + vmMemBytes = int64(4096) * 1024 * 1024 + vmCPUs = int64(2) + ) makeSlot := func(name string, totalMemMiB, totalCPU, usedMemMiB, usedCPU int64) v1alpha1.Reservation { var allocs map[string]v1alpha1.CommittedResourceAllocation @@ -1040,11 +1043,24 @@ func TestPickReservationSlot(t *testing.T) { candidates: []v1alpha1.Reservation{makeSlot("partial", 8192, 8, 2048, 2)}, // 6144 MiB remaining want: "partial", }, + { + name: "CPU exhausted: slot excluded as hard constraint", + candidates: []v1alpha1.Reservation{makeSlot("cpu-full", 8192, 2, 0, 2)}, // remCPU = 0 + want: "", + }, + { + name: "CPU exhausted slot skipped, other slot chosen", + candidates: []v1alpha1.Reservation{ + makeSlot("cpu-full", 8192, 2, 0, 2), // remCPU = 0, excluded + makeSlot("cpu-ok", 8192, 4, 0, 0), // remCPU = 4, fits + }, + want: "cpu-ok", + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got := pickReservationSlot(tt.candidates, vmMemBytes) + got := pickReservationSlot(tt.candidates, vmMemBytes, vmCPUs) if got != tt.want { t.Errorf("pickReservationSlot() = %q, want %q", got, tt.want) } From 2cd5a32553f204c0f49080822e6129d05460ce0e Mon Sep 17 00:00:00 2001 From: mblos Date: Wed, 13 May 2026 09:34:58 +0200 Subject: [PATCH 04/10] fixes --- internal/scheduling/nova/cr_allocation.go | 3 ++- .../scheduling/nova/filter_weigher_pipeline_controller.go | 5 +++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/internal/scheduling/nova/cr_allocation.go b/internal/scheduling/nova/cr_allocation.go index 5a29ef1e2..c3eb74d7f 100644 --- a/internal/scheduling/nova/cr_allocation.go +++ b/internal/scheduling/nova/cr_allocation.go @@ -104,6 +104,7 @@ func (c *FilterWeigherPipelineController) recordCRAllocation(ctx context.Context if latest.Spec.CommittedResourceReservation == nil { return fmt.Errorf("reservation %s lost CommittedResourceReservation spec", slotName) } + base := latest.DeepCopy() if latest.Spec.CommittedResourceReservation.Allocations == nil { latest.Spec.CommittedResourceReservation.Allocations = make(map[string]v1alpha1.CommittedResourceAllocation) } @@ -111,7 +112,7 @@ func (c *FilterWeigherPipelineController) recordCRAllocation(ctx context.Context CreationTimestamp: metav1.Now(), Resources: vmResources, } - return c.Update(ctx, latest) + return c.Patch(ctx, latest, client.MergeFrom(base)) }); retryErr != nil { log.Error(retryErr, "CR allocation: failed to patch reservation", "reservation", slotName, "instanceUUID", instanceUUID) diff --git a/internal/scheduling/nova/filter_weigher_pipeline_controller.go b/internal/scheduling/nova/filter_weigher_pipeline_controller.go index 2e00f5d59..3296bc268 100644 --- a/internal/scheduling/nova/filter_weigher_pipeline_controller.go +++ b/internal/scheduling/nova/filter_weigher_pipeline_controller.go @@ -270,6 +270,11 @@ func (c *FilterWeigherPipelineController) SetupWithManager(mgr manager.Manager, if err != nil { return err } + // Watch committed resource changes so the no-host-found classifier can read them. + bldr, err = bldr.WatchesMulticluster(&v1alpha1.CommittedResource{}, handler.Funcs{}) + if err != nil { + return err + } // Watch decision changes across all clusters. bldr, err = bldr.WatchesMulticluster( &v1alpha1.Decision{}, From 8b21a25c439e7f1a5a323c4656426b38709198ef Mon Sep 17 00:00:00 2001 From: mblos Date: Wed, 13 May 2026 11:24:20 +0200 Subject: [PATCH 05/10] feature gates --- cmd/manager/main.go | 2 ++ helm/bundles/cortex-nova/values.yaml | 16 +++++++++++++--- .../nova/filter_weigher_pipeline_controller.go | 5 ++++- pkg/conf/features.go | 15 +++++++++++++++ 4 files changed, 34 insertions(+), 4 deletions(-) create mode 100644 pkg/conf/features.go diff --git a/cmd/manager/main.go b/cmd/manager/main.go index be41d31f3..e66136dc4 100644 --- a/cmd/manager/main.go +++ b/cmd/manager/main.go @@ -395,9 +395,11 @@ func main() { commitmentsAPI.Init(mux, metrics.Registry, ctrl.Log.WithName("commitments-api")) if slices.Contains(mainConfig.EnabledControllers, "nova-pipeline-controllers") { + featureGates := conf.GetConfigOrDie[conf.FeatureGates]() // Filter-weigher pipeline controller setup. filterWeigherController := &nova.FilterWeigherPipelineController{ Monitor: filterWeigherPipelineMonitor, + FeatureGates: featureGates, NoHostFoundCounter: nova.NewNoHostFoundCounter(), } metrics.Registry.MustRegister(filterWeigherController.NoHostFoundCounter) diff --git a/helm/bundles/cortex-nova/values.yaml b/helm/bundles/cortex-nova/values.yaml index 7283ec2ea..a3496f415 100644 --- a/helm/bundles/cortex-nova/values.yaml +++ b/helm/bundles/cortex-nova/values.yaml @@ -136,15 +136,25 @@ cortex-scheduling-controllers: - failover-reservations-controller - quota-controller - capacity-controller + enabledTasks: + - nova-history-cleanup-task + - commitments-sync-task # required for committed resources + # Feature gates control optional capabilities within running components. + # To enable committed resources, all three must be set: + # enabledControllers must include committed-resource-reservations-controller (above) + # enabledTasks must include commitments-sync-task (above) + # featureGates.committedResourceTracking must be true (below) + featureGates: + # Enables VM allocation write-back into Reservation slots and no-host-found + # classification by committed resource coverage. Enable only on deployments + # that use committed resources. + committedResourceTracking: true # Pipeline used for the empty-state capacity probe (ignores allocations and reservations). capacityTotalPipeline: "kvm-report-capacity" # Pipeline used for the current-state capacity probe (considers current VM allocations). capacityPlaceablePipeline: "kvm-general-purpose-load-balancing" # How often the capacity controller re-runs its scheduler probes. capacityReconcileInterval: 5m - enabledTasks: - - nova-history-cleanup-task - - commitments-sync-task # If true, the external scheduler API will limit the list of hosts in its # response to those included in the scheduling request. novaLimitHostsToRequest: true diff --git a/internal/scheduling/nova/filter_weigher_pipeline_controller.go b/internal/scheduling/nova/filter_weigher_pipeline_controller.go index 3296bc268..484a110b9 100644 --- a/internal/scheduling/nova/filter_weigher_pipeline_controller.go +++ b/internal/scheduling/nova/filter_weigher_pipeline_controller.go @@ -20,6 +20,7 @@ import ( "github.com/cobaltcore-dev/cortex/internal/scheduling/lib" "github.com/cobaltcore-dev/cortex/internal/scheduling/nova/plugins/filters" "github.com/cobaltcore-dev/cortex/internal/scheduling/nova/plugins/weighers" + "github.com/cobaltcore-dev/cortex/pkg/conf" "github.com/cobaltcore-dev/cortex/pkg/multicluster" hv1 "github.com/cobaltcore-dev/openstack-hypervisor-operator/api/v1" ctrl "sigs.k8s.io/controller-runtime" @@ -42,6 +43,8 @@ type FilterWeigherPipelineController struct { // Mutex to only allow one process at a time processMu sync.Mutex + // FeatureGates holds feature flags for this controller. + FeatureGates conf.FeatureGates // Monitor to pass down to all pipelines. Monitor lib.FilterWeigherPipelineMonitor // Candidate gatherer to get all placement candidates if needed. @@ -104,7 +107,7 @@ func (c *FilterWeigherPipelineController) ProcessNewDecisionFromAPI(ctx context. } if pipelineConf.Spec.CreateHistory { c.upsertHistory(ctx, decision, request, err) - if err == nil && decision.Status.Result != nil && request != nil { + if err == nil && decision.Status.Result != nil && request != nil && c.FeatureGates.CommittedResourceTracking { if decision.Status.Result.TargetHost != nil && isUserVMPlacement(decision.Spec.Intent) { c.recordCRAllocation(ctx, decision, *request) } diff --git a/pkg/conf/features.go b/pkg/conf/features.go new file mode 100644 index 000000000..d354570e0 --- /dev/null +++ b/pkg/conf/features.go @@ -0,0 +1,15 @@ +// Copyright SAP SE +// SPDX-License-Identifier: Apache-2.0 + +package conf + +// FeatureGates holds boolean toggles for features that are in active rollout +// or that are only relevant for certain deployment types. Set via conf.json +// under the "featureGates" key. All flags default to false (disabled) when omitted. +type FeatureGates struct { + // CommittedResourceTracking enables committed resource integration: writing + // placed VM UUIDs back into Reservation slots and classifying no-host-found + // results by committed resource coverage. Enable only on deployments that + // use committed resources. + CommittedResourceTracking bool `json:"committedResourceTracking,omitempty"` +} From 33d631a26ffb3c8d1f3bf63f5da3b089a0975cd6 Mon Sep 17 00:00:00 2001 From: mblos Date: Wed, 13 May 2026 11:42:05 +0200 Subject: [PATCH 06/10] tests --- helm/bundles/cortex-nova/values.yaml | 6 +- internal/scheduling/nova/cr_metrics_test.go | 122 ++++++++++++++++++++ pkg/conf/features.go | 4 + 3 files changed, 131 insertions(+), 1 deletion(-) diff --git a/helm/bundles/cortex-nova/values.yaml b/helm/bundles/cortex-nova/values.yaml index a3496f415..1f0946fd3 100644 --- a/helm/bundles/cortex-nova/values.yaml +++ b/helm/bundles/cortex-nova/values.yaml @@ -148,7 +148,11 @@ cortex-scheduling-controllers: # Enables VM allocation write-back into Reservation slots and no-host-found # classification by committed resource coverage. Enable only on deployments # that use committed resources. - committedResourceTracking: true + committedResourceTracking: false + # Enables pessimistic blocking: all candidate hosts are blocked at scheduling + # time to prevent concurrent placements from violating capacity constraints. + # Enable only on deployments that support pessimistic blocking. + pessimisticBlocking: false # Pipeline used for the empty-state capacity probe (ignores allocations and reservations). capacityTotalPipeline: "kvm-report-capacity" # Pipeline used for the current-state capacity probe (considers current VM allocations). diff --git a/internal/scheduling/nova/cr_metrics_test.go b/internal/scheduling/nova/cr_metrics_test.go index 6df9018b3..c24cdd53b 100644 --- a/internal/scheduling/nova/cr_metrics_test.go +++ b/internal/scheduling/nova/cr_metrics_test.go @@ -5,10 +5,13 @@ package nova import ( "context" + "encoding/json" + "fmt" "testing" "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/testutil" + corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -19,6 +22,7 @@ import ( "github.com/cobaltcore-dev/cortex/api/v1alpha1" "github.com/cobaltcore-dev/cortex/internal/knowledge/extractor/plugins/compute" "github.com/cobaltcore-dev/cortex/internal/scheduling/lib" + "github.com/cobaltcore-dev/cortex/pkg/conf" hv1 "github.com/cobaltcore-dev/openstack-hypervisor-operator/api/v1" ) @@ -390,3 +394,121 @@ func TestLogNoHostFound(t *testing.T) { }) } } + +func TestFeatureGate_CommittedResourceTracking(t *testing.T) { + const ( + projectID = "project-1" + flavorName = "m1.large" + flavorGroup = "m1" + ) + + scheme := runtime.NewScheme() + if err := v1alpha1.AddToScheme(scheme); err != nil { + t.Fatalf("AddToScheme: %v", err) + } + + ratio := uint64(2048) + fg := compute.FlavorGroupFeature{ + Name: flavorGroup, + Flavors: []compute.FlavorInGroup{{Name: flavorName, VCPUs: 2, MemoryMB: 4096}}, + LargestFlavor: compute.FlavorInGroup{Name: flavorName, VCPUs: 2, MemoryMB: 4096}, + SmallestFlavor: compute.FlavorInGroup{Name: flavorName, VCPUs: 2, MemoryMB: 4096}, + RamCoreRatio: &ratio, + } + raw, err := v1alpha1.BoxFeatureList([]compute.FlavorGroupFeature{fg}) + if err != nil { + t.Fatalf("BoxFeatureList: %v", err) + } + flavorKnowledge := &v1alpha1.Knowledge{ + ObjectMeta: metav1.ObjectMeta{Name: "flavor-groups"}, + Status: v1alpha1.KnowledgeStatus{ + Raw: raw, + Conditions: []metav1.Condition{{ + Type: v1alpha1.KnowledgeConditionReady, + Status: metav1.ConditionTrue, + Reason: "Ready", + LastTransitionTime: metav1.Now(), + }}, + }, + } + + // Zero hosts → pipeline returns no TargetHost → triggers logNoHostFound path. + novaRequest := api.ExternalSchedulerRequest{ + Spec: api.NovaObject[api.NovaSpec]{ + Data: api.NovaSpec{ + InstanceUUID: "test-instance", + Flavor: api.NovaObject[api.NovaFlavor]{Data: api.NovaFlavor{Name: flavorName}}, + }, + }, + Context: api.NovaRequestContext{ProjectID: projectID}, + Hosts: []api.ExternalSchedulerHost{}, + Pipeline: "test-pipeline", + } + novaRaw, err := json.Marshal(novaRequest) + if err != nil { + t.Fatalf("json.Marshal: %v", err) + } + + pipelineConf := v1alpha1.Pipeline{ + ObjectMeta: metav1.ObjectMeta{Name: "test-pipeline"}, + Spec: v1alpha1.PipelineSpec{ + Type: v1alpha1.PipelineTypeFilterWeigher, + SchedulingDomain: v1alpha1.SchedulingDomainNova, + CreateHistory: true, + Filters: []v1alpha1.FilterSpec{}, + Weighers: []v1alpha1.WeigherSpec{}, + }, + } + + for _, enabled := range []bool{false, true} { + t.Run(fmt.Sprintf("CommittedResourceTracking=%v", enabled), func(t *testing.T) { + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(flavorKnowledge). + WithStatusSubresource(&v1alpha1.Decision{}, &v1alpha1.History{}). + Build() + + counter := NewNoHostFoundCounter() + reg := prometheus.NewRegistry() + reg.MustRegister(counter) + + controller := &FilterWeigherPipelineController{ + BasePipelineController: lib.BasePipelineController[lib.FilterWeigherPipeline[api.ExternalSchedulerRequest]]{ + Client: fakeClient, + Pipelines: make(map[string]lib.FilterWeigherPipeline[api.ExternalSchedulerRequest]), + PipelineConfigs: make(map[string]v1alpha1.Pipeline), + HistoryManager: lib.HistoryClient{Client: fakeClient}, + }, + FeatureGates: conf.FeatureGates{CommittedResourceTracking: enabled}, + NoHostFoundCounter: counter, + } + controller.PipelineConfigs["test-pipeline"] = pipelineConf + initResult := controller.InitPipeline(context.Background(), pipelineConf) + if len(initResult.FilterErrors) > 0 || len(initResult.WeigherErrors) > 0 { + t.Fatalf("pipeline init errors: filters=%v weighers=%v", initResult.FilterErrors, initResult.WeigherErrors) + } + controller.Pipelines["test-pipeline"] = initResult.Pipeline + + decision := &v1alpha1.Decision{ + ObjectMeta: metav1.ObjectMeta{Name: "test-decision", Namespace: "default"}, + Spec: v1alpha1.DecisionSpec{ + SchedulingDomain: v1alpha1.SchedulingDomainNova, + PipelineRef: corev1.ObjectReference{Name: "test-pipeline"}, + NovaRaw: &runtime.RawExtension{Raw: novaRaw}, + }, + } + + if err := controller.ProcessNewDecisionFromAPI(context.Background(), decision); err != nil { + t.Fatalf("ProcessNewDecisionFromAPI: %v", err) + } + + count := testutil.CollectAndCount(counter) + if enabled && count == 0 { + t.Error("expected counter increment with CommittedResourceTracking=true, got 0") + } + if !enabled && count != 0 { + t.Errorf("expected no counter increment with CommittedResourceTracking=false, got %d", count) + } + }) + } +} diff --git a/pkg/conf/features.go b/pkg/conf/features.go index d354570e0..1955c87ba 100644 --- a/pkg/conf/features.go +++ b/pkg/conf/features.go @@ -12,4 +12,8 @@ type FeatureGates struct { // results by committed resource coverage. Enable only on deployments that // use committed resources. CommittedResourceTracking bool `json:"committedResourceTracking,omitempty"` + // PessimisticBlocking enables blocking all candidate hosts at scheduling time + // to prevent concurrent placements from violating capacity constraints. Enable + // only on deployments that support pessimistic blocking. + PessimisticBlocking bool `json:"pessimisticBlocking,omitempty"` } From 399ea71ef155a1430da3e67089c91e4937c13ecd Mon Sep 17 00:00:00 2001 From: mblos Date: Wed, 13 May 2026 17:56:57 +0200 Subject: [PATCH 07/10] . --- api/v1alpha1/committed_resource_types.go | 17 ++ api/v1alpha1/committed_resource_types_test.go | 70 ++++++ api/v1alpha1/reservation_types.go | 5 + api/v1alpha1/reservation_types_test.go | 28 +++ cmd/manager/main.go | 2 + internal/scheduling/nova/cr_allocation.go | 150 +++++++----- internal/scheduling/nova/cr_evaluator.go | 109 +++++++++ internal/scheduling/nova/cr_metrics.go | 73 +++--- internal/scheduling/nova/cr_metrics_test.go | 224 +++++++++++++----- .../filter_weigher_pipeline_controller.go | 4 +- ...filter_weigher_pipeline_controller_test.go | 163 +++++++++---- .../filters/filter_has_enough_capacity.go | 3 +- .../filters/filter_quota_enforcement.go | 2 +- .../reservations/capacity/controller.go | 2 +- .../committed_resource_controller.go | 2 +- .../reservations/commitments/usage.go | 2 +- .../commitments/usage_reconciler.go | 2 +- .../commitments/usage_reconciler_monitor.go | 6 +- 18 files changed, 661 insertions(+), 203 deletions(-) create mode 100644 api/v1alpha1/committed_resource_types_test.go create mode 100644 api/v1alpha1/reservation_types_test.go create mode 100644 internal/scheduling/nova/cr_evaluator.go diff --git a/api/v1alpha1/committed_resource_types.go b/api/v1alpha1/committed_resource_types.go index d49a9be90..027c1c861 100644 --- a/api/v1alpha1/committed_resource_types.go +++ b/api/v1alpha1/committed_resource_types.go @@ -188,6 +188,23 @@ type CommittedResource struct { Status CommittedResourceStatus `json:"status,omitempty,omitzero"` } +// IsActive reports whether the commitment spec has active Reservation slots +// (state is confirmed or guaranteed). +func (s *CommittedResourceSpec) IsActive() bool { + return s.State == CommitmentStatusConfirmed || s.State == CommitmentStatusGuaranteed +} + +// IsActive reports whether the commitment has active Reservation slots +// (state is confirmed or guaranteed). +func (c *CommittedResource) IsActive() bool { + return c.Spec.IsActive() +} + +// MatchesGroup reports whether the commitment targets the given project and flavor group. +func (c *CommittedResource) MatchesGroup(projectID, flavorGroup string) bool { + return c.Spec.ProjectID == projectID && c.Spec.FlavorGroupName == flavorGroup +} + // +kubebuilder:object:root=true // CommittedResourceList contains a list of CommittedResource diff --git a/api/v1alpha1/committed_resource_types_test.go b/api/v1alpha1/committed_resource_types_test.go new file mode 100644 index 000000000..2571a9a66 --- /dev/null +++ b/api/v1alpha1/committed_resource_types_test.go @@ -0,0 +1,70 @@ +// Copyright SAP SE +// SPDX-License-Identifier: Apache-2.0 + +package v1alpha1 + +import "testing" + +func TestCommittedResourceSpec_IsActive(t *testing.T) { + tests := []struct { + state CommitmentStatus + want bool + }{ + {CommitmentStatusConfirmed, true}, + {CommitmentStatusGuaranteed, true}, + {CommitmentStatusPlanned, false}, + {CommitmentStatusPending, false}, + {CommitmentStatusSuperseded, false}, + {CommitmentStatusExpired, false}, + } + for _, tt := range tests { + spec := CommittedResourceSpec{State: tt.state} + if got := spec.IsActive(); got != tt.want { + t.Errorf("IsActive() with state %q = %v, want %v", tt.state, got, tt.want) + } + } +} + +func TestCommittedResource_IsActive(t *testing.T) { + tests := []struct { + state CommitmentStatus + want bool + }{ + {CommitmentStatusConfirmed, true}, + {CommitmentStatusGuaranteed, true}, + {CommitmentStatusPlanned, false}, + {CommitmentStatusPending, false}, + {CommitmentStatusSuperseded, false}, + {CommitmentStatusExpired, false}, + } + for _, tt := range tests { + cr := CommittedResource{Spec: CommittedResourceSpec{State: tt.state}} + if got := cr.IsActive(); got != tt.want { + t.Errorf("IsActive() with state %q = %v, want %v", tt.state, got, tt.want) + } + } +} + +func TestCommittedResource_MatchesGroup(t *testing.T) { + cr := CommittedResource{ + Spec: CommittedResourceSpec{ + ProjectID: "proj-1", + FlavorGroupName: "kvm_v2", + }, + } + tests := []struct { + project string + group string + want bool + }{ + {"proj-1", "kvm_v2", true}, + {"proj-X", "kvm_v2", false}, + {"proj-1", "kvm_v3", false}, + {"proj-X", "kvm_v3", false}, + } + for _, tt := range tests { + if got := cr.MatchesGroup(tt.project, tt.group); got != tt.want { + t.Errorf("MatchesGroup(%q, %q) = %v, want %v", tt.project, tt.group, got, tt.want) + } + } +} diff --git a/api/v1alpha1/reservation_types.go b/api/v1alpha1/reservation_types.go index 988d4b97d..4ec3ee9c5 100644 --- a/api/v1alpha1/reservation_types.go +++ b/api/v1alpha1/reservation_types.go @@ -265,6 +265,11 @@ type ReservationList struct { Items []Reservation `json:"items"` } +// MatchesGroup reports whether the reservation targets the given project and resource group. +func (s *CommittedResourceReservationSpec) MatchesGroup(projectID, resourceGroup string) bool { + return s.ProjectID == projectID && s.ResourceGroup == resourceGroup +} + // IsReady returns true if the reservation has the Ready condition set to True. func (r *Reservation) IsReady() bool { return meta.IsStatusConditionTrue(r.Status.Conditions, ReservationConditionReady) diff --git a/api/v1alpha1/reservation_types_test.go b/api/v1alpha1/reservation_types_test.go new file mode 100644 index 000000000..63019d56e --- /dev/null +++ b/api/v1alpha1/reservation_types_test.go @@ -0,0 +1,28 @@ +// Copyright SAP SE +// SPDX-License-Identifier: Apache-2.0 + +package v1alpha1 + +import "testing" + +func TestCommittedResourceReservationSpec_MatchesGroup(t *testing.T) { + spec := CommittedResourceReservationSpec{ + ProjectID: "proj-1", + ResourceGroup: "kvm_v2", + } + tests := []struct { + project string + group string + want bool + }{ + {"proj-1", "kvm_v2", true}, + {"proj-X", "kvm_v2", false}, + {"proj-1", "kvm_v3", false}, + {"proj-X", "kvm_v3", false}, + } + for _, tt := range tests { + if got := spec.MatchesGroup(tt.project, tt.group); got != tt.want { + t.Errorf("MatchesGroup(%q, %q) = %v, want %v", tt.project, tt.group, got, tt.want) + } + } +} diff --git a/cmd/manager/main.go b/cmd/manager/main.go index e66136dc4..0c4e82505 100644 --- a/cmd/manager/main.go +++ b/cmd/manager/main.go @@ -401,8 +401,10 @@ func main() { Monitor: filterWeigherPipelineMonitor, FeatureGates: featureGates, NoHostFoundCounter: nova.NewNoHostFoundCounter(), + PlacementCounter: nova.NewPlacementCounter(), } metrics.Registry.MustRegister(filterWeigherController.NoHostFoundCounter) + metrics.Registry.MustRegister(filterWeigherController.PlacementCounter) // Inferred through the base controller. filterWeigherController.Client = multiclusterClient if err := filterWeigherController.SetupWithManager(mgr, multiclusterClient); err != nil { diff --git a/internal/scheduling/nova/cr_allocation.go b/internal/scheduling/nova/cr_allocation.go index c3eb74d7f..2e20dfff9 100644 --- a/internal/scheduling/nova/cr_allocation.go +++ b/internal/scheduling/nova/cr_allocation.go @@ -31,6 +31,7 @@ func (c *FilterWeigherPipelineController) recordCRAllocation(ctx context.Context projectID := request.Context.ProjectID flavorName := request.Spec.Data.Flavor.Data.Name selectedHost := *decision.Status.Result.TargetHost + intent := string(decision.Spec.Intent) flavorGroupName, flavorInGroup, err := c.resolveFlavorGroup(ctx, flavorName) if err != nil { @@ -39,52 +40,69 @@ func (c *FilterWeigherPipelineController) recordCRAllocation(ctx context.Context } else { log.Error(err, "CR allocation: failed to resolve flavor group", "flavor", flavorName, "instanceUUID", instanceUUID) + if c.PlacementCounter != nil { + c.PlacementCounter.WithLabelValues("unknown", intent, "error").Inc() + } } return } - // List all CR reservations and filter to candidates matching this placement. - var reservationList v1alpha1.ReservationList - if err := c.List(ctx, &reservationList, - client.MatchingLabels{v1alpha1.LabelReservationType: v1alpha1.ReservationTypeLabelCommittedResource}, - ); err != nil { - log.Error(err, "CR allocation: failed to list reservations", "instanceUUID", instanceUUID) + evaluator, err := BuildCRSlotEvaluator(ctx, c.Client) + if err != nil { + log.Error(err, "CR allocation: failed to build CR slot evaluator", "instanceUUID", instanceUUID) return } - var candidates []v1alpha1.Reservation - for _, res := range reservationList.Items { - cr := res.Spec.CommittedResourceReservation - if cr == nil { - continue - } - if res.Spec.TargetHost != selectedHost || cr.ProjectID != projectID || cr.ResourceGroup != flavorGroupName { + var crList v1alpha1.CommittedResourceList + if err := c.List(ctx, &crList); err != nil { + log.Error(err, "CR allocation: failed to list committed resources", "instanceUUID", instanceUUID) + return + } + var activeCRs []v1alpha1.CommittedResource + for _, cr := range crList.Items { + if !cr.MatchesGroup(projectID, flavorGroupName) || !cr.IsActive() { continue } - // Idempotency: if this VM UUID is already recorded, the work is done. - if _, exists := cr.Allocations[instanceUUID]; exists { - log.Info("CR allocation: VM UUID already in reservation, skipping", - "instanceUUID", instanceUUID, "reservation", res.Name) - return - } - candidates = append(candidates, res) + activeCRs = append(activeCRs, cr) } - if len(candidates) == 0 { - log.V(1).Info("CR allocation: no matching reservation slot, PAYG placement", - "instanceUUID", instanceUUID, "host", selectedHost, - "projectID", projectID, "flavorGroup", flavorGroupName) + candidateHosts := decision.Status.Result.OrderedHosts + crOutcome := classifyCRPlacement(evaluator, activeCRs, candidateHosts, projectID, flavorGroupName) + + log.V(1).Info("CR allocation: placement classified", + "cr_outcome", crOutcome, + "instanceUUID", instanceUUID, + "host", selectedHost, + "projectID", projectID, + "flavorGroup", flavorGroupName, + ) + if c.PlacementCounter != nil { + c.PlacementCounter.WithLabelValues(flavorGroupName, intent, crOutcome).Inc() + } + + if crOutcome != "slot_used" { return } + // slot_used: allocate into a slot on the selected host. + slotsOnTarget := evaluator.SlotsForHost(selectedHost, projectID, flavorGroupName) + + // Idempotency: if this VM UUID is already recorded in any slot, the work is done. + for _, slot := range slotsOnTarget { + if _, exists := slot.Spec.CommittedResourceReservation.Allocations[instanceUUID]; exists { + log.Info("CR allocation: VM UUID already in reservation, skipping", + "instanceUUID", instanceUUID, "reservation", slot.Name) + return + } + } + vmMemoryBytes := int64(flavorInGroup.MemoryMB) * 1024 * 1024 //nolint:gosec // flavor memory bounded by specs vmCPUs := int64(flavorInGroup.VCPUs) //nolint:gosec // VCPUs bounded by specs - slotName := pickReservationSlot(candidates, vmMemoryBytes, vmCPUs) + slotName := pickReservationSlot(slotsOnTarget, vmMemoryBytes) if slotName == "" { - log.Error(nil, "CR allocation: no reservation slot has sufficient remaining capacity", - "instanceUUID", instanceUUID, "vmMemoryBytes", vmMemoryBytes, - "host", selectedHost, "candidates", len(candidates)) + log.V(1).Info("CR allocation: slot_used but target host has no slot with remaining capacity", + "instanceUUID", instanceUUID, "host", selectedHost) return } @@ -122,40 +140,66 @@ func (c *FilterWeigherPipelineController) recordCRAllocation(ctx context.Context log.Info("CR allocation: done", "instanceUUID", instanceUUID, "reservation", slotName) } -// pickReservationSlot selects the reservation slot with the least remaining -// memory that can still fully fit vmMemoryBytes and vmCPUs. -// Tiebreaks: least remaining CPU, then reservation name (lexicographic). -// Returns the slot name, or "" if no slot fits. -func pickReservationSlot(candidates []v1alpha1.Reservation, vmMemoryBytes, vmCPUs int64) string { +// classifyCRPlacement determines the CR slot outcome for a successful placement: +// - no_cr: no active CR or CR capacity fully exhausted (H1) +// - slot_missed: CR has remaining capacity but no candidate host has a slot with remaining > 0 (H2) +// - slot_used: CR has remaining capacity and at least one candidate host has a slot with remaining > 0 (H3) +func classifyCRPlacement( + evaluator *CRSlotEvaluator, + activeCRs []v1alpha1.CommittedResource, + candidateHosts []string, + projectID, flavorGroupName string, +) string { + + if len(activeCRs) == 0 { + return "no_cr" + } + totalCapacity := resource.Quantity{} + totalUsed := resource.Quantity{} + for _, cr := range activeCRs { + totalCapacity.Add(cr.Spec.Amount) + if used, ok := cr.Status.UsedResources["memory"]; ok { + totalUsed.Add(used) + } + } + if totalUsed.Cmp(totalCapacity) >= 0 { + return "no_cr" + } + for _, host := range candidateHosts { + for _, slot := range evaluator.SlotsForHost(host, projectID, flavorGroupName) { + if reservationRemainingMemory(slot) > 0 { + return "slot_used" + } + } + } + return "slot_missed" +} + +// pickReservationSlot selects the best reservation slot for a new VM. +// A slot is usable if it has any remaining memory (overfill is allowed: the VM +// may exceed the slot's remaining capacity, with the overflow covered by the +// host's free capacity which the pipeline already verified). +// Selection: maximise coverage (min(remMem, vmMemoryBytes)), tiebreak by +// smallest remaining memory (tightest fit), then reservation name. +// Returns the slot name, or "" if no slot has any remaining memory. +func pickReservationSlot(candidates []v1alpha1.Reservation, vmMemoryBytes int64) string { bestName := "" - var bestRemMem, bestRemCPU int64 + var bestCoverage, bestRemMem int64 for _, res := range candidates { - cr := res.Spec.CommittedResourceReservation - - totalCPUQ := res.Spec.Resources[hv1.ResourceCPU] - totalCPU := totalCPUQ.Value() - - var usedCPU int64 - for _, alloc := range cr.Allocations { - cpuQ := alloc.Resources[hv1.ResourceCPU] - usedCPU += cpuQ.Value() - } - remMem := reservationRemainingMemory(res) - remCPU := max(totalCPU-usedCPU, 0) - - if remMem < vmMemoryBytes || remCPU < vmCPUs { - continue // Slot doesn't have enough remaining capacity. + if remMem <= 0 { + continue } + coverage := min(remMem, vmMemoryBytes) if bestName == "" || - remMem < bestRemMem || - (remMem == bestRemMem && remCPU < bestRemCPU) || - (remMem == bestRemMem && remCPU == bestRemCPU && res.Name < bestName) { + coverage > bestCoverage || + (coverage == bestCoverage && remMem < bestRemMem) || + (coverage == bestCoverage && remMem == bestRemMem && res.Name < bestName) { bestName = res.Name + bestCoverage = coverage bestRemMem = remMem - bestRemCPU = remCPU } } diff --git a/internal/scheduling/nova/cr_evaluator.go b/internal/scheduling/nova/cr_evaluator.go new file mode 100644 index 000000000..81f61ffc6 --- /dev/null +++ b/internal/scheduling/nova/cr_evaluator.go @@ -0,0 +1,109 @@ +// Copyright SAP SE +// SPDX-License-Identifier: Apache-2.0 + +package nova + +import ( + "context" + + "github.com/cobaltcore-dev/cortex/api/v1alpha1" + hv1 "github.com/cobaltcore-dev/openstack-hypervisor-operator/api/v1" + "k8s.io/apimachinery/pkg/api/resource" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +// CRSlotEvaluator provides post-pipeline evaluation of CR reservation slots. +// Build once per request from HV and Reservation CRDs; query without further reads. +type CRSlotEvaluator struct { + // hvFreeMemory is (EffectiveCapacity - Allocation).memory per host, before reservation blocks. + hvFreeMemory map[string]int64 + // reservationsByHost maps host name to its ready CR reservation slots. + reservationsByHost map[string][]v1alpha1.Reservation +} + +// BuildCRSlotEvaluator lists HV CRDs and CR Reservation CRDs once and returns an evaluator +// that can answer slot-usability queries without further K8s reads. +func BuildCRSlotEvaluator(ctx context.Context, c client.Client) (*CRSlotEvaluator, error) { + eval := &CRSlotEvaluator{ + hvFreeMemory: make(map[string]int64), + reservationsByHost: make(map[string][]v1alpha1.Reservation), + } + + var hvList hv1.HypervisorList + if err := c.List(ctx, &hvList); err != nil { + return nil, err + } + for _, hv := range hvList.Items { + var capacityMap map[hv1.ResourceName]resource.Quantity + if hv.Status.EffectiveCapacity != nil { + capacityMap = hv.Status.EffectiveCapacity + } else { + capacityMap = hv.Status.Capacity + } + effectiveMemQ := capacityMap[hv1.ResourceMemory] + allocMemQ := hv.Status.Allocation[hv1.ResourceMemory] + effectiveMem := effectiveMemQ.Value() + allocMem := allocMemQ.Value() + eval.hvFreeMemory[hv.Name] = max(effectiveMem-allocMem, 0) + } + + var resList v1alpha1.ReservationList + if err := c.List(ctx, &resList, + client.MatchingLabels{v1alpha1.LabelReservationType: v1alpha1.ReservationTypeLabelCommittedResource}, + ); err != nil { + return nil, err + } + for _, res := range resList.Items { + if !res.IsReady() { + continue + } + if res.Spec.CommittedResourceReservation == nil { + continue + } + host := res.Spec.TargetHost + eval.reservationsByHost[host] = append(eval.reservationsByHost[host], res) + } + + return eval, nil +} + +// SlotsForHost returns all CR reservation slots on hostName matching projectID + flavorGroup. +func (e *CRSlotEvaluator) SlotsForHost(hostName, projectID, flavorGroup string) []v1alpha1.Reservation { + var result []v1alpha1.Reservation + for _, res := range e.reservationsByHost[hostName] { + cr := res.Spec.CommittedResourceReservation + if cr.MatchesGroup(projectID, flavorGroup) { + result = append(result, res) + } + } + return result +} + +// HasUsableSlot reports whether hostName has at least one CR slot that can accommodate +// a VM of vmMemBytes under the overfill model: +// +// slot.remaining + host.base_free >= vmMemBytes +// +// where host.base_free = hvFreeMemory[host] - sum(all reservation blocks on host). +// On happy-path candidates the pipeline already guarantees host capacity, so this +// simplifies to slot.remaining > 0 — but the full formula is evaluated regardless. +func (e *CRSlotEvaluator) HasUsableSlot(hostName, projectID, flavorGroup string, vmMemBytes int64) bool { + var allBlocks int64 + for _, res := range e.reservationsByHost[hostName] { + blockQ := res.Spec.Resources[hv1.ResourceMemory] + allBlocks += blockQ.Value() + } + hvFree := e.hvFreeMemory[hostName] + + for _, slot := range e.SlotsForHost(hostName, projectID, flavorGroup) { + slotRemaining := reservationRemainingMemory(slot) + if slotRemaining <= 0 { + continue + } + // host.base_free + slot.remaining = hvFree - allBlocks + slotRemaining + if hvFree-allBlocks+slotRemaining >= vmMemBytes { + return true + } + } + return false +} diff --git a/internal/scheduling/nova/cr_metrics.go b/internal/scheduling/nova/cr_metrics.go index ec99a3be3..7a12538f2 100644 --- a/internal/scheduling/nova/cr_metrics.go +++ b/internal/scheduling/nova/cr_metrics.go @@ -13,7 +13,6 @@ import ( "k8s.io/apimachinery/pkg/api/resource" ctrl "sigs.k8s.io/controller-runtime" - "sigs.k8s.io/controller-runtime/pkg/client" ) // NewNoHostFoundCounter creates the Prometheus counter for no-host-found classification. @@ -21,25 +20,39 @@ import ( func NewNoHostFoundCounter() *prometheus.CounterVec { return prometheus.NewCounterVec(prometheus.CounterOpts{ Name: "cortex_nova_no_host_found_total", - Help: "Nova no-host-found results classified by committed resource coverage (cases A/B/C/D).", + Help: "Nova no-host-found results classified by committed resource coverage (no_cr/cr_exhausted/slot_exhausted/slot_blocked/error).", }, []string{"case", "flavor_group", "intent"}) } +// NewPlacementCounter creates the Prometheus counter for successful Nova placements. +// Labels: flavor_group, intent, cr_slot (no_cr/slot_missed/slot_used/error). PAYG placements +// (flavor not in any group) are not counted — they return before reaching this counter. +// cr_slot=error is emitted when the flavor group lookup fails due to a K8s error. +// Register it with the metrics registry before assigning it to the controller. +func NewPlacementCounter() *prometheus.CounterVec { + return prometheus.NewCounterVec(prometheus.CounterOpts{ + Name: "cortex_nova_placement_total", + Help: "Successful Nova placements by committed resource slot outcome.", + }, []string{"flavor_group", "intent", "cr_slot"}) +} + // classifyNoHostFound determines why no host was found for a nova placement request, // in terms of committed resource coverage: // -// - D: project has no active CommittedResources for the flavor group -// - A: CommittedResources exist but are fully occupied (used >= capacity) -// - B: CommittedResources have remaining capacity but no free Reservation slot -// - C: free Reservation slots exist but placement constraints excluded all candidates +// - no_cr: project has no active CommittedResources for the flavor group (NH1) +// - cr_exhausted: CommittedResources exist but are fully occupied (NH2) +// - slot_exhausted: CR has remaining capacity but no input host has a usable slot (NH3) +// - slot_blocked: a usable slot exists but scheduling constraints excluded all such hosts (NH4) func classifyNoHostFound( activeCRs []v1alpha1.CommittedResource, - reservations []v1alpha1.Reservation, + evaluator *CRSlotEvaluator, + inputHosts []string, projectID, flavorGroupName string, + vmMemBytes int64, ) string { if len(activeCRs) == 0 { - return "D" + return "no_cr" } totalCapacity := resource.Quantity{} @@ -51,19 +64,15 @@ func classifyNoHostFound( } } if totalUsed.Cmp(totalCapacity) >= 0 { - return "A" + return "cr_exhausted" } - for _, res := range reservations { - cr := res.Spec.CommittedResourceReservation - if cr == nil || cr.ProjectID != projectID || cr.ResourceGroup != flavorGroupName { - continue - } - if reservationRemainingMemory(res) > 0 { - return "C" + for _, host := range inputHosts { + if evaluator.HasUsableSlot(host, projectID, flavorGroupName, vmMemBytes) { + return "slot_blocked" } } - return "B" + return "slot_exhausted" } // logNoHostFound classifies a no-host-found result and emits a log line and metric. @@ -75,18 +84,21 @@ func (c *FilterWeigherPipelineController) logNoHostFound(ctx context.Context, de instanceUUID := request.Spec.Data.InstanceUUID intent := decision.Spec.Intent - flavorGroupName, _, err := c.resolveFlavorGroup(ctx, flavorName) + flavorGroupName, flavorInGroup, err := c.resolveFlavorGroup(ctx, flavorName) if err != nil { if errors.Is(err, errFlavorNotInGroup) { - log.V(1).Info("no-host-found: PAYG flavor, not CR-relevant", - "instanceUUID", instanceUUID, "flavor", flavorName, "intent", intent) - } else { - log.Error(err, "no-host-found: failed to resolve flavor group", - "instanceUUID", instanceUUID, "flavor", flavorName) + return // PAYG: flavor has no group, no metric + } + log.Error(err, "no-host-found: failed to resolve flavor group", + "instanceUUID", instanceUUID, "flavor", flavorName) + if c.NoHostFoundCounter != nil { + c.NoHostFoundCounter.WithLabelValues("error", "unknown", string(intent)).Inc() } return } + vmMemBytes := int64(flavorInGroup.MemoryMB) * 1024 * 1024 //nolint:gosec // flavor memory bounded by specs + var crList v1alpha1.CommittedResourceList if err := c.List(ctx, &crList); err != nil { log.Error(err, "no-host-found: failed to list committed resources", "instanceUUID", instanceUUID) @@ -94,24 +106,19 @@ func (c *FilterWeigherPipelineController) logNoHostFound(ctx context.Context, de } var activeCRs []v1alpha1.CommittedResource for _, cr := range crList.Items { - if cr.Spec.ProjectID != projectID || cr.Spec.FlavorGroupName != flavorGroupName { - continue - } - if cr.Spec.State != v1alpha1.CommitmentStatusConfirmed && cr.Spec.State != v1alpha1.CommitmentStatusGuaranteed { + if !cr.MatchesGroup(projectID, flavorGroupName) || !cr.IsActive() { continue } activeCRs = append(activeCRs, cr) } - var reservationList v1alpha1.ReservationList - if err := c.List(ctx, &reservationList, - client.MatchingLabels{v1alpha1.LabelReservationType: v1alpha1.ReservationTypeLabelCommittedResource}, - ); err != nil { - log.Error(err, "no-host-found: failed to list reservations", "instanceUUID", instanceUUID) + evaluator, err := BuildCRSlotEvaluator(ctx, c.Client) + if err != nil { + log.Error(err, "no-host-found: failed to build CR slot evaluator", "instanceUUID", instanceUUID) return } - noHostFoundCase := classifyNoHostFound(activeCRs, reservationList.Items, projectID, flavorGroupName) + noHostFoundCase := classifyNoHostFound(activeCRs, evaluator, request.GetHosts(), projectID, flavorGroupName, vmMemBytes) log.Info("no-host-found classified", "case", noHostFoundCase, diff --git a/internal/scheduling/nova/cr_metrics_test.go b/internal/scheduling/nova/cr_metrics_test.go index c24cdd53b..e934ef5d4 100644 --- a/internal/scheduling/nova/cr_metrics_test.go +++ b/internal/scheduling/nova/cr_metrics_test.go @@ -73,98 +73,132 @@ func TestClassifyNoHostFound(t *testing.T) { proj = "project-1" group = "kvm_v2_hana_s" ) + const MiB = int64(1024 * 1024) + + // emptyEval has no hosts or slots. + emptyEval := &CRSlotEvaluator{ + hvFreeMemory: map[string]int64{}, + reservationsByHost: map[string][]v1alpha1.Reservation{}, + } + + // evalWithSlot builds an evaluator with a single slot on "host-1". + evalWithSlot := func(totalMiB, allocMiB int64) *CRSlotEvaluator { + slot := makeSlot(proj, group, totalMiB, allocMiB) + return &CRSlotEvaluator{ + hvFreeMemory: map[string]int64{"host-1": 16384 * MiB}, + reservationsByHost: map[string][]v1alpha1.Reservation{ + "host-1": {slot}, + }, + } + } tests := []struct { name string activeCRs []v1alpha1.CommittedResource - reservations []v1alpha1.Reservation + eval *CRSlotEvaluator + inputHosts []string + vmMemBytes int64 expectedCase string }{ { - name: "D: no active CRs for project+flavor group", + name: "no_cr: no active CRs for project+flavor group", activeCRs: nil, - reservations: nil, - expectedCase: "D", + eval: emptyEval, + inputHosts: nil, + vmMemBytes: 4096 * MiB, + expectedCase: "no_cr", }, { - name: "A: CRs fully occupied (used == capacity)", + name: "cr_exhausted: CRs fully occupied (used == capacity)", activeCRs: []v1alpha1.CommittedResource{ makeCR(v1alpha1.CommitmentStatusConfirmed, 8192, 8192), }, - reservations: nil, - expectedCase: "A", + eval: emptyEval, + inputHosts: []string{"host-1"}, + vmMemBytes: 4096 * MiB, + expectedCase: "cr_exhausted", }, { - name: "A: CRs fully occupied (used > capacity)", + name: "cr_exhausted: CRs fully occupied (used > capacity)", activeCRs: []v1alpha1.CommittedResource{ makeCR(v1alpha1.CommitmentStatusConfirmed, 8192, 10000), }, - reservations: nil, - expectedCase: "A", + eval: emptyEval, + inputHosts: []string{"host-1"}, + vmMemBytes: 4096 * MiB, + expectedCase: "cr_exhausted", }, { - name: "A: multiple CRs, total used >= total capacity", + name: "cr_exhausted: multiple CRs, total used >= total capacity", activeCRs: []v1alpha1.CommittedResource{ makeCR(v1alpha1.CommitmentStatusConfirmed, 4096, 4096), makeCR(v1alpha1.CommitmentStatusGuaranteed, 4096, 4096), }, - reservations: nil, - expectedCase: "A", + eval: emptyEval, + inputHosts: []string{"host-1"}, + vmMemBytes: 4096 * MiB, + expectedCase: "cr_exhausted", }, { - name: "B: CRs have capacity but no free reservation slot", + name: "slot_exhausted: CRs have capacity but slot fully allocated", activeCRs: []v1alpha1.CommittedResource{ makeCR(v1alpha1.CommitmentStatusConfirmed, 8192, 4096), }, - reservations: []v1alpha1.Reservation{ - makeSlot(proj, group, 8192, 8192), // slot fully allocated - }, - expectedCase: "B", + eval: evalWithSlot(8192, 8192), // slotRemaining=0 → skipped + inputHosts: []string{"host-1"}, + vmMemBytes: 4096 * MiB, + expectedCase: "slot_exhausted", }, { - name: "B: CRs have capacity, no slots at all", + name: "slot_exhausted: CRs have capacity, no slots at all", activeCRs: []v1alpha1.CommittedResource{ makeCR(v1alpha1.CommitmentStatusConfirmed, 8192, 0), }, - reservations: nil, - expectedCase: "B", + eval: emptyEval, + inputHosts: []string{"host-1"}, + vmMemBytes: 4096 * MiB, + expectedCase: "slot_exhausted", }, { - name: "C: free slot exists", + name: "slot_blocked: free slot exists on input host", activeCRs: []v1alpha1.CommittedResource{ makeCR(v1alpha1.CommitmentStatusConfirmed, 8192, 4096), }, - reservations: []v1alpha1.Reservation{ - makeSlot(proj, group, 8192, 4096), // slot has 4096 MiB free - }, - expectedCase: "C", + eval: evalWithSlot(8192, 4096), // 4096 MiB remaining; 16384-8192+4096=12288 >= 4096 + inputHosts: []string{"host-1"}, + vmMemBytes: 4096 * MiB, + expectedCase: "slot_blocked", }, { - name: "C: one slot full, one slot free", + name: "slot_blocked: overfill — slot smaller than VM is still usable", activeCRs: []v1alpha1.CommittedResource{ - makeCR(v1alpha1.CommitmentStatusConfirmed, 16384, 4096), - }, - reservations: []v1alpha1.Reservation{ - makeSlot(proj, group, 8192, 8192), // full - makeSlot(proj, group, 8192, 0), // free + makeCR(v1alpha1.CommitmentStatusConfirmed, 8192, 4096), }, - expectedCase: "C", + eval: evalWithSlot(8192, 6144), // 2048 MiB remaining; 16384-8192+2048=10240 >= 4096 + inputHosts: []string{"host-1"}, + vmMemBytes: 4096 * MiB, + expectedCase: "slot_blocked", }, { - name: "slots for other project are ignored", + name: "slot_exhausted: slots for other project ignored", activeCRs: []v1alpha1.CommittedResource{ makeCR(v1alpha1.CommitmentStatusConfirmed, 8192, 0), }, - reservations: []v1alpha1.Reservation{ - makeSlot("other-project", group, 8192, 0), // different project + eval: &CRSlotEvaluator{ + hvFreeMemory: map[string]int64{"host-1": 16384 * MiB}, + reservationsByHost: map[string][]v1alpha1.Reservation{ + "host-1": {makeSlot("other-project", group, 8192, 0)}, + }, }, - expectedCase: "B", + inputHosts: []string{"host-1"}, + vmMemBytes: 4096 * MiB, + expectedCase: "slot_exhausted", }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got := classifyNoHostFound(tt.activeCRs, tt.reservations, proj, group) + got := classifyNoHostFound(tt.activeCRs, tt.eval, tt.inputHosts, proj, group, tt.vmMemBytes) if got != tt.expectedCase { t.Errorf("classifyNoHostFound() = %q, want %q", got, tt.expectedCase) } @@ -199,7 +233,10 @@ func TestReservationRemainingMemory(t *testing.T) { func TestLogNoHostFound(t *testing.T) { scheme := runtime.NewScheme() if err := v1alpha1.AddToScheme(scheme); err != nil { - t.Fatalf("AddToScheme: %v", err) + t.Fatalf("AddToScheme v1alpha1: %v", err) + } + if err := hv1.AddToScheme(scheme); err != nil { + t.Fatalf("AddToScheme hv1: %v", err) } const ( @@ -285,42 +322,94 @@ func TestLogNoHostFound(t *testing.T) { return res } + makeReadyReservationSlot := func(name, targetHost string, totalMemMiB, allocatedMemMiB int64) *v1alpha1.Reservation { + res := makeReservationSlot(name, totalMemMiB, allocatedMemMiB) + res.Spec.TargetHost = targetHost + res.Status.Conditions = []metav1.Condition{{ + Type: v1alpha1.ReservationConditionReady, + Status: metav1.ConditionTrue, + Reason: "Ready", + LastTransitionTime: metav1.Now(), + }} + return res + } + + makeHV := func(name string, capacityMiB int64) *hv1.Hypervisor { + return &hv1.Hypervisor{ + ObjectMeta: metav1.ObjectMeta{Name: name}, + Status: hv1.HypervisorStatus{ + EffectiveCapacity: map[hv1.ResourceName]resource.Quantity{ + hv1.ResourceMemory: *resource.NewQuantity(capacityMiB*1024*1024, resource.BinarySI), + }, + Allocation: map[hv1.ResourceName]resource.Quantity{ + hv1.ResourceMemory: *resource.NewQuantity(0, resource.BinarySI), + }, + }, + } + } + tests := []struct { - name string - objects []client.Object - payg bool - expectedCase string // "" means no counter increment expected + name string + objects []client.Object + requestHosts []api.ExternalSchedulerHost + payg bool + expectedCase string // "" means no counter increment expected + expectedFlavorGroup string // defaults to flavorGroup if empty }{ { - name: "D: no active CRs", + name: "no_cr: no active CRs", objects: []client.Object{flavorKnowledge()}, - expectedCase: "D", + expectedCase: "no_cr", }, { - name: "A: CRs fully occupied", + name: "cr_exhausted: CRs fully occupied", objects: []client.Object{ flavorKnowledge(), makeCRObject(v1alpha1.CommitmentStatusConfirmed, 8192, 8192), }, - expectedCase: "A", + expectedCase: "cr_exhausted", }, { - name: "B: capacity exists but no free slot", + name: "slot_exhausted: slot exists but fully allocated", objects: []client.Object{ flavorKnowledge(), makeCRObject(v1alpha1.CommitmentStatusConfirmed, 8192, 4096), - makeReservationSlot("slot-full", 8192, 8192), + makeHV("host-1", 16384), + makeReadyReservationSlot("slot-full", "host-1", 8192, 8192), }, - expectedCase: "B", + requestHosts: []api.ExternalSchedulerHost{{ComputeHost: "host-1"}}, + expectedCase: "slot_exhausted", }, { - name: "C: free slot exists", + name: "slot_blocked: free slot on candidate host", objects: []client.Object{ flavorKnowledge(), makeCRObject(v1alpha1.CommitmentStatusConfirmed, 8192, 4096), - makeReservationSlot("slot-free", 8192, 0), + makeHV("host-1", 16384), + makeReadyReservationSlot("slot-free", "host-1", 8192, 4096), + }, + requestHosts: []api.ExternalSchedulerHost{{ComputeHost: "host-1"}}, + expectedCase: "slot_blocked", + }, + { + name: "no_cr: inactive CR (pending state) is filtered by IsActive()", + objects: []client.Object{ + flavorKnowledge(), + makeCRObject(v1alpha1.CommitmentStatusPending, 8192, 0), }, - expectedCase: "C", + expectedCase: "no_cr", + }, + { + name: "no_cr: CR for wrong project is filtered by MatchesGroup()", + objects: []client.Object{ + flavorKnowledge(), + func() *v1alpha1.CommittedResource { + cr := makeCRObject(v1alpha1.CommitmentStatusConfirmed, 8192, 0) + cr.Spec.ProjectID = "other-project" + return cr + }(), + }, + expectedCase: "no_cr", }, { name: "PAYG: flavor not in any group", @@ -329,11 +418,12 @@ func TestLogNoHostFound(t *testing.T) { expectedCase: "", }, { - name: "no knowledge CRD: no counter increment", + name: "error: knowledge CRD unavailable", objects: []client.Object{ makeCRObject(v1alpha1.CommitmentStatusConfirmed, 8192, 0), }, - expectedCase: "", + expectedCase: "error", + expectedFlavorGroup: "unknown", }, } @@ -369,6 +459,7 @@ func TestLogNoHostFound(t *testing.T) { }, }, Context: api.NovaRequestContext{ProjectID: projectID}, + Hosts: tt.requestHosts, } decision := &v1alpha1.Decision{ Spec: v1alpha1.DecisionSpec{Intent: api.CreateIntent}, @@ -377,18 +468,23 @@ func TestLogNoHostFound(t *testing.T) { controller.logNoHostFound(context.Background(), decision, request) if tt.expectedCase == "" { - total := testutil.ToFloat64(counter.WithLabelValues("A", flavorGroup, string(api.CreateIntent))) + - testutil.ToFloat64(counter.WithLabelValues("B", flavorGroup, string(api.CreateIntent))) + - testutil.ToFloat64(counter.WithLabelValues("C", flavorGroup, string(api.CreateIntent))) + - testutil.ToFloat64(counter.WithLabelValues("D", flavorGroup, string(api.CreateIntent))) + total := testutil.ToFloat64(counter.WithLabelValues("no_cr", flavorGroup, string(api.CreateIntent))) + + testutil.ToFloat64(counter.WithLabelValues("cr_exhausted", flavorGroup, string(api.CreateIntent))) + + testutil.ToFloat64(counter.WithLabelValues("slot_exhausted", flavorGroup, string(api.CreateIntent))) + + testutil.ToFloat64(counter.WithLabelValues("slot_blocked", flavorGroup, string(api.CreateIntent))) + + testutil.ToFloat64(counter.WithLabelValues("error", "unknown", string(api.CreateIntent))) if total != 0 { t.Errorf("expected no counter increment, got total %.0f", total) } } else { - got := testutil.ToFloat64(counter.WithLabelValues(tt.expectedCase, flavorGroup, string(api.CreateIntent))) + expectedFG := tt.expectedFlavorGroup + if expectedFG == "" { + expectedFG = flavorGroup + } + got := testutil.ToFloat64(counter.WithLabelValues(tt.expectedCase, expectedFG, string(api.CreateIntent))) if got != 1 { t.Errorf("counter[case=%q, flavorGroup=%q, intent=%q] = %.0f, want 1", - tt.expectedCase, flavorGroup, string(api.CreateIntent), got) + tt.expectedCase, expectedFG, string(api.CreateIntent), got) } } }) @@ -404,9 +500,11 @@ func TestFeatureGate_CommittedResourceTracking(t *testing.T) { scheme := runtime.NewScheme() if err := v1alpha1.AddToScheme(scheme); err != nil { - t.Fatalf("AddToScheme: %v", err) + t.Fatalf("AddToScheme v1alpha1: %v", err) + } + if err := hv1.AddToScheme(scheme); err != nil { + t.Fatalf("AddToScheme hv1: %v", err) } - ratio := uint64(2048) fg := compute.FlavorGroupFeature{ Name: flavorGroup, diff --git a/internal/scheduling/nova/filter_weigher_pipeline_controller.go b/internal/scheduling/nova/filter_weigher_pipeline_controller.go index 484a110b9..4c9a90096 100644 --- a/internal/scheduling/nova/filter_weigher_pipeline_controller.go +++ b/internal/scheduling/nova/filter_weigher_pipeline_controller.go @@ -50,8 +50,10 @@ type FilterWeigherPipelineController struct { // Candidate gatherer to get all placement candidates if needed. gatherer CandidateGatherer - // NoHostFoundCounter counts no-host-found results by CR coverage case (A/B/C/D). + // NoHostFoundCounter counts no-host-found results by CR coverage case (A/B/C/D/payg). NoHostFoundCounter *prometheus.CounterVec + // PlacementCounter counts successful placements by CR slot outcome. + PlacementCounter *prometheus.CounterVec } // The type of pipeline this controller manages. diff --git a/internal/scheduling/nova/filter_weigher_pipeline_controller_test.go b/internal/scheduling/nova/filter_weigher_pipeline_controller_test.go index a57e261cb..2c2e85414 100644 --- a/internal/scheduling/nova/filter_weigher_pipeline_controller_test.go +++ b/internal/scheduling/nova/filter_weigher_pipeline_controller_test.go @@ -11,6 +11,8 @@ import ( "testing" "time" + "github.com/prometheus/client_golang/prometheus" + "github.com/prometheus/client_golang/prometheus/testutil" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -950,11 +952,8 @@ func TestIsUserVMPlacement(t *testing.T) { } func TestPickReservationSlot(t *testing.T) { - // vmMemBytes and vmCPUs for a 4096 MiB / 2 vCPU flavor. - const ( - vmMemBytes = int64(4096) * 1024 * 1024 - vmCPUs = int64(2) - ) + // vmMemBytes for a 4096 MiB flavor. + const vmMemBytes = int64(4096) * 1024 * 1024 makeSlot := func(name string, totalMemMiB, totalCPU, usedMemMiB, usedCPU int64) v1alpha1.Reservation { var allocs map[string]v1alpha1.CommittedResourceAllocation @@ -993,33 +992,25 @@ func TestPickReservationSlot(t *testing.T) { want: "", }, { - name: "single slot fits", + name: "single slot fits fully", candidates: []v1alpha1.Reservation{makeSlot("a", 8192, 8, 0, 0)}, want: "a", }, { - name: "single slot too small after allocations", - candidates: []v1alpha1.Reservation{makeSlot("a", 8192, 8, 8192, 8)}, // fully consumed + name: "slot with zero remaining memory excluded", + candidates: []v1alpha1.Reservation{makeSlot("a", 8192, 8, 8192, 0)}, want: "", }, { - name: "picks slot with least remaining memory", + name: "picks slot with least remaining memory when both cover fully", candidates: []v1alpha1.Reservation{ makeSlot("large", 8192, 8, 0, 0), // 8192 MiB remaining - makeSlot("small", 6144, 8, 0, 0), // 6144 MiB remaining, still fits + makeSlot("small", 6144, 8, 0, 0), // 6144 MiB remaining }, want: "small", }, { - name: "CPU tiebreak on equal remaining memory", - candidates: []v1alpha1.Reservation{ - makeSlot("more-cpu", 6144, 8, 0, 0), // remCPU = 8 - makeSlot("less-cpu", 6144, 4, 0, 0), // remCPU = 4 - }, - want: "less-cpu", - }, - { - name: "name tiebreak on equal remaining memory and CPU", + name: "name tiebreak when coverage and remaining memory are equal", candidates: []v1alpha1.Reservation{ makeSlot("slot-b", 6144, 4, 0, 0), makeSlot("slot-a", 6144, 4, 0, 0), @@ -1039,28 +1030,49 @@ func TestPickReservationSlot(t *testing.T) { want: "", }, { - name: "partially used slot still fits", + name: "partially used slot still usable", candidates: []v1alpha1.Reservation{makeSlot("partial", 8192, 8, 2048, 2)}, // 6144 MiB remaining want: "partial", }, { - name: "CPU exhausted: slot excluded as hard constraint", - candidates: []v1alpha1.Reservation{makeSlot("cpu-full", 8192, 2, 0, 2)}, // remCPU = 0 - want: "", + name: "overfill: slot smaller than VM is usable", + candidates: []v1alpha1.Reservation{makeSlot("a", 4096, 8, 2048, 0)}, // 2048 MiB remaining < vmMemBytes + want: "a", + }, + { + name: "overfill: full coverage preferred over partial", + candidates: []v1alpha1.Reservation{ + makeSlot("partial", 4096, 8, 2048, 0), // 2048 MiB remaining, coverage=2048 + makeSlot("full", 6144, 8, 0, 0), // 6144 MiB remaining, coverage=4096 (full) + }, + want: "full", + }, + { + name: "overfill: highest partial coverage preferred", + candidates: []v1alpha1.Reservation{ + makeSlot("low", 4096, 8, 2048, 0), // 2048 MiB remaining, coverage=2048 + makeSlot("high", 4096, 8, 1024, 0), // 3072 MiB remaining, coverage=3072 + }, + want: "high", + }, + { + name: "CPU exhausted slot is still usable (overfill applies to CPU too)", + candidates: []v1alpha1.Reservation{makeSlot("cpu-full", 8192, 2, 0, 2)}, // remCPU=0 but remMem>0 + want: "cpu-full", }, { - name: "CPU exhausted slot skipped, other slot chosen", + name: "name tiebreak when memory equal (CPU no longer a tiebreak criterion)", candidates: []v1alpha1.Reservation{ - makeSlot("cpu-full", 8192, 2, 0, 2), // remCPU = 0, excluded - makeSlot("cpu-ok", 8192, 4, 0, 0), // remCPU = 4, fits + makeSlot("more-cpu", 6144, 8, 0, 0), + makeSlot("less-cpu", 6144, 4, 0, 0), }, - want: "cpu-ok", + want: "less-cpu", // wins by name, not by CPU }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got := pickReservationSlot(tt.candidates, vmMemBytes, vmCPUs) + got := pickReservationSlot(tt.candidates, vmMemBytes) if got != tt.want { t.Errorf("pickReservationSlot() = %q, want %q", got, tt.want) } @@ -1071,7 +1083,10 @@ func TestPickReservationSlot(t *testing.T) { func TestRecordCRAllocation(t *testing.T) { scheme := runtime.NewScheme() if err := v1alpha1.AddToScheme(scheme); err != nil { - t.Fatalf("AddToScheme: %v", err) + t.Fatalf("AddToScheme v1alpha1: %v", err) + } + if err := hv1.AddToScheme(scheme); err != nil { + t.Fatalf("AddToScheme hv1: %v", err) } const ( @@ -1148,15 +1163,42 @@ func TestRecordCRAllocation(t *testing.T) { } } - makeDecision := func(host string) *v1alpha1.Decision { + makeDecision := func(host string, candidates ...string) *v1alpha1.Decision { h := host return &v1alpha1.Decision{ Status: v1alpha1.DecisionStatus{ - Result: &v1alpha1.DecisionResult{TargetHost: &h}, + Result: &v1alpha1.DecisionResult{ + TargetHost: &h, + OrderedHosts: candidates, + }, }, } } + // makeCRObject creates a CommittedResource for the test project+flavorGroup with remaining capacity. + makeCRObject := func() *v1alpha1.CommittedResource { + return &v1alpha1.CommittedResource{ + ObjectMeta: metav1.ObjectMeta{Name: "cr-1"}, + Spec: v1alpha1.CommittedResourceSpec{ + ProjectID: projectID, + FlavorGroupName: flavorGroup, + State: v1alpha1.CommitmentStatusConfirmed, + Amount: *resource.NewQuantity(int64(8192)*1024*1024, resource.BinarySI), + }, + } + } + + // setReady marks a reservation as Ready so BuildCRSlotEvaluator indexes it. + setReady := func(res *v1alpha1.Reservation) *v1alpha1.Reservation { + res.Status.Conditions = []metav1.Condition{{ + Type: v1alpha1.ReservationConditionReady, + Status: metav1.ConditionTrue, + Reason: "Ready", + LastTransitionTime: metav1.Now(), + }} + return res + } + vmAlloc := func() map[string]v1alpha1.CommittedResourceAllocation { return map[string]v1alpha1.CommittedResourceAllocation{ instanceUUID: { @@ -1176,15 +1218,17 @@ func TestRecordCRAllocation(t *testing.T) { decision *v1alpha1.Decision checkSlot string expectAllocation bool + expectedCRSlot string // if non-empty, asserts PlacementCounter cr_slot label }{ { name: "writes allocation into matching reservation", objects: []client.Object{ flavorKnowledge(), - makeReservation("slot-1", 8192, 8, projectID, flavorGroup, selectedHost, nil), + makeCRObject(), + setReady(makeReservation("slot-1", 8192, 8, projectID, flavorGroup, selectedHost, nil)), }, request: makeRequest(instanceUUID, projectID, flavorName), - decision: makeDecision(selectedHost), + decision: makeDecision(selectedHost, selectedHost), checkSlot: "slot-1", expectAllocation: true, }, @@ -1192,10 +1236,11 @@ func TestRecordCRAllocation(t *testing.T) { name: "idempotent: UUID already in allocations", objects: []client.Object{ flavorKnowledge(), - makeReservation("slot-1", 8192, 8, projectID, flavorGroup, selectedHost, vmAlloc()), + makeCRObject(), + setReady(makeReservation("slot-1", 8192, 8, projectID, flavorGroup, selectedHost, vmAlloc())), }, request: makeRequest(instanceUUID, projectID, flavorName), - decision: makeDecision(selectedHost), + decision: makeDecision(selectedHost, selectedHost), checkSlot: "slot-1", expectAllocation: true, }, @@ -1203,10 +1248,10 @@ func TestRecordCRAllocation(t *testing.T) { name: "PAYG: flavor not in any group", objects: []client.Object{ flavorKnowledge(), - makeReservation("slot-1", 8192, 8, projectID, flavorGroup, selectedHost, nil), + setReady(makeReservation("slot-1", 8192, 8, projectID, flavorGroup, selectedHost, nil)), }, request: makeRequest(instanceUUID, projectID, "unknown-flavor"), - decision: makeDecision(selectedHost), + decision: makeDecision(selectedHost, selectedHost), checkSlot: "slot-1", expectAllocation: false, }, @@ -1214,10 +1259,11 @@ func TestRecordCRAllocation(t *testing.T) { name: "no matching reservation: host mismatch", objects: []client.Object{ flavorKnowledge(), - makeReservation("slot-1", 8192, 8, projectID, flavorGroup, "other-host", nil), + makeCRObject(), + setReady(makeReservation("slot-1", 8192, 8, projectID, flavorGroup, "other-host", nil)), }, request: makeRequest(instanceUUID, projectID, flavorName), - decision: makeDecision(selectedHost), + decision: makeDecision(selectedHost, selectedHost), checkSlot: "slot-1", expectAllocation: false, }, @@ -1225,7 +1271,8 @@ func TestRecordCRAllocation(t *testing.T) { name: "no slot fits: all capacity used", objects: []client.Object{ flavorKnowledge(), - makeReservation("slot-full", 4096, 2, projectID, flavorGroup, selectedHost, + makeCRObject(), + setReady(makeReservation("slot-full", 4096, 2, projectID, flavorGroup, selectedHost, map[string]v1alpha1.CommittedResourceAllocation{ "other-vm": { Resources: map[hv1.ResourceName]resource.Quantity{ @@ -1233,22 +1280,39 @@ func TestRecordCRAllocation(t *testing.T) { hv1.ResourceCPU: *resource.NewQuantity(2, resource.DecimalSI), }, }, - }), + })), }, request: makeRequest(instanceUUID, projectID, flavorName), - decision: makeDecision(selectedHost), + decision: makeDecision(selectedHost, selectedHost), checkSlot: "slot-full", expectAllocation: false, }, + { + name: "inactive CR (pending state) is filtered by IsActive(): no allocation", + objects: []client.Object{ + flavorKnowledge(), + func() *v1alpha1.CommittedResource { + cr := makeCRObject() + cr.Spec.State = v1alpha1.CommitmentStatusPending + return cr + }(), + setReady(makeReservation("slot-1", 8192, 8, projectID, flavorGroup, selectedHost, nil)), + }, + request: makeRequest(instanceUUID, projectID, flavorName), + decision: makeDecision(selectedHost, selectedHost), + checkSlot: "slot-1", + expectAllocation: false, + }, { name: "no knowledge CRD: logs error, no allocation", objects: []client.Object{ makeReservation("slot-1", 8192, 8, projectID, flavorGroup, selectedHost, nil), }, request: makeRequest(instanceUUID, projectID, flavorName), - decision: makeDecision(selectedHost), + decision: makeDecision(selectedHost, selectedHost), checkSlot: "slot-1", expectAllocation: false, + expectedCRSlot: "error", }, } @@ -1259,10 +1323,15 @@ func TestRecordCRAllocation(t *testing.T) { WithObjects(tt.objects...). Build() + counter := NewPlacementCounter() + reg := prometheus.NewRegistry() + reg.MustRegister(counter) + controller := &FilterWeigherPipelineController{ BasePipelineController: lib.BasePipelineController[lib.FilterWeigherPipeline[api.ExternalSchedulerRequest]]{ Client: fakeClient, }, + PlacementCounter: counter, } controller.recordCRAllocation(context.Background(), tt.decision, tt.request) @@ -1278,6 +1347,14 @@ func TestRecordCRAllocation(t *testing.T) { if !tt.expectAllocation && hasAlloc { t.Errorf("expected no allocation for UUID %q but one was written", instanceUUID) } + if tt.expectedCRSlot != "" { + intent := string(tt.decision.Spec.Intent) + got := testutil.ToFloat64(counter.WithLabelValues("unknown", intent, tt.expectedCRSlot)) + if got != 1 { + t.Errorf("PlacementCounter[flavor_group=unknown, intent=%q, cr_slot=%q] = %.0f, want 1", + intent, tt.expectedCRSlot, got) + } + } }) } } 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 b97d3e0e5..e72575c57 100644 --- a/internal/scheduling/nova/plugins/filters/filter_has_enough_capacity.go +++ b/internal/scheduling/nova/plugins/filters/filter_has_enough_capacity.go @@ -139,8 +139,7 @@ func (s *FilterHasEnoughCapacity) Run(traceLog *slog.Logger, request api.Externa // For committed resource reservations: unlock resources only if: // 1. Project ID matches // 2. ResourceGroup matches the flavor's hw_version - reservation.Spec.CommittedResourceReservation.ProjectID == request.Spec.Data.ProjectID && - reservation.Spec.CommittedResourceReservation.ResourceGroup == request.Spec.Data.Flavor.Data.ExtraSpecs["hw_version"]: + reservation.Spec.CommittedResourceReservation.MatchesGroup(request.Spec.Data.ProjectID, request.Spec.Data.Flavor.Data.ExtraSpecs["hw_version"]): traceLog.Info("unlocking resources reserved by matching committed resource reservation with allocation", "reservation", reservation.Name, "instanceUUID", request.Spec.Data.InstanceUUID, diff --git a/internal/scheduling/nova/plugins/filters/filter_quota_enforcement.go b/internal/scheduling/nova/plugins/filters/filter_quota_enforcement.go index 18286fce6..13fcc1fc7 100644 --- a/internal/scheduling/nova/plugins/filters/filter_quota_enforcement.go +++ b/internal/scheduling/nova/plugins/filters/filter_quota_enforcement.go @@ -149,7 +149,7 @@ func (s *FilterQuotaEnforcement) Run(traceLog *slog.Logger, request api.External if spec.ResourceType != v1alpha1.CommittedResourceTypeMemory { continue } - if spec.State != v1alpha1.CommitmentStatusConfirmed && spec.State != v1alpha1.CommitmentStatusGuaranteed { + if !spec.IsActive() { continue } now := time.Now() diff --git a/internal/scheduling/reservations/capacity/controller.go b/internal/scheduling/reservations/capacity/controller.go index 76a8aaf9a..68c24c4bc 100644 --- a/internal/scheduling/reservations/capacity/controller.go +++ b/internal/scheduling/reservations/capacity/controller.go @@ -320,7 +320,7 @@ func (c *Controller) sumCommittedCapacity(ctx context.Context, groupName, az str if cr.Spec.ResourceType != v1alpha1.CommittedResourceTypeMemory { continue } - if cr.Spec.State != v1alpha1.CommitmentStatusGuaranteed && cr.Spec.State != v1alpha1.CommitmentStatusConfirmed { + if !cr.IsActive() { continue } amount := cr.Spec.Amount diff --git a/internal/scheduling/reservations/commitments/committed_resource_controller.go b/internal/scheduling/reservations/commitments/committed_resource_controller.go index fde167152..c5df24173 100644 --- a/internal/scheduling/reservations/commitments/committed_resource_controller.go +++ b/internal/scheduling/reservations/commitments/committed_resource_controller.go @@ -257,7 +257,7 @@ func (r *CommittedResourceController) reconcileCoresHeadroom(ctx context.Context if other.Spec.FlavorGroupName != cr.Spec.FlavorGroupName || other.Spec.AvailabilityZone != cr.Spec.AvailabilityZone { continue } - if other.Spec.State != v1alpha1.CommitmentStatusGuaranteed && other.Spec.State != v1alpha1.CommitmentStatusConfirmed { + if !other.IsActive() { continue } if other.Status.AcceptedSpec == nil { diff --git a/internal/scheduling/reservations/commitments/usage.go b/internal/scheduling/reservations/commitments/usage.go index eb97cbd12..a6bae2f88 100644 --- a/internal/scheduling/reservations/commitments/usage.go +++ b/internal/scheduling/reservations/commitments/usage.go @@ -224,7 +224,7 @@ func buildCommitmentCapacityMap( } // Use AcceptedSpec.State so sibling CRs whose spec is mid-transition (e.g. syncer just // wrote expired before the CR controller accepted it) don't lose capacity prematurely. - if cr.Status.AcceptedSpec.State != v1alpha1.CommitmentStatusConfirmed && cr.Status.AcceptedSpec.State != v1alpha1.CommitmentStatusGuaranteed { + if !cr.Status.AcceptedSpec.IsActive() { continue } diff --git a/internal/scheduling/reservations/commitments/usage_reconciler.go b/internal/scheduling/reservations/commitments/usage_reconciler.go index ef04c329b..f0f8ec554 100644 --- a/internal/scheduling/reservations/commitments/usage_reconciler.go +++ b/internal/scheduling/reservations/commitments/usage_reconciler.go @@ -49,7 +49,7 @@ func (r *UsageReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl log := ctrl.LoggerFrom(ctx).WithValues("component", "usage-reconciler", "committedResource", req.Name) // Only active commitments have assigned VMs. Clear stale usage status if present. - if cr.Spec.State != v1alpha1.CommitmentStatusConfirmed && cr.Spec.State != v1alpha1.CommitmentStatusGuaranteed { + if !cr.IsActive() { log.Info("skipping: commitment state is not active", "state", cr.Spec.State) if len(cr.Status.AssignedInstances) > 0 || len(cr.Status.UsedResources) > 0 { old := cr.DeepCopy() diff --git a/internal/scheduling/reservations/commitments/usage_reconciler_monitor.go b/internal/scheduling/reservations/commitments/usage_reconciler_monitor.go index fe64ac823..2adb25a44 100644 --- a/internal/scheduling/reservations/commitments/usage_reconciler_monitor.go +++ b/internal/scheduling/reservations/commitments/usage_reconciler_monitor.go @@ -18,17 +18,17 @@ type UsageReconcilerMonitor struct { func NewUsageReconcilerMonitor() UsageReconcilerMonitor { m := UsageReconcilerMonitor{ reconcileDuration: prometheus.NewHistogramVec(prometheus.HistogramOpts{ - Name: "cortex_cr_usage_reconcile_duration_seconds", + Name: "cortex_committed_resource_usage_reconcile_duration_seconds", Help: "Duration of committed resource usage reconcile runs in seconds.", Buckets: []float64{0.1, 0.25, 0.5, 1, 2.5, 5, 10, 30}, }, []string{"result"}), statusAge: prometheus.NewHistogram(prometheus.HistogramOpts{ - Name: "cortex_cr_usage_status_age_seconds", + Name: "cortex_committed_resource_usage_status_age_seconds", Help: "Age of CommittedResource usage status at reconcile time, in seconds. Distribution across all active commitments shows freshness spread.", Buckets: []float64{30, 60, 120, 300, 600, 900, 1800}, }), assignedInstances: prometheus.NewGaugeVec(prometheus.GaugeOpts{ - Name: "cortex_cr_usage_assigned_vms_total", + Name: "cortex_committed_resource_usage_assigned_vms_total", Help: "Number of VMs currently assigned to committed resources for a project.", }, []string{"project_id"}), } From 08f283a692cca07eae4edfe4c9c62713d9bd853e Mon Sep 17 00:00:00 2001 From: mblos Date: Wed, 20 May 2026 14:41:08 +0200 Subject: [PATCH 08/10] rename label --- internal/scheduling/nova/cr_metrics.go | 4 ++-- .../scheduling/nova/filter_weigher_pipeline_controller.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/scheduling/nova/cr_metrics.go b/internal/scheduling/nova/cr_metrics.go index 7a12538f2..22f6eb34a 100644 --- a/internal/scheduling/nova/cr_metrics.go +++ b/internal/scheduling/nova/cr_metrics.go @@ -21,7 +21,7 @@ func NewNoHostFoundCounter() *prometheus.CounterVec { return prometheus.NewCounterVec(prometheus.CounterOpts{ Name: "cortex_nova_no_host_found_total", Help: "Nova no-host-found results classified by committed resource coverage (no_cr/cr_exhausted/slot_exhausted/slot_blocked/error).", - }, []string{"case", "flavor_group", "intent"}) + }, []string{"cr_slot", "flavor_group", "intent"}) } // NewPlacementCounter creates the Prometheus counter for successful Nova placements. @@ -121,7 +121,7 @@ func (c *FilterWeigherPipelineController) logNoHostFound(ctx context.Context, de noHostFoundCase := classifyNoHostFound(activeCRs, evaluator, request.GetHosts(), projectID, flavorGroupName, vmMemBytes) log.Info("no-host-found classified", - "case", noHostFoundCase, + "cr_slot", noHostFoundCase, "instanceUUID", instanceUUID, "projectID", projectID, "flavorGroup", flavorGroupName, diff --git a/internal/scheduling/nova/filter_weigher_pipeline_controller.go b/internal/scheduling/nova/filter_weigher_pipeline_controller.go index 4c9a90096..77aa79859 100644 --- a/internal/scheduling/nova/filter_weigher_pipeline_controller.go +++ b/internal/scheduling/nova/filter_weigher_pipeline_controller.go @@ -50,7 +50,7 @@ type FilterWeigherPipelineController struct { // Candidate gatherer to get all placement candidates if needed. gatherer CandidateGatherer - // NoHostFoundCounter counts no-host-found results by CR coverage case (A/B/C/D/payg). + // NoHostFoundCounter counts no-host-found results by CR slot outcome (no_cr/cr_exhausted/slot_exhausted/slot_blocked/error). NoHostFoundCounter *prometheus.CounterVec // PlacementCounter counts successful placements by CR slot outcome. PlacementCounter *prometheus.CounterVec From 9772a55df5299246c94fd3e521c535494829916d Mon Sep 17 00:00:00 2001 From: mblos Date: Wed, 20 May 2026 14:55:39 +0200 Subject: [PATCH 09/10] . --- helm/bundles/cortex-nova/values.yaml | 10 +--------- internal/scheduling/nova/cr_allocation.go | 1 - 2 files changed, 1 insertion(+), 10 deletions(-) diff --git a/helm/bundles/cortex-nova/values.yaml b/helm/bundles/cortex-nova/values.yaml index 6430483f1..bc5451e96 100644 --- a/helm/bundles/cortex-nova/values.yaml +++ b/helm/bundles/cortex-nova/values.yaml @@ -140,19 +140,11 @@ cortex-scheduling-controllers: - nova-history-cleanup-task - commitments-sync-task # required for committed resources # Feature gates control optional capabilities within running components. - # To enable committed resources, all three must be set: - # enabledControllers must include committed-resource-reservations-controller (above) - # enabledTasks must include commitments-sync-task (above) - # featureGates.committedResourceTracking must be true (below) featureGates: # Enables VM allocation write-back into Reservation slots and no-host-found # classification by committed resource coverage. Enable only on deployments - # that use committed resources. + # that use committed resources. Requires also enabling of CR controllers and tasks committedResourceTracking: false - # Enables pessimistic blocking: all candidate hosts are blocked at scheduling - # time to prevent concurrent placements from violating capacity constraints. - # Enable only on deployments that support pessimistic blocking. - pessimisticBlocking: false # Pipeline used for the empty-state capacity probe (ignores allocations and reservations). capacityTotalPipeline: "kvm-report-capacity" # Pipeline used for the current-state capacity probe (considers current VM allocations). diff --git a/internal/scheduling/nova/cr_allocation.go b/internal/scheduling/nova/cr_allocation.go index 2e20dfff9..5c3ffb2a5 100644 --- a/internal/scheduling/nova/cr_allocation.go +++ b/internal/scheduling/nova/cr_allocation.go @@ -84,7 +84,6 @@ func (c *FilterWeigherPipelineController) recordCRAllocation(ctx context.Context return } - // slot_used: allocate into a slot on the selected host. slotsOnTarget := evaluator.SlotsForHost(selectedHost, projectID, flavorGroupName) // Idempotency: if this VM UUID is already recorded in any slot, the work is done. From cdf071e0dbac14336742d553704e0de6ddb3314d Mon Sep 17 00:00:00 2001 From: mblos Date: Wed, 20 May 2026 14:56:55 +0200 Subject: [PATCH 10/10] . --- .../scheduling/nova/filter_weigher_pipeline_controller.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/scheduling/nova/filter_weigher_pipeline_controller.go b/internal/scheduling/nova/filter_weigher_pipeline_controller.go index 77aa79859..7fc695d66 100644 --- a/internal/scheduling/nova/filter_weigher_pipeline_controller.go +++ b/internal/scheduling/nova/filter_weigher_pipeline_controller.go @@ -178,13 +178,13 @@ func (c *FilterWeigherPipelineController) process(ctx context.Context, decision pipelineConf, ok := c.PipelineConfigs[decision.Spec.PipelineRef.Name] if !ok { log.Error(nil, "pipeline config not found", "pipelineName", decision.Spec.PipelineRef.Name) - return nil, errors.New("pipeline config not found") + return &request, errors.New("pipeline config not found") } if pipelineConf.Spec.IgnorePreselection { log.Info("gathering all placement candidates before filtering") if err := c.gatherer.MutateWithAllCandidates(ctx, &request); err != nil { log.Error(err, "failed to gather all placement candidates") - return nil, err + return &request, err } log.Info("gathered all placement candidates", "numHosts", len(request.Hosts)) } @@ -192,7 +192,7 @@ func (c *FilterWeigherPipelineController) process(ctx context.Context, decision result, err := pipeline.Run(request) if err != nil { log.Error(err, "failed to run pipeline") - return nil, err + return &request, err } decision.Status.Result = &result meta.SetStatusCondition(&decision.Status.Conditions, metav1.Condition{