From 81f472b64a099aeec0b3e67a19d34f94ff407d66 Mon Sep 17 00:00:00 2001 From: Malte <140147670+umswmayj@users.noreply.github.com> Date: Tue, 24 Mar 2026 14:55:00 +0100 Subject: [PATCH 01/12] Enable kvm_failover_evacuation weigher (#618) --- .../cortex-nova/templates/pipelines_kvm.yaml | 23 +++- helm/bundles/cortex-nova/values.yaml | 6 +- tools/visualize-reservations/main.go | 127 +++++++++++++++--- 3 files changed, 133 insertions(+), 23 deletions(-) diff --git a/helm/bundles/cortex-nova/templates/pipelines_kvm.yaml b/helm/bundles/cortex-nova/templates/pipelines_kvm.yaml index 865906e15..4273c0af3 100644 --- a/helm/bundles/cortex-nova/templates/pipelines_kvm.yaml +++ b/helm/bundles/cortex-nova/templates/pipelines_kvm.yaml @@ -125,6 +125,13 @@ spec: into the smallest gaps possible, it spreads the load to ensure workloads are balanced across hosts. In this pipeline, the balancing will focus on general purpose virtual machines. + - name: kvm_failover_evacuation + description: | + This weigher prefers hosts with active failover reservations during + evacuation requests. Hosts matching a failover reservation where the + VM is allocated get a higher weight, encouraging placement on + pre-reserved failover capacity. For non-evacuation requests, this + weigher has no effect. --- apiVersion: cortex.cloud/v1alpha1 kind: Pipeline @@ -248,6 +255,13 @@ spec: It pulls the requested vm into the smallest gaps possible, to ensure other hosts with less allocation stay free for bigger vms. In this pipeline, the binpacking will focus on hana virtual machines. + - name: kvm_failover_evacuation + description: | + This weigher prefers hosts with active failover reservations during + evacuation requests. Hosts matching a failover reservation where the + VM is allocated get a higher weight, encouraging placement on + pre-reserved failover capacity. For non-evacuation requests, this + weigher has no effect. --- apiVersion: cortex.cloud/v1alpha1 kind: Pipeline @@ -523,5 +537,12 @@ spec: This step will filter out hosts that do not meet the compute capabilities requested by the nova flavor extra specs, like `{"arch": "x86_64", "maxphysaddr:bits": 46, ...}`. - weighers: [] + weighers: + - name: kvm_failover_evacuation + description: | + This weigher prefers hosts with active failover reservations during + evacuation requests. Hosts matching a failover reservation where the + VM is allocated get a higher weight, encouraging placement on + pre-reserved failover capacity. For non-evacuation requests, this + weigher has no effect. {{- end }} diff --git a/helm/bundles/cortex-nova/values.yaml b/helm/bundles/cortex-nova/values.yaml index a7f7cc930..3409ceaf9 100644 --- a/helm/bundles/cortex-nova/values.yaml +++ b/helm/bundles/cortex-nova/values.yaml @@ -177,13 +177,13 @@ cortex-scheduling-controllers: # Maps flavor name patterns (glob) to required failover count # Example: {"hana_*": 2, "m1.xlarge": 1} flavorFailoverRequirements: - "*": 1 + "*": 2 # How often to check for missing failover reservations (periodic bulk reconciliation) - reconcileInterval: 15m + reconcileInterval: 5m # Used when maxVMsToProcess limits processing, allows faster catch-up and for the first reconcile shortReconcileInterval: 1m # Number of max VMs to process in one periodic reconciliation loop - maxVMsToProcess: 5 + maxVMsToProcess: 25 # Minimum successful reservations to use short interval minSuccessForShortInterval: 1 # Maximum failures allowed to still use short interval diff --git a/tools/visualize-reservations/main.go b/tools/visualize-reservations/main.go index 28d4dee23..6a21d551c 100644 --- a/tools/visualize-reservations/main.go +++ b/tools/visualize-reservations/main.go @@ -20,6 +20,9 @@ // --hide=view1,view2,... Comma-separated list of views to hide (applied after --views) // --filter-name=pattern Filter hypervisors by name (substring match) // --filter-trait=trait Filter hypervisors by trait (e.g., CUSTOM_HANA_EXCLUSIVE_HOST) +// --hypervisor-context=name Kubernetes context for reading Hypervisors (default: current context) +// --reservation-context=name Kubernetes context for reading Reservations (default: current context) +// --postgres-context=name Kubernetes context for reading postgres secret (default: current context) // // To connect to postgres when running locally, use kubectl port-forward: // @@ -45,6 +48,8 @@ import ( "k8s.io/apimachinery/pkg/runtime" utilruntime "k8s.io/apimachinery/pkg/util/runtime" clientgoscheme "k8s.io/client-go/kubernetes/scheme" + "k8s.io/client-go/rest" + "k8s.io/client-go/tools/clientcmd" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/config" ) @@ -177,6 +182,38 @@ func applyHideViews(views viewSet, hideFlag string) { } } +// getClientForContext creates a kubernetes client for the specified context. +// If contextName is empty, it uses the current/default context. +func getClientForContext(contextName string) (client.Client, error) { + var cfg *rest.Config + var err error + + if contextName == "" { + // Use default context + cfg, err = config.GetConfig() + if err != nil { + return nil, fmt.Errorf("getting default kubeconfig: %w", err) + } + } else { + // Use specified context + loadingRules := clientcmd.NewDefaultClientConfigLoadingRules() + configOverrides := &clientcmd.ConfigOverrides{ + CurrentContext: contextName, + } + kubeConfig := clientcmd.NewNonInteractiveDeferredLoadingClientConfig(loadingRules, configOverrides) + cfg, err = kubeConfig.ClientConfig() + if err != nil { + return nil, fmt.Errorf("getting kubeconfig for context %q: %w", contextName, err) + } + } + + k8sClient, err := client.New(cfg, client.Options{Scheme: scheme}) + if err != nil { + return nil, fmt.Errorf("creating client: %w", err) + } + return k8sClient, nil +} + func main() { // Parse command line flags sortBy := flag.String("sort", "vm", "Sort VMs by: vm (UUID), vm-host (VM's host), res-host (reservation host)") @@ -188,6 +225,9 @@ func main() { hideFlag := flag.String("hide", "", "Comma-separated list of views to hide (applied after --views)") filterName := flag.String("filter-name", "", "Filter hypervisors by name (substring match)") filterTrait := flag.String("filter-trait", "", "Filter hypervisors by trait (e.g., CUSTOM_HANA_EXCLUSIVE_HOST)") + hypervisorContext := flag.String("hypervisor-context", "", "Kubernetes context for reading Hypervisors (default: current context)") + reservationContext := flag.String("reservation-context", "", "Kubernetes context for reading Reservations (default: current context)") + postgresContext := flag.String("postgres-context", "", "Kubernetes context for reading postgres secret (default: current context)") flag.Parse() views := parseViews(*viewsFlag) @@ -195,17 +235,40 @@ func main() { ctx := context.Background() - // Create kubernetes client - cfg, err := config.GetConfig() + // Create kubernetes clients for hypervisors and reservations + // They may use different contexts if specified + hvClient, err := getClientForContext(*hypervisorContext) if err != nil { - fmt.Fprintf(os.Stderr, "Error getting kubeconfig: %v\n", err) + fmt.Fprintf(os.Stderr, "Error creating hypervisor client: %v\n", err) os.Exit(1) } - k8sClient, err := client.New(cfg, client.Options{Scheme: scheme}) - if err != nil { - fmt.Fprintf(os.Stderr, "Error creating client: %v\n", err) - os.Exit(1) + // Reuse the same client if contexts are the same, otherwise create a new one + var resClient client.Client + if *reservationContext == *hypervisorContext { + resClient = hvClient + } else { + resClient, err = getClientForContext(*reservationContext) + if err != nil { + fmt.Fprintf(os.Stderr, "Error creating reservation client: %v\n", err) + os.Exit(1) + } + } + + // Create postgres client (for reading the secret) + // This is typically the local cluster where cortex runs + var pgClient client.Client + switch *postgresContext { + case *hypervisorContext: + pgClient = hvClient + case *reservationContext: + pgClient = resClient + default: + pgClient, err = getClientForContext(*postgresContext) + if err != nil { + fmt.Fprintf(os.Stderr, "Error creating postgres client: %v\n", err) + os.Exit(1) + } } // Determine namespace @@ -214,19 +277,19 @@ func main() { ns = "default" // Default fallback } - // Try to connect to postgres + // Try to connect to postgres (use pgClient for reading the secret) var db *sql.DB var serverMap map[string]serverInfo var flavorMap map[string]flavorInfo - db, serverMap, flavorMap = connectToPostgres(ctx, k8sClient, *postgresSecret, ns, *postgresHostOverride, *postgresPortOverride) + db, serverMap, flavorMap = connectToPostgres(ctx, pgClient, *postgresSecret, ns, *postgresHostOverride, *postgresPortOverride, *postgresContext) if db != nil { defer db.Close() } - // Get all hypervisors to find all VMs + // Get all hypervisors to find all VMs (use hvClient) var allHypervisors hv1.HypervisorList - if err := k8sClient.List(ctx, &allHypervisors); err != nil { + if err := hvClient.List(ctx, &allHypervisors); err != nil { fmt.Fprintf(os.Stderr, "Error listing hypervisors: %v\n", err) return } @@ -270,9 +333,9 @@ func main() { } } - // Get all reservations (both failover and committed) + // Get all reservations (both failover and committed) (use resClient) var allReservations v1alpha1.ReservationList - if err := k8sClient.List(ctx, &allReservations); err != nil { + if err := resClient.List(ctx, &allReservations); err != nil { fmt.Fprintf(os.Stderr, "Error listing reservations: %v\n", err) return } @@ -946,6 +1009,24 @@ func main() { if views.has(viewSummary) { printHeader("Summary Statistics") + // Kubernetes context information + hvCtx := *hypervisorContext + if hvCtx == "" { + hvCtx = "(current context)" + } + resCtx := *reservationContext + if resCtx == "" { + resCtx = "(current context)" + } + pgCtx := *postgresContext + if pgCtx == "" { + pgCtx = "(current context)" + } + fmt.Printf("Hypervisor context: %s\n", hvCtx) + fmt.Printf("Reservation context: %s\n", resCtx) + fmt.Printf("Postgres context: %s\n", pgCtx) + fmt.Println() + // Database connection status if db != nil { fmt.Printf("Database: ✅ connected (servers: %d, flavors: %d)\n", len(serverMap), len(flavorMap)) @@ -1265,16 +1346,22 @@ func printHypervisorSummary(hypervisors []hv1.Hypervisor, reservations []v1alpha fmt.Println() } -func connectToPostgres(ctx context.Context, k8sClient client.Client, secretName, namespace, hostOverride, portOverride string) (db *sql.DB, serverMap map[string]serverInfo, flavorMap map[string]flavorInfo) { +func connectToPostgres(ctx context.Context, k8sClient client.Client, secretName, namespace, hostOverride, portOverride, contextName string) (db *sql.DB, serverMap map[string]serverInfo, flavorMap map[string]flavorInfo) { + ctxDisplay := contextName + if ctxDisplay == "" { + ctxDisplay = "(current context)" + } + fmt.Fprintf(os.Stderr, "Postgres: Reading secret '%s' from namespace '%s' using context '%s'\n", secretName, namespace, ctxDisplay) + // Get the postgres secret secret := &corev1.Secret{} if err := k8sClient.Get(ctx, client.ObjectKey{ Namespace: namespace, Name: secretName, }, secret); err != nil { - fmt.Fprintf(os.Stderr, "Warning: Could not get postgres secret '%s' in namespace '%s': %v\n", secretName, namespace, err) + fmt.Fprintf(os.Stderr, "Warning: Could not get postgres secret '%s' in namespace '%s' (context: %s): %v\n", secretName, namespace, ctxDisplay, err) fmt.Fprintf(os.Stderr, " Postgres features will be disabled.\n") - fmt.Fprintf(os.Stderr, " Use --postgres-secret and --namespace flags to specify the secret.\n\n") + fmt.Fprintf(os.Stderr, " Use --postgres-secret, --namespace, and --postgres-context flags to specify the secret location.\n\n") return nil, nil, nil } @@ -1641,7 +1728,11 @@ func printAllServers(serverMap map[string]serverInfo, _ map[string]flavorInfo, a } // Check if VM is in postgres - if server, ok := serverMap[uuid]; ok { + server, inPostgres := serverMap[uuid] + switch { + case !inPostgres: + info.Status = "NOT_IN_PG" + default: info.InPostgres = true info.PGHost = server.OSEXTSRVATTRHost if info.FlavorName == "" { @@ -1658,8 +1749,6 @@ func printAllServers(serverMap map[string]serverInfo, _ map[string]flavorInfo, a default: info.Status = "WRONG_HOST" } - } else { - info.Status = "NOT_IN_PG" } vms = append(vms, info) From eae02741d8f6c3b24b0c210ff833de5c1c66fdfd Mon Sep 17 00:00:00 2001 From: mblos <156897072+mblos@users.noreply.github.com> Date: Tue, 24 Mar 2026 14:57:17 +0100 Subject: [PATCH 02/12] Better committed resource logging (#620) --- api/v1alpha1/reservation_types.go | 7 ++ .../commitments/api_change_commitments.go | 78 +++++++++++-------- .../api_change_commitments_monitor.go | 19 ++++- .../api_change_commitments_monitor_test.go | 24 +++--- .../api_change_commitments_test.go | 33 +++++--- .../reservations/commitments/api_info.go | 11 ++- .../commitments/api_report_capacity.go | 19 ++++- .../api_report_capacity_monitor.go | 14 +++- .../commitments/api_report_usage.go | 15 +++- .../commitments/api_report_usage_monitor.go | 14 +++- .../reservations/commitments/config.go | 12 +++ .../reservations/commitments/controller.go | 66 +++++++--------- .../commitments/controller_test.go | 30 +++++-- .../commitments/reservation_manager.go | 52 +++++-------- .../reservations/commitments/state.go | 2 + 15 files changed, 265 insertions(+), 131 deletions(-) diff --git a/api/v1alpha1/reservation_types.go b/api/v1alpha1/reservation_types.go index 4a7fe5cf2..97950a395 100644 --- a/api/v1alpha1/reservation_types.go +++ b/api/v1alpha1/reservation_types.go @@ -36,6 +36,13 @@ const ( ReservationTypeLabelFailover = "failover" ) +// Annotation keys for Reservation metadata. +const ( + // AnnotationCreatorRequestID tracks the request ID that created this reservation. + // Used for end-to-end traceability across API calls, controller reconciles, and scheduler invocations. + AnnotationCreatorRequestID = "reservations.cortex.cloud/creator-request-id" +) + // CommittedResourceAllocation represents a workload's assignment to a committed resource reservation slot. // The workload could be a VM (Nova/IronCore), Pod (Kubernetes), or other resource. type CommittedResourceAllocation struct { diff --git a/internal/scheduling/reservations/commitments/api_change_commitments.go b/internal/scheduling/reservations/commitments/api_change_commitments.go index 1ef003e30..953319dbe 100644 --- a/internal/scheduling/reservations/commitments/api_change_commitments.go +++ b/internal/scheduling/reservations/commitments/api_change_commitments.go @@ -49,6 +49,13 @@ func (api *HTTPAPI) HandleChangeCommitments(w http.ResponseWriter, r *http.Reque req := liquid.CommitmentChangeRequest{} statusCode := http.StatusOK + // Extract or generate request ID for tracing - always set in response header + requestID := r.Header.Get("X-Request-ID") + if requestID == "" { + requestID = uuid.New().String() + } + w.Header().Set("X-Request-ID", requestID) + // Check if API is enabled if !api.config.EnableChangeCommitmentsAPI { statusCode = http.StatusServiceUnavailable @@ -61,11 +68,6 @@ func (api *HTTPAPI) HandleChangeCommitments(w http.ResponseWriter, r *http.Reque api.changeMutex.Lock() defer api.changeMutex.Unlock() - // Extract or generate request ID for tracing - requestID := r.Header.Get("X-Request-ID") - if requestID == "" { - requestID = uuid.New().String() - } ctx := reservations.WithGlobalRequestID(context.Background(), "committed-resource-"+requestID) logger := LoggerFromContext(ctx).WithValues("component", "api", "endpoint", "/v1/change-commitments") @@ -132,7 +134,7 @@ func (api *HTTPAPI) processCommitmentChanges(ctx context.Context, w http.Respons manager := NewReservationManager(api.client) requireRollback := false failedCommitments := make(map[string]string) // commitmentUUID to reason for failure, for better response messages in case of rollback - logger.Info("processing commitment change request", "availabilityZone", req.AZ, "dryRun", req.DryRun, "affectedProjects", len(req.ByProject)) + creatorRequestID := reservations.GlobalRequestIDFromContext(ctx) knowledge := &reservations.FlavorGroupKnowledgeClient{Client: api.client} flavorGroups, err := knowledge.GetAllFlavorGroups(ctx, nil) @@ -194,8 +196,7 @@ ProcessLoop: } for _, commitment := range resourceChanges.Commitments { - // Additional per-commitment validation if needed - logger.Info("processing commitment change", "commitmentUUID", commitment.UUID, "projectID", projectID, "resourceName", resourceName, "oldStatus", commitment.OldStatus.UnwrapOr("none"), "newStatus", commitment.NewStatus.UnwrapOr("none")) + logger.V(1).Info("processing commitment", "commitmentUUID", commitment.UUID, "oldStatus", commitment.OldStatus.UnwrapOr("none"), "newStatus", commitment.NewStatus.UnwrapOr("none")) // TODO add configurable upper limit validation for commitment size (number of instances) to prevent excessive reservation creation // TODO add domain @@ -247,8 +248,10 @@ ProcessLoop: requireRollback = true break ProcessLoop } + // Set creator request ID for traceability across controller reconciles + stateDesired.CreatorRequestID = creatorRequestID - logger.Info("applying commitment state change", "commitmentUUID", commitment.UUID, "oldState", stateBefore, "desiredState", stateDesired) + logger.V(1).Info("applying commitment state change", "commitmentUUID", commitment.UUID, "oldMemory", stateBefore.TotalMemoryBytes, "desiredMemory", stateDesired.TotalMemoryBytes) touchedReservations, deletedReservations, err := manager.ApplyCommitmentState(ctx, logger, stateDesired, flavorGroups, "changeCommitmentsApi") if err != nil { @@ -257,7 +260,7 @@ ProcessLoop: requireRollback = true break ProcessLoop } - logger.Info("applied commitment state change", "commitmentUUID", commitment.UUID, "touchedReservations", len(touchedReservations), "deletedReservations", len(deletedReservations)) + logger.V(1).Info("applied commitment state change", "commitmentUUID", commitment.UUID, "touchedReservations", len(touchedReservations), "deletedReservations", len(deletedReservations)) reservationsToWatch = append(reservationsToWatch, touchedReservations...) } } @@ -318,6 +321,7 @@ ProcessLoop: } // watchReservationsUntilReady polls until all reservations reach Ready=True or timeout. +// Returns failed reservations and any errors encountered. func watchReservationsUntilReady( ctx context.Context, logger logr.Logger, @@ -332,19 +336,32 @@ func watchReservationsUntilReady( } deadline := time.Now().Add(timeout) + startTime := time.Now() + totalReservations := len(reservations) reservationsToWatch := make([]v1alpha1.Reservation, len(reservations)) copy(reservationsToWatch, reservations) + // Track successful reservations for summary + var successfulReservations []string + pollCount := 0 + for { + pollCount++ var stillWaiting []v1alpha1.Reservation if time.Now().After(deadline) { errors = append(errors, fmt.Errorf("timeout after %v waiting for reservations to become ready", timeout)) + // Log summary on timeout + logger.Info("reservation watch completed (timeout)", + "total", totalReservations, + "ready", len(successfulReservations), + "failed", len(failedReservations), + "timedOut", len(reservationsToWatch), + "duration", time.Since(startTime).Round(time.Millisecond), + "polls", pollCount) return failedReservations, errors } - allAreReady := true - for _, res := range reservationsToWatch { // Fetch current state var current v1alpha1.Reservation @@ -354,9 +371,7 @@ func watchReservationsUntilReady( } if err := k8sClient.Get(ctx, nn, ¤t); err != nil { - allAreReady = false - // Reservation is still in process of being created, or there is a transient error, continue waiting for it - logger.V(1).Info("transient error getting reservation, will retry", "reservation", res.Name, "error", err) + // Reservation is still in process of being created, or there is a transient error stillWaiting = append(stillWaiting, res) continue } @@ -369,43 +384,40 @@ func watchReservationsUntilReady( if readyCond == nil { // Condition not set yet, keep waiting - allAreReady = false stillWaiting = append(stillWaiting, res) continue } switch readyCond.Status { case metav1.ConditionTrue: - // check if host is not set in spec or status: if so, no capacity left to schedule the reservation + // Only consider truly ready if Status.Host is populated if current.Spec.TargetHost == "" || current.Status.Host == "" { - allAreReady = false - failedReservations = append(failedReservations, current) - logger.Info("insufficient capacity for reservation", "reservation", current.Name, "reason", readyCond.Reason, "message", readyCond.Message, "targetHostInSpec", current.Spec.TargetHost, "hostInStatus", current.Status.Host) - } else { - // Reservation is successfully scheduled, no further action needed - logger.Info("reservation ready", "reservation", current.Name, "host", current.Spec.TargetHost) + stillWaiting = append(stillWaiting, res) + continue } + // Reservation is successfully scheduled - track for summary + successfulReservations = append(successfulReservations, current.Name) case metav1.ConditionFalse: - failedReservations = append(failedReservations, res) + // Any failure reason counts as failed + failedReservations = append(failedReservations, current) case metav1.ConditionUnknown: - allAreReady = false stillWaiting = append(stillWaiting, res) } } - if allAreReady || len(stillWaiting) == 0 { - logger.Info("all reservations checked", - "failed", len(failedReservations)) + if len(stillWaiting) == 0 { + // All reservations have reached a terminal state - log summary + logger.Info("reservation watch completed", + "total", totalReservations, + "ready", len(successfulReservations), + "failed", len(failedReservations), + "duration", time.Since(startTime).Round(time.Millisecond), + "polls", pollCount) return failedReservations, errors } reservationsToWatch = stillWaiting - // Log progress - logger.V(1).Info("waiting for reservations to become ready", - "notReady", len(reservationsToWatch), - "total", len(reservations), - "timeRemaining", time.Until(deadline).Round(time.Second)) // Wait before next poll select { diff --git a/internal/scheduling/reservations/commitments/api_change_commitments_monitor.go b/internal/scheduling/reservations/commitments/api_change_commitments_monitor.go index 30a09d49e..d8522008b 100644 --- a/internal/scheduling/reservations/commitments/api_change_commitments_monitor.go +++ b/internal/scheduling/reservations/commitments/api_change_commitments_monitor.go @@ -16,8 +16,10 @@ type ChangeCommitmentsAPIMonitor struct { } // NewChangeCommitmentsAPIMonitor creates a new monitor with Prometheus metrics. +// Metrics are pre-initialized with zero values for common HTTP status codes +// to ensure they appear in Prometheus before the first request. func NewChangeCommitmentsAPIMonitor() ChangeCommitmentsAPIMonitor { - return ChangeCommitmentsAPIMonitor{ + m := ChangeCommitmentsAPIMonitor{ requestCounter: prometheus.NewCounterVec(prometheus.CounterOpts{ Name: "cortex_committed_resource_change_api_requests_total", Help: "Total number of committed resource change API requests by HTTP status code", @@ -35,6 +37,21 @@ func NewChangeCommitmentsAPIMonitor() ChangeCommitmentsAPIMonitor { Help: "Total number of commitment change requests that timed out while waiting for reservations to become ready", }), } + + // Pre-initialize metrics with zero values for common HTTP status codes. + // This ensures metrics exist in Prometheus before the first request, + // preventing "metric missing" warnings in alerting rules. + for _, statusCode := range []string{"200", "400", "409", "500", "503"} { + m.requestCounter.WithLabelValues(statusCode) + m.requestDuration.WithLabelValues(statusCode) + } + + // Pre-initialize commitment change result labels + for _, result := range []string{"accepted", "rejected"} { + m.commitmentChanges.WithLabelValues(result) + } + + return m } // Describe implements prometheus.Collector. diff --git a/internal/scheduling/reservations/commitments/api_change_commitments_monitor_test.go b/internal/scheduling/reservations/commitments/api_change_commitments_monitor_test.go index 322d01aae..255f05bd8 100644 --- a/internal/scheduling/reservations/commitments/api_change_commitments_monitor_test.go +++ b/internal/scheduling/reservations/commitments/api_change_commitments_monitor_test.go @@ -102,11 +102,13 @@ func TestChangeCommitmentsAPIMonitor_MetricLabels(t *testing.T) { // Verify request counter has correct labels for _, family := range families { if *family.Name == "cortex_committed_resource_change_api_requests_total" { - if len(family.Metric) != 3 { - t.Errorf("Expected 3 request counter metrics, got %d", len(family.Metric)) + // At minimum we expect the 3 labels we added (200, 409, 503) + // Plus pre-initialized labels (400, 500) - so >= 5 total + if len(family.Metric) < 3 { + t.Errorf("Expected at least 3 request counter metrics, got %d", len(family.Metric)) } - // Check label names + // Check all metrics have the status_code label for _, metric := range family.Metric { labelNames := make(map[string]bool) for _, label := range metric.Label { @@ -120,11 +122,13 @@ func TestChangeCommitmentsAPIMonitor_MetricLabels(t *testing.T) { } if *family.Name == "cortex_committed_resource_change_api_request_duration_seconds" { - if len(family.Metric) != 1 { - t.Errorf("Expected 1 histogram metric, got %d", len(family.Metric)) + // At minimum we expect the label we used (200) + // Plus pre-initialized labels - so >= 1 total + if len(family.Metric) < 1 { + t.Errorf("Expected at least 1 histogram metric, got %d", len(family.Metric)) } - // Check label names + // Check all metrics have the status_code label for _, metric := range family.Metric { labelNames := make(map[string]bool) for _, label := range metric.Label { @@ -138,11 +142,13 @@ func TestChangeCommitmentsAPIMonitor_MetricLabels(t *testing.T) { } if *family.Name == "cortex_committed_resource_change_api_commitment_changes_total" { - if len(family.Metric) != 2 { - t.Errorf("Expected 2 commitment changes metrics, got %d", len(family.Metric)) + // At minimum we expect the 2 labels we added (success, rejected) + // Plus pre-initialized labels (accepted) - so >= 2 total + if len(family.Metric) < 2 { + t.Errorf("Expected at least 2 commitment changes metrics, got %d", len(family.Metric)) } - // Check label names + // Check all metrics have the result label for _, metric := range family.Metric { labelNames := make(map[string]bool) for _, label := range metric.Label { diff --git a/internal/scheduling/reservations/commitments/api_change_commitments_test.go b/internal/scheduling/reservations/commitments/api_change_commitments_test.go index 4d378af55..d996c498e 100644 --- a/internal/scheduling/reservations/commitments/api_change_commitments_test.go +++ b/internal/scheduling/reservations/commitments/api_change_commitments_test.go @@ -1277,7 +1277,8 @@ func (env *CommitmentTestEnv) processNewReservation(res *v1alpha1.Reservation) { } } -// markReservationSchedulerProcessedStatus updates a reservation to have Ready=True status (scheduling can be succeeded or not - depending on host status) +// markReservationSchedulerProcessedStatus updates a reservation status based on scheduling result. +// If host is non-empty, sets Ready=True (success). If host is empty, sets Ready=False with NoHostsFound (failure). func (env *CommitmentTestEnv) markReservationSchedulerProcessedStatus(res *v1alpha1.Reservation, host string) { ctx := context.Background() @@ -1288,16 +1289,28 @@ func (env *CommitmentTestEnv) markReservationSchedulerProcessedStatus(res *v1alp return } - // Then update status + // Then update status - Ready=True only if host was found, Ready=False otherwise res.Status.Host = host - res.Status.Conditions = []metav1.Condition{ - { - Type: v1alpha1.ReservationConditionReady, - Status: metav1.ConditionTrue, - Reason: "ReservationActive", - Message: "Reservation is ready (set by test controller)", - LastTransitionTime: metav1.Now(), - }, + if host != "" { + res.Status.Conditions = []metav1.Condition{ + { + Type: v1alpha1.ReservationConditionReady, + Status: metav1.ConditionTrue, + Reason: "ReservationActive", + Message: "Reservation is ready (set by test controller)", + LastTransitionTime: metav1.Now(), + }, + } + } else { + res.Status.Conditions = []metav1.Condition{ + { + Type: v1alpha1.ReservationConditionReady, + Status: metav1.ConditionFalse, + Reason: "NoHostsFound", + Message: "No hosts with sufficient capacity (set by test controller)", + LastTransitionTime: metav1.Now(), + }, + } } if err := env.K8sClient.Status().Update(ctx, res); err != nil { env.T.Logf("Warning: Failed to update reservation status: %v", err) diff --git a/internal/scheduling/reservations/commitments/api_info.go b/internal/scheduling/reservations/commitments/api_info.go index 71a84feb4..c189c859a 100644 --- a/internal/scheduling/reservations/commitments/api_info.go +++ b/internal/scheduling/reservations/commitments/api_info.go @@ -12,6 +12,7 @@ import ( "github.com/cobaltcore-dev/cortex/internal/scheduling/reservations" "github.com/go-logr/logr" + "github.com/google/uuid" liquid "github.com/sapcc/go-api-declarations/liquid" ) @@ -19,7 +20,15 @@ import ( // See: https://github.com/sapcc/go-api-declarations/blob/main/liquid/commitment.go // See: https://pkg.go.dev/github.com/sapcc/go-api-declarations/liquid func (api *HTTPAPI) HandleInfo(w http.ResponseWriter, r *http.Request) { - ctx := WithNewGlobalRequestID(r.Context()) + // Extract or generate request ID for tracing + requestID := r.Header.Get("X-Request-ID") + if requestID == "" { + requestID = uuid.New().String() + } + // Set request ID in response header for client correlation + w.Header().Set("X-Request-ID", requestID) + + ctx := reservations.WithGlobalRequestID(r.Context(), "committed-resource-"+requestID) logger := LoggerFromContext(ctx).WithValues("component", "api", "endpoint", "/v1/info") // Only accept GET method diff --git a/internal/scheduling/reservations/commitments/api_report_capacity.go b/internal/scheduling/reservations/commitments/api_report_capacity.go index 2f7618ced..19b7fb24c 100644 --- a/internal/scheduling/reservations/commitments/api_report_capacity.go +++ b/internal/scheduling/reservations/commitments/api_report_capacity.go @@ -9,6 +9,8 @@ import ( "strconv" "time" + "github.com/cobaltcore-dev/cortex/internal/scheduling/reservations" + "github.com/google/uuid" "github.com/sapcc/go-api-declarations/liquid" ) @@ -20,7 +22,22 @@ func (api *HTTPAPI) HandleReportCapacity(w http.ResponseWriter, r *http.Request) startTime := time.Now() statusCode := http.StatusOK - ctx := WithNewGlobalRequestID(r.Context()) + // Extract or generate request ID for tracing - always set in response header + requestID := r.Header.Get("X-Request-ID") + if requestID == "" { + requestID = uuid.New().String() + } + w.Header().Set("X-Request-ID", requestID) + + // Check if API is enabled + if !api.config.EnableReportCapacityAPI { + statusCode = http.StatusServiceUnavailable + http.Error(w, "report-capacity API is disabled", statusCode) + api.recordCapacityMetrics(statusCode, startTime) + return + } + + ctx := reservations.WithGlobalRequestID(r.Context(), "committed-resource-"+requestID) logger := LoggerFromContext(ctx).WithValues("component", "api", "endpoint", "/v1/commitments/report-capacity") // Only accept POST method diff --git a/internal/scheduling/reservations/commitments/api_report_capacity_monitor.go b/internal/scheduling/reservations/commitments/api_report_capacity_monitor.go index d484b6f27..d78af6cc7 100644 --- a/internal/scheduling/reservations/commitments/api_report_capacity_monitor.go +++ b/internal/scheduling/reservations/commitments/api_report_capacity_monitor.go @@ -14,8 +14,10 @@ type ReportCapacityAPIMonitor struct { } // NewReportCapacityAPIMonitor creates a new monitor with Prometheus metrics. +// Metrics are pre-initialized with zero values for common HTTP status codes +// to ensure they appear in Prometheus before the first request. func NewReportCapacityAPIMonitor() ReportCapacityAPIMonitor { - return ReportCapacityAPIMonitor{ + m := ReportCapacityAPIMonitor{ requestCounter: prometheus.NewCounterVec(prometheus.CounterOpts{ Name: "cortex_committed_resource_capacity_api_requests_total", Help: "Total number of committed resource capacity API requests by HTTP status code", @@ -26,6 +28,16 @@ func NewReportCapacityAPIMonitor() ReportCapacityAPIMonitor { Buckets: []float64{0.1, 0.25, 0.5, 1, 2.5, 5, 10}, }, []string{"status_code"}), } + + // Pre-initialize metrics with zero values for common HTTP status codes. + // This ensures metrics exist in Prometheus before the first request, + // preventing "metric missing" warnings in alerting rules. + for _, statusCode := range []string{"200", "500", "503"} { + m.requestCounter.WithLabelValues(statusCode) + m.requestDuration.WithLabelValues(statusCode) + } + + return m } // Describe implements prometheus.Collector. diff --git a/internal/scheduling/reservations/commitments/api_report_usage.go b/internal/scheduling/reservations/commitments/api_report_usage.go index 86b514d87..558758bab 100644 --- a/internal/scheduling/reservations/commitments/api_report_usage.go +++ b/internal/scheduling/reservations/commitments/api_report_usage.go @@ -11,6 +11,7 @@ import ( "strings" "time" + "github.com/google/uuid" "github.com/sapcc/go-api-declarations/liquid" ) @@ -24,12 +25,24 @@ func (api *HTTPAPI) HandleReportUsage(w http.ResponseWriter, r *http.Request) { startTime := time.Now() statusCode := http.StatusOK + // Extract or generate request ID for tracing - always set in response header requestID := r.Header.Get("X-Request-ID") if requestID == "" { - requestID = fmt.Sprintf("req-%d", time.Now().UnixNano()) + requestID = uuid.New().String() } + w.Header().Set("X-Request-ID", requestID) + log := baseLog.WithValues("requestID", requestID, "endpoint", "report-usage") + // Check if API is enabled + if !api.config.EnableReportUsageAPI { + statusCode = http.StatusServiceUnavailable + log.Info("report-usage API is disabled, rejecting request") + http.Error(w, "report-usage API is disabled", statusCode) + api.recordUsageMetrics(statusCode, startTime) + return + } + if r.Method != http.MethodPost { statusCode = http.StatusMethodNotAllowed http.Error(w, "Method not allowed", statusCode) diff --git a/internal/scheduling/reservations/commitments/api_report_usage_monitor.go b/internal/scheduling/reservations/commitments/api_report_usage_monitor.go index d3fc68018..bbcaaaf86 100644 --- a/internal/scheduling/reservations/commitments/api_report_usage_monitor.go +++ b/internal/scheduling/reservations/commitments/api_report_usage_monitor.go @@ -14,8 +14,10 @@ type ReportUsageAPIMonitor struct { } // NewReportUsageAPIMonitor creates a new monitor with Prometheus metrics. +// Metrics are pre-initialized with zero values for common HTTP status codes +// to ensure they appear in Prometheus before the first request. func NewReportUsageAPIMonitor() ReportUsageAPIMonitor { - return ReportUsageAPIMonitor{ + m := ReportUsageAPIMonitor{ requestCounter: prometheus.NewCounterVec(prometheus.CounterOpts{ Name: "cortex_committed_resource_usage_api_requests_total", Help: "Total number of committed resource usage API requests by HTTP status code", @@ -26,6 +28,16 @@ func NewReportUsageAPIMonitor() ReportUsageAPIMonitor { Buckets: []float64{0.1, 0.25, 0.5, 1, 2.5, 5, 10}, }, []string{"status_code"}), } + + // Pre-initialize metrics with zero values for common HTTP status codes. + // This ensures metrics exist in Prometheus before the first request, + // preventing "metric missing" warnings in alerting rules. + for _, statusCode := range []string{"200", "400", "404", "500", "503"} { + m.requestCounter.WithLabelValues(statusCode) + m.requestDuration.WithLabelValues(statusCode) + } + + return m } // Describe implements prometheus.Collector. diff --git a/internal/scheduling/reservations/commitments/config.go b/internal/scheduling/reservations/commitments/config.go index 0004cf19b..98a5d59ae 100644 --- a/internal/scheduling/reservations/commitments/config.go +++ b/internal/scheduling/reservations/commitments/config.go @@ -47,6 +47,16 @@ type Config struct { // When false, the endpoint will return HTTP 503 Service Unavailable. // The info endpoint remains available for health checks. EnableChangeCommitmentsAPI bool `json:"committedResourceEnableChangeCommitmentsAPI"` + + // EnableReportUsageAPI controls whether the report-usage API endpoint is active. + // When false, the endpoint will return HTTP 503 Service Unavailable. + // This can be used as an emergency switch if the usage reporting is causing issues. + EnableReportUsageAPI bool `json:"committedResourceEnableReportUsageAPI"` + + // EnableReportCapacityAPI controls whether the report-capacity API endpoint is active. + // When false, the endpoint will return HTTP 503 Service Unavailable. + // This can be used as an emergency switch if the capacity reporting is causing issues. + EnableReportCapacityAPI bool `json:"committedResourceEnableReportCapacityAPI"` } func DefaultConfig() Config { @@ -58,5 +68,7 @@ func DefaultConfig() Config { ChangeAPIWatchReservationsTimeout: 10 * time.Second, ChangeAPIWatchReservationsPollInterval: 500 * time.Millisecond, EnableChangeCommitmentsAPI: true, + EnableReportUsageAPI: true, + EnableReportCapacityAPI: true, } } diff --git a/internal/scheduling/reservations/commitments/controller.go b/internal/scheduling/reservations/commitments/controller.go index f68e35cde..e03958151 100644 --- a/internal/scheduling/reservations/commitments/controller.go +++ b/internal/scheduling/reservations/commitments/controller.go @@ -50,7 +50,8 @@ type CommitmentReservationController struct { // Note: This controller only handles commitment reservations, as filtered by the predicate. func (r *CommitmentReservationController) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { ctx = WithNewGlobalRequestID(ctx) - logger := LoggerFromContext(ctx).WithValues("component", "controller", "reservation", req.Name, "namespace", req.Namespace) + logger := LoggerFromContext(ctx).WithValues("component", "controller", "reservation", req.Name) + // Fetch the reservation object. var res v1alpha1.Reservation if err := r.Get(ctx, req.NamespacedName, &res); err != nil { @@ -58,6 +59,11 @@ func (r *CommitmentReservationController) Reconcile(ctx context.Context, req ctr return ctrl.Result{}, client.IgnoreNotFound(err) } + // Extract creator request ID from annotation for end-to-end traceability + if creatorReq := res.Annotations[v1alpha1.AnnotationCreatorRequestID]; creatorReq != "" { + logger = logger.WithValues("creatorReq", creatorReq) + } + // filter for CR reservations resourceName := "" if res.Spec.CommittedResourceReservation != nil { @@ -86,7 +92,7 @@ func (r *CommitmentReservationController) Reconcile(ctx context.Context, req ctr } if meta.IsStatusConditionTrue(res.Status.Conditions, v1alpha1.ReservationConditionReady) { - logger.Info("reservation is active, verifying allocations") + logger.V(1).Info("reservation is active, verifying allocations") // Verify all allocations in Spec against actual VM state from database if err := r.reconcileAllocations(ctx, &res); err != nil { @@ -135,13 +141,17 @@ func (r *CommitmentReservationController) Reconcile(ctx context.Context, req ctr // Sync Spec values to Status fields for non-pre-allocated reservations // This ensures the observed state reflects the desired state from Spec - needsStatusUpdate := false + // When TargetHost is set in Spec but not synced to Status, this means + // the scheduler found a host and we need to mark the reservation as ready. if res.Spec.TargetHost != "" && res.Status.Host != res.Spec.TargetHost { - res.Status.Host = res.Spec.TargetHost - needsStatusUpdate = true - } - if needsStatusUpdate { old := res.DeepCopy() + res.Status.Host = res.Spec.TargetHost + meta.SetStatusCondition(&res.Status.Conditions, metav1.Condition{ + Type: v1alpha1.ReservationConditionReady, + Status: metav1.ConditionTrue, + Reason: "ReservationActive", + Message: "reservation is successfully scheduled", + }) patch := client.MergeFrom(old) if err := r.Status().Patch(ctx, &res, patch); err != nil { // Ignore not-found errors during background deletion @@ -152,7 +162,9 @@ func (r *CommitmentReservationController) Reconcile(ctx context.Context, req ctr // Object was deleted, no need to continue return ctrl.Result{}, nil } - logger.Info("synced spec to status", "host", res.Status.Host) + logger.Info("synced spec to status and marked ready", "host", res.Status.Host) + // Return and let next reconcile handle allocation verification + return ctrl.Result{}, nil } // Get project ID from CommittedResourceReservation spec if available. @@ -259,7 +271,7 @@ func (r *CommitmentReservationController) Reconcile(ctx context.Context, req ctr } if len(scheduleResp.Hosts) == 0 { - logger.Info("no hosts found for reservation") + logger.Info("no hosts found for reservation", "reservation", res.Name, "flavorName", resourceName) old := res.DeepCopy() meta.SetStatusCondition(&res.Status.Conditions, metav1.Condition{ Type: v1alpha1.ReservationConditionReady, @@ -280,11 +292,12 @@ func (r *CommitmentReservationController) Reconcile(ctx context.Context, req ctr return ctrl.Result{}, nil // No need to requeue, we didn't find a host. } - // Update the reservation with the found host (idx 0) + // Update the reservation Spec with the found host (idx 0) + // Only update Spec here - the Status will be synced in the next reconcile cycle + // This avoids race conditions from doing two patches in one reconcile host := scheduleResp.Hosts[0] logger.Info("found host for reservation", "host", host) - // First update Spec old := res.DeepCopy() res.Spec.TargetHost = host if err := r.Patch(ctx, &res, client.MergeFrom(old)); err != nil { @@ -297,26 +310,9 @@ func (r *CommitmentReservationController) Reconcile(ctx context.Context, req ctr return ctrl.Result{}, nil } - // Then update Status - old = res.DeepCopy() - meta.SetStatusCondition(&res.Status.Conditions, metav1.Condition{ - Type: v1alpha1.ReservationConditionReady, - Status: metav1.ConditionTrue, - Reason: "ReservationActive", - Message: "reservation is successfully scheduled", - }) - res.Status.Host = host - patch := client.MergeFrom(old) - if err := r.Status().Patch(ctx, &res, patch); err != nil { - // Ignore not-found errors during background deletion - if client.IgnoreNotFound(err) != nil { - logger.Error(err, "failed to patch reservation status") - return ctrl.Result{}, err - } - // Object was deleted, no need to continue - return ctrl.Result{}, nil - } - return ctrl.Result{}, nil // No need to requeue, the reservation is now active. + // The Spec patch will trigger a re-reconcile, which will sync Status in the + // "Sync Spec values to Status" section above + return ctrl.Result{}, nil } // reconcileAllocations verifies all allocations in Spec against actual Nova VM state. @@ -333,7 +329,7 @@ func (r *CommitmentReservationController) reconcileAllocations(ctx context.Conte // Skip if no allocations to verify if len(res.Spec.CommittedResourceReservation.Allocations) == 0 { - logger.Info("no allocations to verify", "reservation", res.Name) + logger.V(1).Info("no allocations to verify", "reservation", res.Name) return nil } @@ -359,9 +355,8 @@ func (r *CommitmentReservationController) reconcileAllocations(ctx context.Conte actualHost := server.OSEXTSRVATTRHost newStatusAllocations[vmUUID] = actualHost - logger.Info("verified VM allocation", + logger.V(1).Info("verified VM allocation", "vm", vmUUID, - "reservation", res.Name, "actualHost", actualHost, "expectedHost", res.Status.Host) } else { @@ -391,8 +386,7 @@ func (r *CommitmentReservationController) reconcileAllocations(ctx context.Conte return fmt.Errorf("failed to patch reservation status: %w", err) } - logger.Info("reconciled allocations", - "reservation", res.Name, + logger.V(1).Info("reconciled allocations", "specAllocations", len(res.Spec.CommittedResourceReservation.Allocations), "statusAllocations", len(newStatusAllocations)) diff --git a/internal/scheduling/reservations/commitments/controller_test.go b/internal/scheduling/reservations/commitments/controller_test.go index 7433165a9..5af5dfca9 100644 --- a/internal/scheduling/reservations/commitments/controller_test.go +++ b/internal/scheduling/reservations/commitments/controller_test.go @@ -292,21 +292,39 @@ func TestCommitmentReservationController_reconcileInstanceReservation_Success(t }, } + // First reconcile: schedules the reservation and sets Spec.TargetHost result, err := reconciler.Reconcile(context.Background(), req) - if err != nil { - t.Errorf("reconcileInstanceReservation() error = %v", err) + t.Errorf("First reconcile error = %v", err) return } + if result.RequeueAfter > 0 { + t.Errorf("Expected no requeue after first reconcile but got %v", result.RequeueAfter) + } + // Verify Spec.TargetHost is set after first reconcile + var afterFirstReconcile v1alpha1.Reservation + if err = client.Get(context.Background(), req.NamespacedName, &afterFirstReconcile); err != nil { + t.Errorf("Failed to get reservation after first reconcile: %v", err) + return + } + if afterFirstReconcile.Spec.TargetHost != "test-host-1" { + t.Errorf("Expected Spec.TargetHost=%v after first reconcile, got %v", "test-host-1", afterFirstReconcile.Spec.TargetHost) + } + + // Second reconcile: syncs Spec.TargetHost to Status and sets Ready=True + result, err = reconciler.Reconcile(context.Background(), req) + if err != nil { + t.Errorf("Second reconcile error = %v", err) + return + } if result.RequeueAfter > 0 { - t.Errorf("Expected no requeue but got %v", result.RequeueAfter) + t.Errorf("Expected no requeue after second reconcile but got %v", result.RequeueAfter) } - // Verify the reservation status + // Verify the reservation status after second reconcile var updated v1alpha1.Reservation - err = client.Get(context.Background(), req.NamespacedName, &updated) - if err != nil { + if err = client.Get(context.Background(), req.NamespacedName, &updated); err != nil { t.Errorf("Failed to get updated reservation: %v", err) return } diff --git a/internal/scheduling/reservations/commitments/reservation_manager.go b/internal/scheduling/reservations/commitments/reservation_manager.go index 773f74122..40cfa08ef 100644 --- a/internal/scheduling/reservations/commitments/reservation_manager.go +++ b/internal/scheduling/reservations/commitments/reservation_manager.go @@ -82,18 +82,15 @@ func (m *ReservationManager) ApplyCommitmentState( deltaMemoryBytes -= memoryQuantity.Value() } - log.Info("applying commitment state", - "commitmentUUID", desiredState.CommitmentUUID, - "desiredMemoryBytes", desiredState.TotalMemoryBytes, - "deltaMemoryBytes", deltaMemoryBytes, - "existingSlots", len(existing), - ) + // Log only if there's actual work to do (delta != 0) + hasChanges := deltaMemoryBytes != 0 nextSlotIndex := GetNextSlotIndex(existing) // Phase 3 (DELETE): Delete inconsistent reservations (wrong flavor group/project) // They will be recreated with correct metadata in subsequent phases. var validReservations []v1alpha1.Reservation + var repairedCount int for _, res := range existing { if res.Spec.CommittedResourceReservation.ResourceGroup != desiredState.FlavorGroupName || res.Spec.CommittedResourceReservation.ProjectID != desiredState.ProjectID { @@ -104,6 +101,7 @@ func (m *ReservationManager) ApplyCommitmentState( "actualFlavorGroup", res.Spec.CommittedResourceReservation.ResourceGroup, "expectedProjectID", desiredState.ProjectID, "actualProjectID", res.Spec.CommittedResourceReservation.ProjectID) + repairedCount++ removedReservations = append(removedReservations, res) memValue := res.Spec.Resources[hv1.ResourceMemory] deltaMemoryBytes += memValue.Value() @@ -145,19 +143,13 @@ func (m *ReservationManager) ApplyCommitmentState( memValue := reservationToDelete.Spec.Resources[hv1.ResourceMemory] deltaMemoryBytes += memValue.Value() - log.Info("deleting reservation (capacity decrease)", - "commitmentUUID", desiredState.CommitmentUUID, - "deltaMemoryBytes", deltaMemoryBytes, - "name", reservationToDelete.Name, - "numAllocations", len(reservationToDelete.Spec.CommittedResourceReservation.Allocations), - "memoryBytes", memValue.Value()) - if err := m.Delete(ctx, reservationToDelete); err != nil { return touchedReservations, removedReservations, fmt.Errorf("failed to delete reservation %s: %w", reservationToDelete.Name, err) } } // Phase 5 (CREATE): Create new reservations (capacity increased) + var createdCount int for deltaMemoryBytes > 0 { // Need to create new reservation slots, always prefer largest flavor within the group // TODO more sophisticated flavor selection, especially with flavors of different cpu/memory ratio @@ -165,13 +157,7 @@ func (m *ReservationManager) ApplyCommitmentState( touchedReservations = append(touchedReservations, *reservation) memValue := reservation.Spec.Resources[hv1.ResourceMemory] deltaMemoryBytes -= memValue.Value() - - log.Info("creating reservation", - "commitmentUUID", desiredState.CommitmentUUID, - "deltaMemoryBytes", deltaMemoryBytes, - "flavorName", reservation.Spec.CommittedResourceReservation.ResourceName, - "name", reservation.Name, - "memoryBytes", memValue.Value()) + createdCount++ if err := m.Create(ctx, reservation); err != nil { if apierrors.IsAlreadyExists(err) { @@ -198,11 +184,15 @@ func (m *ReservationManager) ApplyCommitmentState( } } - log.Info("completed commitment state sync", - "commitmentUUID", desiredState.CommitmentUUID, - "totalReservations", len(existing), - "created", len(touchedReservations)-len(existing), - "deleted", len(removedReservations)) + // Only log if there were actual changes + if hasChanges || createdCount > 0 || len(removedReservations) > 0 || repairedCount > 0 { + log.Info("commitment state sync completed", + "commitmentUUID", desiredState.CommitmentUUID, + "created", createdCount, + "deleted", len(removedReservations), + "repaired", repairedCount, + "total", len(existing)+createdCount) + } return touchedReservations, removedReservations, nil } @@ -221,12 +211,9 @@ func (m *ReservationManager) syncReservationMetadata( (state.StartTime != nil && (reservation.Spec.StartTime == nil || !reservation.Spec.StartTime.Time.Equal(*state.StartTime))) || (state.EndTime != nil && (reservation.Spec.EndTime == nil || !reservation.Spec.EndTime.Time.Equal(*state.EndTime))) { // Apply patch - logger.Info("syncing reservation metadata", - "reservation", reservation, - "desired commitmentUUID", state.CommitmentUUID, - "desired availabilityZone", state.AvailabilityZone, - "desired startTime", state.StartTime, - "desired endTime", state.EndTime) + logger.V(1).Info("syncing reservation metadata", + "reservation", reservation.Name, + "commitmentUUID", state.CommitmentUUID) patch := client.MergeFrom(reservation.DeepCopy()) @@ -322,6 +309,9 @@ func (m *ReservationManager) newReservation( Labels: map[string]string{ v1alpha1.LabelReservationType: v1alpha1.ReservationTypeLabelCommittedResource, }, + Annotations: map[string]string{ + v1alpha1.AnnotationCreatorRequestID: state.CreatorRequestID, + }, }, Spec: spec, } diff --git a/internal/scheduling/reservations/commitments/state.go b/internal/scheduling/reservations/commitments/state.go index dbc3ad72d..c8e75ff83 100644 --- a/internal/scheduling/reservations/commitments/state.go +++ b/internal/scheduling/reservations/commitments/state.go @@ -46,6 +46,8 @@ type CommitmentState struct { StartTime *time.Time // EndTime is when the commitment expires EndTime *time.Time + // CreatorRequestID is the request ID that triggered this state change (for traceability) + CreatorRequestID string } // FromCommitment converts Limes commitment to CommitmentState. From 556236f4bf04bd5b18586725b6fc88ab65d254eb Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Tue, 24 Mar 2026 14:06:43 +0000 Subject: [PATCH 03/12] Bump cortex chart appVersions to sha-eae02741 [skip ci] --- helm/library/cortex/Chart.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/helm/library/cortex/Chart.yaml b/helm/library/cortex/Chart.yaml index 53d9e17e3..9b8e7d2bf 100644 --- a/helm/library/cortex/Chart.yaml +++ b/helm/library/cortex/Chart.yaml @@ -3,6 +3,6 @@ name: cortex description: A Helm chart to distribute cortex. type: application version: 0.0.30 -appVersion: "sha-ca02a516" +appVersion: "sha-eae02741" icon: "https://example.com/icon.png" dependencies: [] From 1aabc1430854b2e2c549df7467d5e3019d34d25e Mon Sep 17 00:00:00 2001 From: Philipp Matthes <27271818+PhilippMatthes@users.noreply.github.com> Date: Tue, 24 Mar 2026 15:46:09 +0100 Subject: [PATCH 04/12] Fix filter capabilities mutating flavor extra specs (#621) The filter was directly assigning `request.Spec.Data.Flavor.Data.ExtraSpecs` to a local variable and then using `delete()` to remove non-capability keys. The filter was directly assigning `request.Spec.Data.Flavor.Data.ExtraSpecs` to a local variable and then using `delete()` to remove non-capability keys. The fix was to change the code to extract only capability keys into a new map instead of deleting from the original. --- .../plugins/filters/filter_capabilities.go | 20 +++-- .../filters/filter_capabilities_test.go | 74 +++++++++++++++++++ 2 files changed, 89 insertions(+), 5 deletions(-) diff --git a/internal/scheduling/nova/plugins/filters/filter_capabilities.go b/internal/scheduling/nova/plugins/filters/filter_capabilities.go index a6c105d29..8cf6afac9 100644 --- a/internal/scheduling/nova/plugins/filters/filter_capabilities.go +++ b/internal/scheduling/nova/plugins/filters/filter_capabilities.go @@ -47,12 +47,25 @@ func hvToNovaCapabilities(hv hv1.Hypervisor) (map[string]string, error) { // in the request spec flavor. func (s *FilterCapabilitiesStep) Run(traceLog *slog.Logger, request api.ExternalSchedulerRequest) (*lib.FilterWeigherPipelineStepResult, error) { result := s.IncludeAllHostsFromRequest(request) - requestedCapabilities := request.Spec.Data.Flavor.Data.ExtraSpecs - if len(requestedCapabilities) == 0 { + extraSpecs := request.Spec.Data.Flavor.Data.ExtraSpecs + if len(extraSpecs) == 0 { traceLog.Debug("no flavor extra spec capabilities in request, skipping filter") return result, nil } + // Extract only capability-related extra specs into a separate map. + // We must not modify the original ExtraSpecs map as it's shared across filters. + requestedCapabilities := make(map[string]string) + for key, value := range extraSpecs { + if strings.HasPrefix(key, "capabilities:") { + requestedCapabilities[key] = value + } + } + if len(requestedCapabilities) == 0 { + traceLog.Debug("no capabilities in flavor extra specs, skipping filter") + return result, nil + } + // Note: currently none of the advanced operators for capabilities are // supported because they are not used by any of our flavors in production. // Ops: https://github.com/sapcc/nova/blob/3ebf80/nova/scheduler/filters/extra_specs_ops.py#L23 @@ -61,9 +74,6 @@ func (s *FilterCapabilitiesStep) Run(traceLog *slog.Logger, request api.External "s==", "s!=", "s<", "s<=", "s>", "s>=", "", // or is special } for key, expr := range requestedCapabilities { - if !strings.HasPrefix(key, "capabilities:") { - delete(requestedCapabilities, key) // Remove non-capability keys. - } for _, op := range unsupportedOps { if strings.Contains(expr, op) { traceLog.Warn( diff --git a/internal/scheduling/nova/plugins/filters/filter_capabilities_test.go b/internal/scheduling/nova/plugins/filters/filter_capabilities_test.go index 8999b3a88..9b5f111dc 100644 --- a/internal/scheduling/nova/plugins/filters/filter_capabilities_test.go +++ b/internal/scheduling/nova/plugins/filters/filter_capabilities_test.go @@ -579,3 +579,77 @@ func TestFilterCapabilitiesStep_Run(t *testing.T) { }) } } + +// TestFilterCapabilitiesStep_DoesNotMutateExtraSpecs verifies that the filter +// does not modify the original ExtraSpecs map from the request. This is important +// because subsequent filters (like filter_has_requested_traits) also need to access +// the full ExtraSpecs including non-capability keys like trait:*. +func TestFilterCapabilitiesStep_DoesNotMutateExtraSpecs(t *testing.T) { + scheme, err := hv1.SchemeBuilder.Build() + if err != nil { + t.Fatalf("expected no error, got %v", err) + } + + hvs := []client.Object{ + &hv1.Hypervisor{ + ObjectMeta: v1.ObjectMeta{Name: "host1"}, + Status: hv1.HypervisorStatus{ + DomainCapabilities: hv1.DomainCapabilities{HypervisorType: "ch"}, + Capabilities: hv1.Capabilities{HostCpuArch: "x86_64"}, + }, + }, + } + + // Create a request with mixed extra specs (capabilities + traits + other) + originalExtraSpecs := map[string]string{ + "capabilities:hypervisor_type": "CH", + "trait:CUSTOM_HANA_EXCLUSIVE_HOST": "forbidden", + "trait:CUSTOM_HW_SAPPHIRE_RAPIDS": "required", + "hw:mem_page_size": "large", + "hw:numa_nodes": "1", + } + + request := api.ExternalSchedulerRequest{ + Spec: api.NovaObject[api.NovaSpec]{ + Data: api.NovaSpec{ + Flavor: api.NovaObject[api.NovaFlavor]{ + Data: api.NovaFlavor{ + ExtraSpecs: originalExtraSpecs, + }, + }, + }, + }, + Hosts: []api.ExternalSchedulerHost{{ComputeHost: "host1"}}, + } + + step := &FilterCapabilitiesStep{} + step.Client = fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(hvs...). + Build() + + _, err = step.Run(slog.Default(), request) + if err != nil { + t.Fatalf("expected no error, got %v", err) + } + + // Verify that the original ExtraSpecs map was NOT mutated + expectedKeys := []string{ + "capabilities:hypervisor_type", + "trait:CUSTOM_HANA_EXCLUSIVE_HOST", + "trait:CUSTOM_HW_SAPPHIRE_RAPIDS", + "hw:mem_page_size", + "hw:numa_nodes", + } + + if len(request.Spec.Data.Flavor.Data.ExtraSpecs) != len(expectedKeys) { + t.Errorf("ExtraSpecs was mutated: expected %d keys, got %d", + len(expectedKeys), len(request.Spec.Data.Flavor.Data.ExtraSpecs)) + } + + for _, key := range expectedKeys { + if _, ok := request.Spec.Data.Flavor.Data.ExtraSpecs[key]; !ok { + t.Errorf("ExtraSpecs was mutated: key %q was deleted", key) + } + } +} From 84d8cabb8be4b9c26a6625506ac15866b806fce5 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Tue, 24 Mar 2026 14:55:40 +0000 Subject: [PATCH 05/12] Bump cortex chart appVersions to sha-1aabc143 [skip ci] --- helm/library/cortex/Chart.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/helm/library/cortex/Chart.yaml b/helm/library/cortex/Chart.yaml index 9b8e7d2bf..000ef72ff 100644 --- a/helm/library/cortex/Chart.yaml +++ b/helm/library/cortex/Chart.yaml @@ -3,6 +3,6 @@ name: cortex description: A Helm chart to distribute cortex. type: application version: 0.0.30 -appVersion: "sha-eae02741" +appVersion: "sha-1aabc143" icon: "https://example.com/icon.png" dependencies: [] From 53969ac434e7aba80e29f6df86834ddebe3e658e Mon Sep 17 00:00:00 2001 From: Malte Viering Date: Tue, 24 Mar 2026 15:57:05 +0100 Subject: [PATCH 06/12] Bump core to 0.0.31 and bundles to 0.0.44 * Better committed resource logging (#620) * Enable kvm_failover_evacuation weigher (#618) * Fix filter capabilities mutating flavor extra specs --- helm/bundles/cortex-cinder/Chart.yaml | 6 +++--- helm/bundles/cortex-crds/Chart.yaml | 4 ++-- helm/bundles/cortex-ironcore/Chart.yaml | 4 ++-- helm/bundles/cortex-manila/Chart.yaml | 6 +++--- helm/bundles/cortex-nova/Chart.yaml | 6 +++--- helm/bundles/cortex-pods/Chart.yaml | 4 ++-- helm/library/cortex/Chart.yaml | 2 +- 7 files changed, 16 insertions(+), 16 deletions(-) diff --git a/helm/bundles/cortex-cinder/Chart.yaml b/helm/bundles/cortex-cinder/Chart.yaml index 407267567..4c4c5889b 100644 --- a/helm/bundles/cortex-cinder/Chart.yaml +++ b/helm/bundles/cortex-cinder/Chart.yaml @@ -5,7 +5,7 @@ apiVersion: v2 name: cortex-cinder description: A Helm chart deploying Cortex for Cinder. type: application -version: 0.0.43 +version: 0.0.44 appVersion: 0.1.0 dependencies: # from: file://../../library/cortex-postgres @@ -16,12 +16,12 @@ dependencies: # from: file://../../library/cortex - name: cortex repository: oci://ghcr.io/cobaltcore-dev/cortex/charts - version: 0.0.30 + version: 0.0.31 alias: cortex-knowledge-controllers # from: file://../../library/cortex - name: cortex repository: oci://ghcr.io/cobaltcore-dev/cortex/charts - version: 0.0.30 + version: 0.0.31 alias: cortex-scheduling-controllers # Owner info adds a configmap to the kubernetes cluster with information on diff --git a/helm/bundles/cortex-crds/Chart.yaml b/helm/bundles/cortex-crds/Chart.yaml index a60a62e48..0d59d0740 100644 --- a/helm/bundles/cortex-crds/Chart.yaml +++ b/helm/bundles/cortex-crds/Chart.yaml @@ -5,13 +5,13 @@ apiVersion: v2 name: cortex-crds description: A Helm chart deploying Cortex CRDs. type: application -version: 0.0.43 +version: 0.0.44 appVersion: 0.1.0 dependencies: # from: file://../../library/cortex - name: cortex repository: oci://ghcr.io/cobaltcore-dev/cortex/charts - version: 0.0.30 + version: 0.0.31 # Owner info adds a configmap to the kubernetes cluster with information on # the service owner. This makes it easier to find out who to contact in case diff --git a/helm/bundles/cortex-ironcore/Chart.yaml b/helm/bundles/cortex-ironcore/Chart.yaml index 95f4a4125..5f975675d 100644 --- a/helm/bundles/cortex-ironcore/Chart.yaml +++ b/helm/bundles/cortex-ironcore/Chart.yaml @@ -5,13 +5,13 @@ apiVersion: v2 name: cortex-ironcore description: A Helm chart deploying Cortex for IronCore. type: application -version: 0.0.43 +version: 0.0.44 appVersion: 0.1.0 dependencies: # from: file://../../library/cortex - name: cortex repository: oci://ghcr.io/cobaltcore-dev/cortex/charts - version: 0.0.30 + version: 0.0.31 # Owner info adds a configmap to the kubernetes cluster with information on # the service owner. This makes it easier to find out who to contact in case diff --git a/helm/bundles/cortex-manila/Chart.yaml b/helm/bundles/cortex-manila/Chart.yaml index 45c0ee766..c253c3da9 100644 --- a/helm/bundles/cortex-manila/Chart.yaml +++ b/helm/bundles/cortex-manila/Chart.yaml @@ -5,7 +5,7 @@ apiVersion: v2 name: cortex-manila description: A Helm chart deploying Cortex for Manila. type: application -version: 0.0.43 +version: 0.0.44 appVersion: 0.1.0 dependencies: # from: file://../../library/cortex-postgres @@ -16,12 +16,12 @@ dependencies: # from: file://../../library/cortex - name: cortex repository: oci://ghcr.io/cobaltcore-dev/cortex/charts - version: 0.0.30 + version: 0.0.31 alias: cortex-knowledge-controllers # from: file://../../library/cortex - name: cortex repository: oci://ghcr.io/cobaltcore-dev/cortex/charts - version: 0.0.30 + version: 0.0.31 alias: cortex-scheduling-controllers # Owner info adds a configmap to the kubernetes cluster with information on diff --git a/helm/bundles/cortex-nova/Chart.yaml b/helm/bundles/cortex-nova/Chart.yaml index 958818df8..e77dfe3b3 100644 --- a/helm/bundles/cortex-nova/Chart.yaml +++ b/helm/bundles/cortex-nova/Chart.yaml @@ -5,7 +5,7 @@ apiVersion: v2 name: cortex-nova description: A Helm chart deploying Cortex for Nova. type: application -version: 0.0.43 +version: 0.0.44 appVersion: 0.1.0 dependencies: # from: file://../../library/cortex-postgres @@ -16,12 +16,12 @@ dependencies: # from: file://../../library/cortex - name: cortex repository: oci://ghcr.io/cobaltcore-dev/cortex/charts - version: 0.0.30 + version: 0.0.31 alias: cortex-knowledge-controllers # from: file://../../library/cortex - name: cortex repository: oci://ghcr.io/cobaltcore-dev/cortex/charts - version: 0.0.30 + version: 0.0.31 alias: cortex-scheduling-controllers # Owner info adds a configmap to the kubernetes cluster with information on diff --git a/helm/bundles/cortex-pods/Chart.yaml b/helm/bundles/cortex-pods/Chart.yaml index facec4423..c44db2b25 100644 --- a/helm/bundles/cortex-pods/Chart.yaml +++ b/helm/bundles/cortex-pods/Chart.yaml @@ -5,13 +5,13 @@ apiVersion: v2 name: cortex-pods description: A Helm chart deploying Cortex for Pods. type: application -version: 0.0.43 +version: 0.0.44 appVersion: 0.1.0 dependencies: # from: file://../../library/cortex - name: cortex repository: oci://ghcr.io/cobaltcore-dev/cortex/charts - version: 0.0.30 + version: 0.0.31 # Owner info adds a configmap to the kubernetes cluster with information on # the service owner. This makes it easier to find out who to contact in case diff --git a/helm/library/cortex/Chart.yaml b/helm/library/cortex/Chart.yaml index 000ef72ff..455cc45da 100644 --- a/helm/library/cortex/Chart.yaml +++ b/helm/library/cortex/Chart.yaml @@ -2,7 +2,7 @@ apiVersion: v2 name: cortex description: A Helm chart to distribute cortex. type: application -version: 0.0.30 +version: 0.0.31 appVersion: "sha-1aabc143" icon: "https://example.com/icon.png" dependencies: [] From d0a62eba6fd2ced7b9f58636d345a732aab8c3f7 Mon Sep 17 00:00:00 2001 From: mblos <156897072+mblos@users.noreply.github.com> Date: Tue, 24 Mar 2026 15:59:44 +0100 Subject: [PATCH 07/12] Added commitment docs (#622) --- cmd/main.go | 2 +- .../committed-resource-reservations.md | 104 ++++++++++++++++++ helm/bundles/cortex-nova/values.yaml | 6 + .../reservations/commitments/api.go | 8 +- .../api_change_commitments_test.go | 2 +- .../commitments/api_report_usage_test.go | 2 +- .../reservations/commitments/usage.go | 12 +- 7 files changed, 130 insertions(+), 6 deletions(-) create mode 100644 docs/reservations/committed-resource-reservations.md diff --git a/cmd/main.go b/cmd/main.go index e34c5b8bc..351bf29eb 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -340,7 +340,7 @@ func main() { // Initialize commitments API for LIQUID interface (with Nova client for usage reporting) commitmentsConfig := conf.GetConfigOrDie[commitments.Config]() commitmentsAPI := commitments.NewAPIWithConfig(multiclusterClient, commitmentsConfig, novaClient) - commitmentsAPI.Init(mux, metrics.Registry) + commitmentsAPI.Init(mux, metrics.Registry, ctrl.Log.WithName("commitments-api")) deschedulingsController := &nova.DetectorPipelineController{ Monitor: detectorPipelineMonitor, diff --git a/docs/reservations/committed-resource-reservations.md b/docs/reservations/committed-resource-reservations.md new file mode 100644 index 000000000..fe4b17cc0 --- /dev/null +++ b/docs/reservations/committed-resource-reservations.md @@ -0,0 +1,104 @@ +# Committed Resource Reservation System + +The committed resource reservation system manages capacity commitments, i.e. strict reservation guarantees usable by projects. +When customers pre-commit to resource usage, Cortex reserves capacity on hypervisors to guarantee availability. +The system integrates with Limes (via the LIQUID protocol) to receive commitments, expose usage and capacity data, and provides acceptance/rejection feedback. + +## File Structure + +```text +internal/scheduling/reservations/commitments/ +├── config.go # Configuration (intervals, API flags, secrets) +├── controller.go # Reconciliation of reservations +├── syncer.go # Periodic sync task with Limes, ensures local state matches Limes' commitments +├── reservation_manager.go # Reservation CRUD operations +├── api.go # HTTP API initialization +├── api_change_commitments.go # Handle commitment changes from Limes and updates local reservations accordingly +├── api_report_usage.go # Report VM usage per project, accounting to commitments or PAYG +├── api_report_capacity.go # Report capacity per AZ +├── api_info.go # Readiness endpoint with versioning (of underlying flavor group configuration) +├── capacity.go # Capacity calculation from Hypervisor CRDs +├── usage.go # VM-to-commitment assignment logic +├── flavor_group_eligibility.go # Validates VMs belong to correct flavor groups +└── state.go # Commitment state helper functions +``` + +## Operations + +### Configuration + +| Helm Value | Description | +|------------|-------------| +| `committedResourceEnableChangeCommitmentsAPI` | Enable/disable the change-commitments endpoint | +| `committedResourceEnableReportUsageAPI` | Enable/disable the usage reporting endpoint | +| `committedResourceEnableReportCapacityAPI` | Enable/disable the capacity reporting endpoint | +| `committedResourceRequeueIntervalActive` | How often to revalidate active reservations | +| `committedResourceRequeueIntervalRetry` | Retry interval when knowledge not ready | +| `committedResourceChangeAPIWatchReservationsTimeout` | Timeout waiting for reservations to become ready while processing commitment changes via API | +| `committedResourcePipelineDefault` | Default scheduling pipeline | +| `committedResourceFlavorGroupPipelines` | Map of flavor group to pipeline name | +| `committedResourceSyncInterval` | How often the syncer reconciles Limes commitments to Reservation CRDs | + +Each API endpoint can be disabled independently. The periodic sync task can be disabled by removing it (`commitments-sync-task`) from the list of enabled tasks in the `cortex-nova` Helm chart. + +### Observability + +Alerts and metrics are defined in `helm/bundles/cortex-nova/alerts/nova.alerts.yaml`. Key metric prefixes: +- `cortex_committed_resource_change_api_*` - Change API metrics +- `cortex_committed_resource_usage_api_*` - Usage API metrics +- `cortex_committed_resource_capacity_api_*` - Capacity API metrics + +## Architecture Overview + +```mermaid +flowchart LR + subgraph State + Res[(Reservation CRDs)] + end + + ChangeAPI[Change API] + UsageAPI[Usage API] + Syncer[Syncer Task] + Controller[Controller] + Scheduler[Scheduler API] + + ChangeAPI -->|CRUD| Res + Syncer -->|CRUD| Res + UsageAPI -->|read| Res + Res -->|watch| Controller + Controller -->|update spec/status| Res + Controller -->|placement request| Scheduler +``` + +Reservations are managed through the Change API, Syncer Task, and Controller reconciliation. The Usage API provides read-only access to report usage data back to Limes. + +### Change-Commitments API + +The change-commitments API receives batched commitment changes from Limes. A request can contain multiple commitment changes across different projects and flavor groups. The semantic is **all-or-nothing**: if any commitment in the batch cannot be fulfilled (e.g., insufficient capacity), the entire request is rejected and rolled back. + +Cortex performs CRUD operations on local Reservation CRDs to match the new desired state: +- Creates new reservations for increased commitment amounts +- Deletes existing reservations +- Cortex preserves existing reservations that already have VMs allocated when possible + +### Syncer Task + +The syncer task runs periodically and fetches all commitments from Limes. It syncs the local Reservation CRD state to match Limes' view of commitments. + +### Controller (Reconciliation) + +The controller watches Reservation CRDs and performs reconciliation: + +1. **For new reservations** (no target host assigned): + - Calls Cortex for scheduling to find a suitable host + - Assigns the target host and marks the reservation as Ready + +2. **For existing reservations** (already have a target host): + - Validates that allocated VMs are still on the expected host + - Updates allocations if VMs have migrated or been deleted + - Requeues for periodic revalidation + +### Usage API + +This API reports for a given project the total committed resources and usage per flavor group. For each VM, it reports whether the VM accounts to a specific commitment or PAYG. This assignment is deterministic and may differ from the actual Cortex internal assignment used for scheduling. + diff --git a/helm/bundles/cortex-nova/values.yaml b/helm/bundles/cortex-nova/values.yaml index 3409ceaf9..192d714c2 100644 --- a/helm/bundles/cortex-nova/values.yaml +++ b/helm/bundles/cortex-nova/values.yaml @@ -164,6 +164,12 @@ cortex-scheduling-controllers: # Whether the change-commitments API endpoint is active # When false, the endpoint returns HTTP 503. The info endpoint remains available. committedResourceEnableChangeCommitmentsAPI: true + # Whether the report-usage API endpoint is active + # When false, the endpoint returns HTTP 503. + committedResourceEnableReportUsageAPI: true + # Whether the report-capacity API endpoint is active + # When false, the endpoint returns HTTP 503. + committedResourceEnableReportCapacityAPI: true # OvercommitMappings is a list of mappings that map hypervisor traits to # overcommit ratios. Note that this list is applied in order, so if there # are multiple mappings applying to the same hypervisors, the last mapping diff --git a/internal/scheduling/reservations/commitments/api.go b/internal/scheduling/reservations/commitments/api.go index f8450ad8a..daedb8a2f 100644 --- a/internal/scheduling/reservations/commitments/api.go +++ b/internal/scheduling/reservations/commitments/api.go @@ -9,6 +9,7 @@ import ( "sync" "github.com/cobaltcore-dev/cortex/internal/scheduling/nova" + "github.com/go-logr/logr" "github.com/prometheus/client_golang/prometheus" "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -46,7 +47,7 @@ func NewAPIWithConfig(client client.Client, config Config, novaClient UsageNovaC } } -func (api *HTTPAPI) Init(mux *http.ServeMux, registry prometheus.Registerer) { +func (api *HTTPAPI) Init(mux *http.ServeMux, registry prometheus.Registerer, log logr.Logger) { registry.MustRegister(&api.monitor) registry.MustRegister(&api.usageMonitor) registry.MustRegister(&api.capacityMonitor) @@ -54,4 +55,9 @@ func (api *HTTPAPI) Init(mux *http.ServeMux, registry prometheus.Registerer) { mux.HandleFunc("/v1/commitments/report-capacity", api.HandleReportCapacity) mux.HandleFunc("/v1/commitments/info", api.HandleInfo) mux.HandleFunc("/v1/commitments/projects/", api.HandleReportUsage) // matches /v1/commitments/projects/:project_id/report-usage + + log.Info("commitments API initialized", + "changeCommitmentsEnabled", api.config.EnableChangeCommitmentsAPI, + "reportUsageEnabled", api.config.EnableReportUsageAPI, + "reportCapacityEnabled", api.config.EnableReportCapacityAPI) } diff --git a/internal/scheduling/reservations/commitments/api_change_commitments_test.go b/internal/scheduling/reservations/commitments/api_change_commitments_test.go index d996c498e..6ceb8b65f 100644 --- a/internal/scheduling/reservations/commitments/api_change_commitments_test.go +++ b/internal/scheduling/reservations/commitments/api_change_commitments_test.go @@ -997,7 +997,7 @@ func newCommitmentTestEnv( } mux := http.NewServeMux() registry := prometheus.NewRegistry() - api.Init(mux, registry) + api.Init(mux, registry, log.Log) httpServer := httptest.NewServer(mux) env.HTTPServer = httpServer diff --git a/internal/scheduling/reservations/commitments/api_report_usage_test.go b/internal/scheduling/reservations/commitments/api_report_usage_test.go index 0500735b1..2a81fa468 100644 --- a/internal/scheduling/reservations/commitments/api_report_usage_test.go +++ b/internal/scheduling/reservations/commitments/api_report_usage_test.go @@ -540,7 +540,7 @@ func newUsageTestEnv( api := NewAPIWithConfig(k8sClient, DefaultConfig(), novaClient) mux := http.NewServeMux() registry := prometheus.NewRegistry() - api.Init(mux, registry) + api.Init(mux, registry, log.Log) httpServer := httptest.NewServer(mux) return &UsageTestEnv{ diff --git a/internal/scheduling/reservations/commitments/usage.go b/internal/scheduling/reservations/commitments/usage.go index d37b35a0a..b71d4e752 100644 --- a/internal/scheduling/reservations/commitments/usage.go +++ b/internal/scheduling/reservations/commitments/usage.go @@ -63,6 +63,12 @@ func (c *UsageCalculator) CalculateUsage( return liquid.ServiceUsageReport{}, fmt.Errorf("failed to get flavor groups: %w", err) } + // Get info version from Knowledge CRD (used by Limes to detect metadata changes) + var infoVersion int64 = -1 + if knowledgeCRD, err := knowledge.Get(ctx); err == nil && knowledgeCRD != nil && !knowledgeCRD.Status.LastContentChange.IsZero() { + infoVersion = knowledgeCRD.Status.LastContentChange.Unix() + } + // Step 2: Build commitment capacity map from K8s Reservation CRDs commitmentsByAZFlavorGroup, err := c.buildCommitmentCapacityMap(ctx, log, projectID) if err != nil { @@ -80,7 +86,7 @@ func (c *UsageCalculator) CalculateUsage( vmAssignments, assignedToCommitments := c.assignVMsToCommitments(vms, commitmentsByAZFlavorGroup) // Step 5: Build the response - report := c.buildUsageResponse(vms, vmAssignments, flavorGroups, allAZs) + report := c.buildUsageResponse(vms, vmAssignments, flavorGroups, allAZs, infoVersion) log.Info("completed usage report", "projectID", projectID, @@ -336,6 +342,7 @@ func (c *UsageCalculator) buildUsageResponse( vmAssignments map[string]string, flavorGroups map[string]compute.FlavorGroupFeature, allAZs []liquid.AvailabilityZone, + infoVersion int64, ) liquid.ServiceUsageReport { // Initialize resources map for flavor groups that accept commitments resources := make(map[liquid.ResourceName]*liquid.ResourceUsageReport) @@ -420,7 +427,8 @@ func (c *UsageCalculator) buildUsageResponse( } return liquid.ServiceUsageReport{ - Resources: resources, + InfoVersion: infoVersion, + Resources: resources, } } From be57043ed00cc5729d29c70be2334619a56e1d31 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Tue, 24 Mar 2026 15:08:50 +0000 Subject: [PATCH 08/12] Bump cortex chart appVersions to sha-d0a62eba [skip ci] --- helm/library/cortex/Chart.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/helm/library/cortex/Chart.yaml b/helm/library/cortex/Chart.yaml index 455cc45da..0c0a6159a 100644 --- a/helm/library/cortex/Chart.yaml +++ b/helm/library/cortex/Chart.yaml @@ -3,6 +3,6 @@ name: cortex description: A Helm chart to distribute cortex. type: application version: 0.0.31 -appVersion: "sha-1aabc143" +appVersion: "sha-d0a62eba" icon: "https://example.com/icon.png" dependencies: [] From 23f03412753755fffbf4e4bae5a7b4a8f80ee343 Mon Sep 17 00:00:00 2001 From: Markus Wieland <44964229+SoWieMarkus@users.noreply.github.com> Date: Tue, 24 Mar 2026 16:23:12 +0100 Subject: [PATCH 09/12] Failover controller to support multicluster watches for reservations (#625) --- .../reservations/failover/controller.go | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/internal/scheduling/reservations/failover/controller.go b/internal/scheduling/reservations/failover/controller.go index 4cc7d3c1e..fbb4f9c01 100644 --- a/internal/scheduling/reservations/failover/controller.go +++ b/internal/scheduling/reservations/failover/controller.go @@ -25,6 +25,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller" "sigs.k8s.io/controller-runtime/pkg/event" + "sigs.k8s.io/controller-runtime/pkg/handler" "sigs.k8s.io/controller-runtime/pkg/predicate" ) @@ -767,10 +768,16 @@ func (c *FailoverReservationController) patchReservationStatus(ctx context.Conte func (c *FailoverReservationController) SetupWithManager(mgr ctrl.Manager, mcl *multicluster.Client) error { c.Recorder = mgr.GetEventRecorder("failover-reservation-controller") - return multicluster.BuildController(mcl, mgr). - For(&v1alpha1.Reservation{}). - WithEventFilter(failoverReservationPredicate). - Named("failover-reservation"). + bldr := multicluster.BuildController(mcl, mgr) + bldr, err := bldr.WatchesMulticluster( + &v1alpha1.Reservation{}, + &handler.EnqueueRequestForObject{}, + failoverReservationPredicate, + ) + if err != nil { + return err + } + return bldr.Named("failover-reservation"). WithOptions(controller.Options{ MaxConcurrentReconciles: 1, }). From 745af7bebe76166fcf72c2ce69ae5161c27e67fb Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Tue, 24 Mar 2026 15:31:30 +0000 Subject: [PATCH 10/12] Bump cortex chart appVersions to sha-23f03412 [skip ci] --- helm/library/cortex/Chart.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/helm/library/cortex/Chart.yaml b/helm/library/cortex/Chart.yaml index 0c0a6159a..a53b5d0aa 100644 --- a/helm/library/cortex/Chart.yaml +++ b/helm/library/cortex/Chart.yaml @@ -3,6 +3,6 @@ name: cortex description: A Helm chart to distribute cortex. type: application version: 0.0.31 -appVersion: "sha-d0a62eba" +appVersion: "sha-23f03412" icon: "https://example.com/icon.png" dependencies: [] From 8894640569d4b9774ce1894954ef5d11e33059cb Mon Sep 17 00:00:00 2001 From: Philipp Matthes Date: Tue, 24 Mar 2026 17:36:00 +0100 Subject: [PATCH 11/12] Upgrade hypervisor crd and add aggregate metadata filter --- go.mod | 2 +- go.sum | 2 + .../cortex-nova/templates/pipelines_kvm.yaml | 12 + .../filters/filter_aggregate_metadata.go | 70 +++ .../filters/filter_aggregate_metadata_test.go | 406 ++++++++++++++++++ 5 files changed, 491 insertions(+), 1 deletion(-) create mode 100644 internal/scheduling/nova/plugins/filters/filter_aggregate_metadata.go create mode 100644 internal/scheduling/nova/plugins/filters/filter_aggregate_metadata_test.go diff --git a/go.mod b/go.mod index 8cca91f86..cf74f39f1 100644 --- a/go.mod +++ b/go.mod @@ -3,7 +3,7 @@ module github.com/cobaltcore-dev/cortex go 1.26 require ( - github.com/cobaltcore-dev/openstack-hypervisor-operator v1.0.1 + github.com/cobaltcore-dev/openstack-hypervisor-operator v1.0.2-0.20260324155836-56b40c7ff846 github.com/go-gorp/gorp v2.2.0+incompatible github.com/gophercloud/gophercloud/v2 v2.11.1 github.com/ironcore-dev/ironcore v0.2.4 diff --git a/go.sum b/go.sum index 22f64fec6..98a4f9477 100644 --- a/go.sum +++ b/go.sum @@ -22,6 +22,8 @@ github.com/cespare/xxhash/v2 v2.3.0 h1:UL815xU9SqsFlibzuggzjXhog7bL6oX9BbNZnL2UF github.com/cespare/xxhash/v2 v2.3.0/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs= github.com/cobaltcore-dev/openstack-hypervisor-operator v1.0.1 h1:wXolWfljyQQZbxNQ2pZVIw8wFz9BKiDIvLrECsqGDT8= github.com/cobaltcore-dev/openstack-hypervisor-operator v1.0.1/go.mod h1:b0KmJdxvRI8UXlGe8cRm5BD8Tm2WhF7zSKMSIRGyVL4= +github.com/cobaltcore-dev/openstack-hypervisor-operator v1.0.2-0.20260324155836-56b40c7ff846 h1:Hg5+F1lOUpU9dZ8gVxeohodtYC4Z1fV/iqwYoF/RuNc= +github.com/cobaltcore-dev/openstack-hypervisor-operator v1.0.2-0.20260324155836-56b40c7ff846/go.mod h1:j1SaxUTo0irugdC7aHuYDKEomIPZwCHoz+4kE8EBBGM= github.com/containerd/continuity v0.4.5 h1:ZRoN1sXq9u7V6QoHMcVWGhOwDFqZ4B9i5H6un1Wh0x4= github.com/containerd/continuity v0.4.5/go.mod h1:/lNJvtJKUQStBzpVQ1+rasXO1LAWtUQssk28EZvJ3nE= github.com/containerd/errdefs v1.0.0 h1:tg5yIfIlQIrxYtu9ajqY42W3lpS19XqdxRQeEwYG8PI= diff --git a/helm/bundles/cortex-nova/templates/pipelines_kvm.yaml b/helm/bundles/cortex-nova/templates/pipelines_kvm.yaml index 4273c0af3..1913ff39a 100644 --- a/helm/bundles/cortex-nova/templates/pipelines_kvm.yaml +++ b/helm/bundles/cortex-nova/templates/pipelines_kvm.yaml @@ -85,6 +85,12 @@ spec: hypervisor resource. Note that hosts allowing all projects are still accessible and will not be filtered out. In this way some hypervisors are made accessible to some projects only. + - name: filter_aggregate_metadata + description: | + This step filters hosts based on metadata defined in their aggregates. For + example, if an aggregate has the metadata "filter_tenant_id": "", + only hosts in that aggregate that match the project ID in the nova request + will pass this filter. - name: filter_live_migratable description: | This step ensures that the target host of a live migration can accept @@ -218,6 +224,12 @@ spec: hypervisor resource. Note that hosts allowing all projects are still accessible and will not be filtered out. In this way some hypervisors are made accessible to some projects only. + - name: filter_aggregate_metadata + description: | + This step filters hosts based on metadata defined in their aggregates. For + example, if an aggregate has the metadata "filter_tenant_id": "", + only hosts in that aggregate that match the project ID in the nova request + will pass this filter. - name: filter_live_migratable description: | This step ensures that the target host of a live migration can accept diff --git a/internal/scheduling/nova/plugins/filters/filter_aggregate_metadata.go b/internal/scheduling/nova/plugins/filters/filter_aggregate_metadata.go new file mode 100644 index 000000000..157a80521 --- /dev/null +++ b/internal/scheduling/nova/plugins/filters/filter_aggregate_metadata.go @@ -0,0 +1,70 @@ +// Copyright SAP SE +// SPDX-License-Identifier: Apache-2.0 + +package filters + +import ( + "context" + "log/slog" + "slices" + + api "github.com/cobaltcore-dev/cortex/api/external/nova" + "github.com/cobaltcore-dev/cortex/internal/scheduling/lib" + hv1 "github.com/cobaltcore-dev/openstack-hypervisor-operator/api/v1" +) + +type FilterAggregateMetadata struct { + lib.BaseFilter[api.ExternalSchedulerRequest, lib.EmptyFilterWeigherPipelineStepOpts] +} + +// Restrict hosts to specific projects if they are in an aggregate that has +// the "filter_tenant_id" metadata key set. +func (s *FilterAggregateMetadata) Run(traceLog *slog.Logger, request api.ExternalSchedulerRequest) (*lib.FilterWeigherPipelineStepResult, error) { + result := s.IncludeAllHostsFromRequest(request) + + hvs := &hv1.HypervisorList{} + if err := s.Client.List(context.Background(), hvs); err != nil { + traceLog.Error("failed to list hypervisors", "error", err) + return nil, err + } + + restrictedProjectsByHost := make(map[string][]string) + for _, hv := range hvs.Items { + for _, aggregate := range hv.Status.Aggregates { + tenantID, ok := aggregate.Metadata["filter_tenant_id"] + if !ok { + traceLog.Info("aggregate does not have filter_tenant_id metadata, skipping", + "aggregate", aggregate.Name) + continue + } + restrictedProjectsByHost[hv.Name] = append(restrictedProjectsByHost[hv.Name], tenantID) + traceLog.Info("host is in aggregate with filter_tenant_id, adding restriction", + "host", hv.Name, "aggregate", aggregate.Name, "tenant_id", tenantID) + } + } + + for host, restrictedProjects := range restrictedProjectsByHost { + if !slices.Contains(restrictedProjects, request.Spec.Data.ProjectID) { + // Project is not allowed on this hypervisor, filter it out. + delete(result.Activations, host) + traceLog.Info( + "filtering host not allowing project based on aggregate metadata", + "host", host, + "project", request.Spec.Data.ProjectID, + "restricted_projects", restrictedProjects, + ) + } else { + traceLog.Info( + "host allows project based on aggregate metadata, keeping", + "host", host, + "project", request.Spec.Data.ProjectID, + "restricted_projects", restrictedProjects, + ) + } + } + return result, nil +} + +func init() { + Index["filter_aggregate_metadata"] = func() NovaFilter { return &FilterAggregateMetadata{} } +} diff --git a/internal/scheduling/nova/plugins/filters/filter_aggregate_metadata_test.go b/internal/scheduling/nova/plugins/filters/filter_aggregate_metadata_test.go new file mode 100644 index 000000000..d1ff9cd2d --- /dev/null +++ b/internal/scheduling/nova/plugins/filters/filter_aggregate_metadata_test.go @@ -0,0 +1,406 @@ +// Copyright SAP SE +// SPDX-License-Identifier: Apache-2.0 + +package filters + +import ( + "context" + "log/slog" + "testing" + + api "github.com/cobaltcore-dev/cortex/api/external/nova" + hv1 "github.com/cobaltcore-dev/openstack-hypervisor-operator/api/v1" + 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" + "sigs.k8s.io/controller-runtime/pkg/client/interceptor" +) + +func TestFilterAggregateMetadata_Run(t *testing.T) { + tests := []struct { + name string + request api.ExternalSchedulerRequest + hypervisors []hv1.Hypervisor + expectedHosts []string + filteredHosts []string + }{ + { + name: "No aggregates with filter_tenant_id - all hosts pass", + request: api.ExternalSchedulerRequest{ + Spec: api.NovaObject[api.NovaSpec]{ + Data: api.NovaSpec{ + ProjectID: "project-a", + }, + }, + Hosts: []api.ExternalSchedulerHost{ + {ComputeHost: "host1"}, + {ComputeHost: "host2"}, + {ComputeHost: "host3"}, + }, + }, + hypervisors: []hv1.Hypervisor{ + { + ObjectMeta: metav1.ObjectMeta{Name: "host1"}, + Status: hv1.HypervisorStatus{ + Aggregates: []hv1.Aggregate{{Name: "agg1", Metadata: map[string]string{}}}, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{Name: "host2"}, + Status: hv1.HypervisorStatus{ + Aggregates: []hv1.Aggregate{{Name: "agg2", Metadata: map[string]string{"other_key": "value"}}}, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{Name: "host3"}, + Status: hv1.HypervisorStatus{ + Aggregates: []hv1.Aggregate{}, + }, + }, + }, + expectedHosts: []string{"host1", "host2", "host3"}, + filteredHosts: []string{}, + }, + { + name: "Host with filter_tenant_id matching project - host passes", + request: api.ExternalSchedulerRequest{ + Spec: api.NovaObject[api.NovaSpec]{ + Data: api.NovaSpec{ + ProjectID: "project-a", + }, + }, + Hosts: []api.ExternalSchedulerHost{ + {ComputeHost: "host1"}, + {ComputeHost: "host2"}, + }, + }, + hypervisors: []hv1.Hypervisor{ + { + ObjectMeta: metav1.ObjectMeta{Name: "host1"}, + Status: hv1.HypervisorStatus{ + Aggregates: []hv1.Aggregate{{Name: "restricted-agg", Metadata: map[string]string{"filter_tenant_id": "project-a"}}}, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{Name: "host2"}, + Status: hv1.HypervisorStatus{ + Aggregates: []hv1.Aggregate{{Name: "open-agg", Metadata: map[string]string{}}}, + }, + }, + }, + expectedHosts: []string{"host1", "host2"}, + filteredHosts: []string{}, + }, + { + name: "Host with filter_tenant_id not matching project - host filtered", + request: api.ExternalSchedulerRequest{ + Spec: api.NovaObject[api.NovaSpec]{ + Data: api.NovaSpec{ + ProjectID: "project-b", + }, + }, + Hosts: []api.ExternalSchedulerHost{ + {ComputeHost: "host1"}, + {ComputeHost: "host2"}, + }, + }, + hypervisors: []hv1.Hypervisor{ + { + ObjectMeta: metav1.ObjectMeta{Name: "host1"}, + Status: hv1.HypervisorStatus{ + Aggregates: []hv1.Aggregate{{Name: "restricted-agg", Metadata: map[string]string{"filter_tenant_id": "project-a"}}}, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{Name: "host2"}, + Status: hv1.HypervisorStatus{ + Aggregates: []hv1.Aggregate{{Name: "open-agg", Metadata: map[string]string{}}}, + }, + }, + }, + expectedHosts: []string{"host2"}, + filteredHosts: []string{"host1"}, + }, + { + name: "Host in multiple aggregates with different filter_tenant_id - project matches one", + request: api.ExternalSchedulerRequest{ + Spec: api.NovaObject[api.NovaSpec]{ + Data: api.NovaSpec{ + ProjectID: "project-b", + }, + }, + Hosts: []api.ExternalSchedulerHost{ + {ComputeHost: "host1"}, + }, + }, + hypervisors: []hv1.Hypervisor{ + { + ObjectMeta: metav1.ObjectMeta{Name: "host1"}, + Status: hv1.HypervisorStatus{ + Aggregates: []hv1.Aggregate{ + {Name: "agg1", Metadata: map[string]string{"filter_tenant_id": "project-a"}}, + {Name: "agg2", Metadata: map[string]string{"filter_tenant_id": "project-b"}}, + }, + }, + }, + }, + expectedHosts: []string{"host1"}, + filteredHosts: []string{}, + }, + { + name: "Host in multiple aggregates - project matches none", + request: api.ExternalSchedulerRequest{ + Spec: api.NovaObject[api.NovaSpec]{ + Data: api.NovaSpec{ + ProjectID: "project-c", + }, + }, + Hosts: []api.ExternalSchedulerHost{ + {ComputeHost: "host1"}, + }, + }, + hypervisors: []hv1.Hypervisor{ + { + ObjectMeta: metav1.ObjectMeta{Name: "host1"}, + Status: hv1.HypervisorStatus{ + Aggregates: []hv1.Aggregate{ + {Name: "agg1", Metadata: map[string]string{"filter_tenant_id": "project-a"}}, + {Name: "agg2", Metadata: map[string]string{"filter_tenant_id": "project-b"}}, + }, + }, + }, + }, + expectedHosts: []string{}, + filteredHosts: []string{"host1"}, + }, + { + name: "Mixed hosts - some restricted, some open", + request: api.ExternalSchedulerRequest{ + Spec: api.NovaObject[api.NovaSpec]{ + Data: api.NovaSpec{ + ProjectID: "project-a", + }, + }, + Hosts: []api.ExternalSchedulerHost{ + {ComputeHost: "host1"}, + {ComputeHost: "host2"}, + {ComputeHost: "host3"}, + {ComputeHost: "host4"}, + }, + }, + hypervisors: []hv1.Hypervisor{ + { + ObjectMeta: metav1.ObjectMeta{Name: "host1"}, + Status: hv1.HypervisorStatus{ + Aggregates: []hv1.Aggregate{{Name: "restricted-a", Metadata: map[string]string{"filter_tenant_id": "project-a"}}}, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{Name: "host2"}, + Status: hv1.HypervisorStatus{ + Aggregates: []hv1.Aggregate{{Name: "restricted-b", Metadata: map[string]string{"filter_tenant_id": "project-b"}}}, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{Name: "host3"}, + Status: hv1.HypervisorStatus{ + Aggregates: []hv1.Aggregate{{Name: "open", Metadata: map[string]string{}}}, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{Name: "host4"}, + Status: hv1.HypervisorStatus{ + Aggregates: []hv1.Aggregate{}, + }, + }, + }, + expectedHosts: []string{"host1", "host3", "host4"}, + filteredHosts: []string{"host2"}, + }, + { + name: "Empty host list", + request: api.ExternalSchedulerRequest{ + Spec: api.NovaObject[api.NovaSpec]{ + Data: api.NovaSpec{ + ProjectID: "project-a", + }, + }, + Hosts: []api.ExternalSchedulerHost{}, + }, + hypervisors: []hv1.Hypervisor{ + { + ObjectMeta: metav1.ObjectMeta{Name: "host1"}, + Status: hv1.HypervisorStatus{ + Aggregates: []hv1.Aggregate{{Name: "restricted", Metadata: map[string]string{"filter_tenant_id": "project-a"}}}, + }, + }, + }, + expectedHosts: []string{}, + filteredHosts: []string{}, + }, + { + name: "All hosts filtered out", + request: api.ExternalSchedulerRequest{ + Spec: api.NovaObject[api.NovaSpec]{ + Data: api.NovaSpec{ + ProjectID: "project-nonexistent", + }, + }, + Hosts: []api.ExternalSchedulerHost{ + {ComputeHost: "host1"}, + {ComputeHost: "host2"}, + }, + }, + hypervisors: []hv1.Hypervisor{ + { + ObjectMeta: metav1.ObjectMeta{Name: "host1"}, + Status: hv1.HypervisorStatus{ + Aggregates: []hv1.Aggregate{{Name: "restricted-a", Metadata: map[string]string{"filter_tenant_id": "project-a"}}}, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{Name: "host2"}, + Status: hv1.HypervisorStatus{ + Aggregates: []hv1.Aggregate{{Name: "restricted-b", Metadata: map[string]string{"filter_tenant_id": "project-b"}}}, + }, + }, + }, + expectedHosts: []string{}, + filteredHosts: []string{"host1", "host2"}, + }, + { + name: "Host not in hypervisor list - passes (no restrictions found)", + request: api.ExternalSchedulerRequest{ + Spec: api.NovaObject[api.NovaSpec]{ + Data: api.NovaSpec{ + ProjectID: "project-a", + }, + }, + Hosts: []api.ExternalSchedulerHost{ + {ComputeHost: "host1"}, + {ComputeHost: "host-unknown"}, + }, + }, + hypervisors: []hv1.Hypervisor{ + { + ObjectMeta: metav1.ObjectMeta{Name: "host1"}, + Status: hv1.HypervisorStatus{ + Aggregates: []hv1.Aggregate{{Name: "open", Metadata: map[string]string{}}}, + }, + }, + }, + expectedHosts: []string{"host1", "host-unknown"}, + filteredHosts: []string{}, + }, + { + name: "Aggregate with nil metadata - treated as no restriction", + request: api.ExternalSchedulerRequest{ + Spec: api.NovaObject[api.NovaSpec]{ + Data: api.NovaSpec{ + ProjectID: "project-a", + }, + }, + Hosts: []api.ExternalSchedulerHost{ + {ComputeHost: "host1"}, + }, + }, + hypervisors: []hv1.Hypervisor{ + { + ObjectMeta: metav1.ObjectMeta{Name: "host1"}, + Status: hv1.HypervisorStatus{ + Aggregates: []hv1.Aggregate{{Name: "agg-nil-metadata", Metadata: nil}}, + }, + }, + }, + expectedHosts: []string{"host1"}, + filteredHosts: []string{}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + scheme := runtime.NewScheme() + if err := hv1.AddToScheme(scheme); err != nil { + t.Fatalf("failed to add hv1 scheme: %v", err) + } + objs := make([]client.Object, len(tt.hypervisors)) + for i := range tt.hypervisors { + objs[i] = &tt.hypervisors[i] + } + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(objs...). + Build() + + step := &FilterAggregateMetadata{} + step.Client = fakeClient + + result, err := step.Run(slog.Default(), tt.request) + if err != nil { + t.Fatalf("expected no error, got %v", err) + } + + for _, host := range tt.expectedHosts { + if _, ok := result.Activations[host]; !ok { + t.Errorf("expected host %s to be present in activations", host) + } + } + + for _, host := range tt.filteredHosts { + if _, ok := result.Activations[host]; ok { + t.Errorf("expected host %s to be filtered out", host) + } + } + + if len(result.Activations) != len(tt.expectedHosts) { + t.Errorf("expected %d hosts, got %d", len(tt.expectedHosts), len(result.Activations)) + } + }) + } +} + +func TestFilterAggregateMetadata_Run_ClientError(t *testing.T) { + request := api.ExternalSchedulerRequest{ + Spec: api.NovaObject[api.NovaSpec]{ + Data: api.NovaSpec{ + ProjectID: "project-a", + }, + }, + Hosts: []api.ExternalSchedulerHost{ + {ComputeHost: "host1"}, + }, + } + + scheme := runtime.NewScheme() + if err := hv1.AddToScheme(scheme); err != nil { + t.Fatalf("failed to add hv1 scheme: %v", err) + } + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithInterceptorFuncs(interceptor.Funcs{ + List: func(ctx context.Context, client client.WithWatch, list client.ObjectList, opts ...client.ListOption) error { + return context.Canceled + }, + }). + Build() + + step := &FilterAggregateMetadata{} + step.Client = fakeClient + + _, err := step.Run(slog.Default(), request) + if err == nil { + t.Errorf("expected error when client fails, got none") + } +} + +func TestFilterAggregateMetadata_IndexRegistration(t *testing.T) { + factory, ok := Index["filter_aggregate_metadata"] + if !ok { + t.Fatal("expected filter_aggregate_metadata to be registered in Index") + } + filter := factory() + if _, ok := filter.(*FilterAggregateMetadata); !ok { + t.Errorf("expected factory to return *FilterAggregateMetadata, got %T", filter) + } +} From c3b2db6ef0cef56327a69cdc8611262db2d4fc83 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Tue, 24 Mar 2026 16:45:12 +0000 Subject: [PATCH 12/12] Bump cortex chart appVersions to sha-88946405 [skip ci] --- helm/library/cortex/Chart.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/helm/library/cortex/Chart.yaml b/helm/library/cortex/Chart.yaml index a53b5d0aa..caf0030ea 100644 --- a/helm/library/cortex/Chart.yaml +++ b/helm/library/cortex/Chart.yaml @@ -3,6 +3,6 @@ name: cortex description: A Helm chart to distribute cortex. type: application version: 0.0.31 -appVersion: "sha-23f03412" +appVersion: "sha-88946405" icon: "https://example.com/icon.png" dependencies: []