From 5f8fb0ca354ecd312734f40e6deb24406e4bf99e Mon Sep 17 00:00:00 2001 From: mblos Date: Tue, 24 Mar 2026 16:10:11 +0100 Subject: [PATCH 1/3] Update commitments to use sapcc/go-api-declarations with support for non-standard units (waiting for release) --- go.mod | 2 +- go.sum | 2 ++ .../scheduling/reservations/commitments/api_info.go | 12 +++++++++++- 3 files changed, 14 insertions(+), 2 deletions(-) diff --git a/go.mod b/go.mod index 8cca91f86..6d67940c4 100644 --- a/go.mod +++ b/go.mod @@ -72,7 +72,7 @@ require ( github.com/poy/onpar v0.3.5 // indirect github.com/prometheus/common v0.67.5 // indirect github.com/prometheus/procfs v0.17.0 // indirect - github.com/sapcc/go-api-declarations v1.20.2 + github.com/sapcc/go-api-declarations v1.20.3-0.20260320144929-b2da5d83efdf github.com/sirupsen/logrus v1.9.3 // indirect github.com/spf13/cobra v1.10.1 // indirect github.com/spf13/pflag v1.0.10 // indirect diff --git a/go.sum b/go.sum index 22f64fec6..89fe99f93 100644 --- a/go.sum +++ b/go.sum @@ -176,6 +176,8 @@ github.com/rogpeppe/go-internal v1.14.1/go.mod h1:MaRKkUm5W0goXpeCfT7UZI6fk/L7L7 github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM= github.com/sapcc/go-api-declarations v1.20.2 h1:GWqv8VgsF4k9id6N051AVTaEpcjT02APsOuz2yCvTPQ= github.com/sapcc/go-api-declarations v1.20.2/go.mod h1:eiRrXXUeQS5C/1kKn8/KMjk0Y0goUzgDQswj30rH0Zc= +github.com/sapcc/go-api-declarations v1.20.3-0.20260320144929-b2da5d83efdf h1:/fRWRdee6F7tjqCGR7sIMQfVyWWdZ65fDQhvkS1DFK0= +github.com/sapcc/go-api-declarations v1.20.3-0.20260320144929-b2da5d83efdf/go.mod h1:eiRrXXUeQS5C/1kKn8/KMjk0Y0goUzgDQswj30rH0Zc= github.com/sapcc/go-bits v0.0.0-20260312170110-034b497ebb7e h1:4wgkrfAlnL6ffM7HTNoHn1HrBBurCRR71WNOszdiDNQ= github.com/sapcc/go-bits v0.0.0-20260312170110-034b497ebb7e/go.mod h1:NZjMiGVm04U25vwR6ZWvMw0XOOnvS1jkmXpjiepOeUw= github.com/sergi/go-diff v1.4.0 h1:n/SP9D5ad1fORl+llWyN+D6qoUETXNZARKjyY2/KVCw= diff --git a/internal/scheduling/reservations/commitments/api_info.go b/internal/scheduling/reservations/commitments/api_info.go index c189c859a..91d95786d 100644 --- a/internal/scheduling/reservations/commitments/api_info.go +++ b/internal/scheduling/reservations/commitments/api_info.go @@ -108,9 +108,19 @@ func (api *HTTPAPI) buildServiceInfo(ctx context.Context, logger logr.Logger) (l attrsJSON = nil } + // Build unit from smallest flavor memory (e.g., "131072 MiB" for 128 GiB) + unit, err := liquid.UnitMebibytes.MultiplyBy(groupData.SmallestFlavor.MemoryMB) + if err != nil { + logger.Error(err, "failed to create unit for flavor group", + "flavorGroup", groupName, + "smallestFlavorMemoryMB", groupData.SmallestFlavor.MemoryMB) + // Fall back to UnitNone if unit creation fails (should not happen in practice) + unit = liquid.UnitNone + } + resources[resourceName] = liquid.ResourceInfo{ DisplayName: displayName, - Unit: liquid.UnitNone, // Countable: multiples of smallest flavor instances + Unit: unit, // Non-standard unit: multiples of smallest flavor RAM Topology: liquid.AZAwareTopology, // Commitments are per-AZ NeedsResourceDemand: false, // Capacity planning out of scope for now HasCapacity: handlesCommitments, // We report capacity via /v1/report-capacity only for groups that accept commitments From b03a625a0b8dd6a0757fa6c5b5ada04bf61e8bdd Mon Sep 17 00:00:00 2001 From: mblos Date: Tue, 24 Mar 2026 17:15:39 +0100 Subject: [PATCH 2/3] Update commitments to use new liquid non-standard units --- cmd/main.go | 4 +- go.mod | 6 +- go.sum | 10 +- .../reservations/commitments/syncer.go | 64 +++++-- .../commitments/syncer_monitor.go | 63 +++++++ .../reservations/commitments/syncer_test.go | 165 ++++++++++++++++++ 6 files changed, 290 insertions(+), 22 deletions(-) create mode 100644 internal/scheduling/reservations/commitments/syncer_monitor.go diff --git a/cmd/main.go b/cmd/main.go index 351bf29eb..26a8ba661 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -665,7 +665,9 @@ func main() { if slices.Contains(mainConfig.EnabledTasks, "commitments-sync-task") { setupLog.Info("starting commitments syncer") - syncer := commitments.NewSyncer(multiclusterClient) + syncerMonitor := commitments.NewSyncerMonitor() + must.Succeed(metrics.Registry.Register(syncerMonitor)) + syncer := commitments.NewSyncer(multiclusterClient, syncerMonitor) syncerConfig := conf.GetConfigOrDie[commitments.SyncerConfig]() syncerDefaults := commitments.DefaultSyncerConfig() if syncerConfig.SyncInterval == 0 { diff --git a/go.mod b/go.mod index 6d67940c4..0f6cfe966 100644 --- a/go.mod +++ b/go.mod @@ -72,7 +72,7 @@ require ( github.com/poy/onpar v0.3.5 // indirect github.com/prometheus/common v0.67.5 // indirect github.com/prometheus/procfs v0.17.0 // indirect - github.com/sapcc/go-api-declarations v1.20.3-0.20260320144929-b2da5d83efdf + github.com/sapcc/go-api-declarations v1.21.0 github.com/sirupsen/logrus v1.9.3 // indirect github.com/spf13/cobra v1.10.1 // indirect github.com/spf13/pflag v1.0.10 // indirect @@ -98,7 +98,7 @@ require ( golang.org/x/oauth2 v0.34.0 // indirect golang.org/x/sync v0.19.0 // indirect golang.org/x/sys v0.42.0 // indirect - golang.org/x/term v0.41.0 // indirect + golang.org/x/term v0.41.0 golang.org/x/text v0.33.0 // indirect golang.org/x/time v0.14.0 // indirect gomodules.xyz/jsonpatch/v2 v2.5.0 // indirect @@ -108,7 +108,7 @@ require ( google.golang.org/protobuf v1.36.11 // indirect gopkg.in/evanphx/json-patch.v4 v4.13.0 // indirect gopkg.in/inf.v0 v0.9.1 // indirect - gopkg.in/yaml.v3 v3.0.1 // indirect + gopkg.in/yaml.v3 v3.0.1 gotest.tools v2.2.0+incompatible // indirect k8s.io/apiextensions-apiserver v0.35.0 // indirect k8s.io/apiserver v0.35.0 // indirect diff --git a/go.sum b/go.sum index 89fe99f93..17b7d4023 100644 --- a/go.sum +++ b/go.sum @@ -174,10 +174,8 @@ github.com/prometheus/procfs v0.17.0/go.mod h1:oPQLaDAMRbA+u8H5Pbfq+dl3VDAvHxMUO github.com/rogpeppe/go-internal v1.14.1 h1:UQB4HGPB6osV0SQTLymcB4TgvyWu6ZyliaW0tI/otEQ= github.com/rogpeppe/go-internal v1.14.1/go.mod h1:MaRKkUm5W0goXpeCfT7UZI6fk/L7L7so1lCWt35ZSgc= github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM= -github.com/sapcc/go-api-declarations v1.20.2 h1:GWqv8VgsF4k9id6N051AVTaEpcjT02APsOuz2yCvTPQ= -github.com/sapcc/go-api-declarations v1.20.2/go.mod h1:eiRrXXUeQS5C/1kKn8/KMjk0Y0goUzgDQswj30rH0Zc= -github.com/sapcc/go-api-declarations v1.20.3-0.20260320144929-b2da5d83efdf h1:/fRWRdee6F7tjqCGR7sIMQfVyWWdZ65fDQhvkS1DFK0= -github.com/sapcc/go-api-declarations v1.20.3-0.20260320144929-b2da5d83efdf/go.mod h1:eiRrXXUeQS5C/1kKn8/KMjk0Y0goUzgDQswj30rH0Zc= +github.com/sapcc/go-api-declarations v1.21.0 h1:Ag6GXgJLTFdBDKmrJU4QFllQbgGSenSGeHpLuvuxeDk= +github.com/sapcc/go-api-declarations v1.21.0/go.mod h1:eiRrXXUeQS5C/1kKn8/KMjk0Y0goUzgDQswj30rH0Zc= github.com/sapcc/go-bits v0.0.0-20260312170110-034b497ebb7e h1:4wgkrfAlnL6ffM7HTNoHn1HrBBurCRR71WNOszdiDNQ= github.com/sapcc/go-bits v0.0.0-20260312170110-034b497ebb7e/go.mod h1:NZjMiGVm04U25vwR6ZWvMw0XOOnvS1jkmXpjiepOeUw= github.com/sergi/go-diff v1.4.0 h1:n/SP9D5ad1fORl+llWyN+D6qoUETXNZARKjyY2/KVCw= @@ -251,12 +249,8 @@ golang.org/x/sync v0.19.0 h1:vV+1eWNmZ5geRlYjzm2adRgW2/mcpevXNg50YZtPCE4= golang.org/x/sync v0.19.0/go.mod h1:9KTHXmSnoGruLpwFjVSX0lNNA75CykiMECbovNTZqGI= golang.org/x/sys v0.0.0-20210616094352-59db8d763f22/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220715151400-c0bba94af5f8/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.40.0 h1:DBZZqJ2Rkml6QMQsZywtnjnnGvHza6BTfYFWY9kjEWQ= -golang.org/x/sys v0.40.0/go.mod h1:OgkHotnGiDImocRcuBABYBEXf8A9a87e/uXjp9XT3ks= golang.org/x/sys v0.42.0 h1:omrd2nAlyT5ESRdCLYdm3+fMfNFE/+Rf4bDIQImRJeo= golang.org/x/sys v0.42.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= -golang.org/x/term v0.39.0 h1:RclSuaJf32jOqZz74CkPA9qFuVTX7vhLlpfj/IGWlqY= -golang.org/x/term v0.39.0/go.mod h1:yxzUCTP/U+FzoxfdKmLaA0RV1WgE0VY7hXBwKtY/4ww= golang.org/x/term v0.41.0 h1:QCgPso/Q3RTJx2Th4bDLqML4W6iJiaXFq2/ftQF13YU= golang.org/x/term v0.41.0/go.mod h1:3pfBgksrReYfZ5lvYM0kSO0LIkAl4Yl2bXOkKP7Ec2A= golang.org/x/text v0.33.0 h1:B3njUFyqtHDUI5jMn1YIr5B0IE2U0qck04r6d4KPAxE= diff --git a/internal/scheduling/reservations/commitments/syncer.go b/internal/scheduling/reservations/commitments/syncer.go index b7db11351..ace9488b2 100644 --- a/internal/scheduling/reservations/commitments/syncer.go +++ b/internal/scheduling/reservations/commitments/syncer.go @@ -42,12 +42,15 @@ type Syncer struct { CommitmentsClient // Kubernetes client for CRD operations client.Client + // Monitor for metrics + monitor *SyncerMonitor } -func NewSyncer(k8sClient client.Client) *Syncer { +func NewSyncer(k8sClient client.Client, monitor *SyncerMonitor) *Syncer { return &Syncer{ CommitmentsClient: NewCommitmentsClient(), Client: k8sClient, + monitor: monitor, } } @@ -58,7 +61,16 @@ func (s *Syncer) Init(ctx context.Context, config SyncerConfig) error { return nil } -func (s *Syncer) getCommitmentStates(ctx context.Context, log logr.Logger, flavorGroups map[string]compute.FlavorGroupFeature) ([]*CommitmentState, error) { +// getCommitmentStatesResult holds both processed and skipped commitment UUIDs +type getCommitmentStatesResult struct { + // states are the commitments that were successfully processed + states []*CommitmentState + // skippedUUIDs are commitment UUIDs that were skipped (e.g., due to unit mismatch) + // but should NOT have their existing CRDs deleted + skippedUUIDs map[string]bool +} + +func (s *Syncer) getCommitmentStates(ctx context.Context, log logr.Logger, flavorGroups map[string]compute.FlavorGroupFeature) (*getCommitmentStatesResult, error) { allProjects, err := s.ListProjects(ctx) if err != nil { return nil, err @@ -69,7 +81,10 @@ func (s *Syncer) getCommitmentStates(ctx context.Context, log logr.Logger, flavo } // Filter for compute commitments with RAM flavor group resources - var commitmentStates []*CommitmentState + result := &getCommitmentStatesResult{ + states: []*CommitmentState{}, + skippedUUIDs: make(map[string]bool), + } for id, commitment := range commitments { if commitment.ServiceType != "compute" { log.Info("skipping non-compute commitment", "id", id, "serviceType", commitment.ServiceType) @@ -99,6 +114,29 @@ func (s *Syncer) getCommitmentStates(ctx context.Context, log logr.Logger, flavo continue } + // Validate unit matches between Limes commitment and Cortex flavor group + // Expected format: " MiB" e.g. "131072 MiB" for 128 GiB + expectedUnit := fmt.Sprintf("%d MiB", flavorGroup.SmallestFlavor.MemoryMB) + if commitment.Unit != "" && commitment.Unit != expectedUnit { + // Unit mismatch: Limes has not yet updated this commitment to the new unit. + // Skip this commitment - trust what Cortex already has stored in CRDs. + // On the next sync cycle after Limes updates, this will be processed. + log.V(0).Info("WARNING: skipping commitment due to unit mismatch - Limes unit differs from Cortex flavor group, waiting for Limes to update", + "commitmentUUID", commitment.UUID, + "flavorGroup", flavorGroupName, + "limesUnit", commitment.Unit, + "expectedUnit", expectedUnit, + "smallestFlavorMemoryMB", flavorGroup.SmallestFlavor.MemoryMB) + if s.monitor != nil { + s.monitor.RecordUnitMismatch(flavorGroupName) + } + // Track skipped commitment so its existing CRDs won't be deleted + if commitment.UUID != "" { + result.skippedUUIDs[commitment.UUID] = true + } + continue + } + // Skip commitments with empty UUID if commitment.UUID == "" { log.Info("skipping commitment with empty UUID", @@ -121,10 +159,10 @@ func (s *Syncer) getCommitmentStates(ctx context.Context, log logr.Logger, flavo "amount", commitment.Amount, "totalMemoryBytes", state.TotalMemoryBytes) - commitmentStates = append(commitmentStates, state) + result.states = append(result.states, state) } - return commitmentStates, nil + return result, nil } // SyncReservations fetches commitments from Limes and synchronizes Reservation CRDs. @@ -158,7 +196,7 @@ func (s *Syncer) SyncReservations(ctx context.Context) error { } // Get all commitments as states - commitmentStates, err := s.getCommitmentStates(ctx, logger, flavorGroups) + commitmentResult, err := s.getCommitmentStates(ctx, logger, flavorGroups) if err != nil { logger.Error(err, "failed to get compute commitments") return err @@ -168,7 +206,7 @@ func (s *Syncer) SyncReservations(ctx context.Context) error { manager := NewReservationManager(s.Client) // Apply each commitment state using the manager - for _, state := range commitmentStates { + for _, state := range commitmentResult.states { logger.Info("applying commitment state", "commitmentUUID", state.CommitmentUUID, "projectID", state.ProjectID, @@ -194,11 +232,15 @@ func (s *Syncer) SyncReservations(ctx context.Context) error { return err } - // Build set of commitment UUIDs we should have + // Build set of commitment UUIDs we should have (processed + skipped) activeCommitments := make(map[string]bool) - for _, state := range commitmentStates { + for _, state := range commitmentResult.states { activeCommitments[state.CommitmentUUID] = true } + // Also include skipped commitments - don't delete their CRDs + for uuid := range commitmentResult.skippedUUIDs { + activeCommitments[uuid] = true + } // Delete reservations for commitments that no longer exist for _, existing := range existingReservations.Items { @@ -221,6 +263,8 @@ func (s *Syncer) SyncReservations(ctx context.Context) error { } } - logger.Info("synced reservations", "commitmentCount", len(commitmentStates)) + logger.Info("synced reservations", + "processedCount", len(commitmentResult.states), + "skippedCount", len(commitmentResult.skippedUUIDs)) return nil } diff --git a/internal/scheduling/reservations/commitments/syncer_monitor.go b/internal/scheduling/reservations/commitments/syncer_monitor.go new file mode 100644 index 000000000..0bbc7fe7f --- /dev/null +++ b/internal/scheduling/reservations/commitments/syncer_monitor.go @@ -0,0 +1,63 @@ +// Copyright SAP SE +// SPDX-License-Identifier: Apache-2.0 + +package commitments + +import ( + "github.com/prometheus/client_golang/prometheus" +) + +// SyncerMonitor provides metrics for the commitment syncer. +type SyncerMonitor struct { + syncRuns prometheus.Counter + syncErrors prometheus.Counter + unitMismatch *prometheus.CounterVec +} + +// NewSyncerMonitor creates a new monitor with Prometheus metrics. +func NewSyncerMonitor() *SyncerMonitor { + m := &SyncerMonitor{ + syncRuns: prometheus.NewCounter(prometheus.CounterOpts{ + Name: "cortex_committed_resource_syncer_runs_total", + Help: "Total number of commitment syncer runs", + }), + syncErrors: prometheus.NewCounter(prometheus.CounterOpts{ + Name: "cortex_committed_resource_syncer_errors_total", + Help: "Total number of commitment syncer errors", + }), + unitMismatch: prometheus.NewCounterVec(prometheus.CounterOpts{ + Name: "cortex_committed_resource_syncer_unit_mismatch_total", + Help: "Total number of commitments with unit mismatch between Limes and Cortex flavor group knowledge", + }, []string{"flavor_group"}), + } + return m +} + +// RecordUnitMismatch records a unit mismatch for a flavor group. +func (m *SyncerMonitor) RecordUnitMismatch(flavorGroup string) { + m.unitMismatch.WithLabelValues(flavorGroup).Inc() +} + +// RecordSyncRun records a syncer run. +func (m *SyncerMonitor) RecordSyncRun() { + m.syncRuns.Inc() +} + +// RecordSyncError records a syncer error. +func (m *SyncerMonitor) RecordSyncError() { + m.syncErrors.Inc() +} + +// Describe implements prometheus.Collector. +func (m *SyncerMonitor) Describe(ch chan<- *prometheus.Desc) { + m.syncRuns.Describe(ch) + m.syncErrors.Describe(ch) + m.unitMismatch.Describe(ch) +} + +// Collect implements prometheus.Collector. +func (m *SyncerMonitor) Collect(ch chan<- prometheus.Metric) { + m.syncRuns.Collect(ch) + m.syncErrors.Collect(ch) + m.unitMismatch.Collect(ch) +} diff --git a/internal/scheduling/reservations/commitments/syncer_test.go b/internal/scheduling/reservations/commitments/syncer_test.go index 75512299a..4c438ea40 100644 --- a/internal/scheduling/reservations/commitments/syncer_test.go +++ b/internal/scheduling/reservations/commitments/syncer_test.go @@ -429,6 +429,171 @@ func TestSyncer_SyncReservations_UpdateExisting(t *testing.T) { } } +func TestSyncer_SyncReservations_UnitMismatch(t *testing.T) { + scheme := runtime.NewScheme() + if err := v1alpha1.AddToScheme(scheme); err != nil { + t.Fatalf("Failed to add scheme: %v", err) + } + + // Create flavor group knowledge CRD with smallest flavor of 1024MB + flavorGroupsKnowledge := createFlavorGroupKnowledge(t, map[string]FlavorGroupData{ + "test_group_v1": { + LargestFlavorName: "test-flavor-large", + LargestFlavorVCPUs: 8, + LargestFlavorMemoryMB: 8192, + SmallestFlavorName: "test-flavor-small", + SmallestFlavorVCPUs: 2, + SmallestFlavorMemoryMB: 1024, + }, + }) + + k8sClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(flavorGroupsKnowledge). + Build() + + // Create mock commitment with a unit that doesn't match Cortex's understanding + // Limes says "2048 MiB" but Cortex's smallest flavor is 1024 MB + mockCommitments := []Commitment{ + { + ID: 1, + UUID: "unit-mismatch-test-uuid", + ServiceType: "compute", + ResourceName: "ram_test_group_v1", + AvailabilityZone: "az1", + Amount: 2, + Unit: "2048 MiB", // Mismatched unit - should be "1024 MiB" + ProjectID: "test-project", + DomainID: "test-domain", + }, + } + + // Create monitor to capture the mismatch metric + monitor := NewSyncerMonitor() + + mockClient := &mockCommitmentsClient{ + listCommitmentsByIDFunc: func(ctx context.Context, projects ...Project) (map[string]Commitment, error) { + result := make(map[string]Commitment) + for _, c := range mockCommitments { + result[c.UUID] = c + } + return result, nil + }, + listProjectsFunc: func(ctx context.Context) ([]Project, error) { + return []Project{ + {ID: "test-project", DomainID: "test-domain", Name: "Test Project"}, + }, nil + }, + } + + syncer := &Syncer{ + CommitmentsClient: mockClient, + Client: k8sClient, + monitor: monitor, + } + + err := syncer.SyncReservations(context.Background()) + if err != nil { + t.Errorf("SyncReservations() error = %v", err) + return + } + + // Verify that NO reservations were created due to unit mismatch + // The commitment is skipped and Cortex trusts existing CRDs + var reservations v1alpha1.ReservationList + err = k8sClient.List(context.Background(), &reservations) + if err != nil { + t.Errorf("Failed to list reservations: %v", err) + return + } + + // Should have 0 reservations - commitment is skipped due to unit mismatch + // Cortex waits for Limes to update the unit before processing + if len(reservations.Items) != 0 { + t.Errorf("Expected 0 reservations (commitment skipped due to unit mismatch), got %d", len(reservations.Items)) + } +} + +func TestSyncer_SyncReservations_UnitMatch(t *testing.T) { + scheme := runtime.NewScheme() + if err := v1alpha1.AddToScheme(scheme); err != nil { + t.Fatalf("Failed to add scheme: %v", err) + } + + // Create flavor group knowledge CRD with smallest flavor of 1024MB + flavorGroupsKnowledge := createFlavorGroupKnowledge(t, map[string]FlavorGroupData{ + "test_group_v1": { + LargestFlavorName: "test-flavor-large", + LargestFlavorVCPUs: 8, + LargestFlavorMemoryMB: 8192, + SmallestFlavorName: "test-flavor-small", + SmallestFlavorVCPUs: 2, + SmallestFlavorMemoryMB: 1024, + }, + }) + + k8sClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(flavorGroupsKnowledge). + Build() + + // Create mock commitment with correct unit matching Cortex's smallest flavor + mockCommitments := []Commitment{ + { + ID: 1, + UUID: "unit-match-test-uuid", + ServiceType: "compute", + ResourceName: "ram_test_group_v1", + AvailabilityZone: "az1", + Amount: 2, + Unit: "1024 MiB", // Correct unit matching smallest flavor + ProjectID: "test-project", + DomainID: "test-domain", + }, + } + + monitor := NewSyncerMonitor() + + mockClient := &mockCommitmentsClient{ + listCommitmentsByIDFunc: func(ctx context.Context, projects ...Project) (map[string]Commitment, error) { + result := make(map[string]Commitment) + for _, c := range mockCommitments { + result[c.UUID] = c + } + return result, nil + }, + listProjectsFunc: func(ctx context.Context) ([]Project, error) { + return []Project{ + {ID: "test-project", DomainID: "test-domain", Name: "Test Project"}, + }, nil + }, + } + + syncer := &Syncer{ + CommitmentsClient: mockClient, + Client: k8sClient, + monitor: monitor, + } + + err := syncer.SyncReservations(context.Background()) + if err != nil { + t.Errorf("SyncReservations() error = %v", err) + return + } + + // Verify that reservations were created + var reservations v1alpha1.ReservationList + err = k8sClient.List(context.Background(), &reservations) + if err != nil { + t.Errorf("Failed to list reservations: %v", err) + return + } + + if len(reservations.Items) != 2 { + t.Errorf("Expected 2 reservations, got %d", len(reservations.Items)) + } +} + func TestSyncer_SyncReservations_EmptyUUID(t *testing.T) { scheme := runtime.NewScheme() if err := v1alpha1.AddToScheme(scheme); err != nil { From 5fb62aba84f84a869071b0203e5ad7f73fed8ac7 Mon Sep 17 00:00:00 2001 From: mblos Date: Tue, 24 Mar 2026 17:31:08 +0100 Subject: [PATCH 3/3] fix logging id --- .../scheduling/reservations/commitments/context.go | 6 ++++++ .../reservations/commitments/controller.go | 13 +++++++------ .../scheduling/reservations/scheduler_client.go | 1 + 3 files changed, 14 insertions(+), 6 deletions(-) diff --git a/internal/scheduling/reservations/commitments/context.go b/internal/scheduling/reservations/commitments/context.go index 64257fcae..aabee0241 100644 --- a/internal/scheduling/reservations/commitments/context.go +++ b/internal/scheduling/reservations/commitments/context.go @@ -21,6 +21,12 @@ func WithNewGlobalRequestID(ctx context.Context) context.Context { return reservations.WithGlobalRequestID(ctx, "committed-resource-"+uuid.New().String()) } +// WithGlobalRequestID creates a new context with the specified global request ID. +// This is used to propagate existing request IDs (e.g., from the creator annotation). +func WithGlobalRequestID(ctx context.Context, greq string) context.Context { + return reservations.WithGlobalRequestID(ctx, greq) +} + // LoggerFromContext returns a logger with greq and req values from the context. // This creates a child logger with the request tracking values pre-attached, // so you don't need to repeat them in every log call. diff --git a/internal/scheduling/reservations/commitments/controller.go b/internal/scheduling/reservations/commitments/controller.go index e03958151..71530cb59 100644 --- a/internal/scheduling/reservations/commitments/controller.go +++ b/internal/scheduling/reservations/commitments/controller.go @@ -49,20 +49,21 @@ type CommitmentReservationController struct { // move the current state of the cluster closer to the desired state. // 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) - - // Fetch the reservation object. + // Fetch the reservation object first to check for creator request ID. var res v1alpha1.Reservation if err := r.Get(ctx, req.NamespacedName, &res); err != nil { // Ignore not-found errors, since they can't be fixed by an immediate requeue return ctrl.Result{}, client.IgnoreNotFound(err) } - // Extract creator request ID from annotation for end-to-end traceability + // Use creator request ID from annotation for end-to-end traceability if available, + // otherwise generate a new one for this reconcile loop. if creatorReq := res.Annotations[v1alpha1.AnnotationCreatorRequestID]; creatorReq != "" { - logger = logger.WithValues("creatorReq", creatorReq) + ctx = WithGlobalRequestID(ctx, creatorReq) + } else { + ctx = WithNewGlobalRequestID(ctx) } + logger := LoggerFromContext(ctx).WithValues("component", "controller", "reservation", req.Name) // filter for CR reservations resourceName := "" diff --git a/internal/scheduling/reservations/scheduler_client.go b/internal/scheduling/reservations/scheduler_client.go index e89628b34..db7829e23 100644 --- a/internal/scheduling/reservations/scheduler_client.go +++ b/internal/scheduling/reservations/scheduler_client.go @@ -118,6 +118,7 @@ func (c *SchedulerClient) ScheduleReservation(ctx context.Context, req ScheduleR Context: api.NovaRequestContext{ RequestID: RequestIDFromContext(ctx), GlobalRequestID: globalReqID, + ProjectID: req.ProjectID, }, Spec: api.NovaObject[api.NovaSpec]{ Data: api.NovaSpec{